From dd6bdab3f6254e7245f8ee76b2c6b4314f0472fa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 18 Nov 2016 16:05:09 -0500 Subject: Write the easy parts of the public entryguard interface. Here we add a little bit of state to origin circuits, and set up the necessary functions for the circuit code to call in order to find guards, use guards, and decide when circuits can be used. There's also an incomplete function for the hard part of the circuit-maintenance code, where we figure out whether any waiting guards are ready to become usable. (This patch finally uses the handle.c code to make safe handles to entry_guard_t objects, so that we are allowed to free an entry_guard_t without checking whether any origin_circuit_t is holding a reference to it.) --- src/or/circuitlist.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index dee103e36a..f13126d76b 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -63,6 +63,7 @@ #include "connection_edge.h" #include "connection_or.h" #include "control.h" +#include "entrynodes.h" #include "main.h" #include "hs_common.h" #include "networkstatus.h" @@ -834,6 +835,7 @@ circuit_free(circuit_t *circ) cpath_ref_decref(ocirc->build_state->service_pending_final_cpath_ref); } tor_free(ocirc->build_state); + circuit_guard_state_free(ocirc->guard_state); circuit_clear_cpath(ocirc); -- cgit v1.2.3-54-g00ecf From 238828c92b1cc186577e44490caf4fa3870e724d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 21 Nov 2016 16:59:00 -0500 Subject: Add a new GUARD_WAIT state for circuits This state corresponds to the WAITING_FOR_BETTER_GUARD state; it's for circuits that are 100% constructed, but which we won't use until we are sure that we wouldn't use circuits with a better guard. --- src/or/circuitlist.c | 20 ++++++++++++++++---- src/or/control.c | 5 ++++- src/or/or.h | 6 +++++- 3 files changed, 25 insertions(+), 6 deletions(-) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index f13126d76b..0189412df0 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -439,7 +439,13 @@ circuit_set_state(circuit_t *circ, uint8_t state) /* add to waiting-circuit list. */ smartlist_add(circuits_pending_chans, circ); } - if (state == CIRCUIT_STATE_OPEN) + if (circ->state == CIRCUIT_STATE_GUARD_WAIT) { + smartlist_remove(circuits_pending_other_guards, circ); + } + if (state == CIRCUIT_STATE_GUARD_WAIT) { + smartlist_add(circuits_pending_other_guards, circ); + } + if (state == CIRCUIT_STATE_GUARD_WAIT || state == CIRCUIT_STATE_OPEN) tor_assert(!circ->n_chan_create_cell); circ->state = state; } @@ -542,6 +548,8 @@ circuit_state_to_string(int state) case CIRCUIT_STATE_BUILDING: return "doing handshakes"; case CIRCUIT_STATE_ONIONSKIN_PENDING: return "processing the onion"; case CIRCUIT_STATE_CHAN_WAIT: return "connecting to server"; + case CIRCUIT_STATE_GUARD_WAIT: return "waiting to see how other " + "guards perform"; case CIRCUIT_STATE_OPEN: return "open"; default: log_warn(LD_BUG, "Unknown circuit state %d", state); @@ -1868,7 +1876,8 @@ circuit_about_to_free(circuit_t *circ) * module then. If it isn't OPEN, we send it there now to remember which * links worked and which didn't. */ - if (circ->state != CIRCUIT_STATE_OPEN) { + if (circ->state != CIRCUIT_STATE_OPEN && + circ->state != CIRCUIT_STATE_GUARD_WAIT) { if (CIRCUIT_IS_ORIGIN(circ)) { origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ); circuit_build_failed(ocirc); /* take actions if necessary */ @@ -1881,7 +1890,9 @@ circuit_about_to_free(circuit_t *circ) } if (CIRCUIT_IS_ORIGIN(circ)) { control_event_circuit_status(TO_ORIGIN_CIRCUIT(circ), - (circ->state == CIRCUIT_STATE_OPEN)?CIRC_EVENT_CLOSED:CIRC_EVENT_FAILED, + (circ->state == CIRCUIT_STATE_OPEN || + circ->state == CIRCUIT_STATE_GUARD_WAIT) ? + CIRC_EVENT_CLOSED:CIRC_EVENT_FAILED, orig_reason); } @@ -2403,7 +2414,8 @@ assert_circuit_ok(const circuit_t *c) tor_assert(c->deliver_window >= 0); tor_assert(c->package_window >= 0); - if (c->state == CIRCUIT_STATE_OPEN) { + if (c->state == CIRCUIT_STATE_OPEN || + c->state == CIRCUIT_STATE_GUARD_WAIT) { tor_assert(!c->n_chan_create_cell); if (or_circ) { tor_assert(or_circ->n_crypto); diff --git a/src/or/control.c b/src/or/control.c index 1d7ec1b57b..9cc99b6b96 100644 --- a/src/or/control.c +++ b/src/or/control.c @@ -2596,6 +2596,8 @@ getinfo_helper_events(control_connection_t *control_conn, if (circ->base_.state == CIRCUIT_STATE_OPEN) state = "BUILT"; + else if (circ->base_.state == CIRCUIT_STATE_GUARD_WAIT) + state = "GUARD_WAIT"; // XXXX prop271 must specify this. else if (circ->cpath) state = "EXTENDED"; else @@ -3379,7 +3381,8 @@ handle_control_extendcircuit(control_connection_t *conn, uint32_t len, goto done; } } else { - if (circ->base_.state == CIRCUIT_STATE_OPEN) { + if (circ->base_.state == CIRCUIT_STATE_OPEN || + circ->base_.state == CIRCUIT_STATE_GUARD_WAIT) { int err_reason = 0; circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_BUILDING); if ((err_reason = circuit_send_next_onion_skin(circ)) < 0) { diff --git a/src/or/or.h b/src/or/or.h index 4bc806619c..eb0025d100 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -438,8 +438,12 @@ typedef enum { /** Circuit state: I'd like to deliver a create, but my n_chan is still * connecting. */ #define CIRCUIT_STATE_CHAN_WAIT 2 +/** Circuit state: the circuit is open but we don't want to actually use it + * until we find out if a better guard will be available. + */ +#define CIRCUIT_STATE_GUARD_WAIT 3 /** Circuit state: onionskin(s) processed, ready to send/receive cells. */ -#define CIRCUIT_STATE_OPEN 3 +#define CIRCUIT_STATE_OPEN 4 #define CIRCUIT_PURPOSE_MIN_ 1 -- cgit v1.2.3-54-g00ecf From de617a471442342fc2abafdde4e250fd31eb45ac Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 22 Nov 2016 09:05:52 -0500 Subject: Maintain a list of all the origin circuits. We'll want this for upgrading waiting circuits. --- src/or/circuitlist.c | 39 +++++++++++++++++++++++++++++++++++++++ src/or/or.h | 4 ++++ 2 files changed, 43 insertions(+) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 0189412df0..c274534759 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -85,6 +85,10 @@ /** A global list of all circuits at this hop. */ static smartlist_t *global_circuitlist = NULL; +/** A global list of all origin circuits. Every element of this is also + * an element of global_circuitlist. */ +static smartlist_t *global_origin_circuit_list = NULL; + /** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */ static smartlist_t *circuits_pending_chans = NULL; @@ -523,6 +527,19 @@ circuit_close_all_marked(void) } circ->global_circuitlist_idx = -1; + /* Remove it from the origin circuit list, if appropriate. */ + if (CIRCUIT_IS_ORIGIN(circ)) { + origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); + int origin_idx = origin_circ->global_origin_circuit_list_idx; + smartlist_del(global_origin_circuit_list, origin_idx); + if (origin_idx < smartlist_len(global_origin_circuit_list)) { + origin_circuit_t *replacement = + smartlist_get(global_origin_circuit_list, origin_idx); + replacement->global_origin_circuit_list_idx = origin_idx; + } + origin_circ->global_origin_circuit_list_idx = -1; + } + circuit_about_to_free(circ); circuit_free(circ); } SMARTLIST_FOREACH_END(circ); @@ -780,6 +797,13 @@ origin_circuit_new(void) init_circuit_base(TO_CIRCUIT(circ)); + /* Add to origin-list. */ + if (!global_origin_circuit_list) + global_origin_circuit_list = smartlist_new(); + smartlist_add(global_origin_circuit_list, circ); + circ->global_origin_circuit_list_idx = + smartlist_len(global_origin_circuit_list) - 1; + circuit_build_times_update_last_circ(get_circuit_build_times_mutable()); return circ; @@ -837,6 +861,18 @@ circuit_free(circuit_t *circ) mem = ocirc; memlen = sizeof(origin_circuit_t); tor_assert(circ->magic == ORIGIN_CIRCUIT_MAGIC); + + if (ocirc->global_origin_circuit_list_idx != -1) { + int idx = ocirc->global_origin_circuit_list_idx; + origin_circuit_t *c2 = smartlist_get(global_origin_circuit_list, idx); + tor_assert(c2 == ocirc); + smartlist_del(global_origin_circuit_list, idx); + if (idx < smartlist_len(global_origin_circuit_list)) { + c2 = smartlist_get(global_origin_circuit_list, idx); + c2->global_origin_circuit_list_idx = idx; + } + } + if (ocirc->build_state) { extend_info_free(ocirc->build_state->chosen_exit); circuit_free_cpath_node(ocirc->build_state->pending_final_cpath); @@ -977,6 +1013,9 @@ circuit_free_all(void) smartlist_free(lst); global_circuitlist = NULL; + smartlist_free(global_origin_circuit_list); + global_origin_circuit_list = NULL; + smartlist_free(circuits_pending_chans); circuits_pending_chans = NULL; diff --git a/src/or/or.h b/src/or/or.h index 8282731eea..c8f39f90d9 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3157,6 +3157,10 @@ typedef struct origin_circuit_t { * whether this circuit can be used. */ struct circuit_guard_state_t *guard_state; + /** Index into global_origin_circuit_list for this circuit. -1 if not + * present. */ + int global_origin_circuit_list_idx; + /** How many more relay_early cells can we send on this circuit, according * to the specification? */ unsigned int remaining_relay_early_cells : 4; -- cgit v1.2.3-54-g00ecf From dbbaa515183e250e20c40fa7b4c00df9487058fa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 21 Nov 2016 17:23:25 -0500 Subject: Use the new guard notification/selection APIs throughout Tor This patch doesn't cover every case; omitted cases are marked with "XXXX prop271", as usual. It leaves both the old interface and the new interface for guard status notification, since they don't actually work in the same way: the new API wants to be told when a circuit has failed or succeeded, whereas the old API wants to know when a channel has failed or succeeded. I ran into some trouble with directory guard stuff, since when we pick the directory guard, we don't actually have a circuit to associate it with. I solved that by allowing guard states to be associated with directory connections, not just circuits. --- src/or/bridges.c | 2 ++ src/or/channel.c | 1 + src/or/circuitbuild.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++---- src/or/circuitbuild.h | 6 +++- src/or/circuitlist.c | 42 +++++++++++++++++++++++++++- src/or/circuitlist.h | 2 ++ src/or/circuituse.c | 2 ++ src/or/connection.c | 1 + src/or/connection_or.c | 6 ++++ src/or/directory.c | 74 +++++++++++++++++++++++++++++++++++++++---------- src/or/directory.h | 6 ++-- src/or/entrynodes.c | 60 ++++++++++++++++++++++++++++++++++++---- src/or/entrynodes.h | 2 +- src/or/main.c | 5 +++- src/or/or.h | 4 +++ src/or/rendclient.c | 2 +- src/or/rendservice.c | 2 +- src/or/routerlist.c | 4 +-- src/test/test_dir.c | 8 ++++-- 19 files changed, 268 insertions(+), 36 deletions(-) (limited to 'src/or/circuitlist.c') diff --git a/src/or/bridges.c b/src/or/bridges.c index 508c77fc9e..2170cc668a 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -724,6 +724,7 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) from_cache ? "cached" : "fresh", router_describe(ri)); /* set entry->made_contact so if it goes down we don't drop it from * our entry node list */ + // XXXX prop271 use new interface here when we hit bridges? entry_guard_register_connect_status(ri->cache_info.identity_digest, 1, 0, now); if (first) { @@ -743,6 +744,7 @@ int any_bridge_descriptors_known(void) { tor_assert(get_options()->UseBridges); + // XXXX prop271 this needs to get fixed. -- bridges return choose_random_entry(NULL) != NULL; } diff --git a/src/or/channel.c b/src/or/channel.c index af5810788c..1e3e99c51d 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2538,6 +2538,7 @@ channel_do_open_actions(channel_t *chan) if (started_here) { circuit_build_times_network_is_live(get_circuit_build_times_mutable()); rep_hist_note_connect_succeeded(chan->identity_digest, now); + // XXXX prop271 this call is no longer useful with the new algorithm. if (entry_guard_register_connect_status( chan->identity_digest, 1, 0, now) < 0) { /* Close any circuits pending on this channel. We leave it in state diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index a33c2ca654..2f4ce7a727 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -964,7 +964,35 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) memset(&ec, 0, sizeof(ec)); if (!hop) { /* done building the circuit. whew. */ - circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_OPEN); + int r; + if (! circ->guard_state) { + if (circuit_get_cpath_len(circ) != 1) { + log_warn(LD_BUG, "%d-hop circuit %p with purpose %d has no " + "guard state", + circuit_get_cpath_len(circ), circ, circ->base_.purpose); + } + r = 1; + } else { + r = entry_guard_succeeded(get_guard_selection_info(), + &circ->guard_state); + } + const int is_usable_for_streams = (r == 1); + if (r == 1) { + circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_OPEN); + } else if (r == 0) { + // XXXX prop271 we might want to probe for whether this + // XXXX one is ready even before the next second rolls over. + circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_GUARD_WAIT); + } else { + return - END_CIRC_REASON_INTERNAL; + } + + /* XXXX prop271 -- the rest of this branch needs careful thought! + * Some of the things here need to happen when a circuit becomes + * mechanically open; some need to happen when it is actually usable. + * I think I got them right, but more checking would be wise. -NM + */ + if (circuit_timeout_want_to_count_circ(circ)) { struct timeval end; long timediff; @@ -1006,7 +1034,8 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) pathbias_count_build_success(circ); circuit_rep_hist_note_result(circ); - circuit_has_opened(circ); /* do other actions as necessary */ + if (is_usable_for_streams) + circuit_has_opened(circ); /* do other actions as necessary */ if (!have_completed_a_circuit() && !circ->build_state->onehop_tunnel) { const or_options_t *options = get_options(); @@ -2206,9 +2235,20 @@ choose_good_middle_server(uint8_t purpose, * * If state is NULL, we're choosing a router to serve as an entry * guard, not for any particular circuit. + * + * Set *guard_state_out to information about the guard that + * we're selecting, which we'll use later to remember whether the + * guard worked or not. + * + * XXXX prop271 this function is used in four ways: picking out guards for + * the old (pre-prop271) guard algorithm; picking out guards for circuits; + * picking out guards for testing circuits on non-bridgees; + * picking out entries when entry guards are disabled. These options + * should be disentangled. */ const node_t * -choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) +choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state, + circuit_guard_state_t **guard_state_out) { const node_t *choice; smartlist_t *excluded; @@ -2223,7 +2263,8 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) (purpose != CIRCUIT_PURPOSE_TESTING || options->BridgeRelay)) { /* This request is for an entry server to use for a regular circuit, * and we use entry guard nodes. Just return one of the guard nodes. */ - return choose_random_entry(state); + tor_assert(guard_state_out); + return guards_choose_guard(state, guard_state_out); } excluded = smartlist_new(); @@ -2306,7 +2347,8 @@ onion_extend_cpath(origin_circuit_t *circ) if (cur_len == state->desired_path_len - 1) { /* Picking last node */ info = extend_info_dup(state->chosen_exit); } else if (cur_len == 0) { /* picking first node */ - const node_t *r = choose_good_entry_server(purpose, state); + const node_t *r = choose_good_entry_server(purpose, state, + &circ->guard_state); if (r) { /* If we're a client, use the preferred address rather than the primary address, for potentially connecting to an IPv6 OR @@ -2574,3 +2616,26 @@ extend_info_has_preferred_onion_key(const extend_info_t* ei) return extend_info_supports_ntor(ei); } +/** Find the circuits that are waiting to find out whether their guards are + * usable, and if any are ready to become usable, mark them open and try + * attaching streams as appropriate. */ +void +circuit_upgrade_circuits_from_guard_wait(void) +{ + smartlist_t *to_upgrade = + circuit_find_circuits_to_upgrade_from_guard_wait(); + + if (to_upgrade == NULL) + return; + + log_info(LD_GUARD, "Upgrading %d circuits from 'waiting for better guard' " + "to 'open'.", smartlist_len(to_upgrade)); + + SMARTLIST_FOREACH_BEGIN(to_upgrade, origin_circuit_t *, circ) { + circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_OPEN); + circuit_has_opened(circ); + } SMARTLIST_FOREACH_END(circ); + + smartlist_free(to_upgrade); +} + diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 56f66a19d6..2c83a16550 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -64,8 +64,12 @@ int extend_info_has_preferred_onion_key(const extend_info_t* ei); const node_t *build_state_get_exit_node(cpath_build_state_t *state); const char *build_state_get_exit_nickname(cpath_build_state_t *state); +struct circuit_guard_state_t; + const node_t *choose_good_entry_server(uint8_t purpose, - cpath_build_state_t *state); + cpath_build_state_t *state, + struct circuit_guard_state_t **guard_state_out); +void circuit_upgrade_circuits_from_guard_wait(void); #ifdef CIRCUITBUILD_PRIVATE STATIC circid_t get_unique_circ_id_by_chan(channel_t *chan); diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index c274534759..2a03f8a0d9 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -92,6 +92,10 @@ static smartlist_t *global_origin_circuit_list = NULL; /** A list of all the circuits in CIRCUIT_STATE_CHAN_WAIT. */ static smartlist_t *circuits_pending_chans = NULL; +/** List of all the (origin) circuits whose state is + * CIRCUIT_STATE_GUARD_WAIT. */ +static smartlist_t *circuits_pending_other_guards = NULL; + /** A list of all the circuits that have been marked with * circuit_mark_for_close and which are waiting for circuit_about_to_free. */ static smartlist_t *circuits_pending_close = NULL; @@ -433,8 +437,10 @@ circuit_set_state(circuit_t *circ, uint8_t state) tor_assert(circ); if (state == circ->state) return; - if (!circuits_pending_chans) + if (PREDICT_UNLIKELY(!circuits_pending_chans)) circuits_pending_chans = smartlist_new(); + if (PREDICT_UNLIKELY(!circuits_pending_other_guards)) + circuits_pending_other_guards = smartlist_new(); if (circ->state == CIRCUIT_STATE_CHAN_WAIT) { /* remove from waiting-circuit list. */ smartlist_remove(circuits_pending_chans, circ); @@ -1022,6 +1028,9 @@ circuit_free_all(void) smartlist_free(circuits_pending_close); circuits_pending_close = NULL; + smartlist_free(circuits_pending_other_guards); + circuits_pending_other_guards = NULL; + { chan_circid_circuit_map_t **elt, **next, *c; for (elt = HT_START(chan_circid_map, &chan_circid_map); @@ -1721,6 +1730,37 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, return best; } +/** + * Check whether any of the origin circuits that are waiting to see if + * their guard is good enough to use can be upgraded to "ready". If so, + * return a new smartlist containing them. Otherwise return NULL. + */ +smartlist_t * +circuit_find_circuits_to_upgrade_from_guard_wait(void) +{ + /* Only if some circuit is actually waiting on an upgrade should we + * run the algorithm. */ + if (! circuits_pending_other_guards || + smartlist_len(circuits_pending_other_guards)==0) + return NULL; + /* Only if we have some origin circuiuts should we run the algorithm. + */ + if (!global_origin_circuit_list) + return NULL; + + /* Okay; we can pass our circuit list to entrynodes.c.*/ + smartlist_t *result = smartlist_new(); + int r = entry_guards_upgrade_waiting_circuits(get_guard_selection_info(), + global_origin_circuit_list, + result); + if (r && smartlist_len(result)) { + return result; + } else { + smartlist_free(result); + return NULL; + } +} + /** Return the number of hops in circuit's path. If circ has no entries, * or is NULL, returns 0. */ int diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index 989c02afd5..73039cc06e 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -77,6 +77,8 @@ void channel_note_destroy_pending(channel_t *chan, circid_t id); MOCK_DECL(void, channel_note_destroy_not_pending, (channel_t *chan, circid_t id)); +smartlist_t *circuit_find_circuits_to_upgrade_from_guard_wait(void); + #ifdef CIRCUITLIST_PRIVATE STATIC void circuit_free(circuit_t *circ); STATIC size_t n_cells_in_circ_queues(const circuit_t *c); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 2f972d1a28..d2a7f20aa2 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1633,6 +1633,8 @@ circuit_build_failed(origin_circuit_t *circ) "Our circuit died before the first hop with no connection"); } if (n_chan_id && !already_marked) { + entry_guard_failed(get_guard_selection_info(), &circ->guard_state); + /* XXXX prop271 -- old API */ entry_guard_register_connect_status(n_chan_id, 0, 1, time(NULL)); /* if there are any one-hop streams waiting on this circuit, fail * them now so they can retry elsewhere. */ diff --git a/src/or/connection.c b/src/or/connection.c index 2964c30f73..c2a7a87d41 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -634,6 +634,7 @@ connection_free_(connection_t *conn) cached_dir_decref(dir_conn->cached_dir); rend_data_free(dir_conn->rend_data); + circuit_guard_state_free(dir_conn->guard_state); } if (SOCKET_OK(conn->s)) { diff --git a/src/or/connection_or.c b/src/or/connection_or.c index ecc5a4511a..fefcc86932 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -735,6 +735,9 @@ connection_or_about_to_close(or_connection_t *or_conn) const or_options_t *options = get_options(); connection_or_note_state_when_broken(or_conn); rep_hist_note_connect_failed(or_conn->identity_digest, now); + entry_guard_chan_failed(get_guard_selection_info(), + TLS_CHAN_TO_BASE(or_conn->chan)); + /* XXXX prop271 -- old API */ entry_guard_register_connect_status(or_conn->identity_digest,0, !options->HTTPSProxy, now); if (conn->state >= OR_CONN_STATE_TLS_HANDSHAKING) { @@ -1673,6 +1676,9 @@ connection_or_client_learned_peer_id(or_connection_t *conn, "Tried connecting to router at %s:%d, but identity key was not " "as expected: wanted %s but got %s.%s", conn->base_.address, conn->base_.port, expected, seen, extra_log); + entry_guard_chan_failed(get_guard_selection_info(), + TLS_CHAN_TO_BASE(conn->chan)); + /* XXXX prop271 old API */ entry_guard_register_connect_status(conn->identity_digest, 0, 1, time(NULL)); control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED, diff --git a/src/or/directory.c b/src/or/directory.c index efa5a3126a..4164672f10 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -128,7 +128,8 @@ static void directory_initiate_command_rend( const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query); + const rend_data_t *rend_query, + circuit_guard_state_t *guard_state); static void connection_dir_close_consensus_fetches( dir_connection_t *except_this_one, const char *resource); @@ -422,7 +423,8 @@ directory_post_to_dirservers(uint8_t dir_purpose, uint8_t router_purpose, directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose, indirection, - NULL, payload, upload_len, 0); + NULL, payload, upload_len, 0, + NULL); } SMARTLIST_FOREACH_END(ds); if (!found) { char *s = authdir_type_to_string(type); @@ -458,7 +460,8 @@ should_use_directory_guards(const or_options_t *options) * information of type type, and return its routerstatus. */ static const routerstatus_t * directory_pick_generic_dirserver(dirinfo_type_t type, int pds_flags, - uint8_t dir_purpose) + uint8_t dir_purpose, + circuit_guard_state_t **guard_state_out) { const routerstatus_t *rs = NULL; const or_options_t *options = get_options(); @@ -467,7 +470,7 @@ directory_pick_generic_dirserver(dirinfo_type_t type, int pds_flags, log_warn(LD_BUG, "Called when we have UseBridges set."); if (should_use_directory_guards(options)) { - const node_t *node = choose_random_dirguard(type); + const node_t *node = guards_choose_dirguard(type, guard_state_out); if (node) rs = node->rs; } else { @@ -548,6 +551,7 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( if (!options->FetchServerDescriptors) return; + circuit_guard_state_t *guard_state = NULL; if (!get_via_tor) { if (options->UseBridges && !(type & BRIDGE_DIRINFO)) { /* We want to ask a running bridge for which we have a descriptor. @@ -556,6 +560,7 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( * sort of dir fetch we'll be doing, so it won't return a bridge * that can't answer our question. */ + // XXXX prop271 update this for bridge support. const node_t *node = choose_random_dirguard(type); if (node && node->ri) { /* every bridge has a routerinfo. */ @@ -605,9 +610,9 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( } } if (!rs && !(type & BRIDGE_DIRINFO)) { - /* */ rs = directory_pick_generic_dirserver(type, pds_flags, - dir_purpose); + dir_purpose, + &guard_state); if (!rs) get_via_tor = 1; /* last resort: try routing it via Tor */ } @@ -630,7 +635,8 @@ MOCK_IMPL(void, directory_get_from_dirserver, ( router_purpose, indirection, resource, NULL, 0, - if_modified_since); + if_modified_since, + guard_state); } else { log_notice(LD_DIR, "While fetching directory info, " @@ -664,7 +670,7 @@ directory_get_from_all_authorities(uint8_t dir_purpose, rs = &ds->fake_status; directory_initiate_command_routerstatus(rs, dir_purpose, router_purpose, DIRIND_ONEHOP, resource, NULL, - 0, 0); + 0, 0, NULL); } SMARTLIST_FOREACH_END(ds); } @@ -775,7 +781,8 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query) + const rend_data_t *rend_query, + circuit_guard_state_t *guard_state) { const or_options_t *options = get_options(); const node_t *node; @@ -830,7 +837,8 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, dir_purpose, router_purpose, indirection, resource, payload, payload_len, if_modified_since, - rend_query); + rend_query, + guard_state); } /** Launch a new connection to the directory server status to @@ -855,13 +863,15 @@ MOCK_IMPL(void, directory_initiate_command_routerstatus, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since)) + time_t if_modified_since, + circuit_guard_state_t *guard_state)) { directory_initiate_command_routerstatus_rend(status, dir_purpose, router_purpose, indirection, resource, payload, payload_len, - if_modified_since, NULL); + if_modified_since, NULL, + guard_state); } /** Return true iff conn is the client side of a directory connection @@ -889,6 +899,11 @@ directory_conn_is_self_reachability_test(dir_connection_t *conn) static void connection_dir_request_failed(dir_connection_t *conn) { + if (conn->guard_state) { + /* We haven't seen a success on this guard state, so consider it to have + * failed. */ + entry_guard_failed(get_guard_selection_info(), &conn->guard_state); + } if (directory_conn_is_self_reachability_test(conn)) { return; /* this was a test fetch. don't retry. */ } @@ -1136,7 +1151,7 @@ directory_initiate_command(const tor_addr_t *or_addr, uint16_t or_port, digest, dir_purpose, router_purpose, indirection, resource, payload, payload_len, - if_modified_since, NULL); + if_modified_since, NULL, NULL); } /** Same as directory_initiate_command(), but accepts rendezvous data to @@ -1151,7 +1166,8 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, const char *resource, const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query) + const rend_data_t *rend_query, + circuit_guard_state_t *guard_state) { tor_assert(or_addr_port); tor_assert(dir_addr_port); @@ -1246,12 +1262,18 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, if (!anonymized_connection && !use_begindir) { /* then we want to connect to dirport directly */ + // XXXX prop271 I think that we never use guards in this case. if (options->HTTPProxy) { tor_addr_copy(&addr, &options->HTTPProxyAddr); port = options->HTTPProxyPort; } + // In this case we should not have picked a directory guard. + if (BUG(guard_state)) { + entry_guard_cancel(get_guard_selection_info(), &guard_state); + } + switch (connection_connect(TO_CONN(conn), conn->base_.address, &addr, port, &socket_error)) { case -1: @@ -1288,6 +1310,14 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, else if (anonymized_connection && !use_begindir) rep_hist_note_used_port(time(NULL), conn->base_.port); + // In this case we should not have a directory guard; we'll + // get a regular guard later when we build the circuit. + if (BUG(anonymized_connection && guard_state)) { + entry_guard_cancel(get_guard_selection_info(), &guard_state); + } + + conn->guard_state = guard_state; + /* make an AP connection * populate it and add it at the right state * hook up both sides @@ -2540,6 +2570,22 @@ connection_dir_process_inbuf(dir_connection_t *conn) tor_assert(conn); tor_assert(conn->base_.type == CONN_TYPE_DIR); + if (conn->guard_state) { + /* we count the connection as successful once we can read from it. We do + * not, however, delay use of the circuit here, since it's just for a + * one-hop directory request. */ + /* XXXXprop271 note that this will not do the right thing for other + * waiting circuits that would be triggered by this circuit becoming + * complete/usable. But that's ok, I think. + */ + /* XXXXprop271 should we count this as only a partial success somehow? + */ + entry_guard_succeeded(get_guard_selection_info(), + &conn->guard_state); + circuit_guard_state_free(conn->guard_state); + conn->guard_state = NULL; + } + /* Directory clients write, then read data until they receive EOF; * directory servers read data until they get an HTTP command, then * write their response (when it's finished flushing, they mark for diff --git a/src/or/directory.h b/src/or/directory.h index 589df7b70d..ee0a198c52 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -49,7 +49,8 @@ MOCK_DECL(void, directory_initiate_command_routerstatus, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since)); + time_t if_modified_since, + struct circuit_guard_state_t *guard_state)); void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, uint8_t dir_purpose, @@ -59,7 +60,8 @@ void directory_initiate_command_routerstatus_rend(const routerstatus_t *status, const char *payload, size_t payload_len, time_t if_modified_since, - const rend_data_t *rend_query); + const rend_data_t *rend_query, + struct circuit_guard_state_t *guard_state); int parse_http_response(const char *headers, int *code, time_t *date, compress_method_t *compression, char **response); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 24a3448969..eca88a947c 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -89,7 +89,7 @@ * [x] Whenever a guard becomes reachable or maybe-reachable, if its filtered * flag is set, set its usable_filtered flag. * - * [ ] Whenever we get a new consensus, call update_from_consensus(). (LATER.) + * [x] Whenever we get a new consensus, call update_from_consensus(). (LATER.) * * [ ] Whenever the configuration changes in a relevant way, update the * filtered/usable flags. (LATER.) @@ -1203,8 +1203,6 @@ entry_guards_note_guard_success(guard_selection_t *gs, if (last_time_on_internet + INTERNET_LIKELY_DOWN_INTERVAL < approx_time()) { mark_primary_guards_maybe_reachable(gs); - } else { - // update_waiting_circuits(gs); // XXXX prop271 write this function. } } @@ -1475,7 +1473,7 @@ circ_state_has_higher_priority(origin_circuit_t *a, */ int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, - smartlist_t *all_circuits, + const smartlist_t *all_circuits, smartlist_t *newly_complete_out) { tor_assert(gs); @@ -2274,7 +2272,7 @@ add_an_entry_guard(guard_selection_t *gs, return NULL; } } else if (!for_directory) { - node = choose_good_entry_server(CIRCUIT_PURPOSE_C_GENERAL, NULL); + node = choose_good_entry_server(CIRCUIT_PURPOSE_C_GENERAL, NULL, NULL); if (!node) return NULL; } else { @@ -3779,6 +3777,58 @@ entries_retry_all(const or_options_t *options) entries_retry_helper(options, 1); } +/** Helper: Update the status of all entry guards, in whatever algorithm + is used. */ +void +guards_update_all(void) +{ + if (get_options()->UseDeprecatedGuardAlgorithm) { + entry_guards_compute_status(get_options(), approx_time()); + } else { + entry_guards_update_all(get_guard_selection_info()); + } +} + +/** Helper: pick a guard for a circuit, with whatever algorithm is + used. */ +const node_t * +guards_choose_guard(cpath_build_state_t *state, + circuit_guard_state_t **guard_state_out) +{ + if (get_options()->UseDeprecatedGuardAlgorithm) { + return choose_random_entry(state); + } else { + // XXXX prop271 we need to look at the chosen exit node if any, and + // not duplicate it. + const node_t *r = NULL; + if (entry_guard_pick_for_circuit(get_guard_selection_info(), + &r, + guard_state_out) < 0) { + tor_assert(r == NULL); + } + return r; + } +} + +/** Helper: pick a directory guard, with whatever algorithm is used. */ +const node_t * +guards_choose_dirguard(dirinfo_type_t info, + circuit_guard_state_t **guard_state_out) +{ + if (get_options()->UseDeprecatedGuardAlgorithm) { + return choose_random_dirguard(info); + } else { + // XXXX prop271 look at info? + const node_t *r = NULL; + if (entry_guard_pick_for_circuit(get_guard_selection_info(), + &r, + guard_state_out) < 0) { + tor_assert(r == NULL); + } + return r; + } +} + /** Free one guard selection context */ STATIC void guard_selection_free(guard_selection_t *gs) diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 60191ab8b9..7dcedd6066 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -328,7 +328,7 @@ void entry_guard_chan_failed(guard_selection_t *gs, channel_t *chan); void entry_guards_update_all(guard_selection_t *gs); int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, - smartlist_t *all_circuits, + const smartlist_t *all_circuits, smartlist_t *newly_complete_out); void entry_guards_note_internet_connectivity(guard_selection_t *gs); diff --git a/src/or/main.c b/src/or/main.c index a508003f97..65d1c1fd79 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -979,7 +979,7 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs) /* if we have enough dir info, then update our guard status with * whatever we just learned. */ - entry_guards_compute_status(options, now); + guards_update_all(); /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo) @@ -1376,6 +1376,9 @@ run_scheduled_events(time_t now) /* 0c. If we've deferred log messages for the controller, handle them now */ flush_pending_log_callbacks(); + /* Maybe enough time elapsed for us to reconsider a circuit. */ + circuit_upgrade_circuits_from_guard_wait(); + if (options->UseBridges && !options->DisableNetwork) { fetch_bridge_descriptors(options, now); } diff --git a/src/or/or.h b/src/or/or.h index c8f39f90d9..8b9ede3607 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1786,6 +1786,10 @@ typedef struct dir_connection_t { /** What rendezvous service are we querying for? */ rend_data_t *rend_data; + /** If this is a one-hop connection, tracks the state of the directory guard + * for this connection (if any). */ + struct circuit_guard_state_t *guard_state; + char identity_digest[DIGEST_LEN]; /**< Hash of the public RSA key for * the directory server's signing key. */ diff --git a/src/or/rendclient.c b/src/or/rendclient.c index b0dcf52507..06744ad795 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -762,7 +762,7 @@ directory_get_from_hs_dir(const char *desc_id, how_to_fetch, desc_id_base32, NULL, 0, 0, - rend_query); + rend_query, NULL); log_info(LD_REND, "Sending fetch request for v2 descriptor for " "service '%s' with descriptor ID '%s', auth type %d, " "and descriptor cookie '%s' to hidden service " diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 8ffd0bc319..cf57c7d061 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3452,7 +3452,7 @@ directory_post_to_hs_dir(rend_service_descriptor_t *renddesc, DIRIND_ANONYMOUS, NULL, desc->desc_str, strlen(desc->desc_str), - 0, rend_data); + 0, rend_data, NULL); rend_data_free(rend_data); base32_encode(desc_id_base32, sizeof(desc_id_base32), desc->desc_id, DIGEST_LEN); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index f51b50e8e5..d2f360a63f 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -971,7 +971,7 @@ authority_certs_fetch_resource_impl(const char *resource, directory_initiate_command_routerstatus(rs, DIR_PURPOSE_FETCH_CERTIFICATE, 0, indirection, resource, NULL, - 0, 0); + 0, 0, NULL); return; } @@ -4946,7 +4946,7 @@ MOCK_IMPL(STATIC void, initiate_descriptor_downloads, directory_initiate_command_routerstatus(source, purpose, ROUTER_PURPOSE_GENERAL, DIRIND_ONEHOP, - resource, NULL, 0, 0); + resource, NULL, 0, 0, NULL); } else { directory_get_from_dirserver(purpose, ROUTER_PURPOSE_GENERAL, resource, pds_flags, DL_WANT_ANY_DIRSERVER); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 6f83ceff00..4501d6b547 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -23,6 +23,7 @@ #include "directory.h" #include "dirserv.h" #include "dirvote.h" +#include "entrynodes.h" #include "hibernate.h" #include "memarea.h" #include "networkstatus.h" @@ -4397,7 +4398,8 @@ directory_initiate_command_routerstatus, (const routerstatus_t *status, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since)); + time_t if_modified_since, + circuit_guard_state_t *guardstate)); static void test_dir_should_not_init_request_to_ourselves(void *data) @@ -4504,7 +4506,8 @@ NS(directory_initiate_command_routerstatus)(const routerstatus_t *status, const char *resource, const char *payload, size_t payload_len, - time_t if_modified_since) + time_t if_modified_since, + circuit_guard_state_t *guardstate) { (void)status; (void)dir_purpose; @@ -4514,6 +4517,7 @@ NS(directory_initiate_command_routerstatus)(const routerstatus_t *status, (void)payload; (void)payload_len; (void)if_modified_since; + (void)guardstate; CALLED(directory_initiate_command_routerstatus)++; } -- cgit v1.2.3-54-g00ecf From f71be7434074a1b7f8508b96cbf55cee44afb993 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 23 Nov 2016 09:15:51 -0500 Subject: When freeing a guard state, cancel it if its state is unknown We don't want a guard to stay "pending" forever if the circuit_guard_state_t for it is freed before it succeeds or fails. --- src/or/circuitlist.c | 5 +++++ src/or/connection.c | 4 ++++ 2 files changed, 9 insertions(+) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 2a03f8a0d9..9d7a5d7f0e 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -885,6 +885,11 @@ circuit_free(circuit_t *circ) cpath_ref_decref(ocirc->build_state->service_pending_final_cpath_ref); } tor_free(ocirc->build_state); + + /* Cancel before freeing, if we haven't already succeeded or failed. */ + if (ocirc->guard_state) { + entry_guard_cancel(get_guard_selection_info(), ô->guard_state); + } circuit_guard_state_free(ocirc->guard_state); circuit_clear_cpath(ocirc); diff --git a/src/or/connection.c b/src/or/connection.c index c2a7a87d41..25c75ff101 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -634,6 +634,10 @@ connection_free_(connection_t *conn) cached_dir_decref(dir_conn->cached_dir); rend_data_free(dir_conn->rend_data); + if (dir_conn->guard_state) { + /* Cancel before freeing, if it's still there. */ + entry_guard_cancel(get_guard_selection_info(), &dir_conn->guard_state); + } circuit_guard_state_free(dir_conn->guard_state); } -- cgit v1.2.3-54-g00ecf From 2ea5aa71823f385e36f20e643a20996dcb164464 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 25 Nov 2016 12:53:00 -0500 Subject: Expire circuits that have been WAITING_FOR_BETTER_GUARD too long (This is required by 3.9 in prop271, but is better done as a separate function IMO) --- src/or/circuitlist.c | 12 +++++++++++- src/or/circuitlist.h | 1 + src/or/circuituse.c | 19 +++++++++++++++++++ src/or/circuituse.h | 1 + src/or/entrynodes.c | 18 +++++++++++++++--- src/or/entrynodes.h | 1 + src/or/main.c | 1 + 7 files changed, 49 insertions(+), 4 deletions(-) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 9d7a5d7f0e..0afe2f8059 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -553,7 +553,7 @@ circuit_close_all_marked(void) smartlist_clear(circuits_pending_close); } -/** Return the head of the global linked list of circuits. */ +/** Return a pointer to the global list of circuits. */ MOCK_IMPL(smartlist_t *, circuit_get_global_list,(void)) { @@ -562,6 +562,16 @@ circuit_get_global_list,(void)) return global_circuitlist; } +/** */ +/** Return a pointer to the global list of origin circuits. */ +smartlist_t * +circuit_get_global_origin_circuit_list(void) +{ + if (NULL == global_origin_circuit_list) + global_origin_circuit_list = smartlist_new(); + return global_circuitlist; +} + /** Function to make circ-\>state human-readable */ const char * circuit_state_to_string(int state) diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index 73039cc06e..e2102a118b 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -15,6 +15,7 @@ #include "testsupport.h" MOCK_DECL(smartlist_t *, circuit_get_global_list, (void)); +smartlist_t *circuit_get_global_origin_circuit_list(void); const char *circuit_state_to_string(int state); const char *circuit_purpose_to_controller_string(uint8_t purpose); const char *circuit_purpose_to_controller_hs_state_string(uint8_t purpose); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index b9f94fb3a2..b925729e01 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -800,6 +800,25 @@ circuit_expire_building(void) } SMARTLIST_FOREACH_END(victim); } +/** + * Mark for close all circuits that start here, that were built through a + * guard we weren't sure if we wanted to use, and that have been waiting + * around for way too long. + */ +void +circuit_expire_waiting_for_better_guard(void) +{ + SMARTLIST_FOREACH_BEGIN(circuit_get_global_origin_circuit_list(), + origin_circuit_t *, circ) { + if (TO_CIRCUIT(circ)->marked_for_close) + continue; + if (circ->guard_state == NULL) + continue; + if (entry_guard_state_should_expire(circ->guard_state)) + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_NONE); + } SMARTLIST_FOREACH_END(circ); +} + /** For debugging #8387: track when we last called * circuit_expire_old_circuits_clientside. */ static time_t last_expired_clientside_circuits = 0; diff --git a/src/or/circuituse.h b/src/or/circuituse.h index 5973978c45..110bdda5b2 100644 --- a/src/or/circuituse.h +++ b/src/or/circuituse.h @@ -13,6 +13,7 @@ #define TOR_CIRCUITUSE_H void circuit_expire_building(void); +void circuit_expire_waiting_for_better_guard(void); void circuit_remove_handled_ports(smartlist_t *needed_ports); int circuit_stream_is_being_handled(entry_connection_t *conn, uint16_t port, int min); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 951ce15f85..1c9349ee03 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1605,9 +1605,6 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, "circuit had higher priority, so not upgrading.", n_complete, n_waiting); - /* XXXX prop271 implement: "(Time them out after a - {NONPRIMARY_GUARD_IDLE_TIMEOUT} seconds.)" - */ return 0; } } @@ -1671,6 +1668,21 @@ entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, return 1; } +/** + * Return true iff the circuit whose state is guard_state should + * expire. + */ +int +entry_guard_state_should_expire(circuit_guard_state_t *guard_state) +{ + if (guard_state == NULL) + return 0; + const time_t expire_if_waiting_since = + approx_time() - NONPRIMARY_GUARD_IDLE_TIMEOUT; + return (guard_state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD + && guard_state->state_set_at < expire_if_waiting_since); +} + /** * Update all derived pieces of the guard selection state in gs. * Return true iff we should stop using all previously generated circuits. diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index ec24011377..648e599310 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -337,6 +337,7 @@ int entry_guards_update_all(guard_selection_t *gs); int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, const smartlist_t *all_circuits, smartlist_t *newly_complete_out); +int entry_guard_state_should_expire(circuit_guard_state_t *guard_state); void entry_guards_note_internet_connectivity(guard_selection_t *gs); /* Used by bridges.c only. */ diff --git a/src/or/main.c b/src/or/main.c index 16106612a9..96ff442c68 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1402,6 +1402,7 @@ run_scheduled_events(time_t now) /* (If our circuit build timeout can ever become lower than a second (which * it can't, currently), we should do this more often.) */ circuit_expire_building(); + circuit_expire_waiting_for_better_guard(); /* 3b. Also look at pending streams and prune the ones that 'began' * a long time ago but haven't gotten a 'connected' yet. -- cgit v1.2.3-54-g00ecf From 89f5f149df984bab00de9868a9305b611c4aa17e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 28 Nov 2016 11:04:28 -0500 Subject: Remove guard_selection argument from status-reporting functions This prevents us from mixing up multiple guard_selections --- src/or/circuitbuild.c | 3 +-- src/or/circuitlist.c | 2 +- src/or/circuituse.c | 2 +- src/or/connection.c | 2 +- src/or/connection_or.c | 6 ++---- src/or/directory.c | 9 ++++----- src/or/entrynodes.c | 31 ++++++++++--------------------- src/or/entrynodes.h | 12 ++++-------- src/test/test_entrynodes.c | 26 +++++++++++++------------- 9 files changed, 37 insertions(+), 56 deletions(-) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 16b53f6e21..5d0a04fe71 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -976,8 +976,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) } r = 1; } else { - r = entry_guard_succeeded(get_guard_selection_info(), - &circ->guard_state); + r = entry_guard_succeeded(&circ->guard_state); } const int is_usable_for_streams = (r == 1); if (r == 1) { diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 0afe2f8059..b25f817d91 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -898,7 +898,7 @@ circuit_free(circuit_t *circ) /* Cancel before freeing, if we haven't already succeeded or failed. */ if (ocirc->guard_state) { - entry_guard_cancel(get_guard_selection_info(), ô->guard_state); + entry_guard_cancel(ô->guard_state); } circuit_guard_state_free(ocirc->guard_state); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index b925729e01..698b158457 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1653,7 +1653,7 @@ circuit_build_failed(origin_circuit_t *circ) } if (n_chan_id && !already_marked) { if (circ->guard_state) - entry_guard_failed(get_guard_selection_info(), &circ->guard_state); + entry_guard_failed(&circ->guard_state); /* XXXX prop271 -- old API */ entry_guard_register_connect_status(n_chan_id, 0, 1, time(NULL)); /* if there are any one-hop streams waiting on this circuit, fail diff --git a/src/or/connection.c b/src/or/connection.c index 25c75ff101..87f0f91fd2 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -636,7 +636,7 @@ connection_free_(connection_t *conn) rend_data_free(dir_conn->rend_data); if (dir_conn->guard_state) { /* Cancel before freeing, if it's still there. */ - entry_guard_cancel(get_guard_selection_info(), &dir_conn->guard_state); + entry_guard_cancel(&dir_conn->guard_state); } circuit_guard_state_free(dir_conn->guard_state); } diff --git a/src/or/connection_or.c b/src/or/connection_or.c index fefcc86932..14d597960e 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -735,8 +735,7 @@ connection_or_about_to_close(or_connection_t *or_conn) const or_options_t *options = get_options(); connection_or_note_state_when_broken(or_conn); rep_hist_note_connect_failed(or_conn->identity_digest, now); - entry_guard_chan_failed(get_guard_selection_info(), - TLS_CHAN_TO_BASE(or_conn->chan)); + entry_guard_chan_failed(TLS_CHAN_TO_BASE(or_conn->chan)); /* XXXX prop271 -- old API */ entry_guard_register_connect_status(or_conn->identity_digest,0, !options->HTTPSProxy, now); @@ -1676,8 +1675,7 @@ connection_or_client_learned_peer_id(or_connection_t *conn, "Tried connecting to router at %s:%d, but identity key was not " "as expected: wanted %s but got %s.%s", conn->base_.address, conn->base_.port, expected, seen, extra_log); - entry_guard_chan_failed(get_guard_selection_info(), - TLS_CHAN_TO_BASE(conn->chan)); + entry_guard_chan_failed(TLS_CHAN_TO_BASE(conn->chan)); /* XXXX prop271 old API */ entry_guard_register_connect_status(conn->identity_digest, 0, 1, time(NULL)); diff --git a/src/or/directory.c b/src/or/directory.c index 4164672f10..6fc8809448 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -902,7 +902,7 @@ connection_dir_request_failed(dir_connection_t *conn) if (conn->guard_state) { /* We haven't seen a success on this guard state, so consider it to have * failed. */ - entry_guard_failed(get_guard_selection_info(), &conn->guard_state); + entry_guard_failed(&conn->guard_state); } if (directory_conn_is_self_reachability_test(conn)) { return; /* this was a test fetch. don't retry. */ @@ -1271,7 +1271,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, // In this case we should not have picked a directory guard. if (BUG(guard_state)) { - entry_guard_cancel(get_guard_selection_info(), &guard_state); + entry_guard_cancel(&guard_state); } switch (connection_connect(TO_CONN(conn), conn->base_.address, &addr, @@ -1313,7 +1313,7 @@ directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, // In this case we should not have a directory guard; we'll // get a regular guard later when we build the circuit. if (BUG(anonymized_connection && guard_state)) { - entry_guard_cancel(get_guard_selection_info(), &guard_state); + entry_guard_cancel(&guard_state); } conn->guard_state = guard_state; @@ -2580,8 +2580,7 @@ connection_dir_process_inbuf(dir_connection_t *conn) */ /* XXXXprop271 should we count this as only a partial success somehow? */ - entry_guard_succeeded(get_guard_selection_info(), - &conn->guard_state); + entry_guard_succeeded(&conn->guard_state); circuit_guard_state_free(conn->guard_state); conn->guard_state = NULL; } diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index e0626cfbe0..0795d19026 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1731,8 +1731,7 @@ entry_guard_pick_for_circuit(guard_selection_t *gs, * XXXXX prop271 tristates are ugly; reconsider that interface. */ int -entry_guard_succeeded(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p) +entry_guard_succeeded(circuit_guard_state_t **guard_state_p) { if (get_options()->UseDeprecatedGuardAlgorithm) return 1; @@ -1741,13 +1740,12 @@ entry_guard_succeeded(guard_selection_t *gs, return -1; entry_guard_t *guard = entry_guard_handle_get((*guard_state_p)->guard); - if (! guard) + if (! guard || BUG(guard->in_selection == NULL)) return -1; - tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument - unsigned newstate = - entry_guards_note_guard_success(gs, guard, (*guard_state_p)->state); + entry_guards_note_guard_success(guard->in_selection, guard, + (*guard_state_p)->state); (*guard_state_p)->state = newstate; (*guard_state_p)->state_set_at = approx_time(); @@ -1763,10 +1761,8 @@ entry_guard_succeeded(guard_selection_t *gs, * success or failure. It is safe to call this function if success or * failure _has_ already been declared. */ void -entry_guard_cancel(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p) +entry_guard_cancel(circuit_guard_state_t **guard_state_p) { - (void) gs; if (get_options()->UseDeprecatedGuardAlgorithm) return; if (BUG(*guard_state_p == NULL)) @@ -1775,8 +1771,6 @@ entry_guard_cancel(guard_selection_t *gs, if (! guard) return; - tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument - /* XXXX prop271 -- last_tried_to_connect_at will be erroneous here, but this * function will only get called in "bug" cases anyway. */ guard->is_pending = 0; @@ -1790,8 +1784,7 @@ entry_guard_cancel(guard_selection_t *gs, * not working, and advances the state of the guard module. */ void -entry_guard_failed(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p) +entry_guard_failed(circuit_guard_state_t **guard_state_p) { if (get_options()->UseDeprecatedGuardAlgorithm) return; @@ -1800,12 +1793,10 @@ entry_guard_failed(guard_selection_t *gs, return; entry_guard_t *guard = entry_guard_handle_get((*guard_state_p)->guard); - if (! guard) + if (! guard || BUG(guard->in_selection == NULL)) return; - tor_assert(gs == guard->in_selection); // XXXX prop271 remove argument - - entry_guards_note_guard_failure(gs, guard); + entry_guards_note_guard_failure(guard->in_selection, guard); (*guard_state_p)->state = GUARD_CIRC_STATE_DEAD; (*guard_state_p)->state_set_at = approx_time(); @@ -1816,10 +1807,8 @@ entry_guard_failed(guard_selection_t *gs, * pending on chan. */ void -entry_guard_chan_failed(guard_selection_t *gs, - channel_t *chan) +entry_guard_chan_failed(channel_t *chan) { - tor_assert(gs); if (!chan) return; if (get_options()->UseDeprecatedGuardAlgorithm) @@ -1832,7 +1821,7 @@ entry_guard_chan_failed(guard_selection_t *gs, continue; origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ); - entry_guard_failed(gs, &origin_circ->guard_state); + entry_guard_failed(&origin_circ->guard_state); } SMARTLIST_FOREACH_END(circ); smartlist_free(pending); } diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 97cc4d254b..0abbea8cb6 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -351,14 +351,10 @@ void circuit_guard_state_free(circuit_guard_state_t *state); int entry_guard_pick_for_circuit(guard_selection_t *gs, const node_t **chosen_node_out, circuit_guard_state_t **guard_state_out); -int entry_guard_succeeded(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p); -void entry_guard_failed(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p); -void entry_guard_cancel(guard_selection_t *gs, - circuit_guard_state_t **guard_state_p); -void entry_guard_chan_failed(guard_selection_t *gs, - channel_t *chan); +int entry_guard_succeeded(circuit_guard_state_t **guard_state_p); +void entry_guard_failed(circuit_guard_state_t **guard_state_p); +void entry_guard_cancel(circuit_guard_state_t **guard_state_p); +void entry_guard_chan_failed(channel_t *chan); int entry_guards_update_all(guard_selection_t *gs); int entry_guards_upgrade_waiting_circuits(guard_selection_t *gs, const smartlist_t *all_circuits, diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 4595e5d675..bc0862a93e 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -2333,7 +2333,7 @@ test_entry_guard_select_for_circuit_highlevel_primary(void *arg) /* Call that circuit successful. */ update_approx_time(start+15); - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); tt_int_op(r, OP_EQ, 1); /* We can use it now. */ tt_assert(guard); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_COMPLETE); @@ -2365,7 +2365,7 @@ test_entry_guard_select_for_circuit_highlevel_primary(void *arg) /* It's failed! What will happen to our poor guard? */ update_approx_time(start+45); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); tt_assert(guard); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_DEAD); tt_i64_op(guard->state_set_at, OP_EQ, start+45); @@ -2401,7 +2401,7 @@ test_entry_guard_select_for_circuit_highlevel_primary(void *arg) /* Call this one up; watch it get confirmed. */ update_approx_time(start+90); - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); tt_int_op(r, OP_EQ, 1); /* We can use it now. */ tt_assert(guard); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_COMPLETE); @@ -2441,7 +2441,7 @@ test_entry_guard_select_for_circuit_highlevel_confirm_other(void *arg) tt_assert(guard); tt_assert(r == 0); tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_USABLE_ON_COMPLETION); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); guard = NULL; node = NULL; @@ -2461,7 +2461,7 @@ test_entry_guard_select_for_circuit_highlevel_confirm_other(void *arg) tt_int_op(g->is_pending, OP_EQ, 1); (void)start; - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); /* We're on the internet (by fiat), so this guard will get called "confirmed" * and should immediately become primary. * XXXX prop271 -- I don't like that behavior, but it's what is specified @@ -2508,7 +2508,7 @@ test_entry_guard_select_for_circuit_highlevel_primary_retry(void *arg) g = entry_guard_handle_get(guard->guard); make_guard_confirmed(gs, g); tt_int_op(g->is_primary, OP_EQ, 1); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); tt_int_op(g->is_reachable, OP_EQ, GUARD_REACHABLE_NO); guard = NULL; @@ -2530,7 +2530,7 @@ test_entry_guard_select_for_circuit_highlevel_primary_retry(void *arg) update_approx_time(start + 3600); /* Say that guard has succeeded! */ - r = entry_guard_succeeded(gs, &guard); + r = entry_guard_succeeded(&guard); tt_int_op(r, OP_EQ, 0); // can't use it yet. tt_int_op(guard->state, OP_EQ, GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD); g = entry_guard_handle_get(guard->guard); @@ -2546,7 +2546,7 @@ test_entry_guard_select_for_circuit_highlevel_primary_retry(void *arg) r = entry_guard_pick_for_circuit(gs, &node, &guard2); tt_assert(r == 0); tt_int_op(guard2->state, OP_EQ, GUARD_CIRC_STATE_USABLE_ON_COMPLETION); - r = entry_guard_succeeded(gs, &guard2); + r = entry_guard_succeeded(&guard2); tt_assert(r == 1); tt_int_op(guard2->state, OP_EQ, GUARD_CIRC_STATE_COMPLETE); @@ -2578,7 +2578,7 @@ test_entry_guard_select_and_cancel(void *arg) tt_int_op(g->is_primary, OP_EQ, 1); tt_int_op(g->is_pending, OP_EQ, 0); make_guard_confirmed(gs, g); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); guard = NULL; node = NULL; @@ -2597,7 +2597,7 @@ test_entry_guard_select_and_cancel(void *arg) tt_int_op(g->is_pending, OP_EQ, 1); /* Whoops! We should never have asked for this guard. Cancel the request! */ - entry_guard_cancel(gs, &guard); + entry_guard_cancel(&guard); tt_assert(guard == NULL); tt_int_op(g->is_primary, OP_EQ, 0); tt_int_op(g->is_pending, OP_EQ, 0); @@ -2649,7 +2649,7 @@ upgrade_circuits_setup(const struct testcase_t *testcase) entry_guard_pick_for_circuit(gs, &node, &guard); g = entry_guard_handle_get(guard->guard); make_guard_confirmed(gs, g); - entry_guard_failed(gs, &guard); + entry_guard_failed(&guard); circuit_guard_state_free(guard); } @@ -2682,14 +2682,14 @@ upgrade_circuits_setup(const struct testcase_t *testcase) int r; update_approx_time(data->start + 32); if (make_circ1_succeed) { - r = entry_guard_succeeded(gs, &data->guard1_state); + r = entry_guard_succeeded(&data->guard1_state); tor_assert(r == 0); tor_assert(data->guard1_state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD); } update_approx_time(data->start + 33); if (make_circ2_succeed) { - r = entry_guard_succeeded(gs, &data->guard2_state); + r = entry_guard_succeeded(&data->guard2_state); tor_assert(r == 0); tor_assert(data->guard2_state->state == GUARD_CIRC_STATE_WAITING_FOR_BETTER_GUARD); -- cgit v1.2.3-54-g00ecf From 7ab2678074e5d49628d948fadb80c5904950236c Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Tue, 6 Dec 2016 14:34:48 -0500 Subject: Trivial documentation improvements. --- src/or/circuitlist.c | 13 ++++++------- src/or/entrynodes.c | 17 +++++++++++++---- src/or/entrynodes.h | 9 ++++++--- 3 files changed, 25 insertions(+), 14 deletions(-) (limited to 'src/or/circuitlist.c') diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index b25f817d91..ab38b54985 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -562,7 +562,6 @@ circuit_get_global_list,(void)) return global_circuitlist; } -/** */ /** Return a pointer to the global list of origin circuits. */ smartlist_t * circuit_get_global_origin_circuit_list(void) @@ -1758,17 +1757,17 @@ circuit_find_circuits_to_upgrade_from_guard_wait(void) if (! circuits_pending_other_guards || smartlist_len(circuits_pending_other_guards)==0) return NULL; - /* Only if we have some origin circuiuts should we run the algorithm. - */ + /* Only if we have some origin circuits should we run the algorithm. */ if (!global_origin_circuit_list) return NULL; /* Okay; we can pass our circuit list to entrynodes.c.*/ smartlist_t *result = smartlist_new(); - int r = entry_guards_upgrade_waiting_circuits(get_guard_selection_info(), - global_origin_circuit_list, - result); - if (r && smartlist_len(result)) { + int circuits_upgraded = entry_guards_upgrade_waiting_circuits( + get_guard_selection_info(), + global_origin_circuit_list, + result); + if (circuits_upgraded && smartlist_len(result)) { return result; } else { smartlist_free(result); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 76070a3b7d..a28603d98d 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -142,7 +142,9 @@ #include "transports.h" #include "statefile.h" +/** A list of existing guard selection contexts. */ static smartlist_t *guard_contexts = NULL; +/** The currently enabled guard selection context. */ static guard_selection_t *curr_guard_context = NULL; /** A value of 1 means that at least one context has changed, @@ -593,7 +595,8 @@ choose_guard_selection(const or_options_t *options, "rest of the world.", (int)(exclude_frac * 100)); } - /* Easy case: no previous selection */ + /* Easy case: no previous selection. Just check if we are in restricted or + normal guard selection. */ if (old_selection == NULL) { if (n_passing_filter >= meaningful_threshold_mid) { *type_out = GS_TYPE_NORMAL; @@ -768,8 +771,9 @@ entry_guard_add_to_sample(guard_selection_t *gs, /** * Backend: adds a new sampled guard to gs, with given identity, - * nickname, and ORPort. rsa_id_digest and bridge_addrport are - * optional, but we need one of them. nickname is optional. + * nickname, and ORPort. rsa_id_digest and bridge_addrport are optional, but + * we need one of them. nickname is optional. The caller is responsible for + * maintaining the size limit of the SAMPLED_GUARDS set. */ static entry_guard_t * entry_guard_add_to_sample_impl(guard_selection_t *gs, @@ -2171,7 +2175,8 @@ entry_guards_all_primary_guards_are_down(guard_selection_t *gs) } /** Wrapper for entry_guard_has_higher_priority that compares the - * guard-priorities of a pair of circuits. + * guard-priorities of a pair of circuits. Return 1 if a has higher + * priority than b. * * If a restriction is provided in rst, then do not consider * a to have higher priority if it violates the restriction. @@ -4180,6 +4185,8 @@ choose_random_entry_impl(guard_selection_t *gs, } #endif +/** Check the pathbias use success count of node and disable it if it + * goes over our thresholds. */ static void pathbias_check_use_success_count(entry_guard_t *node) { @@ -4201,6 +4208,8 @@ pathbias_check_use_success_count(entry_guard_t *node) } } +/** Check the pathbias close count of node and disable it if it goes + * over our thresholds. */ static void pathbias_check_close_success_count(entry_guard_t *node) { diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 3250be1222..116e5abb8c 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -402,11 +402,14 @@ int entry_guard_pick_for_circuit(guard_selection_t *gs, entry_guard_restriction_t *rst, const node_t **chosen_node_out, circuit_guard_state_t **guard_state_out); + +/* We just connected to an entry guard. What should we do with the circuit? */ typedef enum { - GUARD_USABLE_NEVER = -1, - GUARD_MAYBE_USABLE_LATER = 0, - GUARD_USABLE_NOW = 1, + GUARD_USABLE_NEVER = -1, /* Never use the circuit */ + GUARD_MAYBE_USABLE_LATER = 0, /* Keep it. We might use it in the future */ + GUARD_USABLE_NOW = 1, /* Use it right now */ } guard_usable_t; + guard_usable_t entry_guard_succeeded(circuit_guard_state_t **guard_state_p); void entry_guard_failed(circuit_guard_state_t **guard_state_p); void entry_guard_cancel(circuit_guard_state_t **guard_state_p); -- cgit v1.2.3-54-g00ecf