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

Clarify that signed requests for DC API should be rejected if the signature can't be verfied #395

Open
GarethCOliver opened this issue Jan 23, 2025 · 16 comments · May be fixed by #448
Open
Assignees
Milestone

Comments

@GarethCOliver
Copy link

It'd be good to call out that, if the wallet can not verify the signature for the signed version (A.3.2), then they must reject the request. As we were implementing this, initially we assumed that falling back to how we handle the unsigned case would make sense. However, because the origin is not included for signed cases, it would be vulnerable to replay attacks in ways that unsigned is not.

At least to me, this wasn't clear from reading A.3.

It'd be nice (IMO) if an unsigned request, and a signed request that can't be verified by a wallet had the same security properties and would be a motivation for including the origin in all cases.

@c2bo
Copy link
Member

c2bo commented Jan 24, 2025

If that kind of downgrade is wanted, couldn't you in theory just change the client_id to web-origin: in the response? That way you would still include the platform-provided origin in the response and keep the same security assumptions as a standard unsigned flow.

That would also clearly signal to the RP that the wallet was not able to verify the trust framework it provided and it downgraded to the unsigned flow.

Somewhat relevant issue: #362

@Sakurann Sakurann added the ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API label Jan 28, 2025
@jogu
Copy link
Collaborator

jogu commented Jan 29, 2025

Two probably very related clarifying questions:

@GarethCOliver "However, because the origin is not included for signed cases" - do you mean we should state that in the response to a signed request (e.g. the aud value in SD-JWT) the web-origin:https://www.example.com/ value should be used in this case? Or did you mean something else?

@c2bo Included where? In an mdoc response I don't think we return the client_id to the verifier other than encoded in the session transcript?

@jogu
Copy link
Collaborator

jogu commented Jan 29, 2025

It was raised in today's ISO meeting that (if we are going to allow the wallet to accept requests where it can't validate the signature) then we do need to be clear in OID4VP how the wallet should then behave (e.g. if it should use the client_id in the signed request, or as suggested above use one derived from the origin instead), so that the verifier then knows what to expect.

This is particularly important in the case of mdoc/mdl where the verifier will need to know what client_id the wallet decided to use in order to calculate the SessionTranscript correctly.

@GarethCOliver
Copy link
Author

So I was initially assuming we should just reject if the request signature can't be verified (as expected_origins can't be trusted in that case). @c2bo's suggest is more elegant:

Essentially treat the platform-provided origin as one of many different potential client_ids. Whichever one the RP trusts, should be included in the aud (and then which one was trusted needs to be included in the response somewhere to prevent needing to attempt verification multiple times).

(so for a concrete example, if there is a signed request for age-over-18, but the wallet can't verify it, and doesn't recognize the origin, but issuer policy for that mDL is that all origins are allowed to request age-over-18, then the wallet will return the request using client_id=web-origin:.. A wallet that could verify the signature as belong to a trust framework it trusts would return that client_id instead).

This does then open up the follow-up of 'what if the wallet trusts multiple of the provided client-ids'. The answer to which could be:

  1. include whichever one the wallet thinks is strongest (e.g. if you get a client_id and an origin, trust both but trust the trust framework more, include the client id, otherwise if you trust the origin more, include the origin).

  2. include all ones that you trust (this helps with the problem @martijnharing was raising: if you trust both origin and client_id, but the trust framework is weaker than the TLS, having both requires both to be broken).

@c2bo
Copy link
Member

c2bo commented Jan 30, 2025

@c2bo Included where? In an mdoc response I don't think we return the client_id to the verifier other than encoded in the session transcript?

Fair point, we would have a similar problem to the one we have in the multi rp discussion for mdoc. In both cases the RP would need that information explicitly. I guess the same problem would hold true for credentials that are not key-bound since they currently would not include client_id anywhere in the response.

Not entirely sure about this, but it looks to me like we should think about adding client_id as an explicit field somewhere in the Authorization Response (or vp_token).

@Sakurann Sakurann added this to the Final 1.0 milestone Jan 30, 2025
@Sakurann
Copy link
Collaborator

WG discussion:

  • need time to think a bit more, but rough consensus forming around proposal to change client_id to use client_id_scheme web-origin:, when request was signed, but wallet did not validate the request.
    • one issue that arises from this approach is that RP knows the client id scheme changed, one way to cope with this will be to pass client_id in the response, which means breaking change and changing the response structure? Alternative is aud in signed and encrypted response, which is rarely used.
    • explicitly say that it is up to the wallet whether it fails the request when signature is not validated or to continue
    • this means that verifier needs to support both cases if it cares? always? need to explicitly state - in the client metadata?
  • passing client_id in the response is an info leak, which should be clearly documented but isn't being able to detect ignoring the signature more concerning?
  • do we want to detect that wallet did not validate the signature or not?
  • alternative is to always pass the origin in the key binding piece (session transcript, KB JWT, anon creds, etc.), but that has implications on other credential formats.

