Skip to content

Conversation

@bc-pi
Copy link
Member

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

with a default allowance

to help with closes #376

@bc-pi bc-pi linked an issue Jan 29, 2025 that may be closed by this pull request
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.

please separate this PR into two PRs: one that includes what helps with #376 and editorial changes; and the other with what is a normative change we have not agreed in the WG (openid-federation and decentralzied-identifier). otherwise, will close.

Co-authored-by: Tim Cappalli <tim@cappalli.me>
@bc-pi
Copy link
Member Author

bc-pi commented Jan 30, 2025

please separate this PR into two PRs: one that includes what helps with #376 and editorial changes; and the other with what is a normative change we have not agreed in the WG (openid-federation and decentralzied-identifier). otherwise, will close.

I understand that you don't see this as important and are not happy to see it appear given the many competing priorities and constrained timelines from many external factors. But it is no less deserving of the WGs attention than many other items that have been brought to the front of the queue. I will respectfully suggest that "otherwise, will close" is an understandable but wholly inappropriate response. Please reconsider.

@bc-pi
Copy link
Member Author

bc-pi commented Feb 5, 2025

and even though it shouldn't work in this direction, whatever happens here in OpenID4VP will likely have ramifications in regular old OAuth. And it's not okay to push special treatment of DIDs and OpenID Federation onto that layer.

@aaronpk
Copy link
Contributor

aaronpk commented Feb 5, 2025

Sorry for being so blunt, but if the DCP WG wants to do things the OpenID4VP way while ignoring feedback from the underlying OAuth perspective, that's fine with me, but then I expect that those decisions will not be brought back to the OAuth WG at the IETF. Making a decision here that is knowingly not compatible with work happening at the IETF is a decision the DCP WG will have to live with as things develop independently at the IETF.

If the changes in this PR are not made, then I expect to hear no complaints when this topic is brought back up at the IETF.

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 PR proposes breaking changes to the hard-won consensus position on Client ID prefixes developed in PR #263 . It would break existing client_id values using OpenID Federation and existing ones that are DIDs. That seems ill-advised.

Co-authored-by: Aaron Parecki <aaron@parecki.com>
aaronpk added a commit to aaronpk/oauth-client-id-prefix that referenced this pull request Feb 6, 2025
@aaronpk
Copy link
Contributor

aaronpk commented Feb 6, 2025

I would like this to be a DCP working group decision that it's acceptable to diverge from what the OAuth WG may do with client ID schemes in the IETF. Can we get this decision onto the agenda for the next call?

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 think decentralized-identifier:did: sounds very weird, and it's only DIDs that use did: anyway, unlike https: that is used not just by openid federation, so why did: is a preferential treatment? I support openid_federation: but am less sure about decentralized-identifier:

@Sakurann Sakurann self-requested a review February 7, 2025 10:12
@bc-pi
Copy link
Member Author

bc-pi commented Feb 7, 2025

I think decentralized-identifier:did: sounds very weird, and it's only DIDs that use did: anyway, unlike https: that is used not just by openid federation, so why did: is a preferential treatment? I support openid_federation: but am less sure about decentralized-dentifier:

The idea here is to have all client id schemes use a prefix. That's the consistency part. The text prior to this PR has a prefix for the client id schemes except for Federation and DIDs, which used the URI scheme as both the client id scheme and part of the client id value. This PR adjusts things so that all client id schemes consistently have a prefix part that is distinct from the value part. I chose decentralized_identifier: for Decentralized Identifiers (DIDs) v1.0 because I assumed someone would say did:did sounded weird or was confusing. It doesn't have to be decentralized_identifier: though. But it does need to be something. Would w3c_did: or w3c_decentralized_identifiers_v1_0:or justdid: (but that would then have double "did:did") or decentralized_id: or w3c_did_core_architecture_data_model_and_representations: or did_core: or did_core_2021: or something along those lines be less weird to your eye?

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.

One of the core principles of the OpenID Foundation is that we collaboratively support each other's work. This results in OpenID specifications being able to be seamlessly used together, both for specs in the same working group and specs across working groups.

This PR does not live up to that principle, since it would create a normative conflict between two OpenID specifications. OpenID Federation requires that the Client ID be the Entity Identifier (which begins with https:) when using Automatic Registration. This PR would require that it be a different value. This would hinder the use of the specifications together. In fact, it would create a contradiction within the OpenID4VP spec itself, because both the language in this PR and the language in the normatively referenced OpenID Federation specification would apply.

I therefore respectfully request that you withdraw this PR for those reasons.

