[tor-bugs] #21771 [Core Tor/Tor]: Cannot change bridge (with Tor 0.3.0.4-rc)

[Tor Bug Tracker & Wiki]

#21771: Cannot change bridge (with Tor 0.3.0.4-rc)
---------------------------------+------------------------------------
 Reporter:  torvlnt33r           |          Owner:  asn
     Type:  defect               |         Status:  assigned
 Priority:  Very High            |      Milestone:  Tor: 0.3.1.x-final
Component:  Core Tor/Tor         |        Version:  Tor: 0.3.0.4-rc
 Severity:  Normal               |     Resolution:
 Keywords:  bridge 030-backport  |  Actual Points:
Parent ID:                       |         Points:  1
 Reviewer:                       |        Sponsor:
---------------------------------+------------------------------------

Comment (by asn):

 OK here is some recon.

 It seems like we are continuously hitting the following code path with
 this bug:
 {{{
 [warn] Not expanding the guard sample any further; just hit the maximum
 sample threshold of 1
 }}}

 which brings us to the following code:
 {{{
   smartlist_t *eligible_guards = get_eligible_guards(options, gs,
 &n_guards);
 ...
   const int max_sample = get_max_sample_size(gs, n_guards);
 ...
     /* Has our sample grown too large to expand? */
     if (n_sampled >= max_sample) {
       log_warn(LD_GUARD, "Not expanding the guard sample any further; "
                "just hit the maximum sample threshold of %d",
                max_sample);
       goto done;
     }
 }}}

 with `get_max_sample_size()` like this:
 {{{
 get_max_sample_size(guard_selection_t *gs,
                     int n_guards)
 {
 ...
   /* With bridges, max_sample is "all of them" */
   if (using_bridges)
     return n_guards;
 }}}

 and `get_eligible_guards()` sets `n_guards` like this:
 {{{
   if (gs->type == GS_TYPE_BRIDGE) {
     const smartlist_t *bridges = bridge_list_get();
     SMARTLIST_FOREACH_BEGIN(bridges, bridge_info_t *, bridge) {
       ++n_guards;
       if (NULL != get_sampled_guard_for_bridge(gs, bridge)) {
         continue;
       }
       smartlist_add(eligible_guards, bridge);
     } SMARTLIST_FOREACH_END(bridge);
 }}}

 So in our case, because we swapped bridge X for bridge Y, the above code
 will only do a single smartlist loop (since only Y is in our bridge list),
 and it will set `n_guards` to 1.

 But since our sampled guards list already contains bridge X, tor will
 refuse to add bridge Y since we "just hit the maximum sample threshold of
 1".

 Some suggested ways forward:
 a) We could reconsider how `get_max_sample_size()` handles the bridge
 case.
 b) We could edit `get_eligible_guards()` so that it also takes into
 account our sampled guards and not just the size of our bridge list (i.e.
 how many bridges are in our torrc).

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