[tor-bugs] #19877 [Core Tor/Tor]: Implement new guard selection algorithm of prop 271

[Tor Bug Tracker & Wiki]

#19877: Implement new guard selection algorithm of prop 271
-------------------------------------------------+-------------------------
 Reporter:  andrea                               |          Owner:  nickm
     Type:  task                                 |         Status:
                                                 |  assigned
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.0.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  unspecified
 Severity:  Normal                               |     Resolution:
 Keywords:  isaremoved, nickwants029, tor-       |  Actual Points:
  guards-revamp, TorCoreTeam201611               |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  SponsorU-must
-------------------------------------------------+-------------------------

Comment (by nickm):

 Replying to [comment:9 asn]:
 > Hello, I did a very rough initial review based on the WIP branch
 `prop271-wip`. Haven't run it yet. Great work so far, very excited about
 splitting bridge code to another file as well.
 >
 > Some of these comments might be too low level for this stage of
 implementation, so feel free to ignore them. Also, I have not re-loaded
 the whole proposal in my brain yet, so I didn't go too deep in the nitty
 gritty details.
 >
 > - There are a few big scary functions that could be splitted into
 smaller functions to make them more manageable. e.g.
 `sampled_guards_update_from_consensus()` and
 `entry_guards_update_primary`.
 >
 >   Also the fundamental function `select_entry_guard_for_circuit()` could
 be split into three smaller functions, each for one spec case.

 Ack.  I'll add a "XXXX can this split up" comment to those.

 > - Agreed that the `entry_guard_succeeded()` tristate is ugly and makes
 the `circuit_send_next_onion_skin()` logic harder to read. Perhaps the
 retval could be turned into an enum?

 Ack, will fix.

 > - If `add_bridge_as_entry_guard()` is a pub function of the entrynodes
 API, should it have a prefix? Or we are not at that level of tidyness yet?

 The entire bridge->entry interface is in flux; I hope we clean it up by
 the end.

 > - Why do we need the spaceless ISO time functions? Can't we use the
 spaceful ones for state? Or is it to maintain backwards compat?

 For the state format I chose, everything is spaceless.  That makes it much
 much easier.

 > - Parsing guards from state seems a bit of a pain now. For example,
 `entry_guard_parse_from_state()` does ad-hoc string parsing. Would it be
 possible to use the config parsing API for that?

 Oooh.  Interesting.  We'd have to make it recursive, so it can handle the
 values within a line rather than just handling lines.  Could be neat
 though.  Maybe as another ticket?

 > - Could `smartlist_remove_keeporder()` be implemented with
 `smartlist_pos()` and `smartlist_del_keeporder()` to avoid writing more
 smartlist code?

 Only in quadratic-time: smartlist_remove_keeporder() needs to remove every
 occurrence of the target element. So to emulate it with pos and
 del_keeporder, you'd need to say:
 {{{
     int idx;
     while ( (idx = smartlist_pos(sl, element)) >= 0) {
       smartlist_del_keeporder(sl, idx);
     }
 }}}

 > Finally, how should one robustly test this branch? Do you use iptable
 scripts or something?

 I've been doing ad-hoc iptable stuff, doing some of my hacking from nasty
 coffeeshop ISPs, etc.  I haven't figured out how to do a really thorough
 hostile-network integration test though.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19877#comment:10>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online