Opened 5 years ago

Closed 3 years ago

#15988 closed task (fixed)

Update Tor Browser design documentation for 6.5

Reported by: gk Owned by: gk
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-spec, TorBrowserTeam201703R, GeorgKoppen201703
Cc: arthuredelstein, mikeperry, i139, brade, mcs, tom Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We considerably changed Tor Browser behavior when moving from FQDN isolation to binding identifiers and circuits to the base domain. We should update the design document and include a motivation for this move.

Child Tickets

TicketStatusOwnerSummaryComponent
#20447closedtbb-teamTor Browser Spec is not accurate regarding Session IDsApplications/Tor Browser

Attachments (3)

0001-Edit-pass-of-GK-s-design-doc-update.-Note-the-XXX.patch (6.3 KB) - added by mikeperry 3 years ago.
Patch with some edits to GK's branch, and an XXX question about describing the threat model issue with speculative connections.
0002-Improve-the-speculative-and-preconnect-section.patch (3.8 KB) - added by mikeperry 3 years ago.
Describe speculative connection handling more clearly.
0003-Update-speculative-connect-behavior-from-testing-res.patch (3.7 KB) - added by mikeperry 3 years ago.
Third patch in the series, with updates based on testing results (applies on top of the other two).

Download all attachments as: .zip

Change History (62)

comment:1 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201506 added; TorBrowserTeam201505 removed

comment:2 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201508 added; TorBrowserTeam201506 removed
Summary: Update Tor Browser design documentation for 4.5.1Update Tor Browser design documentation for 5.0

I will do a round of updates for 5.0 in August.

comment:3 Changed 4 years ago by gk

While reading some recent and older papers about tracking on the internet and defenses against it it occurred to me we should update our No filters section pointing to these papers and explaining our point in more detail. We should somewhere mention as well that it is hard to make the difference between non-tracking and tracking third parties and only trying to take care of the latter as some approaches try to do. But I think this road is a dead-end as well which we should not go.

Some things to consider:
http://ieee-security.org/TC/SPW2015/W2SP/papers/W2SP_2015_submission_24.pdf
http://ieee-security.org/TC/SPW2015/W2SP/papers/W2SP_2015_submission_32.pdf
https://jonathanmayer.org/papers_data/bau13.pdf
https://www.usenix.org/system/files/conference/nsdi12/nsdi12-final17.pdf

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

comment:4 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201509 added; TorBrowserTeam201508 removed

Move remaining August tickets to September.

comment:5 Changed 4 years ago by gk

Another paper we could point to is

http://w2spconf.com/2009/papers/s2p1.pdf

for the idea to bind client side identifiers to the URL bar domain. Our solution is a bit more strict than their "No Spatial Tracking across Domain and Limited Temporal Tracking"-one but the underlying idea is the same.

comment:6 Changed 4 years ago by gk

Keywords: TorBrowserTeam201510 added; TorBrowserTeam201509 removed

Moving Tor Browser tickets to October 2015.

comment:7 Changed 4 years ago by gk

Keywords: TorBrowserTeam201511 added; TorBrowserTeam201510 removed

comment:8 Changed 4 years ago by gk

Keywords: GeorgKoppen201511 added
Owner: changed from tbb-team to gk
Severity: Normal
Status: newassigned

comment:9 Changed 4 years ago by gk

We should update section 6 of our specific fingerprinting defenses (fonts) as we actually don't ship our broken patch anymore (Gábor Gulyás pointed me to that, thanks).

comment:10 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201512 added; TorBrowserTeam201511 removed

comment:11 Changed 4 years ago by gk

Keywords: GeorgKoppen201512 added; GeorgKoppen201511 removed

comment:12 Changed 4 years ago by gk

Keywords: TorBrowserTeam201601 added; TorBrowserTeam201512 removed

Tickets for Jan 2016.

comment:13 Changed 4 years ago by gk

Keywords: GeorgKoppen201601 added; GeorgKoppen201512 removed

comment:14 Changed 4 years ago by gk

Keywords: TorBrowserTeam201602 added; TorBrowserTeam201601 removed

Putting stuff on the radar for February.

comment:15 Changed 4 years ago by gk

Keywords: GeorgKoppen201602 added; GeorgKoppen201601 removed

Updating my tickets.

comment:16 Changed 4 years ago by gk

Keywords: GeorgKoppen201603 added; GeorgKoppen201602 removed

comment:17 Changed 4 years ago by gk

Keywords: TorBrowserTeam201603 added; TorBrowserTeam201602 removed

comment:18 Changed 4 years ago by gk

Keywords: GeorgKoppen201604 added; GeorgKoppen201603 removed

comment:19 Changed 4 years ago by gk

Keywords: TorBrowserTeam201604 added; TorBrowserTeam201603 removed

comment:20 Changed 4 years ago by gk

Keywords: TorBrowserTeam201605 added; TorBrowserTeam201604 removed

Moving tickets

comment:21 Changed 4 years ago by gk

Keywords: GeorgKoppen201605 added; GeorgKoppen201604 removed

Moving things for me to May.

comment:22 Changed 4 years ago by gk

Keywords: GeorgKoppen201606 added; GeorgKoppen201605 removed

comment:23 Changed 4 years ago by gk

Keywords: TorBrowserTeam201606 added; TorBrowserTeam201605 removed

comment:24 Changed 3 years ago by gk

Keywords: GeorgKoppen201607 added; GeorgKoppen201606 removed

Moving my tickets

comment:25 Changed 3 years ago by gk

Keywords: TorBrowserTeam201607 added; TorBrowserTeam201606 removed

comment:26 Changed 3 years ago by gk

Keywords: TorBrowserTeam201608 added; TorBrowserTeam201607 removed

Moving items to August 2016.

comment:27 Changed 3 years ago by gk

Keywords: GeorgKoppen201608 added; GeorgKoppen201607 removed

Moving my tickets as well.

comment:28 Changed 3 years ago by gk

Keywords: GeorgKoppen201609 added; GeorgKoppen201608 removed

Moving my tickets

comment:29 Changed 3 years ago by gk

Keywords: TorBrowserTeam201609 added; TorBrowserTeam201608 removed

Tickets for September.

comment:30 Changed 3 years ago by gk

Keywords: GeorgKoppen201610 added; GeorgKoppen201609 removed

Moving my tickets

comment:31 Changed 3 years ago by gk

Keywords: TorBrowserTeam201610 added; TorBrowserTeam201609 removed

Moving tickets to October.

comment:32 Changed 3 years ago by gk

Summary: Update Tor Browser design documentation for 5.0Update Tor Browser design documentation for 6.0

comment:33 Changed 3 years ago by bugzilla

What is the reason to hide current development of the design documentation from TBB community?
Maybe, it's better to start with the Working Draft v0.1 rather than trying to produce the Release in the dark?

comment:34 Changed 3 years ago by gk

Keywords: GeorgKoppen201611 added; GeorgKoppen201610 removed

Moving my tickets to November.

comment:35 Changed 3 years ago by gk

Keywords: TorBrowserTeam201611 added; TorBrowserTeam201610 removed

Moving tickets over to November.

comment:36 Changed 3 years ago by gk

Keywords: GeorgKoppen201612 added; GeorgKoppen201611 removed

Moving my tickets

comment:37 Changed 3 years ago by gk

Keywords: TorBrowserTeam201612 added; TorBrowserTeam201611 removed

Moving tickets to December.

comment:38 Changed 3 years ago by gk

Cc: i139 added

Resolved #21070 as duplicate.

comment:39 Changed 3 years ago by mcs

Cc: brade mcs added

comment:40 Changed 3 years ago by gk

Keywords: TorBrowserTeam201701 added; TorBrowserTeam201612 removed

Moving our tickets to January 2017

comment:41 Changed 3 years ago by gk

Keywords: GeorgKoppen201701 added; GeorgKoppen201612 removed

comment:42 Changed 3 years ago by gk

Summary: Update Tor Browser design documentation for 6.0Update Tor Browser design documentation for 6.5

Take 6.5 into account as well. The changes are not that many and considerably fresh in memory...

comment:43 Changed 3 years ago by gk

Keywords: TorBrowserTeam201702 added; TorBrowserTeam201701 removed

Moving our tickets to Feb 2017.

comment:44 Changed 3 years ago by gk

Keywords: GeorgKoppen201702 TorBrowserTeam201702R added; GeorgKoppen201701 TorBrowserTeam201702 removed
Status: assignedneeds_review

Long overdue, but here is a branch for review in my public tor-browser-spec repo: bug_15988_v3 (https://gitweb.torproject.org/user/gk/tor-browser-spec.git/commit/?h=bug_15988_v3&id=761ae60c2f2880a3c17dba4f08e1e207fec8bc1a). It is squashed and rebased onto master. For an unsquashed branch see bug_15988.

Mike: I guess the website traffic fingerprinting parts could need an update as well. Those are the only ones I left untouched as I am not up-to-date wrt the current state of the art of available defenses/attacks.

comment:45 Changed 3 years ago by gk

Cc: tom added

Resolved #20447 as a duplicate.

comment:46 Changed 3 years ago by cypherpunks

Is community feedback needed?

Also Arthur Edelstein suggests to update tor-browser-spec for esr52 in #21256.

comment:47 in reply to:  46 Changed 3 years ago by gk

Replying to cypherpunks:

Is community feedback needed?

"needed" seems to be the wrong word here. "appreciated" would be a good replacement. And, yes, if there is constructive feedback, please go ahead.

comment:48 Changed 3 years ago by gk

Keywords: tbb-spec added

comment:49 Changed 3 years ago by arthuredelstein

Cc: arthuredelstein added

comment:50 Changed 3 years ago by gk

Keywords: TorBrowserTeam201703R added; TorBrowserTeam201702R removed

No February anymore.

comment:51 Changed 3 years ago by cypherpunks

Well, it's hard to determine whether this spec is about the design goals or already implemented features only. So, should feedback be about design decisions or about current features only? Thus, let's start with both.

Tor Browser user needs better handling of https sites, because now

the fact that he's wrong to think that closed padlock assures 100% of security.

https://wiki.mozilla.org/CA:OCSP-HardFail

we don't do OCSP fetching for intermediates unless the user configures several things in about:config.

https://bugzilla.mozilla.org/show_bug.cgi?id=803582

firefox doesn't warn the user if SSL certificate lacks revocation checking info

https://bugzilla.mozilla.org/show_bug.cgi?id=496661

In the Security Slider section description of Medium Settings mentions that media.webaudio.enabled is used, but it was removed from the Firefox codebase long ago.

comment:52 in reply to:  51 Changed 3 years ago by gk

Replying to cypherpunks:

Well, it's hard to determine whether this spec is about the design goals or already implemented features only. So, should feedback be about design decisions or about current features only? Thus, let's start with both.

It's about both in our spec. How can we make that more clear?

Tor Browser user needs better handling of https sites, because now

the fact that he's wrong to think that closed padlock assures 100% of security.

https://wiki.mozilla.org/CA:OCSP-HardFail

we don't do OCSP fetching for intermediates unless the user configures several things in about:config.

https://bugzilla.mozilla.org/show_bug.cgi?id=803582

firefox doesn't warn the user if SSL certificate lacks revocation checking info

https://bugzilla.mozilla.org/show_bug.cgi?id=496661

I think these things are good ones getting fixed upstream. In which section in our design document would they fit?

In the Security Slider section description of Medium Settings mentions that media.webaudio.enabled is used, but it was removed from the Firefox codebase long ago.

That's actually not a spec bug (yet) as we are setting this on the medium level. That said, I opened #21601 to get that fixed.

Changed 3 years ago by mikeperry

Patch with some edits to GK's branch, and an XXX question about describing the threat model issue with speculative connections.

comment:53 Changed 3 years ago by mikeperry

Ok GK, I did a review pass. I made some minor grammar changes and other updates.

Please also see the XXX about speculative connections though. I'm not sure blocking them actually derives from a design requirement. It's fine we did it, but we should make the reasoning clear, and not just specify "MUST" it as if it flows from our security and privacy requirements.

comment:54 in reply to:  53 Changed 3 years ago by gk

Status: needs_reviewneeds_information

Replying to mikeperry:

Ok GK, I did a review pass. I made some minor grammar changes and other updates.

Thanks!

Please also see the XXX about speculative connections though. I'm not sure blocking them actually derives from a design requirement. It's fine we did it, but we should make the reasoning clear, and not just specify "MUST" it as if it flows from our security and privacy requirements.

Hm. I am a bit unsure whether I understand your issue here. Blocking them does not derive from a design requirement. But doing so is not specified as a MUST requirement either. The requirement says

Speculative connections MUST be *isolated* [emphasize mine, G.K.] to the URL bar domain.

And that seems to be well within the scope of our design requirements, especially if one has speculative connections in mind that originate from embedded link-tags.

Firefox does not send those requests by default if a proxy is configured. We don't bother with that and add only a defense-in-depth patch that actually would do the isolation to the url bar domain in case Mozilla changed their mind or disabling those connections would be buggy.

So, maybe we'd just need to reword that paragraph to make it less confusing?

comment:55 Changed 3 years ago by gk

Keywords: GeorgKoppen201703 added; GeorgKoppen201702 removed

Moving my tickets.

Changed 3 years ago by mikeperry

Describe speculative connection handling more clearly.

comment:56 Changed 3 years ago by mikeperry

Ok, I attached a second follow-on patch that updates the speculative connect subsection. It gives what I hope is a more clear description of what we do and why we do it. It is written assuming that rel=prefetch and rel=prerender are also properly isolated for cache and other state via the existing rel=preconnect patch. If the investigation in #21657 finds this not to be the case, then we need to either fix it, or disable prefetching and prerendering and update this patch to say so.

Leaving this as needs_information until #21657 is resolved for that reason.

comment:57 in reply to:  56 Changed 3 years ago by gk

Status: needs_informationneeds_revision

Replying to mikeperry:

Ok, I attached a second follow-on patch that updates the speculative connect subsection. It gives what I hope is a more clear description of what we do and why we do it. It is written assuming that rel=prefetch and rel=prerender are also properly isolated for cache and other state via the existing rel=preconnect patch. If the investigation in #21657 finds this not to be the case, then we need to either fix it, or disable prefetching and prerendering and update this patch to say so.

Leaving this as needs_information until #21657 is resolved for that reason.

Explicit preconnects via the <command>rel</command> attribute are still performed, however.

This is muddying the waters because rel="preconnect" things are not performed even if they are explicit. You linked to the bug in the sentence before the quote which is explaining the findings. The terminal output indicating that there is indeed a connection is lying as those preconnect things are cancelled at a deeper level.

An easy way to fix this is by picking up the distincion you made by adding "and prefetched" into the section caption:

Prefetches via the <command>rel</command> attribute are still performed, however.

This would make it clear that we are making a somewhat similar distinction to the spec which has resource hint links and speculative fetches (confusingly, though, prefetch and prerender are mentioned as example of the former as well (section 2) but looking at the spec further down it seems to talk only about speculative fetches in relation to them).

Marking this second fixup as needs_revision.

Changed 3 years ago by mikeperry

Third patch in the series, with updates based on testing results (applies on top of the other two).

comment:58 Changed 3 years ago by mikeperry

Status: needs_revisionneeds_review

Alright, I think this latest update clarifies and correctly captures the distinction in how prefetch, preconnect, and speculative connect are all handled?

comment:59 in reply to:  58 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to mikeperry:

Alright, I think this latest update clarifies and correctly captures the distinction in how prefetch, preconnect, and speculative connect are all handled?

Almost. :) The patch you link to is not for rel="prefetch" but for rel="preconnect" as the commit message says. In fact, there is a totally distinct nsPrefetchService that is responsible for rel="prefetch" and using the "normal" means of channel creation (see: https://dxr.mozilla.org/mozilla-esr45/source/uriloader/prefetch/nsPrefetchService.cpp#90) which we took care of before dealing with rel="preconnect". I tried to capture that with the following diff:

-the same mechanism as is used for HTTP Keep-alive. For rel="prefetch", we
-isolate them <ulink
+the same mechanism as is used for HTTP Keep-alive. This is true for rel="prefetch"
+requests as well. For rel="preconnect", we isolate them <ulink

I think that is okay and I have squashed my resulting branch into bug_15988_v4 (containing updates to the tor browser links and a version bump to 6.5.1, too).

This is at the same time commit 38086eefafcd63e8e3399534211feea55798fcbe on master.

If there is something I messed up of left, please file follow-up bugs.

Note: See TracTickets for help on using tickets.