search

openid / OpenID4VCI

68 stars 20 forks source link

Consider removing `cwt` proof type #320

Closed awoie closed 3 months ago

awoie commented 6 months ago

It would be worth discussing whether we should remove the cwt proof type for the following reasons:

babisRoutis commented 6 months ago

Totally agree.

TakahikoKawasaki commented 6 months ago

Well, I have already implemented the CWT Key Proof for the Interop Event that POTENTIAL has been hosting. The Interop Event consists of 6 tracks. In the Track 1, CWT Key Proof is used. Demo steps using CWT Key Proof are written here in detail:

4.3. POTENTIAL Interop Event / Track 1 / Light Profile https://www.authlete.com/developers/oid4vci/#43-potential-interop-event--track-1--light-profile

An open-source CBOR library (authlete/cbor) I have written contains a tool for generating a CWT Key Proof. It can be used like below.

git clone git@github.com:authlete/cbor.git
cd cbor
mvn compile
./bin/generate-cwt-key-proof \
    --issuer ${CREDENTIAL_ISSUER_IDENTIFIER} \
    --key ${PRIVATE_KEY_FILE} \
    --client ${CLIENT_ID} \
    --nonce ${C_NONCE}

The name of the property, cwt, naturally implies that the format of a CWT Key Proof conforms to "RFC 8392 CBOR Web Token (CWT)". Therefore, there is no ambiguity. My OID4VCI implementation can accept the following formats.

If the specification of CWT Key Proof were to be removed, someone from the DCP WG would need to explain to the person in charge and the participants of the POTENTIAL Interop Event Track 1 that the CWT Key Proof specification is being discontinued and should not be used.

To discontinue the specification of CWT Key Proof, convincing reasons are needed.

paulbastian commented 6 months ago

@TakahikoKawasaki neither the ISO document nor the POTENTIAL document mentions cwt, in fact the example in ISO is given with jwt, so I think you have some assumptions here that are not reflected in the standards.

Edit: I seem to be wrong... No clue how that got in there

TakahikoKawasaki commented 6 months ago

@paulbastian LSP_POTENTIAL_Interop-Event_Description_Track1_v5.pdf mentions CWT Key Proof. The section, "IV. Parameter specification", explicitly says as follows.

The proof_types_supported value in the credential issuer metadata is cwt

proof_types_supported_in_parameter_specification

In addition, "(2.2) OID4VCI metadata" in "(iii) Worked examples of OID4VCI mdoc profile" shows "a non-normative minimum viable OID4VCI metadata configuration for the light profile" that includes:

// indicates CWT is supported in the credential request as proof type
// may contain additional entries
"proof_types_supported": {
  "cwt": {
    // these indicate ES256 with P-256 is supported
    // may contain additional entries
    "proof_alg_values_supported": [ -7 ],
    "proof_crv_values_supported": [ 1 ]
  }
},
proof_types_supported_including_cwt_only

Also, "(5) Worked examples of credential request/response" contains the following:

{
  "format":"mso_mdoc",
  "doctype":"org.iso.18013.5.1.mDL",
  "proof": {
    "proof_type": "cwt",
    "cwt": "..."
  }
}
cwt_key_proof_in_credential_request

Are you talking about other Tracks of the POTENTIAL Interop Event? For example, Track 2 uses SD-JWT VC and does not use mdoc.

paulbastian commented 6 months ago

I think this has not been present in earlier versions.

The ISO specification still doesn't make a choice on this and I'm tending to agree with Oliver here.

TakahikoKawasaki commented 6 months ago

The operator of POTENTIAL Track 1 removes the previous version from the Git repository when they upload a new version of "LSP_POTENTIAL_Interop-Event_Description_Track1_v{?}.pdf". (IMO, this is a bad practice.)

In any case, the statements "We will not use the CWT Key Proof specification", "We believe people prefer the JWT Key Proof to the CWT Key Proof", "We don't think vendors would implement the CWT Key Proof specification" are different issues from "We should remove the CWT Key Proof specification that has been approved as part of OID4VCI ID1 from the approved specification."

Those who want to prohibit CWT Key Proof can develop their own "profiles" (similar to how POTENTIAL Track 1 has created the "light" profile and the "full" profile), and they don't have to attempt to make breaking changes to the base specification.

bc-pi commented 6 months ago

