Opened 8 years ago

Last modified 6 days ago

#3733 assigned defect

Tor should abandon rendezvous circuits that cause a client request to time out

Reported by: rransom Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs, prop224, tor-client, network-team-roadmap-september
Cc: Actual Points:
Parent ID: #30200 Points: 5
Reviewer: Sponsor: Sponsor27-must

Description

Aug 14 05:59:04.639 [Notice] Rend stream is 120 seconds late. Giving up on address '[scrubbed].onion'.
Aug 14 05:59:09.631 [Notice] Rend stream is 120 seconds late. Giving up on address '[scrubbed].onion'.
Aug 14 06:04:08.637 [Notice] Rend stream is 120 seconds late. Giving up on address '[scrubbed].onion'.
Aug 14 06:04:10.634 [Notice] Rend stream is 120 seconds late. Giving up on address '[scrubbed].onion'.
Aug 14 06:04:30.680 [Notice] Rend stream is 120 seconds late. Giving up on address '[scrubbed].onion'.

All of these streams had been attached to the same rendezvous circuit.

Child Tickets

Change History (35)

comment:1 Changed 8 years ago by rransom

Milestone: Tor: 0.2.3.x-final

This failure mode doesn't seem to harm hidden services or the Tor network, so the fix can be 0.2.3.x-only.

comment:2 Changed 7 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final

comment:3 Changed 7 years ago by nickm

Keywords: tor-hs added

comment:4 Changed 7 years ago by nickm

Component: Tor Hidden ServicesTor

comment:5 Changed 7 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: unspecified

comment:6 Changed 5 years ago by arma

Keywords: SponsorR added

The research question to investigate here is: if you (the client) send your establish-rendezvous cell, and you never get a response, do you just sit there until everything times out? It sounds from the above like the answer is 'yes'.

And worse, it sounds like the next time we get a request for that hidden service, we say to ourselves "oh good there's a rendezvous point in progress, we'll just wait for that".

comment:7 Changed 4 years ago by dgoulet

Keywords: SponsorR tor-hsSponsorR, tor-hs
Milestone: Tor: unspecifiedTor: 0.2.7.x-final
Status: newneeds_review

Reviving this one.

Looking at the code, when a client arrives on the SocksPort, we try to attach on a rendezvous circuit if any. To find that circuit, we call circuit_get_open_circ_or_launch() and immediately circuit_get_best() with the indication that it "must be open". In that function, we use circuit_is_acceptable() that returns 0 if the rendezvous circuit has timed out (origin_circ->hs_circ_has_timed_out)).

That whole process above makes the client launch a new RP circuit. Which is what we want. The issue seems to be that hs_circ_has_timed_out is never set to 1 on the client side for the circuit purpose CIRCUIT_PURPOSE_C_ESTABLISH_REND. Would that patch fix the issue or completely wrong?

diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index b54a4d2..c6bed5a 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -717,6 +717,7 @@ circuit_expire_building(void)
         /* connection_ap_handshake_attach_circuit() will relaunch for us */
       case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT:
       case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED:
