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

Why does every network request skip service workers? #669

Closed
domfarolino opened this issue Jun 24, 2023 · 11 comments
Closed

Why does every network request skip service workers? #669

domfarolino opened this issue Jun 24, 2023 · 11 comments
Labels
spec Relates to the spec

Comments

@domfarolino
Copy link
Collaborator

In Fetch, the default service-workers mode is "all". This is explicitly overrides it to "none" for every network request (click this reference pane), while the only browser that implements this specification too defaults to not skipping service workers, and the implementation of this spec in that browser appears to not override that default.

This seems to be an important mismatch between the (currently only) implementation of this spec and the spec prose itself.

/cc @jyasskin

@domfarolino domfarolino added the spec Relates to the spec label Jun 24, 2023
@qingxinwu
Copy link
Collaborator

You're correct! Our implementations of resource requests do not overwrite the skip_service_worker field which defaults to false. Just noticed when working on a PR for fetching reports. Going to send a PR to fix this.

Our implementations:

@domfarolino
Copy link
Collaborator Author

Web platform tests for this would be great too, since I’m not sure if there are any service workers quirks with browser initiated requests in chromium. But tats would let us know for sure.

@qingxinwu
Copy link
Collaborator

Sure, created a task to track that.
http://b/293383734

@michaelkleber
Copy link
Collaborator

Hmm, this gets into CORS territory, doesn't it? I think we may need to skip service workers for some fetches: Ad auctions occur in an environment where the different parties on a page cannot be assumed to trust each other, and so we need to avoid any case where domain A is able to respond to a request that the API expects to be served from domain B.

This could come up, for example, in the fetch of the decisionLogicURLfor an ad auction. The URL must be same-origin with seller, and the seller is in turn known to buyers as part of the browserSignals argument to generateBid(). If a Service Worker were able to substitute a different script response, then buyers might be misled about the auction they were bidding in.

My understanding of service worker interaction with third-party content is limited, but this does require some careful thinking.

@domfarolino
Copy link
Collaborator Author

My understanding of service worker interaction with third-party content is limited, but this does require some careful thinking.

Mine is too, unfortunately. My understanding is that they are or are being partitioned by top-level origin, but yes I definitely agree this requires a more thorough deep-dive.

@qingxinwu
Copy link
Collaborator

@MattMenke2 has the best knowledge about our request implementation. @MattMenke2 would you mind reviewing the spec's documentation of fetch settings to see if any field differs from the implementaion (see the [links](#669 (comment) above)?
More specifically,

  1. Whether client is "null".
  2. For request/service-workers mode, the implementation uses the default "skip-service-worker = false", so I think it's "all", not "none".
  3. request/referrer
  4. For request/referrer-policy, the implementation uses the default "NEVER_CLEAR", which sames matched to the "unsafe-url" and so may need to add a field for that?

Thanks.

@MattMenke2
Copy link
Contributor

All fetches potentially contain private information in their URLs that I'm not sure we want to shared with a Service Worker (In particular, the case where the fetch is same-origin to the page are a problem, as they reveal the full URL.
Even 3P requests, which the SW may not have access to (I'm not an expert) reveal that the user is in a 3P IG of an origin the SW may still be able to read).

The only ones that might be able to use an SW are seller script URLs, but even then, the fetch itself can potentially reveal if the user is in any IGs. The top-level seller script reveals the least information, since there's only one per auction, but even that is potentially problematic.

The scripts don't go through SW because they're sent directly to the network stack, just like internal Chrome network requests. There's no SW loader stuck in front of the network stack for them, so the value of skip_service_worker has no effect. So despite not explicitly setting it to none currently, it is effectively none, since no service worker loader factory is in front of the real network loader factory.

Referrer (and initiator) are currently set to the page itself, though there's a question about changing the initiator, particularly for the bidder requests. We don't follow redirects (largely due to redirect caching potentially being a problem, though reusing bidder scripts is also currently a problem, which we do allow, since caching those between IGs learned on different sites can allow them to share information cached in the JS file itself), so the referrer policy doesn't currently matter.

For the seller worklet JS fetch (and WASM, I believe), we currently inherit network-state side stuff from the real network loader used by the page (including the origin lock, network partition key, etc), since it is effectively a URL generated entirely by a 3rd party. For the bidder JS/WASM URLs, we make a pseudo-1p request context, with the site's cache partition (We need a better way to cache these in isolated partitions, at some point). For the signals requests, we give the requests their own isolated network partitions, to avoid leaking information to any other contexts. The seller signals URL is a bit funky, as the base URL is specified by a 3rd party, so just what we should do there is probably worth a bit more thought (and this problem, in general, is certainly worth thinking about more).

@MattMenke2
Copy link
Contributor

And just to be explicit: All leaks concerning using an SW are nearly identical to using the browser cache associated with a particular network partition key. We still need to figure out how to address the latter, while still caching JS files.

@qingxinwu
Copy link
Collaborator

PR updating service worker mode is merged.

Copying the comment from that PR here:
Our implementation uses default "all" for service worker mode, not "none". However, given the fact that all of these requests have a null client, one of the side-effects of this is neutering all service worker interception (effectively "none" although set to default "all), despite not having to set the service workers mode. The service worker interception algorithm returns at step 15.2 (our request has the default kEmpty destination, so it's a subresource request), and nothing happened before this step.

@domfarolino
Copy link
Collaborator Author

Thanks a lot! In that case, I think this can be closed. Do you agree? I'll close this, but re-open if you disagree.

@qingxinwu
Copy link
Collaborator

yes, I agree that this can be closed. Thanks!

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

No branches or pull requests

4 participants