1179064 - Ensure that service worker code for signed packages is contained within the package

Status: RESOLVED WONTFIX

(Core :: DOM: Security, defect, P2)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

BUG 1178556 ensures that all script that is loaded into a signed package's origin is inside the package itself (i.e. using script-src self). We need to ensure that the same is true for service workers that are loaded for signed packages (we wouldnt want the service worker code coming from ourside the package). The CSP directive child-src was discussed but this is likely too coarse, since signed packages should be allowed to frame any site they want - its just the workers that we need to restrict here.

Comment 1

[Ethan Tseng [:ethan]]

Hi Dimi,
I suppose this bug should be assigned to you.
If it's not appropriate, please let me know. :)

Comment 2

[Dimi Lee [:dimi]]

this bug will not affect NSec first milestone

Comment 3

[Dimi Lee [:dimi]]

Just check the code and if I understand correctly, ServiceWorkerManager will check SOP for registered worker script[1]. So for SW loaded in signed package, it should not be able to load service worker code comes from different origin. Also, scope is check in[2].

And what we do not handle right now is for two signed packages with same host, it will be treated as same origin[3]. So maybe this bug is not really related to SW but related to we should handle |checkMayLoad| properly based on bug 1178526 ?

Hi Jonas, does this make sense to you?

[1]https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#1531
[2]https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#1537
[3]https://dxr.mozilla.org/mozilla-central/source/netwerk/base/nsNetUtil.cpp#1694

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I suspect that the same-origin checks won't automatically detect that a URL which is outside of the package, but still pointing to the same server, will be considered different-origin. This is because they will only differ in the OriginAttributes, and URLs don't have OriginAttributes, only principals do.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

In other words, I don't think the code as written right now will work.

documentPrincipal->CheckMayLoad(aScopeURI, true, false);

will return success even if documentPrincipal has the signedPkg set as long as aScopeURI is in the same server.

Comment 6

[Dimi Lee [:dimi]]

As discussed with jonas in todays NSec meeting, i will implement this inside ServiceWorker's code by checking signedPkg in documentPrincipak and compare it with the scriptURI & scopeURI arguments

Comment 7

[Dimi Lee [:dimi]]

In today's Nsec meeting, we would like to try not modifying service worker's code. 

And after checking with Christoph, to CSP to restrict service worker could only be loaded inside the signed package and still allow load frame, we could use:
child-src 'self'; frame-src *

Comment 8

[Dimi Lee [:dimi]]

We should add this to default CSP of signed package

Comment 9

[Mahendra Potharaju [:mahe]]

[Tracking Requested - why for this release]:

This bug being part of New Security Model shouldn’t be a 2.5 blocker as New Sec is not part of 2.5 now.

Removing 2.5 blocker flag.

Comment 10

[Ethan Tseng [:ethan]]

The new security model project was suspended.
We had discussed the idea of NSEC v2, which seems we are unlikely to use signed package format.
So there is no reason to keep working on this bug.

p.s. We don't have plan on NSEC v2 either.