1830070 - about:blank doesn't properly resist fingerprinting.

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, defect)

Description

[matc]

Steps to reproduce:

With privacy.resistFingerprinting=true:

onclick = () => {
  let popup = window.open("about:blank", "");
  setTimeout(() => {
    console.log("window", window.navigator.hardwareConcurrency);
    console.log("popup", popup.navigator.hardwareConcurrency);
    popup.close();
  }, 1000);
};

This is not limited to hardwareConcurrency. This affects esr102, tor-browser and mozilla-central. Although esr102 and tor-browser still use nsContentUtils::ResistFingerprinting(aCallerType) in some cases, which are not affected.

Actual results:

Console output:

window 2
popup 4

Expected results:

Console output:

window 2
popup 2

Comment 1

[:Gijs (he/him)]

Well, this is disconcerting.

AFAICT this is because https://searchfox.org/mozilla-central/rev/8329a650e3b4f866176ae54016702eb35fb8b0d6/dom/base/nsContentUtils.cpp#2326,2346 makes decisions based on the final channel URI (passed here), and safelists all of about. This presumably also includes about:srcdoc and therefore srcdoc iframes. :-\

Comment 2

[Tom Ritter [:tjr]]

While Bug 1811505 is public, it's more speculative, so I'm not duping at the moment. I think the patch in Bug 1805101 should resolve this; but it will need testing and we're not really done figuring what the best solution is.

But regardless of 1805101 does, I think a reasonable patch for ESR102 is to just remove the about: exemption - it's not hit in all codepaths (per the original reporter) so it's inconsistent and system privileged about pages shouldn't be affected.

Comment 3

[Frederik Braun [:freddy]]

