Add version and request type to protocol identifier for DC API #381

awoie · 2025-01-10T13:50:30Z

This PR adds version and request type to the protocol identifier for DC API.

Fixes #326, fixes #363

awoie · 2025-01-10T14:04:09Z

Is there any issue with setting the value to unsigned although the request was signed? Does this open up similar attack vectors as alg=none?

c2bo

Not entirely sure about the protocolpart in the urn - do we need that? What about urn:openid:openid4vp:1.0:signed?
Also we should stick with openid4vp (and openid4vci) instead of oid4vp imho

c2bo · 2025-01-10T14:42:15Z

openid-4-verifiable-presentations-1_0.md

@@ -1958,7 +1958,14 @@ And lastly, as part of the request, the Wallet is provided with information abou

 ## Protocol

-To use OpenID4VP over the DC API, the value of the exchange protocol used with the Digital Credentials API (DC API), is `openid4vp`.
+To use OpenID4VP with the Digital Credentials API (DC API), the exchange protocol value has the following format: `urn:openid:protocol:oid4vp:<version>:<request-type>`. The `<version>` field adheres to semantic versioning, and `<request-type>` explicitly specifies the type of request. This approach eliminates the need for wallets to perform implicit parameter matching to accurately identify the version and the expected request and response parameters.


Suggested change

To use OpenID4VP with the Digital Credentials API (DC API), the exchange protocol value has the following format: `urn:openid:protocol:oid4vp:<version>:<request-type>`. The `<version>` field adheres to semantic versioning, and `<request-type>` explicitly specifies the type of request. This approach eliminates the need for wallets to perform implicit parameter matching to accurately identify the version and the expected request and response parameters.

To use OpenID4VP with the Digital Credentials API (DC API), the exchange protocol value has the following format: `urn:openid:protocol:openid4vp:<version>:<request-type>`. The `<version>` field adheres to semantic versioning, and `<request-type>` explicitly specifies the type of request. This approach eliminates the need for wallets to perform implicit parameter matching to accurately identify the version and the expected request and response parameters.

If there are no requirements, I would use the shortest version possible, e.g., urn:openid4vp:1.0:signed, urn:openid4vp:1.0:unsigned. What do you think @c2bo @timcappalli ?

Fine with dropping "protocol" but I believe keeping the org name is common practice. If length is the concern, maybe just use "oidf"? So it would be urn:oidf:openid4vp:1.0:signed.

@selfissued should weigh in here.

I am happy with either one.

Adding the oidf seems to be a bit cleaner in terms of namespacing, but I don't think it's likely to encounter a naming clash here, so I guess urn:openid4vp:1.0:signed would be fine as well if we need/want to optimize for size?

so there is some OIDF precedent. CIBA defines a URN-based grant type that includes both the organization and the type (in this case params): urn:openid:params:grant-type:ciba. There's also urn:openid:params:jwt:claim:auth_req_id and urn:openid:params:jwt:claim:rt_hash.

So if we wanted to be consistent with other OIDF specs, I think we'd want it to be urn:openid:protocol:openid4vp:1.0:signed.

@selfissued @ve7jtb any comments on this?

Current proposal is:

Unsigned requests: urn:openid:protocol:openid4vp:1.0:unsigned

Signed requests: urn:openid:protocol:openid4vp:1.0:signed

openid-4-verifiable-presentations-1_0.md

hlozi

in general the request signature has value only if the Wallet can verify it. if the wallet can't verify the signature (for example in case of not trusting the chain the RP used) the request is considered by the wallet to be unsigned.

openid-4-verifiable-presentations-1_0.md

hlozi

This could potentially lead to relying on the indicator as if the verification was performed by the Wallet, which may choose not to verify the signature.

… with OIDF

awoie · 2025-01-20T15:33:30Z

e chain the RP used) th

Could you please elaborate this a bit further? I'm not sure if this is related to this PR.

Sakurann · 2025-01-20T20:16:44Z

@hlozi I think from the perspective of processing rules in OpenID4VP, if the verifier signed the request, and the wallet cannot validate it, the wallet needs to abort the request. when the request is signed it's not wallet's choice to validate it or not, the wallet muct validate it

paulbastian · 2025-01-21T08:58:52Z

