Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why serialize the baseProof and derivedProof values #329

Closed
jeswr opened this issue Jan 4, 2025 · 4 comments
Closed

Why serialize the baseProof and derivedProof values #329

jeswr opened this issue Jan 4, 2025 · 4 comments
Assignees
Labels
CR2 This item was processed during the second Candidate Recommendation Phase editorial This issue or PR constitutes an editorial change. pr exists A pull request exists to address this issue.

Comments

@jeswr
Copy link
Member

jeswr commented Jan 4, 2025

I'm trying to understand the need to serialise and parse proofValues in the ecdsa-sd-2023 functions. In particular, I am trying to understand the need for defining:

Which results in encoding a bunch of semantics within one literal value of the JSON-LD document; as opposed to making the proofValue take the form:

   ...
   proofValue: {
      baseSignature: ...,
      pubKey: ...,
      hmacKey: ...,
      signatures: ...,
      mandatoryPointers: ...,
   }
   ...

The string isn't encrypted, so it isn't for privacy - so the only other two justifications that I can land on are:

  • compression or,
  • the VC Data Model requiring string proofValues and not wanting to modify the spec given the current stage it is at in its lifecycle & the fact that there are existing implementations
@dlongley
Copy link
Contributor

dlongley commented Jan 9, 2025

The details of the data produced by individual cryptographic suites were intentionally kept isolated / encapsulated from other application-specific data. This data is intended to be only consumed by the security layer and shouldn't be handled / managed by the application as individual components -- with a presumption that doing so might even some pose some security / privacy risk. A similar design is taken with the multikey expression of key material as a single block of bytes at the application layer vs. the alternative of directly expressing individual key components such as elliptic curve coordinates, private / public exponents, etc.

This approach is also somewhat informed by various efforts in the security community to add safer abstractions that expose only higher level APIs / direct data access. The idea is to improve on some of the problems that have occurred with other approaches that expose every low-level detail or element in the same layer as other application data or that exposed too many low-level interfaces without a clear indication of how their safe use depends on a particular composition. These low-level approaches have also had the drawback of the creation of superfluous, potentially incorrect, and highly specific validation code running at more layers (that should be agnostic to the data) than just the security layer.

So the approach is intentional and, regardless, you are correct that it's not something that could be changed at this stage of the process even if there were consensus to change it.

@msporny msporny added the editorial This issue or PR constitutes an editorial change. label Jan 14, 2025
@msporny msporny transferred this issue from w3c/vc-di-ecdsa Jan 14, 2025
@msporny msporny added CR2 This item was processed during the second Candidate Recommendation Phase ready for pr This issue is ready to be resolved via a pull request labels Jan 14, 2025
@msporny
Copy link
Member

msporny commented Jan 14, 2025

We should write up the reasoning in #329 (comment) in the Data Integrity specification (if we haven't done so already). I have transferred this issue from vc-di-ecdsa to vc-data-integrity.

@msporny
Copy link
Member

msporny commented Jan 18, 2025

PR #330 has been raised to address this issue. This issue will be closed once PR #330 has been merged.

@msporny msporny added pr exists A pull request exists to address this issue. and removed ready for pr This issue is ready to be resolved via a pull request labels Jan 18, 2025
@msporny
Copy link
Member

msporny commented Jan 26, 2025

PR #330 has been merged, closing.

@msporny msporny closed this as completed Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR2 This item was processed during the second Candidate Recommendation Phase editorial This issue or PR constitutes an editorial change. pr exists A pull request exists to address this issue.
Projects
None yet
Development

No branches or pull requests

4 participants