- tor-commits - lists.torproject.org

[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

[tor/master] Convert to doubles for all pathbias state.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 2dbb62f1b571ea57af111f1f660a5149d160c4fb Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:53:22 2012 -0800 Convert to doubles for all pathbias state. Let's hope this solves the rounding error issue.. --- src/or/circuitbuild.c | 83 +++++++++++++++++++++++-------------------------- src/or/entrynodes.c | 18 ++++++----- src/or/entrynodes.h | 14 ++++---- 3 files changed, 56 insertions(+), 59 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 04b429b..cbc1af9 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1363,7 +1363,7 @@ pathbias_count_build_success(origin_circuit_t *circ) circ->path_state = PATH_STATE_BUILD_SUCCEEDED; guard->circ_successes++; - log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", + log_info(LD_CIRC, "Got success count %lf/%lf for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } else { @@ -1381,7 +1381,7 @@ pathbias_count_build_success(origin_circuit_t *circ) } if (guard->circ_attempts < guard->circ_successes) { - log_notice(LD_BUG, "Unexpectedly high successes counts (%u/%u) " + log_notice(LD_BUG, "Unexpectedly high successes counts (%lf/%lf) " "for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); @@ -1607,7 +1607,7 @@ pathbias_count_timeout(origin_circuit_t *circ) } // XXX: DOCDOC -int +double pathbias_get_closed_count(entry_guard_t *guard) { circuit_t *circ = global_circuitlist; @@ -1642,7 +1642,7 @@ pathbias_get_closed_count(entry_guard_t *guard) * if it should return guard->circ_successes or * guard->successful_circuits_closed. */ -static int +static double pathbias_get_success_count(entry_guard_t *guard) { if (pathbias_use_close_counts(get_options())) { @@ -1663,11 +1663,11 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) entry_guards_changed(); - if (guard->circ_attempts > (unsigned)pathbias_get_min_circs(options)) { + if (guard->circ_attempts > pathbias_get_min_circs(options)) { /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) + if (pathbias_get_success_count(guard)/guard->circ_attempts < pathbias_get_extreme_rate(options)) { /* Dropping is currently disabled by default. */ if (pathbias_get_dropguards(options)) { @@ -1680,9 +1680,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); guard->path_bias_disabled = 1; guard->bad_since = approx_time(); @@ -1698,9 +1698,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) @@ -1716,9 +1716,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) @@ -1732,45 +1732,40 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } } } /* If we get a ton of circuits, just scale everything down */ - if (guard->circ_attempts > (unsigned)pathbias_get_scale_threshold(options)) { + if (guard->circ_attempts > pathbias_get_scale_threshold(options)) { const int scale_factor = pathbias_get_scale_factor(options); const int mult_factor = pathbias_get_mult_factor(options); - /* Only scale if there will be no rounding error for our scaling - * factors */ - if (((mult_factor*guard->circ_attempts) % scale_factor) == 0 && - ((mult_factor*guard->circ_successes) % scale_factor) == 0) { - log_info(LD_CIRC, - "Scaling pathbias counts to (%u/%u)*(%d/%d) for guard %s=%s", - guard->circ_successes, guard->circ_attempts, mult_factor, - scale_factor, guard->nickname, hex_str(guard->identity, - DIGEST_LEN)); - - guard->circ_attempts *= mult_factor; - guard->circ_successes *= mult_factor; - guard->timeouts *= mult_factor; - guard->successful_circuits_closed *= mult_factor; - guard->collapsed_circuits *= mult_factor; - guard->unusable_circuits *= mult_factor; - - guard->circ_attempts /= scale_factor; - guard->circ_successes /= scale_factor; - guard->timeouts /= scale_factor; - guard->successful_circuits_closed /= scale_factor; - guard->collapsed_circuits /= scale_factor; - guard->unusable_circuits /= scale_factor; - } + log_info(LD_CIRC, + "Scaling pathbias counts to (%lf/%lf)*(%d/%d) for guard %s=%s", + guard->circ_successes, guard->circ_attempts, + mult_factor, scale_factor, guard->nickname, + hex_str(guard->identity, DIGEST_LEN)); + + guard->circ_attempts *= mult_factor; + guard->circ_successes *= mult_factor; + guard->timeouts *= mult_factor; + guard->successful_circuits_closed *= mult_factor; + guard->collapsed_circuits *= mult_factor; + guard->unusable_circuits *= mult_factor; + + guard->circ_attempts /= scale_factor; + guard->circ_successes /= scale_factor; + guard->timeouts /= scale_factor; + guard->successful_circuits_closed /= scale_factor; + guard->collapsed_circuits /= scale_factor; + guard->unusable_circuits /= scale_factor; } guard->circ_attempts++; - log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", + log_info(LD_CIRC, "Got success count %lf/%lf for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); return 0; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 84764d1..21c09f7 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1021,7 +1021,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { const or_options_t *options = get_options(); - unsigned hop_cnt, success_cnt, timeouts, collapsed, successful_closed, + double hop_cnt, success_cnt, timeouts, collapsed, successful_closed, unusable; if (!node) { @@ -1031,20 +1031,22 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - // XXX: We want to convert this to floating point before merge /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - if (tor_sscanf(line->value, "%u %u %u %u %u %u", + if (tor_sscanf(line->value, "%lf %lf %lf %lf %lf %lf", &hop_cnt, &success_cnt, &successful_closed, &collapsed, &unusable, &timeouts) != 6) { - if (tor_sscanf(line->value, "%u %u", &success_cnt, &hop_cnt) != 2) { + int old_success, old_hops; + if (tor_sscanf(line->value, "%u %u", &old_success, &old_hops) != 2) { continue; } log_info(LD_GENERAL, "Reading old-style EntryGuardPathBias %s", escaped(line->value)); - successful_closed = success_cnt; + success_cnt = old_success; + successful_closed = old_success; + hop_cnt = old_hops; timeouts = 0; collapsed = 0; unusable = 0; @@ -1058,7 +1060,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->collapsed_circuits = collapsed; node->unusable_circuits = unusable; - log_info(LD_GENERAL, "Read %u/%u path bias for node %s", + log_info(LD_GENERAL, "Read %lf/%lf path bias for node %s", node->circ_successes, node->circ_attempts, node->nickname); /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't @@ -1068,7 +1070,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, - "Path bias is too high (%u/%u); disabling node %s", + "Path bias is too high (%lf/%lf); disabling node %s", node->circ_successes, node->circ_attempts, node->nickname); } @@ -1198,7 +1200,7 @@ entry_guards_update_state(or_state_t *state) /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - tor_asprintf(&line->value, "%u %u %u %u %u %u", + tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", e->circ_attempts, e->circ_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 0e58802..de8c60c 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -48,17 +48,17 @@ typedef struct entry_guard_t { time_t last_attempted; /**< 0 if we can connect to this guard, or the time * at which we last failed to connect to it. */ - unsigned circ_attempts; /**< Number of circuits this guard has "attempted" */ - unsigned circ_successes; /**< Number of successfully built circuits using + double circ_attempts; /**< Number of circuits this guard has "attempted" */ + double circ_successes; /**< Number of successfully built circuits using * this guard as first hop. */ - unsigned successful_circuits_closed; /**< Number of circuits that carried + double successful_circuits_closed; /**< Number of circuits that carried * streams successfully. */ - unsigned collapsed_circuits; /**< Number of fully built circuits that were + double collapsed_circuits; /**< Number of fully built circuits that were * remotely closed before any streams were * attempted. */ - unsigned unusable_circuits; /**< Number of circuits for which streams were + double unusable_circuits; /**< Number of circuits for which streams were * attempted, but none succeeded. */ - unsigned timeouts; /**< Number of 'right-censored' circuit timeouts for this + double timeouts; /**< Number of 'right-censored' circuit timeouts for this * guard. */ } entry_guard_t; @@ -111,7 +111,7 @@ int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, int validate_pluggable_transports_config(void); -int pathbias_get_closed_count(entry_guard_t *gaurd); +double pathbias_get_closed_count(entry_guard_t *gaurd); #endif

1 0

[tor/master] Flag cannibalized circs as used (non-ideal).
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 930fbb2fec2b0c4e56cc4f10f8faec9d0d135274 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 19:18:04 2012 -0800 Flag cannibalized circs as used (non-ideal). Also add some comments. --- src/or/circuitbuild.c | 4 ++++ src/or/circuituse.c | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index c3a5827..7282d57 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1219,6 +1219,10 @@ pathbias_count_first_hop(origin_circuit_t *circ) return 0; } + // XXX: Technically, we could make this only count from the *second* hop.. + // Until we get per-hop MACs or a lower circ failure rate, this might be + // better from a false positive POV. Should we s/first_hop/circ_attempt/g? + // Then we can control this check from the consensus. if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { /* Help track down the real cause of bug #6475: */ if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) { diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 0b799b1..781e984 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1160,6 +1160,17 @@ circuit_has_opened(origin_circuit_t *circ) { control_event_circuit_status(circ, CIRC_EVENT_BUILT, 0); + /* Cannibalized circuits count as used for path bias. + * (PURPOSE_GENERAL circs especially, since they are + * marked dirty and often go unused after preemptive + * building). */ + // XXX: Cannibalized now use RELAY_EARLY, which is visible + // to taggers end-to-end! We really need to probe these instead. + if (circ->has_opened && + circ->build_state->desired_path_len > DEFAULT_ROUTE_LEN) { + circ->path_state = PATH_STATE_USE_SUCCEEDED; + } + /* Remember that this circuit has finished building. Now if we start * it building again later (e.g. by extending it), we will know not * to consider its build time. */ @@ -1411,7 +1422,11 @@ circuit_launch_by_extend_info(uint8_t purpose, * * Same deal goes for client side introductions. Clients * can be manipulated to connect repeatedly to them - * (especially web clients). */ + * (especially web clients). + * + * If we decide to probe the initial portion of these circs, + * (up to the adversaries final hop), we need to remove this. + */ circ->path_state = PATH_STATE_USE_SUCCEEDED; /* This must be called before the purpose change */ pathbias_check_close(circ);

1 0

[tor/master] Change from first hop accounting to 2nd hop accounting
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 04866055e8dadc9eb5b09773b3bbdc81e3b4dbbf Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:02:41 2012 -0800 Change from first hop accounting to 2nd hop accounting This has several advantages, including more resilience to ambient failure. I still need to rename all the first_hop vars tho.. Saving that for a separate commit. --- src/or/circuitbuild.c | 34 +++++++++++++++++++++++++++++----- 1 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 7282d57..ad52a6c 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1146,6 +1146,34 @@ pathbias_state_to_string(path_state_t state) } /** + * This function decides if a circuit has progressed far enough to count + * as a circuit "attempt". As long as end-to-end tagging is possible, + * we assume the adversary will use it over hop-to-hop failure. Therefore, + * we only need to account bias for the last hop. This should make us + * much more resilient to ambient circuit failure, and also make that + * failure easier to measure (we only need to measure Exit failure rates). + */ +static int +pathbias_is_new_circ_attempt(origin_circuit_t *circ) +{ +#define N2N_TAGGING_IS_POSSIBLE +#ifdef N2N_TAGGING_IS_POSSIBLE + /* cpath is a circular list. We want circs with more than one hop, + * and the second hop must be waiting for keys still (it's just + * about to get them). */ + return circ->cpath->next != circ->cpath && + circ->cpath->next->state == CPATH_STATE_AWAITING_KEYS; +#else + /* If tagging attacks are no longer possible, we probably want to + * count bias from the first hop. However, one could argue that + * timing-based tagging is still more useful than per-hop failure. + * In which case, we'd never want to use this. + */ + return circ->cpath->state == CPATH_STATE_AWAITING_KEYS; +#endif +} + +/** * Decide if the path bias code should count a circuit. * * @returns 1 if we should count it, 0 otherwise. @@ -1219,11 +1247,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) return 0; } - // XXX: Technically, we could make this only count from the *second* hop.. - // Until we get per-hop MACs or a lower circ failure rate, this might be - // better from a false positive POV. Should we s/first_hop/circ_attempt/g? - // Then we can control this check from the consensus. - if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { + if (pathbias_is_new_circ_attempt(circ)) { /* Help track down the real cause of bug #6475: */ if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) { if ((rate_msg = rate_limit_log(&first_hop_notice_limit,

1 0

[tor/master] Space fixes.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 4590993ff3d4393caaa1d9d68d04cf0af95c23c7 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 23:47:04 2012 -0800 Space fixes. --- src/or/circuitbuild.c | 38 +++++++++++++++++++++----------------- src/or/circuitbuild.h | 1 + src/or/circuitlist.c | 6 +++--- src/or/circuituse.c | 4 ++-- src/or/connection_edge.c | 6 +++--- src/or/entrynodes.c | 4 ++-- src/or/entrynodes.h | 2 +- src/or/relay.c | 2 +- src/or/rendservice.c | 4 ++-- 9 files changed, 36 insertions(+), 31 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 349063d..a724006 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1199,7 +1199,7 @@ pathbias_should_count(origin_circuit_t *circ) 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_C_INTRODUCING && + (circ->base_.purpose >= CIRCUIT_PURPOSE_C_INTRODUCING && circ->base_.purpose <= CIRCUIT_PURPOSE_C_INTRODUCE_ACKED)) { return 0; } @@ -1350,7 +1350,7 @@ pathbias_count_build_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) { @@ -1456,14 +1456,14 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) == END_CIRC_REASON_CHANNEL_CLOSED && circ->n_chan && - circ->n_chan->reason_for_closing + 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 %d's channel closed without successful use for reason %d, " - "channel reason %d. Circuit purpose %d currently %s.", + "Circuit %d's channel closed without successful use for reason " + "%d, channel reason %d. Circuit purpose %d currently %s.", ocirc->global_identifier, reason, circ->n_chan->reason_for_closing, circ->purpose, circuit_state_to_string(circ->state)); @@ -1494,7 +1494,7 @@ pathbias_count_successful_close(origin_circuit_t *circ) } if (guard) { - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * circ_failure + stream_failure */ guard->successful_circuits_closed++; entry_guards_changed(); @@ -1511,7 +1511,7 @@ pathbias_count_successful_close(origin_circuit_t *circ) } /** - * Count a circuit that fails after it is built, but before it can + * Count a circuit that fails after it is built, but before it can * carry any traffic. * * This is needed because there are ways to destroy a @@ -1622,7 +1622,7 @@ pathbias_get_closed_count(entry_guard_t *guard) ocirc = TO_ORIGIN_CIRCUIT(circ); - if(!ocirc->cpath || !ocirc->cpath->extend_info) + if (!ocirc->cpath || !ocirc->cpath->extend_info) continue; if (ocirc->path_state >= PATH_STATE_BUILD_SUCCEEDED && @@ -1642,7 +1642,7 @@ pathbias_get_closed_count(entry_guard_t *guard) * if it should return guard->circ_successes or * guard->successful_circuits_closed. */ -double +double pathbias_get_success_count(entry_guard_t *guard) { if (pathbias_use_close_counts(get_options())) { @@ -1680,8 +1680,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); guard->path_bias_disabled = 1; @@ -1698,8 +1699,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1716,8 +1718,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1732,8 +1735,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 53c9fe5..8cd61fa 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -59,3 +59,4 @@ void pathbias_count_timeout(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 6fab492..0ee2900 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1040,9 +1040,9 @@ circuit_unlink_all_from_channel(channel_t *chan, int reason) if (circ->n_chan == chan) { 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. */ + * bit to mark_for_close. */ if (chan->reason_for_closing != CHANNEL_CLOSE_REQUESTED) reason |= END_CIRC_REASON_FLAG_REMOTE; } @@ -1352,7 +1352,7 @@ circuit_mark_for_close_(circuit_t *circ, int reason, int line, } reason = END_CIRC_REASON_NONE; } - + if (CIRCUIT_IS_ORIGIN(circ)) { pathbias_check_close(TO_ORIGIN_CIRCUIT(circ), reason); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 381c2b0..d3b4801 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1161,7 +1161,7 @@ circuit_has_opened(origin_circuit_t *circ) control_event_circuit_status(circ, CIRC_EVENT_BUILT, 0); /* Cannibalized circuits count as used for path bias. - * (PURPOSE_GENERAL circs especially, since they are + * (PURPOSE_GENERAL circs especially, since they are * marked dirty and often go unused after preemptive * building). */ // XXX: Cannibalized now use RELAY_EARLY, which is visible @@ -1418,7 +1418,7 @@ circuit_launch_by_extend_info(uint8_t purpose, 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 diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index ca6060c..570ffe4 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2184,13 +2184,13 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, endreason == END_STREAM_REASON_CONNRESET || endreason == END_STREAM_REASON_NOROUTE || endreason == END_STREAM_REASON_RESOURCELIMIT) { - if(!conn->edge_.on_circuit || + if (!conn->edge_.on_circuit || !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) { // DNS remaps can trigger this. So can failed hidden service // lookups. log_info(LD_BUG, - "(Harmless.) No origin circuit for successful SOCKS stream %ld. " - "Reason: %d", ENTRY_TO_CONN(conn)->global_identifier, endreason); + "No origin circuit for successful SOCKS stream %ld. Reason: " + "%d", ENTRY_TO_CONN(conn)->global_identifier, endreason); } else { TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state = PATH_STATE_USE_SUCCEEDED; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 96b075a..066dbec 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1031,7 +1031,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ if (tor_sscanf(line->value, "%lf %lf %lf %lf %lf %lf", @@ -1197,7 +1197,7 @@ entry_guards_update_state(or_state_t *state) if (e->circ_attempts) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index b9d0e55..2686a4f 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -56,7 +56,7 @@ typedef struct entry_guard_t { double collapsed_circuits; /**< Number of fully built circuits that were * remotely closed before any streams were * attempted. */ - double unusable_circuits; /**< Number of circuits for which streams were + double unusable_circuits; /**< Number of circuits for which streams were * attempted, but none succeeded. */ double timeouts; /**< Number of 'right-censored' circuit timeouts for this * guard. */ diff --git a/src/or/relay.c b/src/or/relay.c index 1638dae..3ee0c83 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -694,7 +694,7 @@ connection_ap_process_end_not_open( (void) layer_hint; /* unused */ if (rh->length > 0) { - /* Path bias: If we get a valid reason code from the exit, + /* Path bias: If we get a valid reason code from the exit, * it wasn't due to tagging */ // XXX: This relies on recognized+digest being strong enough not // to be spoofable.. Is that a valid assumption? diff --git a/src/or/rendservice.c b/src/or/rendservice.c index fbf14e9..e70f969 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1383,7 +1383,7 @@ 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 intro circuit was used successfully */ circuit->path_state = PATH_STATE_USE_SUCCEEDED; @@ -2586,7 +2586,7 @@ 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 */ - if(!circuit->base_.timestamp_dirty) + 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] Allow any valid 'end' cell to mean a circuit was used successfully.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 686fc222593fd46ec82d62f0fa62ca02900c1014 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sat Dec 8 16:37:22 2012 -0800 Allow any valid 'end' cell to mean a circuit was used successfully. Also improve some log messages. --- src/or/circuitbuild.c | 9 ++++++--- src/or/connection_edge.c | 17 +++++++++++------ src/or/relay.c | 8 ++++++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 9b1236f..c3a5827 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1411,16 +1411,18 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) // state.. Can we use that? Does optimistic data change this? log_info(LD_CIRC, - "Circuit closed without successful use for reason %d. " + "Circuit %d closed without successful use for reason %d. " "Circuit purpose %d currently %s.", + ocirc->global_identifier, 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 */ log_info(LD_CIRC, - "Circuit remote-closed without successful use for reason %d. " + "Circuit %d remote-closed without successful use for reason %d. " "Circuit purpose %d currently %s.", + ocirc->global_identifier, reason, circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_collapse(ocirc); } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) @@ -1432,8 +1434,9 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) /* 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, " + "Circuit %d's channel closed without successful use for reason %d, " "channel reason %d. Circuit purpose %d currently %s.", + ocirc->global_identifier, reason, circ->n_chan->reason_for_closing, circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_collapse(ocirc); diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 79bb54c..ca6060c 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2178,14 +2178,19 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, /* Flag this stream's circuit as having completed a stream successfully * (for path bias) */ - if (status == SOCKS5_SUCCEEDED) { + if (status == SOCKS5_SUCCEEDED || + endreason == END_STREAM_REASON_RESOLVEFAILED || + endreason == END_STREAM_REASON_CONNECTREFUSED || + endreason == END_STREAM_REASON_CONNRESET || + endreason == END_STREAM_REASON_NOROUTE || + endreason == END_STREAM_REASON_RESOURCELIMIT) { 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. - // Maybe during addrmaps/resolves? - log_warn(LD_BUG, - "(Harmless.) No origin circuit for successful SOCKS stream. " - "Reason: %d", endreason); + // DNS remaps can trigger this. So can failed hidden service + // lookups. + log_info(LD_BUG, + "(Harmless.) No origin circuit for successful SOCKS stream %ld. " + "Reason: %d", ENTRY_TO_CONN(conn)->global_identifier, endreason); } else { TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state = PATH_STATE_USE_SUCCEEDED; diff --git a/src/or/relay.c b/src/or/relay.c index 7f49299..1638dae 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -693,6 +693,14 @@ connection_ap_process_end_not_open( edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(conn); (void) layer_hint; /* unused */ + if (rh->length > 0) { + /* Path bias: If we get a valid reason code from the exit, + * it wasn't due to tagging */ + // XXX: This relies on recognized+digest being strong enough not + // to be spoofable.. Is that a valid assumption? + circ->path_state = PATH_STATE_USE_SUCCEEDED; + } + if (rh->length > 0 && edge_reason_is_retriable(reason) && /* avoid retry if rend */ !connection_edge_is_rendezvous_stream(edge_conn)) {

1 0