will discuss in the next WG and make a decision .

@Sakurann
Copy link
Collaborator

Sakurann commented Feb 4, 2025

WG discussion

who determines what...:

  • when the RP gets credential it can determine the issuer. it is issuer who defines the security policy, wallet enforces it so verifier needs to know
  • new input: ultimately verifier makes the decision whether it is ok for the wallet not to validate the signature and fallback to webPKI authentication?
  • ultimately it is up to the wallet to decide what to do?
  • ultimately verifier has to know whether the wallet has validated the signature or not. if the wallet did not consider the signature, response can be replayed.
  • can also be issuer's choice for example with embedded disclosure policy with EUDIW

Role of the signature...:

  • difference in perspective is whether signature of the request is THE authentication mechanism or layer on top. In EUDIW ecosystem for example, signature is the authenticaiton mechanism and falling back to web pki is not an option.
    • security of the signed request is bound to the origin?
  • signature is usually tied to a certain trust infrastructure.
  • a lot of this is solved by multi RP PR?

options

  1. say wallet must fail if it cannot validate the signature
  2. say wall is ok not to validate the signature
  3. do not say anything
  4. in openid4vp say it depends on the trust framework; in HAIP define two+ behaviors dependent on the trust framework (like in EUDIW wallet must validate, outside it, ok not to?)
  5. define a mechanism
    a. verifier indicates if it requires to validate
    b. wallet communicates if it validated or not
  6. issuer communicates whether it mandates the wallet to validate or not and wallet and verifier has to follow that

1, 2 does not seem to be able to get consensus?
3, 4 are not ideal for interop?
5 or 6 seems to be an option...?

@tlodderstedt
Copy link
Collaborator

I perceive there is common ground regarding the governance model for RP authentication: The issuer defines the policy, the wallet enforces it.

I think the fundamental difference is in assumptions regarding what the RP knows. Some people believe the verifier does not know anything about what mechanism/credentials are needed to obtain a certain credential from the wallet. I would like to understand how the RP would determine whether to sign a request or not and with what key material (tying into some trust infrastructure).

I believe the RP wil need to know and can know. The RP knows what issuer's credentials it accepts. So it will also determine from the set of issuers their requirements on RP authentication and, if needed, get the respective credentials and use them with the respective requests.

@tlodderstedt
Copy link
Collaborator

Summary after the discussion today: we need to decide what a signed request means wrt to client identifiers:

  1. there is one client identifier used, it's the client_id from the request payload. It is only available if the request signature can be validated.
  2. there are two client identifiers. (a) the web origin identifier and (b) the client_id from the payload. (b) is only available if the request signature can be validated.

(2) requires a change in the way the OID4VP response is built, as there could be one out of the two client identifiers in the audience restriction field of the respective credential (session transcript for mdoc and aud for sd-jwt). The RP must be aware of this.

If we go this way, PR 308 is an extension of this logic, which adds more client identifiers of category (b) to the set of client identifiers.
It would also mean the RP cannot decide to not use its web origin to authenticate with the wallet (not in the sense of technical transport but in the sense of client identifier used in policy evaluation as basis for credential release).

@andprian
Copy link

My view is that the wallet needs to abide by many security requirements and is responsible for enforcing them. I do not think the wallet should comply with whatever the Issuer or Verifier believe the wallet should enforce (except when applying further restrictions).

I think the user (via the wallet) should be the only one allowed to make the decision to respond to an unauthenticated RP, even if I am not sure this fallback mechanism is a good idea (since I do not trust all users to make informed choices). In any case, the fallback should not be the default behavior.

@paulbastian
Copy link
Contributor

Imo we need 2 parameters: "client_id" for the trust framework and "transport_binding" or something similar that would be the origin in DC-API or bluetooths parameters in the future. Then:

  • vanilla OpenID4VP would only use client_id (as there is no real transport binding mechanism)
  • unsigned DC-API would only use "transport_binding"
  • signed DC-API would use both

That would also enable the RP to easily see whether the signed request was processed or not.

@martijnharing
Copy link
Contributor

martijnharing commented Feb 21, 2025

There’s been a lot of discussion about whether the RP needs to know if the wallet verified the RP signature. Is there a specific attack that’s mitigated by providing this signal back to the RP?

From a wallet’s perspective, the signals to use and how to use them to decide whether to interact with an RP are up to the wallet, issuer, ecosystem, and user. RP authentication can be one of those signals.

However, from an RP’s perspective, I’m struggling to see an attack that’s being mitigated when a wallet decides to return a response to a particular RP, but the RP then discards that response because it indicates that the RP authentication signature wasn’t validated. And if that signal isn’t used by the RP, providing it only adds complexity.

