privacycg / storage-partitioning

Client-Side Storage Partitioning
https://privacycg.github.io/storage-partitioning/
72 stars 9 forks source link

consider including a "cross-site ancestor chain" bit in the storage key #25

Open wanderview opened 3 years ago

wanderview commented 3 years ago

Currently service workers have poor SameSite cookie protections because its "site for cookies" is simply set to the origin:

https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.2.2.2

In contrast, documents take into account the top-level-site and the ancestor chain when computing "site for cookies":

https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis#section-5.2.1

This is problematic because it means adding a service worker to a site can reduce the safety of SameSite cookies.

With storage partitioning we have the opportunity to fix this. We already plan to include top-level site in the storage key which will allow us to include it in the "site for cookies" computation for service worker. We lack any ancestor chain information, however.

The ancestor chain is important for "site for cookies" because it helps protect against clickjacking attacks. To extend this protection to service workers we propose:

Include a "cross-site ancestor chain" bit in the storage key. This bit would be true if there are any sites between the current context and the top-level context that are cross-site to the current context. So it would be true for A -> B -> C or A -> B -> A. It would be false for A -> A or A -> B.

With this bit in the storage key it would permit us to compute a "site for cookies" value for service workers that is equivalent to any document controlled by that service worker.

This was discussed at the recent service worker virtual F2F: w3c/ServiceWorker#1604.

annevk commented 3 years ago

Just to be clear on the implications here:

  1. Since it's part of the storage key, it would affect an entire storage shelf. I.e., service workers, localStorage, etc.
  2. It would mean that documents that have synchronous script access (in A1 -> B -> A2, this would go for A1 and A2) no longer observe the same storage shelf so you have to be careful where you peek into storage.

The benefit of this is that sites can adopt service workers easily without ending up in a worse security situation. That makes me think it's worth it, despite the fact that 2 above is something I had been trying to avoid.

wanderview commented 3 years ago

Are there known use cases for synchronous scripting in this scenario? Could we look to make synchronous scripting also dependent on the cross-site entries in the ancestor chain?

annevk commented 3 years ago

In theory it's possible, but I'm not sure what the compat story for that is. Would it be a new agent cluster essentially or something in between? (We'd also have to untangle the window name targeting algorithm presumably, but that needs solving anyway.)

annevk commented 2 years ago

In the last call @bvandersloot-mozilla mentioned Mozilla was in principle on board with this. @johnwilander has Apple looked at this?

A point that @johannhof brought up that I think is quite important is offering consistency to web developers. I don't think it would be a great outcome if storage is partitioned, but network and cookies are not. Ideally we offer the same boundary across the board.

wanderview commented 2 years ago

A point that @johannhof brought up that I think is quite important is offering consistency to web developers. I don't think it would be a great outcome if storage is partitioned, but network and cookies are not. Ideally we offer the same boundary across the board.

I agree with this in principle, but I've been told there are important use cases for cookies that make the situation different from other storage. I would also note cookies are already quite different from other storage in other ways. Expiration, quota, network transmission, etc. They also tend to be used for different use cases; auth vs stateful app logic, etc. While undesirable, it doesn't seem too extreme to diverge on this nuanced aspect of partitioning.

annevk commented 2 years ago

I opened #31, #32, and https://github.com/WICG/CHIPS/issues/40 for the various cookie-related issues around this. I agree that in principle we could end up with a different solution there, but I'm rather worried about subtle application bugs and web developer confusion. As well as making it difficult to offer consistent APIs around partitioning, such as whether a given environment is considered partitioned or not.

wanderview commented 2 years ago

Note, this statement from the original post above ended up not being quite right:

This bit would be true if there are any sites between the current context and the top-level context that are cross-site to the current context.

We need to also check the top-level context and not just ancestors in-between. Basically we set the bit to true if site-for-cookies is not populated.

guest271314 commented 1 year ago

This concept - implemented as default on Chromium 112 - is causing issues.

Before this I could append an iframe to a Web document, set src to chrome-extension: URL and the iframe would be set as a WindowClient of the MV3 ServiceWorker, and navigator.serviceWorker.getRegistrations() fulfilled to an array containing the MV3 ServiceWorker. With this enabbled by default neither occur.

We have to launch Chromium and Chrome with --disable-features=ThirdPartyStoragePartitioning to get the behaviour we expect.

Is the goal of this really for embedded iframes to not be a WindowClient of the ServiceWorker scope they are registered under; and for a document loaded under the scope of a ServiceWorker registration to not get a reference to the active ServiceWorker?

wanderview commented 1 year ago

Please file a chromium bug:

https://bugs.chromium.org/p/chromium/issues/entry?labels=StoragePartitioning-trial-bugs&components=Blink%3EStorage

We have some affordances for extensions and we might be able to accomodate this as well. I think this is more of an extensions thing, though, and does not really belong in this issue.

wanderview commented 1 year ago

I went ahead and filed https://crbug.com/1411422.

guest271314 commented 1 year ago

You will have to see the matter re extensions through. Chromium authors banned me from filing bugs there. W3C (where Web Extensions has a group), and Google groups (extensions) banned me.

wanderview commented 1 year ago

Understood. We'll take a look at it. At first glance it seems like something we should fix. Thanks for the report.

(And also, we have not enabled partitioning by default in any chrome release yet. I'm guessing maybe you have experimental web features enabled in chrome://flags or something.)