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

Clarify security check for URL-based payment method identifier #65

Open
fred-wang opened this issue Apr 19, 2021 · 2 comments
Open

Clarify security check for URL-based payment method identifier #65

fred-wang opened this issue Apr 19, 2021 · 2 comments

Comments

@fred-wang
Copy link

fred-wang commented Apr 19, 2021

In https://w3c.github.io/payment-method-id/#validation the following security checks are performed:

  • If url's scheme is not "https", return false.
  • If url's username or password is not the empty string, return false.

Chromium's implementation does the following ( https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/modules/payments/payments_validators.cc;l=160;drc=82d9604867706f5b9833f511acd47dffc58e6d91 ):

(And equivalent form would be url's scheme is https://fetch.spec.whatwg.org/#http-scheme and url is https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy)

One argument for allowing APIs on non-HTTPS server is so that web developers to experiment their implementation before publication e.g. using an easy-to-test localhost server. I'm not sure whether that applies to web payment, will open a bug on Chromium's side (--edit: done at https://bugs.chromium.org/p/chromium/issues/detail?id=1200225 )

@rsolomakhin
Copy link

The "potentially trustworthy" http is being allowed for local testing on http://127.0.0.1 and allow-listed (on command-line) origins. We can update the spec to note that web browsers can provide affordances for local testing such as this.

@fred-wang
Copy link
Author

I believe referring to https://fetch.spec.whatwg.org/#http-scheme + https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy would be enough. Note that there is already some explanation about why browsers may consider non-https URLs trustworthy on https://w3c.github.io/webappsec-secure-contexts/#implementation-considerations (albeit not for local servers).

pull bot pushed a commit to Mu-L/chromium that referenced this issue Apr 19, 2021
PaymentsValidators::IsValidMethodFormat currently does the following:

* If url's scheme is "https", return true.
* If url's scheme is not "http", return false.
* Otherwise return whether url's origin is potentially trustworthy [1]

This CL replaces the above checks with the equivalent form:

* Return whether url is HTTP(S) [2] and potentially trustworthy [3]

Note that the specification currently says we should only allow https
but that would be a behavior change. A ticket has been opened [4].

Additionally, this helps with the goal of bug 1153336 by removing one
call to SecurityOrigin::IsPotentiallyTrustworthy.

Bug: 1200225, 1153336

[1] https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
[2] https://fetch.spec.whatwg.org/#http-scheme
[3] https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy
[4] w3c/payment-method-id#65

Change-Id: I0d57e2d4c14e07931be91e32bc798aab2e690106
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2834133
Reviewed-by: Nick Burris <[email protected]>
Commit-Queue: Frédéric Wang <[email protected]>
Cr-Commit-Position: refs/heads/master@{#873836}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
PaymentsValidators::IsValidMethodFormat currently does the following:

* If url's scheme is "https", return true.
* If url's scheme is not "http", return false.
* Otherwise return whether url's origin is potentially trustworthy [1]

This CL replaces the above checks with the equivalent form:

* Return whether url is HTTP(S) [2] and potentially trustworthy [3]

Note that the specification currently says we should only allow https
but that would be a behavior change. A ticket has been opened [4].

Additionally, this helps with the goal of bug 1153336 by removing one
call to SecurityOrigin::IsPotentiallyTrustworthy.

Bug: 1200225, 1153336

[1] https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
[2] https://fetch.spec.whatwg.org/#http-scheme
[3] https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy
[4] w3c/payment-method-id#65

Change-Id: I0d57e2d4c14e07931be91e32bc798aab2e690106
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2834133
Reviewed-by: Nick Burris <[email protected]>
Commit-Queue: Frédéric Wang <[email protected]>
Cr-Commit-Position: refs/heads/master@{#873836}
GitOrigin-RevId: a72c5d8c637c28b9040e191e8af2ccaa19edebd3
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

No branches or pull requests

2 participants