The name of the property, cwt, naturally implies that the format of a CWT Key Proof conforms to "RFC 8392 CBOR Web Token (CWT)". Therefore, there is no ambiguity.

The suggested ambiguity is in how that binary structure is included as a string in the Credential Request JSON. Which is definitely not specified in OpenID4VCI.

TakahikoKawasaki commented 6 months ago

In standard specifications related to CBOR, CBOR data is represented as hexadecimal strings. I guess that this is primarily because the hexadecimal notation is much easier for explaining the CBOR specification compared to base64(url).

On the other hand, as "7.3. Credential Response" of OID4VCI says as follows:

The encoding of the Credential returned in the credential parameter depends on the Credential Format. Credential Formats expressed as binary data MUST be base64url-encoded and returned as a string.

CBOR-based credentials must be represented in the base64url notation. (I initially used the hexadecimal notation in my OID4VCI implementation, but switched to the base64url notation after discovering this requirement in the specification.)

Therefore, I naturally assumed that a CWT Key Proof in a credential request should also be represented as base64url. However, as you pointed out, the "7.2.1.3. cwt Proof Type" section does not specify the notation for CWT Key Proof.

To resolve this ambiguity, it would suffice to add one sentence to the specification such as "the value of the cwt property MUST be base64url-encoded." Lack of this sentence cannot be a convincing reason to drop the entire specification of CWT Key Proof.

bc-pi commented 6 months ago

... "the value of the cwt property MUST be base64url-encoded." Lack of this sentence cannot be a convincing reason to drop the entire specification of CWT Key Proof.

It's not a reason in and of itself, but it is evidence that very few people are even looking at this part of what is already an arguably too long and complicated specification. I would even suggest that your explanation of how you arrived at base64url versus hexadecimal is further evidence of thereof.

Somewhat related, what did your implementation do with the content type? The way it's defined doesn't seem like it could possibly be correct https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0-13.html#section-7.2.1.3-2.1.2.2

Sakurann commented 5 months ago

since POTENTIAL LSP mandated cwt proof type, i think it would be wise to wait for their implementation feedback before we make a decision to remove or keep cwt proof type.

Sakurann commented 5 months ago

@bc-pi from your emojis, you seem to be very excited at the potential of removing cwt proof type.

The way it's defined doesn't seem like it could possibly be correct https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0-13.html#section-7.2.1.3-2.1.2.2

can you please elaborate why? cwt claims can be of value type text string.

babisRoutis commented 5 months ago

I believe that @awoie in his initial comment provided the best reason for simplifying VCI by removing CWT proofs:

It cannot be used with attestation-based client authentication ( A requirement of ISO23220-3 & HAIP, if I am not mistaken)

Even for POTENTIAL LSP it is not clear, at least to me, how & if attestation-based client authentication can be implemented in their full profile when using CWT.

awoie commented 5 months ago

Another issue that I have with cwt is the usage of the String label COSE_Key to convey the actual key. CWTs typically use integer identifier for this. Using String labels is not the natural way COSE would do that.

Instead of using COSE_Key the spec should have probably used cnf (8) with COSE_Key (1).

awoie commented 5 months ago

I think this has not been present in earlier versions.

The ISO specification still doesn't make a choice on this and I'm tending to agree with Oliver here.

JFYI: ISO does not use cwt either. I remember when we had our initial conversations with the interop event organizers that they insisted on cwt (for Track 1) which is why the latest profile is using it as well. AFAIK, the initial proposal we did on Track 1 used jwt.

awoie commented 5 months ago

Another issue I found is the requirement to use the content type header which MUST be set to openid4vci-proof+cwt. This is not how the content type header is used normally. It indicates what is secured, not what the CWT is. Note that the jwt proof type uses typ instead of cty. IANA just registered typ for CWTs and assigned the number 16.

paulbastian commented 5 months ago

My view:

TakahikoKawasaki commented 5 months ago

The OID4VCI specification is agnostic about client authentication methods. ISO 23220-3 and HAIP may require wallet attestation, but the OID4VCI specification itself does not require wallet attestation and it allows public clients and even anonymous clients (cf. pre-authorized_grant_anonymous_access_supported).

bc-pi commented 5 months ago

@bc-pi from your emojis, you seem to be very excited at the potential of removing cwt proof type.

The way it's defined doesn't seem like it could possibly be correct https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0-13.html#section-7.2.1.3-2.1.2.2

can you please elaborate why? cwt claims can be of value type text string.

