OpenID4VP icon indicating copy to clipboard operation
OpenID4VP copied to clipboard

define a mechanism for the verifier and verifier to communicate profiles supported

Open jogu opened this issue 1 year ago • 13 comments

[edited on Apr-24-2025] this issue used to be called Using 'detached' session information as part of encryption key derivation to solve 'lazy verifier' problem. Renaming since the direction evolved based on the discussion.

The current encryption methods supported in OID4VP require that any information that is used as part of the key derivation is included in the JWE header (e.g. apu and apv). This is basically how the JWE specs and libraries currently work and it's not really possible to not send this information and be compliant with the current JWE RFCs.

It seems it be advantageous to not send some of this information in the JWE and to instead have the verifier provide it from it's view of the world - in particular if information like the web origin, keys, etc, where included in the key derivation and not transmitted, then the verifier would hopefully fail to decrypt a response that has been injected into the wrong session.

Given we have seen a long history of OAuth clients not implementing security checks like session binding correctly, a mechanism where a response from a different session simply fails to decrypt is highly advantageous from an "overall security of an ecosystem" point of view.

jogu avatar Nov 26 '24 21:11 jogu

In my opinion, the only way to achieve this with a regular JWE compact form is by using a detached protected header. Note that apu, apv, or any additional parameters in the protected header are part of the AAD (Additional Authenticated Data). The AAD is sent in plaintext but is integrity-protected by the authentication tag of the JWE. However, the JWE compact form does not support a dedicated component for additional AAD beyond what is provided via the protected header. In contrast, the JWE JSON serialization form includes a dedicated aad parameter that is concatenated with the protected header to form the full AAD.

Example (Unprotected Header / JARM):

{
  // Could be negotiated or static
  "alg": "ECDH-ES",
  // Could be negotiated or static
  "enc": "A256GCM",
  // Could be a web origin, e.g., https://rp.com
  "apu": "MTIzNDU2Nzg5MGFiY2RlZmdo",
  // Could be a web origin, e.g., https://rp.com
  "apv": "YWJjZGVmZ2gxMjM0NTY3ODkw",
  // Verifier decryption key identifier
  "kid": "P8p0virRlh6fAkh5-YSeHt4EIv-hFGneYk14d8DF51w",
  // Ephemeral decryption key produced by the wallet
  "epk": {
    "kty": "EC",
    "crv": "P-256",
    "x": "laKMaRZltDtdJV0fmSivSI2dhGyOJilIZcXjdsheEfM",
    "y": "jwiLJu_o4PlxGg0RS3zjjT7g3mNcydj5Vc0n5Neby0Y"
  }
}

For the JWE compact form, all these headers are part of the AAD. If we want to detach any AAD value, the entire protected header must be detached.

This creates challenges:

  • Sending the protected header detached would likely result in a non-compliant JWE/JARM during transit, i.e., breaking the JWE specification.
  • Alternatively, we could send an unprotected header with some values omitted, requiring the verifier to compute those missing values which is not great either.
  • In both cases, if the verifier must compute the protected header, a consistent canonicalization process is required. Without canonicalization, the verifier and wallet may generate different versions of the protected header, breaking the integrity check.

Alternatively, we could explore JWE JSON serialization where only the aad JWE component is detached which might be much simpler. The aad is optional and if it is omitted, it is still a compliant JWE, although it cannot be validated without knowing its value (in the case of AES GCM one could probably still decrypt the ciphertext but most implementations would probably still reject it).

awoie avatar Dec 09 '24 17:12 awoie

Thanks Oliver. I'd tend to agree that calculating the protected header at the verifier is going to be hard/problematic.

I think this is the advantage of using JOSE HPKE - that can potentially update the JWE RFC. We only need a very small addition really - a sentence saying that when using compact serialisation a JWE AAD (which is distinct from AAD) can be communicated out of band. As far as I can see the rest of the steps should all work. (The AAD is defined to be ASCII(Encoded Protected Header || '.' || BASE64URL(JWE AAD)). in the case where a JWE AAD is available.)

The language e.g. around step 14 of https://datatracker.ietf.org/doc/html/rfc7516#section-5.1 makes this probably technically a bit of a grey area:

Screenshot 2024-12-11 at 11 16 41

Technically I think we could perhaps use that same approach with ECDH-ES too, but I think getting existing libraries updated to support that is potentially troublesome. (If I missed some other nuance please let me know!)

jogu avatar Dec 11 '24 02:12 jogu

Related issue: https://github.com/ietf-wg-jose/draft-ietf-jose-hpke-encrypt/issues/13

