[tor-bugs] #17882 [Core Tor/Tor]: Remove needless *_support_ntor()

[Tor Bug Tracker & Wiki]

#17882: Remove needless *_support_ntor()
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  (none)
     Type:  defect                               |         Status:  closed
 Priority:  Low                                  |      Milestone:  Tor:
                                                 |  unspecified
Component:  Core Tor/Tor                         |        Version:
 Severity:  Trivial                              |     Resolution:  wontfix
 Keywords:  easy tor-client code-removal         |  Actual Points:
  technical-debt                                 |
Parent ID:                                       |         Points:  small
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by teor):

 * status:  needs_review => closed
 * resolution:   => wontfix


Comment:

 Replying to [comment:8 irl]:
 > In `33da2abd05`, directory authorities began rejecting relays that did
 not publish a valid ntor key. This means that the consensus will only
 contain relays that do have ntor keys published in their descriptors.
 >
 > `circuit_cpath_supports_ntor` is indeed redundant and only duplicating
 the check that has already been performed by the directory authorities.

 This is not an appropriate justification for this change: we expect newer
 clients to work with older authorities and relays. This is important for
 private and test tor networks. Clients can't trust the authorities to
 check ntor support for them.

 However, recent client versions ignore descriptors that don't have an ntor
 key.

 But, we still need to check for the edge cases described in
 onion_populate_cpath().
 We will never be able to bootstrap using ntor, so we must keep this
 function.
 However, we can simplify it when we get rid of v2 onion services.

 > There is one other function in the `_supports_ntor` realm:
 `extend_info_supports_ntor`. dir-spec currently does not require ntor in
 extra info descriptors and so this is not yet removable. For example, v2
 hidden services still rely on the TAP handshake.

 This is about extend_info structures, not extra info descriptors.
 Also, bootstrapping relies on CREATE_FAST, not ntor.

 > The above patch removes `circuit_cpath_supports_ntor`. `make test`
 passes and I have tested client functionality for bootstrapping, "normal"
 circuits, v2 hidden service circuits and v3 hidden service circuits. It
 also updates a comment in `dirserv.c` to explain that clients will now
 assume ntor support, where it previously said clients would check.

 The patch removes two important security checks:
 * we must not use TAP or CREATE_FAST for multi-hop paths, unless those
 paths are onion service paths, and
 * we must not use TAP or CREATE_FAST for single-hop paths, unless we don't
 have the ntor key for the node.

 I'm sorry, but I don't think we should accept this patch or remove this
 function.

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