revamp vp formats #500
Merged
revamp vp formats #500
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TimoGlastra
force-pushed
the
revamp-vp-formats
branch
from
c5818cd to
080ba3a
Compare
TimoGlastra
force-pushed
the
revamp-vp-formats
branch
from
080ba3a to
d2edbe6
Compare
TimoGlastra
commented
Apr 4, 2025
Sakurann
requested review from
GarethCOliver,
awoie,
babisRoutis,
bc-pi,
c2bo,
danielfett,
hlozi,
jogu,
paulbastian,
tlodderstedt and
tplooker
Sakurann
reviewed
Apr 5, 2025
Member
Author
|
@OR13 given your experience with DataIntegrityProof, does the new ldp_vp syntax with proof_type_values and cryptosuite_values makes sense? (I will update the outdated examples) |
OR13
approved these changes
Apr 5, 2025
|
Looks ok to me. |
danielfett
approved these changes
Apr 5, 2025
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: Timo Glastra <timo@animo.id>
TimoGlastra
force-pushed
the
revamp-vp-formats
branch
from
75f526e to
e25a5d0
Compare
Signed-off-by: Timo Glastra <timo@animo.id>
c2bo
approved these changes
Apr 6, 2025
Signed-off-by: Timo Glastra <timo@animo.id>
Sakurann
reviewed
Apr 11, 2025
paulbastian
reviewed
Apr 11, 2025
paulbastian
reviewed
Apr 11, 2025
Collaborator
|
WG discussion.
|
Sakurann
reviewed
Apr 11, 2025
| The `vp_formats_supported` parameter of the Verifier metadata or Wallet metadata MUST have the Credential Format Identifier as a key, and the value MUST be an object consisting of the following name/value pairs: | ||
|
|
||
| * `issuer_signed_alg_values`: OPTIONAL. A JSON array containing fully-specified identifiers of cryptographic algorithms (as defined in [@!I-D.ietf-jose-fully-specified-algorithms]) supported for an `IssuerSigned` CBOR structure of an mdoc. If present, the `alg` COSE header (as defined in [@!RFC8152]) of the `IssuerSigned` structure of the presented mdoc MUST match one of the array values. If the `IssuerSigned` structure is not signed with a fully-specified cryptographic algorithm identifier (commonly known as a polymoprhic cryptographic algorithm identifier), the fully-specified algorithm derived from the combination of the `alg` value from the COSE header and the `crv` parameter from the signing key MUST match one of the array values. | ||
| * `device_signed_alg_values`: OPTIONAL. A JSON array containing fully-specified identifiers of cryptographic algorithms (as defined in [@!I-D.ietf-jose-fully-specified-algorithms]) supported for an `DeviceSigned` CBOR structure of an mdoc. If present, the `alg` COSE header (as defined in [@!RFC8152]) of the `DeviceSigned` structure of the presented mdoc MUST match one of the array values. If the `DeviceSigned` structure is not signed with a fully-specified cryptographic algorithm identifier (commonly known as a polymoprhic cryptographic algorithm identifier), the fully-specified algorithm derived from the combination of the `alg` value from the COSE header and the `crv` parameter from the signing key MUST match one of the array values. |
Collaborator
There was a problem hiding this comment.
mdoc device signed can be digitally signed or HMAC-ed. for HMAC-ed device signed, agreed to define identifiers in openid4vp specification
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Member
Author
|
Okay so i understand what needs to happen before merge:
|
Sakurann
reviewed
Apr 14, 2025
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Sakurann
requested review from
Sakurann,
babisRoutis,
martijnharing and
paulbastian
Collaborator
|
@TimoGlastra sorry if notes were not clear enough
|
danielfett
approved these changes
Apr 15, 2025
Sakurann
approved these changes
Apr 15, 2025
Signed-off-by: Timo Glastra <timo@animo.id>
Member
Author
|
I updated the PR based on the feedback and main. With the merging of the vc and vp formats for W3C I also merged the |
Collaborator
|
wg discussion: ok to merge once @c2bo re-reviews |
c2bo
approved these changes
Apr 16, 2025
Co-authored-by: Christian Bormann <8774236+c2bo@users.noreply.github.com>
Sakurann
dismissed
martijnharing’s stale review
upon a verbal agreement that Martijn is ok for this PR to be merged conditional to his remaining comments around device_signed_alg_values are addressed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #488
Fixes #495
Fixes #487
Dependant on #479 (it started from that branch due to the conflicts, once that PR is merged i will rebase). You can see the changes from this PR here: c5818cdvp_formatsandvp_formats_supportedfor each formatvp_formatsandvp_formats_supported.cryptosuiteto make it easier to query/define supported schemes (since most proofs will useDataIntegrityProofNOTE: this has breaking changes, but I think the previous definitions were really underspecified (and leaning on PEX), and not consistent. So I think it's worth it to make these changes before 1.0