awoie avatar Dec 12 '24 21:12 awoie

[editor hat on] Already agreed in https://github.com/openid/oid4vc-haip/issues/131#issuecomment-25395281x36 that not doing detached in Final 1.0, so this is 1.1. However, we need to make sure in PR #380 that how the spec is written does not preclude this detached in the future before going to 1.0, then will change the milestone to 1.1

Sakurann avatar Jan 13 '25 15:01 Sakurann

#380 doesn't describe how both the RP and the wallet know which parameters go into the detached information. (in the case of ECDH-ES, what goes into the APU and APV headers). Since APU and APV headers don't provide any security if they are ignored (i.e. not verified), we have to make sure in 1.0 that if APU and APV headers (or another mechanism to add detached information) are used, both the wallet and RP know about this or otherwise give an error. We need some kind of mechanism or parameter in 1.0 to describe how this works. I don't think this is currently in the spec, if that's indeed the case this should be labeled 1.0 for now. If that mechanism is already there, it can stay at 1.1.

martijnharing avatar Mar 18 '25 11:03 martijnharing

As this item is labeled 1.1, I would like to make sure that we don't need to do anything in 1.0, however I don't think that's the case.

To give a concrete example, the specification says the APU and APV headers can be used, but that whether and how to use them is profile specific. However when a profile wants to use values for APU and APV it still needs a way for the Wallet and Verifier to communicate about the contents of those values and the profile may want to make sure that the Verifier rejects the transaction if it cannot verify them. Therefore in this example we need:

  • A way for the verifier to say that it supports a particular profile / definition of APU and APV values.
  • A way for the Wallet to say it uses a particular profile / definition of APU and APV values and that verification of those values is required.

If this can be done with 1.0 already that would be great as no changes are necessary. If that's the case, how would the example above be implemented? If this cannot be implemented we need to add a parameter, to make sure that the APU/APV or detached information can actually be used if a particular profile wants to.

martijnharing avatar Apr 03 '25 11:04 martijnharing

I was asked what a potential solution would look like. This depends quite a bit on what's already possible, but one potential solution could be to add an extra parameter to the jwks like "authorization_encrypted_response_details" that would define what other encryption parameters (e.g. apu/apv, detached info) are used.

martijnharing avatar Apr 09 '25 13:04 martijnharing

wg discussion:

  • define validate_apu_apv boolean in the sprit of require_cryptographic_holder_binding that is set to false by default

Sakurann avatar Apr 11 '25 19:04 Sakurann

This would be a change to how JWE [RFC 7516] and its implementations work. I think it's out of scope for this working group to make this change. This new parameter should be proposed in the IETF JOSE working group at [email protected], if wanted.

selfissued avatar Apr 11 '25 19:04 selfissued

I don't think "validate_apu_apv" would address the full problem. (Also if this would require a change to JWE, then that seems like it's not a solution that we should pursue). The issue is that the RP and the Wallet need to communicate about which specification / profile they are using for things like APU/APV values or detached information. If there is something used in the encryption / authentication where multiple options exist (i.e. detached information, apu/apv), then that needs to be communicated somehow.

When using detached information, not being able to validate it will likely result in failure and therefore will automatically be 'validated' by the RP. However when using a mechanism like APU/APV where this information is actually transmitted, there is no guarantee that the APU/APV values are validated by the RP. We need a parameter that can enforce this / communicate this.

martijnharing avatar Apr 15 '25 13:04 martijnharing

@awoie suggested to get input from NIST

Sakurann avatar Apr 15 '25 19:04 Sakurann

WG discussion:

  • from the co-chairs:
    • question 1: do we believe it is a good idea to define apu/apv?
    • question 2: if we do, is it sufficient to define a parameter is

discussion:

  • agreed to define a parameter that allows wallet and verifier to communicate profile of openid4vp being used. ok to do during the 60day review period.
  • NIST clarified that defining specific apu/apv values is arguably a “hygiene” issue, but there isn’t a clear benefit. And I’ve been confused by its relation to the lazy verifier problem.

Sakurann avatar Apr 22 '25 22:04 Sakurann

Specific mechanisms suggestion:

  • define an optional encryption_details_supported parameter (array of strings) to be included in the client_metadata parameter in the request (can bikeshed later too).
  • define a encryption_details parameter (string) to be included in the JWE protected header in the response

Sakurann avatar Apr 24 '25 16:04 Sakurann

resolved by https://github.com/openid/OpenID4VP/pull/628#issuecomment-2945215753

Sakurann avatar Jun 05 '25 18:06 Sakurann