Closed Bug 1560623 Opened 7 months ago Closed 5 months ago

First-party isolation makes it impossible to turn off content blocking

Categories

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

69 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1572240
Tracking Status
firefox68 --- unaffected
firefox69 --- fix-optional

People

(Reporter: bugzilla, Assigned: tjr)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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

Steps to reproduce:

  • In a fresh profile, set content blocking to custom, change block list to level 2.
  • Enable first-party isolation in about:config.
  • Open a private window and navigate to https://patrickhlauke.github.io/recaptcha/
  • Turn off blocking temporarily.

Actual results:

The Recaptcha widget does not appear. The console notifies that "The resource at “https://www.google.com/recaptcha/api.js” was blocked because content blocking is enabled."

Expected results:

The Recaptcha widget appears because content blocking is turned off.

Summary: First-party isolation breaks content blocking → First-party isolation makes it impossible to turn off content blocking
Component: Untriaged → Tracking Protection

Upon further investigation, things are even more screwy: Do the reproduction steps, but use a normal instead of a private window. Now blocking doesn't work even though it should.

I can reproduce this in Nightly but not in Beta, which leads me to believe that we broke this in bug 1330467. Because of bug 1557872 the UI picks up the change correctly, but we still check the URI instead of principal here: https://searchfox.org/mozilla-central/rev/543e1fbd04a85bae7b4d38357b185d99bcfc8e4b/toolkit/components/antitracking/AntiTrackingCommon.cpp#1784

Gary, would you be interested in taking this?

Status: UNCONFIRMED → NEW
Component: Tracking Protection → Privacy: Anti-Tracking
Ever confirmed: true
Flags: needinfo?(xeonchen)
Keywords: regression
Priority: -- → P2
Product: Firefox → Core
Regressed by: 1330467
See Also: → 1557872

(In reply to Johann Hofmann [:johannh] (Away until July 3rd) from comment #2)

I can reproduce this in Nightly but not in Beta, which leads me to believe that we broke this in bug 1330467. Because of bug 1557872 the UI picks up the change correctly, but we still check the URI instead of principal here: https://searchfox.org/mozilla-central/rev/543e1fbd04a85bae7b4d38357b185d99bcfc8e4b/toolkit/components/antitracking/AntiTrackingCommon.cpp#1784

Gary, would you be interested in taking this?

Sure, thanks for the investigation.

Assignee: nobody → xeonchen
Flags: needinfo?(xeonchen)
See Also: → 1572240

Per discussion offline, Tom will take over this bug.

Assignee: xeonchen → tom

I looked at this a bit last night. I want to make sure I'm on the right track.

I want to change this call to TestPermissionOriginNoSuffix into a call to TestPermissionFromPrincipal.

So I need to change IsOnContentBlockingAllowList (and CheckContentBlockingAllowList) to take a principal instead of a nsIURI.

I'm kind of hunting around for the right principal for all the callsites. For this callsite I got the following code to compile (untested though):

  nsresult rv;
  nsCOMPtr<nsIPrincipal> principal;
  nsCOMPtr<nsIScriptSecurityManager> securityManager =
      do_GetService(NS_SCRIPTSECURITYMANAGER_CONTRACTID, &rv);
  if (NS_WARN_IF(NS_FAILED(rv))) {
    return false;
  }
  securityManager->GetChannelResultPrincipal(aChannel, getter_AddRefs(principal));
Flags: needinfo?(ehsan)

Tom, sorry you spent time on this... The principal we use for checking the content blocking allow list isn't the channel result principal, it's a modified version, see https://searchfox.org/mozilla-central/rev/9ae20497229225cb3fa729a787791f424ff8087b/toolkit/components/antitracking/ContentBlockingAllowList.jsm#51.

It turns out that I've been working on bug 1572240, which should fix this, among other things. I have my patches almost ready, they're currently running what I hope would be the final try run but let me post the patches to that bug. Can you please test them out? If they don't fix the bug it's a sign that I've messed up the origin attributes somewhere...

Flags: needinfo?(ehsan) → needinfo?(tom)
Depends on: 1572240
See Also: 1572240

Fixed for me with bug 1572240.

Awesome, thanks for testing! Marking this a dupe then.

Status: NEW → RESOLVED
Closed: 5 months ago
Flags: needinfo?(tom)
Resolution: --- → DUPLICATE
Duplicate of bug: 1572240

I guess one question here is whether we would want to backport something here for 69?

I think bug 1572240 is a rather risky backport for beta as it changes how we assign the content blocking allow list permission internally, but if we are going to backport it then I'd rather expose it to as many beta releases as possible to catch any potential issues (e.g. crashes) early on. I'm not super comfortable with backporting it to be honest. And I think a smaller scale patch wouldn't be very easy to create either, because we need to make CheckContentBlockingAllowList() accept a principal instead of a URI, and for that to happen we need to be able to get such a principal from a document and a channel and providing mechanisms for a document and a channel is already the majority of the patch series.

You need to log in before you can comment on or make changes to this bug.