[tor-bugs] #13698 [Tor]: Wrong failure report when closing parallel intro points

[Tor Bug Tracker & Wiki]

#13698: Wrong failure report when closing parallel intro points
---------------------+---------------------
 Reporter:  dgoulet  |          Owner:
     Type:  defect   |         Status:  new
 Priority:  normal   |      Milestone:
Component:  Tor      |        Version:
 Keywords:  tor-hs   |  Actual Points:
Parent ID:           |         Points:
---------------------+---------------------
 This is a part of the larger bug #13644.

 When opening intro points, tor does that in parallel with all the possible
 node in the descriptor it has fetched and cached. Once an intro points
 successfully acknowledged, a close "all parallel pending circuit(s)" is
 triggered and here comes trouble. Here is the chain of actions:

 {{{
 rend_client_introduction_acked()
 |- rend_client_close_other_intros()
    |- circuit_mark_for_close(c, END_CIRC_REASON_TIMEOUT)
 }}}

 So far so good, notice the reason being a timeout. That would be "OK"
 because the timeout reason does not remove the intro point from the rend
 cache object intro list. It's still wrong but at least the intro point
 would have been kept.

 That mark for close function is only called under certain conditions and
 one is that {{{CIRCUIT_IS_ORIGIN(c)}}} is true. Now in
 {{{circuit_mark_for_close()}}}, a check is done on exactly that condition
 and if true, the reason is changed from {{{END_CIRC_REASON_TIMEOUT}}} to
 {{{END_CIRC_REASON_NONE}}}.

 Even if the intro point purpose was {{{CIRCUIT_PURPOSE_C_INTRODUCING}}} or
 {{{CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT}}}, in both cases since the reason
 was changed, a generic failure or unreachable error is thrown. Both will
 end up removing the intro points from the cache object (the unreachable
 error will try 3 times to connect before though). You can found this in
 {{{rend_client_report_intro_point_failure()}}}.


 What I think should happen here is to:

 1) add a reason for closing a parellel intro circuit. Something like
 {{{END_CIRC_REASON_IP_PARALLEL}}}. Handling that reason to simply close
 the circuit without changing anything of the rend cache state.

 2) Don't change the reason if it's an intro point circuit and
 CIRCUIT_IS_ORIGIN(c) resolved yes because the reason indicates the type of
 intro point failure that {{{rend_client_report_intro_point_failure()}}}
 uses for specific behaviour.

 Thoughts?

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