CPSP vs shared worker problem

[Fergal Daly]

Hi site-isolation and worker people,

I fixed a security bug (https://crbug.com/517484284) to block PDFs from having access to storage. I did this by adding a call to `CanAccessDataForOrigin`. The unintended side effect is http://b/528332884, where a shared worker now no longer has access to origin private file system (and I assume various other types of storage that it should have).

It seems to require both of these headers (from server.mjs in the repro)

      'cross-origin-opener-policy': 'same-origin',
      'cross-origin-embedder-policy': 'require-corp',

I do not have a good understanding of what these do in this case but this quote from the bug (which seems to be AI-written) seems important.

A worker process never commits an origin, so for a freshly created SharedWorker host CanAccessDataForOrigin(worker_process, page_origin) is false, and the new check denies OPFS. A DedicatedWorker is allocated in the creator frame's process (which has committed the origin), so it is unaffected.

Assuming this is actually correct, should ShWs be doing something different to ensure that their origin is correctly recognized by CanAccessDataForOrigin?

I have several other bugs in my inbox related to isolating PDFs (e.g. https://crbug.com/517634450) and I fear that fixing them will break workers even further.

Thanks,

F

[Fergal Daly]

Post-firework bump on this.

F

[Alex Moshchuk]

Hi Fergal,

Sorry about the late response - a few of us were OOO and just getting back.

We do want to track origins for worker processes so that CanAccessDataForOrigin checks work properly. For ServiceWorkers, we explicitly register their origin with CPSP when creating the worker process. For other workers, including shared workers, we have so far assumed that the creator document and the worker share the same process, so we didn't separately register the worker's origin (as it should match the creator's origin already registered for that process). Your bug shows that this assumption is likely not true for shared workers.

Looking closer at the code, it looks like shared workers can indeed be created in a different process than their creator in exactly the case you ran into, where the creator document is cross-origin-isolated via COOP and COEP headers. This likely happens due to this workaround, which keeps the worker in a non-cross-origin-isolated SiteInstance because shared worker process creation currently happens before the worker script is fetched, and at that point we don't know whether the response headers will include COEP to determine if the worker should actually be cross-origin-isolated.

The solution should be to add a call to ChildProcessSecurityPolicyImpl::GetInstance()->AddCommittedOrigin() for the shared worker case somewhere around here, similarly to how it's done for the for ServiceWorker case.

Thanks,

Alex

--
You received this message because you are subscribed to the Google Groups "navigation-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to navigation-de...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/navigation-dev/CAAozHLkX5syf%3D7UfFNratC2mtpBq%3DVXVkjHpQNrOSSp9yfHBbA%40mail.gmail.com.

[Fergal Daly]

Thanks Alex,

Is https://crrev.com/c/8097788 what you had in mind? The test I added still fails

https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2787483/test-results?q=ExactID%3A%3A%2F%2F%5C%3Aheadless_shell_wpt%21webtest%3A%3Aexternal%2Fwpt%2Ffs%23opfs-shared-worker-coep.https.window.html+VHash%3A100c17e713d67275

A bunch of other tests fail too since it seems `Add` is being called from somewhere else too but I didn't have time to fix that. I wanted to get your take on the shape of the fix,

F

[Alex Moshchuk]

Yes, that's generally what I had in mind, with two changes:

- You should use worker_process_host->GetID() to add the origin, not the PID. CPSP tracks processes with RPH IDs, not the underlying OS pid. This is probably what's making your new test fail, as the subsequent CPSP lookup for OPFS fails to find the origin in the right process.

- You don't need to Add() the worker process to CPSP explicitly, just the origin. All RPHs already get registered with CPSP when they're created. This is probably what's making the other tests fail.

Thanks,

Alex