Bug 1794508 Comment 3 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I'm going to attempt to summarize this situation which is discussed and investigated in much greater depth at https://mozilla.slack.com/archives/C4S6616E9/p1665422205062729 (although there's a process of discovery there).

:pascalc I see you are listed as the release owner for Firefox 106 at https://fx-trains.herokuapp.com/release/?version=106 so I wanted to bring this to your attention as it seems like if we don't land https://phabricator.services.mozilla.com/D159002 into the beta we may want it to be able to get into a dot release.

### General Problem Statement

On release, beta, and nightly, it's possible for ServiceWorkers to be persisted to disk in private browsing mode for origins where [ePartitionForeignOrDeny or ePartitionTrackersOrDeny](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/toolkit/components/antitracking/StorageAccess.h#32-37) are returned from the [StorageAllowedFor* methods in StorageAccess.h](https://searchfox.org/mozilla-central/source/toolkit/components/antitracking/StorageAccess.h).  I don't fully understand what situations these are, but ideally :timhuang can help contextualize when this would be, I've set a needinfo.

The concern is that we have never intended to allow ServiceWorkers to be persisted to disk in private browsing mode because our threat model has generally been "data does not get written to disk in private browsing mode"[1].  When this happens, an origin the user has indirectly visited will have its name written to disk in the clear and any data related to the script will be written in the clear, albeit compressed.  It seems likely that the ServiceWorker would fail to be able to write any data to the Cache API storage though, so it should be just the ServiceWorker that gets installed.

This bug should stop new ServiceWorkers from being installed.

### What happens to profiles where the bug happened? / Follow-ups

It's quite possible that ServiceWorkers installed via this mechanism will not be cleaned up and will stick around until the user cleans them up.

