Audit HTTP/2 and SPDY if needed
HTTP/2 is enabled in Firefox 38. We should make sure it adheres to our design specifcation. If not we need to either patch the problematic things or switch them of via preferences. Related: #6101 (moved) and #4100 (moved).
Activity
Trac:
Description: HTTP/2 is enabled in Firefox 38. We should make sure it adheres to our design specifcation. If not we need either patch the problematic things or switch them of via preferences. Related: #6101 (moved) and #4100 (moved).to
HTTP/2 is enabled in Firefox 38. We should make sure it adheres to our design specifcation. If not we need to either patch the problematic things or switch them of via preferences. Related: #6101 (moved) and #4100 (moved).
Looking over Georg and my mails with Mark Nottingham, our concerns then were:
- Long-lived connections, especially without proper first party isolation in terms of connection re-use.
- We probably want to disable server push, due to cache inference attacks (see: https://gnunet.org/sites/default/files/uzunov2013torspdy.pdf section 6.3.2)
- PING and SETTINGS timing side channels.
I had this to say about PING and SETTINGS:
Relatedly, I'm somewhat worried about the bidirectional nature of PING and SETTINGS from the tor-specific perspective. Because these can be sent async by the server and are acked immediately by the client, they might be able to introduce a timing signature by the server beyond what is contained in response to active requests from the client.. Granted, such things are possible with Javascript (or even by simply shoving a bunch of junk inside an html comment in a hidden element), but people are able to disable Javascript, and static content-based mechanisms will also show load progress indication in the UI.
Therefore, In Tor Browser, I'd be tempted to close the connection if I got more than some frequency of PING or SETTINGS updates from the server, especially in the absence of other activity.
Mark said that closing such connections seemed fine, but we'd have to be careful about content elements just reopening them either through JS or dynamic CSS-based element loads. One option for that is to disable browser-based JS/HTML network activity in inactive tabs using Firefox's request filter APIs.
We also discussed playing with HTTP/2's various batching and padding parameters as another possible defense against website traffic fingerprinting, but I'm not sure there is enough there already to make this very useful beyond the types of things that our pipelining defense already does, and we still need more research in this area to know what we'd want to add/change and how. He suggested waiting for HTTP/3 for requesting such things in spec form, since there will be resistance to late spec changes that may change cost profiles for the server side, especially for deployment (since that has already begun).
-
Note: don't forget the keep-alive audit for SPDY (see: #4100 (moved)).
Trac:
Cc: mcs to mcs, dcf@torproject.org-
Keep #16673 (moved) in mind.
Trac:
Owner: mikeperry to tbb-team
Keywords: N/A deleted, tbb-usability-website, tbb-linkability added -
Trac:
Cc: mcs, dcf@torproject.org to mcs, dcf@torproject.org, gk I wondered what the performance effects of SPDY/HTTP2 are. As a simple test, I used the network panel in Tor Browser to compare the loading speed of https://en.wikipedia.org/wiki/Main_Page. Here are the page loading times, in seconds:
network.http.spdy.enabled = false 8.36, 7.21, 7.16, 6.47, 9.39, 7.19, 7.10, 7.05, 7.21, 7.03, 8.58, 9.41, 7.03, 6.93, 6.81, 7.40 Average: 7.52 ± 0.90 s network.http.spdy.enabled = true (also, sub-prefs enabled) 4.67, 5.18, 4.81, 4.56, 5.03, 4.62, 5.21, 4.95, 4.67, 4.69, 5.04, 4.73, 4.86, 4.71, 4.60, 4.82 Average: 4.82 ± 0.21 sSo, at least on that page, Tor Browser would benefit from re-enabled HTTP2/SPDY (unsuprisingly).
Trac:
Keywords: N/A deleted, tbb-performance added
Cc: mcs, dcf@torproject.org, gk to mcs, dcf@torproject.org, gk, arthuredelsteinHere's a bug that was resolved for Firefox 55: https://bugzilla.mozilla.org/show_bug.cgi?id=1334693 So it looks like a lot of work has been done already.
There's also a bug suggesting some unit tests should be written: https://bugzilla.mozilla.org/show_bug.cgi?id=1337868
Patrick McManus suggested we check if origin frames are properly isolated. Firefox has a pref to disable them if needed.
https://tools.ietf.org/html/draft-ietf-httpbis-origin-frame-04
This document on H2 fingerprinting also seems very relevant: https://www.blackhat.com/docs/eu-17/materials/eu-17-Shuster-Passive-Fingerprinting-Of-HTTP2-Clients-wp.pdf
FWIW according to w3techs HTTP/2 is used by 24.5% of all the websites.
Replying to cypherpunks:
FWIW according to w3techs HTTP/2 is used by 24.5% of all the websites.
Yes, this is on the roadmap, we should be looking at this real-soon-now. Thanks!
-
We have first sites that basically break without having HTTP/2 enabled, see #25735 (moved).
Trac:
Username: mahrud
Cc: mcs, dcf@torproject.org, gk, arthuredelstein to mcs, dcf@torproject.org, gk, arthuredelstein, mahrud-
Trac:
Parent: N/A to #25735 (moved) 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/14952
Trac:
Keywords: TorBrowserTeam201808 deleted, TorBrowserTeam201808R added
Status: assigned to needs_review-
Nice, thanks for the investigation. Some first thoughts while reading through your notes:
- Is the disk avoidance requirement respected in case there is some caching going on?
- Does New Identity give us a clean slate with HTTP/2 enabled?
- I don't see why we want to have server push enabled. Let's try with that disabled first.
- 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 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-osbugs are still bugs. I guess this can be done in a new bug as well.
Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201808R deleted, TorBrowserTeam201808 added Trac:
Cc: mcs, dcf@torproject.org, gk, arthuredelstein, mahrud to mcs, dcf@torproject.org, gk, arthuredelstein, mahrud, dmrReplying to gk:
Nice, thanks for the investigation. Some first thoughts while reading through your notes:
Thanks for the review and these good questions:
- 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.
- 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-connectionsnotification. 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.- I don't see why we want to have server push enabled. Let's try with that disabled first.
OK, I've opened #27127 (moved) to remind us to audit and potentially enable push in the future. In the meantime I will set
network.http.spdy.allow-pushto false.- 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 (moved).
- 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-osbugs are still bugs. I guess this can be done in a new bug as well.
OK, I have opened #27128 (moved).
Replying to 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 (moved)).
Here's the revised patch for review:
https://github.com/arthuredelstein/tor-browser/commit/14952+1
-
Replying to arthuredelstein:
[snip]
Here's the revised patch for review:
https://github.com/arthuredelstein/tor-browser/commit/14952+1
Looks good. commit 36724cc11e94d0dc3094c94f046d76fb5ce44a2b on
tor-browser-60.1.0esr-8.0-1has the changes.Trac:
Keywords: TorBrowserTeam201808R deleted, TorBrowserTeam201808 added
Status: needs_review to new