@aaronpk
Copy link
Contributor

aaronpk commented Feb 7, 2025

This change originally stemmed from a security issue with the possible ambiguous interpretation of the client identifier value when used in conjunction with other specifications.

There is of course precedent for making breaking changes to OpenID specs when security issues are found, as demonstrated by the current changes being made across a number of OpenID and OAuth specifications regarding private key JWT client authentication.

That said, the security issue in question is only relevant if an AS intends on supporting multiple different client metadata resolution methods. I would actually prefer plain HTTPS URLs to be valid client IDs, since many deployments will only ever have one method of resolving metadata from a URL, such as the one described in the draft Client ID Metadata Document that I presented at the last IETF meeting in November.

So if there is going to be an exception to the client ID prefix, I would prefer that exception be a blanket exception for https URLs, not explicitly for OpenID Federation, which is just one metadata resolution method.

It is particularly not collaborative or cooperative to have an OpenID Federation exception backported into an IETF draft, while blocking non-OpenID uses of the mechanism.

@bc-pi
Copy link
Member Author

bc-pi commented Feb 7, 2025

One of the core principles of the OpenID Foundation is that we collaboratively support each other's work.

That's a lovely sounding platitude on the surface of things but is actually absurd and untenable.

@selfissued
Copy link
Member

@aaronpk, the security vulnerability was closed by PR #263 unambiguously defining that Client IDs beginning with https: are OpenID Federation Entity Identifiers. Relaxing that to leave the interpretation up to implementations would re-open the security vulnerability.

It's not a special case. It's a clear definition to remove ambiguity.

@bc-pi
Copy link
Member Author

bc-pi commented Apr 7, 2025

Honestly, if we're worried about "unnecessary developer confusion", we should remove OpenID Federation entirely from the 4VC specs. DIDs too.

@Sakurann
Copy link
Collaborator

Sakurann commented Apr 7, 2025

WG discussion:

  • agreement to adding decentralized_identity: and openid-federation:
  • remaining discussion is about adding behavior of non-prefixed client identifiers that start with https://
    • option 1: prohibit not-prefixed client identifiers including those that start with https://
    • option 2: define a behavior for not-prefixed client identifiers that start with https://
  • another option would be to remove any text about https:// now and re-add it during 1.0 wglc
  • this change would not break existing openid federation implementations with OIDC/federations; this will only impact openid federation implementations with the wallets/openid4vp systems. because prefix only exists in the context of OpenID4VP

@Sakurann Sakurann removed the discuss label Apr 7, 2025
@lj-raidiam
Copy link

lj-raidiam commented Apr 9, 2025

  • agreement to adding decentralized_identity:

I respect the decision; however, we should ask ourselves whether ultimate parsing convenience should prevail over keeping it logical and not prefixing "did" with de facto another "did".

  • option 2: define a behavior for not-prefixed client identifiers that start with https://

What does stop us from going with this option and merging this PR? Would making federation that default mechanism in addition to prefixes introduced by the vp itself or oauth in the future be harmful in any way for the vp spec? If yes, how?

In addition to that looking at the notes from the WG meeting I wanted to draw attention of the WG to one detail. Correct me if I am wrong, but the client_id_scheme can't be directly compared to the prefix mechanism. When the client_id_scheme param was present, the entity identified with a certain clientid could span across multiple namespaces (client_id _schemes). A single client using https based identifier, could use various schemes, but it was still a single client. This is not the case now when the prefix defining the namespace is included in the client id and the above mentioned rule is not in the spec.

As long as this is not an issue for non-global identifiers it has an impact on https. Given the above, why shouldn't we be worried that prefixing https, which is designed to uniquely identify resources globally, would lead to either clients identified with the same URLs yet supporting multiple metadata discovery or trust establishment mechanisms having different identities? (also commented here)

@peppelinux
Copy link
Member

Trust Frameworks might decide to describe how to use a single, general purpose, client_id scheme identifier, with multiple trust evaluation methods. For instance, https or x509_san_uri might be reintroduced by implementation profiles supported by trust frameworks.

I would have preferred client identifiers that are not strictly tied to trust evaluation technology, as defining a client's unique identity serves a different purpose than assessing its trust. The current PR results in client ID schemes that involve distinct, mutually exclusive trust evaluation mechanisms.

It also doesn't give any opportunity for a client to use more than a single client_id scheme or client_id, forcing implementers supporting more than a single trust evaluation method to create different deploys of an equivalent RP configuration. This doesn't match the vision I personally have about scalability of the solutions within multiple contexts.