Not entirely sure about the protocolpart in the urn - do we need that? What about urn:openid:openid4vp:1.0:signed?
Also we should stick with openid4vp (and openid4vci) instead of oid4vp imho

Would agree here, aren't those always "protocols" that do into DC API?

timcappalli · 2025-01-21T14:15:41Z

Not entirely sure about the protocolpart in the urn - do we need that? What about urn:openid:openid4vp:1.0:signed?
Also we should stick with openid4vp (and openid4vci) instead of oid4vp imho

Would agree here, aren't those always "protocols" that do into DC API?

While I agree it seems unnecessary at face, isn't there value in being consistent with other OIDF URNs?

bc-pi · 2025-01-21T17:38:13Z

openid-4-verifiable-presentations-1_0.md

+The following exchange protocol values are defined by this specification:
+
+* Unsigned requests: `urn:openid:protocol:openid4vp:1.0:unsigned`
+* Signed requests: `urn:openid:protocol:openid4vp:1.0:signed`


As a thought experiment, what happens when OpenID4VP v1.1 comes along and is compatible with v1.0 but not strictly v1.0?

and what of #308?

IMO, the version identifier does not have to match the OID4VP version. In this regards, we might have to think about if this PR should really set the version to 1.0 already or if we should start with 0.1 first.

The version in the protocol identifier will need to be mapped back to a specification in the registry, so the version needs to at a minimum be defined somewhere in the spec, if its not going to the be the spec version itself.

Or, and hear me out, having that level of specificity in the identifier is not a good idea.

agreement that there is a need to differentiate major versions (not differentiating minor versions like 1.1, 1.2, etc.).

version is part of URN, i.e., :1.0

based on that agreement, bullet 1 should be just :1 or :v1

yes

just major

yes

urn

1.No
2.voted No on 1 but in case we do versioning, just major.
3.No,
4.just a string, we can avoid collisions using a Reverse domain name.

Answers to @timcappalli questions

Yes

Major

Yes

urn

yes

just major is fine

not sure (not opinionated)

urn sounds reasonable to achieve the goal

Sakurann · 2025-01-21T19:14:42Z

openid-4-verifiable-presentations-1_0.md

