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.

Sign up for GitHub

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

Jump to bottom

about:blank" or "about:srcdoc" with query string or fragment should be potentially trustworthy #81

Open
fred-wang opened this issue Nov 26, 2020 · 6 comments
Open

about:blank" or "about:srcdoc" with query string or fragment should be potentially trustworthy #81

fred-wang opened this issue Nov 26, 2020 · 6 comments

Comments

@fred-wang
Copy link
Contributor

From https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy

If url is "about:blank" or "about:srcdoc" return "Potentially Trustworthy".

I think the spec is not really explicit here (compare with other places where we talk about host component or scheme), but I understand the intention is to accept query string or fragment too.

So maybe it should be

if the url is made of an "about" scheme, a path matching "blank" or "srcdoc" and optional query/fragment then return "Potentially Trustworthy".

(Note: Chromium currently just checks if the scheme is "about" but ideally it should use these

https://source.chromium.org/chromium/chromium/src/+/master:url/gurl.h;l=216;drc=5607fbe5f50d8539be9f26e36a5c2517fc18fad7

which accepts query string or fragment.)
@fred-wang
Copy link
Contributor Author

@annevk @mikewest Any opinion on this ?

@annevk
Copy link
Member

annevk commented Nov 30, 2020

This sounds reasonable. HTML does some checks for about:blank too and I've wondered about this same thing, but never gotten around to writing tests and such.

(Fetch does do the thing you suggest at https://fetch.spec.whatwg.org/#concept-scheme-fetch and I suspect we want to adopt the same language.)

pull bot pushed a commit to Mu-L/chromium that referenced this issue Dec 3, 2020
@fred-wang
Prepare code to improve handling of potentially trustworthy url/origin
063077d 
This CL adds comments & TODOs, reorganizes a bit the code and adds tests
in order to make comparison against the specification easier [1] [2] [3]
and to prepare follow-up patches that will require approvals from API
owners. There is no behavior change.

[1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-origin
[2] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
[3] w3c/webappsec-secure-contexts#81

Bug: 1119740
Change-Id: Id1b379964a7724d7d53a279fbb5a187cb6a5453f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560953
Reviewed-by: Matthias Körber <[email protected]>
Reviewed-by: Łukasz Anforowicz <[email protected]>
Reviewed-by: Matt Falkenhagen <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Frédéric Wang <[email protected]>
Cr-Commit-Position: refs/heads/master@{#833034}
@annevk
Copy link
Member

annevk commented Jan 12, 2021

Did you write tests for this by any chance @fred-wang? I can take care of updating the specification.

@fred-wang
Copy link
Contributor Author

@annevk Only internal c++ tests for chromium.

mariospr added a commit to brave/brave-core that referenced this issue Jan 20, 2021
@mariospr
Adapt test expectations to only consider about:{blank,srcdoc} secure
c9dc13a 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Jan 21, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
67bdfe0 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mariospr added a commit to brave/brave-core that referenced this issue Jan 25, 2021
@mariospr
Adapt test expectations to only consider about:{blank,srcdoc} secure
5353c18 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Jan 26, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
eebf4c9 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Jan 27, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
6437055 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Jan 27, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
4bdcba6 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Feb 3, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
645a96a 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Feb 4, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
f41a20b 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
mkarolin pushed a commit to brave/brave-core that referenced this issue Feb 5, 2021
@mariospr @mkarolin
Adapt test expectations to only consider about:{blank,srcdoc} secure
c9db6bd 
Any other about: URL is now considered unsecure according to the spec
linked from the upstream commit's description (see [1])

Chromium change:

https://source.chromium.org/chromium/chromium/src/+/58fcd77538b8bc6989b2e3290b789f864ddf4dd9

commit 58fcd77538b8bc6989b2e3290b789f864ddf4dd9
Author: Frédéric Wang <[email protected]>
Date:   Thu Dec 3 00:47:31 2020 +0000

    Limit about: URLs that are treated as potentially trustworthy

    Per [1], only about:blank and about:srcdoc URLs should be treated as
    potentially trustworthy, but Chromium currently accepts all about: URLs.
    This CL aligns with the current spec, with the additional assumption
    that query and fragment components are accepted too [2]. This change is
    not web-visible.

    [1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
    [2] w3c/webappsec-secure-contexts#81

    Bug: 1153335, 1153336
@fred-wang
Copy link
Contributor Author

Did you write tests for this by any chance @fred-wang? I can take care of updating the specification.

@annevk IIUC, one reviewer mentioned this is probably not web-observable, so not sure we need to or can write WPT tests...

@annevk
Copy link
Member

annevk commented May 10, 2021

It should be possible to navigate a browsing context to about:blank?something, no?

I do think that as I mentioned elsewhere this should probably not end up as a secure context because of that, but rather because of who did the navigating. So ultimately this might indeed not be testable much.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@fred-wang @copybara-github
Prepare code to improve handling of potentially trustworthy url/origin
7454785 
This CL adds comments & TODOs, reorganizes a bit the code and adds tests
in order to make comparison against the specification easier [1] [2] [3]
and to prepare follow-up patches that will require approvals from API
owners. There is no behavior change.

[1] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-origin
[2] https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url
[3] w3c/webappsec-secure-contexts#81

Bug: 1119740
Change-Id: Id1b379964a7724d7d53a279fbb5a187cb6a5453f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560953
Reviewed-by: Matthias Körber <[email protected]>
Reviewed-by: Łukasz Anforowicz <[email protected]>
Reviewed-by: Matt Falkenhagen <[email protected]>
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Frédéric Wang <[email protected]>
Cr-Commit-Position: refs/heads/master@{#833034}
GitOrigin-RevId: 063077da1caa6e4407e3d95a74164dc5b7543e43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fred-wang @annevk