Specifically, as I understand it, Private Browsing cleanup happens via the ["last-pb-context-exited"](https://searchfox.org/mozilla-central/search?q=%22last-pb-context-exited%22&path=&case=false&regexp=false) observer notification.  I do not currently see any connective linkage between this observer notification and nsIClearDataService or sanitizer.jsm which could serve as a back-stop, but I hopefully am missing something[2].  Such glue would potentially look like a call to [nsIClearDataService.deleteDataFromOriginAttributesPattern](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/toolkit/components/cleardata/nsIClearDataService.idl#127-134) specifying a pattern specifying [privateBrowsingId: 1](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/dom/chrome-webidl/ChromeUtils.webidl#887).  This would potentially be a reasonable thing for us to add if it's not already there.  In particular, this should clean up the origin directory as well as the registration.

In terms of the simplest way to clean this up, we can add logic [similar to the web-extension ServiceWorker purging logic](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/dom/serviceworkers/ServiceWorkerManager.cpp#1511-1523) we already implement which ensures that we ignore registrations we don't want and purge their related cache.  I think there is actually an oversight in that logic, however, in that I think we're failing to call [MaybeSendUnregister](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/dom/serviceworkers/ServiceWorkerManager.cpp#3305) which will actually cause the registration to be dropped from the ServiceWorkerRegistrar's copy of the registrations.  Without that, although we won't show the registration in about:serviceworkers, it would still be persistent on disk.  This will remove the serviceworker but I do not believe it will be sufficient to clear the QuotaManager-managed origin directory.

An option to ensure that the origin directory gets cleaned up (separate from the registration) is to ensure that QuotaManager's initialization notices any directories with a private browsing id and purges them.  This would not be sufficient to remove the registration from the `serviceworker.txt` file on its own.

Arguably implementing either of these clean-ups does make it more clear what's going on, so it might make sense for clean-up logic to land in a second set of fixes in the future if we're concerned about bad actors intentionally trying to install ServiceWorkers in private browsing mode.  It's not clear if this is actually a concern though, compared to cleaning up the private browsing byproducts on disk.  That said, the reversion in https://phabricator.services.mozilla.com/D159002 is extremely straightforward and safe to land and uplift all over the place, whereas most of these potential cleanup fixes want testing, etc. that we don't have ready to go yet.

1: Note that we are moving towards allowing encrypted data to be written to disk as long as all file paths also obscure the origins at play.
2: Like how does our IndexedDB private-browsing support clean up?
I'm going to attempt to summarize this situation which is discussed and investigated in much greater depth at https://mozilla.slack.com/archives/C4S6616E9/p1665422205062729 (although there's a process of discovery there).

:pascalc I see you are listed as the release owner for Firefox 106 at https://fx-trains.herokuapp.com/release/?version=106 so I wanted to bring this to your attention as it seems like if we don't land https://phabricator.services.mozilla.com/D159002 into the beta we may want it to be able to get into a dot release.

### General Problem Statement

On release, beta, and nightly, it's possible for ServiceWorkers to be persisted to disk in private browsing mode for origins where [ePartitionForeignOrDeny or ePartitionTrackersOrDeny](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/toolkit/components/antitracking/StorageAccess.h#32-37) are returned from the [StorageAllowedFor* methods in StorageAccess.h](https://searchfox.org/mozilla-central/source/toolkit/components/antitracking/StorageAccess.h).  I don't fully understand what situations these are, but ideally :timhuang can help contextualize when this would be, I've set a needinfo.

The concern is that we have never intended to allow ServiceWorkers to be persisted to disk in private browsing mode because our threat model has generally been "data does not get written to disk in private browsing mode"[1].  When this happens, an origin the user has indirectly visited will have its name written to disk in the clear as well as the first-party top-level origin name and any data related to the script will be written in the clear, albeit compressed.  It seems likely that the ServiceWorker would fail to be able to write any data to the Cache API storage though, so it should be just the ServiceWorker that gets installed.

This bug should stop new ServiceWorkers from being installed.

### What happens to profiles where the bug happened? / Follow-ups

It's quite possible that ServiceWorkers installed via this mechanism will not be cleaned up and will stick around until the user cleans them up.

Specifically, as I understand it, Private Browsing cleanup happens via the ["last-pb-context-exited"](https://searchfox.org/mozilla-central/search?q=%22last-pb-context-exited%22&path=&case=false&regexp=false) observer notification.  I do not currently see any connective linkage between this observer notification and nsIClearDataService or sanitizer.jsm which could serve as a back-stop, but I hopefully am missing something[2].  Such glue would potentially look like a call to [nsIClearDataService.deleteDataFromOriginAttributesPattern](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/toolkit/components/cleardata/nsIClearDataService.idl#127-134) specifying a pattern specifying [privateBrowsingId: 1](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/dom/chrome-webidl/ChromeUtils.webidl#887).  This would potentially be a reasonable thing for us to add if it's not already there.  In particular, this should clean up the origin directory as well as the registration.

In terms of the simplest way to clean this up, we can add logic [similar to the web-extension ServiceWorker purging logic](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/dom/serviceworkers/ServiceWorkerManager.cpp#1511-1523) we already implement which ensures that we ignore registrations we don't want and purge their related cache.  I think there is actually an oversight in that logic, however, in that I think we're failing to call [MaybeSendUnregister](https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/dom/serviceworkers/ServiceWorkerManager.cpp#3305) which will actually cause the registration to be dropped from the ServiceWorkerRegistrar's copy of the registrations.  Without that, although we won't show the registration in about:serviceworkers, it would still be persistent on disk.  This will remove the serviceworker but I do not believe it will be sufficient to clear the QuotaManager-managed origin directory.

An option to ensure that the origin directory gets cleaned up (separate from the registration) is to ensure that QuotaManager's initialization notices any directories with a private browsing id and purges them.  This would not be sufficient to remove the registration from the `serviceworker.txt` file on its own.

Arguably implementing either of these clean-ups does make it more clear what's going on, so it might make sense for clean-up logic to land in a second set of fixes in the future if we're concerned about bad actors intentionally trying to install ServiceWorkers in private browsing mode.  It's not clear if this is actually a concern though, compared to cleaning up the private browsing byproducts on disk.  That said, the reversion in https://phabricator.services.mozilla.com/D159002 is extremely straightforward and safe to land and uplift all over the place, whereas most of these potential cleanup fixes want testing, etc. that we don't have ready to go yet.

1: Note that we are moving towards allowing encrypted data to be written to disk as long as all file paths also obscure the origins at play.
2: Like how does our IndexedDB private-browsing support clean up?

Back to Bug 1794508 Comment 3