@awoie touches on this in https://github.com/openid/OpenID4VCI/issues/320#issuecomment-2133474858 but the way that 4VCI uses the content type COSE header there at where I linked to previously https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0-13.html#section-7.2.1.3-2.1.2.2 is not semantically or syntactically correct.

babisRoutis commented 4 months ago

since POTENTIAL LSP mandated cwt proof type, i think it would be wise to wait for their implementation feedback before we make a decision to remove or keep cwt proof type.

Was there any feedback?

mickrau commented 4 months ago

my feedback from the POTENTIAL LSP:

We did indeed have some interop problems with the cwt proof at the beginning of the lsp interop, caused by:

  1. Encoding of cwt element in Credential Request. Base64url seems natural, but it is not clearly defined.
  2. Outer tag of cwt proof. There are three patterns (RFC 8392):

After conversations between the teams we had some compatible services in the end. I think for points 1+2 giving guidance makes sense. Point 3 seems counterintuitive to me, but is at least clearly defined.

babisRoutis commented 4 months ago

Hi @mickrau

Indeed these were also - more or less - my findings. I would add the variations in the related credential issuer metadata described #341

So, there are 4 points - to my understanding to improve CWT requirements in VCI:

  1. Encoding of CWT proof (base64url) needs to explicitly stated
  2. CWT proof CBOR encoding (which of the 3 variants have to be supported by wallets/issuers? Could be just one?)
  3. Cose_key encoding

  4. 354

Yet the main question of the present issue is whether cwt proofs should be removed

TakahikoKawasaki commented 4 months ago

The fact that the removal is currently being discussed means that someone in the past proposed adding CWT Key Proof to the OID4VCI specification. It would be better to have that person explain what necessity they felt in proposing the addition of CWT Key Proof to the OID4VCI specification. If this is not clarified, developers are likely to become reluctant to actively follow future changes or additions to the OID4VCI specification. They may decide to wait until the specification stabilizes, making it more difficult for the working group to receive valuable feedback from implementers.

jogu commented 4 months ago

The fact that the removal is currently being discussed means that someone in the past proposed adding CWT Key Proof to the OID4VCI specification. It would be better to have that person explain what necessity they felt in proposing the addition of CWT Key Proof to the OID4VCI specification. If this is not clarified, developers are likely to become reluctant to actively follow future changes or additions to the OID4VCI specification. They may decide to wait until the specification stabilizes, making it more difficult for the working group to receive valuable feedback from implementers.

I checked out the history - the CWT proof type was added here:

https://bitbucket.org/openid/connect/pull-requests/384/overview

I couldn't find an issue linked to that PR, but a rationale is mentioned by Oliver on the PR:

In general, I think this is a useful PR and as ISO mdoc implementers, I believe this PR could make the implementer’s life easier since otherwise implementers would need to convert JWK to COSE_Key which is not great.

Almost everyone involved in that PR was on yesterday's call and no one on the call wanted to retain the CWT proof type after the experience in the Potential Large Scale Pilot interop; the impression I got was that it has turned out that a pure CBOR world is no longer a reality (as other surrounding infrastructure requires JWTs, e.g. when presenting mdls the wallet has to parse a signed jwt request object, and wallet attestations only currently exist in JWT format) so having multiple proof types is now a burden rather than a help.

babisRoutis commented 4 months ago

so having multiple proof types is now a burden rather than a help.

Looking back over the last 10 months or so, VCI changed significantly. Refinements, new features, simplifications, clarifications, that, at the end of the day, have improved the spec. I have the impression that early adopters, recognize these efforts and keep up with the spec changes.

FWIW, I believe that enough arguments have been made & adequate feedback has been provided to justify the removal of CWT proofs.

Sakurann commented 3 months ago

agreed to remove on the wg call

jogu commented 3 months ago

Kristina sent an email to the working group on 23rd July too asking for feedback on removing this: https://lists.openid.net/pipermail/openid-specs-digital-credentials-protocols/Week-of-Mon-20240722/000406.html (and no feedback received so far other than above comments on the issue

deshmukhrajvardhan commented 3 months ago

Agree with removing cwt.

I was recently introduced to SPICE (Secure Patterns for Internet CrEdentials) https://datatracker.ietf.org/wg/spice/documents/

and they have a few docs related to using CBOR instead of JSON for OIDC. My impression is that, If that work continues we will have separate implementation for it and have a clean delineation.