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

first attempt of sd-jwt vcdm #134

Closed
wants to merge 4 commits into from
Closed

first attempt of sd-jwt vcdm #134

wants to merge 4 commits into from

Conversation

Sakurann
Copy link
Contributor

resolves #128

Comment on lines 270 to 274
## SD-JWT VCDM

SD-JWT VCDM MAY be used. SD-JWT VCDM is a data model that follows [@!I-D.ietf-oauth-sd-jwt-vc], but is compatible with the [@!W3C.VCDM2.0].

The following is a non-normative example of an unsecured payload of an SD-JWT VCDM, that is built using the example of unsecured payload in Section 3.3 of [@!I-D.ietf-oauth-sd-jwt-vc]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since SD-JWT VC DM is not a specification, I would say "in accordance with the SD-JWT VC DM proposal, " or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I don't think we should point to a private repo. so i am reluctant to mention SD-JWT VC DM proposal at all

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point is to define SD-JWT VCDM in HAIP, here, from scratch

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the authors contribute?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the sd-jwt vcdm being moved here? Was there an agreement with the authors?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of rewriting from scratch, I believe we should adapt the original idea to the new reality of the EUDI Wallet and ARF. This means ensuring compatibility with SD-JWT and W3C VC DM, while not leaving behind the use cases that have been developed in environments such as education or powers of representation (mainly within the EBSI ecosystem), and iterating.

I am convinced that together we can create an open profile for VCs that anyone can adopt to address the complex reality of use cases that will arise in the coming years.

Let’s open this line of work where the original authors can guide the development of the specification at a low level.

@@ -272,11 +267,79 @@ Note: The issuer MAY decide to support both options. In which case, it is at the

A Credential Format Profile for Credentials complying with IETF SD-JWT VCs [@!I-D.ietf-oauth-sd-jwt-vc] is defined in Annex A.3 of [@!OIDF.OID4VCI] and Annex A.4 of [@!OIDF.OID4VP].

## SD-JWT VCDM
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## SD-JWT VCDM
## SD-JWT VC Data Model (VCDM)

Comment on lines +310 to +311
"iss": "https://example.com/issuer",
"issuer": "https://example.com/issuer",
Copy link
Collaborator

@awoie awoie Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to not have these duplicates, e.g., changes to SD-JWT VC itself?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the jwt encoding for w3c vcs was definitely a lot simpler than the JSON-LD counterpart, the thing i disliked about it the most was the duplication of values and the issues it caused in validation. Every duplicated parameter introduces a chance of inconsistent data in the credential. So i agree with @awoie here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to not have these duplicates, e.g., changes to SD-JWT VC itself?

if we want a data structure that is compliant to ietf sd-jwt vc and w3c vcdm as-is, that is the only way I think?

@@ -272,11 +267,79 @@ Note: The issuer MAY decide to support both options. In which case, it is at the

A Credential Format Profile for Credentials complying with IETF SD-JWT VCs [@!I-D.ietf-oauth-sd-jwt-vc] is defined in Annex A.3 of [@!OIDF.OID4VCI] and Annex A.4 of [@!OIDF.OID4VP].

## SD-JWT VCDM
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should clearly say something about the goals of SD-JWT VCDM and who can consume it. For example, we should define what of the following scenarios are possible:

  • SD-JWT VC processor without JSON-LD processing
  • SD-JWT VC processor with JSON-LD processing
  • W3C VCDM 2.0 processor (which requires JSON-LD), with securing mechanism verification. This might be an issue because of the external context definitions will be required, and because SD-JWT VC has to be defined as a W3C VCDM 2.0 securing mechanism.
  • W3C VCDM 2.0 processor without securing mechanism verification; just do SD-JWT VC verification. This will also require external context definitions for all public/private JWT claims which are not defined in the W3C VCDM 2.0 context already.
  • any other options?

After this PR got merged: w3c/vc-data-model#1520, VCDM 2.0 requires all claims to be defined explicitly in dedicated @context definitions referenced by the @context array. This is required because all VCDM 2.0 credentials have to be JSON-LD compliant. Before this PR, this was not strictly required (although kind of recommended). Not having these properly defined in these context definitions would break VCDM 2.0 processors. The VCDM 2.0 context does have definitions for all the standard SD-JWT terms but does not have definitions of SD-JWT VC claims such as status, schema etc. Previously all these claims got added the default context as issuer-dependent terms via the "@vocab": "https://www.w3.org/ns/credentials/issuer-dependent#" statement in the W3C VCDM 2.0 context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to add a default vocab in HAIP but it should be noted that this MUST NOT be done if Data Integrity Proofs are used. I'm not sure yet if this is a good idea though.

However, we would need to use a different IRI for the value of @vocab since https://www.w3.org/ns/credentials/issuer-dependent#" does not go anywhere.

Co-authored-by: Timo Glastra <[email protected]>
Co-authored-by: Oliver Terbu <[email protected]>
@tlodderstedt
Copy link
Contributor

@Sakurann wouldn't we aim for VCDM 1.1 support?

@tlodderstedt
Copy link
Contributor

tlodderstedt commented Dec 3, 2024

Discussion during WG Call:

  • VCDM 1.1 and/or 2.0?
  • Are we aiming for VCDM compliance (would require 2.0 and definition of securing mechanisms ...) or support for implementers wanting to add selective disclosure to 1.1/2.0 VCs?

@Sakurann
Copy link
Contributor Author

Sakurann commented Dec 5, 2024

at least my intention was to allow using JSON-LD. i think we can do it in a way that does not require VCDM compliance, especially given 1.1/2.0 confusion. will try refactor the PR in that direction.

Copy link

@javereec javereec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review

@ssanchocanela
Copy link

That looks great to me.
I agree with @tlodderstedt that we should mention W3C VCDM 1.1 and point to 2.0. Current eIDAS2 IAs approved include W3C VCDM 1.1 but the intention looking at the roadmap is to replace it with 2.0 when ready.

* The `iss` claim MUST be an HTTPS URL. The `iss` value is used to obtain Issuer’s signing key as defined in (#issuer-key-resolution).
* The `vct` JWT claim as defined in [@!I-D.ietf-oauth-sd-jwt-vc].
* The `cnf` claim [@!RFC7800] MUST conform to the definition given in [@!I-D.ietf-oauth-sd-jwt-vc]. Implementations conforming to this profile MUST include the JSON Web Key [@!RFC7517] in the `jwk` sub claim.
* The `cnf` claim [@!RFC7800] MUST include the JSON Web Key [@!RFC7517] in the `jwk` sub claim.

Note: Currently this profile only supports presentation of credentials with cryptographic Holder Binding: the holder's signature is required to proof the credential is presented by the holder it was issued to. This profile might support claim-based and biometrics-based holder binding once OpenID for Verifiable Credentials adds support for other forms of Holder Binding. See https://bitbucket.org/openid/connect/issues/1537/presenting-vc-without-a-vp-using-openid4vp

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this section be removed? Feels like it's using old POV and references an old issue

@Sakurann
Copy link
Contributor Author

input that we would probably need to define sd-jwt vcdm for json serialization, too.

"vct": "https://credentials.example.com/identity_credential",

//W3C VCDM 2.0 compliant claims
"type": ["VerifiableCredential", "https://credentials.example.com/identity_credential"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this duplication since it is not required.

Suggested change
"type": ["VerifiableCredential", "https://credentials.example.com/identity_credential"],
"type": ["VerifiableCredential"],

@Sakurann
Copy link
Contributor Author

closing in favor of #147. will incorporate relevant comments on this PR into a new one

@Sakurann Sakurann closed this Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add sd-jwt vcdm to HAIP
8 participants