New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Security

Blocking:
issue 662443



Sign in to add a comment

Defend against long-running service workers

Project Member Reported by falken@chromium.org, Sep 21 2016

Issue description

We don't want the web to start relying on being able to keep a sw alive indefinitely.

Rough ideas:
1. Add UMA for lifetime
2. Implement a soft kill/flush state: the SW should restart but events shouldn't be dropped.
3. Add something in the DevTools do to the soft kill/flush on each event (an idea at the F2F).
 
Blocking: 662443
Labels: -Type-Feature Type-Bug-Security
I'm not sure this is a security bug per se, but setting the Bug-Security type so security team is aware of it (see the writeup at  bug 662443 ).

See also  issue 662443 ,  issue 647943 , and https://github.com/w3c/ServiceWorker/issues/980
Cc: homakov@gmail.com
cc'ing homakov since he reported  issue 662443 

falken, any updates on this?
Labels: Security_Impact-Stable Security_Severity-Low OS-All
Can I be re-added to 662443?

I don't have an update since the original comment. This'll need a design doc. Given our workload in Q4 already I was planning on taking this on in 2017Q1 but am willing to adjust priorities if that'd be prudent.
Personally I think this deserves some time/attention before Foreign Fetch ships out of Origin Trials. But if that's not until after Q1, addressing this in Q1 seems reasonable to me.

Comment 7 by homakov@gmail.com, Nov 16 2016

Can anyone aware with the codebase confirm that the Origin Trial rule of %0.03 is active for this particular case #662443 when a service worker fetches a new URL that spanws a new SW, so there's no ForeignFetchEvent apparently and it could be used with no limitations on all users?

Comment 8 by falken@chromium.org, Nov 17 2016

Cc: mek@chromium.org
mek@ would know details about when the origin trials limitation kicks in for foreign fetch.

Comment 9 by mek@chromium.org, Nov 19 2016

I'm not sure what you mean with the "Origin Trial rule of %0.03"? We're definitely monitoring usage of the various parts of the foreign fetch origin trial and if any of it reaches .03% usage we will kill the origin trial.
Cc: jochen@chromium.org
someone pointed out that the SW spec doesn't say what should happen to global variables between several events, is that still the case?
Project Member

Comment 11 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94

commit f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94
Author: mek <mek@chromium.org>
Date: Mon Dec 05 20:42:16 2016

Prevent a service worker from keeping itself alive by self postMessage.

This changes the timeout for message events when the message was sent from
a service worker to be the curent timeout of that service worker. So while
a service worker can still post message to itself, this won't allow it to
stay alive any longer than a single event would.

BUG= 647943 , 648836 

Review-Url: https://codereview.chromium.org/2506263005
Cr-Commit-Position: refs/heads/master@{#436393}

[modify] https://crrev.com/f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
[modify] https://crrev.com/f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/f88b52ec1e4aebd7c1f28b13a58bfad92d04ba94/content/browser/service_worker/service_worker_version.h

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57c41f066cffadb2301263bda7811231d2db02f5

commit 57c41f066cffadb2301263bda7811231d2db02f5
Author: mek <mek@chromium.org>
Date: Mon Dec 05 22:34:16 2016

Limit timeout for foreign fetch events when triggered by another service worker.

BUG= 648836 

Review-Url: https://codereview.chromium.org/2518523003
Cr-Commit-Position: refs/heads/master@{#436431}

[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/foreign_fetch_request_handler.cc
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/foreign_fetch_request_handler.h
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/foreign_fetch_request_handler_unittest.cc
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_controllee_request_handler.cc
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_fetch_dispatcher.cc
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_fetch_dispatcher.h
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_provider_host.h
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_storage.h
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_url_request_job.h
[modify] https://crrev.com/57c41f066cffadb2301263bda7811231d2db02f5/content/browser/service_worker/service_worker_url_request_job_unittest.cc

jochen@ I think it's specd. "Handle Functional Event" which dispatches an event to the service worker, first runs "Run Service Worker". "Run Service Worker" early exits if the worker is already running, otherwise it creates a new ServiceWorkerGlobalScope and settings object an runs the worker. So the global state persists between events if the worker has not been terminated; otherwise the state is reset when the worker starts again.
Cc: -mek@chromium.org falken@chromium.org
Owner: mek@chromium.org
Status: Started (was: Assigned)
mek: OK to assign this to you?
Status: Fixed (was: Started)
I think we can close this one as mek@ closed the known loopholes. I'll open a new one for adding UMA to track the issue in the wild and we can open a new for a more general termination mechanisms if needed.
Labels: M-57
The fixes are in M57.
Project Member

Comment 17 by sheriffbot@chromium.org, Jan 20 2017

Labels: Restrict-View-SecurityNotify
Labels: Release-0-57
Labels: -Release-0-57 Release-0-M57
Project Member

Comment 20 by sheriffbot@chromium.org, Apr 28 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment