tor-commits December 2012

tor-commits@lists.torproject.org

17 participants
1600 discussions

[tor/master] Document that care needs to be taken with any_streams_attached.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 9b4046607232c636c58f04672c1b762968a8b023 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Wed Nov 21 16:31:58 2012 -0800 Document that care needs to be taken with any_streams_attached. --- src/or/or.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 8501235..d165705 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2875,6 +2875,10 @@ typedef struct origin_circuit_t { /** * Did any SOCKS streams or hidserv introductions actually succeed on * this circuit? + * + * Note: If we ever implement end-to-end stream timing through test + * stream probes (#5707), we must *not* set this for those probes + * (or any other automatic streams). */ unsigned int any_streams_succeeded : 1;

1 0

[tor/master] Note a strange case for SOCKS streams.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit ecaeb505fab12985d314aace49f1277b1b58dc1b Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Wed Nov 21 16:40:25 2012 -0800 Note a strange case for SOCKS streams. --- src/or/connection_edge.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index a654b61..31ff900 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2181,6 +2181,7 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, if (status == SOCKS5_SUCCEEDED) { if(!conn->edge_.on_circuit || !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) { + // XXX: Weird. We hit this a lot, and yet have no unusable_circs log_warn(LD_BUG, "No origin circuit for successful SOCKS stream"); } else { TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->any_streams_succeeded = 1;

1 0

[tor/master] Fix another crash bug.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 7a28862d56c15e4b83efc514621a330085781323 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Wed Nov 21 16:33:16 2012 -0800 Fix another crash bug. --- src/or/circuitbuild.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index aaa1959..8304ad8 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1501,14 +1501,19 @@ pathbias_get_closed_count(entry_guard_t *guard) /* Count currently open circuits. Give them the benefit of the doubt */ for ( ; circ; circ = circ->next) { + origin_circuit_t *ocirc = NULL; if (!CIRCUIT_IS_ORIGIN(circ) || /* didn't originate here */ - circ->marked_for_close || /* already counted */ - !circ->cpath || !circ->cpath->extend_info) + circ->marked_for_close) /* already counted */ continue; - if (TO_ORIGIN_CIRCUIT(circ)->path_state == PATH_STATE_SUCCEEDED && + ocirc = TO_ORIGIN_CIRCUIT(circ); + + if(!ocirc->cpath || !ocirc->cpath->extend_info) + continue; + + if (ocirc->path_state == PATH_STATE_SUCCEEDED && (memcmp(guard->identity, - TO_ORIGIN_CIRCUIT(circ)->cpath->extend_info->identity_digest, + ocirc->cpath->extend_info->identity_digest, DIGEST_LEN) == 0)) { open_circuits++;

1 0

[tor/master] Fix some hidden service edge cases.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 5f733ccd7382e8bb8289e4f8adf07f8ac001c28a Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sat Dec 8 12:07:58 2012 -0800 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(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index af36cb2..7eae0e7 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 = &ocirc->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 77822a3..9362e24 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 3393e0f..88241a4 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. */

1 0

[tor/master] Fix a crash bug and pass down a remote reason code.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 721f7e375114abfcb1a41ade58ac59ec79b8a3af Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Wed Nov 21 16:32:38 2012 -0800 Fix a crash bug and pass down a remote reason code. Unexpected channel closures count as remote circ failures. --- src/or/circuitlist.c | 19 +++++++++++++++---- 1 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 66cdbe1..eb7fc75 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1038,8 +1038,13 @@ circuit_unlink_all_from_channel(channel_t *chan, int reason) for (circ = global_circuitlist; circ; circ = circ->next) { int mark = 0; if (circ->n_chan == chan) { - circuit_set_n_circid_chan(circ, 0, NULL); - mark = 1; + circuit_set_n_circid_chan(circ, 0, NULL); + mark = 1; + + /* If we didn't request this closure, pass the remote + * bit to mark_for_close. */ + if (chan->reason_for_closing != CHANNEL_CLOSE_REQUESTED) + reason |= END_CIRC_REASON_FLAG_REMOTE; } if (! CIRCUIT_IS_ORIGIN(circ)) { or_circuit_t *or_circ = TO_OR_CIRCUIT(circ); @@ -1355,14 +1360,20 @@ circuit_mark_for_close_(circuit_t *circ, int reason, int line, int pathbias_is_normal_close = 1; /* FIXME: Is timestamp_dirty the right thing for these two checks? - * Should we use isolation_any_streams_attached instead? */ + * Should we use isolation_any_streams_attached instead? + * isolation_any_streams_attached is not used for hidservs.. */ if (!circ->timestamp_dirty) { - if (reason & END_CIRC_REASON_FLAG_REMOTE) { + if (orig_reason & END_CIRC_REASON_FLAG_REMOTE) { /* Unused remote circ close reasons all could be bias */ + // XXX: We hit this a lot for hidserv circs with purposes: + // CIRCUIT_PURPOSE_S_CONNECT_REND (reasons: 514,517,520) + // CIRCUIT_PURPOSE_S_REND_JOINED (reasons: 514,517,520) + // == reasons: 2,3,8. Client-side timeouts? pathbias_is_normal_close = 0; pathbias_count_collapse(ocirc); } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) == END_CIRC_REASON_CHANNEL_CLOSED && + circ->n_chan && circ->n_chan->reason_for_closing != CHANNEL_CLOSE_REQUESTED) { /* If we didn't close the channel ourselves, it could be bias */

1 0

[tor/master] Sadly, we can't safely count client intro circ success
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit b599a6ed07fd588500a256ef815e87729449626a Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sat Dec 8 14:16:29 2012 -0800 Sadly, we can't safely count client intro circ success --- src/or/circuitbuild.c | 42 +++++++++++++++++------------------------- src/or/circuituse.c | 9 +++++++-- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 7eae0e7..9b1236f 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -739,8 +739,12 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) circuit_has_opened(circ); /* do other actions as necessary */ /* We're done with measurement circuits here. Just close them */ - if (circ->base_.purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) + if (circ->base_.purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) { + /* If a measurement circ ever gets back to us, consider it + * succeeded for path bias */ + circ->path_state = PATH_STATE_USE_SUCCEEDED; circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); + } return 0; } @@ -1156,13 +1160,19 @@ pathbias_should_count(origin_circuit_t *circ) /* We can't do path bias accounting without entry guards. * Testing and controller circuits also have no guards. + * * We also don't count server-side rends, because their - * endpoint could be chosen maliciously. */ + * endpoint could be chosen maliciously. + * Similarly, we can't count client-side intro attempts, + * because clients can be manipulated into connecting to + * malicious intro points. */ if (get_options()->UseEntryGuards == 0 || circ->base_.purpose == CIRCUIT_PURPOSE_TESTING || circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER || circ->base_.purpose == CIRCUIT_PURPOSE_S_CONNECT_REND || - circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED) { + circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED || + (circ->base_.purpose >= CIRCUIT_PURPOSE_C_INTRODUCING && + circ->base_.purpose <= CIRCUIT_PURPOSE_C_INTRODUCE_ACKED)) { return 0; } @@ -1388,7 +1398,7 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) { circuit_t *circ = &ocirc->base_; - if (!pathbias_should_count(circ)) { + if (!pathbias_should_count(ocirc)) { return; } @@ -1399,33 +1409,15 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) // 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: 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, + + log_info(LD_CIRC, "Circuit closed without successful use for reason %d. " "Circuit purpose %d currently %s.", reason, circ->purpose, circuit_state_to_string(circ->state)); - pathbias_count_unusable(ocirc); - } + pathbias_count_unusable(ocirc); } else { if (reason & END_CIRC_REASON_FLAG_REMOTE) { /* Unused remote circ close reasons all could be bias */ - // XXX: We hit this a lot for hidserv circs with purposes: - // CIRCUIT_PURPOSE_S_CONNECT_REND (reasons: 514,517,520) - // CIRCUIT_PURPOSE_S_REND_JOINED (reasons: 514,517,520) - // == reasons: 2,3,8. Client-side timeouts? log_info(LD_CIRC, "Circuit remote-closed without successful use for reason %d. " "Circuit purpose %d currently %s.", diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 9362e24..0b799b1 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1402,11 +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 && + if ((purpose == CIRCUIT_PURPOSE_S_CONNECT_REND || + purpose == CIRCUIT_PURPOSE_C_INTRODUCING) && 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. */ + * because the rend point could be malicious. + * + * Same deal goes for client side introductions. Clients + * can be manipulated to connect repeatedly to them + * (especially web clients). */ circ->path_state = PATH_STATE_USE_SUCCEEDED; /* This must be called before the purpose change */ pathbias_check_close(circ);

1 0

[tor/master] Refactor path use bias code into own function.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 26fa47226cab49b260ba764aa050880f71927ea0 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Fri Dec 7 17:47:23 2012 -0800 Refactor path use bias code into own function. Also, improve and log some failure cases. --- src/or/circuitbuild.c | 98 +++++++++++++++++++++++++++++++++++++++------ src/or/circuitbuild.h | 4 +- src/or/circuitlist.c | 48 +---------------------- src/or/connection_edge.c | 12 ++++-- src/or/or.h | 24 +++++------ src/or/rendclient.c | 2 +- src/or/rendservice.c | 9 ++-- 7 files changed, 112 insertions(+), 85 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 8304ad8..af36cb2 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -60,7 +60,10 @@ static int onion_extend_cpath(origin_circuit_t *circ); static int count_acceptable_nodes(smartlist_t *routers); static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); static int entry_guard_inc_first_hop_count(entry_guard_t *guard); -static void pathbias_count_success(origin_circuit_t *circ); +static void pathbias_count_build_success(origin_circuit_t *circ); +static void pathbias_count_successful_close(origin_circuit_t *circ); +static void pathbias_count_collapse(origin_circuit_t *circ); +static void pathbias_count_unusable(origin_circuit_t *circ); /** This function tries to get a channel to the specified endpoint, * and then calls command_setup_channel() to give it the right @@ -731,7 +734,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) } } - pathbias_count_success(circ); + pathbias_count_build_success(circ); circuit_rep_hist_note_result(circ); circuit_has_opened(circ); /* do other actions as necessary */ @@ -1129,8 +1132,10 @@ pathbias_state_to_string(path_state_t state) return "new"; case PATH_STATE_DID_FIRST_HOP: return "first hop"; - case PATH_STATE_SUCCEEDED: - return "succeeded"; + case PATH_STATE_BUILD_SUCCEEDED: + return "build succeeded"; + case PATH_STATE_USE_SUCCEEDED: + return "use succeeded"; } return "unknown"; @@ -1216,7 +1221,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) } } - /* Don't count cannibalized circs for path bias */ + /* Don't re-count cannibalized circs.. */ if (!circ->has_opened) { entry_guard_t *guard = NULL; @@ -1291,7 +1296,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) * Also check for several potential error cases for bug #6475. */ static void -pathbias_count_success(origin_circuit_t *circ) +pathbias_count_build_success(origin_circuit_t *circ) { #define SUCCESS_NOTICE_INTERVAL (600) static ratelim_t success_notice_limit = @@ -1303,7 +1308,8 @@ pathbias_count_success(origin_circuit_t *circ) return; } - /* Don't count cannibalized/reused circs for path bias */ + /* Don't count cannibalized/reused circs for path bias + * build success.. They get counted under use success */ if (!circ->has_opened) { if (circ->cpath && circ->cpath->extend_info) { guard = entry_guard_get_by_id_digest( @@ -1312,7 +1318,7 @@ pathbias_count_success(origin_circuit_t *circ) if (guard) { if (circ->path_state == PATH_STATE_DID_FIRST_HOP) { - circ->path_state = PATH_STATE_SUCCEEDED; + circ->path_state = PATH_STATE_BUILD_SUCCEEDED; guard->circuit_successes++; log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", @@ -1354,7 +1360,7 @@ pathbias_count_success(origin_circuit_t *circ) } } } else { - if (circ->path_state != PATH_STATE_SUCCEEDED) { + if (circ->path_state < PATH_STATE_BUILD_SUCCEEDED) { if ((rate_msg = rate_limit_log(&success_notice_limit, approx_time()))) { log_info(LD_BUG, @@ -1371,9 +1377,70 @@ pathbias_count_success(origin_circuit_t *circ) } /** - * Count a successfully closed circuit. + * Check if a circuit was used and/or closed successfully. */ void +pathbias_check_close(origin_circuit_t *ocirc, int reason) +{ + circuit_t *circ = &ocirc->base_; + + if (ocirc->path_state == PATH_STATE_BUILD_SUCCEEDED) { + if (circ->timestamp_dirty) { + // 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, + "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); + } else { + if (reason & END_CIRC_REASON_FLAG_REMOTE) { + /* Unused remote circ close reasons all could be bias */ + // XXX: We hit this a lot for hidserv circs with purposes: + // CIRCUIT_PURPOSE_S_CONNECT_REND (reasons: 514,517,520) + // CIRCUIT_PURPOSE_S_REND_JOINED (reasons: 514,517,520) + // == 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)); + pathbias_count_collapse(ocirc); + } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) + == END_CIRC_REASON_CHANNEL_CLOSED && + circ->n_chan && + circ->n_chan->reason_for_closing + != CHANNEL_CLOSE_REQUESTED) { + /* If we didn't close the channel ourselves, it could be bias */ + /* FIXME: Only count bias if the network is live? + * 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.", + reason, circ->n_chan->reason_for_closing, + circuit_purpose_to_string(circ->purpose), + circuit_state_to_string(circ->state)); + pathbias_count_collapse(ocirc); + } else { + pathbias_count_successful_close(ocirc); + } + } + } else if (ocirc->path_state == PATH_STATE_USE_SUCCEEDED) { + pathbias_count_successful_close(ocirc); + } +} + +/** + * Count a successfully closed circuit. + */ +static void pathbias_count_successful_close(origin_circuit_t *circ) { entry_guard_t *guard = NULL; @@ -1411,7 +1478,7 @@ pathbias_count_successful_close(origin_circuit_t *circ) * circuit after it has successfully completed. Right now, this is * used for purely informational/debugging purposes. */ -void +static void pathbias_count_collapse(origin_circuit_t *circ) { entry_guard_t *guard = NULL; @@ -1439,7 +1506,7 @@ pathbias_count_collapse(origin_circuit_t *circ) } } -void +static void pathbias_count_unusable(origin_circuit_t *circ) { entry_guard_t *guard = NULL; @@ -1511,7 +1578,7 @@ pathbias_get_closed_count(entry_guard_t *guard) if(!ocirc->cpath || !ocirc->cpath->extend_info) continue; - if (ocirc->path_state == PATH_STATE_SUCCEEDED && + if (ocirc->path_state >= PATH_STATE_BUILD_SUCCEEDED && (memcmp(guard->identity, ocirc->cpath->extend_info->identity_digest, DIGEST_LEN) @@ -2371,6 +2438,11 @@ circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit) circuit_mark_for_close(TO_CIRCUIT(circ), -err_reason); return -1; } + + /* Set timestamp_dirty, so we can check it for path use bias */ + if (!circ->base_.timestamp_dirty) + circ->base_.timestamp_dirty = time(NULL); + return 0; } diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 2d9eac1..53c9fe5 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -56,8 +56,6 @@ const node_t *choose_good_entry_server(uint8_t purpose, double pathbias_get_extreme_rate(const or_options_t *options); int pathbias_get_dropguards(const or_options_t *options); void pathbias_count_timeout(origin_circuit_t *circ); -void pathbias_count_successful_close(origin_circuit_t *circ); -void pathbias_count_collapse(origin_circuit_t *circ); -void pathbias_count_unusable(origin_circuit_t *circ); +void pathbias_check_close(origin_circuit_t *circ, int reason); #endif diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 7163c35..6fab492 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1354,53 +1354,7 @@ circuit_mark_for_close_(circuit_t *circ, int reason, int line, } if (CIRCUIT_IS_ORIGIN(circ)) { - origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); - - if (ocirc->path_state == PATH_STATE_SUCCEEDED) { - int pathbias_is_normal_close = 1; - - /* FIXME: Is timestamp_dirty the right thing for these two checks? - * Should we use isolation_any_streams_attached instead? - * isolation_any_streams_attached is not used for hidservs.. */ - if (!circ->timestamp_dirty) { - if (orig_reason & END_CIRC_REASON_FLAG_REMOTE) { - /* Unused remote circ close reasons all could be bias */ - // XXX: We hit this a lot for hidserv circs with purposes: - // CIRCUIT_PURPOSE_S_CONNECT_REND (reasons: 514,517,520) - // CIRCUIT_PURPOSE_S_REND_JOINED (reasons: 514,517,520) - // == reasons: 2,3,8. Client-side timeouts? - pathbias_is_normal_close = 0; - pathbias_count_collapse(ocirc); - } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) - == END_CIRC_REASON_CHANNEL_CLOSED && - circ->n_chan && - circ->n_chan->reason_for_closing - != CHANNEL_CLOSE_REQUESTED) { - /* If we didn't close the channel ourselves, it could be bias */ - /* FIXME: Only count bias if the network is live? - * What about clock jumps/suspends? */ - pathbias_is_normal_close = 0; - pathbias_count_collapse(ocirc); - } - } else if (circ->timestamp_dirty && !ocirc->any_streams_succeeded) { - // 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 */ - /* FIXME: This may be better handled by limiting the number of retries - * per stream? */ - pathbias_is_normal_close = 0; - pathbias_count_unusable(ocirc); - } - - if (pathbias_is_normal_close) { - pathbias_count_successful_close(ocirc); - } - } + pathbias_check_close(TO_ORIGIN_CIRCUIT(circ), reason); /* We don't send reasons when closing circuits at the origin. */ reason = END_CIRC_REASON_NONE; diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 31ff900..79bb54c 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2181,10 +2181,14 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, if (status == SOCKS5_SUCCEEDED) { if(!conn->edge_.on_circuit || !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) { - // XXX: Weird. We hit this a lot, and yet have no unusable_circs - log_warn(LD_BUG, "No origin circuit for successful SOCKS stream"); + // XXX: Weird. We hit this a lot, and yet have no unusable_circs. + // Maybe during addrmaps/resolves? + log_warn(LD_BUG, + "(Harmless.) No origin circuit for successful SOCKS stream. " + "Reason: %d", endreason); } else { - TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->any_streams_succeeded = 1; + TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state + = PATH_STATE_USE_SUCCEEDED; } } @@ -2457,7 +2461,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) connection_exit_connect(n_stream); /* For path bias: This circuit was used successfully */ - origin_circ->any_streams_succeeded = 1; + origin_circ->path_state = PATH_STATE_USE_SUCCEEDED; tor_free(address); return 0; diff --git a/src/or/or.h b/src/or/or.h index d165705..c8ea12f 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2768,9 +2768,17 @@ typedef enum { /** This circuit has completed a first hop, and has been counted by * the path bias logic. */ PATH_STATE_DID_FIRST_HOP = 1, - /** This circuit has been completely built, and has been counted as - * successful by the path bias logic. */ - PATH_STATE_SUCCEEDED = 2, + /** This circuit has been completely built */ + PATH_STATE_BUILD_SUCCEEDED = 2, + /** Did any SOCKS streams or hidserv introductions actually succeed on + * this circuit? + * + * Note: If we ever implement end-to-end stream timing through test + * stream probes (#5707), we must *not* set this for those probes + * (or any other automatic streams) because the adversary could + * just tag at a later point. + */ + PATH_STATE_USE_SUCCEEDED = 3, } path_state_t; /** An origin_circuit_t holds data necessary to build and use a circuit. @@ -2872,16 +2880,6 @@ typedef struct origin_circuit_t { */ unsigned int isolation_any_streams_attached : 1; - /** - * Did any SOCKS streams or hidserv introductions actually succeed on - * this circuit? - * - * Note: If we ever implement end-to-end stream timing through test - * stream probes (#5707), we must *not* set this for those probes - * (or any other automatic streams). - */ - unsigned int any_streams_succeeded : 1; - /** A bitfield of ISO_* flags for every isolation field such that this * circuit has had streams with more than one value for that field * attached to it. */ diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 1d473de..3393e0f 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -363,7 +363,7 @@ rend_client_introduction_acked(origin_circuit_t *circ, /* For path bias: This circuit was used successfully. Valid * nacks and acks count. */ - circ->any_streams_succeeded = 1; + circ->path_state = PATH_STATE_USE_SUCCEEDED; if (request_len == 0) { /* It's an ACK; the introduction point relayed our introduction request. */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 74e4bad..fbf14e9 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1383,9 +1383,9 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, if (circuit_init_cpath_crypto(cpath,keys+DIGEST_LEN,1)<0) goto err; memcpy(cpath->handshake_digest, keys, DIGEST_LEN); - - /* For path bias: This circuit was used successfully */ - circuit->any_streams_succeeded = 1; + + /* For path bias: This intro circuit was used successfully */ + circuit->path_state = PATH_STATE_USE_SUCCEEDED; goto done; @@ -2586,7 +2586,8 @@ rend_service_rendezvous_has_opened(origin_circuit_t *circuit) tor_assert(circuit->rend_data); /* Declare the circuit dirty to avoid reuse, and for path-bias */ - circuit->base_.timestamp_dirty = time(NULL); + if(!circuit->base_.timestamp_dirty) + circuit->base_.timestamp_dirty = time(NULL); hop = circuit->build_state->service_pending_final_cpath_ref->cpath;

1 0

[tor/master] Note more potential issues.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit dc86d7c35bd48d12d84feb6f63014904eabe0902 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Nov 25 17:29:16 2012 -0800 Note more potential issues. --- src/or/circuitlist.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index eb7fc75..7163c35 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1383,6 +1383,12 @@ circuit_mark_for_close_(circuit_t *circ, int reason, int line, pathbias_count_collapse(ocirc); } } else if (circ->timestamp_dirty && !ocirc->any_streams_succeeded) { + // 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 */ /* FIXME: This may be better handled by limiting the number of retries

1 0

[tor/master] Actually, both nacks and acks indicate a valid path
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit c3b71a3fc96c6f3eaaebd96ef8c15d4298d9639e Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Fri Dec 7 15:50:31 2012 -0800 Actually, both nacks and acks indicate a valid path --- src/or/rendclient.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index ec43041..1d473de 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -361,6 +361,10 @@ rend_client_introduction_acked(origin_circuit_t *circ, #endif tor_assert(circ->rend_data); + /* For path bias: This circuit was used successfully. Valid + * nacks and acks count. */ + circ->any_streams_succeeded = 1; + if (request_len == 0) { /* It's an ACK; the introduction point relayed our introduction request. */ /* Locate the rend circ which is waiting to hear about this ack, @@ -378,9 +382,6 @@ rend_client_introduction_acked(origin_circuit_t *circ, * it to specify when a circuit entered the * _C_REND_READY_INTRO_ACKED state. */ rendcirc->base_.timestamp_dirty = time(NULL); - - /* For path bias: This circuit was used successfully */ - circ->any_streams_succeeded = 1; } else { log_info(LD_REND,"...Found no rend circ. Dropping on the floor."); }

1 0

[tor/master] Add intro+rend cannibalize param..
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit fbbf894d4db1b1109fedca5f74891642f61370fe Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:02:08 2012 -0800 Add intro+rend cannibalize param.. --- src/or/circuituse.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 781e984..381c2b0 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1429,7 +1429,7 @@ circuit_launch_by_extend_info(uint8_t purpose, */ circ->path_state = PATH_STATE_USE_SUCCEEDED; /* This must be called before the purpose change */ - pathbias_check_close(circ); + pathbias_check_close(circ, END_CIRC_REASON_FINISHED); } circuit_change_purpose(TO_CIRCUIT(circ), purpose);

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits December 2012