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

Distinguish SD-JWT from SD-JWT+KB #394

Merged
merged 38 commits into from
Jun 12, 2024
Merged

Conversation

bifurcation
Copy link
Contributor

@bifurcation bifurcation commented Dec 5, 2023

This PR limits the scope of the term "SD-JWT" so that it refers only to an Issuer-signed JWT and a set of disclosures. We introduce the term "Fnord" to refer to an SD-JWT with key binding. As a bonus, we are able to clean up stale uses of Presentation, which mostly, but not always align with Fnord.

"Fnord" is deliberately nonsensical, of course. We should replace this with a more intuitive word. We could reintroduce "Presentation" for this, with the allowance that one might "present" either a Presentation or an SD-JWT. Or we could pick another term, suggestions welcome.

I have tried to be fairly minimal in the changes here. In particular, I have not changed any of the algorithms, though I suspect they would be clearer if a bit more separate. I have rewritten a few paragraphs, especially close to the top and in the Security Considerations.

Fixes #374
Fixes #392

@OR13
Copy link
Contributor

OR13 commented Dec 6, 2023

Its very confusing when communicating regarding SD-JWT.

You really can't ever use the term by itself.

You must use the term with:

"Issuer Signed SD-JWT" and "Holder Disclosed SD-JWT".

If you don't make this clear, its easy to imagine scenarios where implementers accidentally disclose more than they should.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

I think the terminology direction is good, details will need to be sorted out.

I will add a wrinkle, that "presentation" is sorta "owned" by W3C, and it means something every different.

In W3C terms a "presentation" is a special kind of RDF Graph Fragment, with or without integrity.

You can make "integrity protected" "presentations" with an SD-JWT.... and they look like this today:

I'd recommend choosing another word than presentation at this point, it will be safer than trying to square terms with W3C.

Here are some alternatives to "presentation disclosures with integrity and key binding" to consider:

@Sakurann
Copy link
Collaborator

I find it immensely challenging to read and review this PR with Fnord being used as a term. please replace it with some specific term that we can bikeshed.

@Sakurann
Copy link
Collaborator

Sakurann commented Feb 8, 2024

@bifurcation can you please replace Fnord with SD-JWT/KB, which is a short form for SD-JWT with Key Binding (SD-JWT+KB could have worked too, but we were just worried it will be confused with media types, etc.), so that reviewing is easier.

@bifurcation
Copy link
Contributor Author

@Sakurann - Changed to SD-JWT-KB, since the SD-JWT/KB looked weird to me (division seemed like the wrong metaphor) and SD-JWT~KB seemed a little too cute. Also merged master while I was at it.

Copy link
Member

@c2bo c2bo left a comment

Choose a reason for hiding this comment

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

Not entirely sure about the SD-JWT-KB vs SD-JWT+KB part. Should the kb-jwt then also be renamed to kb+jwt (even outside the typ)?

draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@bc-pi bc-pi left a comment

Choose a reason for hiding this comment

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

pining for the fnords [edit: this was my comment on submitting approval]

Co-authored-by: Christian Bormann <8774236+c2bo@users.noreply.github.com>
@bc-pi
Copy link
Collaborator

bc-pi commented May 3, 2024

Not entirely sure about the SD-JWT-KB vs SD-JWT+KB part. Should the kb-jwt then also be renamed to kb+jwt (even outside the typ)?

I'm gonna say no.

Co-authored-by: Michael B. Jones <michael_b_jones@hotmail.com>
Copy link
Collaborator

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

Thanks for the updates.

I'd appreciate it if you'd merge my other suggestion, which is a clarification.

selfissued and others added 2 commits May 3, 2024 12:56
Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>
@bc-pi
Copy link
Collaborator

bc-pi commented May 15, 2024

@bifurcation and @danielfett and @Sakurann to hopefully do a re-review soon(ish) and get this one merged soon(ish)

Copy link
Member

@danielfett danielfett left a comment

Choose a reason for hiding this comment

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

I left some editorial suggestions.

I'll note that we need to make a few changes to the section on JWS JSON Serialization as well, but I filed a separate issue for that: #426

draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@bc-pi bc-pi left a comment

Choose a reason for hiding this comment

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

comments on comments (am I doing this right?)

draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Outdated Show resolved Hide resolved
draft-ietf-oauth-selective-disclosure-jwt.md Show resolved Hide resolved
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
@Sakurann
Copy link
Collaborator

Thank you, Richard "fnord" Barnes

@Sakurann Sakurann merged commit 9641acb into oauth-wg:master Jun 12, 2024
1 check passed
@bc-pi
Copy link
Collaborator

bc-pi commented Jun 12, 2024

Thank you, Richard "fnord" Barnes

It's the little things that keep me going

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants