CSP: Content Security Policy should allow '*' to match the originating page's scheme
Created attachment 281389 [details] Patch
<rdar://problem/26819568>
Comment on attachment 281389 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281389&action=review > LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:8 > + <meta http-equiv="Content-Security-Policy" content="script-src * "> Nit: There is trailing whitespace after '*' > LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:11 > +<p>Test passes if the javascript src is loaded.</p> Nit: "javascript src" => "external JavaScript script" > LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:12 > +<script src="resources/append-body.js"></script> Can we make use of an existing script instead of adding a new script, append-body.js?
Comment on attachment 281389 [details] Patch Attachment 281389 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1509373 New failing tests: fast/dom/HTMLImageElement/image-with-file-url-blocked-by-csp-img-src-star.html fast/dom/HTMLLinkElement/link-with-file-url-blocked-by-csp-style-src-star.html media/video-with-file-url-blocked-by-csp-media-src-star.html
Created attachment 281395 [details] Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 281389 [details] Patch Attachment 281389 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1509377 New failing tests: fast/dom/HTMLImageElement/image-with-file-url-blocked-by-csp-img-src-star.html fast/dom/HTMLLinkElement/link-with-file-url-blocked-by-csp-style-src-star.html media/video-with-file-url-blocked-by-csp-media-src-star.html
Created attachment 281396 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 281389 [details] Patch Attachment 281389 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1509379 New failing tests: fast/dom/HTMLImageElement/image-with-file-url-blocked-by-csp-img-src-star.html fast/dom/HTMLLinkElement/link-with-file-url-blocked-by-csp-style-src-star.html
Created attachment 281397 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Comment on attachment 281389 [details] Patch Attachment 281389 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1509380 New failing tests: fast/dom/HTMLImageElement/image-with-file-url-blocked-by-csp-img-src-star.html fast/dom/HTMLLinkElement/link-with-file-url-blocked-by-csp-style-src-star.html media/video-with-file-url-blocked-by-csp-media-src-star.html
Created attachment 281398 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 281389 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281389&action=review Thanks Dan for reviewing my patch! >> LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:8 >> + <meta http-equiv="Content-Security-Policy" content="script-src * "> > > Nit: There is trailing whitespace after '*' Fixed. >> LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:11 >> +<p>Test passes if the javascript src is loaded.</p> > > Nit: "javascript src" => "external JavaScript script" Fixed. >> LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:12 >> +<script src="resources/append-body.js"></script> > > Can we make use of an existing script instead of adding a new script, append-body.js? I don't see any usable javascript script inside the resources folder. Hence, I think there is no choice but to have append-body.js.
Created attachment 281445 [details] Patch
Comment on attachment 281445 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281445&action=review > LayoutTests/ChangeLog:17 > + * fast/dom/HTMLImageElement/image-with-file-url-allowed-by-csp-img-src-star-expected.html: Renamed from LayoutTests/fast/dom/HTMLImageElement/image-with-file-url-blocked-by-csp-img-src-star-expected.html. > + * fast/dom/HTMLImageElement/image-with-file-url-allowed-by-csp-img-src-star.html: Renamed from LayoutTests/fast/dom/HTMLImageElement/image-with-file-url-blocked-by-csp-img-src-star.html. > + * fast/dom/HTMLLinkElement/link-with-file-url-allowed-by-csp-style-src-star-expected.html: Renamed from LayoutTests/fast/dom/HTMLLinkElement/link-with-file-url-blocked-by-csp-style-src-star-expected.html. > + * fast/dom/HTMLLinkElement/link-with-file-url-allowed-by-csp-style-src-star.html: Renamed from LayoutTests/fast/dom/HTMLLinkElement/link-with-file-url-blocked-by-csp-style-src-star.html. > + * media/video-with-file-url-allowed-by-csp-media-src-star-expected.html: Copied from LayoutTests/media/video-with-file-url-blocked-by-csp-media-src-star.html. > + * media/video-with-file-url-allowed-by-csp-media-src-star.html: Renamed from LayoutTests/media/video-with-file-url-blocked-by-csp-media-src-star.html. > + * media/video-with-file-url-blocked-by-csp-media-src-star-expected.html: Removed. > + * security/content-security-policy-matches-star-with-originator-scheme-expected.txt: Added. > + * security/content-security-policy-matches-star-with-originator-scheme.html: Added. I suggest that we create a new directory hierarchy LayoutTests/security/contentSecurityPolicy and move all these tests into LayoutTests/security/contentSecurityPolicy I choose LayoutTests/security/contentSecurityPolicy so as to more closely match the directory hierarchy we use for HTTP CSP tests in LayoutTests/http/tests/security/contentSecurityPolicy. > LayoutTests/fast/dom/HTMLImageElement/image-with-file-url-allowed-by-csp-img-src-star-expected.html:5 > +<img src="resources/green.png" width="128" height="128" alt="FAIL"> I suggest that we remove the HTML attribute alt to make this test more robust and less error prone. This will prevent the test from passing in the unlikely event that resources/green.png is removed or this file is modified such that the src attribute has a typo among other issues. > LayoutTests/fast/dom/HTMLImageElement/image-with-file-url-allowed-by-csp-img-src-star.html:1 > +<!DOCTYPE html> If we choose to move this file to LayoutTests/security/contentSecurityPolicy then I suggest that remove "csp" from the name of this file because its ambient directory name conveys that this is a Content Security Policy test and call this file image-with-file-url-allowed-by-img-src-star.html. > LayoutTests/fast/dom/HTMLLinkElement/link-with-file-url-allowed-by-csp-style-src-star.html:1 > +<!DOCTYPE html> If we choose to move this file to LayoutTests/security/contentSecurityPolicy then I suggest that remove "csp" from the name of this file because its ambient directory name conveys that this is a Content Security Policy test and call this file link-with-file-url-allowed-by-style-src-star.html. > LayoutTests/fast/dom/HTMLLinkElement/link-with-file-url-allowed-by-csp-style-src-star.html:15 > +<p>This tests that loading a stylesheet with a file URL is allowed when the page that is loaded from file URL has Content Security Policy "style-src *". This test PASSED if you see a red square below. Otherwise, it FAILED.</p> We tend to follow the recommendations of the CSS2.1 Test Case Authoring Guidelines, <http://www.w3.org/Style/CSS/Test/guidelines.html>, when writing layout tests and indicate success and failure by using the color green [1] and red [2], respectively. Therefore, we should indicate that the test PASSED if you see a green square and failure should be indicated by a red square. Towards this we can make use of the external stylesheet <http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/HTMLLinkElement/resources/green-background-color.css> and initially set the background color of <div id="test"> to red. [1] <http://www.w3.org/Style/CSS/Test/guidelines.html#indicating> [2] <http://www.w3.org/Style/CSS/Test/guidelines.html#indicating0> > LayoutTests/media/video-with-file-url-allowed-by-csp-media-src-star.html:1 > +<!DOCTYPE html> If we choose to move this file to LayoutTests/security/contentSecurityPolicy then I suggest that remove "csp" from the name of this file because its ambient directory name conveys that this is a Content Security Policy test and call this file video-with-file-url-allowed-by-media-src-star.html. > LayoutTests/media/video-with-file-url-allowed-by-csp-media-src-star.html:36 > +<p>This tests that loading a video with a file URL is allowed when the page that is loaded from file URL has Content Security Policy "media-src *". This test PASSED if you don't see a solid green square. Otherwise, it FAILED.</p> As aforementioned we should use the color red to signify failure instead of green. > LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:1 > +<!DOCTYPE html> I would name this file script-with-file-url-allowed-by-script-src-star.html and put this file in LayoutTests/security/contentSecurityPolicy. > LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:8 > + <script> > + if (window.testRunner) > + testRunner.dumpAsText(); > + </script> > + <meta http-equiv="Content-Security-Policy" content="script-src *"> Minor: The indentation for the children of the HTML head element differs from the indentation used for children of the HTML body element. I suggest that we do not indent the children of the HTML head element. Regardless, we should use a consistent indentation style. > LayoutTests/security/resources/append-body.js:1 > +document.body.innerHTML += '<p>Loaded. Test passes.</p>'; I would name this file alert-pass.js to match the name of file of the same purpose in LayoutTests/http/tests/security/contentSecurityPolicy/resources and I would change the implementation to: alert("PASS");
Created attachment 281483 [details] Patch
Comment on attachment 281445 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281445&action=review >> LayoutTests/ChangeLog:17 >> + * security/content-security-policy-matches-star-with-originator-scheme.html: Added. > > I suggest that we create a new directory hierarchy LayoutTests/security/contentSecurityPolicy and move all these tests into LayoutTests/security/contentSecurityPolicy I choose LayoutTests/security/contentSecurityPolicy so as to more closely match the directory hierarchy we use for HTTP CSP tests in LayoutTests/http/tests/security/contentSecurityPolicy. Fixed. >> LayoutTests/fast/dom/HTMLImageElement/image-with-file-url-allowed-by-csp-img-src-star-expected.html:5 >> +<img src="resources/green.png" width="128" height="128" alt="FAIL"> > > I suggest that we remove the HTML attribute alt to make this test more robust and less error prone. This will prevent the test from passing in the unlikely event that resources/green.png is removed or this file is modified such that the src attribute has a typo among other issues. Fixed. >> LayoutTests/fast/dom/HTMLImageElement/image-with-file-url-allowed-by-csp-img-src-star.html:1 >> +<!DOCTYPE html> > > If we choose to move this file to LayoutTests/security/contentSecurityPolicy then I suggest that remove "csp" from the name of this file because its ambient directory name conveys that this is a Content Security Policy test and call this file image-with-file-url-allowed-by-img-src-star.html. Fixed. >> LayoutTests/fast/dom/HTMLLinkElement/link-with-file-url-allowed-by-csp-style-src-star.html:1 >> +<!DOCTYPE html> > > If we choose to move this file to LayoutTests/security/contentSecurityPolicy then I suggest that remove "csp" from the name of this file because its ambient directory name conveys that this is a Content Security Policy test and call this file link-with-file-url-allowed-by-style-src-star.html. Fixed. >> LayoutTests/fast/dom/HTMLLinkElement/link-with-file-url-allowed-by-csp-style-src-star.html:15 >> +<p>This tests that loading a stylesheet with a file URL is allowed when the page that is loaded from file URL has Content Security Policy "style-src *". This test PASSED if you see a red square below. Otherwise, it FAILED.</p> > > We tend to follow the recommendations of the CSS2.1 Test Case Authoring Guidelines, <http://www.w3.org/Style/CSS/Test/guidelines.html>, when writing layout tests and indicate success and failure by using the color green [1] and red [2], respectively. Therefore, we should indicate that the test PASSED if you see a green square and failure should be indicated by a red square. Towards this we can make use of the external stylesheet <http://trac.webkit.org/browser/trunk/LayoutTests/fast/dom/HTMLLinkElement/resources/green-background-color.css> and initially set the background color of <div id="test"> to red. > > [1] <http://www.w3.org/Style/CSS/Test/guidelines.html#indicating> > [2] <http://www.w3.org/Style/CSS/Test/guidelines.html#indicating0> Fixed. >> LayoutTests/media/video-with-file-url-allowed-by-csp-media-src-star.html:1 >> +<!DOCTYPE html> > > If we choose to move this file to LayoutTests/security/contentSecurityPolicy then I suggest that remove "csp" from the name of this file because its ambient directory name conveys that this is a Content Security Policy test and call this file video-with-file-url-allowed-by-media-src-star.html. Fixed. >> LayoutTests/media/video-with-file-url-allowed-by-csp-media-src-star.html:36 >> +<p>This tests that loading a video with a file URL is allowed when the page that is loaded from file URL has Content Security Policy "media-src *". This test PASSED if you don't see a solid green square. Otherwise, it FAILED.</p> > > As aforementioned we should use the color red to signify failure instead of green. Fixed. >> LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:1 >> +<!DOCTYPE html> > > I would name this file script-with-file-url-allowed-by-script-src-star.html and put this file in LayoutTests/security/contentSecurityPolicy. Fixed. >> LayoutTests/security/content-security-policy-matches-star-with-originator-scheme.html:8 >> + <meta http-equiv="Content-Security-Policy" content="script-src *"> > > Minor: The indentation for the children of the HTML head element differs from the indentation used for children of the HTML body element. I suggest that we do not indent the children of the HTML head element. Regardless, we should use a consistent indentation style. Fixed >> LayoutTests/security/resources/append-body.js:1 >> +document.body.innerHTML += '<p>Loaded. Test passes.</p>'; > > I would name this file alert-pass.js to match the name of file of the same purpose in LayoutTests/http/tests/security/contentSecurityPolicy/resources and I would change the implementation to: > > alert("PASS"); Fixed
Comment on attachment 281483 [details] Patch Please rebase this patch after landing the fix for bug #158859.
Created attachment 281518 [details] Patch for landing
Comment on attachment 281518 [details] Patch for landing Attachment 281518 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1515563 New failing tests: security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star.html
Created attachment 281522 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Committed r202155: <http://trac.webkit.org/changeset/202155>
(In reply to comment #19) > Comment on attachment 281518 [details] > Patch for landing > > Attachment 281518 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.webkit.org/results/1515563 > > New failing tests: > security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star. > html This test is frequently failing on the bots https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=security%2FcontentSecurityPolicy%2Fvideo-with-file-url-allowed-by-media-src-star.html