Skip to content

Conversation

@aaronpk
Copy link
Contributor

@aaronpk aaronpk commented Mar 24, 2025

unambiguously defines the plain https Client Identifier URLs as a fallback to mean OpenID Federation

This is a PR against the open PR #401

Context of this PR is documented in #401 (comment)

unambiguously defines the plain https Client Identifier URLs as a fallback to mean OpenID Federation
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.

i am not comfortable with keeping https to be reserved only for openid federation. what I said in #401 (comment)


For example, if an Authorization Request contains `client_id=example-client`, the Wallet would interpret the Client Identifier as referring to a pre-registered client.

If the Client Identifier begins with `https://` the full Client Identifier URL represents an Entity Identifier defined in OpenID Federation [@!OpenID.Federation] and MUST be processed as though it was prefixed with `openid_federation` as described below.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I personally don't have an issue with OpenID4VP defining the default behavior of https URLs as long as it's defined only within the context of OpenID4VP and doesn't spill out to unrelated communities and ecosystems.

if this is the case, it must be much more explicit that this https:// behavior is only for openid4vp context

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aaron's intent is to make it not ambiguous that within the context of openid4vp, https client_id_scheme is federation.
Brian's PR says, if there is no prefix, the wallet implementation picks the default. so this PR clarifies, when the url does not have a prefix, default is openidfederation.

@jogu
Copy link
Collaborator

jogu commented Mar 26, 2025

I got a request for a rendered version of this PR so uploaded html that would result from this PR here in case it's useful to others:

https://www.heenan.me.uk/~joseph/openid-4-verifiable-presentations-1_0-pr468-2025-04-26.html#section-5.10

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.

This improves the PR that it would be merged into by removing the breaking change.

* `x509_hash`: When the Client Identifier Scheme is `x509_hash`, the Client Identifier MUST be a hash and match the hash of 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 parameter [@!RFC7515] of the signed request object. The value of `x509_hash` is the base64url encoded value of the SHA-256 hash of the DER-encoded X.509 certificate. The Wallet MUST validate the signature and the trust chain of the X.509 leaf certificate. All verifier metadata other than the public key MUST be obtained from the `client_metadata` parameter. Example Client Identifier: `x509_hash:Uvo3HtuIxuhC92rShpgqcT3YXwrqRxWEviRiA0OZszk`

To use the Client Identifier Schemes `https`, `did`, `verifier_attestation`, `x509_san_dns` and `x509_hash`, Verifiers MUST be confidential clients. This might require changes to the technical design of native apps as such apps are typically public clients.
* `openid_federation`: This value indicates that the Client Identifier is an Entity Identifier defined in OpenID Federation [@!OpenID.Federation]. Processing rules given in [@!OpenID.Federation] MUST be followed. Automatic Registration as defined in [@!OpenID.Federation] MUST be used. The Authorization Request MAY also contain a `trust_chain` parameter. The final Verifier metadata is obtained from the Trust Chain after applying the policies, according to [@!OpenID.Federation]. The `client_metadata` parameter, if present in the Authorization Request, MUST be ignored when this Client Identifier scheme is used. Example Client Identifier: `openid_federation:https://federation-verifier.example.com`.

Choose a reason for hiding this comment

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

We would need to add here (without the openid_federation: prefix) like in case of the redirect_uri

@Sakurann
Copy link
Collaborator

Sakurann commented Apr 1, 2025

WG discussion:

  • be clear that https is not a prefix, because it is not.
  • default is openid_federation: prefix is used for openid federation deployments.
    • dids also get prefixed with decentralzied-identifier:
  • but! if client_id does not have a prefix, and it happens to start with https://, there is one expected behavior, which is the same as openid_federation:
    • do we need to be explicit about this "no prefix, but it's https" scenario? if we are trying to discourage no-prefix scenario at all? if not harmful, might be better to define a behavior rather than keeping ?
  • add security considerations on this
    • as long as someone is using https prefix, security issue that led to prefixing client id remains? vs this issue exists only when there are multiple ways to interpret, but not when there is one way to interpret https prefix.

@Sakurann
Copy link
Collaborator

Sakurann commented Apr 1, 2025

chair hat off, we should also define behavior when client_id does not have a prefix, and does not happen to start with https://

@jogu
Copy link
Collaborator

jogu commented Apr 1, 2025

Just for info for readers: there is some overlap with the federation spec owned connect wg here, and this is scheduled to be discussed on Thursday's Connect WG call, we're trying to the best of our ability to have alignment between Connect, DCP & OAuth working groups.

@jogu
Copy link
Collaborator

jogu commented Apr 1, 2025

@Sakurann

chair hat off, we should also define behavior when client_id does not have a prefix, and does not happen to start with https://

There's text here (which I think has been there a while):

https://www.heenan.me.uk/~joseph/openid-4-verifiable-presentations-1_0-pr468-2025-04-26.html#section-5.10.2

in particular:

If a : character is not present in the Client Identifier, the Wallet MUST treat the Client Identifier as referencing a pre-registered client. This is equivalent to the [RFC6749] default behavior, i.e., the Client Identifier needs to be known to the Wallet in advance of the Authorization Request. The Verifier metadata is obtained using [RFC7591] or through out-of-band mechanisms.

Do you think this needs any change?

@sloops77
Copy link
Contributor

sloops77 commented Apr 2, 2025

i listened to the discussion and it seems strange to have two ways of being able to utilize openid federation. What are the consequences?

  • The wallets in the existing ecosystems may end up implementing the openid_federation prefix for ecosystem interop anyway.
  • For new implementors how do they navigate having two ways to implement open id fed? Should they implement the prefix or rely on the default behaviour or should they do both? The right answer will depends on their use case - but the decision can be quite nuanced and therefore its opening up room for incorrect decisions by implementors and therefore interop issues

Co-authored-by: Michael B. Jones <michael_b_jones@hotmail.com>
For example, if an Authorization Request contains `client_id=example-client`, the Wallet would interpret the Client Identifier as referring to a pre-registered client.

If the Client Identifier begins with `https://` the full Client Identifier URL represents an Entity Identifier defined in OpenID Federation [@!OpenID.Federation] and MUST be processed as though it was prefixed with `openid_federation` as described below.
Note that like all requirements in this specification, this requirement only applies when using this specification.
Copy link
Contributor

@sloops77 sloops77 Apr 3, 2025

Choose a reason for hiding this comment

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

this new sentence is a tautology

@Sakurann
Copy link
Collaborator

Sakurann commented Apr 5, 2025

please re-review PR #401 in light of new understanding in DCP WG that is documented in #401 (comment)

Copy link
Member

@bc-pi bc-pi left a comment

Choose a reason for hiding this comment

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

the underlying openid:equal-treatment-under-the-prefix branch and open PR #401 against which this one was created no longer has default so this has effectively been overcome by events

@aaronpk
Copy link
Contributor Author

aaronpk commented Apr 7, 2025

I don't think this PR is necessary anymore since the changes to #401, as has been discussed in the WG call and in the issue comments.

@aaronpk aaronpk closed this Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants