Closed Bug 1532287 Opened 6 years ago Closed 5 years ago

Make sure Cross-Origin policy is passed to workers and service workers and respected in SW fetch

Categories

(Core :: DOM: Service Workers, enhancement, P2)

60 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla78
Fission Milestone Future
Tracking Status
firefox78 --- fixed

People

(Reporter: valentin, Assigned: edenchuang)

References

Details

Attachments

(2 files)

Bug 1525036 adds the crossOriginPolicy to the BrowsingContext, but we also need to make sure it is passed when creating new workers/service-workers, and a SW fetch checks the policy of the service-worker.

See https://gist.github.com/annevk/17f580379c45802d5c3aef5a8fd53c7d

Fission Milestone: --- → ?

The priority flag is not set for this bug and there is no activity for 2 weeks.
:overholt, could you have a look please?

Flags: needinfo?(overholt)
Flags: needinfo?(overholt)
Priority: -- → P2
Fission Milestone: ? → Future

Restating Overview:

  • We want to make sure that opaque response data and meta-data never ends up inside a process where SharedArrayBuffer/anything that enables high-resolution-timers and thereby enable Spectre-style attacks can get at the data.
  • We are not creating a separate storage partition, we are creating a notional "CORS clean room" which explicitly results in errors if any attempt is made to bring opaque (non-CORS) data into the clean room.
  • Because the crossOriginPolicy of the top-level script Response of a ServiceWorker defines the global's crossOriginPolicy and we persist that response, a given WebIDL ServiceWorker instance should always have the same crossOriginPolicy.
  • All the other checks can then be performed based on the value on the global/client.

At an implementation level (which potentially will need to be addressed in other bugs), our SW concerns are:

  • The decision of where to spawn the ServiceWorker (in the parent-intercept case) doesn't actually have the top-level script Response available. It just has the registration info as stored in ServiceWorkerRegistrationData[1].
    • So we need to cache additional information on the registration and update ServiceWorkerRegistrar to provide it. This information characterizes only the installed worker, and currentWorkerHandlesFetch is already updated in lock-step. The cacheName is currently stored as a {}-enclosed UUID and could be hacked up to overload that info. More ideally, perhaps we could move to a rust-based ServiceWorkerRegistrar since we're dealing with an IPDL type that allows for drawing an implementation boundary.
  • The WorkerPrivate::Constructor invocation needs the crossOriginPolicy at instantiation time which is also before the Response is available.
  • The ScriptLoader will need to be updated to process the crossOriginPolicy and assert that it's consistent with the global and the current process (which can be a project:fission provided helper.)
  • We need checks and assertions that ensure that we never send opaque data/metadata to clean-room processes in the Cache API, inclusive of fd's. It might make sense to make these changes directly in the Response implementation.
  • FetchDriver also needs to be updated to apply the https://hg.mozilla.org/integration/autoland/rev/b77be7643141a8f3646257e599b38a3be862e6e0 logic so that it applies for workers. In that patch the (document, main-thread-specific) BrowsingContext is extracted from the document.
    • The FetchDriver gets the WorkerPrivate's nsIPrincipal, nsILoadGroup, CookieSettings (via its mLoadInfo.mCookieSettings), and ClientInfo. There's always the question of what's the practical and semantically ideal way to get this data across and unify it across documents/workers. Ideally that's the client. And right now the pseudo-spec text says "Let corsCredentials be request's client's global object's cross-origin policy" so arguably client is the right answer here.
    • So we might also want to refactor crossOriginPolicy off of the BrowsingContext and onto the client, as much of the point of the Clients API's implementation in Gecko internals was to unify the code.

1: https://searchfox.org/mozilla-central/source/dom/serviceworkers/ServiceWorkerRegistrarTypes.ipdlh

I'm notionally taking this, but if anyone wants to steal, feel free. I'll switch to ASSIGNED next week once work begins in earnest.

Assignee: nobody → bugmail

Maybe we should have a chat about what we need here for a minimum viable product and what we need long term. At a minimum I think we need these things:

  1. Dedicated workers need to respect COEP of their creator. Dedicated workers have ways of fetching opaque responses and therefore need to block loading if COEP is not set for them. (They should not inherit COEP as that leads to inconsistent fetching state.)
  2. We need to ensure that service workers end up in a separate process from the process that has the "allow postMessage() for SharedArrayBuffer" bit set (i.e., a document that opted in with COOP + COEP).
  3. If a service worker feeds an opaque response to a process that has the bit set, ensure it has the relevant header and otherwise network error before the response reaches the process.

In the future we want:

  1. Allowing shared/service workers to opt into COEP themselves so they can use SharedArrayBuffer in threaded fashion as well. (I'm not actually sure if we allow nested dedicated workers yet in these worker types. If that's not a thing that would also need to become a thing for this to be truly useful.)

Background reading on COOP and COEP: https://docs.google.com/document/d/1zDlfvfTJ_9e8Jdc8ehuV4zMEu9ySMCiTGMS9y0GU92k/edit.

Happy to chat more in person if that would help.

Depends on: 1543068
Assignee: bugmail → perry
Status: NEW → ASSIGNED
Blocks: 1613061

(To be clear, this still blocks resab, just no longer directly.)

No longer blocks: resab
Assignee: perry → echuang
Blocks: 1603168
Attachment #9145491 - Attachment description: P2 Propagate Worker's COEP to HttpChannels → P2 Propagate loading document/worker's COEP to nsHttpChannel through nsILoadInfo
Attachment #9145489 - Attachment description: P1 Save COEP in InternalRequest → P1 Saving the loading document/worker's COEP in InternalRequest.
Attachment #9145489 - Attachment description: P1 Saving the loading document/worker's COEP in InternalRequest. → Bug 1532287 - P1 Saving the loading document/worker's COEP in InternalRequest.
Pushed by btara@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9217f5aa798c P1 Saving the loading document/worker's COEP in InternalRequest. r=dom-workers-and-storage-reviewers,perry https://hg.mozilla.org/integration/autoland/rev/7c9708ccec41 P2 Propagate loading document/worker's COEP to nsHttpChannel through nsILoadInfo r=necko-reviewers,valentin,JuniorHsu
Blocks: 1575095
No longer depends on: 1575095
Blocks: 1636699
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/5571b75d82b6 Set wpt tests as passing. CLOSED TREE DONTBUILD
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: