Opened 7 years ago

Closed 7 years ago

#7706 closed enhancement (implemented)

Excluded Tor nodes are still being used when their "Country" location field is "?"

Reported by: bugcatcher Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version: Tor: 0.2.4.6-alpha
Severity: Keywords: GeoIP, relay, security, vulnerability tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I noticed a potential security exploit affecting the latest Tor 0.2.4.6-alpha Qt 4.8.1 (Linux 64-bit) and possibly other versions.

Background:
When the ExcludeNodes (or ExcludeExitNodes, etc.) directive is set in the torc file to avoid the relays located in the entire undesirable countries (for example, ExcludeNodes {RU},{US}), Tor relies on the GeoIP database to determine which nodes must be excluded from the circuits.

Problem:
Currently about 50-55 relays somehow hide their country attribute, or else the GeoIP database fails to identify their location. Yet Tor includes such nodes in the circuits, and thus they bypass the exclusion. It's a potential security exploit.

Fix needed:
The relays with an un-indentified country attribute MUST NOT be allowed in the Tor's circuit pool.

This problem can be visually observed in the Vidalia's Network Map window. Here are the steps to reproduce the problem:

  1. Add 'ExcludeNodes {US},{RU}' to the torc file (this is an example, since many of the questioned nodes are in fact originating in the mentioned countries) .
  1. Open the Vidalia's Network Map window and look at the list of the relays on left. While most relays correctly display their location country flag, Some relays have a question mark instead of the country flag. When they are selected, the detail info section in the lower middle doesn't show their country info (unlike the normal relays).

To see all of these questionable relays better, sort the listed relays by country - click on the 2nd tab above the relay list, and they will be grouped at the top or at the bottom of list. There are typically 50-55 of these relays.

  1. Over time, some of these relays without the listed country will be randomly included in the built circuits. I see it often when many developed countries (US, UK, etc.) are excluded. The more countries are excluded, the faster it will happen.

Suspicious relay behavior:
I observed these "un-countried" relays over a few days. Most of them remain present and unchanged.
However, a few unique relays without the country attribute are added each day, and a few disappear.

Interestingly, they seem to originate from the same IP address segments. Mostly it is the 5.xxx.xxx.xxx, 89., 91., 142., 192., 194., 198. and 199. IP segments.

Some relays share the same name (Unnamed), so it's hard to track them.

One of the relays ($EDFCBC44226B6DE6B28AFDEA6C8C63A3F5050665 [199.254.110.16]) only keeps changing its letter name (I don't see it today, though).

Please address the vulnerability as soon as you can - I'm tired of excluding 3 new individual fingerprints daily. :-)

Thank you!

Child Tickets

Change History (11)

comment:1 Changed 7 years ago by nickm

Component: - Select a componentTor
Keywords: tor-client added

You should be able to exclude nodes with unknown country by using the country code "??", as in "ExcludeNodes {us},{ru},{??}". If that doesn't work for excluding unknown countries, that's a bug.

Some nodes have been getting stuffed into the virtual "A1" country code by maxmind. That should be fixed by our fix to #6626, which should go out in the next version of Tor.

comment:2 Changed 7 years ago by nickm

I wonder if the new default behavior should be to consider all ?? nodes as excluded whenever any country is excluded. The current behavior could confuse people. We'd want a way to turn it off, though.

comment:3 Changed 7 years ago by bugcatcher

Summary: Excluded Tor nodes are still being used by hiding or altering their "Country" location fieldExcluded Tor nodes are still being used when their "Country" location field is "?"

The {??} and {A1} country codes - are they documented somewhere? All I've seen so far in the Tor Documentation is that torrc uses the country codes according to ISO 3166-1 alpha, and there wasn't any mention of {??} or {A1}.

Since they are not widely documented, then - Yes, definitely, the relays with {??} and {A1} must be disallowed either completely, or at least when any ExcludeNode command is used in torrc.

Until then a potential attacker can setup a node in a way that defeats the country exclusion setting.

Thanks.

comment:4 Changed 7 years ago by nickm

Status: newneeds_review
Type: defectenhancement

Implemented in the branch "feature7706" in my public git repository (see https://gitweb.torproject.org/nickm/tor.git) . That was relatively simple! Please review.

comment:5 Changed 7 years ago by nickm

I just tweaked the manpage a little more. Still needs review.

comment:6 Changed 7 years ago by nickm

Velope points out that we should have a look at the discussion on #1094 here too.

comment:7 Changed 7 years ago by andrea

The routerset_add_unknown_ccs() function and config file changes look fine to me. Am I correct in the belief that options->ExcludeExitNodesUnion_ should always be maintained as the union of options->ExcludeNodes and options->ExcludeExitNodes? If so, then I think the update on that is fine too.

Re: 1094, I think the choice of default behavior to exclude unknowns if the user wants any country excluded, but include an option to change it, is probably the right thing.

comment:8 Changed 7 years ago by nickm

Am I correct in the belief that options->ExcludeExitNodesUnion_ should always be maintained as the union of options->ExcludeNodes and options->ExcludeExitNodes?

That's the intended invariant, yes.

Re: 1094, I think the choice of default behavior to exclude unknowns if the user wants any country excluded, but include an option to change it, is probably the right thing.

I tend to agree. Right now, there are 7 nodes in ??; so unless our geoip coverage degrades a lot, it's not much hardship to exclude {??} whenever a user says "ExcludeNodes {tv},{nu},{pn}".

comment:9 Changed 7 years ago by nickm

(I'm going to leave this ticket open for a bit in case somebody wants to explain to me why I'm wrong about the above)

comment:10 in reply to:  7 ; Changed 7 years ago by arma

Replying to andrea:

I think the choice of default behavior to exclude unknowns if the user wants any country excluded, but include an option to change it, is probably the right thing.

Sounds fine to me (mostly because I think people who exclude by country are generally doing a foolish thing, and so I don't much care what happens to them).

We should not fool ourselves into thinking we have resolved a big security issue though: if an attacker wants to put his relay into a particular country, the geoip db's are pretty easy to game.

comment:11 in reply to:  10 Changed 7 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Replying to arma:

Replying to andrea:

I think the choice of default behavior to exclude unknowns if the user wants any country excluded, but include an option to change it, is probably the right thing.

Sounds fine to me (mostly because I think people who exclude by country are generally doing a foolish thing, and so I don't much care what happens to them).

Merged into master; thanks for having a look!

We should not fool ourselves into thinking we have resolved a big security issue though: if an attacker wants to put his relay into a particular country, the geoip db's are pretty easy to game.

I concur; this is more a UI matter and a least-surprise matter than a security fix.

Note: See TracTickets for help on using tickets.