[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:
                                                 |  merge_ready
 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):

 I am ok to merge this as-is, but let's think about the remaining issues in
 other tickets.

 Also, this comment is misleading:

 {{{
 /* Is our guard an outdated dirserver? */
 }}}

 It would be better as:

 {{{
 /* Last time we tried to fetch microdescs, was this directory mirror
 missing any we asked for? */
 }}}

 Replying to [comment:18 asn]:
 > OK, I pushed a branch `bug23817_v2` with the following changes:
 >
 > - It's now rebased on latest master (I will also prepare an 032 branch)

 Should we prepare an 030 branch instead/as well?

 > - Renames `relay_digest` to `dir_id` in `directory.c`.
 > - Adds a log message for failed mds.
 > - Does not add dirauths as outdated dirservers.

 Hmm, I wonder if a BUG() is the right thing to do here. It's probably ok
 in an alpha, but if a dirauth is out of sync, clients will log this as a
 BUG(), even though it's not their bug. (In most cases, if a dirauth is out
 of sync, that should self-correct, because it will be dropped from the
 consensus. Except for IPv6-only clients, which fetch mds from fallbacks
 and authorities so they can bootstrap.)

 In any case, this should be rare.

 > - Cleans up the outdated list if it grows above 30 elements.

 I think this is ok for the moment (it is no worse than the previous
 behaviour), but we should think about setting this limit low, and then
 trying an authority when we reach it (and then giving up on the md).
 That's a separate ticket - #23863.

 > A few notes:
 >
 > - I'm not checking if we have a recent consensus when we blacklist
 dirservers, as Nick suggested, which might actually be a problem. I think
 teor's suggestion of "check if our consensus is recent and try to get a
 newer one" is a whole project of its own that warrants its own ticket (or
 not?). What should we do here? Maybe we should not register outdated
 dirservers if we don't have a live consensus?

 Seems to make sense to me.

 > - teor, I'm not sure if I agree with your exponential backoff + list
 size argument, since IIUC the exponential backoff applies only when we
 fail to fetch the same md multiple times, whereas the outdated list can
 overgrow  just by failing many fetches for different mds. In any case, I
 opted for resetting the list after 30 arguments, which is a bit arbitrary
 but perhaps can save us from any surprising issues (?).

 I think there is a small risk of a client -> relay DDoS here, but it's no
 worse than the previous behaviour.

 > - I did not reset the list based on elapsed time because of teor's
 argument about clients fetching consensuses every 1 hour or so. Let me
 know if you don't like this.

 Seems fine to me.

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