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

DC API: Always use Origin for binding response #448

Merged
merged 5 commits into from
Mar 24, 2025

Conversation

jogu
Copy link
Collaborator

@jogu jogu commented 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

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
Comment on lines 576 to 577

* `web-origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme if the request is not sent via the Digital Credentials API defined in (#dc_api).
* `origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we remove this whole section? This is not about a client_id anymore - we are just changing what to put into aud for DC API?

Suggested change
* `web-origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme if the request is not sent via the Digital Credentials API defined in (#dc_api).
* `origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@c2bo I did wonder about that, but it's defined that what goes into aud is the client_id, and we're back to the problem of putting something into client_id that I think we need to be clear if it was intended for a client that authenticated using federation (+ custom url schemes) or one authenticated using just web PKI (+ DC API), for the same reasons we originally put the client id scheme into the aud - there's no other way for clients to tell the difference between those two types of responses.

Or to put another way, we are defining a meaning for 'origin' client id scheme in the response, so it's important that people don't define an origin client id scheme that means something different, hence retaining the definition in the client id scheme section as it would eventually need to be registered in any future IANA registry of client id schemes.

If Federation was always prefixed (with something other than "https://") then I think using just the url would make sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The alternative to say that in OID4VC over DC API, the audience of the credential presentation is the origin of the request and not the client identifier. That would require us to clearly split the spec in the "traditional" and the "DC" part including all security considerations.

Copy link
Member

Choose a reason for hiding this comment

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

That feels like the better/cleaner approach to me, but a pretty big change - especially given the current timelines

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tlodderstedt

The alternative to say that in OID4VC over DC API, the audience of the credential presentation is the origin of the request and not the client identifier. That would require us to clearly split the spec in the "traditional" and the "DC" part including all security considerations.

I don't think we really have a defined concept of the "audience of the credential presentation" in our credential format profiles though - we'd have to either introduce that concept, or have separate text for "traditional" versus "DC" in the credential format profiles too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we really have a defined concept of the "audience of the credential presentation"

why? we do use a term audience (the Client Identifier of the Verifier) in the spec already.

That feels like the better/cleaner approach to me, but a pretty big change - especially given the current timelines

as an option, we could merge this PR and do this as part of a PR addressing #453 ? (cc @awoie )

Copy link
Collaborator

Choose a reason for hiding this comment

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

so basically, we want the aud value to be prefixed with origin:, right? (to basically prevent the attacks that made us turn client id scheme parameter into a prefix in the first place). in which case, i agree it makes sense to keep this, but I would like to see more explanation that just The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses. something like

The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses. aud claims of the Verifiable Presentation MUST be the origin value prefixed with origin:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added a sentence of explanation like @Sakurann suggested.

@Sakurann Sakurann added this to the Final 1.0 milestone Mar 11, 2025

Every OpenID4VP Authorization Request results in a response being provided through the Digital Credentials API (DC API). The response is an instance of the `DigitalCredential` interface, as defined in [@!W3C.Digital_Credentials_API], and the OpenID4VP Authorization Response parameters as defined for the Response Type are represented as an object within the `data` attribute.

In places where the Client Identifier would be used in the response (for example, the `aud` value in a Key Binding JWT), the Origin MUST be used instead, prefixed with `origin:`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should go into a general description of the DC API profile. Here I would mention that client ids with authentication credentials can go into the request, but only for authentication/authorization purposes. The audience of the credential presentation is not affected by any of those client ids.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agreed. made suggestion above

Copy link
Collaborator Author

@jogu jogu Mar 18, 2025

Choose a reason for hiding this comment

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

I've rewritten this text and also added an explicit sentence about explaining the audience isn't affected by the client id in the request.

Comment on lines 576 to 577

* `web-origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme if the request is not sent via the Digital Credentials API defined in (#dc_api).
* `origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses.
Copy link
Collaborator

Choose a reason for hiding this comment

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

so basically, we want the aud value to be prefixed with origin:, right? (to basically prevent the attacks that made us turn client id scheme parameter into a prefix in the first place). in which case, i agree it makes sense to keep this, but I would like to see more explanation that just The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses. something like

The Wallet MUST NOT accept this Client Identifier Scheme in requests - it is only used in responses. aud claims of the Verifiable Presentation MUST be the origin value prefixed with origin:


