openid / OpenID4VCI

68 stars 20 forks source link

CWT proof headaches #341

Closed babisRoutis closed 3 months ago

babisRoutis commented 5 months ago

While trying various credential issuers I see 4 common variations to their metadata:

1st

"proof_types_supported": {
    "cwt": {
        "proof_signing_alg_values_supported": [  "ES256"  ]
    }
}

2nd

"proof_types_supported": {
    "cwt": {
        "proof_signing_alg_values_supported": [  "-7" ],
        "proof_alg_values_supported": [ -7 ],
        "proof_crv_values_supported": [ 1 ]
   }
}

3d

"proof_types_supported": {
    "cwt": {
        "proof_alg_values_supported": [ -7 ],
        "proof_crv_values_supported": [ 1 ]
   }
}

4th

"proof_types_supported": {
    "cwt": {
        "proof_signing_alg_values_supported": [ -7 ],
        "proof_alg_values_supported": [ -7 ],
        "proof_crv_values_supported": [ 1 ]
   }
}

To my understand :

Can you please clarify the expected content of proof_signing_alg_values_supported?

Sakurann commented 5 months ago

(separate topic, but @babisRoutis can you please sign OIDF IPR agreement/Contribution agreement? it would make it much easier for us to take in your feedback, which we really appreciate but there is so me process that we need to follow.)

babisRoutis commented 5 months ago

@Sakurann Just singed & send it.

awoie commented 5 months ago

IMO, we should update the OID4VCI cwt section after the POTENTIAL interop event. The POTENTIAL profile (see description of Track 1 in their opencode gitlab) currently defines:

"proof_alg_values_supported": [ -7 ],
"proof_crv_values_supported": [ 1 ]

However, this is not aligned with OID4VCI that REQUIRES "proof_signing_alg_values_supported": [ -7 ],.

Our issuer implementation supports both just in case but fully agree, this should be fixed after the POTENTIAL interop event.

IMO, if we keep cwt proofs, then we should probably keep proof_signing_alg_values_supported because otherwise we would break OID4VCI that requires it and introduce a second parameter, e.g., proof_crv_values_supported that indicates the cryptographic curve (until we have fully-specified algorithms in COSE). proof_alg_values_supported is probably not needed.

bc-pi commented 5 months ago

Noting the existence of #320 "Consider removing cwt proof type" for posterity

babisRoutis commented 3 months ago

Closing this due to #320