1413615 - consider allowing service workers when cookie policy is "keep until I close firefox"

Status: RESOLVED WORKSFORME

(Core :: DOM: Service Workers, defect, P3)

Description

[lists]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171030163911

Steps to reproduce:

Open a https:// secured corporate website that registers a service worker  with "/serviceworker.js" (so it's definitionally the same origin) 


Actual results:

the the following error
DOMException
code: 18
columnNumber: 0
data: null
filename: "https://redacted/sw-client.js"
lineNumber: 22
message: "The operation is insecure."
name: "SecurityError"
result: 2152923154
stack: "@https://redcated/sw-client.js:22:5\n"


Expected results:

Service worker should have been registered .

Comment 1

[lists]

html snippet that laods sw-client.js:

 <script type="text/javascript" src="/chips-sw-client.js"></script>



Code from sw-client.js that attempts registration:

var workerJS      = "/serviceworker.js";

if (!('serviceWorker' in navigator)) {
    //
    // Service Workers unsupported.
    //
      console.log("Service worker not supported");
    } else {
     
    navigator.serviceWorker.register(workerJS).then(
             function(registration) {
                //
                // Registration was successful
                //
                console.log('ServiceWorker registration successful with scope: ', registration.scope);
             }).catch(
             function(err) {
                // 
                // Registration failed
                // 
                console.log('ServiceWorker registration failed: ', err);
             });

    navigator.serviceWorker.addEventListener ('message',function (evt) {
                                                             console.log(evt);
                                                             console.log("SWClient: recevied message from serviceWorker");
                                                             processSWMessage(evt);
                                                        });

   }

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Can you please attach about:support text for the effected profile?  In particular, if cookie policy has been modified we often do not permit storage to be used which also now blocks service workers.

Also, is this being opened in an iframe that might have an http:// parent?  That is also considered insecure.

Comment 3

[lists]

Attachment: firefox-about-support.txt

Not in an iframe
Worked in Firefox 56 with same unmodified (by me) profile
Works in Chrome

Comment 4

[lists]

Cookie policy is way to blunt of an instrument for deciding on whether or not a service worker can be registered.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

The about:support shows:

  network.cookie.cookieBehavior: 3

Which means cookies are disabled:

  https://searchfox.org/mozilla-central/source/netwerk/cookie/nsICookieService.idl#89

Which means storage is disabled.