+To use OpenID4VP with the Digital Credentials API (DC API), the exchange protocol value has the following format: `urn:openid:protocol:openid4vp:<version>:<request-type>`. The `<version>` field adheres to semantic versioning, and `<request-type>` explicitly specifies the type of request. This approach eliminates the need for wallets to perform implicit parameter matching to accurately identify the version and the expected request and response parameters.
+
+The value `1.0` MUST be used for the `<version>` field to indicate the request and response conform to this version of the specification. For `<request-type>`, unsigned requests, as defined in (#unsigned_request), MUST use `unsigned`, and signed requests, as defined in (#signed_request), MUST use `signed`.
+
+The following exchange protocol values are defined by this specification:
+
+* Unsigned requests: `urn:openid:protocol:openid4vp:1.0:unsigned`
+* Signed requests: `urn:openid:protocol:openid4vp:1.0:signed`


it increasingly seems better long-term not to include both version and signed/unsigned in the protocol identifier:

no version, because for 1.0, we know there will be backward compatible 1.1, 1.2, etc. and having to maintain those most likely will turn out to be an unnecessary maintenance overhead.

no signed/unsigned because wallets already need to differentiate signed/unsigned requests and is not worth the complexity of asking the platforms to support both.

Suggested change

To use OpenID4VP with the Digital Credentials API (DC API), the exchange protocol value has the following format: `urn:openid:protocol:openid4vp:<version>:<request-type>`. The `<version>` field adheres to semantic versioning, and `<request-type>` explicitly specifies the type of request. This approach eliminates the need for wallets to perform implicit parameter matching to accurately identify the version and the expected request and response parameters.

The value `1.0` MUST be used for the `<version>` field to indicate the request and response conform to this version of the specification. For `<request-type>`, unsigned requests, as defined in (#unsigned_request), MUST use `unsigned`, and signed requests, as defined in (#signed_request), MUST use `signed`.

The following exchange protocol values are defined by this specification:

* Unsigned requests: `urn:openid:protocol:openid4vp:1.0:unsigned`

* Signed requests: `urn:openid:protocol:openid4vp:1.0:signed`

To use OpenID4VP over the DC API, the value of the exchange protocol used with the Digital Credentials API (DC API), is `urn:openid:protocol:openid4vp`.

Seems much better for a wallet to not even to have to start parsing a request if it knows it cannot support the options being presented, rather than to always have to parse requests it subsequently finds out it cannot support

If not in the protocol string (so that platforms/wallet don't have to support multiple "protocols"), I would like to see the signed vs unsigned indication somewhere in the request

It feels like bad practice to have wallets determine the type of request by failing to parse the request and then trying the other option. For example, this would make it hard to debug (and print easy-to-understand log messages) for truly malformed requests, since that could mean either: the request is actually malformed, or it is of the other type...

For the version: I agree with the comment from awoie yesterday essentially stating that the OpenID4VP spec version and the "API" version might be different concepts.

If theres agreement with that, we would just need to find some versioning system that each OpenID4VP Spec version is in charge of clearly indicating (and is semantically different from the OpenID4VP major.minor scheme to avoid having folks correlate the two concepts).

I liked v1, and any non-backwards compatible change in the spec can be update the "API" version to v2

Sakurann

having heard more opinions, i think version and signed/unsigned should be dropped

timcappalli · 2025-01-21T19:39:45Z

This has been discussed extensively in both groups and consensus was reached. Why the sudden change?

bc-pi

having a minor version is a nonstarter for me

signed vs. unsigned (vs. signed many times) seems unnecessary but I understand that some/many want it

changes made after approval

openid-4-verifiable-presentations-1_0.md

Co-authored-by: Giuseppe De Marco <[email protected]>

awoie · 2025-01-30T13:41:12Z

@hlozi @timcappalli @David-Chadwick @peppelinux @Sakurann @bc-pi @dyiop I updated the PR to remove the minor version from the <version> field. Please review it again. So far, I've seen more support for the unsigned/signed distinction, so I haven't made any changes to that part.

openid-4-verifiable-presentations-1_0.md

Co-authored-by: Joseph Heenan <[email protected]>

openid-4-verifiable-presentations-1_0.md

Co-authored-by: Kristina <[email protected]>

bc-pi

I now "don't hate" this enough to approve. [but forgot to approve then did later]

c2bo · 2025-01-30T19:19:55Z

I don't have a strong opinion on the signed vs unsigned suffix and from an implementation perspective, having an identifier for signed seems to make ones life easier with parsing which is good.

One concern I have would be #395. If we agree that this kind of flow is a good idea and want to support such a planned downgrade in the protocol, then that would also mean that we are switching from a signed request to a response that would belong to an unsigned flow.

This wouldn't change the data structure of the response, but having two different identifiers for signed and unsigned and then switching between them in a request/response sequence would be pretty weird imho.

better now

bc-pi

I now "don't hate" this enough to approve.

dyiop · 2025-01-30T22:37:07Z

Will there be any part of the request which can tell the Wallet which minor version this request is using?

This could be useful for the Wallet to track errors for different minor versions coming in from various verifiers?

Agreed that it should not need to be part of the protocol identifier!

Otherwise LGTM!

awoie · 2025-01-31T09:24:55Z

Will there be any part of the request which can tell the Wallet which minor version this request is using?

The protocol identifier won't have any minor version. I believe this was the consensus that was achieved in this PR and in the DCP WG meeting.

Sakurann

I now "don't hate" this enough to approve.

same

@hlozi ? your request for changes is one remaining

hlozi

I’m still not convinced about making the identifier format too complicated. In reality, it’ll mostly be used to uniquely identify a specification, so the browser will still need to look at the payload regardless. I guess this is a discussion for the DC API group, not this group so I don't want to block this.

openid-4-verifiable-presentations-1_0.md

hlozi · 2025-01-21T20:45:30Z

openid-4-verifiable-presentations-1_0.md

+The following exchange protocol values are defined by this specification:
+
+* Unsigned requests: `urn:openid:protocol:openid4vp:1.0:unsigned`
+* Signed requests: `urn:openid:protocol:openid4vp:1.0:signed`


1.No
2.voted No on 1 but in case we do versioning, just major.
3.No,
4.just a string, we can avoid collisions using a Reverse domain name.

fix: add version and request type to protocol identifier

d472f38

awoie requested review from jogu, tlodderstedt, timcappalli, c2bo, paulbastian, Sakurann, leecam and hlozi January 10, 2025 13:51

fix: added normativity

8cb8eb7

c2bo reviewed Jan 10, 2025

View reviewed changes

hlozi requested changes Jan 10, 2025

View reviewed changes

openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved

hlozi self-requested a review January 10, 2025 16:11

hlozi reviewed Jan 10, 2025

View reviewed changes

fix: adjusted protocol identifier prefix to Tim's suggestion; aligned…

4fba63f

… with OIDF

Sakurann approved these changes Jan 20, 2025

View reviewed changes

Sakurann added this to the Final 1.0 milestone Jan 20, 2025

c2bo approved these changes Jan 21, 2025

View reviewed changes

timcappalli previously approved these changes Jan 21, 2025

View reviewed changes

bc-pi reviewed Jan 21, 2025

View reviewed changes

Sakurann reviewed Jan 21, 2025

View reviewed changes

Sakurann requested changes Jan 21, 2025

View reviewed changes

bc-pi previously requested changes Jan 21, 2025

View reviewed changes

timcappalli mentioned this pull request Jan 22, 2025

Adding a version to the protocol identifier, and making it a URN #363

Closed

peppelinux reviewed Jan 22, 2025

View reviewed changes

openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved

c2bo self-requested a review January 23, 2025 09:39

awoie and others added 2 commits January 23, 2025 12:47

Applied Giuseppe's editorial comment

a15ee43

Co-authored-by: Giuseppe De Marco <[email protected]>

fix: adjusted PR to have major versions only

ca8030c

awoie requested review from hlozi, bc-pi, Sakurann, dyiop, timcappalli, David-Chadwick and peppelinux January 30, 2025 13:39

David-Chadwick approved these changes Jan 30, 2025

View reviewed changes

jogu reviewed Jan 30, 2025

View reviewed changes

openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved

Applied Joseph's suggestion

1e4c78e

Co-authored-by: Joseph Heenan <[email protected]>

Sakurann reviewed Jan 30, 2025

View reviewed changes

openid-4-verifiable-presentations-1_0.md Outdated Show resolved Hide resolved

jogu approved these changes Jan 30, 2025

View reviewed changes

Applied Kristina's suggestion

8be38ee

Co-authored-by: Kristina <[email protected]>

awoie requested a review from Sakurann January 30, 2025 16:50

bc-pi approved these changes Jan 30, 2025

View reviewed changes

timcappalli approved these changes Jan 31, 2025

View reviewed changes

Sakurann approved these changes Jan 31, 2025

View reviewed changes

hlozi approved these changes Feb 6, 2025

View reviewed changes

Sakurann merged commit 9b05dea into main Feb 7, 2025
2 checks passed

Add version and request type to protocol identifier for DC API #381

Add version and request type to protocol identifier for DC API #381

Conversation

awoie commented Jan 10, 2025 • edited Loading

awoie commented Jan 10, 2025

c2bo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timcappalli Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timcappalli Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlozi left a comment

Choose a reason for hiding this comment

hlozi left a comment

Choose a reason for hiding this comment

awoie commented Jan 20, 2025

Sakurann commented Jan 20, 2025

paulbastian commented Jan 21, 2025

timcappalli commented Jan 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bc-pi Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

timcappalli Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dyiop Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sakurann left a comment

Choose a reason for hiding this comment

timcappalli commented Jan 21, 2025

bc-pi left a comment

Choose a reason for hiding this comment

awoie commented Jan 30, 2025 • edited Loading

bc-pi left a comment • edited Loading

Choose a reason for hiding this comment

c2bo commented Jan 30, 2025

bc-pi left a comment

Choose a reason for hiding this comment

dyiop commented Jan 30, 2025 • edited Loading

awoie commented Jan 31, 2025

Sakurann left a comment • edited Loading

Choose a reason for hiding this comment

hlozi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awoie commented Jan 10, 2025 •

edited

Loading

timcappalli Jan 14, 2025 •

edited

Loading

timcappalli Jan 15, 2025 •

edited

Loading

bc-pi Jan 21, 2025 •

edited

Loading

timcappalli Jan 21, 2025 •

edited

Loading

dyiop Jan 22, 2025 •

edited

Loading

awoie commented Jan 30, 2025 •

edited

Loading

bc-pi left a comment •

edited

Loading

dyiop commented Jan 30, 2025 •

edited

Loading

Sakurann left a comment •

edited

Loading