-
Notifications
You must be signed in to change notification settings - Fork 37
Add encryption details parameter #597
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
Conversation
Examples to be added if this approach is accepted
|
We would need to add IANA registration requests for both parameters in their respective registries I assume? |
This needs registration in two IANA registries:
I suggest adding the corresponding registrations to the IANA section. |
Sakurann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see #response_encryption for details of how to include it in the JWE in the response)
i think text on the response is missing in #response_encryption section?
Co-authored-by: Oliver Terbu <o.terbu@gmail.com>
awoie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following two statements allow for a gap in implementations that lead to broken flows, right?
the Verifier MUST verify that one of the referenced profiles is included in the response
AND
the selected profile MAY be included in the JWE protected header using the
encryption_detailsheader
Why would the latter be a MAY/SHOULD, if it would break the verifier?
selfissued
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not specified in sufficient detail to enable interoperable implementations.
What are the parameters intended to be passed with this parameter and what are the processing rules for them?
|
@martijnharing Generally fine with this PR but just had a question/suggestion above. |
|
Would it make sense to define a "profile" name for the current/default behaviour? |
Change MAY to SHOULD for encryption_details header parameter Co-authored-by: Oliver Terbu <o.terbu@gmail.com>
How about: |
That works for me |
| * `jwks`: OPTIONAL. A JSON Web Key Set, as defined in [@!RFC7591], that contains one or more public keys, such as those used by the Wallet as an input to a key agreement that may be used for encryption of the Authorization Response (see (#response_encryption)), or where the Wallet will require the public key of the Verifier to generate a Verifiable Presentation. This allows the Verifier to pass ephemeral keys specific to this Authorization Request. Public keys included in this parameter MUST NOT be used to verify the signature of signed Authorization Requests. Each JWK in the set MUST have a `kid` (Key ID) parameter that uniquely identifies the key within the context of the request. | ||
| * `encrypted_response_enc_values_supported`: OPTIONAL. Non-empty array of strings, where each string is a JWE [@!RFC7516] `enc` algorithm that can be used as the content encryption algorithm for encrypting the Response. When a `response_mode` requiring encryption of the Response (such as `dc_api.jwt` or `direct_post.jwt`) is specified, this MUST be present for anything other than the default single value of `A128GCM`. Otherwise, this SHOULD be absent. | ||
| * `vp_formats_supported`: REQUIRED when not available to the Wallet via another mechanism. As defined in (#client_metadata_parameters). | ||
| * `encryption_details_supported`: OPTIONAL. A non-empty array of strings, where each string references a profile of encryption details that the Verifier supports. The meaning and expected behavior of the Verifier and Wallet is profile-specific and out of scope of this specification. When the Verifier includes this parameter in a request, the Wallet MUST either reject the request or include one of the referenced profiles in the response and implement all of the requirements defined by the referenced profile (see (#response_encryption) for details of how to include it in the JWE in the response). When the Verifier includes this parameter in a request, the Verifier MUST verify that one of the referenced profiles is included in the response and MUST implement all of the requirements required by the profile included in the response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WG discussion:
- define a behavior of not explicitly specifying apu/apv values aka a string that can be included in
encruption_details_supportedarray - @c2bo to make a suggestion for an exact wording on how we describe "default behavior"
- some people questioning if this needs to be added at all, and whether it adds an unnecessary complication for wallets and the verifiers (@leecam @bc-pi @ve7jtb etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would very much like to understand what value is actually added by introducing an under-defined but interoperability hampering extension point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- @c2bo to make a suggestion for an exact wording on how we describe "default behavior"
I'd furthermore argue that there has to be a way to indicate that the "default behavior" is also an acceptable referenced profile. Otherwise there's going to be a rather sharp interoperability edge.
bc-pi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As others have noted, precedent and consistency would require that these new things have IANA registration requests for both in their respective registries. Writing those IANA requests and taking them to the designated experts and IANA might underscore the rather tenuous nature of the problem and solution here.
bc-pi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As others have noted, precedent and consistency would require that these new things have IANA registration requests for both in their respective registries. Writing those IANA requests and taking them to the designated experts and IANA might underscore the rather tenuous nature of the problem and solution here.
Sorry for the double double comment. Undoubtedly user error on my part. The same user that can't seem to find how to delete a comment. Sorry. |
| If the selected public key contains a `kid` parameter, the JWE MUST include the same value in the `kid` JWE Header Parameter (as defined in [@!RFC7516, Section 4.1.6]) of the encrypted response. This enables the Verifier to easily identify the specific public key that was used to encrypt the response. | ||
| The JWE `enc` content encryption algorithm used is obtained from the `encrypted_response_enc_values_supported` parameter of client metadata, such as the `client_metadata` request parameter, allowing for the default value of `A128GCM` when not explicitly set. | ||
|
|
||
| When the `encryption_details_supported` parameter is present in the `client_metadata` parameter of the request (see #new_parameters) and requires the Wallet to include a referenced profile in the response, the selected profile SHOULD be included in the JWE protected header using the `encryption_details` header. When present, the `encryption_details` header MUST contain a string of the selected profile. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why wouldn't indicating the profile used be required? what value is there in allowing for that ambiguity? that complicates following the "MUST implement all of the requirements required by the profile included in the response" above
| When the `encryption_details_supported` parameter is present in the `client_metadata` parameter of the request (see #new_parameters) and requires the Wallet to include a referenced profile in the response, the selected profile SHOULD be included in the JWE protected header using the `encryption_details` header. When present, the `encryption_details` header MUST contain a string of the selected profile. | |
| When the `encryption_details_supported` parameter is present in the `client_metadata` parameter of the request (see #new_parameters) and requires the Wallet to include a referenced profile in the response, the selected profile MUST be included in the JWT header using the `encryption_details` header parameter. |
|
suggestion:
|
|
WG discussion:
|
|
WG discussion about defining apu/apv values for ECDH-ES JWE..
wg agreed to discuss and tackle #347 during wglc/review period and as an outcome this PR and PR #628 will be closed for the above reasons since there is no wg rough consensus to proceed with either of them. |
Resolves #347