[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #6475 [Tor Client]: circuit_send_next_onion_skin(): Bug: Unexpectedly high circuit_successes
#6475: circuit_send_next_onion_skin(): Bug: Unexpectedly high circuit_successes
-----------------------------+----------------------------------------------
Reporter: grarpamp | Owner: mikeperry
Type: defect | Status: assigned
Priority: major | Milestone: Tor: 0.2.3.x-final
Component: Tor Client | Version: Tor: 0.2.3.19-rc
Keywords: MikePerry201208 | Parent:
Points: | Actualpoints:
-----------------------------+----------------------------------------------
Comment(by mikeperry):
Ok. Here's some initial analysis:
1. There may be a few codepaths that allow circuit_send_next_onion_skin()
to get called multiple times for the same circuit with a NULL
onion_next_hop_in_cpath() value (which is the signal we use to determine
the circuit is done). In particular the control port can do this for sure.
rend_client_reextend_intro_circuit() and rend_client_receive_rendezvous()
seem also able to do it. Possibly .exit notation, too.
2. *However*, there is *no* codepath in circuit_send_next_onion_skin()
that allows us to count a successful circuit without also setting
circ->has_opened = 1. There are also no codepaths that *unset*
circ->has_opened. Thus, double-counting on the success side should be
impossible, even in the above cases for point 1.
3. Therefore, this is an issue with the first_hop counting logic. Somehow
circuits are completing a first hop without getting counted by
entry_guard_inc_first_hop_count().
4. There appear to be no codepaths that allow us to finish a hop in a
circuit without going through circuit_finish_handshake(), though this is a
bit trickier to determine for sure.
All of this together seems to point to an issue with the checks in
circuit_finish_handshake() itself.
As a kludge and to protect against future bugs, nickm suggested creating a
new field of origin_circuit_t that we set inside
entry_guard_inc_first_hop_count() and then check for in
circuit_send_next_onion_skin(), so that we are absolutely sure we only
count successes for circuits whose first hops we counted. We could also
update this field to a third value when we count the success, to avoid
potentially insane cases where we *again* try to count a new first hop for
a circuit after counting the success for it.
All of these unexpected cases should have logging for them.
Also, this might mean that whatever hidden service circumstances that are
triggering this bug won't be protected by the Path Bias defense until we
figure out what the hell codepaths are involved. Hopefully the log lines +
a repro script will help us find them.
I'll get to work on a patch for this as soon as I can.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/6475#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs