openid / OpenID4VCI

68 stars 20 forks source link

Separation of cryptographic_binding_methods_supported and cryptographic_suites_supported parameters may cause problems #214

Closed markuskreusch-bdr closed 9 months ago

markuskreusch-bdr commented 10 months ago

In the credential issuer metadata in credential_configurations_supported the parameters cryptographic_binding_methods_supported and cryptographic_suites_supported are defined as a format unspecific parameter and can be configured separately.

Several problems exist with this approach

  1. When using a single value in cryptographic_binding_methods_supported the meaning of cryptographic_suites_supported is obvious, all included values refer to the one binding method. But if several binding methods are used, it is not that easy. Some of the suites may only be valid for some of the binding methods. Different binding methods may require different values and formats for the same crypto suites (like JOSE and COSE).
  2. Currently the values for cryptographic_suites_supported are tied to the credential format. For jwt_vc IANA.JOSE.ALGS should be used and for ldp_vc LD_SUITES_REGISTRY. In fact the valid values depend more on the cryptographic_binding_method used and not the credential format.
  3. It is unclear which binding method will be used in the credential when several are supported.

I suggest

  1. to remove cryptographic_suites_supported and use objects as values in cryptographic_binding_methods_supported. This way the suites valid for a specific binding method can be mentioned there,
  2. to remove the suggestion to use specific values depending on the credential format and suggest specific values depending on the binding method,
  3. to leave it up to credential format profiles to further constrain the usage of specific binding methods and suites with specific formats and
  4. to discuss problem 3 raised above.

Another option could be to remove both parameters completely as format unspecific parameters and let the credential format profiles define custom parameters if required.

Example:

"credential_configurations_supported": {
        "UniversityDegreeCredential": {
            "format": "jwt_vc_json",
            "scope": "UniversityDegree",
            "cryptographic_binding_methods_supported": {
                "jwk": {
                  "cryptographic_suites_supported": [
                     "ES256"
                  ]
                },
                "cose_key": {
                  "cryptographic_suites_supported": [
                     {
                       "cose_algorithm": -7,  // COSE Algorithms value for ES256
                       "curve": 1 // COSE Curve P-256
                     }
                  ]
                }
            },
            ...
}
paulbastian commented 10 months ago

There also seems to be some overlap with proof_types.

I was also wondering why cryptographic_binding_methods_supported is not cnf but jwk.

It seems that the general sections already contain a lot of profile specific information, so I' leaning towards removing them from Section 11.

Sakurann commented 10 months ago

each entry in credential_configurations_supported is of only one format. And I think each format is pretty clear how the cryptographic binding is represented. mdoc mandates cose_key; jwt credential formats (w3c jwt_vc or ietf sd-jwt vc) can be used only with jwk or DIDs; and LDP credential formats can be used with jwk or LDP. (I don't think an example in the issue to use cose_key to bind a jwt_vc is realistic..)

For LDP case, it's probably not an issue since as you point out, the identifier would be different in ANA.JOSE.ALGS and LD_SUITES_REGISTRY?

Is there a non-hypothetical use-case where, for jwt credential formats (w3c jwt_vc or ietf sd-jwt vc), the issuer supports different crypto_suites for cnf.jwk and DIDs?

Sakurann commented 10 months ago

There also seems to be some overlap with proof_types.

Yes and no. proof_type is about what proof wallet sends to the Issuer to prove possession of the key and cryptographic_binding_methods_supported is about how the issuer represents that key when issuing the credential after validating PoP of the key material. The easiest is to use the same key representation in both proof and cryptographic binding, but the following is possible (even though key transformation would probably be messy)

I was also wondering why cryptographic_binding_methods_supported is not cnf but jwk.

Because not all json-based credential formats mandate cnf (w3c vc-jose doesn't afaik)? but it might be a good idea to revisit these parameter values because I am not sure what cryptographic_binding_methods_supported value should be for DataIntegrityProof..

It seems that the general sections already contain a lot of profile specific information, so I' leaning towards removing them from Section 11.

I would encourage us to tackle parameters one by one as opposed to making sweeping changes.

TimoGlastra commented 10 months ago

I was also wondering why cryptographic_binding_methods_supported is not cnf but jwk.

I think cnf allows multiple ways to do cryptographic binding methods. E.g. you can use a kid in a cnf object that is still a did. So for sd-jwts we are using the entries for how to construct the cnf claims (if jwk we use jwk in the cnf, if did:* we use a kid in the cnf claim to bind it to a did).

I feel like changing jwk to cnf will then assume that cnf is always jwk, which I think is RECOMMENDED in SD-JWT but not required.

it might be a good idea to revisit these parameter values because I am not sure what cryptographic_binding_methods_supported value should be for DataIntegrityProof..

DataIntegrityProof is just VCs right so you'd use the same credentialSubject.id or holder properties to bind the credential right? We're currently implementing DataIIntegrityProof and the same values for cryptographic_binding_methods supported.

For cryptographic_suites_supported the values will need to be different, but that depends on what the outcome of #37 will be, as how I understand it now, there's no need to indicate which cryptosuite from DataIntegrityProof are supported by the issuer, as it will only be used to verify the proof in the credential request (which is cwt or jwt and so only uses algs).

If there needs to be a way to indicate which DI cryptosuites are supported it could be by including the cryptosuite value rather than the proof.type value, as also described here: https://github.com/decentralized-identity/claim-format-registry/issues/8

In this case maybe it would make sense to make it an object, as that would allow to support something like the claim format registry definition for different credential formats. However, this is again dependant on whether it's actually needed to convey this metadata rather than which algs are supported for verifying the credential_request.proof

markuskreusch-bdr commented 9 months ago

With the changes in #235, especially the rename of cryptographic_suites_supported to credential_signing_alg_values_supported, together with the change of the purpose of the parameter, two of the three issues I raised are resolved now.

Concerning

  1. It is unclear which binding method will be used in the credential when several are supported.

I do no longer think this is a real problem. If there is a choice, because a given proof and credential format are compatible with multiple binding methods, it is the issuers decision what to use for now.

It could be considered to let the wallet choose which one to use. This might apply as well to choosing one of the credential_signing_alg_values_supported. But this is probably out of scope here.

Sakurann commented 9 months ago

Thank you for clarifying @markuskreusch-bdr !