OpenID4VCI icon indicating copy to clipboard operation
OpenID4VCI copied to clipboard

Wallet can't always identity whether key proof is required

Open jogu opened this issue 1 year ago • 12 comments

https://github.com/openid/OpenID4VCI/pull/87 added a mechanism within credentials_configurations_supported for the wallet to know if the issuer requires proof of possession for a particular credential configuration, and if so what types are supported.

There are some cases where the wallet can't lookup this value though / it isn't clear to the credential issuer which credential the wallet is asking for.

An oversimplified example that shows the problem, e.g. in the pre-authorised code flow where the wallet has been offered more than one credential:

credential_configurations_supported

{
  "credential_1": {
    "format": "my_format",
    "claims": { "family_name": {} }
  },
  "credential_2": {
    "format": "my_format",
    "claims": { "family_name": {} },
    "proof_types": [ "jwt" ]
  }
}

credential offer

{
  "credential_issuer": "...",
  "credential_configurations": [ "credential_1", "credential_2" ],
  "grants": { ... }
}

credential request

{
  "format": "my_format",
  "claims": { "family_name": {} }
}

This could be solved by having the wallet supply credential_configuration_id in the credential request. (Hence making format unnecessary.) i.e.:

credential request

{
  "credential_configuration_id": "credential_2",
  "claims": { "family_name": {} }
}

(Spotted by Taka@Authlete)

jogu avatar Dec 28 '23 03:12 jogu

I think this is related to issues #132 and #197.

Sakurann avatar Jan 31 '24 05:01 Sakurann

Credential is always identified by the combination of format and type (type claims are mandatory in the credential format profiles).

proof_types_supported is optional to enable the use-cases when issuer wants to issue bearer credentials. we can probably make this clearer in the description that if the issuer requires key binding, the parameter must be present.

if issuer metadata is used, wallet can look up proof_types_supported based on the credential_configuration_id. credential_configuraiton_id can be received in the credential offer. if the wallet is starting auth code flow without the offer, it will still need to know the scope or credential_configuration_id to put into the authorization_details from the issuer metadata, so the wallet should be able to look up proof_types_supported too.

Sakurann avatar Feb 19 '24 19:02 Sakurann

Credential is always identified by the combination of format and type (type claims are mandatory in the credential format profiles).

I checked with Taka and although it might be the intention that format & type uniquely identify a credential_configuration_id, we don't seem to actually say that as far as I can see. So e.g. here is an updated example that has vct but still have the ambiguity:

credential_configurations_supported

{
  "credential_1": {
    "format": "vc+sd-jwt",
    "vct": "SD_JWT_VC_example"
    "claims": { "family_name": {} }
  },
  "credential_2": {
    "format": "vc+sd-jwt",
    "vct": "SD_JWT_VC_example"
    "claims": { "family_name": {} },
    "proof_types": [ "jwt" ]
  }
}

credential offer

{
  "credential_issuer": "...",
  "credential_configuration_ids": [ "credential_1", "credential_2" ],
  "grants": { ... }
}

credential request

{
  "format": "my_format",
  "vct": "SD_JWT_VC_example"
}

The fix would be I guess to say something like a vct value must only be present once in an issuer's metadata?

jogu avatar Feb 21 '24 14:02 jogu

I have the fear that in the long run, saying format+type is enough could hurt us, because this puts significant requirements on the possibly supported credential formats and maybe in the future this constraint doesn't make sense anymore.

I consider this a flaw of the protocol and it keeps popping up from time to time. The ideal solution in my mind is to either:

  • make credential instance identifier from token response mandatory and use that one in the credential request
  • use credential supported identifier in the credential request

The first one is superior and makes sense in the proposed post-id1 changes but two is still a good improvement

paulbastian avatar Feb 21 '24 17:02 paulbastian

@jogu, it feels like multiple problems are being confused. a problem that one of the credential_configurations_supported object lacks proofs_supported so the wallet does not know which proof_type is supported is different from what to do when two credential_configurations_supported objects have different credential_configuration_ids, but the same combination of format and type.

Sakurann avatar Feb 21 '24 17:02 Sakurann

for the first problem, as I said previously, we can make it clearer in the description that if the issuer requires key binding, the parameter must be present.

