Opened 11 years ago

Last modified 7 years ago

#743 closed defect (Fixed)

Handle Reception of RENDEZVOUS_ESTABLISHED and RENDEZVOUS2 cells immediately

Reported by: chrisw Owned by:
Priority: Low Milestone:
Component: Core Tor/Tor Version: 0.2.1.2-alpha
Severity: Keywords:
Cc: chrisw, karsten, nickm, sjmurdoch, arma Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Two client-side events during hidden-service establishment are not
handled event-based, but observed by the one second loop in main.c. This
leads to unnecessary delays of hidden-service establishment up to two
seconds together.

While other events like finishing to establish the introduction circuit
in rend_client_introcirc_has_opened() call the function
connection_ap_attach_pending() to proceed immediately, for two events
this does not happen. Therefore the protocol proceeds after the one
second loop has checked, if anything has changed. Depending on when the
event occurs this check can happen anytime between zero and one second
after the event.

One of the two events is the reception of a RENDEZVOUS2 cell in
rend_client_receive_rendezvous().

The other situation containing the bug only occurs if the introduction
circuit is established before the RENDEZVOUS_ESTABLISHED cell is
received. If establishing the introduction circuit takes longer than
sending the ESTABLISH_RENDEZVOUS cell and receiving the acknowledgment,
everything is fine, because as mentioned before the
rend_client_introcirc_has_opened() function initiates further steps. The
buggy sequence of those events occurred in 28 per cent of 1,200 hidden
service access attempts.

To fix this bug the connection_ap_attach_pending() function needs to be
called after receiving RENDEZVOU2 and RENDEZVOUS_ESTABLISHED cells, too.

The bug was introduced in revision 817, when the
connection_ap_attach_pending() function was called in the main loop for
the first time.

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Attachments (1)

patch_event-based_handling_of_rend2_and_est_rend_cells.diff (1.5 KB) - added by chrisw 11 years ago.
Patch for this bug.

Download all attachments as: .zip

Change History (9)

Changed 11 years ago by chrisw

Patch for this bug.

comment:1 Changed 11 years ago by nickm

I've applied the patch in svn as r15699

Is connection_ap_attach_pending() really the right function to call? It walks over the entire connection list. Can
we just walk over the list of connections pending on the given circuit and call connection_ap_handshake_attach_circuit
on those?

BTW, the bug was *not* introduced when connection_ap_attach_pending() was added to the main loop; it was introduced
in the first version that handled rendezvous2 and rendezvous_established cells without calling
connection_ap_attach_pending(), apparently 0.0.6pre1.

comment:2 Changed 11 years ago by nickm

Come to think of it, we should make sure we do the right thing here with regular streams.

comment:3 Changed 11 years ago by chrisw

Thanks for your comments!

I assumed that calling connection_ap_attach_pending() was ok because I've seen it in
rend_client_introcirc_has_opened(). But you are right, there are less expensive ways to find the right connections.

I can add a function below connection_ap_attach_pending() that expects a given circuit as parameter and
calls connection_ap_handshake_attach_circuit() for all connections associated with that circuit.

This function can be called after the reception of the two cells mentioned in the title of this task as well
as after opening the introduction and rendezvous circuits on client side.

But I'm not sure what you mean with "doing the right thing here with regular streams". Can you give a hint
on that one?

comment:4 Changed 11 years ago by nickm

Sorry; I meant "we should make sure that we also try to reattach regular streams when an appropriate circuit
arrives, rather than waiting a while for them too."

comment:5 Changed 11 years ago by nickm

And yeah, a function like you describe would be great, especially if it isn't linear in the number of connections.

comment:6 Changed 11 years ago by nickm

I'm closing this as "fixed" and marking the problems noted above as an XXXX022 ('fix me in 0.2.2') in the code.
The bug is gone, even if the fix isn't the best fix imaginable.

comment:7 Changed 11 years ago by nickm

flyspray2trac: bug closed.

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.