search

openid / OpenID4VP

56 stars 20 forks source link

Remove `did:` based client ID's #278

Open tplooker opened 1 month ago

tplooker commented 1 month ago

Practically speaking I don't believe DID based client ID's offer much in the way of value for the OpenID4VP protocol and instead only add complexity to implementations. The current text for did: based client ID's is generally under-defined in the sense that:

To that end I believe the simplest path is to remove this feature from OpenID4VP, it doesn't prevent implementations from using DID's in other places such as credential binding if they so wish.

bc-pi commented 1 month ago

It also adds complexity to cognitive overhead involved in just trying to comprehend the document itself.

bc-pi commented 1 month ago

Remove it

Sakurann commented 1 month ago

don't remove it.

Sakurann commented 1 month ago

I don't understand how it is underspecified more than others. majority of client_id_schemes need something additional to be defined, not just DID one. like verifier_attestation scheme requires it to be defined how to get the keys to validate that verifier attestation jwt. openid federation leaves a lot of things open too.

profiles are supposed to define details for did method and they do. like here: https://identity.foundation/jwt-vc-presentation-profile/

in the end of the day, did: client_id_scheme is optional, if you don't like it, don't use it.

tplooker commented 1 month ago

don't remove it.

Can you please elaborate on why not?

I don't understand how it is underspecified more than others. majority of client_id_schemes need something additional to be defined, not just DID one. like verifier_attestation scheme requires it to be defined how to get the keys to validate that verifier attestation jwt. openid federation leaves a lot of things open too.

I've clearly stated above how it is under-defined, even though its an optional feature it creates complexity and burden for implementers that attempt to use it and for us as editors to maintain it. The feature needs to be properly defined or removed, I'm in favour of removing.

bj-ms commented 1 month ago

Practically speaking I don't believe DID based client ID's offer much in the way of value for the OpenID4VP protocol and instead only add complexity to implementations. The current text for did: based client ID's is generally under-defined in the sense that:

  • Which DID methods implementations are suppose to support are not defined.
  • Which verification method types implementations are suppose to support are not defined.
  • Which key representations formats inside the verification method types is not sufficiently defined.

To that end I believe the simplest path is to remove this feature from OpenID4VP, it doesn't prevent implementations from using DID's in other places such as credential binding if they so wish.

If did: is removed from the client_id scheme, how will verifiers with DIDs be able to authenticate themselves to the wallet?

I would argue that this option is necessary to validate a signed authorization request sent to the wallet by a verifier with a DID.

From your points above, one addition that makes sense to me is to define what type of public key encoding is supported.

nemqe commented 1 month ago

I always saw did: support here as an enabler for DID centric ecosystems, and then it is their business what specific DID methods they want to use, what key types they want to support, and in what format they want to present them - all of which is defined in the base DID spec and the corresponding method specs.

What is the main concern of keeping this in, do we worry about wallets needing to support things like DIDs and not knowing what to expect because the range of possible methods is big?

tplooker commented 1 month ago

What is the main concern of keeping this in, do we worry about wallets needing to support things like DIDs and not knowing what to expect because the range of possible methods is big?

If we remove this, DID's can still be used in otherways with the OpenID* family, including the issuer identifier and for binding a credential in formats like W3C VC's. The issue for using it as a client ID is that it is creating quite a bit of implementation complexity for no clear value. We either need to fully define how to use DIDs here which is quite a lift OR remove it.

nemqe commented 1 month ago

With the risk of sounding stupid, what would the expectation be here, for verifiers that currently use DIDs to switch to a different client_id_scheme like federation, x509, or attestation JWTs?

tplooker commented 1 month ago

With the risk of sounding stupid, what would the expectation be here, for verifiers that currently use DIDs to switch to a different client_id_scheme like federation, x509, or attestation JWTs?

Yes exactly.

bj-ms commented 1 month ago

We plan to use OpenID4VP with DIDs and the other client_id_schemes do not work for us because we would either have to set up new infrastructure to support federation/x509 or a new service to issue JWT attestations instead of just using DIDs which we already support.

OpenID4VP is in my eyes format and identifier agnostic and I would keep that aspect and suggest to fully define how to use DIDs here.

nemqe commented 1 month ago

We too use DIDs, and we haven't had too much problems with implementing it to be completely honest for the methods we chose to support. Now I agree, there are some pain points, and specificities, but these are things common for the DIDs themselves as things are a tad too fragmented, not the OpenID4VC protocol in my honest opinion.

I would agree with @bj-ms that the other client_id_schemes might not be a good fit:

