Changing JARM references to JWT directly (Issue #463) #477
Conversation
Sakurann
requested review from
Sakurann,
awoie,
babisRoutis,
bc-pi,
c2bo,
danielfett,
davidz25,
hlozi,
jogu,
martijnharing,
paulbastian,
tlodderstedt and
tplooker
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Co-authored-by: Oliver Terbu <o.terbu@gmail.com>
Co-authored-by: Oliver Terbu <o.terbu@gmail.com>
Co-authored-by: Oliver Terbu <o.terbu@gmail.com>
|
|
||
| `response_mode`: | ||
| : REQUIRED. Defined in [@!OAuth.Responses]. This parameter is used (through the new Response Mode `direct_post`) to ask the Wallet to send the response to the Verifier via an HTTPS connection (see (#response_mode_post) for more details). It is also used to request signing and encrypting (see (#jarm) for more details). | ||
| : REQUIRED. Defined in [@!OAuth.Responses]. This parameter is used (through the new Response Mode `direct_post`) to ask the Wallet to send the response to the Verifier via an HTTPS connection (see (#response_mode_post) for more details). It is also used to request encrypting (see (#response_encryption) for more details). |
There was a problem hiding this comment.
This paragraph currently says "is used" and "is also used". This should be changed to "can be used".
There was a problem hiding this comment.
but those are two uses that are both required. so i think it is also used is beter. "to request encrypting" is not an optional use of this parameter, right?
There was a problem hiding this comment.
Don't have a strong opinion here, I think 'can be used' is a bit better (as it can also be used for dc_api for instance). If we get consensus can make the change, but given it is existing text unrelated to the change I'd otherwise punt to a follow-up.
| This specification also defines a new Response Mode `direct_post.jwt`, which allows for encryption to be used with Response Mode `direct_post` defined in (#response_mode_post). | ||
|
|
||
| The Response Mode `direct_post.jwt` causes the Wallet to send the Authorization Response using an HTTP POST request instead of redirecting back to the Verifier as defined in (#response_mode_post). The Wallet adds the `response` parameter containing the JWT as defined in Section 4.1. of [@!JARM] and (#jarm) in the body of an HTTP POST request using the `application/x-www-form-urlencoded` content type. The names and values in the body MUST be encoded using UTF-8. | ||
| The Response Mode `direct_post.jwt` causes the Wallet to send the Authorization Response using an HTTP POST request instead of redirecting back to the Verifier as defined in (#response_mode_post). The Wallet adds the `response` parameter containing the JWT as defined in (#response_encryption) in the body of an HTTP POST request using the `application/x-www-form-urlencoded` content type. The names and values in the body MUST be encoded using UTF-8. |
There was a problem hiding this comment.
Do we also define the "response" parameter in the context of the DC API? If not, where should be add that?
There was a problem hiding this comment.
yeah, this section https://openid.github.io/OpenID4VP/openid-4-verifiable-presentations-wg-draft.html#appendix-A.4, right?
(this comment is probably out of scope of this PR)
| * clarify that `dcql_query` and `presentation_definition` are passed as JSON objects (not strings) in request objects | ||
| * support returning multiple presentations for a single dcql credential query when requested using `multiple` | ||
| * Added support for multiple Client Identifiers and corresponding Request Signature to the DC API profile | ||
| * remove JARM and response signing, using JWT directly for unsigned, encrypted responses. |
There was a problem hiding this comment.
i can move this to -26 in #497 or you can do it here
There was a problem hiding this comment.
I went ahead and moved it with 6335eb8. Apologies for jumping into your fork/branch @GarethCOliver, I'm gonna try and do the updates coming out of the meeting yesterday.
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
bc-pi
left a comment
bc-pi
left a comment
There was a problem hiding this comment.
thanks @GarethCOliver for addressing my prior comments, LGTM now.
|
|
||
| - `iss`, `exp` and `aud` MUST be omitted in the JWT Claims Set of the JWE, and the processing rules as per [@!JARM] Section 2.4 related to these claims do not apply. | ||
| - The processing rules as per [@!JARM] Section 2.4 related to JWS processing MUST be ignored. | ||
| The Verifier's public key for input into the key agreement SHOULD be obtained from the `jwks` claim within the `client_metadata` request parameter, within the metadata defined in the Entity Configuration when [@!OpenID.Federation] is used, or other mechanisms. The Wallet MAY select any key for which encryption is a valid operation. |
There was a problem hiding this comment.
WG discussion. enhance The Wallet MAY select any key for which encryption is a valid operation. to address #483 and close #438 .
clarify
- how to know which key to use if multiple keys are present in jwks
- why multiple keys are allowed.
- another PR already clarified
kidbehavior Requirekidfromjwksto be included in JWE response header #478 - if non-jwks mechanism is used, you're on your own :)
| This section defines how an Authorization Response containing a VP Token can be encrypted at the application level when the Response Type value is `vp_token` or `vp_token id_token`. Encrypting the Authorization Response can, for example, prevent personal data in the Authorization Response from leaking. For security considerations see (#encrypting_unsigned_response). | ||
|
|
||
| If the JWT is only a JWE, the following processing rules MUST be followed: | ||
| To encrypt the Authorization Response, implementations MUST use an unsigned, encrypted JWT as described in [@!RFC7519]. The `enc` algorithm used MUST be obtained from `authorization_encrypted_response_enc` claim within the `client_metadata`. The `alg` algorithm used MUST be the `alg` value of the chosen `jwk`. |
There was a problem hiding this comment.
I think the last sentence is not clear enough, we mean a key from the jwks claim within client_metadata as written in the next section?
There was a problem hiding this comment.
"the chosen jwk" is not easy to understand, as it has not been mentioned before I believe
There was a problem hiding this comment.
yeah, fair. Maybe remove it from this section and place it after the key-selection text (so it's a more clear)?
There was a problem hiding this comment.
I think this all touches on clarifications I (was) volunteered to make yesterday kinda per #477 (comment) but I'll definitely try to address this kind of point. Hopefully a bit later today. Probably as suggestions on this PR.
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
| If a Wallet is unable to generate a JARM response, it MAY send an error response without using JARM as per (#response_mode_post). | ||
| If a Wallet is unable to generate an encrypted response, it MAY send an error response as per (#response_mode_post). | ||
|
|
||
| The following is a non-normative example of a response using the `presentation_submission` and `vp_token` values from (#jwt_vc). (line breaks for display purposes only): |
There was a problem hiding this comment.
a "direct_post.jwt" example of a response using the presentation_submission and vp_token values... with PE having gone away and this having been a JWS rather than JWE... and merge conflicts but not here. I'm not sure I have the skills to make the mostly agreed on changes in the context of this PR. Might have to start a new one borrowing from Gareth's work as reference.
Co-authored-by: Brian Campbell <71398439+bc-pi@users.noreply.github.com>
3 approvals carried over from #477. request for changes to #477 has been addressed in this PR. WG consensus to merge it during the WG meeting. people are encouraged to re-read and file issues on this topic during WGLC/public review period. there will be a follow up PR adding examples. few issues might follow.
8 participants
Removes the JARM option for response signing, referencing JWT encryption directly
This continues to use the JARM client_metadata parameters, for compatibility and because we can not simply re-define them. How they are used for response encryption is updated and the option for signing removed.
This closes #463, closes #438