Opened 3 years ago

Last modified 17 hours ago

#14952 needs_revision task

Audit HTTP/2 and SPDY if needed

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-linkability, tbb-usability-website, tbb-performance, ff60-esr, TorBrowserTeam201808
Cc: mcs, dcf@…, gk, arthuredelstein, mahrud, dmr Actual Points:
Parent ID: #25735 Points:
Reviewer: Sponsor:

Description (last modified by gk)

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 and #4100.

Child Tickets

Change History (46)

comment:1 Changed 3 years ago by gk

Keep possible timing vectors wrt PING and SETTINGS frames in mind.

comment:2 Changed 3 years ago by gk

Description: modified (diff)

comment:3 Changed 3 years ago by mcs

Cc: mcs added

comment:4 Changed 3 years ago by mikeperry

Looking over Georg and my mails with Mark Nottingham, our concerns then were:

  1. Long-lived connections, especially without proper first party isolation in terms of connection re-use.
  2. 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)
  3. 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).

comment:5 Changed 3 years ago by gk

Note: don't forget the keep-alive audit for SPDY (see: #4100).

comment:6 Changed 3 years ago by gk

Priority: normalmajor

comment:7 Changed 3 years ago by mikeperry

Keywords: tbb-5.0a-highrisk added

Tag the set of things that are risky to debut in the 5.0-stable release without testing in a prior alpha.

comment:8 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201506 added

Ensure all tbb-5.0a items are on the June radar.

comment:9 Changed 3 years ago by mikeperry

Keywords: MikePerry201506 added
Owner: changed from tbb-team to mikeperry
Status: newassigned

comment:10 Changed 3 years ago by mikeperry

I pushed prefs to disable HTTP/2 for 5.0a3. I took a quick look at the source code, and the HTTP/2 implementation in Firefox is heavily dependent on the SPDY implementation.. We're probably going to have to review both HTTP/2 and SPDY v3.1 as a result. :/

comment:11 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201507 added; TorBrowserTeam201506 removed

Move over remaining June items to July

comment:12 Changed 3 years ago by mikeperry

Keywords: MikePerry201507 added; MikePerry201506 removed

Move my tickets over for July

comment:13 Changed 3 years ago by mikeperry

Keywords: tbb-5.0a4 added

Tag some 5.0a4 goals.

comment:14 Changed 3 years ago by dcf

Cc: dcf@… added

comment:15 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201508 added; TorBrowserTeam201507 removed

comment:16 Changed 3 years ago by mikeperry

Keywords: MikePerry201508 added; ff38-esr tbb-5.0a-highrisk MikePerry201507 tbb-5.0a4 removed

I am pretty much done with this audit. I need to write this up and file one or more tickets for support.

This is not a 5.0 item. We won't be able to do this by 5.0.

comment:17 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201509 added; TorBrowserTeam201508 removed

Move remaining August tickets to September.

comment:18 Changed 3 years ago by gk

Keywords: TorBrowserTeam201510 added; TorBrowserTeam201509 removed

Moving Tor Browser tickets to October 2015.

comment:19 Changed 3 years ago by gk

Keywords: TorBrowserTeam201511 added; TorBrowserTeam201510 removed

comment:20 Changed 3 years ago by mikeperry

Keywords: MikePerry201508 TorBrowserTeam201511 removed
Severity: Normal

comment:21 Changed 2 years ago by gk

Summary: Audit HTTP/2Audit HTTP/2 and SPDY if needed

comment:22 Changed 19 months ago by gk

Keywords: tbb-linkability tbb-usability-website added
Owner: changed from mikeperry to tbb-team

Keep #16673 in mind.

comment:23 Changed 19 months ago by gk

Cc: gk added

comment:24 Changed 8 months ago by arthuredelstein

Cc: arthuredelstein added
Keywords: tbb-performance added

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 s

So, at least on that page, Tor Browser would benefit from re-enabled HTTP2/SPDY (unsuprisingly).

Last edited 8 months ago by arthuredelstein (previous) (diff)

comment:25 Changed 8 months ago by arthuredelstein

(Deleting this comment as a separate issue from HTTP/2.)

Last edited 8 months ago by arthuredelstein (previous) (diff)

comment:26 Changed 8 months ago by arthuredelstein

Here'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

comment:27 Changed 8 months ago by arthuredelstein

Keywords: ff-59esr added

comment:28 Changed 8 months ago by cypherpunks

Keywords: ff59-esr added; ff-59esr removed

comment:29 Changed 8 months ago by arthuredelstein

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

comment:30 Changed 8 months ago by arthuredelstein

comment:31 Changed 7 months ago by gk

Keywords: ff60-esr added; ff59-esr removed

Firefox 60 is the new ESR.

comment:33 Changed 5 months ago by cypherpunks

comment:34 in reply to:  33 Changed 5 months ago by sysrqb

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!

comment:35 Changed 4 months ago by gk

We have first sites that basically break without having HTTP/2 enabled, see #25735.

comment:36 Changed 3 months ago by gk

Keywords: TorBrowserTeam201805 added

comment:37 Changed 3 months ago by gk

Priority: HighVery High

comment:38 Changed 2 months ago by gk

Keywords: TorBrowserTeam201806 added; TorBrowserTeam201805 removed

Moving our tickets to June 2018

comment:39 Changed 6 weeks ago by mahrud

Cc: mahrud added

comment:40 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201807 added; TorBrowserTeam201806 removed

Moving first batch of tickets to July 2018

comment:41 Changed 6 weeks ago by gk

Parent ID: #25735

comment:42 Changed 13 days ago by gk

Keywords: TorBrowserTeam201808 added; TorBrowserTeam201807 removed

Move our tickets to August.

comment:43 Changed 4 days ago by arthuredelstein

Keywords: TorBrowserTeam201808R added; TorBrowserTeam201808 removed
Status: assignedneeds_review

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.

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

I also confirmed the file cache is isolated:

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

Last edited 4 days ago by arthuredelstein (previous) (diff)

comment:44 Changed 23 hours ago by gk

Keywords: TorBrowserTeam201808 added; TorBrowserTeam201808R removed
Status: needs_reviewneeds_revision

Nice, thanks for the investigation. Some first thoughts while reading through your notes:

1) Is the disk avoidance requirement respected in case there is some caching going on?
2) Does New Identity give us a clean slate with HTTP/2 enabled?
3) I don't see why we want to have server push enabled. Let's try with that disabled first.
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.
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.

comment:45 Changed 23 hours ago by 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?

Last edited 23 hours ago by gk (previous) (diff)

comment:46 Changed 17 hours ago by dmr

Cc: dmr added
Note: See TracTickets for help on using tickets.