While this choice has been extensively debated and only partially considered, it introduces a breaking change for systems currently in production. Nevertheless, it's important to remain flexible and seek solutions that simplify our work.

I truly appreciate the efforts of this working group and express my support by facilitating your work. This is the reason for my approval.

@danielfett
Copy link
Contributor

@lj-raidiam @peppelinux A single client ID that supports different schemes (or trust evaluation technologies) may be considered a feature in some settings, but it can be, in fact, a security vulnerability. This was discussed here and the working group decided that a single, clearly identified mechanism is the way forward. That also explains why a generic "https" prefix would have to be tied to a single mechanism (federation) and cannot stand for a range of mechanisms (federation, metadata from somewher else, x509-something, etc.). So "https" would be a hard-coded exception for federation that is not of much value for anyone not using federation.

```

* `https`: This value indicates that the Client Identifier is an Entity Identifier defined in OpenID Federation [@!OpenID.Federation]. Since the Entity Identifier is already defined to start with `https:`, this Client Identifier Prefix MUST NOT be prefixed additionally. 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 Prefix is used. Example Client Identifier: `https://federation-verifier.example.com`.
* `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 Prefix 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.

The meaning of Client Identifier is not perfectly consistent in this spec and even in the scope of this bullet point. Usually it refers to the entire client identifier (also referred to as "full Client Identifier"), while sometimes it is orig_client_id (aka "usual Client Identifier"). Could we make it more specific which Client Identifier identifier it is.

In the redirect_uri we got that (without the prefix redirect_uri:) that didn't look great but did the job. Now when it got removed over there too this section got ambiguous. This problem does not apply only to this bullet.

Copy link

@lj-raidiam lj-raidiam Apr 9, 2025

Choose a reason for hiding this comment

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

Automatic Registration as defined in [@!OpenID.Federation] MUST be used.

With this new wording, what implementers should do to satisfy both the requirements of both VP and OpenID Federation automated registration, given that the latter requires the client_id to be the entity ID, while the former requires adding the openid_federation prefix to it?

Copy link
Member Author

@bc-pi bc-pi Apr 10, 2025

Choose a reason for hiding this comment

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

Is there really ambiguity or confusion about the prefix being a prefix and the other part being the part used as the value to do stuff with? I would like to have the text be clear and do acknowledge that these kinds of things can be difficult to describe precisely in spec text. But I am struggling to see that there's a meaningful problem here. Can you suggest something that would make the text more clear? Something that works in the context of this larger change that has prefixing applied equally to all.

Choose a reason for hiding this comment

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

@bc-pi
I am referring to the difference in e.g. these two statements.
"This value indicates that the Client Identifier is an Entity Identifier"
Example Client Identifier: openid_federation:https://federation-verifier.example.com.

I don't have specific wording that I could suggest but I think that the definition of this prefix should be something like: This prefix indicates that OpenID Federation is in use and the value following it is an Entity Identifier ...

Copy link
Member Author

Choose a reason for hiding this comment

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

0229054 attempts to bring some clarity to that treatment for all the prefixes.

@lj-raidiam
Copy link

@lj-raidiam @peppelinux A single client ID that supports different schemes (or trust evaluation technologies) may be considered a feature in some settings, but it can be, in fact, a security vulnerability. This was discussed here and the working group decided that a single, clearly identified mechanism is the way forward.

@danielfett +1. I remember that. We got that PR that introduced changes that addressed that problem. Nevertheless, after merging we got https and dids without a prefix. Https URLs were pure play identifiers without info on mechanism being part of the identifier. It complied with the Federation because it (and not only it) uses URLs to identify entities and locate metadata.

That also explains why a generic "https" prefix would have to be tied to a single mechanism (federation) and cannot stand for a range of mechanisms (federation, metadata from somewher else, x509-something, etc.). So "https" would be a hard-coded exception for federation that is not of much value for anyone not using federation.

It was a compromise that worked yet was federation-centric. I recognize that there may be many mechanisms that leverage https and ideally they should use pure play https URLs as identifiers, but they'll end up being prefixed in VP irrespective whether there's a default option or not.

Having a default option to maintain the current behaviour maybe wouldn't provide value to others (whoever they are or will be) but I don't see how it could be harmful. Additionally it would be important for harmonization with and increase of chances of adoption of other openid spec with signifiant potential. Merging this PR without federation as the default option on the other hand will cause incompatibility between these specs. That's why I would encourage everyone to rethink why we would not want to do it.

@selfissued selfissued dismissed their stale review April 10, 2025 18:26

As I said in the hybrid DCP meeting on Monday, April 7, 2025, this PR would be the first time to my knowledge that one OpenID working group would introduce normative text that contradicts normative text in an another working group's specification. I don't view this as being a good development for collaboration in the Foundation.

When used with OpenID4VP, it prevents use of Federation Client ID values as they are intended. Yes, developers can work around it and add the prefix when mandated. But I still want to go on record to say that I think this is a truly unfortunate development.

That's said, in the name of facilitating progress and acknowledging rough consensus, I'm no longer going to actively oppose this PR. I look forward there being a final stable OpenID4VP specification.

@Razumain
Copy link

I have problems with this PR, and the path that led to this PR on principle grounds and personal experience of going down similar routes.

The base problem is that the client identifier has overloaded semantics, which I argue is problematic for several reasons.

From the format of the client identifier we decode a number of things:

  1. Whether is forms a redirect URI
  2. Whether it is an OpenID federation entity
  3. What type of mechanism you should use to establish trust (OpenID Federation client registration)

This type of overloaded semantics becomes problem when reality demonstrates new valid combinations that were not realized at the time of design.

It also creates problems when certain applications need to derive the actual identifier to be a fraction of the total identifier.

I'm personally guilty of trying to do these types of solutions with bad results. In general it is much better to define separate parameters and allow them to be combined freely and to keep the identifier as just an identifier without added semantics.

One problem I can see right away with this solution is how we plan to deal with trust in our national eIDAS wallet implementation in Sweden. Here we must support both Trusted List and OpenID federation. It is very possible to allow services to be available through multiple trust infrastructures. But we do not plan to do the client registration, or to enforce it. This is up to each party to decide what functions of the infrastructure they want to use (Trusted List, OpenID federation resolvers, or OpenID federation client registration with chain validation).

If the format of the client identifier limits our freedom to build and offer trust infrastructures, we have big problems.

Isn't there a better alternative than prefixing?

bc-pi and others added 3 commits April 11, 2025 04:14
…er-the-prefix' into equal-treatment-under-the-prefix
…part and what is whole client identifier part and what is the part without prefix part that you use to do stuff after striping the prefix part and what to call what
@bc-pi bc-pi requested review from lj-raidiam and removed request for lj-raidiam April 11, 2025 10:44
@Sakurann Sakurann dismissed lj-raidiam’s stale review April 11, 2025 16:15

verbal agreement that Lukascz that he does not want to block this PR

@Sakurann Sakurann merged commit f621ef8 into main Apr 11, 2025
2 checks passed
@jogu
Copy link
Collaborator

jogu commented Apr 12, 2025

Just for the record, the chairs and other working group members have had several discussions with Lukasz and we appreciate his feedback - we're happy to discuss further once he's back from holiday.

@Razumain We didn't reply to your comment before merging - apologies for that - but please rest assured that it had been read and considered - to respond a little:

The base problem is that the client identifier has overloaded semantics, which I argue is problematic for several reasons.

This isn't a new problem in this PR. And we don't disagree that it's not great, and the initial proposal was not to overload client id - see the client_id_scheme parameter in this older draft: https://openid.net/specs/openid-4-verifiable-presentations-1_0-18.html#section-5

That proposal unfortunately had a security issue ( #124 ) and the only sensible way to solve it was to move the scheme into the client_id. We are expecting the OAuth WG to take a similar approach.

One problem I can see right away with this solution is how we plan to deal with trust in our national eIDAS wallet implementation in Sweden. Here we must support both Trusted List and OpenID federation. It is very possible to allow services to be available through multiple trust infrastructures. But we do not plan to do the client registration, or to enforce it. This is up to each party to decide what functions of the infrastructure they want to use (Trusted List, OpenID federation resolvers, or OpenID federation client registration with chain validation).

If the format of the client identifier limits our freedom to build and offer trust infrastructures, we have big problems.

Note that even before this PR, it would not have been compliant with the specification to use a single client identifier to mean multiple trust schemes. https:// client ids could only mean OpenID Federation.

I think the problem you mention can be solved in at least two different ways:

  1. You could define a new Client Id Prefix with the properties you want of supporting both Trusted List and OpenID Federation.
  2. In the DC API the verifier can sign the request using multiple methods, allowing the wallet to select which one it wants to use.
  3. In the advanced post mode for request_uri, the wallet can indicate to the verifier which methods it supports.

Isn't there a better alternative than prefixing?

We have not been able to come up with one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client Identifier Schemes violate RFC 3986