Every OpenID4VP Authorization Request results in a response being provided through the Digital Credentials API (DC API). The response is an instance of the `DigitalCredential` interface, as defined in [@!W3C.Digital_Credentials_API], and the OpenID4VP Authorization Response parameters as defined for the Response Type are represented as an object within the `data` attribute.

In places where the Client Identifier would be used in the response (for example, the `aud` value in a Key Binding JWT), the Origin MUST be used instead, prefixed with `origin:`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

agreed. made suggestion above

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.

looks good - editorial suggestions. assuming majority of them would be incorporated

@Sakurann Sakurann requested review from tlodderstedt and c2bo March 18, 2025 21:26
@jogu
Copy link
Collaborator Author

jogu commented Mar 20, 2025

@Sakurann

looks good - editorial suggestions. assuming majority of them would be incorporated

Thank you! I've committed all your suggestions, a few I tweaked a little first.

@@ -574,7 +574,7 @@ Body

* `x509_san_dns`: When the Client Identifier Scheme is `x509_san_dns`, the Client Identifier MUST be a DNS name and match a `dNSName` Subject Alternative Name (SAN) [@!RFC5280] entry in the leaf certificate passed with the request. The request MUST be signed with the private key corresponding to the public key in the leaf X.509 certificate of the certificate chain added to the request in the `x5c` JOSE header [@!RFC7515] of the signed request object. The Wallet MUST validate the signature and the trust chain of the X.509 certificate. All Verifier metadata other than the public key MUST be obtained from the `client_metadata` parameter. If the Wallet can establish trust in the Client Identifier authenticated through the certificate, e.g. because the Client Identifier is contained in a list of trusted Client Identifiers, it may allow the client to freely choose the `redirect_uri` value. If not, the FQDN of the `redirect_uri` value MUST match the Client Identifier without the prefix `x509_san_dns:`. Example Client Identifier: `x509_san_dns:client.example.org`.

* `web-origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme if the request is not sent via the Digital Credentials API defined in (#dc_api).
* `origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme in requests. In OpenID4VP over the Digital Credentials API, the audience of the Credential Presentation is always the origin value prefixed by `origin:`, for example `origin:https://verifier.example.com/`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this scheme?

Copy link
Member

Choose a reason for hiding this comment

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

I brought up the same question before (#448 (comment)) and this seems to be more of a placeholder so nobody use that as a scheme. It is technically not a real client id scheme (cannot be used in the request), but blocked for the response.

I don't really like it but I guess this is fine?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, what Christian said - client id schemes are basically defining prefixes to be used in two places:

  1. The client id parameter
  2. The aud value in responses

The origin: prefix is definitely used in "2" so I think to avoid any potential for future confusion it should be a "registered" client id scheme.

@@ -574,7 +574,7 @@ Body

* `x509_san_dns`: When the Client Identifier Scheme is `x509_san_dns`, the Client Identifier MUST be a DNS name and match a `dNSName` Subject Alternative Name (SAN) [@!RFC5280] entry in the leaf certificate passed with the request. The request MUST be signed with the private key corresponding to the public key in the leaf X.509 certificate of the certificate chain added to the request in the `x5c` JOSE header [@!RFC7515] of the signed request object. The Wallet MUST validate the signature and the trust chain of the X.509 certificate. All Verifier metadata other than the public key MUST be obtained from the `client_metadata` parameter. If the Wallet can establish trust in the Client Identifier authenticated through the certificate, e.g. because the Client Identifier is contained in a list of trusted Client Identifiers, it may allow the client to freely choose the `redirect_uri` value. If not, the FQDN of the `redirect_uri` value MUST match the Client Identifier without the prefix `x509_san_dns:`. Example Client Identifier: `x509_san_dns:client.example.org`.

* `web-origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme if the request is not sent via the Digital Credentials API defined in (#dc_api).
* `origin`: This Client Identifier Scheme is defined in (#dc_api_request). The Wallet MUST NOT accept this Client Identifier Scheme in requests. In OpenID4VP over the Digital Credentials API, the audience of the Credential Presentation is always the origin value prefixed by `origin:`, for example `origin:https://verifier.example.com/`.
Copy link
Member

Choose a reason for hiding this comment

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

I brought up the same question before (#448 (comment)) and this seems to be more of a placeholder so nobody use that as a scheme. It is technically not a real client id scheme (cannot be used in the request), but blocked for the response.

I don't really like it but I guess this is fine?

Co-authored-by: Torsten Lodderstedt <[email protected]>
@Sakurann Sakurann merged commit 76dacc9 into main Mar 24, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
7 participants