1535697 - Use separate connections for isolated third-party trackers

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement, P2)

Description

[(no longer active)]

Right now with Enhanced Tracking Protection enabled, two third-party tracking connections from two separate tabs for example (or a third-party tracking connection from a tab and a first-party tracking connection from another tab) may end up sharing the same connection together, which would allow the tracker to tie the user's browsing across multiple tabs together on the server side (similar to bug 1283319.)

We need to add the top-level page's origin to the nsHttpConnectionInfo's hash key for third-party tracking channels in order to prevent this from happening.

Comment 1

[(no longer active)]

Gary said he would be interested to take this. Here is some info on how a possible for this could work.

The core of the fix would be in nsHttpConnectionInfo::BuildHashKey(), through ensuring that we mix in the origin of the top-level document if our channel is an third-party tracking resource. In order to do this, we need to know two things: whether the channel is an isolated third-party tracking resource, and the origin of the top-level document.

You can determine whether the channel is an isolated third-party tracking resource using:

mIsThirdPartyTrackingResource &&
      !AntiTrackingCommon::IsFirstPartyStorageAccessGrantedFor(this, mURI,
                                                               nullptr)

We already compute this once when connecting to the network here: https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/netwerk/protocol/http/nsHttpChannel.cpp#3911. You'd want to compute this a bit earlier in BeginConnect() around https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/netwerk/protocol/http/nsHttpChannel.cpp#6430. You should probably call the same code in a helper function or something and assign the result to a boolean member and use it in both places to avoid calling IsFirstPartyStorageAccessGrantedFor() twice.

The origin of the top-level document can be obtained from the load info object: https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/netwerk/base/nsILoadInfo.idl#900

Testing this isn't as simple as testing bug 1283319. I think you'd want to add a mochitest which sets up test trackers, then loads a third-party tracking iframe, and then looks at the document's channel and reads the connectionInfoHashKey attribute from it.

Comment 2

[(no longer active)]

The patch in bug 1500533 makes us call SetPrivate(true) on the nsHttpConnectionInfo object belonging to an isolated third-party tracking channel, which is part of what we needed to do here. So the patch needed here can probably be built on top of what we have there.

Comment 3

[Ethan Tseng [:ethan]]

Tanvi, do we have a timeline for this bug? (Gary and I need to sort out his priority.)

Comment 4

[Ethan Tseng [:ethan]]

From Anti-tracking Engineering meeting:

1. We are targeting 68 Nightly.
2. Ehsan will ping Gary when bug 1500533 is done.

Comment 5

[(no longer active)]

This change should be documented on https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Privacy/Storage_access_policy.

Comment 6

[(no longer active)]

I'm picking this one since it's the continuation of bug 1500533.

Comment 7

