1511303 - Internal storage checks result in STATE_COOKIES_LOADED entries in the content blocking log

Status: VERIFIED FIXED

(Firefox :: Protections UI, defect, P1)

Description

[Johann Hofmann [:johannh]]

When visiting sites with iframes containing bare-bone sites that have clearly no intention of using any kind of storage, cookies, service workers, appcache, or parent-frame interaction, we still do a storage access check for the iframe origin, resulting in a STATE_COOKIES_LOADED entry in the content blocking log. This is an example stack I recorded in lldb:

* frame #0: 0x0000000101a16df0 XUL`nsGlobalWindowOuter::NotifyContentBlockingState(this=0x000000011fd2e400, aState=32768, aChannel=0x000000011ffa0000, aBlocked=false, aURIHint=0x000000011ff78a00) at nsGlobalWindowOuter.cpp:5404 [opt]
   frame #1: 0x00000001053bf2b2 XUL`mozilla::AntiTrackingCommon::NotifyBlockingDecision(aChannel=0x000000011ffa0000, aDecision=eAllow, aRejectedReason=0) at AntiTrackingCommon.cpp:1459 [opt]
   frame #2: 0x00000001019a5ca5 XUL`nsContentUtils::StorageDisabledByAntiTracking(aWindow=<unavailable>, aChannel=0x000000011ffa0000, aPrincipal=<unavailable>, aURI=<unavailable>) at nsContentUtils.cpp:9120 [opt]
   frame #3: 0x00000001019a4dd6 XUL`nsContentUtils::InternalStorageAllowedForPrincipal(aPrincipal=0x0000000146c89dc0, aWindow=<unavailable>, aURI=0x0000000000000000, aChannel=0x000000011ffa0000) at nsContentUtils.cpp:9214 [opt]
   frame #4: 0x00000001019a5294 XUL`nsContentUtils::StorageAllowedForChannel(aChannel=0x000000011ffa0000) at nsContentUtils.cpp:8901 [opt]
   frame #5: 0x0000000103bfa39d XUL`mozilla::dom::ServiceWorkerInterceptController::ShouldPrepareForIntercept(this=<unavailable>, aURI=0x000000011ff78a00, aChannel=0x000000011ffa0000, aShouldIntercept=0x00007ffeefbfb2bf) at ServiceWorkerInterceptController.cpp:41 [opt]
   frame #6: 0x0000000100a1d986 XUL`mozilla::net::HttpBaseChannel::ShouldIntercept(this=<unavailable>, aURI=<unavailable>) at HttpBaseChannel.cpp:3291 [opt]
   frame #7: 0x0000000100a3c34b XUL`mozilla::net::HttpChannelChild::ShouldInterceptURI(this=<unavailable>, aURI=<unavailable>, aShouldUpgrade=<unavailable>) at HttpChannelChild.cpp:4018 [opt]
   frame #8: 0x0000000100a3b2d5 XUL`mozilla::net::HttpChannelChild::AsyncOpen(this=0x000000011ffa0000, listener=0x000000011ffa0000, aContext=<unavailable>) at HttpChannelChild.cpp:2677 [opt]
   frame #9: 0x0000000100a3ae35 XUL`mozilla::net::HttpChannelChild::AsyncOpen2(this=0x000000011ffa0000, aListener=<unavailable>) at HttpChannelChild.cpp:2732 [opt]

I'm quite sure there are more of these instances, maybe AppCache or Quota Manager.

The problem with that is of course that developers will not understand the criteria by which we flag an iframe as having cookies and users might perceive a higher "threat" from third party cookies than was actually present.

We should try to reduce the number of these false positives as much as possible and document the remaining edge cases.

Comment 1

[Johann Hofmann [:johannh]]

Note that the new service worker architecture seems to get rid of at least the above mentioned storage check, which means that browser_trackingUI_state.js will need to be fixed up after that merges to reflect that the cookies section is sometimes hidden.

Comment 2

[(no longer active)]

The easiest way to reproduce this is on data:text/html,<iframe src="https://example.com"></iframe>.

The content blocking log will be:

await gBrowser.selectedBrowser.getContentBlockingLog()
"{
 \"https://example.com\": [[32768, true, 1]]
}
"

Comment 3

[(no longer active)]

Attachment: Bug 1511303 - Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80bcfbcc44ea
Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre r=baku

Comment 5

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 80bcfbcc44ea (Bug 1511303) for bc failures on browser_alltabslistener.js.

Backout: https://hg.mozilla.org/integration/autoland/rev/14de868f0332ccec8e8cfc65a2d8a69d223169e0

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=80bcfbcc44eab54f75a411450a0611bca94dea9f&selectedJob=217706132

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=217706132&repo=autoland&lineNumber=1561

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3956dbb7e65d
Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre; r=baku

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 3956dbb7e65d (Bug 1511303) for browser_ignore_same_page_navigation.js failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=browser-chrome&selectedJob=217785997&revision=3956dbb7e65d72c4f69afa3ab4638257b1ae9b21

Backout link: https://hg.mozilla.org/integration/autoland/rev/a988e9edb0cf017fa9bc383c5242f940c45de7fd

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=217785996&repo=autoland&lineNumber=2195

22:42:30     INFO - TEST-START | browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js
22:42:30     INFO - TEST-INFO | started process screenshot
22:42:30     INFO - TEST-INFO | screenshot: exit 0
22:42:30     INFO - Buffered messages logged at 22:42:30
22:42:30     INFO - Entering test bound 
22:42:30     INFO - TEST-PASS | browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js | should have 1 onLocationChange event - 
22:42:30     INFO - Buffered messages finished
22:42:30     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js | should have 2 onSecurityChange event - Got 1, expected 2
22:42:30     INFO - Stack trace:
22:42:30     INFO - chrome://mochikit/content/browser-test.js:test_is:1312
22:42:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js:null:34
22:42:30     INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:111
22:42:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js:null:10
22:42:30     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1102
22:42:30     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1093
22:42:30     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:991
22:42:30     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
22:42:30     INFO - TEST-PASS | browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js | should have 2 onLocationChange events - 
22:42:30     INFO - Not taking screenshot here: see the one that was previously logged
22:42:30     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js | should still have only 2 onSecurityChange event - Got 1, expected 2
22:42:30     INFO - Stack trace:
22:42:30     INFO - chrome://mochikit/content/browser-test.js:test_is:1312
22:42:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js:null:39
22:42:30     INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:111
22:42:30     INFO - chrome://mochitests/content/browser/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js:null:10
22:42:30     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1102
22:42:30     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1093
22:42:30     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:991
22:42:30     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
22:42:30     INFO - Leaving test bound

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/556920bb426c
Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre; r=baku

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/556920bb426c

Comment 10

[Rares Doghi, Desktop QA]

Hi, I could reproduce this issue in older builds but I can no longer reproduce it in the latest Nightly : https://tools.taskcluster.net/index/gecko.v2.mozilla-central.latest.firefox/win64-opt, when I reach data:text/html,<iframe src="https://example.com"></iframe> there are no cookies detected in the cookies sub-panel where in the older builds a third party cookies was displayed. I will mark this issue accordingly.

Comment 11

[(no longer active)]

Comment on attachment 9031954 [details]
Bug 1511303 - Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1514070

User impact if declined: (not exactly a regression from bug 1514070 but that bug made this information visible in the first place.)

See comment 0

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch is quite low risk, it's changing the order in which we call some functions to make sure the result of our storage checks isn't made visible in the UI if we can't use a service worker for embedded iframes.

String changes made/needed: None

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

So, the regression is exposed by the patch in bug 1514070 which you also want to uplift to beta - Is that correct?

Comment 13

[(no longer active)]

Oops, no, my apologies.  I pasted the wrong bug number in comment 11.  I meant to paste bug 1501992 which has already landed on 65.  That bug is the bug which added the cookies sub-panel to the control centre initially...

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9031954 [details]
Bug 1511303 - Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre

[Triage Comment]
Needed for the Control Center UI changes shipping in 65. Approved for 65.0b6.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fef8ee3bdc39

Comment 16

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx 65.0a1 (2018-11-30), on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 65.0b6, on Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13.6.