Skip to content
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

Fix: Replace redirect_uri with response_uri for direct_post Response Mode #73

Merged
merged 4 commits into from
Mar 23, 2024

Conversation

cryptphil
Copy link
Contributor

Closes #71.

📑 Description

The HAIP currently defines that the response mode has to be direct_post with redirect_uri. However, using redirect_uri isn't allowed in the OpenID4VP spec for direct_post. Therefore, this PR proposes to change the Authorization Request parameter to response_uri.

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in response mode direct_post, after the wallet sends HTTP POST request to the verifier, there are two possibilities:

  1. the flow ends there
  2. the verifier returns redirect_uri and the wallet redirects the user to verifier front end
    this sentence intends to mean option 2.

@cryptphil cryptphil requested a review from Sakurann November 29, 2023 11:30
Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made some more suggestions since the original text i suggested was pretty awkward.. approving assuming something like my current suggestions would be accepted

draft-oid4vc-haip-sd-jwt-vc.md Outdated Show resolved Hide resolved
@peppelinux
Copy link
Member

See also openid/OpenID4VP#72

@cryptphil cryptphil requested a review from peppelinux December 18, 2023 08:00
cryptphil and others added 4 commits January 29, 2024 09:07
Signed-off-by: Philipp-Florens Lehwalder <philipp.lehwalder@lissi.id>
Co-authored-by: Kristina <52878547+sakurann@users.noreply.github.com>
Signed-off-by: Philipp-Florens Lehwalder <philipp.lehwalder@lissi.id>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Signed-off-by: Philipp-Florens Lehwalder <philipp.lehwalder@lissi.id>
@cryptphil cryptphil force-pushed the 71-response-uri-fix branch from 4b85b73 to 1ab7a48 Compare January 29, 2024 08:09
@@ -204,7 +204,7 @@ This is an example of a Wallet Instance Attestation:
* MUST support protocol extensions for SD-JWT VC credential format profile as defined in this specification (#vc_sd_jwt_profile).
* 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.
* Response type MUST be `vp_token`.
* Response mode MUST be `direct_post` with `redirect_uri` as defined in Section 6.2 of [@!OIDF.OID4VP].
* Response mode MUST be `direct_post`. The Verifier MUST return `redirect_uri` in response to the HTTP POST request from the Wallet, where the Wallet redirects the User to, as defined in Section 6.2 of [@!OIDF.OID4VP]. Implementation considerations for the response mode `direct_post` are given in Section 11.5 of [@!OIDF.OID4VP].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure we want the redirect_uri in cross device scenarios, too? Otherwise, this text is to generic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But doesn't the session fixation attack described in 12.2 apply to cross-device flows with direct_post when the redirect_uri is not used? At least that's how I understood this attack.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's true - question is whether this means HAIP must be narrowed to same device or whether there are situations where this could be otherwise detected (note: this is a problem with all oob protocols)
@danielfett what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this probably needs to be clarified in 4VP spec itself. there is an issue openid/OpenID4VP#25
but if we can go ahead and limit it in HAIP, I would not againt it, but probably out of scope for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenID4VP: Replace redirect_uri with response_uri for Response Mode direct_post
6 participants