for the second problem, would like to continue a discussion in #132. but to reiterate here:

  • making credential instance identifier from token response mandatory and use that one in the credential request would not work for the implementations that re-use existing ASs and cannot make breaking changes to them.
  • using credential supported identifier in the credential request would work very nicely and aligns to what is being proposed/discussed in #132.
  • to say something like a vct value must only be present once in an issuer's metadata would probably limit use-cases, as I think we had a use-case where we were using same format/type combination, but different information for the rest of the metadata (like display).

Sakurann avatar Feb 21 '24 17:02 Sakurann

@jogu it feels like multiple problems are being confused. a problem that one of the credential_configurations_supported object lacks proofs_supported so the wallet does not know which proof_type is supported is different from what to do when two credential_configurations_supported objects have different credential_configuration_ids, but the same combination of format and type.

To be clear, the intention of this issue was to discuss the second problem. (It seemed clear to me that a credential_configurations_supported without proofs_supported means that a proof is not required for that credential. It that's not actually clear I'm happy for it to be fixed.)

I think you're right that it overlaps with #132, can we expand #132 to include the case where RAR is used and a credential_configuration_id is not returned?

jogu avatar Feb 21 '24 18:02 jogu

make credential instance identifier from token response mandatory and use that one in the credential request

Our implementation experience found that this was essential, especially when a user/wallet can have two credentials of the same type but with slightly different contents e.g. Chemistry Degree and Maths Degree from the same university.

David-Chadwick avatar Feb 21 '24 18:02 David-Chadwick

  • making credential instance identifier from token response mandatory and use that one in the credential request would not work for the implementations that re-use existing ASs and cannot make breaking changes to them.

This is occasionally stated, but I always wonder how an authorization server implementation that is too inflexible to add a new parameter in a token response can support the OID4VCI specification. The assumption doesn't sound very convincing.

If the discussion on whether to add a new parameter to the token response is hindered to protect the implementation of a specific company, it is a significant concern for the community. (cf. https://github.com/openid/OpenID4VCI/pull/276#discussion_r1500427191 )

TakahikoKawasaki avatar Feb 23 '24 10:02 TakahikoKawasaki

  • making credential instance identifier from token response mandatory and use that one in the credential request would not work for the implementations that re-use existing ASs and cannot make breaking changes to them.

This is occasionally stated, but I always wonder how an authorization server implementation that is too inflexible to add a new parameter in a token response can support the OID4VCI specification. The assumption doesn't sound very convincing.

If the discussion on whether to add a new parameter to the token response is hindered to protect the implementation of a specific company, it is a significant concern for the community. (cf. https://github.com/openid/OpenID4VCI/pull/276#discussion_r1500427191 )

For existing AS, couldn't you make a wrapper AS that supports these additional features and adds the credential instance identifier to the token response?

paulbastian avatar Feb 23 '24 16:02 paulbastian

@paulbastian

For existing AS, couldn't you make a wrapper AS that supports these additional features and adds the credential instance identifier to the token response?

In theory, to some extent, in some situations. I don't think it would work and/or be possible in some situations and it potentially gets confusing. I think it would need to be more than a simple wrapper and much closer to a full fledged AS that is federating out to the underlying AS, unless they two ASes because intrinsically coupled in their backends.

The VCI spec has so far catered, albeit with reduced functionality, for existing Authorization Servers without them needing to change (other than adding new scope values and other fairly standard things). Departing from that goal would be a significant change and may harm adoption of VCI.

jogu avatar Mar 04 '24 13:03 jogu

@TakahikoKawasaki there is more than one company that has ASs that have limitations that do not allow them to make changes easily or to have intermediary ASs/wrappers, etc. and as a community we should value every single company's implementation.

Sakurann avatar Mar 05 '24 05:03 Sakurann

@paulbastian @c2bo is this addressed in #389 PR?

Sakurann avatar Oct 28 '24 20:10 Sakurann

@paulbastian @c2bo is this addressed in #389 PR?

I don't think key attestation helps us here - #392 seems more relevant but doesn't seem to fully cover this?

c2bo avatar Oct 28 '24 21:10 c2bo

I believe that this is covered by the resolution on #392 of today's DCP Hybrid Call :+1:

paulbastian avatar Oct 28 '24 21:10 paulbastian