#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
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs