oauth-wg / oauth-sd-jwt-vc

draft-terbu-sd-jwt-vc
Creative Commons Zero v1.0 Universal
20 stars 12 forks source link

Clarify the optionality of the cnf claim #213

Closed bc-pi closed 8 months ago

bc-pi commented 9 months ago

Clarify the optionality of the cnf claim (to fix #196)

bc-pi commented 9 months ago

Hang on. Even if there might be a difference in how a key inside cnf claim is represented (jwk, kid, etc) it should still be cnf claim for the interop purposes, no? What are you trying to account for by making cnf optional?

please see issue #196 and note that the text in this PR has "This [cnf] claim MUST be present when cryptographic Key Binding is to be supported."

Also note this PR isn't for issue #205 and it was probably a mistake on my part to have suggested text there that touches two issues. But they were both on cnf so I did what I did [no pun intended]. This PR only attempts to clarify the optionality of the cnf claim for issue #196. There doesn't yet seem to be consensus on #205.

bc-pi commented 9 months ago

https://drafts.oauth.net/oauth-sd-jwt-vc/clarify-cnf-optionality/draft-ietf-oauth-sd-jwt-vc.html#section-3.2.2.2-3.4.1

Sakurann commented 9 months ago

@bc-pi you probably made the comment while I was updating my original comment based on re-reading the PR. I still think it should remain "required when cryptographic binding is used"

Sakurann commented 9 months ago

CNF is more optional to me

what does this mean? you believe sd-jwt-vc should have more than one way to do cryptographic holder binding? or that many of the credentials will not use cryptographic holder binding?

paulbastian commented 9 months ago

No, I just mean SD-JWT VCs can be key bound, but they don't need to be. Therefore it is an optional feature.

paulbastian commented 9 months ago

I see, the difference here is that if key binding shall be used, then cnf must be used while the other formulation leaves more space?

Sakurann commented 9 months ago

If that's the intent, required when cryptographic binding is used sounds more appropriate to me. just saying optional would lead to confusion that it is optional even when cryptographic binding is used.

awoie commented 9 months ago

I also like the proposed version more. The proposed language addresses #196 which is based on developer feedback that were confused by the current language that uses REQUIRED if ... .

So I think that OPTIONAL seems to solve that issue. IMO, it is also appropriate since we have the additional requirement in the paragraph that says MUST be present if cryptographic binding is required.

Sakurann commented 9 months ago

if key binding shall be used, then cnf must be used while the other formulation leaves more space?

Sorry, I don't understand why more space is needed here? The less optionality in key binding, more interop. with the current PR, it would be ok to use a top level sub claim and put a DID there (instead of using cnf.kid). I think it is harmful for the interop, that SD-JWT VC is hoped to bring

awoie commented 9 months ago

if key binding shall be used, then cnf must be used while the other formulation leaves more space?

Sorry, I don't understand why more space is needed here? The less optionality in key binding, more interop. with the current PR, it would be ok to use a top level sub claim and put a DID there (instead of using cnf.kid). I think it is harmful for the interop, that SD-JWT VC is hoped to bring

I don't see that this optionality is allowed by the following?

This claim MUST be present when cryptographic Key Binding is to be supported.

paulbastian commented 9 months ago

if key binding shall be used, then cnf must be used while the other formulation leaves more space?

Sorry, I don't understand why more space is needed here? The less optionality in key binding, more interop. with the current PR, it would be ok to use a top level sub claim and put a DID there (instead of using cnf.kid). I think it is harmful for the interop, that SD-JWT VC is hoped to bring

I see your point. The proposed text opens a door for key binding without using the cnf claim. However, the new text is more initiative and we could add this constraint as an additional sentence or in the section on key binding

paulbastian commented 9 months ago

Okay, it is already present. Sorry I didn't recheck the changes. Lgtm then

bc-pi commented 9 months ago

I'm struggling to understand the disagreement/argument here. But I'm trying...

The text for cnf in the PR has:

OPTIONAL. Contains the confirmation method identifying the proof of possession key as defined in [@!RFC7800]. This claim MUST be present when cryptographic Key Binding is to be supported. It is RECOMMENDED that this contains a JWK as defined in Section 3.2 of [@!RFC7800]. For proof of cryptographic Key Binding, the Key Binding JWT in the presentation of the SD-JWT MUST be signed by the key identified in this claim.

which seems to me to clearly says that cnf is the required way to do key binding, if doing key binding. Especially the "This claim MUST be present when cryptographic Key Binding is to be supported" sentence.

Would changing that sentence to "This claim is REQUIRED when cryptographic Key Binding is to be supported" help? Maybe I'm missing the point of contention but that and the current PR text and the current text in the draft all say the same thing - that cnf is the one and only way to do key binding in SD-JWT VC but b/c key binding isn't itself required, the claim also isn't required.

bc-pi commented 9 months ago

Would changing that sentence to "This claim is REQUIRED when cryptographic Key Binding is to be supported" help?

I thought that was better phrasing anyway so made the change w/ 9edae9d4aa1dc2b647c77029b7d415d384cded49

awoie commented 8 months ago

Would changing that sentence to "This claim is REQUIRED when cryptographic Key Binding is to be supported" help?

I thought that was better phrasing anyway so made the change w/ 9edae9d

Seems great to me.

bc-pi commented 8 months ago

I'm generally fine with the change, but I think I want to be pedantic here and resolve the contradiction.

You're suggested pedantic wording is indeed better than what I'd had. Thanks. I've accepted it w/ c6b138036e035e6df2a2f983aa4cf8b183d39fc2