[tor-bugs] #14952 [Applications/Tor Browser]: Audit HTTP/2 and SPDY if needed

[Tor Bug Tracker & Wiki]

#14952: Audit HTTP/2 and SPDY if needed
-------------------------------------------------+-------------------------
 Reporter:  gk                                   |          Owner:  tbb-
                                                 |  team
     Type:  task                                 |         Status:
                                                 |  needs_revision
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-linkability, tbb-usability-      |  Actual Points:
  website, tbb-performance, ff60-esr,            |
  TorBrowserTeam201808                           |
Parent ID:  #25735                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by arthuredelstein):

 Replying to [comment:44 gk]:
 > Nice, thanks for the investigation. Some first thoughts while reading
 through your notes:

 Thanks for the review and these good questions:

 > 1) Is the disk avoidance requirement respected in case there is some
 caching going on?

 I'm still working on figuring this out and will post here when I have an
 answer.

 > 2) Does New Identity give us a clean slate with HTTP/2 enabled?

 I looked into network connections, which are supposed to be killed as a
 result of torbutton sending a `net:prune-all-connections` notification. I
 followed the code to
 nsHttpConnectionMgr::ClosePersistentConnections(nsConnectionEntry *ent),
 which removes idle nsHttpConnections, causes active nsHttpConnections
 (which represents both HTTP1 and HTTP/2 connections) to immediately
 expire, regardless of whether these nsHttpConnections are HTTP/2
 (mSpdySession) or not. So I think New Identity is correctly stopping
 HTTP/2 connections.

 > 3) I don't see why we want to have server push enabled. Let's try with
 that disabled first.

 OK, I've opened #27127 to remind us to audit and potentially enable push
 in the future. In the meantime I will set `network.http.spdy.allow-push`
 to false.

 > 4) I am fine leaving possible PING/SETTINGS-related timing side-channels
 for a different bug for now. If so, please open a new one.

 I opened #27123.

 > 5) I am not overly happy about the different values of some of the prefs
 you mentioned above depending on being on a desktop/mobile platform we
 should investigate the impact of shipping the same configuration for both
 of them. After all, `tbb-fingerprinting-os` bugs are still bugs. I guess
 this can be done in a new bug as well.

 OK, I have opened #27128.

 Replying to [comment:45 gk]:
 > Oh, and 6): What about `dom.push.http2.*`? Are we getting the code
 behind those as well and, if so, are we good with it?

 These prefs are used by the Push API implementation. We are not currently
 affected by them, because we have dom.push.enabled set to false and
 dom.servicesWorkers.enabled set to false (both following Firefox 60ESR).
 These will be enabled in the next ESR, however (#15563).

 Here's the revised patch for review:

 https://github.com/arthuredelstein/tor-browser/commit/14952+1

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/14952#comment:47>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online