I would prefer to keep this "hook" for DID based ecosystems so that DIDs can be used to sign all sort of stuff (credentials, PoPs, JARs, id tokens...) in a similar manner (kid in the header). Possibly there are some gaps, and it might be under-specified, but if that is the concensus I would rather volunteer my time to work on it along with other interested parties from the group to close the gaps and specify it better compared to seeing it being removed.

alenhorvat commented 4 weeks ago

@tplooker, by defining

do you consider that the definition will be sufficient?

Because:

Let's also remove x509. TLS certificate is in an x509 format, so is a key I generate locally. Furthermore, I can create an x509 certificate with a DID in it as a CN or SAN.URI.

Also for OIDC/OAuth schemas: what is the entity identifier if their service is hosted? (https://github.com/oauth-wg/oauth-sd-jwt-vc/issues/253)

awoie commented 3 weeks ago

If there was a profile (not in OID4VP) of how to use DIDs with JWTs to express the following:

Then, we could easily drop did: and ask people to use Verifier Attestations instead.

I'm suggesting to create this profile somewhere else since other specs would also benefit from that, e.g., SD-JWT VC, or even W3C JOSE-COSE. I would assume that such a profile would only use JWKs to represent keys.

alenhorvat commented 3 weeks ago

Following this logic, I propose to remove all references to client identifiers, as other specs would also benefit from that.

awoie commented 3 weeks ago

Following this logic, I propose to remove all references to client identifiers, as other specs would also benefit from that.

I was proposing to merge Verifier Attestations with DIDs. The other proposal was because how DIDs are used in PoPs and JWTs etc. is a bit underspecified in general, I proposed to define this in a dedicated spec.

alenhorvat commented 3 weeks ago

But still, current specification states:

"How the trust is established between Wallet and Issuer and how the public key is obtained for validating the attestation's signature is out of scope of this specification." yet, "did" and "entity_id", "x509*" are being defined where each of them defines how to obtain the keys and how trust is established.

Either all of them are encapsulated within the Verifier JWT or neither. IMO Verifier JWT is at a different level than the aforementioned identifiers. Namely:

Since DID specs define how to resolve the keys, it's unclear what should be defined in addition to the resolution and referencing the DID keys. If this specification defines (clearly) how to use the certificates/... to validate the signatures for all identifiers, it's unclear what is the exact argument for removing the DID as identifier.

decentralgabe commented 3 weeks ago

These political games are unnecessary and need to end.

DIDs are useful to many, including my company, and plenty of other organizations that have shipped products with customers relying on them. Removing their support here may make your world simpler, but that's not how we should make decisions, right? Standards are about listening to the community, implementers, and understanding a diverse set of needs and making decisions based on those needs.

Which DID methods implementations are suppose to support are not defined.

Irrelevant. Left up to the implementer. If you wish you can specify a common resolution interface DID methods must conform to. In fact this work is actively ongoing in the DID WG and has already been proven at scale independent of this standardization effort.

Which verification method types implementations are suppose to support are not defined. Which key representations formats inside the verification method types is not sufficiently defined.

Also irrelevant. If you wish, say JWK and multikey. Or just one. This is not insurmountable.

Of course, you already know this. You have previously been involved in both DIF and the W3C efforts for quite some time.

If there are any legitimate technical hurdles to be overcome (I haven't heard any) then please open an issue and assign it to me to add language to this spec. I'll do the work. Attempting to shut down DIDs and other W3C specs you don't care for is not the right path.

If not, this issue needs to be closed, and these types of issues need to stop popping up. We have more important things to work on.

jogu commented 3 weeks ago

@decentralgabe Just to double check, can you confirm if you/your company are using did: values specifically in the client_id authorization request parameter please?

decentralgabe commented 3 weeks ago

@jogu that's correct; clients are identified by DID in the auth request.

peacekeeper commented 3 weeks ago

In general, -1 to removing support for DIDs, for the reasons listed here: https://github.com/oauth-wg/oauth-sd-jwt-vc/issues/250#issuecomment-2256016913

However, I have to say that I found @awoie 's idea in https://github.com/openid/OpenID4VP/issues/278#issuecomment-2422455336 not un-interesting, i.e. defining a generic mechanism for using DIDs with JWTs, which can then be re-used in various other building blocks including in verifier attestations. But on the other hand, if we actually "drop did: and ask people to use Verifier Attestations instead", wouldn't that then exclude use cases where you already know how to trust a did: client ID in other ways, without requiring a separate verifier attestation?

earizon commented 2 weeks ago

My intuition tells me that providing hints about did client_id identification makes code implementations simpler.

If a given application does not support did client_id, it just need to check for this value and fail fast with a sensitive error instead of trying to continue with "random execution paths" until if find "deep in the stack" that some credential in some yet undefined format has a subject of type did that it can not validate.

The fail-fast approach is a pattern that always work since it reduces software entropy: Code that is not executed can not fail!.