Opened 11 years ago

Last modified 7 years ago

#874 closed defect (Fixed)

Problem establishing new introduction points after reloading

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

Description

Reported by Special (John Brooks) on #tor:

00:27:02 < Special> i've got an interesting looking bug here.
00:27:13 < Special> related to establishing hidden services
00:38:46 < Special> http://rafb.net/p/Dl4prP86.html <- hidden service introduction bug, seen at least after reload, haven't tested at startup.
00:38:51 < Special> on 0.2.1.7-alpha
00:41:23 < Special> 7 cycles of it were seen in roughly 12 seconds; every time except one it chose 'smafio' to cannibalize

http://rafb.net/p/Dl4prP86.html:


This cycle repeats indefinitely after a reload, possibly all the time (unsure). Hidden service is not operable.

Hidden service replaced with 'aaaaaaaaaaaaaaaa'
Nodes used as introduction replaced with a unique 'SR-xxx' name
smafio ($6C0F99C574A739BD51698076B2C513182379689F) and Lifuka are not mentioned elsewhere, and have been preserved.

Nov 24 16:22:48.803 [info] rend_services_introduce(): Picked router SR-1 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.803 [info] rend_services_introduce(): Picked router SR-2 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.804 [info] rend_services_introduce(): Picked router SR-3 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.804 [info] rend_services_introduce(): Picked router SR-4 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.805 [info] rend_services_introduce(): Picked router SR-5 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.805 [info] rend_service_launch_establish_intro(): Launching circuit to introduction point [scrubbed] for service aaaaaaaaaaaaaaaa
Nov 24 16:22:48.805 [debug] circuit_find_to_cannibalize(): Hunting for a circ to cannibalize: purpose 13, uptime 1, capacity 0, internal 1
Nov 24 16:22:48.805 [info] circuit_launch_by_extend_info(): Cannibalizing circ 'smafio' for purpose 13
Nov 24 16:22:48.805 [info] rend_service_launch_establish_intro(): The intro circuit we just cannibalized ends at $6C0F99C574A739BD51698076B2C513182379689F, but we requested an intro circuit to SR-1. Updating our service.
Nov 24 16:22:48.805 [info] rend_service_intro_has_opened(): We have just finished an introduction circuit, but we already have enough. Redefining purpose to general.
Nov 24 16:22:48.805 [info] rend_service_launch_establish_intro(): Launching circuit to introduction point [scrubbed] for service aaaaaaaaaaaaaaaa
Nov 24 16:22:48.805 [debug] circuit_find_to_cannibalize(): Hunting for a circ to cannibalize: purpose 13, uptime 1, capacity 0, internal 1
Nov 24 16:22:48.805 [info] circuit_launch_by_extend_info(): Cannibalizing circ 'smafio' for purpose 13
Nov 24 16:22:48.805 [info] rend_service_launch_establish_intro(): The intro circuit we just cannibalized ends at $6C0F99C574A739BD51698076B2C513182379689F, but we requested an intro circuit to SR-2. Updating our service.
Nov 24 16:22:48.805 [info] rend_service_intro_has_opened(): We have just finished an introduction circuit, but we already have enough. Redefining purpose to general.
Nov 24 16:22:48.806 [info] rend_service_launch_establish_intro(): Launching circuit to introduction point [scrubbed] for service aaaaaaaaaaaaaaaa
Nov 24 16:22:48.806 [debug] circuit_find_to_cannibalize(): Hunting for a circ to cannibalize: purpose 13, uptime 1, capacity 0, internal 1
Nov 24 16:22:48.806 [info] circuit_launch_by_extend_info(): Cannibalizing circ 'smafio' for purpose 13
Nov 24 16:22:48.806 [info] rend_service_launch_establish_intro(): The intro circuit we just cannibalized ends at $6C0F99C574A739BD51698076B2C513182379689F, but we requested an intro circuit to SR-3. Updating our service.
Nov 24 16:22:48.806 [info] rend_service_intro_has_opened(): We have just finished an introduction circuit, but we already have enough. Redefining purpose to general.
Nov 24 16:22:48.806 [info] rend_service_launch_establish_intro(): Launching circuit to introduction point [scrubbed] for service aaaaaaaaaaaaaaaa
Nov 24 16:22:48.806 [debug] circuit_find_to_cannibalize(): Hunting for a circ to cannibalize: purpose 13, uptime 1, capacity 0, internal 1
Nov 24 16:22:48.806 [info] circuit_launch_by_extend_info(): Cannibalizing circ 'smafio' for purpose 13
Nov 24 16:22:48.806 [info] rend_service_launch_establish_intro(): The intro circuit we just cannibalized ends at $6C0F99C574A739BD51698076B2C513182379689F, but we requested an intro circuit to SR-4. Updating our service.
Nov 24 16:22:48.806 [info] rend_service_intro_has_opened(): We have just finished an introduction circuit, but we already have enough. Redefining purpose to general.
Nov 24 16:22:48.806 [info] rend_service_launch_establish_intro(): Launching circuit to introduction point [scrubbed] for service aaaaaaaaaaaaaaaa
Nov 24 16:22:48.806 [debug] circuit_find_to_cannibalize(): Hunting for a circ to cannibalize: purpose 13, uptime 1, capacity 0, internal 1
Nov 24 16:22:48.807 [info] circuit_launch_by_extend_info(): Cannibalizing circ 'smafio' for purpose 13
Nov 24 16:22:48.807 [info] rend_service_launch_establish_intro(): The intro circuit we just cannibalized ends at $6C0F99C574A739BD51698076B2C513182379689F, but we requested an intro circuit to SR-5. Updating our service.
Nov 24 16:22:48.807 [info] rend_service_intro_has_opened(): We have just finished an introduction circuit, but we already have enough. Redefining purpose to general.
Nov 24 16:22:48.807 [info] rend_services_introduce(): Giving up on smafio as intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.807 [info] rend_services_introduce(): Giving up on smafio as intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.807 [info] rend_services_introduce(): Giving up on Lifuka as intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.807 [info] rend_services_introduce(): Giving up on smafio as intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.807 [info] rend_services_introduce(): Giving up on smafio as intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:48.886 [info] rend_services_introduce(): Picked router SR-6 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:49.034 [info] rend_services_introduce(): Picked router SR-7 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:49.167 [info] rend_services_introduce(): Picked router SR-8 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:49.218 [info] rend_services_introduce(): Picked router SR-9 as an intro point for aaaaaaaaaaaaaaaa.
Nov 24 16:22:49.242 [info] rend_services_introduce(): Picked router SR-10 as an intro point for aaaaaaaaaaaaaaaa.

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Attachments (3)

