Correctly set initiated_in_secure_context on requests from shared/service workers,

Correctly set initiated_in_secure_context on requests from shared/service workers,

This field was added in https://crrev.com/dd933bda0baa6a13ab0120f0056a2b783e459efb
to limit foreign fetch to only intercept requests made from secure contexts,
but wasn't set for shared and service worker initiated requests. So this
fixes intercepting requests made from shared and service workers by a
foreign fetch service worker.

BUG=540509
Committed: https://crrev.com/ebb3beacf3c308eb79b4f3e034a8144df0a16a18
Cr-Commit-Position: refs/heads/master@{#403372}

[Marijn Kruisselbrink, 2016-06-29 22:47:16]

Description was changed from

==========
Correctly set initiated_in_secure_context on requests from shared/service
workers,

This fixes intercepting requests made from shared and service workers by a
foreign fetch service worker.

BUG=540509
==========

to

==========
Correctly set initiated_in_secure_context on requests from shared/service
workers,

This field was added in
https://crrev.com/dd933bda0baa6a13ab0120f0056a2b783e459efb
to limit foreign fetch to only intercept requests made from secure contexts,
but wasn't set for shared and service worker initiated requests. So this
fixes intercepting requests made from shared and service workers by a
foreign fetch service worker.

BUG=540509
==========

[Marijn Kruisselbrink, 2016-06-29 22:47:46]

mek@chromium.org changed reviewers:
+ horo@chromium.org

[horo, 2016-06-30 10:12:09]

This patch lgtm.

But if the fetch request from SW goes to the other SW, it is possible to make
infinite loop.
Do you have any plan to protect the browser from the infinite loop attack?

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html
(right):

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html:50:
let worker = new SharedWorker('resources/foreign-fetch-helper-script.js');
nit: Wrap at 80 columns. 
https://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html:86:
}, 'Service Worker does not intercept fetches from an insecure dedicated
worker.');
ditto

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html:101:
}, 'Service Worker does not intercept fetches from an insecure shared worker.');
ditto

[Marijn Kruisselbrink, 2016-06-30 22:43:12]

On 2016/06/30 at 10:12:09, horo wrote:
> But if the fetch request from SW goes to the other SW, it is possible to make
infinite loop.
> Do you have any plan to protect the browser from the infinite loop attack?

Good question. I think we had some discussions about this from the standards
side a while ago, although I can't find those at the moment. But the conclusion
there was that actually detecting any kind of loops like this is going to be
nearly impossible, and probably we don't need to do so. To actually end up in a
loop a SW will have to do make a different request than what it was handling in
its onforeignfetch event, and it can even do so asynchronously (or it could even
postmessage a different same-origin service worker, and have that service worker
make the new fetch request).

The worst that can happen is that multiple service workers bombard each other
with foreignfetch events. It shouldn't really effect the browser too much. All
this stuff is async, so from the browsers point of view this isn't very
different from a page repeatedly calling fetch. I guess the only extra bad thing
about this is that this is yet another vector for a service worker (or in this
case a set of service workers) to keep themselves alive, arbitrarily long after
any page that might have used them has gone away (postmessage can already do
that). Not sure what heuristics we could come up with automatically detect
misbehaving service workers. If the things we're worried about is actually
actively malicious service workers those heuristics have to be a lot better than
if we're just trying to catch accidentally misbehaving service workers.

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html
(right):

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html:50:
let worker = new SharedWorker('resources/foreign-fetch-helper-script.js');
On 2016/06/30 at 10:12:09, horo wrote:
> nit: Wrap at 80 columns. 
> https://www.chromium.org/blink/serviceworker/testing#TOC-Layout-Tests

Done

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html:86:
}, 'Service Worker does not intercept fetches from an insecure dedicated
worker.');
On 2016/06/30 at 10:12:09, horo wrote:
> ditto

Done

https://codereview.chromium.org/2110163002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-workers.html:101:
}, 'Service Worker does not intercept fetches from an insecure shared worker.');
On 2016/06/30 at 10:12:09, horo wrote:
> ditto

Done

[Marijn Kruisselbrink, 2016-06-30 23:04:10]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-06-30 23:04:10]

The patchset sent to the CQ was uploaded after l-g-t-m from horo@chromium.org
Link to the patchset: https://codereview.chromium.org/2110163002/#ps20001
(title: "wrap layout test at 80 columns")

[commit-bot: I haz the power, 2016-06-30 23:05:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 00:32:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-01 00:34:14]

Description was changed from

==========
Correctly set initiated_in_secure_context on requests from shared/service
workers,

This field was added in
https://crrev.com/dd933bda0baa6a13ab0120f0056a2b783e459efb
to limit foreign fetch to only intercept requests made from secure contexts,
but wasn't set for shared and service worker initiated requests. So this
fixes intercepting requests made from shared and service workers by a
foreign fetch service worker.

BUG=540509
==========

to

==========
Correctly set initiated_in_secure_context on requests from shared/service
workers,

This field was added in
https://crrev.com/dd933bda0baa6a13ab0120f0056a2b783e459efb
to limit foreign fetch to only intercept requests made from secure contexts,
but wasn't set for shared and service worker initiated requests. So this
fixes intercepting requests made from shared and service workers by a
foreign fetch service worker.

BUG=540509

Committed: https://crrev.com/ebb3beacf3c308eb79b4f3e034a8144df0a16a18
Cr-Commit-Position: refs/heads/master@{#403372}
==========

[commit-bot: I haz the power, 2016-07-01 00:34:15]

Patchset 2 (id:??) landed as
https://crrev.com/ebb3beacf3c308eb79b4f3e034a8144df0a16a18
Cr-Commit-Position: refs/heads/master@{#403372}