[tor-bugs] #24509 [Core Tor/Tor]: circuit_can_use_tap() should only allow TAP for v2 onion services

[Tor Bug Tracker & Wiki]

#24509: circuit_can_use_tap() should only allow TAP for v2 onion services
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  (none)
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.3.2.1-alpha
 Severity:  Normal                               |     Resolution:
 Keywords:  prop224, tor-hs, security-low,       |  Actual Points:
  easy, intro                                    |
Parent ID:                                       |         Points:  0.5
 Reviewer:  dgoulet                              |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision
 * reviewer:   => dgoulet


Comment:

 Ok the check is the way to do it for now.

 In `circuit_can_use_tap()`, I might suggest you to return the result of
 the 3 conditions instead of this if(2 cond) then return 1 cond.

 {{{
   return circuit_purpose_can_use_tap_impl(circ->base_.purpose) &&
          extend_info_supports_tap(circ->cpath->extend_info)) &&
          return circuit_rend_version_can_use_tap_impl(circ);
 }}}

 Final thing, may I suggest also a rename of
 `circuit_rend_version_can_use_tap_impl()` to `hs_circuit_is_v2()` or
 something around those lines that you can put in `hs_circuit.c` to enclose
 anything related to HS into its subsystem?

 The reason here is that this new function is really returning the circuit
 HS version, not if it is supporting tap per-se. With this we'll also win
 some extra semantic in the code which clearly shows that only HS v2
 circuit *can* do tap. And because `circuit_purpose_can_use_tap_impl()` is
 tested both the purpose of the circuit and the version needs to match for
 the tap to be supported.

 Thanks!

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