(In reply to lists from comment #4)
> Cookie policy is way to blunt of an instrument for deciding on whether or
> not a service worker can be registered.

Service workers must also be disabled since they implicitly require storage and persistent state to function.  If we allow service workers to function they can effectively implement super cookies.  So leaving it enabled defeats the privacy intention of disabling cookies.

Do service workers function if you restore your cookie settings back to the default?

Comment 6

[lists]

Seems as though it's your cookie policy big-hammer at work.  My cookies were set to allow for session (delete when I close firefox).  Added an exception for the site to "Allow" and it no longer throws this error.   You really need a better way to manage this, than to overload cookie prefs (since cookies are long established old things).  Allowing IndexedDB LocalStorage and ServiceWorkers are really distinct from cookies.

At the very least you really should update the UI in the prefs/options to say Cookies/Local Storage and ServiceWorkers since it's not just for cookies anymore.  You should have updated that when you made the Cookies options control local storage. 

and this is all grouped under history, which is NOT where someone would look to control this.  Pull this out from there and make it a Storage/Workers/Cookies section at the very least.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

(In reply to lists from comment #6)
> Seems as though it's your cookie policy big-hammer at work.  My cookies were
> set to allow for session (delete when I close firefox).

I was under the impression we blocked storage for that case, but we actually allow IDB, etc.  We could open service worker back up in that case.  Of course, no quota manager storage is cleared on browser exit with that setting enabled which is probably unexpected:

https://bugzilla.mozilla.org/show_bug.cgi?id=1400678

I need to ask the storage UX folks what the plan is here.

> You really
> need a better way to manage this, than to overload cookie prefs (since
> cookies are long established old things).  Allowing IndexedDB LocalStorage
> and ServiceWorkers are really distinct from cookies.

Non-technical users do not understand this distinction.  As I understand it our UX is going to align cookies/storage/history since users tend to think of them as the same thing; the browser remembering browser usage.

Let me ask the storage folks what they think...

Mike, what do you think we should do here?  Should we match current IDB/Cache behavior by allowing storage when "keep until I close firefox" is selected, even though we don't actually delete on close?  I blocked service workers in this case in bug 1336364 because I thought it was better to err on the side of privacy protection.

I guess I could make SW conform to other quota manager storage types for now and then we can fix bug 1400678 later.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

Re: The current IDB behavior, it seems like bug 1147821 which implemented IndexedDB's use of StorageAllowedFor (itself implemented in bug 1184973) didn't really discuss the decision beyond this hand-waving comment[1]:
  // the factory callsite records whether the browser is in private browsing.
  // and thus we don't have to respect that setting here. IndexedDB has no
  // concept of session-local storage, and thus ignores it.

That's also effectively codified by the WorkerPrivate logic that only compares against eDeny as well[2]:
  loadInfo.mStorageAllowed = access > nsContentUtils::StorageAccess::eDeny;

I think the most correct behavior in these situations is to treat session storage like private-browsing mode.  The cookie service and LocalStorage both handle this mode by storing all new data as in-memory.  (However, there's also the unusual behavior where LocalStorage will load what already existed on disk as the starting point, instead of a blank slate like you'd experience in private browsing.  Those would add a frightful amount of complexity, and that behavior seems super un-intuitive in the first place.)

1: https://searchfox.org/mozilla-central/rev/aa1343961fca52e8c7dab16788530da31aab7c8d/dom/indexedDB/IDBFactory.cpp#317
2: https://searchfox.org/mozilla-central/rev/aa1343961fca52e8c7dab16788530da31aab7c8d/dom/workers/WorkerPrivate.cpp#4939

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Andrew Sutherland [:asuth] from comment #8)
> I think the most correct behavior in these situations is to treat session
> storage like private-browsing mode.

Well, in that case maybe the current behavior blocking service workers in this mode is correct then.  We explicitly don't allow SW in private browsing mode.

Comment 10

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

(In reply to Ben Kelly [:bkelly] from comment #7)
> (In reply to lists from comment #6)
> > Seems as though it's your cookie policy big-hammer at work.  My cookies were
> > set to allow for session (delete when I close firefox).
> 
> I was under the impression we blocked storage for that case, but we actually
> allow IDB, etc.  We could open service worker back up in that case.  Of
> course, no quota manager storage is cleared on browser exit with that
> setting enabled which is probably unexpected:
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1400678
> 
> I need to ask the storage UX folks what the plan is here.
> 
> > You really
> > need a better way to manage this, than to overload cookie prefs (since
> > cookies are long established old things).  Allowing IndexedDB LocalStorage
> > and ServiceWorkers are really distinct from cookies.
> 
> Non-technical users do not understand this distinction.  As I understand it
> our UX is going to align cookies/storage/history since users tend to think
> of them as the same thing; the browser remembering browser usage.
> 
> Let me ask the storage folks what they think...
> 
> Mike, what do you think we should do here?  Should we match current
> IDB/Cache behavior by allowing storage when "keep until I close firefox" is
> selected, even though we don't actually delete on close?  I blocked service
> workers in this case in bug 1336364 because I thought it was better to err
> on the side of privacy protection.
> 

I am guessing the question should go to Mark Liang (Storage UX), not Mike, who is working on Hasal.
Also NI Jacqueline since she is working on aligning cookies/storage/history.

> I guess I could make SW conform to other quota manager storage types for now
> and then we can fix bug 1400678 later.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Thanks.  Sorry for the NI typo!

Comment 12

[Mark Liang(:mark_liang)]

I agree that general users consider deleting on close equals don't remember anything during my last session. 

Currently in Nightly if users select "clear history when Nightly closes" and go to settings, they can select what should be cleared including "History" and "Data". Right now under the Data section there's only "Site Preferences" and "Offline Website Data". We should probably add more options to make sure user find the storage types they want to clear, or we could simply add a checkbox to clear all site data which could match with user's expectation.

Comment 13

[Jacqueline Savory [:jsavory] UX]

I agree as well with airing on the side of more privacy and that users probably understand this checkbox as not remembering anything from the previous session. 

I don't think that the work I'm doing currently will directly affect this in the short term, but when we do consider improving the History section, the improvements should probably also apply to the Settings menu that Mark mentioned. As well, if we add any extra options, it will need to be consistent with History as well.

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

At the most recent all-hands Storage UX meeting on the Wednesday morning at 8:30 (https://austinyallhands2017.sched.com/event/D5dI/next-steps-on-storage-persistence-and-permissions), I believe we reached consensus that we can treat eSessionOnly as "clear on shutdown" as proposed in bug 1400678 (and contradicting my comment 8 here where I suggested things should be in-memory-only).  So once bug 1400678 is fixed, we can/should indeed "allow service workers when cookie policy is 'keep until I close firefox'".

I'm also adding bug 1183245 (Service worker registration should be wiped when origin storage is wiped) as a dependency because we need the registrations to also be wiped out by the same logic whether at shutdown or startup.

Comment 17

[Jens Stutte [:jstutte]]

:asuth, can you clarify the relation to bug 1429714 (which might not be resolved completely, yet) ?

Comment 18

[skaraman]

Hello, we have found that the issue is still present when "Delete cookies and site data when Firefox is closed" is enabled, we see "This operation is insecure" errors.

Comment 19

[Andy Bajka]

Please change "Delete cookies and site data when Firefox is closed" to "Delete cookies and Service Workers data when Firefox is closed"

Allow service workers while using Firefox even if "Delete cookies and Service Workers data when Firefox is closed" is checked.

When we exit Firefox clear cookies and service workers if "Delete cookies and Service Workers data when Firefox is closed" is checked.

Comment 20

[puneet.sabharwal]

@jstutte is anyone looking into this? this has been open for 3 years

Comment 21

[Glenn Linderman]

Pretty sad that "delete cookies when I close firefox" disables service workers without warning. I certainly didn't expect that, until I noticed the error in the log and searched on the text. I wouldn't expect a setting with that name to disable anything, only to delete cookies when I exit Firefox. I wrote a fine service worker some years ago in test mode, and now that I'm trying to deploy it, I run into this issue. So in retesting, it seems that it works when I set the exception for my deployment site, for a while. And then it vanishes. The service worker is accessing the IDB and allowing off-line access to cached things, and that seemed to work for a while after breaking the internet connection, but then it quit, and the service worker vanished.

I do agree that many users think there is no difference between a cookie, local storage, IDB storage, but rather than coddle their ignorance, the settings should have a set of check boxes regarding what gets deleted when exiting: then they'd learn that something other that cookies exist, and learn how to deal with them. And it certainly shouldn't disable the service workers, without an explicit checkbox for that.

Comment 22

[Andrew Sutherland [:asuth] (he/him)]

The ability to wipe the storage is no longer a concern as it's been abstracted over in Sanitizer.jsm. The primary limiting factor at this time is the UX around clearing data at shutdown, which is unfortunately that Firefox silently can take a very long time depending on the I/O speed of the device the profile is stored on and how much data there is to be stored. This shutdown can impede re-launching Firefox and potentially trip the watchdog timer if the shutdown takes long enough. For sites like slack.com, we expect that site to cache 300 MiB+ of data across many small individual files.

I've filed bug 1756724 on having a data clearing progress dialog at shutdown (which would stop the watchdog timer too).

Note that it's also possible to try and accelerate shutdown by some combination of:

• Trying to store smaller Response bodies inline in the Cache API database as blobs. The theory here would be that the filesystem would have an easier time of deletion since it'd just be deleting a single database with a lot of pages. For spinning disk media, we'd likely want to set the maximum inlined blob size close to the SQLite page size (which could mean raising the page size) or we'd risk fragmentation-based performance regressions compared to the single-shot append-only file writes we currently use.
• Moving to encrypted storage with the privacy model that we forget the key at shutdown but not blocking Firefox shutdown/restart (which could involve handing off actual deletion to a daemon helper).
• Auditing the deletion calls actually being made.

Comment 23

[Master ? [:masterquestionable]]

    [ Quote Glenn Linderman @ CE 2021-12-12 07:11 UTC:
https://bugzilla.mozilla.org/show_bug.cgi?id=1413615#c21
    Pretty sad that "delete cookies when I close firefox" disables service workers without warning. ]
<^>    "Delete browsing data on quit" at the moment (version 115) doesn't seem to cause which.

Comment 24

[abma]

I've hit this bug, too.

Backreference: https://github.com/webcompat/web-bugs/issues/126555

Comment 25

[Kestrel]

@abma: Your test case works since version 103 (Bug 1681495).

Comment 26

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Kestrel from comment #25)

@abma: Your test case works since version 103 (Bug 1681495).

Thanks for checking and testing this. I think there was also relevant work in bug 1764761 that removed StorageAccess::eSessionScoped but the configuration changes in bug 1681495 indeed preceded that and changed ServiceWorkers to no longer see that (unsupported) value.