Opened 6 years ago

Last modified 15 months ago

#3555 reopened enhancement

Pin *.torproject.org's certs in TBB

Reported by: tagnaq Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-firefox-patch
Cc: rransom, torbox@…, adrelanos@…, intrigeri@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

TBB should do a hardcoded check for the *.tpo certificate to prevent MITM attacks.

Mozilla does the same for their update mechanism (addons and browser).

https://lists.torproject.org/pipermail/tor-talk/2011-July/020784.html

Child Tickets

Change History (32)

comment:1 Changed 6 years ago by mikeperry

We could also sign the Torbutton update.rdf with https://wiki.mozilla.org/McCoy.. It is GUI only though, so it would mean torbutton release xpi creation can't be fully scripted...

comment:2 Changed 6 years ago by mikeperry

Milestone: TorBrowserBundle 2.2.x-stable

There are no command line versions of McCoy. We should just include the www.torproject.org cert like mozilla does for addons.mozilla.org.

comment:3 Changed 6 years ago by erinn

Owner: changed from erinn to mikeperry
Status: newassigned

comment:4 Changed 6 years ago by mikeperry

Points: 1

comment:5 Changed 6 years ago by mikeperry

Keywords: MikePerryIteration20110828 added

comment:6 Changed 6 years ago by mikeperry

Priority: normalmajor

comment:7 Changed 6 years ago by mikeperry

Bleh, the only reference I can find for this in the Firefox source is the pref app.update.certs.1.commonName. The pref is used in ./toolkit/mozapps/update/nsUpdateService.js seems to only be used to check the common name in the Checker.onLoad handler via CertUtils.checkCert.

The checkCert function does some additional checks to make sure the channel is using a built-in cert, but I still don't see where in the source distribution this builtin lives.

It also seems to say that we can't just include our cert, even if we wanted. We must also cause this checkCert to get called for our addon updates, otherwise the adversary could use a CA independent of our builtin and it will still work..

comment:8 Changed 6 years ago by mikeperry

Keywords: MikePerryIteration20110911 added; MikePerryIteration20110828 removed

comment:9 Changed 6 years ago by rransom

Priority: majorblocker

comment:11 Changed 6 years ago by mikeperry

Cc: rransom added

I'm reading the code closer, and it appears that onLoad is only "pinning" the common name, the issuer string, and the builtin property. This will at least pin them to a specific builtin CA (I think), but it can't pin a specific cert.

It also doesn't apply to addon updates: only Firefox updates. Addon updates are handled elsewhere.

Sorry, rransom, but I think we're going to have to wait for proper pinning support in Firefox to be able to do this for our addon updates. :(

Either that, or we begin signing the Torbutton XPI with that horrid McCoy tool?

comment:12 Changed 6 years ago by mikeperry

Keywords: MikePerryIteration20110911 removed
Points: 1

This is not 1 point. Need to decide if McCoy signing is worth the pain, or if we should just cross our fingers and pray for real pinning.

comment:13 Changed 6 years ago by mikeperry

Priority: blockermajor

comment:14 Changed 6 years ago by mikeperry

Milestone: TorBrowserBundle 2.2.x-stableTorBrowserBundle 2.3.x-stable

comment:15 Changed 6 years ago by mikeperry

Component: Tor bundles/installationTor Browser

This will require a Firefox patch. I think #5092 is the way to go, though.

comment:16 Changed 5 years ago by tagnaq

Hi Mike,

What is the current status on this ticket?
I still think that cert pinning is very important.
#5092 mitigates a smaller subset of possible MITM attacks on *.tpo.

comment:17 Changed 5 years ago by mikeperry

Milestone: TorBrowserBundle 2.3.x-stable
Type: defectenhancement

For us, it's now lower priority. Pinning should be provided by the actual upstream browser makers. Doing it ourselves for all *.tpo is complicated by the pinning system in Firefox being done through cert-specific and use-case specific haxx, and not a generalized mechanism (unless that's changed).

Chrome, for example, properly pins *.tpo through a generalized mechanism that is easy to alter+extend for arbitrary certs. We should get Mozilla to do it that way too, then we can think about adding our own certs to that mechanism in Tor Browser.

comment:18 Changed 5 years ago by runa

Did we ask Mozilla to pin when Chrome did it a while back?

comment:19 Changed 5 years ago by proper

Cc: torbox@… added

I can't agree with "low priority". *.tpo had been spoofed in past by compromised CA already.

