Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consult document CSP in Register algorithm #755

Open
wanderview opened this issue Sep 30, 2015 · 10 comments
Open

consult document CSP in Register algorithm #755

wanderview opened this issue Sep 30, 2015 · 10 comments
Milestone

Comments

@wanderview
Copy link
Member

It seems during .register() we should be consulting the CSP of the document to see if the script is allowed. Or is this handled in Run Service Worker in some way? It was not obvious to me.

@annevk
Copy link
Member

annevk commented Sep 30, 2015

Isn't that handled by Fetch?

@wanderview
Copy link
Member Author

Ah, yes. Register algorithm runs Update which in turn uses fetch. Sorry for my confusion.

@wanderview
Copy link
Member Author

Actually, there may be a small issue.

Consider two documents:

  • example.com/foo.html with no CSP headers
  • example.com/bar.html with CSP header child-src: none (or whatever is needed to disallow worker scripts)

Both of these documents execute navigator.serviceWorker.register('sw.js').

If bar.html is loaded first, its register() will fail because of the CSP header when sw.js is fetched.

If foo.html is loaded and then bar.html is loaded in another tab, then the register() in bar.html will succeed. (I think no fetch is initiated here based on Register Algorithm step 4.2.1.2.)

Is this inconsistency desired?

Also, if a document has CSP saying it wants no child scripts, should we ever allow that document to be controlled by a service worker registered by a separate document without CSP?

@wanderview wanderview reopened this Oct 14, 2015
@wanderview
Copy link
Member Author

To be conservative, I believe gecko is going to do a CSP check directly in the .register() before we ever fetch. This will make the results consistent.

@annevk annevk added this to the Version 1 milestone Oct 27, 2015
@annevk
Copy link
Member

annevk commented Oct 27, 2015

getRegistrations() would still work, no? I think leaving CSP to Fetch is probably sanest.

Worrying about intra-origin breakage is kind of not that useful since it can always be broken somehow.

@jakearchibald
Copy link
Contributor

SharedWorker has the same issue.

@jakearchibald jakearchibald modified the milestones: Version 2, Version 1 Oct 27, 2015
@jakearchibald
Copy link
Contributor

+@mikewest

@wanderview
Copy link
Member Author

I recently ran into this again while trying to clean up some old code.

@annevk When you say we should leave this to fetch, how does the fetch for a service worker script during update see the CSP of the document that called register()? AFAICT the fetch triggered by the service worker to get its script does not have a client.

@annevk
Copy link
Member

annevk commented Apr 19, 2018

I think my position is that if CSP didn't ban it for the initial fetch, then it's fine for subsequent fetches to succeed. That's also how SharedWorker works and it seems okay (again, because it's all same-origin).

@wanderview
Copy link
Member Author

wanderview commented Apr 19, 2018

So I was about to write a comment saying that the "initial fetch" does not have the client associated with it so it can't perform CSP checking. I see now, though, we propagate the client for register() and update() calls. For self-update there is no client, however.

That's a bit surprising to me for a couple reasons:

First, consider two pages from the same origin. Page A has no CSP. Page B has CSP which would block a service worker script load. Page A registers a service worker. Page B is loaded and becomes controlled. Page B tries to call update(), but it fails because its CSP blocks it. If Page A calls update() then it succeeds.

Edit: Also, navigation to page B would trigger an automatic soft update internally which would succeed since its done without any knowledge of page B's client or its CSP.

Also, it seems like using the caller's client for these cases could put the service worker script loading in the outer document's fetch group. Should we a register() call that has started, but not fetched yet, to complete if the calling window is closed? I think it should probably complete.

Edit: Maybe this is not an issue in the spec because fetch only adds subresource requests to the client's fetch group.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants