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

Mandate the use of apu/apv in the JWE header of OpenID4VP encrypted responses #380

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bc-pi
Copy link
Member

@bc-pi bc-pi commented Jan 10, 2025

Copy link
Member

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

Given that the nonce is part of the encrypted payload, it already contributes to the cryptographic output. Therefore also using it as the "apu" value is redundant.

Likewise, the Client ID is part of the encrypted content, and so need not also be in "apv".

@bc-pi
Copy link
Member Author

bc-pi commented Jan 10, 2025

Given that the nonce is part of the encrypted payload, it already contributes to the cryptographic output. Therefore also using it as the "apu" value is redundant.

Likewise, the Client ID is part of the encrypted content, and so need not also be in "apv".

Arguably none of this is needed or useful but this is my attempt to carry out the will of the WG in a compromise that isn't a complete departure from the specification stack this all builds on and hopefully put an end to months of confusing and confused but very heated discussion on the topic.

@tlodderstedt tlodderstedt added the ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API label Jan 13, 2025
Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

Approving this change since it implements what was agreed in the DCP WG call. I want to note that this will not work with the Multi RP PR #308 since the response might be encrypted to different client IDs. After the Multi RP RP got merged, this text will need to be revised accordingly.

@Sakurann
Copy link
Collaborator

@selfissued

Given that the nonce is part of the encrypted payload, it already contributes to the cryptographic output. Therefore also using it as the "apu" value is redundant.

Likewise, the Client ID is part of the encrypted content, and so need not also be in "apv".

the purpose of using nonce/client_id as apu/apv values is to ensure interoperability so that the verifier can decrypt, and NOT audience restriction or session binding

@bc-pi
Copy link
Member Author

bc-pi commented Jan 13, 2025

Approving this change since it implements what was agreed in the DCP WG call. I want to note that this will not work with the Multi RP PR #308 since the response might be encrypted to different client IDs. After the Multi RP RP got merged, this text will need to be revised accordingly.

Noted. Though I'm not sure what one response encrypted to different client IDs would look like...

Comment on lines +1133 to +1134
- The `apu` (Agreement PartyUInfo) header parameter MUST be set to the value of the `nonce` from the Authorization Request.
- The `apv` (Agreement PartyVInfo) header parameter MUST be set to the value of the `client_id` of the Verifier.
Copy link
Contributor

Choose a reason for hiding this comment

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

In ISO18013-7 the apv is the nonce instead of apu, does that make more or less sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

