Bug 182160 – CSP post checks should be done for service worker responses

• Component: Service Workers
• Status: RESOLVED FIXED

[youenn fablet]

CSP post checks should be done for service worker responses

[youenn fablet]

Created attachment 332336 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/36889274>

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[youenn fablet]

Created attachment 332344 [details]
Patch

[Daniel Bates]

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.

[youenn fablet]

(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.

[youenn fablet]

Created attachment 332374 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 332374 [details]
Patch for landing

Clearing flags on attachment: 332374

Committed r227680: <https://trac.webkit.org/changeset/227680>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.