openid / OpenID4VP

56 stars 20 forks source link

DC API: Protocol identifier for signed vs unsigned requests #326

Open timcappalli opened 1 day ago

timcappalli commented 1 day ago

Problem Statement

The top level properties of the request object that are passed into the Digital Credentials API (via the data property), are different for signed and unsigned requests. For example, a signed request only has payload and signature while an unsigned request has a completely different set of properties.

For implementers, the only way to know whether the request is signed or unsigned is to do some implicit matching based on the properties which are present at the top level of the data object (e.g. look for payload or signature). This doesn't scale well, can be error prone, and thus makes implementation more challenging.

Proposal

The loose consensus from the 2024-11-13 Digital Credentials API call was to bring this issue forward for discussion in the DCP WG: https://github.com/WICG/digital-credentials/issues/185.

The proposal in the issue is to use different protocol strings when the top level structure is different. So for OID4VP, for signed vs unsigned requests, openid4vp-signed and openid4vp-unsigned respectively.

const credential = await navigator.credentials.get({
  digital: {
    providers: [{
      protocol: "openid4vp-signed",
      data: {
        "payload": "eyAiaXNzIjogImh0dHBzOi8...NzY4Mzc4MzYiIF0gfQ",
        "signatures": [
          {
            "protected": "eyJhbGciOiJFUzI1NiJ9",
            "header": {
              "client_id": "987647789",
              "client_id_scheme": "x509_san_dns"
            },
            "signature": "PFwem0Ajp2Sag...T2z784h8TQqgTR9tXcif0jw"
          }
        ]
      }
    }
    ]
  }
});

Alternative Proposal

An alternative would be to have an additional top level property for the DigitalCredentialsRequest object, such as requestType, which would be an enum of values appropriate to the protocol (and would be part of the registry).

const credential = await navigator.credentials.get({
  digital: {
    providers: [{
      protocol: "openid4vp",
      requestType: "signed",
      data: {
        "payload": "eyAiaXNzIjogImh0dHBzOi8...NzY4Mzc4MzYiIF0gfQ",
        "signatures": [
          {
            "protected": "eyJhbGciOiJFUzI1NiJ9",
            "header": {
              "client_id": "987647789",
              "client_id_scheme": "x509_san_dns"
            },
            "signature": "PFwem0Ajp2Sag...T2z784h8TQqgTR9tXcif0jw"
          }
        ]
      }
    }
    ]
  }
});
Sakurann commented 1 day ago

discussed on a WG call. seems to be a slight preference towards using different protocol identifiers: openid4vp-signed, openid4vp-unsigned, openid4vp-multisigned. on Android, wallets need to register protocol identifier, so wallets will have to register multiple protocol identifiers this is about the structure of the request, so, caveat is that, in the future, if we will have another reason other than signed/unsigned that causes deviation in structure, that might require other mechanisms to differentiate.

MasterKale commented 1 day ago

...caveat is that, in the future, if we will have another reason other than signed/unsigned that causes deviation in structure, that might require other mechanisms to differentiate.

What have discussions been like here in OID4VP on how to communicate a protocol definition's "version" when there are breaking changes in the future? Could identifiers like openid4vp-signed/openid4vp-unsigned/etc... incorporate a "-v1" suffix to achieve this if there's a desire to keep something like "version": 1 out of the presentation request?

leecam commented 1 day ago

Sorry I missed the call.

The wallets on Android don't actually register for protocol. They just get the whole digital: {} object as a JSON payload. So they won't need to register multiple protocols. The matcher figures out what the protocol is internally and parses the data. Its a bit easier for the matcher if there is there are top level identifiers like openid4vp-signed, openid4vp-unsigned, openid4vp-multisigned. Otherwise it needs to probe looking for certain fields and impliclty tries to figure out what type of openid4vp request it is. Defining the different top level types makes it more explicit (and future proof)

+1 on the versioning. I was going to open an issue on that. In our sample implementations we use openid4vp-1.0 as the protocol.

GarethCOliver commented 1 day ago

Correcting my mistake from a pre-alpha android version: wallets only register for the DC API on android, protocol is just a matcher-concern, so there are no functional difference to branching at the protocol level, or adding an 'requestType' enum to the data payload. As such, no objection from my end providing the extra clarity where-ever it is added.

The only motivation to add a requestType enum to the 'data' payload (rather than doing so at the 'protocol' level) would be to allow for common parsing logic for engagement mechanisms other than the DC API (as those would need to use another mechanism, or do implicit querying as protocol is absent).

versioning makes sense, though I personally would prefer a separate field, I hate implicit structure in string names, verse explicit separation (and you may want to be able to express 'I support versions < X' so would have to pull apart the name, which isn't hard but is inelegant).

timcappalli commented 1 day ago

Early on in the DC API work, I proposed that protocol be a urn instead of a string. Consensus at the time was to keep it a string.

I'd like to propose that we consider it again, as we address some of these new items.

Ex: urn:openid:protocol:oid4vp:signed:v1.0