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

Can data: URLs be part of a secure context? #69

Closed
annevk opened this issue Jan 13, 2020 · 10 comments
Closed

Can data: URLs be part of a secure context? #69

annevk opened this issue Jan 13, 2020 · 10 comments

Comments

@annevk
Copy link
Member

annevk commented Jan 13, 2020

I think the resolution of #26 is no, but this resolution creates a vary weird state. In effect, we're back to mixed content that isn't blocked.

If data: URLs would simply network error it'd be fine, but now they can get all the capabilities they want if they have the cooperation of the embedder.

This in turn makes other stateful features such as COOP and COEP hard to define.

cc @bzbarsky @mikewest

@annevk
Copy link
Member Author

annevk commented Jan 13, 2020

See whatwg/html#5198 and whatwg/html#4916 for additional context (and links to various tests).

@annevk
Copy link
Member Author

annevk commented Jan 13, 2020

The risks here are that the data: URL is something that is injected and navigated to (which is not possible with srcdoc) or the result of an "open redirector" with a similar injection hole.

From those perspectives it makes sense to me to treat them as insecure contexts, but I'm not sure what this means for COEP.

Is this the one case where your document has COOP and COEP but it does not have cross-origin isolation? Or are data: URLs disallowed in this new world?

(I think by-and-large this issue is now about how data: URLs create a weird mixed-content-like state and the specification should call that out and ideally offer advice for how to deal with that.)

@bzbarsky
Copy link

or the result of an "open redirector" with a similar injection hole.

This is the "HTTP 3xx to data:" case? That case should not inherit secure context state, just like it didn't use to inherit origin in Gecko when data: in general inherited origins.

As far as injection and navigation, how is that different in practice from someone injecting a navigation to an https:// URL they control, which would also be secure context?

@annevk
Copy link
Member Author

annevk commented Jan 13, 2020

It's not. I think it's only different in that someone might imbue their own data: URL with privileges and does not consider that an attacker might be able to get it replaced through XSS. I'm not sure that's a strong enough reason and I would kinda prefer either forbidding data: URLs entirely or making them always inherit state (other than origin) as the current state of affairs is really special and will require lots of thinking each time we add a new policy.

(And yeah, HTTPS redirect to a data: URL. I did not test what implementations do today.)

@annevk
Copy link
Member Author

annevk commented Jan 14, 2020

To state it somewhat more strongly, I now think we should change the specification so that data: URLs creating documents or workers can no longer create this unique problem and instead inherit this state as they do for other state.

@annevk
Copy link
Member Author

annevk commented Feb 12, 2020

web-platform-tests/wpt#21146 has a test for this (workers only atm).

@annevk
Copy link
Member Author

annevk commented Feb 13, 2020

More tests: web-platform-tests/wpt#21781. I couldn't find any existing data: URL tests for secure contexts.

@mikewest
Copy link
Member

I talked with @camillelamy about this earlier today, and I can get behind the notion of inheriting transport secureness from a parent context. I'm a little reluctant to give data: more capability, but I agree that it' a strange state of affairs for a document to be able to delegate capability to a data: frame via postMessage(), but not grant it directly. I also accept that it simplifies the model by removing some edge cases (we push CSP down to the data: URL, we should probably push everything that would eventually end up in the "policy container" we'll eventually define in whatwg/html#4926).

How would this affect top-level data: documents? Presumably we've made those hard enough to get to that it would be reasonable to continue denying them capability?

@mikewest
Copy link
Member

(Would you be willing to send a spec PR?)

@annevk
Copy link
Member Author

annevk commented Feb 13, 2020

A top-level data: URL can only come from a user-initiated navigation (well, that's the plan for standards and implemented in at least Chrome and Firefox, see whatwg/html#5279) and I agree that those should not create a secure context.

I can see about writing a PR.

annevk added a commit that referenced this issue Feb 13, 2020
As long as they have a creator, anyway. (Note that data: URLs cannot be opened in a top-level browsing context, such as a popup, except via a user-initiated navigation.)

Tests: web-platform-tests/wpt#21146 and web-platform-tests/wpt#21781.

Fixes #69. Nice!
annevk added a commit to web-platform-tests/wpt that referenced this issue Feb 13, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 17, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781

UltraBlame original commit: 9168eaebb7a09addc3a9c1700a2b2312bae790c5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781

UltraBlame original commit: 9168eaebb7a09addc3a9c1700a2b2312bae790c5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Feb 18, 2020
Automatic update from web-platform-tests
Secure contexts: add data: URLs

See w3c/webappsec-secure-contexts#69. Complements web-platform-tests/wpt#21146.
--

wpt-commits: cc7860faa93b6c404a5767b7635843d274eb7d0a
wpt-pr: 21781

UltraBlame original commit: 9168eaebb7a09addc3a9c1700a2b2312bae790c5
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2023
…exts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2023
…exts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 3, 2023
…exts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.


[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4377256
Commit-Queue: Jonathan Hao <phao@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1125248}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 3, 2023
…exts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4377256
Commit-Queue: Jonathan Hao <phao@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1125248}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 3, 2023
…exts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4377256
Commit-Queue: Jonathan Hao <phao@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1125248}
cookiecrook pushed a commit to cookiecrook/wpt that referenced this issue Apr 8, 2023
…exts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4377256
Commit-Queue: Jonathan Hao <phao@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1125248}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 15, 2023
…from data urls in secure contexts, a=testonly

Automatic update from web-platform-tests
Fix isSecureContext for workers created from data urls in secure contexts

Worker created from data urls in secure contexts should be considered
as in secure contexts as well.
Spec: https://html.spec.whatwg.org/multipage/webappapis.html#secure-contexts
Discussions: w3c/webappsec-secure-contexts#69

Alternatives considered:
- Make ExecutionContext::IsSecureContext virtual
and provide a specialized WorkerGlobalScope. The problem is that it
bypasses the UseCounter logging in [1]
- Call security_origin.SetOpaqueOriginIsPotentiallyTrustworthy(true) at
[2]. This makes IsPotentiallyTrustworthy() true for this specific
security_origin. However, other places call IsPotentiallyTrustworthy()
too so I am not sure whether this breaks other intended behaviors.

[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/execution_context/security_context.cc;l=124-126
[2] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/workers/worker_global_scope.cc;l=137;bpv=1;bpt=1

Bug: 1325494
Change-Id: Id5c7c3bc61b320426249bde0e346bd1f5f0b33d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4377256
Commit-Queue: Jonathan Hao <phao@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1125248}

--

wpt-commits: 6116305fb201fa1653471e463e44b2195934afa0
wpt-pr: 39303
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

3 participants