[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:  dgoulet
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.4.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:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by dgoulet):

 * milestone:  Tor: 0.3.3.x-final => Tor: 0.3.4.x-final


Comment:

 Ok honestly, what I'm worried about here (for 033) is the size of the
 change touching many layers just to have this defense in depth. I'm worry
 to break things or even introduce bugs in a version that we are currently
 stabilizing.

 But this ticket also imo highlights a problem we have in tor generally
 which is that we don't have a good way to identify what a circuit is for
 exactly. For HS, because we have two versions now, the `purpose` is not
 enough. Even for a normal client, everything is `GENERAL` purpose, we
 don't know if it is for fetching a descriptor or a consensus or an HS
 descriptor which makes us look at the connection object to know that but
 that isn't set until the circuit is launched...

 And being unable to distinguish HS version is annoying actually. In
 theory, we would look at `rend_data_t` and `hs_ident_t` that both have a
 version field that was put there so when the circuit layer calls back in
 the HS subsystem, we do know what it is for. This highlights another thing
 which is that we launch circuit _before_ they are ever setup with per-
 subsystem metadata (which can be OK imo).

 Because we set the HS data after, it means we need to pass metadata to the
 circuit layer to tell it how we want the circuit. And that layer imo
 should remain agnostic of the "why of the circuit". The subsystem that
 requests the circuit should be the one with insane amount of checks and
 passing down the details on how it wants it constructed.

 If we tell the circuit layer "what the circuit is for" so that it can make
 decision itself (for which I'm not convinced it should but it is already
 doing A LOT anyway right now), it means we should think of a proper way to
 pass that information down to that subsystem. Circuit flags? More metadata
 in `extend_info_t`? ...

 If we want to go that way that is "hey circuit layer, I want a HSv2 client
 circuit for rendezvous", I would be much happier if we could get in a nice
 interface to identify which circuit it is for exactly (origin circuit),
 have it tested and then we can just hook on that for this defense.

 But adding a circuit flag for "v2 circuit creation" and passing it down
 layers or even having a version parameter in `extend_info_t` are hacks
 that aren't very sustainable options over time.

 For the record, I'm strongly in favor of keeping the circuit subsystem
 agnostic here and having a clear separation of work between the circuit
 layer and the layer that wants a circuit from it. Overlapping checks and
 validation like that imo leads to spaghetti code and it is error prone.
 And we already have much infrastructure in place for this that is the
 `extend_info_t` object and the `CIRCLAUNCH_` flags.

 I'm happy to open a ticket for this kind of idea but for now I'm pushing
 this to 034.

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