oauth-wg / oauth-selective-disclosure-jwt

https://datatracker.ietf.org/doc/draft-ietf-oauth-selective-disclosure-jwt/
Other
56 stars 30 forks source link

Remove initial underscore on `_sd_hash` #371

Closed bifurcation closed 10 months ago

bifurcation commented 10 months ago

It makes sense for _sd to start with an underscore, since it is being inserted into a JSON object supplied by the caller and it needs to be something unlikely to conflict with an existing claim.

The _sd_hash field appears in the key binding JWT, a format being defined by this document. So it can safely live without the underscore.

danielfett commented 10 months ago

That's a fair point, I think.

bifurcation commented 10 months ago

I think the same argument also applies to _sd_alg. It's just a normal JWT claim, so it can be deconflicted using the usual registry process.

bc-pi commented 10 months ago

Really they are all just JWT claims and will be registered. The leading underscore isn't magical and doesn't exempt from registration. But it is a nicety to help avoid conflict, which is particularly relevant with where/how _sd and _sd_alg are used. They both are in the issuer signed JWT where there is some potential for name conflicts with application supplied data (JWT's whole registered, public and private claims guidance is supposed to help here too but conflicts are still possible).

For general clarity and consistency, I agree (I think anyway) with removing the initial underscore on _sd_hash. But _sd_alg is more similar to _sd in usage/context and those two should keep their leading underscore.

bifurcation commented 10 months ago

To me the difference between _sd and _sd_alg seems to be that _sd_alg is at the top level of the JWT claims, where we're used to dealing with registered claims. But _sd can be down in the guts of the claims, which the JWT signer might have less control over. So I would go for _sd, but sd_hash and sd_alg.

bc-pi commented 10 months ago

Registered claims (I'm a so-called DE on there so I have some familiarity) don't fully address conflicting names even at the top level with the possibility of the use of private claim names. More could be said about that registry and what it does and doesn't provide in practice but that'd be a digression...

Anyway, that _sd and _sd_alg show up in the content of issuer-signed JWT is my point of vew for keeping the underscore on both. Also for keeping those two similar to each other.

Lastly note that _sd and _sd_alg have been in the draft for many revisions (back to -02 I think) whereas _sd_hash was just introduced in -06. Changing _sd_hash at this point likely wouldn't impact implementations much but changing _sd_alg would.

OR13 commented 10 months ago

I'd prefer to not see ANY public claim names that start with _.

It starts a convention which I would prefer not to see repeated.

selfissued commented 10 months ago

In OpenID Connect, we decided that our "meta-claims" would start with underscore to visually distinguish them from normal claims. For instance, see the claims _claim_names and _claim_sources in the registry at https://www.iana.org/assignments/jwt/jwt.xhtml.

I support SD-JWT continuing to follow this convention and so would prefer that the leading underscore be kept for its meta-claims.

bifurcation commented 10 months ago

@selfissued What is the difference between "normal claims" and "meta-claims" here? And why did OpenID Connect go that way?

selfissued commented 10 months ago

"Meta-claims" are claims about the structure of where actual claim values can be found - claim values that are not simply top-level members in the JWT Claims Set. They're road-signs saying where to go to get them - not the destinations themselves.

bifurcation commented 10 months ago

Thanks, that distinction makes sense. Applying that to SD-JWT, it seems directly applicable to _sd, and _sd_alg sort of fits as well, since it tells you how to read the road-signs (to continue @selfissued's metaphor). I'm OK with these keeping _.

But _sd_hash is not a road-sign, it's the main event. If you ignore _sd or _sd_alg, you miss out on some claims, but nothing fundamental breaks. If you ignore _sd_hash, you're not enforcing the key security properties of the system. So I think the original issue here is still valid.

bc-pi commented 10 months ago

PR https://github.com/oauth-wg/oauth-selective-disclosure-jwt/pull/387 would remove the leading underscore on _sd_hash (and only there). Which is the original issue here.