patch874.txt (3.7 KB) - added by karsten 11 years ago.
Patch
patch_874_nm_1.txt (1.3 KB) - added by nickm 11 years ago.
patch874a.txt (3.0 KB) - added by karsten 11 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by karsten

A possible explanation is that when Tor decides whether it needs to pick
new introduction points, it refers to two different numbers:

a) the number of introduction points for a hidden service that are either
in progress or have succeeded (see rend_service_t->intro_nodes in
rendservice.c) and

b) the number of circuits in the global circuit list that are introduction
circuits for a given hidden service (see count_established_intro_points in
rendservice.c).

Usually, these numbers are synchronized in the sense that Tor does not
build introduction circuits that are not listed in the intro_nodes list of
a service.

But after a restart using HUP, all configurations are re-read, including
those containing hidden service settings. As a result, all intro_nodes
lists are emptied, but the introduction circuits are not closed. Tor thinks
that 0 introduction points are established, picks 5 new ones, and after
establishing them finds that there are already enough introduction circuits
for that service and discards the 5 again. This happens repeatedly.

There are two fixes for this: a) Discard those introduction circuits that
are in progress or already established, but that are not contained in the
hidden service's intro_nodes list or b) reassign already established
introduction circuits or those that are in progress to the hidden services
after reloading.

The more general question is how Tor should behave upon reloading: Should
it throw away existing introduction circuit and build new ones (current
behavior and that before this bug was introduced in 0.2.1.7-alpha) or
should it keep these circuits?

Changed 11 years ago by karsten

Attachment: patch874.txt added

Patch

comment:2 Changed 11 years ago by karsten

See patch in Attachments tab.

comment:3 Changed 11 years ago by nickm

Walking over the whole circuit list to rebuild things is pretty bad; we should probably just make rend_config_services
or rend_add_service smarter. After all, we start with a complete list of what our intro points were at the start of
rend_config_services. To deliberately throw this information away seems like a bug.

I'm attaching an untested patch that sketches out one possible approach to this: it matches new services to old ones by
the hidden service directory.

This is still not finished, though. If I read the code right, we'll sometimes needlessly send a redundant
ESTABLISH_INTRO cell, which leaks the fact that we had a SIGHUP. That's bad.

Also, any leftover orphaned circuits will still not get closed. I think these circuits need to get closed, not reused!
If we ever use a circuit as an intro point for two difference services, we are leaking the fact that those two services
are run by the same host, which is Bad For Anonymity.

Changed 11 years ago by nickm

Attachment: patch_874_nm_1.txt added

Changed 11 years ago by karsten

Attachment: patch874a.txt added

comment:4 Changed 11 years ago by karsten

Okay, let's take your approach.

There is one problem with your patch: The hidden service directory does not
uniquely identify a hidden service. A service might publish both version 0
and 2 descriptors (actually, that's the current default). That service
would be treated as two separate services internally. (For 0.2.2.x we
might think about dropping support for version 0 descriptors. All clients
since 0.2.0.x support version 2 descriptors now.)

Also, you are right with closing introduction circuits that we don't need.
I'm not perfectly sure whether they would have been reused for other
purposes, because that would only happen after creating them and before
sending an ESTABLISH_INTRO cell. But it's better to close them and not wait
for the edge case when they make trouble.

I attached a patch with both issues resolved (hopefully).

Where do you see that we are sending redundant ESTABLISH_INTRO cells?

comment:5 Changed 11 years ago by nickm

Almost:

1) The function you want is strcmp(), right?
2) The O(n^2) nested loops bother me some. Don't fix this now, though; it probably won't matter.
3) It's cleaner to iterate over all the intro points you want to close and then close them than to iterate over all

the intro circuits there are and check them against a list of the intro points to keep. You can find the circuits
to close with circuit_get_next_by_pk_and_purpose().

I'm attaching another patch based on yours; what do you think?

In retrospect, I don't remember what I meant about the ESTABLISH_INTRO cells.

comment:6 Changed 11 years ago by nickm

Actually, circuit_get_next_by_pk_and_purpose isn't quite right. It checks only for a single purpose; we probably want
to allow either establishing or established intro circuits. So, no patch from me for now.

I still am not in love with looping over the list of circuits from outside circuitlist.c; perhaps in the morning I
will know just the right API to add there. If not, let's go with your last patch.

comment:7 Changed 11 years ago by nickm

Patch applied and marked for backport; thanks!

comment:8 Changed 11 years ago by nickm

flyspray2trac: bug closed.

comment:9 Changed 7 years ago by nickm

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