+      case CIRCUIT_PURPOSE_C_ESTABLISH_REND:
         /* If we have reached this line, we want to spare the circ for now. */
         log_info(LD_CIRC,"Marking circ %u (state %d:%s, purpose %d) "
                  "as timed-out HS circ",

comment:8 Changed 4 years ago by nickm

It looks plausible to me, but I don't know my way around this code as well as I used to. Have you tried this out? Does it work for you? Is there some way for us to test it?

comment:9 Changed 4 years ago by dgoulet

Keywords: TorCoreTeam201508 added
Owner: changed from rransom to dgoulet
Status: needs_reviewassigned

comment:10 in reply to:  8 Changed 4 years ago by dgoulet

Status: assignedneeds_information

Replying to nickm:

It looks plausible to me, but I don't know my way around this code as well as I used to. Have you tried this out? Does it work for you? Is there some way for us to test it?

Not too easy to test... I've tried couple of things but unsuccessful...

But now I'm unsure here that CIRCUIT_PURPOSE_C_ESTABLISH_REND is the right purpose here... The warning only happens if the purpose is CIRCUIT_PURPOSE_C_REND_JOINED so seems that maybe once we hit that for the client connection, we should maybe flag the circuit has timed out and unattached all connections from it and either start a new rend circuit or kill everyone?

comment:11 Changed 4 years ago by nickm

Keywords: TorCoreTeam201509 added; TorCoreTeam201508 removed

comment:12 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:13 Changed 4 years ago by dgoulet

Keywords: TorCoreTeam201509 removed

Removing it of the 201509 milestone since this is now for 0.2.8 for which I agree since I'm still quite unsure of what's the best course of action.

comment:14 Changed 4 years ago by nickm

Keywords: SponsorR removed
Sponsor: SponsorR

Bulk-replace SponsorR keyword with SponsorR sponsor field in Tor component.

comment:15 Changed 4 years ago by dgoulet

Points: small

comment:16 Changed 3 years ago by dgoulet

Milestone: Tor: 0.2.8.x-finalTor: 0.2.???
Severity: Normal

comment:17 Changed 3 years ago by dgoulet

Sponsor: SponsorRSponsorR-must

comment:18 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:19 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:20 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:21 Changed 2 years ago by nickm

Keywords: needs-insight added

comment:22 Changed 2 years ago by dgoulet

Keywords: prop224 tor-client added; needs-insight removed
Parent ID: #17242
Points: small
Sponsor: SponsorR-mustSponsorR-can
Status: needs_informationnew

I'm not sure it's worth putting more work in the legacy HS subsystem but definitely consider this in the client implementation of prop224 (#17242).

comment:23 Changed 2 years ago by dgoulet

Parent ID: #17242#23300

Switching parent so we can close #17242.

(Only speaking v3):

CIRCUIT_PURPOSE_C_ESTABLISH_REND is handled properly, if it's open and passes its cutoff, it will get expired.

CIRCUIT_PURPOSE_C_REND_READY is not handled properly I believe. An intro and rend circuit are linked together when the client sends an INTRODUCE1 cell that is when the rendezvous_cookie is copied from the rend circuit hs_ident to the intro circuit one. Once they are linked together, the rend circuit is closed by any error on the intro circuit side.

CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED is handled properly, same exact cutoff check as the establish rend purpose but then it is flagged has hs_circ_has_timed_out.

So all in all, we need to handle correctly the rend ready purpose for a v3 service and we should have this fixed.

comment:24 Changed 23 months ago by dgoulet

Milestone: Tor: unspecifiedTor: 0.3.2.x-final
Status: newaccepted

I can probably fix this in 032 after the freeze.

comment:25 Changed 22 months ago by dgoulet

This turns out to be slightly more complicated and convoluted. I'll need to talk to Mike about this.

comment:26 Changed 21 months ago by dgoulet

Milestone: Tor: 0.3.2.x-finalTor: 0.3.3.x-final

Going to 033 for now. It is not critical and it probably needs much more work considering #23100 for instance.

comment:27 Changed 18 months ago by asn

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

comment:28 Changed 16 months ago by nickm

Keywords: 034-triage-20180328 added

comment:29 Changed 16 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:30 Changed 16 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.

comment:31 Changed 4 months ago by asn

Points: 5
Sponsor: SponsorR-canSponsor27-must

Stealing from SponsorR to Sponsor27, this might be a reachability issue.

comment:32 Changed 4 months ago by pili

Parent ID: #23300#29995

comment:33 Changed 3 months ago by asn

Parent ID: #29995#30200

comment:34 Changed 6 weeks ago by gaba

Owner: dgoulet deleted
Status: acceptedassigned

dgoulet will assign himself to the ones he is working on right now.

comment:35 Changed 6 days ago by gaba

Keywords: network-team-roadmap-september added; 034-triage-20180328 034-removed-20180328 removed
Note: See TracTickets for help on using tickets.