Closed Bug 1525917 Opened 6 years ago Closed 6 years ago

Webextensions don't respect cookie policy

Categories

(WebExtensions :: General, defect, P1)

65 Branch
defect

Tracking

(firefox68 verified)

VERIFIED FIXED
mozilla68
Tracking Status
firefox68 --- verified

People

(Reporter: u633076, Assigned: rpl)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

Firefox 65.0 64-bit / Windows 7 64-bit

I've just noticed that Webextensions which include calling sites don't respect my general cookie policy.

IF I set network.cookie.cookieBehavior = 2 = Block all cookies :

example 1 : uBlock Origin : i.e. https://secure.fanboy.co.nz/r/fanboy-ultimate.txt
example 2 : Feedbro : i.e. https://fileforum.betanews.com/rss

Actual results:

fanboy.co.nz has set a cookie
betanew.com has set a cookie

Expected results:

no cookie given network.cookie.cookieBehavior = 2

I forgot to mention this:

BUT, if I explicitly make a blocking exception, in above examples,
for https://secure.fanboy.co.nz,
for https://fileforum.betanews.com
THEN there are no cookies from these sites.

In other words Webextensions consider specific site cookie permissions but not the global cookie behavior policy all in network.cookie.cookieBehavior

I'm aware of Webextensions required and accepted permissions upon install but I didn't believe these would prevail on Firefox's default cookie policy. I'm quite surprised. Is this a bug or is it in the Webextensions' logic? Thanks.

Can we get QA verification of this, please?

Flags: needinfo?(kraj)

Cosmin, mind taking a look at this?

Flags: needinfo?(kraj) → needinfo?(cbadescu)

Hello Stangets, we've starting looking over this issue but are unsure of the steps to reproduce, could you please clarify them? I understand you've added the 'fanboy-ultimate.txt' to the uBlock custom filter list and due to the fact that the cookieBehavior has been set to 2, no cookies should be set?
Currently I'm seeing the cookie being added regardless of the cookieBehavior value even without having uBlock installed.

Flags: needinfo?(cbadescu) → needinfo?(vic-72vpxno3p2s4)

Hello Vlad Jiman,you've understood correctly. uBlockO and Feedbro extensions are mentioned not because i'd consider the issue concerns them only but because it is with these two that I've encountered the issue.

You wrote, "Currently I'm seeing the cookie being added regardless of the cookieBehavior value even without having uBlock installed"

Not here. network.cookie.cookieBehavior is set to 2, if I open either from here above, either from the urlbar https://secure.fanboy.co.nz/r/fanboy-ultimate.txt I get NO ccokie; I never get cookies with network.cookie.cookieBehavior set to 2 unless i've provided an accept exception for a site. This is why cookies (for sites sending them of course) appearing with network.cookie.cookieBehavior=2 AND no accept exception WHEN called from Webextensions (at least uBlockO and Feedbro) puzzles me.

This is really puzzling.

Flags: needinfo?(vic-72vpxno3p2s4)

STR

FF65, vanilla profile

  • make sure network.cookie.cookieBehavior = 0 (accept all = default)
  • about:preferences#privacy > check Cookies & Site Data > Manage Data (should be empty)
  • add uBlock Origin (uBO)
  • open the uBO dashboard > filter lists, click update now
  • if about:preferences#privacy is open, close it. Reopen it and check your Manage Data
  • result: easylist.to has set a cookie

FF65, NEW: vanilla profile

  • make sure network.cookie.cookieBehavior = 2 (block all)
  • repeat above steps
  • result: easylist.to has set a cookie
  • expected result: easylist.to should not be allowed to set a cookie

@STR, exactly. And this is not specific to uBO given I've encountered the same issue as mentioned in my first post with the Feedbro webextension.

Another strange phenomenon is that if you explicitly block cookies for easylist.to then no cookies will have been set (as i mentioned as well in my first post). In your scheme, try explicitly blocking cookies for https://easylist.to and cookies willl be blocked... but not if only network.cookie.cookieBehavior set to 2

Stangets and Simon, thanks for the clarifications,

I've reproduced the described issue using 65.0b11 and also the latest Nightly, and it does appear that extensions can add cookies even while having the 'network.cookie.cookieBehavior' set to block all cookies, it will have to be decided if this behavior is intended or not.

The behavior is currently the intended one, it has been introduced in Bug 1406675.

The reason for this behavior is that customizing the cookieBehavior was resulting in broken extension behaviors (in particular by breaking the access to the storage webAPIs, like IndexedDB and localStorage).

The rationale is that cookieBehavior is meant to affect the behavior of the web content, whereas the extension was to be considered more as a part of the browser (and not as part of the regular web content that this preference was meant to affect).

Blocking the cookies on specific domains works because that doesn't impact on the extension ability to use storage-related webAPIs from its own extension pages.

See Also: → 1406675

OK, thanks Luca for this clarification.
I'll keep in mind that cookie behavior corresponds in the facts to storage behavior.

The reasons you relate, once read, appear totally coherent with what we know of Firefox Quantum. Yet the relationship didn't appear obvious to me.

Again, the power of Webextensions, meant to be more secure than legacy add-ons but at the same time far more powerful.

Thanks.

Allowing a first party cookie per UUID so it can utilize localStorage & IDB (& service workers? etc) makes sense. Instead, this is a web extension doing third party calls from behind-the-scenes. Why is the third party being allowed to set cookies? Is there not a distinction here?

"[...]a web extension doing third party calls from behind-the-scenes". Indeed. In fact such cookies are 3rd-party cookies.

I use an extension which has an option I neglected until now given I was relying on Firefox's network.cookie.cookieBehavior set to block all cookies, which it doesn't.

This extension, ForgetmeNot, has an option 'Remove Thirdparty Cookies on creation'
ForgetMeNot defines what it considers as Thirdparty cookies :

"When a cookie is set without belonging to a domain which is open in a tab, it is considered a thirdparty cookie."

I've now checked it and neither 'uBlock Origin' filter updates nor 'Feedbro' RSS updates leave cookies, given they've been removed by 'ForgetmeNot' at creation time.

This leads of course to a problem on sites requiring a 3rd-party cookie, i.e. Youtube calling Google. This cross-site requirement is unhealthy anyway and should be avoided.

This is a rough 'n' tough way of proceeding but an average user such as myself doesn't understand why a browser's setting stating it will block ALL cookies doesn't. It just does not. Or it should be explicitly stated :

network.cookie.cookieBehavior set to 2 blocks all webcontent cookies but not cookies initialized by the browser, in particular by its webextensions. This is far too complicated and means that in order to have one's Firefox settings apply the user's wishes this user must get into a labyrinth of analysis and knowledge. Webextensions are a nightmare, to put it mildly.

Average users rely on basic semantic, not on complications tied to technical nuances they ignore.

Priority: -- → P1

As mentioned in comment 9, the reason why network.cookie.cookieBehavior set to 2 is not blocking cookies on the WebExtensions pages has to be related to the changes introduced in Bug 1406675, in particular the changes applied to dom/base/nsContentUtils.cpp.

We would still like to keep IndexedDB and localStorage working in the WebExtensions pages while cookieBehavior is set to 2, nevertheless the effect it has in the WebExtensions pages for the third party cookies sounds like something we may reasonably want to avoid.

Before deciding how to proceed with it, we would like to hear your opinion about this issue (as well as any ideas and/or pointers you may have).

Flags: needinfo?(ehsan)
Flags: needinfo?(amarchesini)

(In reply to Luca Greco [:rpl] from comment #13)

We would still like to keep IndexedDB and localStorage working in the WebExtensions pages while cookieBehavior is set to 2, nevertheless the effect it has in the WebExtensions pages for the third party cookies sounds like something we may reasonably want to avoid.

Those two statements seem to be contradictory, no? Consider the fact that cookies and other forms of storage are in fact one thing (irrespective of the mechanism used to access the storage): client-side storage. Now, when WebExtension pages are in third-party context, are you suggesting they should or should not have access to client-side storage?

Before bug 1406675 they did not. After bug 1406675 they do now. We can go back to the behaviour before that bug. But a mixture of behaviours across different storage APIs makes the least amount of sense to me...

Flags: needinfo?(ehsan) → needinfo?(lgreco)

I strongly agree with Ehsan here, we've been trying to reduce the instances of applying different policies for storage and cookies in various ways so far.

(In reply to :Ehsan Akhgari from comment #14)

(In reply to Luca Greco [:rpl] from comment #13)

We would still like to keep IndexedDB and localStorage working in the WebExtensions pages while cookieBehavior is set to 2, nevertheless the effect it has in the WebExtensions pages for the third party cookies sounds like something we may reasonably want to avoid.

Those two statements seem to be contradictory, no? Consider the fact that cookies and other forms of storage are in fact one thing (irrespective of the mechanism used to access the storage): client-side storage.

I don't disagree, I understand why all the form of client side storage should be blocked on all websites when that preference is set to BEHAVIOR_REJECT = 2, and that is why we agreed with :asuth on that change as part of Bug 1406675.

In short the reasoning was that:
users that choose to set "Accept cookies from websites" as disabled don't expect the Extensions to break with that settings, because users do not perceive extensions as part of the websites, they consider extensions more similar to a part of the browser (and so there was a mismatch between the "user expectations" and the side-effect it had on the extensions).

(And, as an additional side note, extensions can also control the cookieBehavior preference using the privacy.websites API and by doing so, without the changes introduced by Bug 1406675, they were going to be able to break other extensions, or even the same extension that originated the call to browser.privacy.websites.cookieConfig.set).

Now, when WebExtension pages are in third-party context, are you suggesting they should or should not have access to client-side storage?

Before bug 1406675 they did not. After bug 1406675 they do now.

This is only partially true, the extensions can also use browser.storage.local and browser.storage.sync as other forms of client-side storage, and so technically the extensions were able to use other forms of client-side storage with cookieBehavior set to 2 even without the changes introduced by bug 1406675.

We can go back to the behaviour before that bug. But a mixture of behaviours across different storage APIs makes the least amount of sense to me...

yeah, as I said above I agree, and I'm not sure that there is something that we could do differently that still make sense based on the meaning that the cookieBehavior should have, but I've not yet been able to "convince myself" that this issue can be closed just as "expected behavior" (even if the behavior is actually expected), nor that going back to "before Bug 1406675" is an option.

Nevertheless, based on this issue, it seems to me that there is still a mismatch between the "users expectations" and the actual behavior that the extension pages currently present:

the part that seems to do not match the "users expectations" is that the Extension are still accepting cookies (and then send them back automatically) from third party services that the extension may fetch data from internally, despite any value the user could have set on the cookieBehavior about:config preference.

With the current implementation (with the changes from Bug 1406675) when the principal is an extension principal the cookieBehavior preference is completely ignored and the behavior is always set to nsICookieService::BEHAVIOR_ACCEPT, and what I'm wondering is if "completely ignoring the cookieBehavior" was our only option, or if we could instead still allow cookieBehavior to have an effect on the extension pages on some extent.

As an example, could a strategy similar to the following be something that would still make sense and at the same time better match the "user expectations"?

  • when the current principal is an extension principal
    • leave behavior unchanged if it is nsICookieService::BEHAVIOR_REJECT_FOREIGN
    • set behavior to nsICookieService::BEHAVIOR_REJECT_FOREIGN for the extension principals, if it is
      set to nsICookieService::BEHAVIOR_REJECT for all the websites

(well, at that point I would also ask myself if a behavior set to nsICookieService::BEHAVIOR_LIMIT_FOREIGN or nsICookieService::BEHAVIOR_REJECT_TRACKER should also be leaved unchanged and what would be their meaning for an extension page).

Flags: needinfo?(lgreco) → needinfo?(ehsan)

(In reply to :Luca Greco from comment #16)

"[...]users do not perceive extensions as part of the websites[...]"

I even cannot imagine ever perceiving that. This means that websites have more authority on an extension the user has installed than the user himself. To perceive is the right word because understanding it is the next step and accepting it the next floor.

I won't elaborate, I'm in no way able to. I had simply shared a fact which appeared to me in contradiction with good sense.
I understand Luca Greco tries to conciliate this (perceived) good sense with the requirements of Firefox Quantum's architecture. Maybe as tough as squaring a circle.

His example at the end of his comment is maybe the way to go.

Anyway thanks to you guys, devs and others, for at least evaluating the problematic rather than simply avoiding it. Appreciated.

(In reply to Luca Greco [:rpl] from comment #16)

(In reply to :Ehsan Akhgari from comment #14)

(In reply to Luca Greco [:rpl] from comment #13)

We would still like to keep IndexedDB and localStorage working in the WebExtensions pages while cookieBehavior is set to 2, nevertheless the effect it has in the WebExtensions pages for the third party cookies sounds like something we may reasonably want to avoid.

Those two statements seem to be contradictory, no? Consider the fact that cookies and other forms of storage are in fact one thing (irrespective of the mechanism used to access the storage): client-side storage.

I don't disagree, I understand why all the form of client side storage should be blocked on all websites when that preference is set to BEHAVIOR_REJECT = 2, and that is why we agreed with :asuth on that change as part of Bug 1406675.

In short the reasoning was that:
users that choose to set "Accept cookies from websites" as disabled don't expect the Extensions to break with that settings, because users do not perceive extensions as part of the websites, they consider extensions more similar to a part of the browser (and so there was a mismatch between the "user expectations" and the side-effect it had on the extensions).

(And, as an additional side note, extensions can also control the cookieBehavior preference using the privacy.websites API and by doing so, without the changes introduced by Bug 1406675, they were going to be able to break other extensions, or even the same extension that originated the call to browser.privacy.websites.cookieConfig.set).

All great points.

Now, when WebExtension pages are in third-party context, are you suggesting they should or should not have access to client-side storage?

Before bug 1406675 they did not. After bug 1406675 they do now.

This is only partially true, the extensions can also use browser.storage.local and browser.storage.sync as other forms of client-side storage, and so technically the extensions were able to use other forms of client-side storage with cookieBehavior set to 2 even without the changes introduced by bug 1406675.

Yes, true. When I was talking about before, I meant web-accessible client-side storage. Of course extension-accessible storage may have privacy implications as well, but not strictly more so than the implications of running an extension in the first place IMO.

We can go back to the behaviour before that bug. But a mixture of behaviours across different storage APIs makes the least amount of sense to me...

yeah, as I said above I agree, and I'm not sure that there is something that we could do differently that still make sense based on the meaning that the cookieBehavior should have, but I've not yet been able to "convince myself" that this issue can be closed just as "expected behavior" (even if the behavior is actually expected), nor that going back to "before Bug 1406675" is an option.

Nevertheless, based on this issue, it seems to me that there is still a mismatch between the "users expectations" and the actual behavior that the extension pages currently present:

the part that seems to do not match the "users expectations" is that the Extension are still accepting cookies (and then send them back automatically) from third party services that the extension may fetch data from internally, despite any value the user could have set on the cookieBehavior about:config preference.

I'd posit that the real issue here is that the original assumption that users expect that extension-embedded websites are "part of the application not a website" is probably wrong.

In fact, maybe the fix to bug 1406675 was just way too broad? What we really wanted there was to make IndexedDB work on moz-extension:// pages, right? But the code that landed there is allowing much more than that to happen. Per comment 0 here we are failing to check properly what cookie policy to apply for example when such pages load a normal HTTP resource (presumably through an API like XHR). Why can't we detect such cases and fall back to the normal cookie policy?

With the current implementation (with the changes from Bug 1406675) when the principal is an extension principal the cookieBehavior preference is completely ignored and the behavior is always set to nsICookieService::BEHAVIOR_ACCEPT, and what I'm wondering is if "completely ignoring the cookieBehavior" was our only option, or if we could instead still allow cookieBehavior to have an effect on the extension pages on some extent.

No, that is certainly not our only option. That in fact should be the last resort option.

As an example, could a strategy similar to the following be something that would still make sense and at the same time better match the "user expectations"?

  • when the current principal is an extension principal
    • leave behavior unchanged if it is nsICookieService::BEHAVIOR_REJECT_FOREIGN
    • set behavior to nsICookieService::BEHAVIOR_REJECT_FOREIGN for the extension principals, if it is
      set to nsICookieService::BEHAVIOR_REJECT for all the websites

AFAIK we have issues with detecting when content is first-party vs third-party with the presence of moz-extension:// frames (for example I think we treat an http iframe inside a moz-extension:// iframe as third-party whereas we should really treat it as first-party). So this would probably cause a number of problems because of those issues. But more importantly, this would still be enforcing a different cookie policy than what the user wants. If the user really doesn't want any cookie and site data, maybe there is a reason for that. Why not just honour that for web frames?

(well, at that point I would also ask myself if a behavior set to nsICookieService::BEHAVIOR_LIMIT_FOREIGN or nsICookieService::BEHAVIOR_REJECT_TRACKER should also be leaved unchanged and what would be their meaning for an extension page).

I'd much rather first focus on treating all cookie policies as they are unmodified and if there is a reason why we can't do that then fall back to other solutions...

Flags: needinfo?(ehsan)

I have a question. Two fold. First, where is this cookie being stored (it is not in cookies.sqlite)? Secondly, are origin attributes being applied.

STR: FF65, vanilla profile

  • make sure network.cookie.cookieBehavior = 2 (block all)
  • about:preferences#privacy > check Cookies & Site Data > Manage Data (should be empty)
  • add uBlock Origin (uBO)
  • open the uBO dashboard > filter lists, click update now
  • if about:preferences#privacy is open, close it. Reopen it and check your Manage Data
  • result: easylist.to and gitcdn.xyz have set a cookie each
  • open cookies.sqlite (firefox is still open) - there are no easylist.to or gitcdn.xyz cookies

If cookies (and related 3rd party stroage) are already isolated, then I don't see an issue with privacy here. TIA

Depends on D22347

Assignee: nobody → lgreco
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(amarchesini)
Component: Untriaged → General
Blocks: 1537753
Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/c3bddd2b3f0a
Do not override cookieBehavior to accept for an extension top level principal. r=Ehsan,baku
https://hg.mozilla.org/integration/autoland/rev/83024ed7327b
Add test for background page request cookies on cookieBehaviors. r=Ehsan,mixedpuppy
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Fixed with FF68 as target : great!

These changes have been landed along with some new test cases which runs in automation, but let's also QA verify it using the STR described in comment 6.

Besides the verify this issue using the above STR, given the kind of changes there still a chance that the new behavior may have introduced some regression related to scenarios that may be missing from the existing test cases, and so I think we should also verify explicitly there is not regressions related to the behavior expected from the changes previously applied from Bug 1509112 and Bug 1502045.

Flags: qe-verify?

Verified the fix using windows 10 x64 on Nightly Fx68 (20190410100020) and it looks like the extensions can no longer override network.cookie.cookieBehavior once it's set to 2. Additional testing is in progress to check for possible regressions as requested.

Status: RESOLVED → VERIFIED
Flags: qe-verify?
Depends on: 1576944
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: