[tbb-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_review
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-linkability, tbb-usability-      |  Actual Points:
  website, tbb-performance, ff60-esr,            |
  TorBrowserTeam201808R                          |
Parent ID:  #25735                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by arthuredelstein):

 * keywords:
     tbb-linkability, tbb-usability-website, tbb-performance, ff60-esr,
     TorBrowserTeam201808
     =>
     tbb-linkability, tbb-usability-website, tbb-performance, ff60-esr,
     TorBrowserTeam201808R
 * status:  assigned => needs_review


Comment:

 To audit the HTTP2 implementation, I read through the code in:
 * nsHttpConnectionManager.h/.cpp
 * ASpdySession.h/.cpp
 * Http2Push.h/.cpp
 * Http2Session.h/.cpp
 * Http2Stream.h/.cpp

 I didn't find any obvious instances where cross-first-party state might be
 leaked to content. I did note a number of places in the code where Origin
 Attributes are used to isolate by first-party or container ID:

 == HTTP/2 ==

 Each HTTP2 connection is specified by an nsConnectionEntry, which is
 assigned a single nsHttpConnectionInfo object. nsHttpTransaction is also
 assigned an nsHttpConnectionInfo object.

 * nsHttpConnectionInfo hashes use OriginAttributes:
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/netwerk/protocol/http/nsHttpConnectionInfo.cpp#115

 * HTTP2Stream shares origin attributes with the socket transport:
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/netwerk/protocol/http/Http2Stream.cpp#1684

 Stateful HTTP/2 features need to be isolated by first party as well.

 * BuildOriginFrameHashKey uses OriginAttributes:
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/netwerk/protocol/http/nsHttpConnectionMgr.cpp#743
  (Note that OriginFrames are disabled when Firefox uses a proxy.)

 * Coalescing Origins:
  mCoalescingKeys.AppendElement uses OriginAttributes
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/netwerk/protocol/http/nsHttpConnectionMgr.cpp#5097

 * Push Stream hashes use OriginAttributes:
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/netwerk/protocol/http/Http2Stream.cpp#368

 I also confirmed the file cache is isolated

 * CacheFileUtils::AppendKeyPrefix uses Origin Attributes:
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/netwerk/cache2/CacheFileUtils.cpp#203

 == AltSvc ==

 I also wanted to check that AltSvc will be isolated by first party. I
 confirmed that Alternate Service hashes use OriginAttributes:
  https://dxr.mozilla.org/mozilla-
 esr60/rev/dd52b41d2b775e5c7261ce52795268b7670635fc/
 netwerk/protocol/http/AlternateServices.cpp#221

 == Passive Fingerprinting ==

 Both documents above (linked to in comment:30 and comment:32) raise the
 possibility of passive fingerprinting via HTTP/2 Settings parameters.
 Here's what I found:

 * SETTINGS_HEADER_TABLE_SIZE depends on the "network.http.spdy.default-
 hpack-buffer" pref. In Firefox it is set by default to 65536 on desktop
 and 4096 on mobile.
 * SETTINGS_ENABLE_PUSH and SETTINGS_MAX_CONCURRENT_STREAMS depend on
 "network.http.spdy.allow-push" pref, which is "true" by default.
 * SETTINGS_INITIAL_WINDOW_SIZE depends on "network.http.spdy.push-
 allowance", which is 131072 on desktop and 32768 on mobile by default.
 * SETTINGS_MAX_FRAME_SIZE is always set to 0x4000.

 The above prefs don't provide significant entropy, unless the user has
 modified the one or more of them from their default value. Otherwise they
 mainly serve to distinguish different browsers or platforms.

 PING or SETTINGS updates timing side-channels are something I am not
 addressing here. I think they could do with further investigation.

 == Results and patch ==

 The isolation of state to origin attributes looks well done to me in
 Firefox 60ESR's implementation of HTTP2. That said, the code is highly
 complex so it would be impossible to say I haven't missed any leaks of
 state to content or fingerprinting attacks. It would be good to have
 https://bugzilla.mozilla.org/show_bug.cgi?id=1337868 implemented by the
 Tor uplift team if possible.

 Over the long term, I would suggest that we should look into first-party
 isolation by process of both content and chrome code (the latter includes
 caching, networking code, and other stateful things.)

 But given the convincing effort by Mozilla to use Origin Attributes to
 provide state isolation everywhere in the HTTP2 code, I think we should
 enable HTTP2. Here's my proposed patch for review:

 https://github.com/arthuredelstein/tor-browser/commit/14592

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