[tor-bugs] #21425 [Core Tor/Tor]: entry_list_is_constrained() should look at the guard_selection_t object

[Tor Bug Tracker & Wiki]

#21425: entry_list_is_constrained() should look at the guard_selection_t object
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  (none)
     Type:  defect                               |         Status:
                                                 |  needs_information
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  guards, 031-deferred-20170425,       |  Actual Points:
  review-group-18                                |
Parent ID:  #20822                               |         Points:  .5
 Reviewer:  asn                                  |        Sponsor:
                                                 |  SponsorV-can
-------------------------------------------------+-------------------------
Changes (by asn):

 * status:  needs_revision => needs_information


Comment:

 Replying to [comment:24 teor]:
 > Replying to [comment:23 asn]:
 > > 1) What are we trying to achieve with this new
 `entry_list_is_constrained()` logic? Can someone explain to me why we
 consider the entry list constrained if it contains more than 50 guards,
 and why this logic makes sense given the way `entry_list_is_constrained()`
 is used in the rest of the code? I think this is a very important question
 and ideally we should have a clear and concise answer :)
 >
 > We check guards more aggressively when entry list is constrained,
 > There's no point checking guards more aggressively if we have a lot of
 potential guards.

 Hmm, as arma said in comment:2, this function has been used in the past to
 query whether our guard list is fixed, and not whether it contains lots of
 elements. Said otherwise, whether for each circuit we go back to the
 consensus and get a new random guard (''not constrained''), or we will
 just keep on trying the same ones all the time (''constrained'').

 Furthermore, the general approach in this ticket has been to look at the
 guard sample size (which is always between 20 and 60: see
 `get_max_sample_size()`) and denotes how many guards we have sampled in
 the lifetime of the Tor state file. It does not denote how many of those
 guards are currently reachable or usable. That's bad since any Tor state
 file has lived long enough will have accumulated 60 sampled guards (e.g.
 all of my state files) and considered '''not constrained'''. However,
 those Tors are definitely constrained since we don't know how many of
 those 60 sampled guards are currently reachable (in prop#271 terms belong
 to `USABLE_FILTERED_GUARDS` set): it could be that only 2-3 of them are
 currently usable. And even if many of them are reachable, Tor will
 constrain itself by persistently trying the primaries and confirmed guards
 before falling back to the rest of its list.

 In any case, we should try to understand what we are trying to gain with
 this ticket because it's not clear to me currently. Perhaps this ticket is
 a red herring and we should WONTFIX this, or perhaps switch from checking
 the sampled set to the usable filtered set, but we should think some more
 here.

 That's not helped by the fact that we have two uses for
 `entry_list_is_constrained()` and they are both kinda arbitrary:
 - In `circuit_get_open_circ_or_launch()`, if we lack dirinfo and the
 guards '''are constrained''' then we retry all our guards again. I guess
 that's needed in this case so that we don't run out of guards to try if
 our list is constrained.
 - In `connection_dir_request_failed()`, where if our guards '''are not
 constrained''' and we just failed a dir request due to network error, we
 mark that guard as down.

 Maybe Nick remembers what we wanted to gain with this ticket?

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