[tor-bugs] #13837 [Core Tor/Tor]: Mitigate guard discovery by pinning middle node

[Tor Bug Tracker & Wiki]

#13837: Mitigate guard discovery by pinning middle node
-------------------------------------------------+-------------------------
 Reporter:  asn                                  |          Owner:
                                                 |  mikeperry
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-hs, tor-guard, guard-discovery-  |  Actual Points:
  prop247-controller                             |
Parent ID:  #9001                                |         Points:
 Reviewer:  asn                                  |        Sponsor:
                                                 |  SponsorV-can
-------------------------------------------------+-------------------------

Comment (by mikeperry):

 Replying to [comment:28 asn]:
 > I encountered some annoying failures during testing this which I
 reported here:
 https://oniongit.eu/mikeperry/tor/commit/7e962536f2d89ab0e2b8dd8821503ed66bd115ac#note_1804

 I am pretty sure this is two separate bugs that are orthogonal to this
 code:

 First, we are failing to find a second or third hop for the path because
 you specified an IP network mask  in HSLayer2Guards and HSLayer3Guards. It
 seems that routersets have a bug/quirk in their network mask handling. See
 routerset_contains(). They only return "true" for address range checks if
 the match REJECTED the specified address. If I change that
 routerset_contains() check to return true if the match is ACCEPTED, the
 very same netmasks suddenly work. However, if I just patch that
 routerset_contains function, disparate things that use routersets like
 excludenodes and exitpolicies suddenly break (in fact, about 12 unittests
 fail when I change this).

 Since we don't need network masks for our usecase, one option is to simply
 forbid their use for these options. I worry that if we try to do anything
 complicated than we absolutely need here, we will miss the 0.3.3 merge
 deadline. Is there something easier/saner we could do here?

 Second, I don't think that changing circuit_build_failed() to ignore guard
 failures if circ->n_chan is NULL is correct. If you can't connect to a
 guard at all in other circumstances, that should still count as a failure.
 One thing we could do, though, is add a special check in
 circuit_build_failed() to not fault the guard node if vanguards are in use
 *and* we know that the vanguard failed because none of the specified
 vanguards are available. I can try to do this in a fixup, at least.

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