diff --git a/changes/bug6174 b/changes/bug6174 new file mode 100644 index 0000000000..79d2930ec3 --- /dev/null +++ b/changes/bug6174 @@ -0,0 +1,6 @@ + o Major bugfixes: + - When we mark a circuit as unusable for new circuits, have it + continue to be unusable for new circuits even if MaxCircuitDirtiness + is increased too much at the wrong time, or the system clock jumped + backwards. Fix for bug 6174; bugfix on 0.0.2pre26. + diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index bb5b253c83..8e768e76f5 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -901,7 +901,7 @@ circuit_note_clock_jumped(int seconds_elapsed) control_event_client_status(severity, "CIRCUIT_NOT_ESTABLISHED REASON=%s", "CLOCK_JUMPED"); circuit_mark_all_unused_circs(); - circuit_expire_all_dirty_circs(); + circuit_mark_all_dirty_circs_as_unusable(); } /** Take the 'extend' cell, pull out addr/port plus the onion diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index d4c07fc82b..6c7629c7ba 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1207,6 +1207,7 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, if ((!need_uptime || circ->build_state->need_uptime) && (!need_capacity || circ->build_state->need_capacity) && (internal == circ->build_state->is_internal) && + !circ->unusable_for_new_conns && circ->remaining_relay_early_cells && circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN && !circ->build_state->onehop_tunnel && @@ -1302,20 +1303,17 @@ circuit_mark_all_unused_circs(void) * This is useful for letting the user change pseudonyms, so new * streams will not be linkable to old streams. */ -/* XXX024 this is a bad name for what this function does */ void -circuit_expire_all_dirty_circs(void) +circuit_mark_all_dirty_circs_as_unusable(void) { circuit_t *circ; - const or_options_t *options = get_options(); for (circ=global_circuitlist; circ; circ = circ->next) { if (CIRCUIT_IS_ORIGIN(circ) && !circ->marked_for_close && - circ->timestamp_dirty) - /* XXXX024 This is a screwed-up way to say "This is too dirty - * for new circuits. */ - circ->timestamp_dirty -= options->MaxCircuitDirtiness; + circ->timestamp_dirty) { + mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ)); + } } } diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index e81c0785fe..d67f80b065 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -46,7 +46,7 @@ or_circuit_t *circuit_get_intro_point(const char *digest); origin_circuit_t *circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, int flags); void circuit_mark_all_unused_circs(void); -void circuit_expire_all_dirty_circs(void); +void circuit_mark_all_dirty_circs_as_unusable(void); void circuit_mark_for_close_(circuit_t *circ, int reason, int line, const char *file); int circuit_get_cpath_len(origin_circuit_t *circ); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 51d8716faa..4d1065bbd8 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -85,10 +85,14 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ, } if (purpose == CIRCUIT_PURPOSE_C_GENERAL || - purpose == CIRCUIT_PURPOSE_C_REND_JOINED) + purpose == CIRCUIT_PURPOSE_C_REND_JOINED) { if (circ->timestamp_dirty && circ->timestamp_dirty+get_options()->MaxCircuitDirtiness <= now) return 0; + } + + if (origin_circ->unusable_for_new_conns) + return 0; /* decide if this circ is suitable for this conn */ @@ -799,9 +803,12 @@ circuit_stream_is_being_handled(entry_connection_t *conn, circ->purpose == CIRCUIT_PURPOSE_C_GENERAL && (!circ->timestamp_dirty || circ->timestamp_dirty + get_options()->MaxCircuitDirtiness > now)) { - cpath_build_state_t *build_state = TO_ORIGIN_CIRCUIT(circ)->build_state; + origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); + cpath_build_state_t *build_state = origin_circ->build_state; if (build_state->is_internal || build_state->onehop_tunnel) continue; + if (!origin_circ->unusable_for_new_conns) + continue; exitnode = build_state_get_exit_node(build_state); if (exitnode && (!need_uptime || build_state->need_uptime)) { @@ -843,6 +850,7 @@ circuit_predict_and_launch_new(void) /* First, count how many of each type of circuit we have already. */ for (circ=global_circuitlist;circ;circ = circ->next) { cpath_build_state_t *build_state; + origin_circuit_t *origin_circ; if (!CIRCUIT_IS_ORIGIN(circ)) continue; if (circ->marked_for_close) @@ -851,7 +859,10 @@ circuit_predict_and_launch_new(void) continue; /* only count clean circs */ if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL) continue; /* only pay attention to general-purpose circs */ - build_state = TO_ORIGIN_CIRCUIT(circ)->build_state; + origin_circ = TO_ORIGIN_CIRCUIT(circ); + if (origin_circ->unusable_for_new_conns) + continue; + build_state = origin_circ->build_state; if (build_state->onehop_tunnel) continue; num++; @@ -2275,3 +2286,22 @@ circuit_change_purpose(circuit_t *circ, uint8_t new_purpose) } } +/** Mark circ so that no more connections can be attached to it. */ +void +mark_circuit_unusable_for_new_conns(origin_circuit_t *circ) +{ + const or_options_t *options = get_options(); + tor_assert(circ); + + /* XXXX025 This is a kludge; we're only keeping it around in case there's + * something that doesn't check unusable_for_new_conns, and to avoid + * deeper refactoring of our expiration logic. */ + if (! circ->base_.timestamp_dirty) + circ->base_.timestamp_dirty = approx_time(); + if (options->MaxCircuitDirtiness >= circ->base_.timestamp_dirty) + circ->base_.timestamp_dirty = 1; /* prevent underflow */ + else + circ->base_.timestamp_dirty -= options->MaxCircuitDirtiness; + + circ->unusable_for_new_conns = 1; +} diff --git a/src/or/circuituse.h b/src/or/circuituse.h index d4d68aad92..11e5a64163 100644 --- a/src/or/circuituse.h +++ b/src/or/circuituse.h @@ -55,6 +55,7 @@ void circuit_change_purpose(circuit_t *circ, uint8_t new_purpose); int hostname_in_track_host_exits(const or_options_t *options, const char *address); +void mark_circuit_unusable_for_new_conns(origin_circuit_t *circ); #endif diff --git a/src/or/config.c b/src/or/config.c index da5f97eace..0ebf3b6942 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1503,7 +1503,7 @@ options_act(const or_options_t *old_options) "preferred or excluded node lists. " "Abandoning previous circuits."); circuit_mark_all_unused_circs(); - circuit_expire_all_dirty_circs(); + circuit_mark_all_dirty_circs_as_unusable(); revise_trackexithosts = 1; } diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 0660b1fafd..9c39c25219 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -674,12 +674,10 @@ connection_ap_expire_beginning(void) /* un-mark it as ending, since we're going to reuse it */ conn->edge_has_sent_end = 0; conn->end_reason = 0; - /* kludge to make us not try this circuit again, yet to allow - * current streams on it to survive if they can: make it - * unattractive to use for new streams */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->timestamp_dirty); - circ->timestamp_dirty -= options->MaxCircuitDirtiness; + /* make us not try this circuit again, but allow + * current streams on it to survive if they can */ + mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ)); + /* give our stream another 'cutoff' seconds to try */ conn->base_.timestamp_lastread += cutoff; if (entry_conn->num_socks_retries < 250) /* avoid overflow */ @@ -1806,9 +1804,7 @@ connection_ap_handshake_send_begin(entry_connection_t *ap_conn) connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); /* Mark this circuit "unusable for new streams". */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->base_.timestamp_dirty); - circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness; + mark_circuit_unusable_for_new_conns(circ); return -1; } @@ -1899,9 +1895,7 @@ connection_ap_handshake_send_resolve(entry_connection_t *ap_conn) connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL); /* Mark this circuit "unusable for new streams". */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->base_.timestamp_dirty); - circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness; + mark_circuit_unusable_for_new_conns(circ); return -1; } diff --git a/src/or/main.c b/src/or/main.c index 75a0971534..fd8b6cf674 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1125,7 +1125,7 @@ signewnym_impl(time_t now) return; } - circuit_expire_all_dirty_circs(); + circuit_mark_all_dirty_circs_as_unusable(); addressmap_clear_transient(); rend_client_purge_state(); time_of_last_signewnym = now; @@ -1844,7 +1844,7 @@ do_hup(void) /* Rotate away from the old dirty circuits. This has to be done * after we've read the new options, but before we start using * circuits for directory fetches. */ - circuit_expire_all_dirty_circs(); + circuit_mark_all_dirty_circs_as_unusable(); /* retry appropriate downloads */ router_reset_status_download_failures(); diff --git a/src/or/or.h b/src/or/or.h index a341109d4d..08b32d5ec3 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2945,6 +2945,10 @@ typedef struct origin_circuit_t { */ ENUM_BF(path_state_t) path_state : 3; + /* If this flag is set, we should not consider attaching any more + * connections to this circuit. */ + unsigned int unusable_for_new_conns : 1; + /** * Tristate variable to guard against pathbias miscounting * due to circuit purpose transitions changing the decision diff --git a/src/or/relay.c b/src/or/relay.c index 8e80d14a87..ef37c32810 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -17,6 +17,7 @@ #include "channel.h" #include "circuitbuild.h" #include "circuitlist.h" +#include "circuituse.h" #include "config.h" #include "connection.h" #include "connection_edge.h" @@ -851,9 +852,7 @@ connection_ap_process_end_not_open( /* We haven't retried too many times; reattach the connection. */ circuit_log_path(LOG_INFO,LD_APP,circ); /* Mark this circuit "unusable for new streams". */ - /* XXXX024 this is a kludgy way to do this. */ - tor_assert(circ->base_.timestamp_dirty); - circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness; + mark_circuit_unusable_for_new_conns(circ); if (conn->chosen_exit_optional) { /* stop wanting a specific exit */