If they takeover check.tpo, a site which every Tor user will see each time he starts Tor Browser, and advise users to install some malware, that's something, isn't it?

comment:20 Changed 5 years ago by proper

Cc: adrelanos@… added

comment:21 Changed 5 years ago by intrigeri

Cc: intrigeri@… added

comment:22 Changed 5 years ago by mikeperry

Owner: changed from mikeperry to cyperpunks
Priority: majornormal
Summary: TBB: hardcode SSL cert check to prevent MITMPin *.torproject.org's certs in TBB

The reality is that if Mozilla doesn't support cert pinning for arbitrary requests (beyond just updates+addons: see https://wiki.mozilla.org/Security/Features/CA_pinning_functionality), I'm not going to implement this. Until that point, our priority is not "major" for this ticket.

If you feel otherwise, you must write a patch to implement pinning before arguing over the priority of this ticket, or point me to the actual Firefox pinning implementation.

comment:23 Changed 3 years ago by erinn

Keywords: tbb-firefox-patch added

comment:24 Changed 3 years ago by erinn

Component: Firefox Patch IssuesTor Browser

comment:25 Changed 3 years ago by vynX

If the torproject.org sites were available as hidden services then the self-authenticating feature of public-key addresses would obsolete the need to pin any certificates.

comment:26 in reply to:  25 Changed 3 years ago by arma

Replying to vynX:

If the torproject.org sites were available as hidden services then the self-authenticating feature of public-key addresses would obsolete the need to pin any certificates.

I like where you're trying to go with this, but it is alas wrong. It assumes that somehow everybody knows the right onion names for each service. And then we're back to a very similar problem.

But more generally, it is not useful to get into a discussion here about what security properties onion services get. The previous comments here make this look like we should close as a wont-fix.

comment:27 Changed 3 years ago by vynX

Arma, we are talking about Tor Browser the way it gets shipped here, it should know its own onions and it doesn't take any extra certificate pinning implementation to know them – it is enough to have the right .onion names in all of the default URLs that are shipped with the browser. So the problem of users not being able to memorize onion addresses except for Facebook's and mine, with all due respect, doesn't exist in this constellation. Also I know of trusted Tor developers owning powerful multi-processor machines in their trusted homes capable of generating some really neat and memorizable torproject12core.onion address, but that isn't even necessary to complete this feature request – or rather, security bugfix, since it isn't neither nice nor logical that even the Tor Browser is susceptible to MITM when accessing its own homebase.

Last edited 3 years ago by vynX (previous) (diff)

comment:28 Changed 15 months ago by bugzilla

Owner: changed from cyperpunks to tbb-team
Severity: Normal

Despite the recent fail, this ticket can be closed as implemented (since FF34).
https://bugzilla.mozilla.org/show_bug.cgi?id=1004353

Last edited 15 months ago by bugzilla (previous) (diff)

comment:29 Changed 15 months ago by gk

Resolution: fixed
Status: assignedclosed

Indeed. This got implemented upstream.

comment:30 Changed 15 months ago by yawning

Resolution: fixed
Status: closedreopened

No. aus1.torproject.org is not pinned. Unless we don't care about just the alpha/hardened channels update metadata information.

comment:31 in reply to:  30 Changed 15 months ago by cypherpunks

Replying to yawning:

No. aus1.torproject.org is not pinned. Unless we don't care about just the alpha/hardened channels update metadata information.

Indeed, and this sounds like mistakenly pinned/missed subdomains:
torproject.org
"torproject.org", false, false, false, -1, &kPinset_tor, doesn't include subdomains
www.torproject.org
"www.torproject.org", true, false, false, -1, &kPinset_tor, do include subdomains

comment:32 in reply to:  30 Changed 15 months ago by bugzilla

Replying to yawning:

No. aus1.torproject.org is not pinned. Unless we don't care about just the alpha/hardened channels update metadata information.

Hmm, neverending ticket? (gk doesn't like when closed tickets are reopened for regressions)
Have you read all Mike's comments here?
This story ended and got upstreamed (yes, the summary is talking about something else).
As you can see, *.tpo was not pinned entirely (despite it's easier), so there were reasons to create this special handling. And for your "regression" there is #20180.

But it's good to mention that thoughts discussed in this ticket about creating your own pinning infrastructure instead of relying of FF's pinsets are worth to be addressed in a new proposal.

Note: See TracTickets for help on using tickets.