[tor-bugs] #23817 [Core Tor/Tor]: Tor re-tries directory mirrors that it knows are missing microdescriptors

[Tor Bug Tracker & Wiki]

#23817: Tor re-tries directory mirrors that it knows are missing microdescriptors
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  (none)
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-guard, tor-hs, prop224,          |  Actual Points:
  032-backport? 031-backport? spec-change        |
Parent ID:  #21969                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by teor):

 Replying to [comment:15 nickm]:
 > Quick notes, not comprehensive:
 >   * The changes file doesn't say which version this is a bugfix on.
 (Does "make check" pass?)

 Some release early in the 0.3.0 series, probably 0.3.0.1-alpha.

 >   * Should this be a backport candidate for any earlier versions?

 0.3.0 and 0.3.1.

 >   * It looks like the first two commits on this branch are actually part
 of #23862, and already merged into 0.3.2?
 >   * Minor: The `relay_digest` argument to
 dir_microdesc_download_failed() would be better named "dir_id" or
 something, since it's the identity of the failing directory server?
 >   * Digestmap_t would be a better type to use for
 outdated_dirserver_list, but we can fix that later if need be.
 >   * Should entries on outdated_dirserver_list expire after some amount
 of time, in addition to just "whenever we get the next consensus?"

 This is potentially dangerous, because it could create slow zombies.
 If we do this, 1 hour (consensus interval), 3 hours (live consensus), or
 24 hours (reasonably live consensus) seem like reasonable expiry times,
 depending on when we stop fetching microdescs.

 >   * I wonder if we can think of a better name for this than "outdated
 dirserver" -- it's not outdated itself -- it just failed to give us
 microdescriptors for some consensus we had.

 It's a dirserver that has a consensus that we don't.

 >   * Do we check to make sure that the consensus is fairly recent before
 marking a dirserver as "outdated" in this way?

 We should check if our consensus is recent, and try to get a newer one.

 >   * Maybe we should make sure that we never mark an authority as
 outdated?

 Yes, I think this is #23863: rather than marking an authority outdated, we
 should mark the relay with the missing microdesc as dead.
 Until then, we should log a warning with the authority and specific
 microdesc id rather than marking the authority as dead.
 (Is it possible for an authority to be out of sync and not serve the
 latest microdescs? What should happen is that it should adopt the latest
 majority consensus, download its microdescs, and serve them.)

 > Adding teor in cc -- asn asked if you could review this patch too.  I'd
 like to merge it in the alpha coming out in a couple of days, if possible,
 though I understand you might not be able to review on that schedule.

 Referencing a trac comment is fragile, let's summarise the exponential
 backoff reasoning instead:
 {{{
 (see
 + * #23817 comment:12 for teor's argument)
 }}}
 Something like "since we use exponential backoff for directory fetches,
 clients should not fetch microdescs more than log2(3*3600) = 14 times per
 consensus. The backoff is random between last+1 and last*4, so we allow
 some extra fetches.".

 Would it help to have the log contain the missing microdesc IDs, or at
 least the first N of them?

 relay_is_outdated_dirserver should use smartlist_contains_string for
 consistency with relay_note_outdated_dirserver.

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