-
Notifications
You must be signed in to change notification settings - Fork 15
refactor OID4VC sections to include ISO mdocs #228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ts since PEX was removed from OID4VP
|
Why do we need this bullet in the "redirects" section? "The DCQL query and response as defined in Section 6 of [@!OIDF.OID4VP] MUST be used." |
That's already being removed in https://github.com/openid/oid4vc-haip/pull/223/files |
Co-authored-by: Joseph Heenan <joseph@authlete.com>
| - [@!OIDF.OID4VCI] – Annex A.2 | ||
| - [@!OIDF.OID4VP] – Annex B.2 | ||
|
|
||
| # Crypto Suites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to #193 ,this section is not explicitly referenced in the different sections and the question is whether it's applicable to all the different flows (issuance / presentment of sd-jwt, mdocs etc)
It's important because it's currently not clear whether it's possible to present / issue any mdoc that's compliant with ISO/IEC 18013-5. For example, are the following flows compliant to HAIP:
- Issuing an mdoc or sd-jwt to a wallet that only supports P-384 key
- Issuing an mdoc or sd-jwt a wallet that only supports brainpool-256 keys
- Issuing an mdoc or sd-jwt by an issuer that only wants to issue brainpool curves
- Having an RP certificate with a P-384 key
- Having a root RP certificate with a P-521 key
I'm not necessarily saying that all of these should be HAIP compliant, but it should be clear whether they are or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree but I think we should continue this discussion in #112 as further WG discussion is definitely needed before we reach a consensus.
| This specification enables interoperable implementations of the following flows: | ||
|
|
||
| * Issuance of IETF SD-JWT VC using OpenID4VCI | ||
| * Issuance of ISO mdocs using OpenID4VCI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My interpretation of the mdoc issuance profile as proposed in this PR is that it mandates support of custom scheme haip:// for credential issuance, this has a few issues:
- There is no normative language for what it means to support a custom scheme. Does it mean you have to be reachable, register etc. As an example, what does requirement mean in the context of a wearable.
- This means that all mdocs issued using vci are all required to support/register the same custom scheme. While the goal is understandable, it's not clear to me if this requirement is actually helpful for interoperability.
- Schemes have known security issues depending on how they are used, mandating support for them independent of whether protecting measures are in place is awkward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As currently only the authorization code flow is supported, I do not see any security issues with using custom schemes or cross-device flows, e.g. with a QR code, as the Credential Offer only contains a pointer to the AS and actual authorization therefore always takes place "same-device". HAIP furthermore says:
As a way to invoke the Wallet, at least a custom URL scheme haip:// MUST be supported. Implementations MAY support other ways to invoke the Wallets as agreed by trust frameworks/ecosystems/jurisdictions, not limited to using other custom URL schemes.
Therefore HAIP custom scheme is still a way to go, but not necessarily the only one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the haip:// scheme is not reliable and less secure when used for the redirect URI but we have an issue for this here #240. In my view, it should not be mandated.
| This specification enables interoperable implementations of the following flows: | ||
|
|
||
| * Issuance of IETF SD-JWT VC using OpenID4VCI | ||
| * Issuance of ISO mdocs using OpenID4VCI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My interpretation of the mdoc issuance profile as proposed in this PR is that it mandates support for supporting the same-device and cross-device flow for credential offers. However it's not well-defined what that means. VCI says the following for same-device and cross-device:
"The End-User may receive the Credential Offer from the Credential Issuer either on the same device as the device the Wallet resides on, or through any other means, such as another device or postal mail, so that the Credential Offer can be communicated to the Wallet."
So this would mean that as long as the credential offer can be received "through any other means" there is compliance to the cross-device flow requirement. I don't think that in itself provides any interoperability benefits.
For the same-device scenario, how does the same-device requirement apply to companion devices. Does this means that in order to be HAIP-compliant, the companion device must be able to handle all requirements in HAIP on its own, i.e. to effectively act as a stand-alone device.
One option would be to recommend support for the different flows, but if we mandate them it should be clear what it means to be compliant to these requirements and how that works for companion devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree that we shoud more clearly define and restrict what the mandatory options for cross-device are. Many are using QR-Codes containing a custom scheme for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "companion devices" question applies to the existing SD-JWT profile as well - I'd suggest we handle that as a new issue rather than trying to fix in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we would need at least two issues for this:
- companion devices
- clarify what it means to support cross-device and same-device flows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a ticket for the cross-device/same-device already: #241 so it's only the companion devices one we still need to create.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raised #248 to further discuss companion devices.
paulbastian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor editorial things
| This specification enables interoperable implementations of the following flows: | ||
|
|
||
| * Issuance of IETF SD-JWT VC using OpenID4VCI | ||
| * Issuance of ISO mdocs using OpenID4VCI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As currently only the authorization code flow is supported, I do not see any security issues with using custom schemes or cross-device flows, e.g. with a QR code, as the Credential Offer only contains a pointer to the AS and actual authorization therefore always takes place "same-device". HAIP furthermore says:
As a way to invoke the Wallet, at least a custom URL scheme haip:// MUST be supported. Implementations MAY support other ways to invoke the Wallets as agreed by trust frameworks/ecosystems/jurisdictions, not limited to using other custom URL schemes.
Therefore HAIP custom scheme is still a way to go, but not necessarily the only one.
| This specification enables interoperable implementations of the following flows: | ||
|
|
||
| * Issuance of IETF SD-JWT VC using OpenID4VCI | ||
| * Issuance of ISO mdocs using OpenID4VCI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree that we shoud more clearly define and restrict what the mandatory options for cross-device are. Many are using QR-Codes containing a custom scheme for example.
paulbastian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding requirements from #98 as merging would otherwise be complicated
| This specification enables interoperable implementations of the following flows: | ||
|
|
||
| * Issuance of IETF SD-JWT VC using OpenID4VCI | ||
| * Issuance of ISO mdocs using OpenID4VCI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "companion devices" question applies to the existing SD-JWT profile as well - I'd suggest we handle that as a new issue rather than trying to fix in this PR.
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Paul Bastian <paul.bastian@posteo.de>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
| * As a way to invoke the Wallet, at least a custom URL scheme `haip://` MUST be supported by the Wallet and the Verifier. Implementations MAY support other ways to invoke the Wallets as agreed by trust frameworks/ecosystems/jurisdictions, not limited to using other custom URL schemes. | ||
| * Signed Authorization Requests MUST be used by utilizing JWT-Secured Authorization Request (JAR) [@!RFC9101] with the `request_uri` parameter. | ||
| * Response encryption MUST be used by utilizing response mode `direct_post.jwt`, as defined in Section 8.3 of [@!OIDF.OID4VP]. Security Considerations in Section 14.3 of [@!OIDF.OID4VP] MUST be applied. | ||
| * Same-device flows MUST be used by enforcing `redirect_uri` in response to the HTTP POST request from the Wallet, as defined in Section 8.2 of [@!OIDF.OID4VP]. Implementation considerations in Section 13.3 and Security Considerations in Section 14.2 of [@!OIDF.OID4VP] MUST be applied. Verifiers MUST abort the validation if the `redirect_uri` was not called by the Wallet or could not be matched with the initial browser session. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed on this morning's WG call:
Does this prevent wallets from implementing cross-device for non-high assurance cases? That would seem like an undesirable outcome.
Also due to DC API not being widely available yet (e.g. on iOS, or for web wallets), HAIP should consider (at least in 1.0) allowing cross device flows using redirects as otherwise some use cases won't work.
(I added this comment on #98 too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the text on same-device flows.
|
|
||
| # Scope | ||
|
|
||
| This specification enables interoperable implementations of the following flows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combining the multiple bullets into less bullets and using an OR makes it look a bit cleaner, but makes it less clear from a normative perspective.
Since HAIP defines multiple profiles/flow, compliance to HAIP in itself doesn't mean anything without defining which flow is used. Therefore when referencing HAIP, it's important that you can say "implement flow x as defined in HAIP". But with the ways these flows are listed here that's difficult. So it's better to list all the flows individually. That means that we end up with a longer list, but it's good to be explicit.
For each flow we can then also explicitly say which exact sections must be implemented for that particular flow. While it's implicitly reasonably clear with the refactoring, actually listing the sections ensures that there is never any ambiguity whether a particular section applies or not.
An example of what this could look like is:
"Issuance of IETF SD-JWT VC using OpenID4VCI. This requires implementation of section x, y.
And we could then separately say "for all profiles, section z applies" if that's still necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have an open issue on this topic so could defer some of this to be handled in this issue: #193 (which is also targeted for 1.0).
Discussed with Martijn - he's okay with us moving this forward, we've opened issues tagged for 1.0 to address all his comments, so dismissing the 'request changes'.
fixes #206
partially fixes #225 but perhaps it fixes it entirely @c2bo , what do you think (since you created the issue)?
This PR refactors the OID4VP related sections to make it clear that ISO mdocs are included in the OID4VP profile defined in HAIP. Furthermore, it removes PEX references and redundant normative statements that require DCQL.
I kept the SD-JWT VC requirements for OID4VP section but tbh, after PEX got removed, there is not much to require there that is not mandated by OID4VP.
IMO, we don't even need to require the format identifier for SD-JWT VC since it is given by OID4VP but I kept it for discussion.
resolves #161
resolves #98