Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1114)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2699663002: CSP: 'self' should work inside sandboxes. (Closed)

Created:
3 years, 10 months ago by Mike West
Modified:
3 years, 10 months ago
CC:
andypaicu, blink-reviews, chromium-reviews, mikispag
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

CSP: 'self' should work inside sandboxes. We ought to be looking at the URL of a sandboxed resource when resolving the CSP source expression 'self'. Currently, we're looking at the origin of the resource, which is generally correct, but fails if the resource has been pushed into an opaque origin. This patch uses the fallback base URL of a document rather than its origin to do the comparison. BUG=692475 R=jochen@chromium.org CC=andypaicu@chromium.org Review-Url: https://codereview.chromium.org/2699663002 Cr-Commit-Position: refs/heads/master@{#451626} Committed: https://chromium.googlesource.com/chromium/src/+/9d3329c4a3d3dd0ab85869b7de4d62a8e2797520

Patch Set 1 #

Total comments: 2

Patch Set 2 : Test + Formatting #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -1 line) Patch
A third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html View 1 1 chunk +49 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp View 2 chunks +8 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/frame/csp/SourceListDirectiveTest.cpp View 1 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (8 generated)
Mike West
WDYT, Jochen? CC andypaicu@, mikispag@, FYI. https://codereview.chromium.org/2699663002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html File third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html (right): https://codereview.chromium.org/2699663002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html#newcode1 third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html:1: <!DOCTYPE html> I'd ...
3 years, 10 months ago (2017-02-15 13:38:06 UTC) #3
jochen (gone - plz use gerrit)
lgtm https://codereview.chromium.org/2699663002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html File third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html (right): https://codereview.chromium.org/2699663002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html#newcode11 third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html:11: async_test(t => { why are you using 4sp ...
3 years, 10 months ago (2017-02-20 09:55:19 UTC) #6
Mike West
On 2017/02/20 at 09:55:19, jochen wrote: > lgtm > > https://codereview.chromium.org/2699663002/diff/1/third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html > File third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/1.1/base-uri-self.html (right): ...
3 years, 10 months ago (2017-02-20 12:45:42 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2699663002/20001
3 years, 10 months ago (2017-02-20 12:58:27 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/9d3329c4a3d3dd0ab85869b7de4d62a8e2797520
3 years, 10 months ago (2017-02-20 15:04:49 UTC) #13
Mike West
3 years, 10 months ago (2017-02-25 13:18:05 UTC) #14
Message was sent while issue was closed. 
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2711363004/ by mkwst@chromium.org.

The reason for reverting is: I suspect that this breaks Synology's UI, which
submits a form through
an `about:blank` frame. Perhaps we're not persisting the fallback base
URL correctly?

BUG=695058.

Powered by Google App Engine
This is Rietveld 408576698