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

Friendly TAG Review: service worker integration #50

Closed
travisleithead opened this issue Sep 28, 2017 · 11 comments
Closed

Friendly TAG Review: service worker integration #50

travisleithead opened this issue Sep 28, 2017 · 11 comments
Assignees

Comments

@travisleithead
Copy link
Member

travisleithead commented Sep 28, 2017

In response to w3ctag/design-reviews#195

  • The natural place to do this logging, etc., is in the service worker. So... having the requests opt-out of Service Worker interception is strange.
  • It seems redundant (if the reports are routed through a service worker--which we think they should be), that a JavaScript API would exist to observe the report on the UI thread. We think the service worker solution already has the mechanics to do this well, and allows customization/modification at no extra cost :o)
@RByers RByers self-assigned this Oct 3, 2017
@RByers
Copy link
Collaborator

RByers commented Oct 3, 2017

Thanks for the feedback!

Being able to observe reports from a SW makes a lot of sense to me. This was an ask from Facebook in their recent BlinkOn talk as well (@n8schloss).

I disagree with that eliminating the need for a JS API though. That requirement (#29) came out of discussions with RUM/analytics providers (eg. https://github.com/WICG/reporting/issues/27#issuecomment-274673657) who often have only the ability to run JS on a page (they're just a library). I don't think, for example, that Google Analytics, would be willing to register a new service worker in order to collect stats like deprecated API usage (and would be unable to do so in the cases where GA is used in an insecure context).

@igrigorik @nicjansma @patrickkettner WDYT?

@igrigorik
Copy link
Member

igrigorik commented Oct 6, 2017

Being able to observe reports from a SW makes a lot of sense to me. This was an ask from Facebook in their recent BlinkOn talk as well (@n8schloss).

AFAIK, we don't route CSP and other error reports through SW today, in part to ensure that a SW can't simply suppress knowledge of such violations — paging @mikewest for input. If that's not an issue, maybe the opt-out is not necessary. That said, also worth noting that some types of errors may not be in a position to spin up a SW.. e.g. crash reports, OOMs, etc., so I'm not sure we can guarantee that all reports would be routed via SW either.

@RByers
Copy link
Collaborator

RByers commented Nov 10, 2017

To @mikewest for above.

@patrickkettner
Copy link
Collaborator

AFAIK, we don't route CSP and other error reports through SW today, in part to ensure that a SW can't simply suppress knowledge of such violations

Whether or not a SW blackholes CSP violations feels like it should be up to the author, no? Ultimately they may decide to do the exact same blackhole-ing on the backend, too. Unless you are suggesting that a malicious SW could be installed that would do this. In which case, I have to imagine they would have much much larger todos MITMing your traffic than just modifying CSP violations.

More to the point, a SW should already be able to intercept and strip content-security-policy header at will from all requests, right? In which case they could rewrite the CSP rules, the uri endpoint, or drop the feature all together.

I'm currently of the opinion that if we are to have a ReportingObserver at all, it should at minimum be in service workers.

cc @igrigorik @mikewest

@igrigorik
Copy link
Member

@patrickkettner I'll defer to @mikewest on the particulars of CSP + SW.

To clarify, I'm not opposed to making reports visible to SW, but I just want to point out that there may be circumstances where reports have to bypass SW, and that we shouldn't make SW a requirement for accessing reports.

@nicjansma
Copy link

As a third-party analytics provider, a JavaScript API is really the only feasible way for us to use the Reporting API. Convincing our customers to add a service-worker (or merge into their SW) is a huge battle.

@juliatuttle
Copy link
Contributor

@nicjansma Is the header-based approach not feasible? I do think a native JS API is a better approach than hooking the HTTP-based uploads with SW, but the HTTP uploads seems doable too.

@nicjansma
Copy link

As a third-party analytics provider (mPulse) that also happens to be part of a CDN (Akamai), a header-based approach can be feasible for us -- because we can possibly do it for our customers at the edge. But even then, not all of our mPulse customers use the CDN. And not every customer even knows how to modify their HTTP headers.

A JavaScript API is night-and-day easier for us to consume, mostly because all it requires a customer to do is ensure our JavaScript is running on their site.

  • Asking a customer to add a JavaScript tag to their site: "sure! I'll add it to the other 25 I have"
  • Asking a customer to add a HTTP response header to their site: [blank stare] "let me have my developers get back to you" followed by convincing them that it won't break their site, followed by 3 months of testing and deployment before it's ready
  • Asking a customer to deploy a service worker to their site: [blank stare] "let me have my developers get back to you" followed by "no"

:)

At least, that's our experience.

@mikewest
Copy link
Member

Sorry I missed this thread, thanks for the ping. :)

AFAIK, we don't route CSP and other error reports through SW today, in part to ensure that a SW can't simply suppress knowledge of such violations — paging @mikewest for input.

I don't actually think there's real danger in exposing CSP violation reports to service workers. My recollection is that we made that decision out of an abundance of caution, way back when service workers were new and scary. Now that they're old (and still scary), it might be reasonable to revisit.

@andypaicu and @dvedtiz might have contrary opinions, but it's worth filing a bug against CSP for discussion.

I think in general, I'm fine with sending reports through the service worker as long as we're careful not to expose data the service worker shouldn't have access to. But I think the calculus for "Should a service worker be able to see this?" is basically the same as "Should the server be able to see this?", so I don't anticipate any problems there.

@andypaicu
Copy link

I agree with the sentiment that CSP reports should be routed through SW. If the SW is maliciously installed, CSP reports are probably very low on the list of bad things that can be done.

I also think we should keep the event based JS API as is, I can imagine there are circumstances where installing a service worker to do the same work as handling an event is not reasonable.

@mikewest
Copy link
Member

Being able to observe reports from a SW makes a lot of sense to me.

Actually, thinking about this again, I don't think it's a good fit for the reporting model to route reports requests through the service worker as requests. One of the reasons that the reporting API is interesting is that it moves away from a synchronous model (like CSP reports today) towards a batched model where an arbitrary set of reports for an endpoint are delivered together. The page that generated any specific report might or might not be alive at the time the report is delivered, and it's not clear that a service worker would be available to route the request through.

A more fundamental issue is that we can only reveal a specific reporting POST to a service worker if it contains data from a single origin. That is, if three sites all decide to use report-to.com as their reporting backend, the client can (and should!) coalesce the various reports into a small number of actual requests over the network. That request would not be generated from a specific, SW-controlled page, but from the browser, at some random point, outside the context of a given page.

I think there could be value in exposing a JS API of some sort in the service worker that would reveal the reports generated from pages it controls, but I don't think it's a good idea to send the actual reporting POST through the worker. It's not a good fit for the model we're putting together.

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

No branches or pull requests

9 participants