Changes to additional SD-JWT VC requirements #165
Conversation
selfissued
left a comment
selfissued
left a comment
There was a problem hiding this comment.
We should not do this, for the reasons described at #164 (comment) .
|
I think that |
There was a problem hiding this comment.
The normal time claims in JWT-based tokens are iat and exp. `nbf is only useful if the token is future dated - meaning that it is not valid when issued. This is an odd corner case that we should discourage, at best.
Including nbf would introduce unnecessary corner cases, harming interoperability. It has no place in an interoperability profile.
|
@joelposti, to answer to your comment #164 (comment), you can refer to the ETSI TS 119 472-1 V0.0.6 that was made available for public review a few weeks ago: https://docbox.etsi.org/esi/Open/Latest_Drafts/ETSI%20DRAFT%20TS_119_472-1v0.0.6-public.pdf . Indeed, what is mandated in ETSI are
It is clear that |
|
I pushed three new commits according to the discussion in this pull request. I also rebased the branch of this pull request. I also changed the title of this pull request from ‘Add |
joelposti
changed the title
nbf claim
Sakurann
requested review from
bc-pi,
c2bo,
danielfett,
jogu and
martijnharing
|
@joelposti It would be good if you could update both the title and the contents of the initial comment please - they don't seem aligned with the actual PR content anymore! |
joelposti
changed the title
|
There is currently a conflict with the | cnf | MUST | [@!RFC7800]|to | cnf | MUST if the corresponding Credential Configuration requires cryptographic holder binding | [@!RFC7800]|Shall we go ahead with removing the table including the above addition to the * 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` member. |
| * 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. | ||
| * It is at the discretion of the Issuer whether to use `exp` claim and/or a `status` claim to express the validity period of an SD-JWT VC. The Wallet and the Verifier MUST support both mechanisms. | ||
| * The `iss` claim, if present, MUST be an HTTPS URL. |
There was a problem hiding this comment.
I don't actually understand the reason why we require it to be a https url if we don't define any situation where the iss claim is actually processed by the verifier/wallet? (See also my questions in #247 )
There was a problem hiding this comment.
This and https://openid.github.io/OpenID4VC-HAIP/openid4vc-high-assurance-interoperability-profile-wg-draft.html#section-7.1 basically, i think, say that the x509 mechanism is required but one could also support the JWT VC Issuer Metadata mechanism at the same time https://datatracker.ietf.org/doc/html/draft-ietf-oauth-sd-jwt-vc-10#name-issuer-signature-mechanisms but constrains it from being something else/more than that.
There was a problem hiding this comment.
Ah, thanks!
That seems kind of pointless to me given issuer metadata only seems able to hold keys, which would seem redundant if x509 based keys are required to be available and everyone is required to be able to validate via x5c - not sure if I'm missing something.
If iss is present and is a https url, is it optional or required for an /.well-known/jwt-vc-issuer based on that url to actually exist?
There was a problem hiding this comment.
If
issis present and is a https url, is it optional or required for an/.well-known/jwt-vc-issuerbased on that url to actually exist?
From my understanding, it is not required, but HAIP could make it required.
However, as web based key resolution was removed from HAIP, there wouldn't really be any benefit to adding such a requirement.
Web based key resolution is supposed to make it simpler for Verifiers to verify the signature as there doesn't need to be pre-sharing of keys/certificates, they simply need to trust the Issuer url.
There was a problem hiding this comment.
I don't know tbh but I don't think it implies a requirement that /.well-known/jwt-vc-issuer exist. Only that, the iss claim, if present, MUST be an HTTPS URL. Which means it cannot be data:image/png;base64,iVBORw0KGgoAAA ANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg== or did:ffs:abc123 or mysupercoolissuername or whatever.
My personal take is that adding that 'if the corresponding Credential Configuration requires cryptographic holder binding' clarification into the |
| * The `iss` claim MUST be an HTTPS URL. | ||
| * 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. | ||
| * It is at the discretion of the Issuer whether to use `exp` claim and/or a `status` claim to express the validity period of an SD-JWT VC. The Wallet and the Verifier MUST support both mechanisms. |
There was a problem hiding this comment.
This seems redundant. I suggest removing it but not a hill I would die on.
| * It is at the discretion of the Issuer whether to use `exp` claim and/or a `status` claim to express the validity period of an SD-JWT VC. The Wallet and the Verifier MUST support both mechanisms. |
There was a problem hiding this comment.
I like the additional clarity that the bullet point brings since both exp and status are optional in SD-JWT VC. Without the bullet point there is nothing that requires status implementation on the part of wallets and verifiers which might result in spotty support.
There was a problem hiding this comment.
I'd tend to agree, I think in particular that MUST is not covered elsewhere.
I am not sure if the current language means "issuer can choose to use neither exp nor status" or if it means "issuer has to use at least one of exp & status"? That might be best handled in a new issue though.
|
@joelposti I think we're likely good to merge this now, but it's got conflicts from one of the other recently merged PRs - sorry - could you try and resolve them please? |
…t` in section ‘Validity Period of the Signature and the Claim Values’ with `nbf`.
…idity period of both the signature and the claims about the subject, unless there is a separate claim used to express the validity of the claims.’ as agreed in the pull request. Removed the whole section ‘Validity Period of the Signature and the Claim Values’ as the aforementioned removed sentence was all it contained.
…tiate issuance & presentation)’ bullet point and a claim table following it from section ‘SD-JWT VCs’ according to discussion in the pull request. Removed ‘The Issuer MUST NOT make any of the JWT Claims in the table above to be selectively disclosable, so that they are always present in the SD-JWT-VC presented by the Holder.’ bullet point because it did not make sense after removing the table. Added a new bullet point ‘Claims containing time-related information, such as issuance or expiration dates, MUST be either individually randomized within an appropriate time window (e.g., within the last 24 hours), or rounded (e.g., to the start of the day), to avoid unintended correlation factors.’ as requested in the pull request.
…TTPS URL.’ as suggested in the pull request. Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>
… such as issuance or expiration dates, MUST be either individually randomized within an appropriate time window (e.g., within the last 24 hours), or rounded (e.g., to the start of the day), to avoid unintended correlation factors.’ as requested in the pull request discussion. Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
…tf-oauth-sd-jwt-vc].’ as it did not add anything to what is already defined in the SD-JWT VC spec.
…ether to use `exp` claim and/or a `status` claim to express the validity period of an SD-JWT-VC. The Wallet and the verifier MUST support both mechanisms.’ according to suggestions in the pull request. Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>
…er suggestion in the pull request. Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>
…raphic holder binding’ to the `cnf`/`jwk` bullet point to resolve a conflict with the `main` branch where the cited part had been added to the claim table that is removed in this branch. Co-authored-by: Joseph Heenan <joseph@heenan.me.uk>
|
Hello @jogu and others! I rebased the branch to fix the conflicts with the |
|
I think this should be good to go but realised we still have request changes from @scvenema and @selfissued - I've just sent them messages on signal asking if they can re-review. |
13 participants
Changes to additional SD-JWT VC requirements.
The pull request initially proposed solving #164 by adding
nbfclaim requirements. The discussion and the commits added into this pull request steered this pull request towards being a more general changeset for the additional SD-JWT VC requirements.