1637516 - Consider changing the key of StoragePrincipal from registrable domain (eTLD+1) to site

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

As mentioned in bug 1630868 this would be a better boundary as insecure and secure connections wouldn't share a key.

See https://html.spec.whatwg.org/#obtain-a-site for a formal definition of obtaining a site from an origin.

Addressing this before shipping in Nightly would prevent some database migration hassle as I understand it.

Comment 1

[Gary Chen [:xeonchen]]

Do we plan to make this change apply to FPI? This may affect Tor Browser so we might need a separate pref for this?

Comment 2

[Anne (:annevk)]

That's bug 1630869. And yeah, we should not touch FPI I think.

Comment 3

[Johann Hofmann [:johannh]]

This sounds like a lot of hassle for a bit unclear/what seems like minor security advantages. Can you add some more reasoning for this, considering that we have mixed content blocking? I feel like we may not want to spend our time on this for now.

Comment 4

[Johann Hofmann [:johannh]]

Note that if we have a different firstPartyDomain based on a pref we lose storage compat between FPI and dFPI and might need to do special casing in some places...

Comment 5

[Johann Hofmann [:johannh]]

Though I'll admit that if we want to do this at all it's probably preferable to do it before shipping (maybe it's enough to address this before shipping to release).

But can't we just dupe this with bug 1630869 and resolve that one then?

Comment 6

[Anne (:annevk)]

If we don't do this and a website does not use HSTS + includSubDomains, network attackers have a pretty easy way to perform cache attacks on that website.

Our competitors are also pushing this meaning that not doing this would mean we're not standards compliant. I don't really care how we go about implementing it, but touching FPI would require a whole migration story as I understand it whereas only doing it for dFPI would not.

Comment 7

[Johann Hofmann [:johannh]]

If we don't do this and a website does not use HSTS + includSubDomains, network attackers have a pretty easy way to perform cache attacks on that website.

Is cache attacks really the thing to worry about on an insecure network with no HSTS?

Anyway, yeah, we should probably do it for standards compliance in any case.

We could consider just accepting the fact that we clear web storage for all FPI users. I'm really not a big fan of making the fPD attribute conditional...

Comment 8

[Anne (:annevk)]

The network is unsafe. And while websites are adopting HSTS, not all websites can set includeSubDomains. Currently these attacks are easier as we don't partition, but once we do this would be an easy step for an attacker to make to continue being able to do their attacks.

Comment 9

[Gary Chen [:xeonchen]]

(In reply to Johann Hofmann [:johannh] from comment #5)

But can't we just dupe this with bug 1630869 and resolve that one then?

We can keep FPI/dFPI have the same behavior here, and (if necessary, ) use another pref to control whether we should use site or not.
The pref may keep some backward-compatibility for special cases, e.g. Tor Browser, and we can turn this pref on to use "site".

Comment 10

[Gary Chen [:xeonchen]]

Attachment: Bug 1637516 - part 1: refine functions for further update;

Comment 11

[Gary Chen [:xeonchen]]

Attachment: Bug 1637516 - part 2: make first-party domain support site;

Depends on D75548

Comment 12

[Gary Chen [:xeonchen]]

Attachment: Bug 1637516 - part 3: update tests;

Depends on D75549

Comment 13

[Gary Chen [:xeonchen]]

To be compatible with Fission, we should consider making this "site" info being hashed with salt to avoid leaking first-party domain info its children.
If we don't do this in this bug, it will require data migration in the future.

Currently all first-party domain info are stored and transmitted by plain-text, we're not sure how much effort it may take to achieve this goal.
Nika, could you give some advice to see if we need this now? Thanks!

Comment 14

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

In general we'd like to avoid having ancestor principals be visible in content processes. There are a couple of places we leak the information into remote subframes, such as through LoadInfo, and don't have any concrete near-term plans to get rid of those leaks, though we'd like to get rid of them in the future.

I don't want to block important work on getting our data leak story perfect, especially when we already have leaks of a similar kind in other code, so I think it's up to your judgement call as to whether it's better to do the easy thing now, and potentially need to do data migration and a redesign in the long-term, or to put in the extra work, and avoid this information leak and future migrations today.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c71a64381d2
part 1: refine functions for further update; r=timhuang,baku
https://hg.mozilla.org/integration/autoland/rev/a77c31684931
part 2: make first-party domain support site; r=baku,timhuang
https://hg.mozilla.org/integration/autoland/rev/9e1ecd2a9947
part 3: update tests; r=timhuang,baku

Comment 16

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/0c71a64381d2
https://hg.mozilla.org/mozilla-central/rev/a77c31684931
https://hg.mozilla.org/mozilla-central/rev/9e1ecd2a9947