Opened 2 years ago

Closed 2 years ago

#21340 closed defect (fixed)

Identify and backport new patches from Firefox

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff52-esr, TorBrowserTeam201704R, tbb-7.0-must-alpha
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor4

Description

There are a number of first-party isolation patches, as well as other patches that have landed after FF52ESR but will be useful to us in TBB52ESR.

Child Tickets

Change History (25)

comment:1 Changed 2 years ago by gk

Sponsor: Sponsor4

comment:2 Changed 2 years ago by cypherpunks

The most useful patches besides Tor-related ones are https://blog.mozilla.org/security/2016/11/10/enforcing-content-security-by-default-within-firefox/, but they seems hard to backport. Maybe, Mozillians could help with it.

comment:3 Changed 2 years ago by arthuredelstein

Keywords: TorBrowserTeam2017R added
Status: newneeds_review

Here's a list of patches I cherry-picked or backported from Firefox >=53 without too much difficulty. They are Tor uplift patches or addition first-party isolation work.

1334690 Isolate AlternateService mappings by Origin Attributes
1334693 Investigate and isolate SPDY/HTTP2 state by first-party domain when privacy.firstparty.isolate = true
1315602 Remove the assertion of FirstPartyDomain should be empty in HTTP redirect
1317927 Media caching needs to use origin attributes
1274020 Add a test to show that the DOM Cache is separated by origin attributes
1282655 Add a test case to test whether site permissions are universal or isolated for each type of OriginAttribute
1305144 Spoof referrer when leaving a .onion domain (Tor 17334)
1216893 Add pref to optionally disable SVG (Tor 12827)

Here's the branch with these patches. If this seems reasonable I will merge these with my latest #20680 branch.

https://github.com/arthuredelstein/tor-browser/commits/20680

A few patches have substantial conflicts: namely HSTS/HPKP isolation and the network predictor isolation patch. These are going to take further work:

1290529 clear HSTS and HPKP for subdomains as well when bug 1115712 is fixed
1323644 Isolate the HSTS and HPKP cache by first party domain.
1336867 Remove unsafeProcessHeader and isSecureHost in nsISiteSecurityService
1115712 make DataStorage for HPKP and HSTS enumerable via xpcom

1312954 Making the network predictor obey originAttributes and updating SpeculativeConnect() to SpeculativeConnect2().
Version 0, edited 2 years ago by arthuredelstein (next)

comment:4 Changed 2 years ago by gk

Keywords: ff52-esr tbb-7.0-must TorBrowserTeam201703 added; TorBrowserTeam2017R removed
Status: needs_reviewneeds_revision

I have not looked closely at the patches but for now we don't need

1334690 Isolate AlternateService mappings by Origin Attributes
1334693 Investigate and isolate SPDY/HTTP2 state by first-party domain when privacy.firstparty.isolate = true

.
We have H2 and SPDY still disabled and will have to audit it first for other issues. We can then apply the backported patches (if we get to that before esr59).

As to the patches with substantial conflicts: we would want to have those. (Or write our own).

Last edited 2 years ago by gk (previous) (diff)

comment:5 in reply to:  4 Changed 2 years ago by gk

Replying to gk:

As to the patches with substantial conflicts: we would want to have those. (Or write our own).

Although the network predictor is currently disabled. Thus here we should weigh the benefits with the complexity the new code introduces (we have #16633 for that fwiw and we might want to look at #16625 again as well. Especially: "The seer does not record any data, nor does it take any action, when in private browsing mode." seems worth to contemplate as we are in permanent private browsing mode.

Last edited 2 years ago by gk (previous) (diff)

comment:6 Changed 2 years ago by arthuredelstein

I have now backported the HSTS/HPKP patches. It required several other patches to be backported as well.
So here is a branch based on 20680+8 that contains all previously backported patches for this ticket plus HSTS/HPKP stuff.
https://github.com/arthuredelstein/tor-browser/commits/21340+4

Note there is also a pending ticket here whose patches Jonathan suggests should be backported once they are ready:
https://bugzilla.mozilla.org/show_bug.cgi?id=1342178

comment:7 Changed 2 years ago by gk

Parent ID: #20680
Status: needs_revisionassigned

comment:8 Changed 2 years ago by gk

Keywords: TorBrowserTeam201704 added; TorBrowserTeam201703 removed

Moving tickets over to April

comment:9 Changed 2 years ago by gk

Keywords: tbb-7.0-must-alpha added; tbb-7.0-must removed

Getting this on our radar for alpha release in less than two weeks.

comment:10 Changed 2 years ago by cypherpunks

tbb-nightly:
SVG is visible with svg.disabled=true (also svg.in-content.enabled=true exists on High Sec)

comment:11 in reply to:  10 ; Changed 2 years ago by gk

Replying to cypherpunks:

tbb-nightly:
SVG is visible with svg.disabled=true (also svg.in-content.enabled=true exists on High Sec)

Thanks. That's #21885 now.

comment:12 in reply to:  11 ; Changed 2 years ago by cypherpunks

Replying to gk:

Replying to cypherpunks:

tbb-nightly:
SVG is visible with svg.disabled=true (also svg.in-content.enabled=true exists on High Sec)

Thanks. That's #21885 now.

You're welcome (:
Also downloading of zip file stalls reliably at 0 bytes with

[04-07 10:07:18] Torbutton INFO: External app requested
http channel Listener OnDataAvailable contract violation

comment:13 in reply to:  12 ; Changed 2 years ago by gk

Replying to cypherpunks:

Replying to gk:

Replying to cypherpunks:

tbb-nightly:
SVG is visible with svg.disabled=true (also svg.in-content.enabled=true exists on High Sec)

Thanks. That's #21885 now.

You're welcome (:
Also downloading of zip file stalls reliably at 0 bytes with

[04-07 10:07:18] Torbutton INFO: External app requested
http channel Listener OnDataAvailable contract violation

Yes, I had seem something like that before but did not get to file a ticket yet. It is #21886 now (which contains a workaround as well), thanks.

comment:14 in reply to:  13 Changed 2 years ago by cypherpunks

Replying to gk:

Replying to cypherpunks:

Replying to gk:

Replying to cypherpunks:

tbb-nightly:
SVG is visible with svg.disabled=true (also svg.in-content.enabled=true exists on High Sec)

Thanks. That's #21885 now.

You're welcome (:
Also downloading of zip file stalls reliably at 0 bytes with

[04-07 10:07:18] Torbutton INFO: External app requested
http channel Listener OnDataAvailable contract violation

Yes, I had seem something like that before but did not get to file a ticket yet. It is #21886 now (which contains a workaround as well), thanks.

I think it's the same issue as in #21766, but with e10s off. Your workaround is the same as mine (:
Also enabling plug-ins works only once: enable, press "no" to undo, enable again, and then there is no message box again, and no way to disable plug-ins as "disable plugins" button is missed in esr52.

comment:15 Changed 2 years ago by cypherpunks

Also I should note that your old patch enables vulnerable outdated flash, blocked by add-ons blocklist

    State: Enabled (STATE_VULNERABLE_UPDATE_AVAILABLE)

comment:17 in reply to:  7 Changed 2 years ago by arthuredelstein

Replying to gk:

https://bugzilla.mozilla.org/show_bug.cgi?id=1348841 seems relevant as well

This has now landed in the latest ESR52 so we will get it when we rebase for the alpha.

comment:18 Changed 2 years ago by gk

Status: assignedneeds_information

Arthur: where are we here given the previous comments? We have the HSTS/HPKP-related backport to consider. Something else as well?

comment:19 in reply to:  18 ; Changed 2 years ago by arthuredelstein

Keywords: TorBrowserTeam201704R added; TorBrowserTeam201704 removed

Replying to gk:

Arthur: where are we here given the previous comments? We have the HSTS/HPKP-related backport to consider. Something else as well?

I have rebased what I had in comment:6 on top of tor/tor-browser-52.0.2esr-7.0-2, except I left out the SVG patches which I think we can postpone:

https://github.com/arthuredelstein/tor-browser/commits/21340+5

These include the HSTS/HPKP-related patches, as well as these:

1315602 Remove the assertion of FirstPartyDomain should be empty in HTTP redirect
1274020 Add a test to show that the DOM Cache is separated by origin attributes
1282655 Add a test case to test whether site permissions are universal or isolated for each type of OriginAttribute
1305144 Spoof referrer when leaving a .onion domain (Tor 17334)

comment:20 Changed 2 years ago by arthuredelstein

Status: needs_informationneeds_review

comment:21 Changed 2 years ago by arthuredelstein

I should also mention I attempted to rebase the implementation patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1342178 but found that it didn't seem to fix the problem described on that ticket, presumably because my rebasing isn't quite right. It's rather complex, so I'm leaving it out for now.

Last edited 2 years ago by arthuredelstein (previous) (diff)

comment:22 in reply to:  19 Changed 2 years ago by gk

Replying to arthuredelstein:

Replying to gk:

Arthur: where are we here given the previous comments? We have the HSTS/HPKP-related backport to consider. Something else as well?

I have rebased what I had in comment:6 on top of tor/tor-browser-52.0.2esr-7.0-2, except I left out the SVG patches which I think we can postpone:

https://github.com/arthuredelstein/tor-browser/commits/21340+5

These include the HSTS/HPKP-related patches, as well as these:

1315602 Remove the assertion of FirstPartyDomain should be empty in HTTP redirect
1274020 Add a test to show that the DOM Cache is separated by origin attributes
1282655 Add a test case to test whether site permissions are universal or isolated for each type of OriginAttribute
1305144 Spoof referrer when leaving a .onion domain (Tor 17334)

I think I don't get to those anymore for 7.0a3. :( We should consider them for the next alpha.

comment:23 Changed 2 years ago by cypherpunks

First half-baked alpha? o_0

comment:24 in reply to:  19 Changed 2 years ago by gk

Replying to arthuredelstein:

Replying to gk:

Arthur: where are we here given the previous comments? We have the HSTS/HPKP-related backport to consider. Something else as well?

I have rebased what I had in comment:6 on top of tor/tor-browser-52.0.2esr-7.0-2, except I left out the SVG patches which I think we can postpone:

Sounds good to me.

https://github.com/arthuredelstein/tor-browser/commits/21340+5

These include the HSTS/HPKP-related patches, as well as these:

I put the HSTS/HPKP patch review into #17965. Right now I am inclined to postpone inclusion and have those patches the first alpha of the 7.5 series as this is quite complex and might need more testing. Not sure about backporting to the 7.0 series in that case.

> 1315602 Remove the assertion of FirstPartyDomain should be empty in HTTP redirect

commit efd86213b996d351757498968481962eb610c06c

> 1274020 Add a test to show that the DOM Cache is separated by origin attributes

commit 3da33fc90ce348fbc594d5aa45e85d8a4f08e539

> 1282655 Add a test case to test whether site permissions are universal or isolated for each type of OriginAttribute

commit 59cba8d0681caf53c46cd3718e34c9a49f9c5921

> 1305144 Spoof referrer when leaving a .onion domain (Tor 17334)

That got already included and is commit 4317d7a834b0abf95ba6afdb18902758c691da49. All commits are on tor-browser-52.1.0esr-7.0-2.

comment:25 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.