[tor-bugs] #24392 [Core Tor/Tor]: Ignore cached bridge descriptors until we check if they are running

[Tor Bug Tracker & Wiki]

#24392: Ignore cached bridge descriptors until we check if they are running
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  teor
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.2.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.3.0.1-alpha
 Severity:  Normal                               |     Resolution:
 Keywords:  030-backport, 031-backport,          |  Actual Points:  0.5
  regression, tor-bridge-client, s8-errors,      |
  review-group-27                                |
Parent ID:  #24367                               |         Points:  0.3
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by nickm):

 Hi! I have some questions, that are not necessarily blockers.

 In the new commit bca43d548ad7301fc3698e8ded0a94d43a41858d:
 {{{
 -    int first = num_bridges_usable() <= 1;
 +    /* Retry directory downloads whenever we get a bridge descriptor:
 +     * - when bootstrapping, and
 +     * - when we aren't sure if any of our bridges are reachable.
 +     * Keep on retrying until we have at least one reachable bridge. */
 +    int first = num_bridges_usable(0) < 1;
 }}}

 Do we know why this was `<=` before?  It looks like I introduced the code
 in bca43d548ad7301fc3698e8ded0a94d43a41858d, but I don't understand what
 my original rationale was, so I'm a little uncertain here.  I'll try to
 see if I can puzzle this out.

 Second question: In e9cb0cd7c8fe03dbd7e0fef91ab868cb56f280b0, when we do
 {{{
 -      } else if (!options->UseBridges || num_bridges_usable() > 0) {
 +      } else {
 }}}
 can we add a `tor_nonfatal_assert()` there to make sure that the second
 case really and truly only happens when we think it does?

 Last:
 > I am no longer convinced that this bug is restricted to 0.3.2, we should
 consider backporting these changes to 0.3.0 as a precautionary measure.

 This is a pretty complex set of changes, and the rebase isn't clean.  Is
 there a minimal set of changes that, we could put into a branch based on
 0.3.0?

 And final question: how is the testing on this?

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