None of this makes much sense to me, to be honest. Per JWA - apu is information about the producer and apv is information about the recipient. ISO18013-7 has (or so I'm told) mdocGeneratedNonce for apu and nonce for apv which makes a certain amount of sense if you squint at it right. The wallet creates the mdocGeneratedNonce/apu so I guess it's information about the producer while the nonce/apv is generated by the verifier so I guess it's information about the recipient. If you squint at it a different way, it doesn't make any sense at all. I went with nonce as apu because it's about the the transaction or session the producer (Wallet) has with the Verifier and so seemed close enough to being about the producer and there are only two KDF contributing headers and client_id in apv made sense to me as information about the recipient because it's an identifier for the recipient.

That's a lot of words that probably don't make much sense either.

The upside though, as I said in #380 (comment), is that I don't think any of this is needed or useful.

@tlodderstedt
Copy link
Collaborator

Discussion in the WG call on the 14th of Jan 2025:

  • With the latest change in Add Multi RP Credentials/Authentication capability #308, we can no longer assume the encryption key to be related to a certain Client Identifier. We might change this into the origin. @paulbastian raised the question how the RP would determine the correct origin if the request contains multiple expected_origins.
  • @mike pointed out use of apv/apu would preclude algorithms that do not have those features.

@paulbastian
Copy link
Contributor

I think I agree with Mike, that defining new top-level parameters beside the JWE may be more future-proof than using apu and apv, as long as those things are not directly involved in the decryption itself

@awoie
Copy link
Contributor

awoie commented Jan 14, 2025

I think I agree with Mike, that defining new top-level parameters beside the JWE may be more future-proof than using apu and apv, as long as those things are not directly involved in the decryption itself

Tend to agree as well since all protected headers are part of the AAD which would include these newly defined headers. This would also meet the ISO requirement:

Response encryption authentication must be bound to the origin, e.g. RP URL.

@Sakurann
Copy link
Collaborator

so am I getting it right that the proposed solution is to add origin to the protected header?
since the requirement is Response encryption authentication must be bound to the origin, e.g. RP URL. I assume there is no need to add any other information to the protected header?

Does it also mean that we do not use apu and apv values? if we do not define them, there will be no way for both parties to be able to use the same values.

@bc-pi bc-pi closed this Jan 15, 2025
@bc-pi bc-pi reopened this Jan 15, 2025
@bc-pi
Copy link
Member Author

bc-pi commented Jan 15, 2025

the purpose of using nonce/client_id as apu/apv values is to ensure interoperability so that the verifier can decrypt, and NOT audience restriction or session binding

that was not my understanding but perhaps my not understanding is part of the problem

@bc-pi
Copy link
Member Author

bc-pi commented Jan 15, 2025

Discussion in the WG call on the 14th of Jan 2025:

* With the latest change in [Add Multi RP Credentials/Authentication capability #308](https://github.com/openid/OpenID4VP/pull/308), we can no longer assume the encryption key to be related to a certain Client Identifier. We might change this into the origin. @paulbastian raised the question how the RP would determine the correct origin if the request contains multiple `expected_origins`.

The rational for multiple expected_origins was questioned from it's introduction but it was explained that at IIW it was determined as needed because the backend infrastructure thing that signs these requests might not know the exact origin the whole page or whatever the script invoking the API is being served on. Which I can kinda see. But it'll have to know the set of possible origins in order to make the expected_origins list. I think an RP/Verifer would just need to ensure that the origin in the JWE header is one of those that it expects.

* @mike pointed out use of apv/apu would preclude algorithms that do not have those features.

I don't understand what that means exactly but what it seems to say on the face of things isn't correct.

@bc-pi
Copy link
Member Author

bc-pi commented Jan 15, 2025

I think I agree with Mike, that defining new top-level parameters beside the JWE may be more future-proof than using apu and apv, as long as those things are not directly involved in the decryption itself

I apologize for missing the call where this all was discussed and for my not being able to understand much of this but I don't understand how or why defining new top-level parameters beside the JWE would contribute anything to anything.

@bc-pi
Copy link
Member Author

bc-pi commented Jan 15, 2025

I think I agree with Mike, that defining new top-level parameters beside the JWE may be more future-proof than using apu and apv, as long as those things are not directly involved in the decryption itself

Tend to agree as well since all protected headers are part of the AAD which would include these newly defined headers. This would also meet the ISO requirement:

Response encryption authentication must be bound to the origin, e.g. RP URL.

"protected headers" are different than "top-level parameters beside the JWE" and maybe I'm being too literal but it's a meaningful distinction. Also apu and apv are protected headers. All headers are in the compact serialization (but please please don't take that as any kind of support for the JSON serialization). So yeah, the AAD would include these newly defined headers but it also includes the apu and apv headers (which also contribute to the KDF in ECDH). The text in the PR even tries to say this. So I am indeed sorry but I am pretty confused about what was discussed yesterday and what the concerns are and what to do with any of it.

@bc-pi
Copy link
Member Author

bc-pi commented Jan 15, 2025

so am I getting it right that the proposed solution is to add origin to the protected header? since the requirement is Response encryption authentication must be bound to the origin, e.g. RP URL. I assume there is no need to add any other information to the protected header?

Is this where the WG wants to take this? Asking 'cause I honestly don't know.

Does it also mean that we do not use apu and apv values? if we do not define them, there will be no way for both parties to be able to use the same values.

We don't define them now, they don't need to be used and default behavior when they aren't there is specified. So I'm not sure why it would be a problem. But I've very likely misunderstood something.

@Sakurann
Copy link
Collaborator

Sakurann commented Jan 16, 2025

outcome of the WG discussion: if the purpose of the ISO requirement ("Response encryption authentication must be bound to the origin, e.g. RP URL.") is replay prevention, there is already audience restriction via origin and client_id and nonce being present in the session transcript. if this is for HPKE and potentially detached ECDH-ES then DCP WG will tackle it later.

since this is about origin, this applies only for browser API.

two options were discussed, but in the course of the discussion, the WG realized that if the WG's understanding of the purpose of the ISO requirement is clear, there is no need for this PR.

Next step would be to close/update this PR conditional to the outcome of the virtual ISO mtg.


(Below options are documented for the sake of note-taking but they become irrelevant if the agreement is not to use apv/apu)

  • option 1: set apv or apu as origin value.
  • option 2: define a new protected header origin. do not define apv/apu values?
    -> Brian thought this might be less of an evil. should work with some of the JWE libraries Brian is intimately aware of.
    -> this might help to future proof for the algorithms that do not use apu/apv ie HPKE. but will prevent the goal of detached AAD because protected header is not detached?

Comment on lines +1153 to +1160
Whenever the response is encrypted, the following additional processing directives apply to JWE. This enables binding response encryption to the Verifier's identifier and its respective transaction with the End-User:

- The `apu` (Agreement PartyUInfo) header parameter MUST be set to the value of the `nonce` from the Authorization Request.
- The `apv` (Agreement PartyVInfo) header parameter MUST be set to the value of the `client_id` of the Verifier.

Note that for the ECDH JWE algorithms (from section 4.6 of [@!RFC7518]), the `apu` and `apv` values are inputs
into the key derivation process that is used to derive the content encryption key. However, regardless of algorithm used, the values are always part of the AEAD tag computation so will still be bound to the encrypted response.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Whenever the response is encrypted, the following additional processing directives apply to JWE. This enables binding response encryption to the Verifier's identifier and its respective transaction with the End-User:
- The `apu` (Agreement PartyUInfo) header parameter MUST be set to the value of the `nonce` from the Authorization Request.
- The `apv` (Agreement PartyVInfo) header parameter MUST be set to the value of the `client_id` of the Verifier.
Note that for the ECDH JWE algorithms (from section 4.6 of [@!RFC7518]), the `apu` and `apv` values are inputs
into the key derivation process that is used to derive the content encryption key. However, regardless of algorithm used, the values are always part of the AEAD tag computation so will still be bound to the encrypted response.
Note that for the ECDH JWE algorithms (from section 4.6 of [@!RFC7518]), the `apu` and `apv` values are inputs
into the key derivation process that is used to derive the content encryption key. Regardless of algorithm used, the values are always part of the AEAD tag computation so will still be bound to the encrypted response.

Copy link
Member Author

Choose a reason for hiding this comment

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

That'd be okay by me FWIW

@@ -2767,6 +2775,7 @@ The technology described in this specification was made available from contribut

-24

* Mandate the use of apu/apv in the JWE header of encrypted responses
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Mandate the use of apu/apv in the JWE header of encrypted responses
* add a note on the use of apu/apv in the JWE header of encrypted responses

also needs to be moved to -25

Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

after the virtual ISO mtg, there has been an agreement not to define apu/apv values to be client_id/nonce, etc.
option 1 is to close this PR.
option 2 is to refactor this PR to add a note on apv/apu.
I made suggestions for option 2 which could be slightly nicer, but do not have strong opinion

@martijnharing
Copy link

martijnharing commented Jan 30, 2025

RFC 7518 also says that:

Applications need to specify how the "apu" and "apv" Header
   Parameters are used for that application.  The "apu" and "apv" values
   MUST be distinct, when used.  Applications wishing to conform to
   [[NIST.800-56A](https://datatracker.ietf.org/doc/html/rfc7518#ref-NIST.800-56A)] need to provide values that meet the requirements of
   that document, e.g., by using values that identify the producer and
   consumer.

So I think we need to normatively specify how the apu and apv header are used (which could be that they must be empty). I'm not sure what the exact implications are for the requirements regarding NIST 800-56A. Is anyone familiar with making the use of ECDH-ES compliant to NIST 800-56A?

@awoie
Copy link
Contributor

awoie commented Jan 30, 2025

The "apu" and "apv" values
MUST be distinct, when used.

I think the important part of the sentence is "when used". Since we are not using them, it is up to more specific profiles to define these values "when used".

@awoie
Copy link
Contributor

awoie commented Jan 30, 2025

RFC 7518 also says that:

Applications need to specify how the "apu" and "apv" Header
   Parameters are used for that application.  The "apu" and "apv" values
   MUST be distinct, when used.  Applications wishing to conform to
   [[NIST.800-56A](https://datatracker.ietf.org/doc/html/rfc7518#ref-NIST.800-56A)] need to provide values that meet the requirements of
   that document, e.g., by using values that identify the producer and
   consumer.

So I think we need to normatively specify how the apu and apv header are used (which could be that they must be empty). I'm not sure what the exact implications are for the requirements regarding NIST 800-56A. Is anyone familiar with making the use of ECDH-ES compliant to NIST 800-56A?

It is a fair point. I believe the implications are that applications who care about NIST 800-56A should set the apu/apv values accordingly. For example, HAIP could decide to address this requirement which I'm not in favor of because of the following. Since both values are set by the Wallet, it could be also up to the Wallet to decide whether NIST 800-56A is a requirement and set the values accordingly which is probably the better approach. Note that from a verifier's point of view, it shouldn't change anything. For example, a verifier that does not care about NIST 800-56A but correctly implements ECDH-ES, would automatically use the apu/apv values provided by a Wallet that cares about NIST 800-56A compliance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encryption details for OpenID4VP over the digital credentials API
7 participants