[(no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c22fbfff6963a68fd2d1838188502d0305db989c

Comment 8

[(no longer active)]

Attachment: Bug 1535697 - Part 1: Avoid calling AntiTrackingCommon::IsFirstPartyStorageAccessGrantedFor() more than once per channel;

Comment 9

[(no longer active)]

Attachment: Bug 1535697 - Part 2: Represent whether the channel is isolated by anti-tracking as a separate axis on the connection info hash key;

Comment 10

[(no longer active)]

Attachment: Bug 1535697 - Part 3: Only consider third-party tracking resources as isolated channels;

Comment 11

[(no longer active)]

Attachment: Bug 1535697 - Part 4: Refactor the code for computing the origin of the top window for a channel and remember its result on the channel object;

Comment 12

[(no longer active)]

Attachment: Bug 1535697 - Part 5: Pass the top window origin to nsHttpConnectionInfo objects when constructing them;

Comment 13

[(no longer active)]

Attachment: Bug 1535697 - Part 6: Use separate network connections for isolated third-party trackers;

Comment 14

[(no longer active)]

Attachment: Bug 1535697 - Part 7: Pass the isolated flag to the nsHttpConnectionInfo constructor when cloning the object;

Building the hashkey for these objects will soon depend on the isolated flag,
therefore we need to ensure that it is available when cloning the objects
inside the constructor. This patch refactors the clone method to avoid using
SetIsolated().

Comment 15

[(no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f2eae413c6524a2c574f4cd2072a7a86ab74d06

Comment 16

[Steven Englehardt [:englehardt]]

I'll wait until this lands to document (keeping ni). Thanks for flagging me -- I'm excited to see progress here :).

Comment 17

[(no longer active)]

(In reply to Steven Englehardt [:englehardt] from comment #16)

I'll wait until this lands to document (keeping ni).

Sounds good!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9191d987a9e1
Part 1: Avoid calling AntiTrackingCommon::IsFirstPartyStorageAccessGrantedFor() more than once per channel; r=michal
https://hg.mozilla.org/integration/autoland/rev/4a5475f6aa49
Part 2: Represent whether the channel is isolated by anti-tracking as a separate axis on the connection info hash key; r=michal
https://hg.mozilla.org/integration/autoland/rev/c316d49df8c4
Part 3: Only consider third-party tracking resources as isolated channels; r=michal
https://hg.mozilla.org/integration/autoland/rev/5cbdeb635a3e
Part 4: Refactor the code for computing the origin of the top window for a channel and remember its result on the channel object; r=michal
https://hg.mozilla.org/integration/autoland/rev/969945148b3d
Part 5: Pass the top window origin to nsHttpConnectionInfo objects when constructing them; r=michal
https://hg.mozilla.org/integration/autoland/rev/ebad998aae7a
Part 6: Use separate network connections for isolated third-party trackers; r=michal
https://hg.mozilla.org/integration/autoland/rev/037390836504
Part 7: Pass the isolated flag to the nsHttpConnectionInfo constructor when cloning the object; r=michal

Comment 19

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/9191d987a9e1
https://hg.mozilla.org/mozilla-central/rev/4a5475f6aa49
https://hg.mozilla.org/mozilla-central/rev/c316d49df8c4
https://hg.mozilla.org/mozilla-central/rev/5cbdeb635a3e
https://hg.mozilla.org/mozilla-central/rev/969945148b3d
https://hg.mozilla.org/mozilla-central/rev/ebad998aae7a
https://hg.mozilla.org/mozilla-central/rev/037390836504

Comment 20

[Raul Gurzau (:RaulG)]

Backed out 7 changesets (bug 1535697) for causing Bug 1547596. a=backout

Backout link: https://hg.mozilla.org/mozilla-central/rev/588535ba7633dee062173d41059b403fd3939e4e

Comment 21

[(no longer active)]

Bug 1547596 was caused because of https://hg.mozilla.org/mozilla-central/rev/969945148b3d#l1.164. Here we would be trying to deserialize a string saved by an older build of Firefox which doesn't include mTopWindowOrigin. The fix is to move the code to serialize/deserialize this new member to the very end obviously.

I also added some comments about this subtle semantics for the next person to add a new member to AltSvcMapping.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47c353bd446d
Part 1: Avoid calling AntiTrackingCommon::IsFirstPartyStorageAccessGrantedFor() more than once per channel; r=michal
https://hg.mozilla.org/integration/autoland/rev/6b122a9ce1a4
Part 2: Represent whether the channel is isolated by anti-tracking as a separate axis on the connection info hash key; r=michal
https://hg.mozilla.org/integration/autoland/rev/c79109688f29
Part 3: Only consider third-party tracking resources as isolated channels; r=michal
https://hg.mozilla.org/integration/autoland/rev/c22a48c48548
Part 4: Refactor the code for computing the origin of the top window for a channel and remember its result on the channel object; r=michal
https://hg.mozilla.org/integration/autoland/rev/86be6123ca53
Part 5: Pass the top window origin to nsHttpConnectionInfo objects when constructing them; r=michal
https://hg.mozilla.org/integration/autoland/rev/3e02969bf413
Part 6: Use separate network connections for isolated third-party trackers; r=michal
https://hg.mozilla.org/integration/autoland/rev/251ef3905140
Part 7: Pass the isolated flag to the nsHttpConnectionInfo constructor when cloning the object; r=michal

Comment 23

[Steven Englehardt [:englehardt]]

Documented in https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Privacy/Storage_access_policy$compare?from=1544621&to=1546858

Comment 24

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/47c353bd446d
https://hg.mozilla.org/mozilla-central/rev/6b122a9ce1a4
https://hg.mozilla.org/mozilla-central/rev/c79109688f29
https://hg.mozilla.org/mozilla-central/rev/c22a48c48548
https://hg.mozilla.org/mozilla-central/rev/86be6123ca53
https://hg.mozilla.org/mozilla-central/rev/3e02969bf413
https://hg.mozilla.org/mozilla-central/rev/251ef3905140