Source '*' should not match URLs with schemes blob, data, or filesystem as per section Matching Source Expressions of Content Security Policy 2.0 spec. (29 August 2015): [[ 4.2.2. Matching Source Expressions A URL url is said to match a source expression for a protected resource if the following algorithm returns does match: 1. Let url be the result of processing the URL through the URL parser. 2. If the source expression consists of a single U+002A ASTERISK character (*), and url’s scheme is not one of blob, data, filesystem, then return does match. ... ]] <https://w3c.github.io/webappsec-csp/2/#match-source-expression> This is further stressed in section Security Considerations for GUID URL schemes: [[ 4.2.2.1. Security Considerations for GUID URL schemes This section is not normative. As defined above, special URL schemes that refer to specific pieces of unique content, such as "data:", "blob:" and "filesystem:" are excluded from matching a policy of * and must be explicitly listed. ]] <https://w3c.github.io/webappsec-csp/2/#source-list-guid-matching>
<rdar://problem/24613336>
Similar behavior was implemented in Gecko in <https://bugzilla.mozilla.org/show_bug.cgi?id=1086999>. This change broke some popular web sites, including CNN and Fastmail, that use CSP "default-src *" or "img-src *" and embed data URL images. See <https://lists.w3.org/Archives/Public/public-webappsec/2015Apr/0242.html> for more details.
Created attachment 273068 [details] Patch and Layout Tests For some reason the included test http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html fails with standard error ouput: com.apple.WebKit.WebContent.Development[9414:325774] NSURLSession/NSURLConnection HTTP load failed (kCFStreamErrorDomainSSL, -9813). I marked this test as Failure in LayoutTests/TestExpectations for now. We use the same code to determine if a HTTP or HTTPS URL can match * with respect to directive media-src as we do for any other directive. So, the other HTTPS tests included in this patch should provide sufficient coverage. When I have a moment, I will look to further investigate the failure of http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html and/or file a bug for this failure.
Comment on attachment 273068 [details] Patch and Layout Tests View in context: https://bugs.webkit.org/attachment.cgi?id=273068&action=review I think this patch looks good, but I'm confused why two of the new tests are expected to fail. Isn't that what this patch is supposed to be enabling support for? > Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:102 > + // FIXME: We should not hardcode the directive names. We should make use of the constants in ContentSecurityPolicyDirectiveList.cpp. Do you intend to do this in a follow-up patch? If so, please provide a Bug ID. > Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:107 > + isAllowed |= url.protocolIsData() || url.protocolIs("blob"); weird we don't have a ProtocolIsBlob predicate! > LayoutTests/TestExpectations:864 > +http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html [ Failure ] Can you provide a Bug for this, please? > LayoutTests/platform/wk2/TestExpectations:628 > +fast/dom/HTMLImageElement/image-with-blob-url-blocked-by-csp-img-src-star.html Why is this a failure under WK2? > LayoutTests/platform/wk2/TestExpectations:681 > +media/video-with-blob-url-allowed-by-csp-media-src-star.html Ditto
I have delayed approving the patch until I understand why two of the new tests fail under WK2.
(In reply to comment #4) > > Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:102 > > + // FIXME: We should not hardcode the directive names. We should make use of the constants in ContentSecurityPolicyDirectiveList.cpp. > > Do you intend to do this in a follow-up patch? If so, please provide a Bug > ID. > Filed bug #155133 to address this. > > Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp:107 > > + isAllowed |= url.protocolIsData() || url.protocolIs("blob"); > > weird we don't have a ProtocolIsBlob predicate! > Filed bug #155127 to add convenience function URL::protocolIsBlob(). Will update patch to make use of this function. > > LayoutTests/TestExpectations:864 > > +http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html [ Failure ] > > Can you provide a Bug for this, please? > Filed bug #155132. Will update this TestExpectation line to reference bug #155132 as the reason for the test failure. > > LayoutTests/platform/wk2/TestExpectations:628 > > +fast/dom/HTMLImageElement/image-with-blob-url-blocked-by-csp-img-src-star.html > > Why is this a failure under WK2? Notice that this test is in the list of tests that fail because they make use of eventSender.beginDragWithFiles(), which is not implemented for WebKit2. We need to fix bug #64285. > > > LayoutTests/platform/wk2/TestExpectations:681 > > +media/video-with-blob-url-allowed-by-csp-media-src-star.html > > Ditto Similarly, this test makes use of eventSender.beginDragWithFiles(), which is not implemented for WebKit2.
Created attachment 273212 [details] Patch and Layout Tests Updated patch to make use of URL::protocolIsBlob() (landed in <http://trac.webkit.org/changeset/197706>). Also updated comment in ContentSecurityPolicySourceList::isProtocolAllowedByStar() to reference bug #155133, updated LayoutTest/TestExpectations file entry for http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html to reference bug #155132 and added a remark to LayoutTests/ChangeLog about the reason for skipping tests fast/dom/HTMLImageElement/image-with-blob-url-blocked-by-csp-img-src-star.html and media/video-with-blob-url-allowed-by-csp-media-src-star.html due to bug #64285.
Comment on attachment 273212 [details] Patch and Layout Tests Attachment 273212 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/938240 New failing tests: http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star.html
Created attachment 273222 [details] Archive of layout-test-results from ews106 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 273212 [details] Patch and Layout Tests Attachment 273212 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/938279 New failing tests: http/tests/security/contentSecurityPolicy/javascript-url-blocked-by-default-src-star.html
Created attachment 273223 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 273229 [details] Patch and Layout Tests Update expected result for added test javascript-url-blocked-by-default-src-star.html following <http://trac.webkit.org/changeset/197697> (bug #153153). Following this changeset we apply the object-src directive instead of the frame-src directive to an HTML embed/object element that loads non-plugin content (as expected). And a JavaScript URL is considered non-plugin content.
Comment on attachment 273229 [details] Patch and Layout Tests Thank you for addressing those comments. r=me!
Comment on attachment 273229 [details] Patch and Layout Tests Clearing flags on attachment: 273229 Committed r197724: <http://trac.webkit.org/changeset/197724>
All reviewed patches have been landed. Closing bug.