(In reply to :Gijs (he/him) from comment #1)

Well, this is disconcerting.

AFAICT this is because https://searchfox.org/mozilla-central/rev/8329a650e3b4f866176ae54016702eb35fb8b0d6/dom/base/nsContentUtils.cpp#2326,2346 makes decisions based on the final channel URI (passed here)

Maybe FPP shouldn't make its decision for a Channel, Document or a URL but based on the BrowsingContextGroup (same-site)?
Normally, I would suggest the Principal (same-origin), but we've been bitten by that for exactly these edge cases (about:blank in firame/popup, about:srcdoc and these 3 combined with sandbox (through CSP or iframe sandbox), where you end up getting a NullPrincipal (opaque origin), that looks disassociated but can easily collude.

We should also note that a "new origin" is free of charge and fully within the attacker's capabilities :|

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Tom Ritter [:tjr] from comment #2)

But regardless of 1805101 does, I think a reasonable patch for ESR102 is to just remove the about: exemption - it's not hit in all codepaths (per the original reporter) so it's inconsistent and system privileged about pages shouldn't be affected.

What if we factor the about case within ShouldResistFingerprinting_dangerous and do a special case handling using the about module flags (aboutModule->GetURIFlags()) and then special case using URI_SAFE_FOR_UNTRUSTED_CONTENT which then allows us to not return false for all about pages.

Is that a viable option?

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(In reply to Frederik Braun [:freddy] from comment #3)

Maybe FPP shouldn't make its decision for a Channel, Document or a URL but based on the BrowsingContextGroup (same-site)?

I don't know much about the fingerprinting protection code, but if it needs to be origin-specific it could potentially use something like DocGroup I suppose. This is sort-of the smallest unit of communicating pages, but wouldn't cover the NullPrincipal/sandboxing case you mention below.

Normally, I would suggest the Principal (same-origin), but we've been bitten by that for exactly these edge cases (about:blank in firame/popup, about:srcdoc and these 3 combined with sandbox (through CSP or iframe sandbox), where you end up getting a NullPrincipal (opaque origin), that looks disassociated but can easily collude.

FWIW we do do a better job of crediting NullPrincipal origins nowadays thanks to PrecursorPrincipal (we use this for sandboxing null principal pages with Fission). I didn't exactly design it to be used for proper security purposes (just best effort things like process sandboxing), but perhaps RFP is in a similar boat?

Comment 6

[Tom Ritter [:tjr]]

So for RFP we need to change behavior based on the following conditions:

1. Is it the System Principal asking for something?
2. Are you PBM?
3. Is this a WebExtension asking for something? (Not counting content scripts, I believe those run in the context of the page and if they get spoofed data, that's fine. It's possible for the content script to communicate with a non-content script if it needs real data
4. Is it a non-content about: page? This would include privileged about pages like about:preferences, but also unprivileged ones like about:rights. It would not include ones that content can act on or interact with like about:srcdoc or about:blank
5. Is the site exempted via the ETP toggle? This exemption propogates down the iframe stack - if the top level document is exempted in this way, all iframes are exempted.
6. Is the site exempted via 'the other' mechanism? Right now 'the other' mechanism is a list of domains in a pref, in the future it will be something nicer. This exemption does not propagate down the iframe stack - if the top level document is exempted this way, an iframe is only exempted if its domain is also exempted. And if the top level document is not exempted this way, the iframe is not exempted even if its domain is on the list.

To accomplish (6) we need to look at CookieJarSettings. (5) is done via ContentBlockingAllowList.h, all of which kind of cascade into checking the top-level document's principal and the pbm bit.

I am not sure about URI_SAFE_FOR_UNTRUSTED_CONTENT - it looks like pages without URI_SAFE_FOR_UNTRUSTED_CONTENT are System Principal pages. But pages with URI_SAFE_FOR_UNTRUSTED_CONTENT we still want to exempt RFP on, for example about:welcome, about:pocket-home, and about:reader*.

* I have an email out to verify that about:reader is safe to disable RFP on.

I am trying to find a way to reliably enforce RFP on about:blank when appropriate in Bug 1805101 - that attempt at the moment is focusing on either finding a reference from the about:black document up the document chain so I can accurately query RFP on the parent (this is in the comments on the phab revision and I'm trying to test it) or pushing the correct RFP bit down into the about:blank document at creation (which is what the current patch does.)

I'll need to look at BrowserContextGroup a bit more to figure out if replacing the Channel/Principal APIs is accurate for it. emilio recently refactored the RFP behavior on documents to cache the bit and a lot of stuff uses that to be both fast and accurate, so I'm not sure how that would behave in a switch to BrowserContextGroup also.

Comment 7

[Tom Ritter [:tjr]]

While I hoped Bug 1805101 would fix this, it did not, because the popup does not have a parent document. It is an about:blank document with a ContentPrincipal. It would be easy to only exempt Null-Principaled about: documents, but I am not certain you can't create a popup with a null principal... Maybe with a blob. I think this calls for another suite of tests unfortunately.

Comment 8

[Frederik Braun [:freddy]]

If the resource loaded in a popup has a Content-Security-Policy header with the sandbox directive, then it would result in a NullPrincipal, but that wouldn't be an about:blank scheme. I think the page could redirect itself to javascript: or such, which should make it about:blank and keep inheriting the sandbox flag. Maybe that helps creating a test case.

There is also a proposal to add anonymous blobs in the WHATWG https://github.com/w3c/FileAPI/issues/192#issuecomment-1471016979, but that's early stages

Comment 9

[:Gijs (he/him)]

I think this check should use the document principal and do the same checks as the base principal's IsL10nAllowed - https://searchfox.org/mozilla-central/rev/f60cf6bfa8bd096efd9bb3a445364f5a0f32897a/caps/BasePrincipal.cpp#769

That code allows:

• network error pages
• system principal
• pages not loadable by web content (this includes all the about: pages we care about)
• chrome/resource pages
• (privileged) add-ons

Judging by the code there, I _ think_ this means a straightforward fix to the code I linked in my comment would be to switch from the protocol allowlist to using NS_URIChainHasFlags(uri, nsIProtocolHandler::URI_DANGEROUS_TO_LOAD, &hasFlags); and an analogous check for URI_IS_UI_RESOURCE. That should be a relatively small and safe patch and I believe should fix the cases we care about here.

Comment 10

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for an RFP test for a popup r?timhuang

I was able to adapt the simple iframe test to a popup
test with pretty minimal adaptations, so I did that.

Remains to be seen if I can do it for the others.

Depends on D177830

Comment 11

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for popup that is a data: uri r?timhuang

Depends on D178684

Comment 12

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for a popup opening a blob url r?timhuang

Depends on D178685

Comment 13

[Tom Ritter [:tjr]]

(In reply to :Gijs (he/him) from comment #9)

Judging by the code there, I _ think_ this means a straightforward fix to the code I linked in my comment would be to switch from the protocol allowlist to using NS_URIChainHasFlags(uri, nsIProtocolHandler::URI_DANGEROUS_TO_LOAD, &hasFlags); and an analogous check for URI_IS_UI_RESOURCE. That should be a relatively small and safe patch and I believe should fix the cases we care about here.

I tested this, and found that it did solve the issue where about: documents were being exempted when they should not be. It actually works a little too well, because it also applies RFP to these documents even when they should be exempted. For instance if I revert this patch and get rid of the parent document check, it causes many of the tests added to that bug to fail - documents are not being exempted when they should be. Keeping the mParentDocument deferral code though, those tests pass.

However the 'exempted-domain.com opens an about:blank popup' scenario still fails because it should be exempted, but it not: is an about:blank document with no mParentDocument - there is nothing to let it know it should be exempted.

Comment 14

[:Gijs (he/him)]

(In reply to Tom Ritter [:tjr] from comment #13)

However the 'exempted-domain.com opens an about:blank popup' scenario still fails because it should be exempted, but it not: is an about:blank document with no mParentDocument - there is nothing to let it know it should be exempted.

Ah, I think this is because we're using the document URI straight up. I would expect the principal for the about:blank document to be a codebase principal for exempted-domain.com. If we use that URI then it should work. This would still not propagate for things opened from sandboxed pages (because they'll get a null principal), you'd want the precursorPrincipal off the channel/loadInfo that Nika mentioned for that.

Comment 15

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for popup that is an about:blank document r?timhuang

Depends on D178686

Comment 16

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Correctly apply RFP Checks to about: documents and deal with pop-ups r?smaug

This patch has three parts to it:

1. Use URI_DANGEROUS_TO_LOAD and URI_IS_UI_RESOURCE to ensure that
   only safe about: documents get exempted. Confusingly, some of
   the safe ones to exempt are the ones with the DANGEROUS flag.

   With this change, we will no longer allow about:blank or
   about:srcdoc to be exempted base on URI. If they are to be
   exempted, it will need to be base on other information.

2. In Document::RecomputeResistFingerprinting we previously
   deferred to a Parent Document if we had one, and either the
   principals matched or we were a null principal.

   We will do the same thing, except we will also defer to our
   opener as well as the parent document. Now about:blank
   documents can be exempted.

   However, this deferral only works if the opener is
   same-process. For cross-process openers, we make the decision
   ourselves.

We can make the wrong decision though. CookieJarSettings is
inherited through iframes but it is not inherited through popups.
(Yet. There's some discussion there, but it's not implemented.)

Conceptually; however, we do want CJS to inherit, and we do want
RFP to inherit as well. Because a popup can collude with its
opener to bypass RFP and Storage restrictions, we should propagate
the CJS information.

This does lead to an unusual situation: if you have exempted
b.com, and a.com (which is not exempted) creates a popup for b.com
then that popup will not be exempted. But an open tab for b.com
would be. And it might be hard to tell those two apart, or why
they behave differently.

The third part of the patch:

3. In LoadInfo we want to populate information down from the
   opener to the popup. This is needed because otherwise a
   cross-origin popup will not defer to its opener (because in
   Fission they're in different processes) and will decide if
   it should be exempted itself. It's the CookieJarSettings
   object that prevents the cross-origin document from thinking
   it should be exempted - CJS tells it 'No, you're a child
   (either a subdocument or a popup) and if I say you don't get
   an exemption, you don't.'

Finally, there is one more caveat: we can only defer to a parent
document or opener if it still exists. A popup may outlive its
opener. If that happens, and something induces a call to
RecomputeResistFingerprinting, then (e.g.) an about:blank popup
may lose an RFP exemption that it had received from its parent.
This isn't expected to happen in practice -
RecomputeResistFingerprinting is only called on document creation
and pref changes I believe.

It is not possible for a popup to gain an exemption though,
because even if the parent document is gone, the CJS lives on and
restricts it.

Depends on D178865

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

It would be easy to only exempt Null-Principaled about: documents, but I am not certain you can't create a popup with a null principal

sandboxed iframe, http://mozilla.pettay.fi/moztests/sandbox_aboutblank_popup.html

Edit: I guess one of the tests is doing that.

Comment 18

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for opening a popup with noopener r?timhuang

Depends on D178866

Comment 19

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for opening a data URI in a popup with noopener r?timhuang

Depends on D178977

Comment 20

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Add a test for opening a blob URI in a popup with noopener r?timhuang

Depends on D179105

Comment 21

[Tom Ritter [:tjr]]

[Tracking Requested - why for this release]: 115 is ESR and this patch stack is important for Tor Browser's fingerprinting resistance.

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed but backed out for URI related build bustage:
https://hg.mozilla.org/integration/autoland/rev/1e9ddd307894fd2055ca49dcaa461478d0acea9e

Push with failures
Failure log

dom/base/nsContentUtils.cpp:2474:29: error: Principal->GetURI is deprecated and will be removed soon. Please consider using the new helper functions of nsIPrincipal

Comment 23

[Tom Ritter [:tjr]]

:(

I am checking URI_DANGEROUS_TO_LOAD and URI_IS_UI_RESOURCE (from IsL10nAllowed) on a principal. I am not using IsL10nAllowed directly, because I need to do the same check on a bare URI, so I extracted the logic into my own function URISaysShouldNotResistFingerprinting to be certain they matched and stayed in sync. But to do this, I call Principal->GetURI (which is what IsL10nAllowed calls also).

I now see that IsL10nAllowed is main-thread only, because of a comment URI_DANGEROUS_TO_LOAD is not threadsafe to query. That was added in Bug 1443925 (Make nsIPrincipal objects threadsafe) but the reason isn't elaborated upon.

I'm not sure what the path forward here is - I am not keen on adding a thread safety requirement to the ShouldRFP functions, I don't believe they have any right now. Christoph, do you have any background on this flag, its thread safety, or any suggestions here?

Comment 24

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Tom Ritter [:tjr] from comment #23)

:(

I am checking URI_DANGEROUS_TO_LOAD and URI_IS_UI_RESOURCE (from IsL10nAllowed) on a principal. I am not using IsL10nAllowed directly, because I need to do the same check on a bare URI, so I extracted the logic into my own function URISaysShouldNotResistFingerprinting to be certain they matched and stayed in sync. But to do this, I call Principal->GetURI (which is what IsL10nAllowed calls also).

I now see that IsL10nAllowed is main-thread only, because of a comment URI_DANGEROUS_TO_LOAD is not threadsafe to query. That was added in Bug 1443925 (Make nsIPrincipal objects threadsafe) but the reason isn't elaborated upon.

I think it's best to loop in Nika. Nika can you help here? Do you have any suggestions for Tom?

Comment 25

[Nika Layzell [:nika] (ni? for response)]

(In reply to Tom Ritter [:tjr] from comment #23)

I am checking URI_DANGEROUS_TO_LOAD and URI_IS_UI_RESOURCE (from IsL10nAllowed) on a principal. I am not using IsL10nAllowed directly, because I need to do the same check on a bare URI, so I extracted the logic into my own function URISaysShouldNotResistFingerprinting to be certain they matched and stayed in sync. But to do this, I call Principal->GetURI (which is what IsL10nAllowed calls also).

I now see that IsL10nAllowed is main-thread only, because of a comment URI_DANGEROUS_TO_LOAD is not threadsafe to query. That was added in Bug 1443925 (Make nsIPrincipal objects threadsafe) but the reason isn't elaborated upon.

Yup, URI_DANGEROUS_TO_LOAD is not threadsafe, nor are a few other flags specified in nsIProtocolHandler::DYNAMIC_URI_FLAGS (https://searchfox.org/mozilla-central/rev/b91e9bef5a6d6f7b549fbc9cba70cb4665ed3866/netwerk/base/nsIProtocolHandler.idl#299-310). This limitation is because unlike the other flags (which can be fully determined from the scheme, and don't depend on the specific URL), these flags require calling nsIProtocolHandlerWithDynamicFlags::GetFlagsForURI to determine.

nsIProtocolHandler implementations are unfortunately still non-threadsafe, and we can't even make just the dyanmic ones threadsafe, as the nsAboutProtocolHandler needs to access about modules (which can be, and are implemented in JS) to determine whether URI_DANGEROUS_TO_LOAD is specified.

Changing this would require reworking nsIAboutModule and its flags in a similar way to how I reworked nsIProtocolHandler and its flags in bug 1443925, which I think is probably out of scope for this bug.

I'm not sure what the path forward here is - I am not keen on adding a thread safety requirement to the ShouldRFP functions, I don't believe they have any right now. Christoph, do you have any background on this flag, its thread safety, or any suggestions here?

Querying URI_DANGEROUS_TO_LOAD from off-main-thread is not something which is OK for us to do right now, as you noted. Fortunately for us, the case which you're interested in here is specifically around which about: URIs are (potentially) content-accessible, which is a limited and known set.

In bug 1820280, :emilio added NS_IsContentAccessibleAboutURI which is used to as part of a function to decide whether chrome CSS rules should be enabled for a URI. I imagine that a similar strategy could be used in your patch.

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed but backed out for build bustage in /dom/base/nsContentUtils.cpp:2475:
https://hg.mozilla.org/integration/autoland/rev/74cc38ff4acbbc7fc7d2f0e8326f6c8a1671d5cc

Push with failures
Failure log

dom/base/nsContentUtils.cpp:2475:5: error: use of undeclared identifier 'rv'

Comment 27

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Landed & backed out for causing build bustages in nsContentUtils.cpp:
https://hg.mozilla.org/integration/autoland/rev/c8f1d9f2dbf28345000613f309fdf7de281beb74

Push with failures
Failure log

[task 2023-06-19T18:17:19.984Z] 18:17:19    ERROR -  /builds/worker/checkouts/gecko/dom/base/nsContentUtils.cpp:2215:25: error: Principal->GetURI is deprecated and will be removed soon. Please consider using the new helper functions of nsIPrincipal
[task 2023-06-19T18:17:19.984Z] 18:17:19     INFO -    Unused << aPrincipal->GetURI(getter_AddRefs(prinURI));
[task 2023-06-19T18:17:19.984Z] 18:17:19     INFO -                          ^

Comment 28

[Tom Ritter [:tjr]]

Sorry, hopefully resolved all the issues...

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for causing assertion failures in nsAboutProtocolUtils.h:
https://hg.mozilla.org/integration/autoland/rev/7cebe3215c7d9c9fac055234e967913b672aa4cf

Push with failures
Failure log

Assertion failure: aURI->SchemeIs("about") (Should be used only on about: URIs), at /builds/worker/workspace/obj-build/dist/include/nsAboutProtocolUtils.h:33

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

There is also a mochitest failure
Log

TEST-UNEXPECTED-FAIL | browser/components/resistfingerprinting/test/browser/browser_hwconcurrency_popups_aboutblank.js | Uncaught exception in test bound bound testA - at https://example.com/browser/browser/components/resistfingerprinting/test/browser/file_hwconcurrency_aboutblank_popupmaker.html:24 - TypeError: can't access property "postMessage", popup is undefined

Comment 31

[Tom Ritter [:tjr]]

Okay I had to do some futzing to get it to pass on all the platforms - there's some timing problems, and it might still cause intermittents - we'll see how bad they are and how much more I should futz. But I did it to go green across the board.

Comment 32

[Atila Butkovits]

Backed out for causing failures at browser_hwconcurrency_popups_blob_noopener.js.

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=62f439f5b2adbf7a3a542daba70890da14876c8b&selectedTaskRun=Ec2_jdc1TnCDcs_d4pqnug.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=420693762&repo=autoland&lineNumber=6785

Comment 33

[Tom Ritter [:tjr]]

Okay, I reproduced the failures 5/10 times, then patched and got 10/10 clean. Here's hoping a new test doesn't intermittent...

Comment 34

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Add a test for an RFP test for a popup r=timhuang
https://hg.mozilla.org/integration/autoland/rev/faf05868d7f3b412434678d1c6ab71326a05d392
https://hg.mozilla.org/mozilla-central/rev/faf05868d7f3

Add a test for popup that is a data: uri r=timhuang
https://hg.mozilla.org/integration/autoland/rev/dae761eb8d25e188fe25f3597a7df0d49caf34ef
https://hg.mozilla.org/mozilla-central/rev/dae761eb8d25

Add a test for a popup opening a blob url r=timhuang
https://hg.mozilla.org/integration/autoland/rev/d340c184d82d93649b93f18e38ab987aab0d7f49
https://hg.mozilla.org/mozilla-central/rev/d340c184d82d

Add a test for popup that is an about:blank document r=timhuang
https://hg.mozilla.org/integration/autoland/rev/0a3205f0bac88c877c4970a97690bfe7e47d7353
https://hg.mozilla.org/mozilla-central/rev/0a3205f0bac8

Correctly apply RFP Checks to about: documents and deal with pop-ups r=smaug,necko-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/b8a4944534c0103f90e7ac27584f08ef1c4674ec
https://hg.mozilla.org/mozilla-central/rev/b8a4944534c0

Add a test for opening a popup with noopener r=timhuang
https://hg.mozilla.org/integration/autoland/rev/072143bbd8f813153904461a42565b16c3d52d61
https://hg.mozilla.org/mozilla-central/rev/072143bbd8f8

Add a test for opening a data URI in a popup with noopener r=timhuang
https://hg.mozilla.org/integration/autoland/rev/5b186a06e9ee962df16dbef901f1ae73534b0e1e
https://hg.mozilla.org/mozilla-central/rev/5b186a06e9ee

Add a test for opening a blob URI in a popup with noopener r=timhuang
https://hg.mozilla.org/integration/autoland/rev/f66f672d3775391b28778f07e0469f736fa04c6d
https://hg.mozilla.org/mozilla-central/rev/f66f672d3775

apply code formatting via Lando
https://hg.mozilla.org/integration/autoland/rev/7da886075f6af031c35eff22de24b44bb826f859
https://hg.mozilla.org/mozilla-central/rev/7da886075f6a

Comment 35

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:tjr, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.

Comment 36

[Tom Ritter [:tjr]]

I hope we can uplift this to ESR next cycle for Tor.

Comment 37

[Tom Ritter [:tjr]]

Comment on attachment 9335925 [details]
Bug 1830070: Add a test for opening a blob URI in a popup with noopener r?timhuang

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This code will only affect people with RFP/FPP enabled, but Tor will run on ESR 115 so it will reduce the number of patches they'll need.
• User impact if declined: Tor carries more patches
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):

Comment 38

[Ryan VanderMeulen [:RyanVM]]

FYI, these don't all graft cleanly to ESR115. Are there other bugs we need to also take?

Comment 39

[Tom Ritter [:tjr]]

Attachment: Bug 1830070: Correctly apply RFP Checks to about: documents and deal with pop-ups (ESR) r=smaug,emilio

This patch has three parts to it, in addition to the many tests
it adds:

1. Use NS_IsContentAccessibleAboutURI to ensure that only safe
   about: documents get exempted.

   With this change, we will no longer allow about:blank or
   about:srcdoc to be exempted base on URI. If they are to be
   exempted, it will need to be base on other information.

2. In Document::RecomputeResistFingerprinting we previously
   deferred to a Parent Document if we had one, and either the
   principals matched or we were a null principal.

   We will do the same thing, except we will also defer to our
   opener as well as the parent document. Now about:blank
   documents can be exempted.

   However, this deferral only works if the opener is
   same-process. For cross-process openers, we make the decision
   ourselves.

We can make the wrong decision though. CookieJarSettings is
inherited through iframes but it is not inherited through popups.
(Yet. There's some discussion there, but it's not implemented.)

Conceptually; however, we do want CJS to inherit, and we do want
RFP to inherit as well. Because a popup can collude with its
opener to bypass RFP and Storage restrictions, we should propagate
the CJS information.

This does lead to an unusual situation: if you have exempted
b.com, and a.com (which is not exempted) creates a popup for b.com
then that popup will not be exempted. But an open tab for b.com
would be. And it might be hard to tell those two apart, or why
they behave differently.

The third part of the patch:

3. In LoadInfo we want to populate information down from the
   opener to the popup. This is needed because otherwise a
   cross-origin popup will not defer to its opener (because in
   Fission they're in different processes) and will decide if
   it should be exempted itself. It's the CookieJarSettings
   object that prevents the cross-origin document from thinking
   it should be exempted - CJS tells it 'No, you're a child
   (either a subdocument or a popup) and if I say you don't get
   an exemption, you don't.'

Finally, there is one more caveat: we can only defer to a parent
document or opener if it still exists. A popup may outlive its
opener. If that happens, and something induces a call to
RecomputeResistFingerprinting, then (e.g.) an about:blank popup
may lose an RFP exemption that it had received from its parent.
This isn't expected to happen in practice -
RecomputeResistFingerprinting is only called on document creation
and pref changes I believe.

It is not possible for a popup to gain an exemption though,
because even if the parent document is gone, the CJS lives on and
restricts it.

Comment 40

[Tom Ritter [:tjr]]

The patches do actually apply cleanly, but different workflow steps for me on the same patch would alternately apply cleanly or have errors. Weird, probably a bug.

I manually patched all of them onto esr115 and squashed them to a single commit.

Comment 41

[Dianna Smith [:diannaS]]

Comment on attachment 9343125 [details]
Bug 1830070: Correctly apply RFP Checks to about: documents and deal with pop-ups (ESR) r=smaug,emilio

Approved for 115.1esr

Comment 42

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/ac88490d08ba

Comment 43

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-release/rev/9f191db9730e
Add a test for an RFP test for a popup r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/fe40ea9e3300
Add a test for popup that is a data: uri r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/37f15e0b3ffb
Add a test for a popup opening a blob url r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/04787193a095
Add a test for popup that is an about:blank document r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/f5edcdb4a031
Correctly apply RFP Checks to about: documents and deal with pop-ups r=smaug,necko-reviewers,emilio
https://hg.mozilla.org/releases/mozilla-release/rev/80217d355f2a
Add a test for opening a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/591595f4171b
Add a test for opening a data URI in a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/919b884d5a2b
Add a test for an RFP test for a popup r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/3ed6c302f108
Add a test for popup that is a data: uri r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/443d83b8fe70
Add a test for a popup opening a blob url r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/3d372cedcfdb
Add a test for popup that is an about:blank document r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/dfbd51c0a3a6
Correctly apply RFP Checks to about: documents and deal with pop-ups r=smaug,necko-reviewers,emilio
https://hg.mozilla.org/releases/mozilla-release/rev/3628e8b8d28a
Add a test for opening a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/dd73f3c56f5e
Add a test for opening a data URI in a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/e962979d701e
Add a test for an RFP test for a popup r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/e41d8c2f85df
Add a test for popup that is a data: uri r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/7cf935920d9a
Add a test for a popup opening a blob url r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/dc7d31b22ca7
Add a test for popup that is an about:blank document r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/c84325c71e91
Correctly apply RFP Checks to about: documents and deal with pop-ups r=smaug,necko-reviewers,emilio
https://hg.mozilla.org/releases/mozilla-release/rev/db1529c19fb3
Add a test for opening a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/2b18a4962ce4
Add a test for opening a data URI in a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/de379bee27d0
Add a test for an RFP test for a popup r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/582e28b83b50
Add a test for popup that is a data: uri r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/785e1775c13d
Add a test for a popup opening a blob url r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/f0f70e7c8250
Add a test for popup that is an about:blank document r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/fa89c51ef6fc
Correctly apply RFP Checks to about: documents and deal with pop-ups r=smaug,necko-reviewers,emilio
https://hg.mozilla.org/releases/mozilla-release/rev/45032e474c29
Add a test for opening a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/f264c5b9c200
Add a test for opening a data URI in a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/e4490bf3d040
Add a test for an RFP test for a popup r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/7e5b72c73304
Add a test for popup that is a data: uri r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/0a2d5f339e41
Add a test for a popup opening a blob url r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/7a86f85ae0e8
Add a test for popup that is an about:blank document r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/7ea9676f22ae
Correctly apply RFP Checks to about: documents and deal with pop-ups r=smaug,necko-reviewers,emilio
https://hg.mozilla.org/releases/mozilla-release/rev/4800b1312e99
Add a test for opening a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/ae685d3825b7
Add a test for opening a data URI in a popup with noopener r=timhuang
https://hg.mozilla.org/releases/mozilla-release/rev/92b5f8af326f
Add a test for opening a blob URI in a popup with noopener r=timhuang

Comment 44

[Daniel Veditz [:dveditz]]

Sorry for the burst of bugspam: filter on tinkling-glitter-filtrate
Adding reporter-external keyword to security bugs found by non-employees for accounting reasons