#22688: Make sure HSDir3s never know service, client, or bridge IP addresses -------------------------------------------------+------------------------- Reporter: teor | Owner: Type: defect | Status: | needs_revision Priority: Medium | Milestone: Tor: | 0.3.1.x-final Component: Core Tor/Tor | Version: Tor: | unspecified Severity: Normal | Resolution: Keywords: prop224, relay-safety, | Actual Points: 0.3 031-backport, maybe-030-backport-with-21406 | Parent ID: #17945 | Points: 0.3 Reviewer: | Sponsor: -------------------------------------------------+-------------------------
Comment (by teor): Replying to [comment:5 dgoulet]: > Some comments: > > * We should break this assert() in two different ones else if triggered, we won't know which condition triggered it: > > {{{ > + /* A clever compiler might complain this is always true */ > + tor_assert(TO_CONN(conn) && TO_CONN(conn)->linked); > }}} Agreed. > * How do we know that this is a `one-hop` circuit with this condition? > > {{{ > + /* Well, we won't be sending anything back on that, will we? > + * (Avoid giving the wrong answer because state has been reset.) */ > + if (TO_CONN(conn)->linked_conn_is_closed || > + !l_conn || l_conn->marked_for_close) { > + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, > + "Refusing %s one-hop encrypted directory connection.", > + TO_CONN(conn)->linked_conn_is_closed ? "closed linked" : > + !l_conn ? "NULL linked" : "marked for closed linked"); > + return 0; > + } > }}} > > Same goes with these condition later: > > {{{ > + if (BUG(!exitconn) || !exitconn->on_circuit) { > [...] > + if (BUG(!orcirc) || !orcirc->p_chan) { > }}} We don't. > * Would using `CIRCUIT_IS_ORCIRC()` me more appropriate here? > > {{{ > + /* We should always be using an OR circuit */ > + if (BUG(exitconn->on_circuit->purpose != CIRCUIT_PURPOSE_OR)) { > + return 0; > + } > }}} Agreed: I couldn't find it when I was writing the patch. > * I'm unclear on where this is checked? Maybe it's done through some indirect checks that I haven't spotted but is there a way you can know that with an `or_circuit_t` ? > > {{{ > + * For client circuits via relays, this is true for 2-hop or greater paths, > + * for client circuits via bridges, this is true for 3-hop or greater paths. > }}} This is checked at the end of the function using `!channel_is_client(p_chan)`. `channel_is_client()` is true if the channel is connected to a non- authenticated peer (something without a fingerprint). This can either be a client or a bridge. So to pass the `!channel_is_client(p_chan)` check: * a client can't connect directly, so a client has to have a 2-hop path through a relay, * a bridge can't connect directly, so a client has to have a 3-hop path through a bridge. Does that make sense? -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22688#comment:6> 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