Circuits through entry guards aren't distinguished on whether they originated locally
When a node is both a relay and a client, and another node extends a circuit through it to one of its entry guards, that circuit isn't distinguished from a circuit which originated locally. If entry_guard_register_connect_status() decides we should retry a different entry guard, the circuit will be killed in channel_do_open_actions(). This potentially could leak information about entry guards in circumstances which appear to be hard to exploit.
07:42 < athena> skruffy: what makes you believe 'channel_do_open_actions() can leak of used guards if relay used as client' ? 07:43 < skruffy> if entry_guard_register_connect_status() failed it will close circuits 07:49 < athena> sooo - in other words, you think if an attacker can build a circuit through a node N which is both a relay and a client to an another node E, and then arrange for entry_guard_register_connect_status() to fail, the attacker can test a hypothesis about whether E is an entry guard for N? 07:52 < skruffy> something like that 07:53 < athena> okay; how would the attacker do that? 07:54 < athena> you may have something there; it seems fishy that killing pending circuits on an entry guard we don't want to use doesn't distinguish between locally originating circuits and circuits from another relay - but i think if so it probably was there pre- channels and i'm not convinced it's exploitable yet 07:59 < skruffy> yes it's pre-chans. 08:02 < athena> hmm, it looks like the only circumstance in which entry_guard_register_connect_status() can fail is if this is a first connection to a new entry guard *and* an old entry guard which was offline has just come back 08:03 < athena> it doesn't seem that practical to exploit - for an attacker to try to selectively manipulate connectivity to the old entry guards to induce those conditions requires already knowing them 08:04 < nickm> It couldn't hurt to treat this as a bug that we should fix sooner or later, though. 08:04 < athena> i think agree conceptually that locally originating circuits and relay circuits should be distinguished and failing entry_guard_register_connect_status() should only kill the local ones, though
Modify circuit_n_chan_done() to take another possible parameter that notifies or_circuits of success but tells all other pending circuits to give up, and pass this from channel_do_open_actions() in case of entry_guard_register_connect_status() failing.