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

Add IANA registration for JWS types #533

Merged
merged 4 commits into from
Mar 9, 2024
Merged

Conversation

jricher
Copy link
Collaborator

@jricher jricher commented Mar 8, 2024

No description provided.

Copy link

netlify bot commented Mar 8, 2024

Deploy Preview for gnap-core-protocol-editors-draft ready!

Name Link
🔨 Latest commit 618e3e8
🔍 Latest deploy log https://app.netlify.com/sites/gnap-core-protocol-editors-draft/deploys/65eb863c6f2b1200081d44b8
😎 Deploy Preview https://deploy-preview-533--gnap-core-protocol-editors-draft.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

To indicate that the content is a GNAP message to be bound with a detached JWS mechanism:

* Type name: application
* Subtype name: gnap-binding+jwsd
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't have examples using this anymore, will it be clear how it is used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Suggested change
* Subtype name: gnap-binding+jwsd
* Subtype name: gnap-binding+jws

Copy link

@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.

jwsd is unnecessary and should be deleted. Per https://www.rfc-editor.org/rfc/rfc7515.html#appendix-F, you can tell whether the payload is detached by whether the payload field is empty.


To indicate the content is a compact form JWS as defined by {{RFC7515}} with detached payload:

* Name: Detached JWS

Choose a reason for hiding this comment

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

jwsd is unnecessary and should be deleted. Per https://www.rfc-editor.org/rfc/rfc7515.html#appendix-F, you can tell whether the payload is detached or not by whether the payload field is empty or not.

* Name: JWS
* +suffix: +jws
* References: {{&SELF}}
* Encoding considerations: binary; JWS values are a series of base64url-encoded values (some of which may be the empty string) separated by period ('.') characters.

Choose a reason for hiding this comment

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

Suggested change
* Encoding considerations: binary; JWS values are a series of base64url-encoded values (some of which may be the empty string) separated by period ('.') characters.
* Encoding considerations: binary; JWS values using the compact serialization are a series of base64url-encoded values (some of which may be the empty string) separated by period ('.') characters.

To indicate that the content is a GNAP message to be bound with a detached JWS mechanism:

* Type name: application
* Subtype name: gnap-binding+jwsd

Choose a reason for hiding this comment

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

Suggested change
* Subtype name: gnap-binding+jwsd
* Subtype name: gnap-binding+jws

To indicate that the content is a GNAP token rotation message to be bound with a detached JWS mechanism:

* Type name: application
* Subtype name: gnap-binding-rotation+jwsd

Choose a reason for hiding this comment

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

Suggested change
* Subtype name: gnap-binding-rotation+jwsd
* Subtype name: gnap-binding-rotation+jws


## Structured Syntax Suffix Registration

This section requests registration of the "+jws" and "+jwsd" structured syntax suffixes in

Choose a reason for hiding this comment

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

Suggested change
This section requests registration of the "+jws" and "+jwsd" structured syntax suffixes in
This section requests registration of the "+jws" structured syntax suffix in

@jricher
Copy link
Collaborator Author

jricher commented Mar 8, 2024

@selfissued I'm fine with changing the values here, but the question is more what the appropriate containment is at the protocol level. I don't believe the signaling you are referring to works in this case. There are two JOSE-based mechanisms defined by GNAP, one uses JWS as the message payload, effectively replacing the JSON with its own type -- that's the "Attached JWS" payload. The other keeps the payload as-is (JSON) but uses JWS to create a detached signature. The payload of the JWS then becomes a hash of the content instead of the content itself. Within GNAP these are signaled by separate field values, but shouldn't they be separate typ values as well, as per the best practice RFCs?

If we remove +jwsd as a suffix, would you then recommend that the distinction move to the root types instead? If we go that route, I think we would probably just remove the subtypes entirely. So instead of gnap-binding+jwsd we'd have gnap-binding-jwsd, instead of gnap-binding+jws we'd have gnap-binding-jws. Would that address your concerns?

@selfissued
Copy link

Your suggestion above not using structured suffixes would be fine. Or use something like gnap-normal-binding+jws and gnap-detached-binding+jws (feel free to pick more appropriate names), if you need to make the distinction at the Content-Type level, rather than after you have the content, if you still want the media type to indicate that the content is a JWS using the Compact Serialization. Your call.

The core of my review is to not create a separate jwsd media type and structured suffix.

Oh, and to be clear in the registration text that +jws is about the Compact Serialization.

@jricher jricher merged commit 4076a1b into main Mar 9, 2024
4 checks passed
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.

3 participants