Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4641 closed defect (fixed)

Proposal 171 changes broke ‘tor2web mode’

Reported by: rransom Owned by: rransom
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-hs
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

A ‘git bisect’ traced the problem to the proposal 171 changes, and then to commit e8b9815711e7cd1ef0b83153aefcc0d05e817f4e.

This commit causes circuit_has_opened to send two ESTABLISH_RENDEZVOUS cells on each rendezvous circuit, because connection_ap_attach_pending can never attach a stream to a CIRCUIT_PURPOSE_C_ESTABLISH_REND circuit. ‘wanoskarnet’ says that hidden service client support seemed to still be working in non-tor2web-mode versions of 0.2.3.x because they cannibalized pre-built three-hop circuits, and circuit_has_opened is never called for those.

This is also the cause of #4171 (wanoskarnet diagnosed this, too).

Child Tickets

Change History (8)

comment:1 Changed 8 years ago by rransom

Status: newneeds_review

See my bug4641 branch for the fix.

The call to connection_ap_attach_pending after rend_client_rendcirc_has_opened in that section of code is not harmful, and might be slightly helpful (it'll launch an introduction circuit if we don't have one; if that call were removed, we would launch an intro circ anyway when we receive an ack for our ESTABLISH_RENDEZVOUS cell), even though no circuits can be attached to a rend circ by that call to connection_ap_attach_pending.

comment:2 in reply to:  1 Changed 8 years ago by rransom

Replying to rransom:

... even though no circuits can be attached to a rend circ by that call to connection_ap_attach_pending.

s/circuits/streams/

comment:3 Changed 8 years ago by Sebastian

Hrm, I'm not so sure. Why is it ok not to call circuit_clear_isolation() for CIRCUIT_PURPOSE_C_ESTABLISH_REND circs?

comment:4 in reply to:  3 Changed 8 years ago by rransom

Replying to Sebastian:

Hrm, I'm not so sure. Why is it ok not to call circuit_clear_isolation() for CIRCUIT_PURPOSE_C_ESTABLISH_REND circs?

If we want to call circuit_clear_isolation for rend circs, we should do so when they become _C_REND_JOINED and we have tried to attach streams to them.

comment:5 Changed 8 years ago by nickm

Looks good; nice catch!

I'm going to add a comment about why we can't clear isolation there; please let me know if I get it wrong.

Also, please add new tickets as needed for any issues mentioned above that you think should be fixed, and close this ticket when you're satisfied?

comment:6 in reply to:  5 Changed 8 years ago by rransom

Resolution: fixed
Status: needs_reviewclosed

Replying to nickm:

Looks good; nice catch!

I'm going to add a comment about why we can't clear isolation there; please let me know if I get it wrong.

Your comment is correct. (I had to rewrite it anyway on my bug4655 branch.)

Also, please add new tickets as needed for any issues mentioned above that you think should be fixed, and close this ticket when you're satisfied?

Opened #4655 for ‘we should try clearing stream-isolation state for rend circs’.

comment:7 Changed 7 years ago by nickm

Keywords: tor-hs added

comment:8 Changed 7 years ago by nickm

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