New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 611314 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: Feature



Sign in to add a comment

Security: Content Security Policy - wildcard source expression matching should be stricter

Reported by shek...@gmail.com, May 12 2016

Issue description

CSP specification states that only http:, https:, ws: and wss: schemes should be captured by a '*" source. Other schemes should be explicitly present in the source list to match.

Current implementation in Chromium follows previous iteration of the specification, where any shceme should match wildcard, except blob:, data:, filesystem: (and about:, which is missing in Chromium implementation).

 
Cc: jww@chromium.org mkwst@chromium.org

Comment 3 by shek...@gmail.com, May 13 2016

Can you please change type to feature request. Did not have the option when I was reporting the issue. 
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam Type-Feature
Sure, switching some labels around then.
Status: Untriaged (was: Unconfirmed)
Marking the above issue as Untriaged as this is a feature request.

Thank you!
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6104167b0bf16a3520a898dbe67227637d1c214e

commit 6104167b0bf16a3520a898dbe67227637d1c214e
Author: shekyan <shekyan@gmail.com>
Date: Fri Jul 29 16:30:38 2016

Change wildcard source expression matching to conform latest spec

This changes wildcard source expression matching to require
any schemes other than http/https/ws/wss be explicitly present
in the source list to match, per
https://w3c.github.io/webappsec-csp/#match-url-to-source-expression

This also updates CSP injected in inline_login_ui.cc to explicitly allow
 `chrome:` as in `connect-src * chrome:` to fix failing tests.

Previous CL at https://codereview.chromium.org/1973933002/ is stale,
so creating a new one to nail it this time.
BUG= 611314 
R=mkwst@chromium.org

Review-Url: https://codereview.chromium.org/2160983002
Cr-Commit-Position: refs/heads/master@{#408654}

[modify] https://crrev.com/6104167b0bf16a3520a898dbe67227637d1c214e/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
[modify] https://crrev.com/6104167b0bf16a3520a898dbe67227637d1c214e/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/6104167b0bf16a3520a898dbe67227637d1c214e/chrome/browser/ui/webui/signin/inline_login_ui.cc
[modify] https://crrev.com/6104167b0bf16a3520a898dbe67227637d1c214e/third_party/WebKit/Source/core/frame/csp/CSPSourceList.cpp
[modify] https://crrev.com/6104167b0bf16a3520a898dbe67227637d1c214e/third_party/WebKit/Source/core/frame/csp/CSPSourceListTest.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14f3d8cdac5f46ec73fd38e5d9d731e57966427f

commit 14f3d8cdac5f46ec73fd38e5d9d731e57966427f
Author: mkwst <mkwst@chromium.org>
Date: Wed Aug 03 08:55:31 2016

Revert of Change wildcard source expression matching to conform latest spec (patchset #5 id:80001 of https://codereview.chromium.org/2160983002/ )

Reason for revert:
This breaks PDFium's handling of PDFs hosted on `file:`. Unfortunately, we didn't have tests covering this functionality, but we still need to fix it. :)

Original issue's description:
> Change wildcard source expression matching to conform latest spec
>
> This changes wildcard source expression matching to require
> any schemes other than http/https/ws/wss be explicitly present
> in the source list to match, per
> https://w3c.github.io/webappsec-csp/#match-url-to-source-expression
>
> This also updates CSP injected in chrome/browser/ui/webui/ to explicitly allow
>  `chrome:` as in `connect-src chrome:` to fix failing tests.
>
> Previous CL at https://codereview.chromium.org/1973933002/ is stale,
> so creating a new one to nail it this time.
> BUG= 611314 
> R=mkwst@chromium.org
>
> Committed: https://crrev.com/6104167b0bf16a3520a898dbe67227637d1c214e
> Cr-Commit-Position: refs/heads/master@{#408654}

TBR=jochen@chromium.org,mfoltz@chromium.org,shekyan@gmail.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 611314 

Review-Url: https://codereview.chromium.org/2207643003
Cr-Commit-Position: refs/heads/master@{#409480}

[modify] https://crrev.com/14f3d8cdac5f46ec73fd38e5d9d731e57966427f/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
[modify] https://crrev.com/14f3d8cdac5f46ec73fd38e5d9d731e57966427f/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/14f3d8cdac5f46ec73fd38e5d9d731e57966427f/chrome/browser/ui/webui/signin/inline_login_ui.cc
[modify] https://crrev.com/14f3d8cdac5f46ec73fd38e5d9d731e57966427f/third_party/WebKit/Source/core/frame/csp/CSPSourceList.cpp
[modify] https://crrev.com/14f3d8cdac5f46ec73fd38e5d9d731e57966427f/third_party/WebKit/Source/core/frame/csp/CSPSourceListTest.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39d12f5665910578d2e18251a0225a7aebe6964b

commit 39d12f5665910578d2e18251a0225a7aebe6964b
Author: shekyan <shekyan@gmail.com>
Date: Tue Aug 09 03:38:48 2016

Change wildcard source expression matching to conform latest spec

This patch changes wildcard source expression matching to require
schemes other than http/https/ws/wss be explicitly present
in the source list to match, per
https://w3c.github.io/webappsec-csp/#match-url-to-source-expression

This also updates CSP injected in inline_login_ui.cc to explicitly allow
`chrome:` as in `connect-src * chrome:` to fix failing tests.

This also changes Content Security Policy in manifests of Feedback and
PDF Viewer extensions to allow objects from `file:`.

BUG= 611314 
R=mkwst@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2209113002
Cr-Commit-Position: refs/heads/master@{#410566}

[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/chrome/browser/resources/feedback/manifest.json
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/chrome/browser/resources/pdf/manifest.json
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/chrome/browser/ui/webui/signin/inline_login_ui.cc
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/third_party/WebKit/Source/core/frame/csp/CSPSourceList.cpp
[modify] https://crrev.com/39d12f5665910578d2e18251a0225a7aebe6964b/third_party/WebKit/Source/core/frame/csp/CSPSourceListTest.cpp

Comment 9 by jleedev@gmail.com, Aug 10 2016

This breaks the Feedback extension again, because it needs "style-src: chrome://resources".
What's the status here?

Comment 11 by shek...@gmail.com, Oct 13 2016

This was fixed in https://codereview.chromium.org/2160983002/. Then reverted as was breaking PDFium, that didn't have test coverage for the broken functionality.
Re-landed in https://codereview.chromium.org/2209113002/ with additional changes to PDFium and feedback extension. Another corner-case fix for feedback extension was landend in https://codereview.chromium.org/2236233002/
Status: Fixed (was: Untriaged)
Marking "Fixed" per #11.

Note this regressed FTP support (see https://bugs.chromium.org/p/chromium/issues/detail?id=656956) which was corrected elsewhere.

Sign in to add a comment