@Sakurann Sakurann added priority and removed ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API labels Feb 27, 2025
@Sakurann
Copy link
Collaborator

Sakurann commented Mar 4, 2025

WG discussion

  1. proposal on the table: there are two client identifiers in the request: (a) the web origin identifier and (b) the client_id from the payload. (b) is only available if the request signature can be validated. Response looks the same, but requires a change in the way it is built, as there could be one out of the two client identifiers in the audience restriction field of the respective credential (session transcript for mdoc and aud for sd-jwt). The RP must be aware of this.

  2. another proposal: with DC API OpenID4VP, use origin as aud. when request is signed and validated, client_id is added as an additional parameter to convey how the wallet authenticated the RP.

  3. adapted proposal: with DC API OpenID4VP, use client_id as aud. origin is added as an additional parameter. might complicate things for unsigned and not-signature-validated-requests.

  4. adapted proposal: with DC API OpenID4VP, use origin as aud. for signed requests add client_id as an additional parameter (removes client_id from session transcript - solves 308). for unsigned or when the wallet did not validate the signature client_id is omitted.

  5. adapted proposal: with DC API OpenID4VP, use origin as aud. removes client_id from the response/session transcript - solves 308.

aaaaand the winner is...... option 5 ;)

origin is identification; request signature is RP authentication. Is it important for the RP to know how the wallet authenticated the RP? if yes, we would add an additional parameter.

for app-to-app will be app package

need a PR to modify session transcript and adjust any language related to client_id being the origin in the context of DC API and once this PR is merged, 308 should be merged as-is

@Sakurann
Copy link
Collaborator

Sakurann commented Mar 4, 2025

@danielfett please take a look at the security implications of this option 5, and ping Stuggart team on this change.

jogu added a commit that referenced this issue Mar 6, 2025
Implements option 5 from #395 (comment)

The means that, for example, the aud value in the SD-JWT key binding is
based on the Origin obtained for the platform, even in the case of signed
requests.

The Client Identified Scheme prefix on the value is retained in these
cases, to avoid any potential mixup attack between these DC API responses
and non-DC API responses to clients using OpenID Federation.

The Client Identifier Scheme name is changed from 'web-origin' to just
'origin' as it may contain a native app identifier, as per #351.

The "client_id" is removed from the mdoc OpenID4VPDCAPIHandover as it
now serves no clear purpose.

It is left unsaid whether the Wallet is required to validate the
signature on signed requests in all cases, this is a policy decision for
Wallets. This change allows Wallets to skip the validation but still
return a response that the Verifier can successfully process, as there
is no longer any uncertaintly over what would be used for `aud` in
SD-JWT KB JWT or for `client_id` in the ISO mdoc OpenID4VPDCAPIHandover
structure.

closes #351
closes #395
@jogu jogu linked a pull request Mar 6, 2025 that will close this issue
@danielfett
Copy link
Contributor

@Sakurann I have a bad feeling about this. The client_id is a pretty important concept and knowing which client_id was sent in the request might be important for security (even though I don't have a concrete attack in mind).

I'm not sure what the expectation is on the Stuttgart team, I'll ping them, but before they do their analysis, they won't be able to tell whether that's a problem or not.

jogu added a commit that referenced this issue Mar 7, 2025
Implements option 5 from #395 (comment)

The means that, for example, the aud value in the SD-JWT key binding is
based on the Origin obtained for the platform, even in the case of signed
requests.

The Client Identified Scheme prefix on the value is retained in these
cases, to avoid any potential mixup attack between these DC API responses
and non-DC API responses to clients using OpenID Federation.

The Client Identifier Scheme name is changed from 'web-origin' to just
'origin' as it may contain a native app identifier, as per #351.

The "client_id" is removed from the mdoc OpenID4VPDCAPIHandover as it
now serves no clear purpose.

It is left unsaid whether the Wallet is required to validate the
signature on signed requests in all cases, this is a policy decision for
Wallets. This change allows Wallets to skip the validation but still
return a response that the Verifier can successfully process, as there
is no longer any uncertaintly over what would be used for `aud` in
SD-JWT KB JWT or for `client_id` in the ISO mdoc OpenID4VPDCAPIHandover
structure.

closes #351
closes #395
@jogu
Copy link
Collaborator

jogu commented Mar 12, 2025

Clarity on option 5: In a JARM signed response over DC API, is aud the client_id or the origin?

I've assumed for consistency it'd be the origin, but I think there's definitely an argument that if someone did for some reason decide to use a signed response then client_id makes more sense if the request was signed. (But then that just brings back the problems of what to use for client_id if the request is unsigned or the signature isn't validated.)

I'm not sure anyone actually uses JARM signed responses so it's probably not a big issue either way...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants