Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#2798 closed defect (fixed)

Special-case country codes and address set patterns in EntryNodes

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Right now, Tor only lets you specify EntryNodes verbatim: you can say "I want my entry nodes to be $12345 and FriendlyNode1", but you can't say "I want my entry nodes to be in {de} and {gb} and 18.0.0.0/8."

This limitation exists because of how Tor implements EntryNodes: it set your guard nodes equal to the set of all members of EntryNodes. This make sense if EntryNodes is a pure list of nodes, but when it contains stuff like {de} and {gb}, that probably isn't what you want. Instead, you want your guard nodes to be selected from among the set of EntryNodes.

Child Tickets

Change History (10)

comment:1 Changed 8 years ago by arma

I did this in September 2010 in git 0ed8d5a537.

Except on closer inspection, it isn't what you describe above. Currently we just add all the nodes in Germany to your entry guard set.

I guess you're suggesting that we should special-case the {xx} country-level way of specifying a routerset, and have Tor use entry guards like before but only choose from those routers.

But in this case we actually *do* do what you want. We put all the nodes in germany into your entry guard list, and then pick the first k of them:

      if (smartlist_len(live_entry_guards) >= options->NumEntryGuards)
        break; /* we have enough */

But we don't randomize them first. So everybody who sets entrynodes {de} based on the same consensus will use the same subset of german relays as guards? Hm.

comment:2 Changed 8 years ago by arma

Summary: Add country code and address set pattern support to EntryNodesSpecial-case country codes and address set patterns in EntryNodes
Type: enhancementdefect

comment:3 Changed 8 years ago by arma

(Somewhere during this process, after #1090 is merged in, we will want to fix the dev man page to say something smarter about what we allow in 0.2.3.)

comment:4 Changed 8 years ago by arma

Priority: normalmajor

Marking major, because we'd best do *something* here for 0.2.3 (either fix it one way or fix it another)

comment:5 Changed 8 years ago by nickm

Status: newneeds_review

So, I think the right solution is this:

  • Revise entry_guards_prepend_from_config() to only add a random subset of the nodes in EntryList, if that list is too long.
  • Revise entry_guards_prepend_from_config() to try to add only the best nodes from EntryNodes, if that's possible. (IOW, don't ignore Guard.)
  • Maybe, revise entry_is_live / entry_set_status so that they don't make quite the same exceptions when EntryGuards is set with a big big list.

I've done the first two in a "bug2798" branch in my public repo. Comments & reviews much appreciated.

comment:6 Changed 8 years ago by arma

It looks like entry_guards_prepend_from_config() no longer prepends from config? It might want a name change then. I guess its new behavior is more like 'append' than 'prepend'.

I'd call this a major bugfix, since in 0.2.3.1 we did some pretty dangerous things to your entry guard behavior if you set entrynodes {de}.

The 'entry_routers' term remains in the comment even though the variable has been renamed for a while.

s/reset/rest/

   smartlist_clear(entry_guards);
   /* First, the previously configured guards that are in EntryNodes. */
   smartlist_add_all(entry_guards, old_entry_guards_on_list);
+  /* Next, scramble the reset of EntryNodes, putting the guards first. */
+  smartlist_shuffle(entry_nodes);

So it looks like we're reordering the list of guards every time we get a new consensus? It might be smarter to leave the previous set in order.

comment:7 Changed 8 years ago by nickm

22:55 < nickm> We aren't scrambling the list.
22:56 < nickm> The entry_guards list remains in the same order it was in before.
22:56 < nickm> it's only things that we add from entry_nodes that gets shuffled.
22:57 < armadev> ah ha. entry_nodes is the local var and entry_guards is the 
                 global var
22:57 < armadev> that is not entirely intuitive :)
22:57 < nickm> indeed not
22:57 < armadev>       log_notice(LD_GENERAL, "Dropping node: fascist 
                 firewall");
22:58 < armadev> i wonder if you mean to leave this on notice
22:58 < nickm> I don't believe so.
22:58 < armadev> ok. looks good overall
22:59 < nickm> oh hey, and that xxxx023 comment can die now
23:00 < armadev> true
23:00 < nickm> mind if I copy and paste the above?
23:00 < armadev> go for it

Updated my branch.

comment:8 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

armadev likes. Merging!

comment:9 Changed 7 years ago by nickm

Keywords: tor-client added

comment:10 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.