From 5f733ccd7382e8bb8289e4f8adf07f8ac001c28a Mon Sep 17 00:00:00 2001 From: Mike Perry Date: Sat, 8 Dec 2012 12:07:58 -0800 Subject: Fix some hidden service edge cases. --- src/or/circuitbuild.c | 60 +++++++++++++++++++++++++++++++++++---------------- src/or/circuituse.c | 10 +++++++++ src/or/rendclient.c | 7 ++++++ 3 files changed, 59 insertions(+), 18 deletions(-) (limited to 'src/or') diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index af36cb2c3..7eae0e7a9 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1155,10 +1155,14 @@ pathbias_should_count(origin_circuit_t *circ) char *rate_msg = NULL; /* We can't do path bias accounting without entry guards. - * Testing and controller circuits also have no guards. */ + * Testing and controller circuits also have no guards. + * We also don't count server-side rends, because their + * endpoint could be chosen maliciously. */ if (get_options()->UseEntryGuards == 0 || circ->base_.purpose == CIRCUIT_PURPOSE_TESTING || - circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER) { + circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER || + circ->base_.purpose == CIRCUIT_PURPOSE_S_CONNECT_REND || + circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED) { return 0; } @@ -1384,22 +1388,37 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) { circuit_t *circ = ô->base_; + if (!pathbias_should_count(circ)) { + return; + } + if (ocirc->path_state == PATH_STATE_BUILD_SUCCEEDED) { if (circ->timestamp_dirty) { + /* Any circuit where there were attempted streams but no successful + * streams could be bias */ // XXX: May open up attacks if the adversary can force connections // on unresponsive hosts to use new circs. Vidalia displayes a "Retrying" // state.. Can we use that? Does optimistic data change this? - // XXX: For the hidserv side, we could only care about INTRODUCING purposes - // for server+client, and REND purposes for the server... Can we - // somehow only count those? - /* Any circuit where there were attempted streams but no successful - * streams could be bias */ - log_info(LD_CIRC, + // XXX: Sub-attack: in collusion with an intro point, you can induce bias + // through the web. Need a Torbutton patch to prevent this. + + /* FIXME: This is not ideal, but it prevents the case where a + * CPU overloaded intro point is chosen. + * XXX: Is this reason code authenticated? */ + if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCING && + reason == + END_CIRC_REASON_FLAG_REMOTE|END_CIRC_REASON_RESOURCELIMIT) { + log_info(LD_CIRC, + "Ignoring CPU overload intro circuit without successful use. " + "Circuit purpose %d currently %s.", + reason, circ->purpose, circuit_state_to_string(circ->state)); + } else { + log_info(LD_CIRC, "Circuit closed without successful use for reason %d. " - "Circuit is a %s currently %s.", - reason, circuit_purpose_to_string(circ->purpose), - circuit_state_to_string(circ->state)); - pathbias_count_unusable(ocirc); + "Circuit purpose %d currently %s.", + reason, circ->purpose, circuit_state_to_string(circ->state)); + pathbias_count_unusable(ocirc); + } } else { if (reason & END_CIRC_REASON_FLAG_REMOTE) { /* Unused remote circ close reasons all could be bias */ @@ -1409,9 +1428,8 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) // == reasons: 2,3,8. Client-side timeouts? log_info(LD_CIRC, "Circuit remote-closed without successful use for reason %d. " - "Circuit is a %s currently %s.", - reason, circuit_purpose_to_string(circ->purpose), - circuit_state_to_string(circ->state)); + "Circuit purpose %d currently %s.", + reason, circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_collapse(ocirc); } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) == END_CIRC_REASON_CHANNEL_CLOSED && @@ -1423,10 +1441,9 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) * What about clock jumps/suspends? */ log_info(LD_CIRC, "Circuit's channel closed without successful use for reason %d, " - "channel reason %d. Circuit is a %s currently %s.", + "channel reason %d. Circuit purpose %d currently %s.", reason, circ->n_chan->reason_for_closing, - circuit_purpose_to_string(circ->purpose), - circuit_state_to_string(circ->state)); + circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_collapse(ocirc); } else { pathbias_count_successful_close(ocirc); @@ -1548,6 +1565,13 @@ pathbias_count_timeout(origin_circuit_t *circ) return; } + /* For hidden service circs, they can actually be used + * successfully and then time out later (because + * the other side declines to use them). */ + if (circ->path_state == PATH_STATE_USE_SUCCEEDED) { + return; + } + if (circ->cpath && circ->cpath->extend_info) { guard = entry_guard_get_by_id_digest( circ->cpath->extend_info->identity_digest); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 77822a36a..9362e2420 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1402,6 +1402,16 @@ circuit_launch_by_extend_info(uint8_t purpose, build_state_get_exit_nickname(circ->build_state), purpose, circuit_purpose_to_string(purpose)); + if (purpose == CIRCUIT_PURPOSE_S_CONNECT_REND && + circ->path_state == PATH_STATE_BUILD_SUCCEEDED) { + /* Path bias: Cannibalized rends pre-emptively count as a + * successfully used circ. We don't wait until the extend, + * because the rend point could be malicious. */ + circ->path_state = PATH_STATE_USE_SUCCEEDED; + /* This must be called before the purpose change */ + pathbias_check_close(circ); + } + circuit_change_purpose(TO_CIRCUIT(circ), purpose); /* Reset the start date of this circ, else expire_building * will see it and think it's been trying to build since it diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 3393e0f62..88241a4b2 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -862,6 +862,13 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request, /* Set timestamp_dirty, because circuit_expire_building expects it * to specify when a circuit entered the _C_REND_READY state. */ circ->base_.timestamp_dirty = time(NULL); + + /* From a path bias point of view, this circuit is now successfully used. + * Waiting any longer opens us up to attacks from Bob. He could induce + * Alice to attempt to connect to his hidden service and never reply + * to her rend requests */ + circ->path_state = PATH_STATE_USE_SUCCEEDED; + /* XXXX This is a pretty brute-force approach. It'd be better to * attach only the connections that are waiting on this circuit, rather * than trying to attach them all. See comments bug 743. */ -- cgit v1.2.3