Bug 182160

Summary: CSP post checks should be done for service worker responses
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, dbates, ews-watchlist, japhet, mkwst, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews107 for mac-sierra-wk2
none
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
Patch
none
Patch for landing none

Description youenn fablet 2018-01-25 17:00:24 PST
CSP post checks should be done for service worker responses
Comment 1 youenn fablet 2018-01-25 17:02:25 PST
Created attachment 332336 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2018-01-25 17:03:55 PST
<rdar://problem/36889274>
Comment 3 EWS Watchlist 2018-01-25 17:51:55 PST
Comment on attachment 332336 [details]
Patch

Attachment 332336 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/6216133

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-referrer-policy.https.html
Comment 4 EWS Watchlist 2018-01-25 17:51:56 PST
Created attachment 332340 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 5 EWS Watchlist 2018-01-25 18:20:52 PST
Comment on attachment 332336 [details]
Patch

Attachment 332336 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6216405

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-event-referrer-policy.https.html
Comment 6 EWS Watchlist 2018-01-25 18:20:54 PST
Created attachment 332341 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 7 youenn fablet 2018-01-25 19:33:41 PST
Created attachment 332344 [details]
Patch
Comment 8 Daniel Bates 2018-01-25 19:56:28 PST
Comment on attachment 332344 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=332344&action=review

This patch does more than adds a CSP check. It also adds a mixed content check. We also need to add a nosniff check and the Fetch spec also has a MIME type check. Do you plan to follow up to add the other checks?

> Source/WebCore/loader/SubresourceLoader.cpp:305
> +            || !loader.checkInsecureContent(m_resource->type(), response.url())) {

Please add test(s) for mixed content.
Comment 9 youenn fablet 2018-01-26 08:16:44 PST
(In reply to Daniel Bates from comment #8)
> Comment on attachment 332344 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332344&action=review
> 
> This patch does more than adds a CSP check. It also adds a mixed content
> check. We also need to add a nosniff check and the Fetch spec also has a
> MIME type check. Do you plan to follow up to add the other checks?

no sniff and mime type checks are done at the response processing level right now.
We could do some refactoring in the future to better match the spec but this is not needed right now.

> > Source/WebCore/loader/SubresourceLoader.cpp:305
> > +            || !loader.checkInsecureContent(m_resource->type(), response.url())) {
> 
> Please add test(s) for mixed content.

I'll remove the check for now and will investigate potential mixed content issues as a follow-up.
Comment 10 youenn fablet 2018-01-26 08:22:42 PST
Created attachment 332374 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2018-01-26 09:36:52 PST
Comment on attachment 332374 [details]
Patch for landing

Clearing flags on attachment: 332374

Committed r227680: <https://trac.webkit.org/changeset/227680>
Comment 12 WebKit Commit Bot 2018-01-26 09:36:53 PST
All reviewed patches have been landed.  Closing bug.