diff options
| -rw-r--r-- | changes/bug11970 | 7 | ||||
| -rw-r--r-- | changes/bug12191 | 7 | ||||
| -rw-r--r-- | changes/bug6799 | 20 | ||||
| -rw-r--r-- | changes/more_8387_diagnosis | 4 | ||||
| -rw-r--r-- | changes/not_bug_8093 | 4 | ||||
| -rw-r--r-- | src/common/address.c | 2 | ||||
| -rw-r--r-- | src/common/sandbox.c | 160 | ||||
| -rw-r--r-- | src/common/sandbox.h | 20 | ||||
| -rw-r--r-- | src/or/channel.c | 11 | ||||
| -rw-r--r-- | src/or/channel.h | 6 | ||||
| -rw-r--r-- | src/or/channeltls.c | 2 | ||||
| -rw-r--r-- | src/or/circuitlist.c | 10 | ||||
| -rw-r--r-- | src/or/circuituse.c | 63 | ||||
| -rw-r--r-- | src/or/command.c | 53 | ||||
| -rw-r--r-- | src/or/connection.c | 4 | ||||
| -rw-r--r-- | src/or/connection_or.c | 46 | ||||
| -rw-r--r-- | src/or/connection_or.h | 2 | ||||
| -rw-r--r-- | src/or/main.c | 40 | ||||
| -rw-r--r-- | src/or/or.h | 7 | ||||
| -rw-r--r-- | src/or/relay.c | 10 |
20 files changed, 353 insertions, 125 deletions
diff --git a/changes/bug11970 b/changes/bug11970 new file mode 100644 index 000000000..896f0cfaf --- /dev/null +++ b/changes/bug11970 @@ -0,0 +1,7 @@ + o Minor bugfixes (linux seccomp sandbox): + - Refactor the getaddrinfo workaround that the seccomp sandbox + uses to avoid calling getaddrinfo() after installing the sandbox + filters. Previously, it preloaded a cache with the IPv4 address + for our hostname, and nothing else. Now, it loads the cache with + every address that it used to initialize the Tor process. Fixes + bug 11970; bugfix on 0.2.5.1-alpha. diff --git a/changes/bug12191 b/changes/bug12191 new file mode 100644 index 000000000..77589ab31 --- /dev/null +++ b/changes/bug12191 @@ -0,0 +1,7 @@ + o Minor bugfixes: + + - We now drop CREATE cells for already-existent circuit IDs and + for zero-valued circuit IDs, regardless of other factors that + might otherwise have called for DESTROY cells. Fixes bug 12191; + bugfix on 0.0.8pre1. + diff --git a/changes/bug6799 b/changes/bug6799 new file mode 100644 index 000000000..72b6519a2 --- /dev/null +++ b/changes/bug6799 @@ -0,0 +1,20 @@ + o Major features: + + - Increase the base amount of time that a canonical connection + (one that we have made to a known OR) is allowed to stay idle + from 3 minutes to 15 minutes. This leaks less information + about when circuits have closed, and avoids unnecessary overhead + from renegotiating connections. Part of a fix for ticket 6799. + + - Instead of closing connections after they have been idle for a + fixed interval, randomly add up to 50% to each connection's + maximum timeout. This makes it harder to tell when the last + circuit closed by looking at when a connection closes. Part of a + fix for ticket 6799. + + - Base connection idleness tests on the actual time elapsed since + the connection last had circuits, not on the time when we last + added non-padding. This change also makes it harder for an + observer to tell when the last circuit closed by looking at when + a connection closes. Part of a fix for ticket 6799. + Incidentally fixes bug 12023; bugfix on 0.2.5.1-alpha. diff --git a/changes/more_8387_diagnosis b/changes/more_8387_diagnosis new file mode 100644 index 000000000..68a36a113 --- /dev/null +++ b/changes/more_8387_diagnosis @@ -0,0 +1,4 @@ + o Minor features (diagnostic): + - Improve the diagnostic log message for bug #8387 even further to + try to improve our odds of figuring out why one-hop directory + circuits sometimes do not get closed. diff --git a/changes/not_bug_8093 b/changes/not_bug_8093 new file mode 100644 index 000000000..98b421926 --- /dev/null +++ b/changes/not_bug_8093 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - When rejecting DATA cells for stream_id zero, still count them against + the circuit's deliver window so that we don't get fail to send a + SENDME. Fix for bug 11246; bugfix on 0.2.4.10-alpha. diff --git a/src/common/address.c b/src/common/address.c index 2825b123d..29d4c0447 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -264,7 +264,7 @@ tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr) &((struct sockaddr_in6*)best->ai_addr)->sin6_addr); result = 0; } - freeaddrinfo(res); + sandbox_freeaddrinfo(res); return result; } return (err == EAI_AGAIN) ? 1 : -1; diff --git a/src/common/sandbox.c b/src/common/sandbox.c index 4721b8dfc..05b91be7b 100644 --- a/src/common/sandbox.c +++ b/src/common/sandbox.c @@ -33,6 +33,8 @@ #include "util.h" #include "tor_queue.h" +#include "ht.h" + #define DEBUGGING_CLOSE #if defined(USE_LIBSECCOMP) @@ -93,8 +95,6 @@ static int sandbox_active = 0; /** Holds the parameter list configuration for the sandbox.*/ static sandbox_cfg_t *filter_dynamic = NULL; -/** Holds a list of pre-recorded results from getaddrinfo().*/ -static sb_addr_info_t *sb_addr_info = NULL; #undef SCMP_CMP #define SCMP_CMP(a,b,c) ((struct scmp_arg_cmp){(a),(b),(c),0}) @@ -1332,73 +1332,153 @@ sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...) } #endif +/** Cache entry for getaddrinfo results; used when sandboxing is implemented + * so that we can consult the cache when the sandbox prevents us from doing + * getaddrinfo. + * + * We support only a limited range of getaddrinfo calls, where servname is null + * and hints contains only socktype=SOCK_STREAM, family in INET,INET6,UNSPEC. + */ +typedef struct cached_getaddrinfo_item_t { + HT_ENTRY(cached_getaddrinfo_item_t) node; + char *name; + int family; + /** set if no error; otherwise NULL */ + struct addrinfo *res; + /** 0 for no error; otherwise an EAI_* value */ + int err; +} cached_getaddrinfo_item_t; + +static unsigned +cached_getaddrinfo_item_hash(const cached_getaddrinfo_item_t *item) +{ + return (unsigned)siphash24g(item->name, strlen(item->name)) + item->family; +} + +static unsigned +cached_getaddrinfo_items_eq(const cached_getaddrinfo_item_t *a, + const cached_getaddrinfo_item_t *b) +{ + return (a->family == b->family) && 0 == strcmp(a->name, b->name); +} + +static void +cached_getaddrinfo_item_free(cached_getaddrinfo_item_t *item) +{ + if (item == NULL) + return; + + tor_free(item->name); + if (item->res) + freeaddrinfo(item->res); + tor_free(item); +} + +static HT_HEAD(getaddrinfo_cache, cached_getaddrinfo_item_t) + getaddrinfo_cache = HT_INITIALIZER(); + +HT_PROTOTYPE(getaddrinfo_cache, cached_getaddrinfo_item_t, node, + cached_getaddrinfo_item_hash, + cached_getaddrinfo_items_eq); +HT_GENERATE(getaddrinfo_cache, cached_getaddrinfo_item_t, node, + cached_getaddrinfo_item_hash, + cached_getaddrinfo_items_eq, + 0.6, tor_malloc_, tor_realloc_, tor_free_); + int sandbox_getaddrinfo(const char *name, const char *servname, const struct addrinfo *hints, struct addrinfo **res) { - sb_addr_info_t *el; + int err; + struct cached_getaddrinfo_item_t search, *item; - if (servname != NULL) - return -1; + if (servname != NULL) { + log_warn(LD_BUG, "called with non-NULL servname"); + return EAI_NONAME; + } + if (name == NULL) { + log_warn(LD_BUG, "called with NULL name"); + return EAI_NONAME; + } *res = NULL; - for (el = sb_addr_info; el; el = el->next) { - if (!strcmp(el->name, name)) { - *res = tor_malloc(sizeof(struct addrinfo)); + memset(&search, 0, sizeof(search)); + search.name = (char *) name; + search.family = hints ? hints->ai_family : AF_UNSPEC; + item = HT_FIND(getaddrinfo_cache, &getaddrinfo_cache, &search); - memcpy(*res, el->info, sizeof(struct addrinfo)); - /* XXXX What if there are multiple items in the list? */ - return 0; + if (! sandbox_is_active()) { + /* If the sandbox is not turned on yet, then getaddrinfo and store the + result. */ + + err = getaddrinfo(name, NULL, hints, res); + log_info(LD_NET,"(Sandbox) getaddrinfo %s.", err ? "failed" : "succeeded"); + + if (! item) { + item = tor_malloc_zero(sizeof(*item)); + item->name = tor_strdup(name); + item->family = hints ? hints->ai_family : AF_UNSPEC; + HT_INSERT(getaddrinfo_cache, &getaddrinfo_cache, item); } - } - if (!sandbox_active) { - if (getaddrinfo(name, NULL, hints, res)) { - log_err(LD_BUG,"(Sandbox) getaddrinfo failed!"); - return -1; + if (item->res) { + freeaddrinfo(item->res); + item->res = NULL; } + item->res = *res; + item->err = err; + return err; + } - return 0; + /* Otherwise, the sanbox is on. If we have an item, yield its cached + result. */ + if (item) { + *res = item->res; + return item->err; } - // getting here means something went wrong + /* getting here means something went wrong */ log_err(LD_BUG,"(Sandbox) failed to get address %s!", name); - if (*res) { - tor_free(*res); - res = NULL; - } - return -1; + return EAI_NONAME; } int -sandbox_add_addrinfo(const char* name) +sandbox_add_addrinfo(const char *name) { - int ret; + struct addrinfo *res; struct addrinfo hints; - sb_addr_info_t *el = NULL; - - el = tor_malloc(sizeof(sb_addr_info_t)); + int i; + static const int families[] = { AF_INET, AF_INET6, AF_UNSPEC }; memset(&hints, 0, sizeof(hints)); - hints.ai_family = AF_INET; hints.ai_socktype = SOCK_STREAM; + for (i = 0; i < 3; ++i) { + hints.ai_family = families[i]; - ret = getaddrinfo(name, NULL, &hints, &(el->info)); - if (ret) { - log_err(LD_BUG,"(Sandbox) failed to getaddrinfo"); - ret = -2; - tor_free(el); - goto out; + res = NULL; + (void) sandbox_getaddrinfo(name, NULL, &hints, &res); + if (res) + sandbox_freeaddrinfo(res); } - el->name = tor_strdup(name); - el->next = sb_addr_info; - sb_addr_info = el; + return 0; +} - out: - return ret; +void +sandbox_free_getaddrinfo_cache(void) +{ + cached_getaddrinfo_item_t **next, **item; + + for (item = HT_START(getaddrinfo_cache, &getaddrinfo_cache); + item; + item = next) { + next = HT_NEXT_RMV(getaddrinfo_cache, &getaddrinfo_cache, item); + cached_getaddrinfo_item_free(*item); + } + + HT_CLEAR(getaddrinfo_cache, &getaddrinfo_cache); } /** diff --git a/src/common/sandbox.h b/src/common/sandbox.h index c3c676663..20d5d5080 100644 --- a/src/common/sandbox.h +++ b/src/common/sandbox.h @@ -91,21 +91,6 @@ struct sandbox_cfg_elem { struct sandbox_cfg_elem *next; }; -/** - * Structure used for keeping a linked list of getaddrinfo pre-recorded - * results. - */ -struct sb_addr_info_el { - /** Name of the address info result. */ - char *name; - /** Pre-recorded getaddrinfo result. */ - struct addrinfo *info; - /** Next element in the list. */ - struct sb_addr_info_el *next; -}; -/** Typedef to structure used to manage an addrinfo list. */ -typedef struct sb_addr_info_el sb_addr_info_t; - /** Function pointer defining the prototype of a filter function.*/ typedef int (*sandbox_filter_func_t)(scmp_filter_ctx ctx, sandbox_cfg_t *filter); @@ -130,11 +115,16 @@ struct addrinfo; int sandbox_getaddrinfo(const char *name, const char *servname, const struct addrinfo *hints, struct addrinfo **res); +#define sandbox_freeaddrinfo(addrinfo) ((void)0) +void sandbox_free_getaddrinfo_cache(void); #else #define sandbox_getaddrinfo(name, servname, hints, res) \ getaddrinfo((name),(servname), (hints),(res)) #define sandbox_add_addrinfo(name) \ ((void)(name)) +#define sandbox_freeaddrinfo(addrinfo) \ + freeaddrinfo((addrinfo)) +#define sandbox_free_getaddrinfo_cache() #endif #ifdef USE_LIBSECCOMP diff --git a/src/or/channel.c b/src/or/channel.c index 63af2f91c..1cc786487 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -112,7 +112,9 @@ HT_GENERATE(channel_idmap, channel_idmap_entry_s, node, channel_idmap_hash, static cell_queue_entry_t * cell_queue_entry_dup(cell_queue_entry_t *q); static void cell_queue_entry_free(cell_queue_entry_t *q, int handed_off); +#if 0 static int cell_queue_entry_is_padding(cell_queue_entry_t *q); +#endif static cell_queue_entry_t * cell_queue_entry_new_fixed(cell_t *cell); static cell_queue_entry_t * @@ -726,7 +728,7 @@ channel_init(channel_t *chan) chan->global_identifier = n_channels_allocated++; /* Init timestamp */ - chan->timestamp_last_added_nonpadding = time(NULL); + chan->timestamp_last_had_circuits = time(NULL); /* Warn about exhausted circuit IDs no more than hourly. */ chan->last_warned_circ_ids_exhausted.rate = 3600; @@ -1595,6 +1597,7 @@ cell_queue_entry_free(cell_queue_entry_t *q, int handed_off) tor_free(q); } +#if 0 /** * Check whether a cell queue entry is padding; this is a helper function * for channel_write_cell_queue_entry() @@ -1623,6 +1626,7 @@ cell_queue_entry_is_padding(cell_queue_entry_t *q) return 0; } +#endif /** * Allocate a new cell queue entry for a fixed-size cell @@ -1681,11 +1685,6 @@ channel_write_cell_queue_entry(channel_t *chan, cell_queue_entry_t *q) chan->state == CHANNEL_STATE_OPEN || chan->state == CHANNEL_STATE_MAINT); - /* Increment the timestamp unless it's padding */ - if (!cell_queue_entry_is_padding(q)) { - chan->timestamp_last_added_nonpadding = approx_time(); - } - { circid_t circ_id; if (is_destroy_cell(chan, q, &circ_id)) { diff --git a/src/or/channel.h b/src/or/channel.h index bd9a02f32..3e164c689 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -187,8 +187,10 @@ struct channel_s { time_t timestamp_recv; /* Cell received from lower layer */ time_t timestamp_xmit; /* Cell sent to lower layer */ - /* Timestamp for relay.c */ - time_t timestamp_last_added_nonpadding; + /** Timestamp for run_connection_housekeeping(). We update this once a + * second when we run housekeeping and find a circuit on this channel, and + * whenever we add a circuit to the channel. */ + time_t timestamp_last_had_circuits; /** Unique ID for measuring direct network status requests;vtunneled ones * come over a circuit_t, which has a dirreq_id field as well, but is a diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 539ead193..632bc328b 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1554,7 +1554,7 @@ channel_tls_process_netinfo_cell(cell_t *cell, channel_tls_t *chan) return; } if (tor_addr_eq(&addr, &(chan->conn->real_addr))) { - chan->conn->is_canonical = 1; + connection_or_set_canonical(chan->conn, 1); break; } cp = next; diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index ba16bb145..a2dd07fbe 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -334,10 +334,13 @@ circuit_set_p_circid_chan(or_circuit_t *or_circ, circid_t id, circuit_set_circid_chan_helper(circ, CELL_DIRECTION_IN, id, chan); - if (chan) + if (chan) { tor_assert(bool_eq(or_circ->p_chan_cells.n, or_circ->next_active_on_p_chan)); + chan->timestamp_last_had_circuits = approx_time(); + } + if (circ->p_delete_pending && old_chan) { channel_mark_circid_unusable(old_chan, old_id); circ->p_delete_pending = 0; @@ -356,9 +359,12 @@ circuit_set_n_circid_chan(circuit_t *circ, circid_t id, circuit_set_circid_chan_helper(circ, CELL_DIRECTION_OUT, id, chan); - if (chan) + if (chan) { tor_assert(bool_eq(circ->n_chan_cells.n, circ->next_active_on_n_chan)); + chan->timestamp_last_had_circuits = approx_time(); + } + if (circ->n_delete_pending && old_chan) { channel_mark_circid_unusable(old_chan, old_id); circ->n_delete_pending = 0; diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 467bef652..221afea91 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -792,7 +792,8 @@ void circuit_log_ancient_one_hop_circuits(int age) { #define MAX_ANCIENT_ONEHOP_CIRCUITS_TO_LOG 10 - time_t cutoff = time(NULL) - age; + time_t now = time(NULL); + time_t cutoff = now - age; int n_found = 0; smartlist_t *log_these = smartlist_new(); const circuit_t *circ; @@ -823,18 +824,74 @@ circuit_log_ancient_one_hop_circuits(int age) SMARTLIST_FOREACH_BEGIN(log_these, const origin_circuit_t *, ocirc) { char created[ISO_TIME_LEN+1]; + int stream_num; + const edge_connection_t *conn; + char *dirty = NULL; circ = TO_CIRCUIT(ocirc); + format_local_iso_time(created, (time_t)circ->timestamp_created.tv_sec); + if (circ->timestamp_dirty) { + char dirty_since[ISO_TIME_LEN+1]; + format_local_iso_time(dirty_since, circ->timestamp_dirty); + + tor_asprintf(&dirty, "Dirty since %s (%ld seconds vs %ld-second cutoff)", + dirty_since, (long)(now - circ->timestamp_dirty), + (long) get_options()->MaxCircuitDirtiness); + } else { + dirty = tor_strdup("Not marked dirty"); + } + log_notice(LD_HEARTBEAT, " #%d created at %s. %s, %s. %s for close. " - "%s for new conns.", + "%s for new conns. %s.", ocirc_sl_idx, created, circuit_state_to_string(circ->state), circuit_purpose_to_string(circ->purpose), circ->marked_for_close ? "Marked" : "Not marked", - ocirc->unusable_for_new_conns ? "Not usable" : "usable"); + ocirc->unusable_for_new_conns ? "Not usable" : "usable", + dirty); + tor_free(dirty); + + stream_num = 0; + for (conn = ocirc->p_streams; conn; conn = conn->next_stream) { + const connection_t *c = TO_CONN(conn); + char stream_created[ISO_TIME_LEN+1]; + if (++stream_num >= 5) + break; + + format_local_iso_time(stream_created, c->timestamp_created); + + log_notice(LD_HEARTBEAT, " Stream#%d created at %s. " + "%s conn in state %s. " + "%s for close (%s:%d). Hold-open is %sset. " + "Has %ssent RELAY_END. %s on circuit.", + stream_num, + stream_created, + conn_type_to_string(c->type), + conn_state_to_string(c->type, c->state), + c->marked_for_close ? "Marked" : "Not marked", + c->marked_for_close_file ? c->marked_for_close_file : "--", + c->marked_for_close, + c->hold_open_until_flushed ? "" : "not ", + conn->edge_has_sent_end ? "" : "not ", + conn->edge_blocked_on_circ ? "Blocked" : "Not blocked"); + if (! c->linked_conn) + continue; + + c = c->linked_conn; + + log_notice(LD_HEARTBEAT, " Linked to %s connection in state %s " + "(Purpose %d). %s for close (%s:%d). Hold-open is %sset. ", + conn_type_to_string(c->type), + conn_state_to_string(c->type, c->state), + c->purpose, + c->marked_for_close ? "Marked" : "Not marked", + c->marked_for_close_file ? c->marked_for_close_file : "--", + c->marked_for_close, + c->hold_open_until_flushed ? "" : "not "); + } } SMARTLIST_FOREACH_END(ocirc); done: diff --git a/src/or/command.c b/src/or/command.c index 105bdc637..fa2a0e74e 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -227,6 +227,34 @@ command_process_create_cell(cell_t *cell, channel_t *chan) (unsigned)cell->circ_id, U64_PRINTF_ARG(chan->global_identifier), chan); + /* We check for the conditions that would make us drop the cell before + * we check for the conditions that would make us send a DESTROY back, + * since those conditions would make a DESTROY nonsensical. */ + if (cell->circ_id == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Received a create cell (type %d) from %s with zero circID; " + " ignoring.", (int)cell->command, + channel_get_actual_remote_descr(chan)); + return; + } + + if (circuit_id_in_use_on_channel(cell->circ_id, chan)) { + const node_t *node = node_get_by_id(chan->identity_digest); + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Received CREATE cell (circID %u) for known circ. " + "Dropping (age %d).", + (unsigned)cell->circ_id, + (int)(time(NULL) - channel_when_created(chan))); + if (node) { + char *p = esc_for_log(node_get_platform(node)); + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, + "Details: router %s, platform %s.", + node_describe(node), p); + tor_free(p); + } + return; + } + if (we_are_hibernating()) { log_info(LD_OR, "Received create cell but we're shutting down. Sending back " @@ -248,14 +276,6 @@ command_process_create_cell(cell_t *cell, channel_t *chan) return; } - if (cell->circ_id == 0) { - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Received a create cell (type %d) from %s with zero circID; " - " ignoring.", (int)cell->command, - channel_get_actual_remote_descr(chan)); - return; - } - /* If the high bit of the circuit ID is not as expected, close the * circ. */ if (chan->wide_circ_ids) @@ -274,23 +294,6 @@ command_process_create_cell(cell_t *cell, channel_t *chan) return; } - if (circuit_id_in_use_on_channel(cell->circ_id, chan)) { - const node_t *node = node_get_by_id(chan->identity_digest); - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Received CREATE cell (circID %u) for known circ. " - "Dropping (age %d).", - (unsigned)cell->circ_id, - (int)(time(NULL) - channel_when_created(chan))); - if (node) { - char *p = esc_for_log(node_get_platform(node)); - log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Details: router %s, platform %s.", - node_describe(node), p); - tor_free(p); - } - return; - } - circ = or_circuit_new(cell->circ_id, chan); circ->base_.purpose = CIRCUIT_PURPOSE_OR; circuit_set_state(TO_CIRCUIT(circ), CIRCUIT_STATE_ONIONSKIN_PENDING); diff --git a/src/or/connection.c b/src/or/connection.c index cef9172ff..0b03092f7 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -269,8 +269,6 @@ dir_connection_new(int socket_family) /** Allocate and return a new or_connection_t, initialized as by * connection_init(). * - * Set timestamp_last_added_nonpadding to now. - * * Initialize active_circuit_pqueue. * * Set active_circuit_pqueue_last_recalibrated to current cell_ewma tick. @@ -283,7 +281,7 @@ or_connection_new(int type, int socket_family) tor_assert(type == CONN_TYPE_OR || type == CONN_TYPE_EXT_OR); connection_init(now, TO_CONN(or_conn), type, socket_family); - or_conn->timestamp_last_added_nonpadding = time(NULL); + connection_or_set_canonical(or_conn, 0); if (type == CONN_TYPE_EXT_OR) connection_or_set_ext_or_identifier(or_conn); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 6572a918e..16f87349f 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -826,6 +826,45 @@ connection_or_update_token_buckets(smartlist_t *conns, }); } +/** How long do we wait before killing non-canonical OR connections with no + * circuits? In Tor versions up to 0.2.1.25 and 0.2.2.12-alpha, we waited 15 + * minutes before cancelling these connections, which caused fast relays to + * accrue many many idle connections. Hopefully 3-4.5 minutes is low enough + * that it kills most idle connections, without being so low that we cause + * clients to bounce on and off. + * + * For canonical connections, the limit is higher, at 15-22.5 minutes. + * + * For each OR connection, we randomly add up to 50% extra to its idle_timeout + * field, to avoid exposing when exactly the last circuit closed. Since we're + * storing idle_timeout in a uint16_t, don't let these values get higher than + * 12 hours or so without revising connection_or_set_canonical and/or expanding + * idle_timeout. + */ +#define IDLE_OR_CONN_TIMEOUT_NONCANONICAL 180 +#define IDLE_OR_CONN_TIMEOUT_CANONICAL 900 + +/* Mark <b>or_conn</b> as canonical if <b>is_canonical</b> is set, and + * non-canonical otherwise. Adjust idle_timeout accordingly. + */ +void +connection_or_set_canonical(or_connection_t *or_conn, + int is_canonical) +{ + const unsigned int timeout_base = is_canonical ? + IDLE_OR_CONN_TIMEOUT_CANONICAL : IDLE_OR_CONN_TIMEOUT_NONCANONICAL; + + if (bool_eq(is_canonical, or_conn->is_canonical) && + or_conn->idle_timeout != 0) { + /* Don't recalculate an existing idle_timeout unless the canonical + * status changed. */ + return; + } + + or_conn->is_canonical = !! is_canonical; /* force to a 1-bit boolean */ + or_conn->idle_timeout = timeout_base + crypto_rand_int(timeout_base / 2); +} + /** If we don't necessarily know the router we're connecting to, but we * have an addr/port/id_digest, then fill in as much as we can. Start * by checking to see if this describes a router we know. @@ -850,7 +889,7 @@ connection_or_init_conn_from_address(or_connection_t *conn, /* XXXX proposal 186 is making this more complex. For now, a conn is canonical when it uses the _preferred_ address. */ if (tor_addr_eq(&conn->base_.addr, &node_ap.addr)) - conn->is_canonical = 1; + connection_or_set_canonical(conn, 1); if (!started_here) { /* Override the addr/port, so our log messages will make sense. * This is dangerous, since if we ever try looking up a conn by @@ -1966,9 +2005,6 @@ connection_or_write_cell_to_buf(const cell_t *cell, or_connection_t *conn) if (conn->base_.state == OR_CONN_STATE_OR_HANDSHAKING_V3) or_handshake_state_record_cell(conn, conn->handshake_state, cell, 0); - - if (cell->command != CELL_PADDING) - conn->timestamp_last_added_nonpadding = approx_time(); } /** Pack a variable-length <b>cell</b> into wire-format, and write it onto @@ -1989,8 +2025,6 @@ connection_or_write_var_cell_to_buf(const var_cell_t *cell, cell->payload_len, TO_CONN(conn)); if (conn->base_.state == OR_CONN_STATE_OR_HANDSHAKING_V3) or_handshake_state_record_var_cell(conn, conn->handshake_state, cell, 0); - if (cell->command != CELL_PADDING) - conn->timestamp_last_added_nonpadding = approx_time(); /* Touch the channel's active timestamp if there is one */ if (conn->chan) diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 8d9302893..143540edd 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -48,6 +48,8 @@ void connection_or_report_broken_states(int severity, int domain); MOCK_DECL(int,connection_tls_start_handshake,(or_connection_t *conn, int receiving)); int connection_tls_continue_handshake(or_connection_t *conn); +void connection_or_set_canonical(or_connection_t *or_conn, + int is_canonical); int connection_init_or_handshake_state(or_connection_t *conn, int started_here); diff --git a/src/or/main.c b/src/or/main.c index 3c661cd12..090503e07 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1001,15 +1001,6 @@ directory_info_has_arrived(time_t now, int from_cache) consider_testing_reachability(1, 1); } -/** How long do we wait before killing OR connections with no circuits? - * In Tor versions up to 0.2.1.25 and 0.2.2.12-alpha, we waited 15 minutes - * before cancelling these connections, which caused fast relays to accrue - * many many idle connections. Hopefully 3 minutes is low enough that - * it kills most idle connections, without being so low that we cause - * clients to bounce on and off. - */ -#define IDLE_OR_CONN_TIMEOUT 180 - /** Perform regular maintenance tasks for a single connection. This * function gets run once per second per connection by run_scheduled_events. */ @@ -1020,6 +1011,8 @@ run_connection_housekeeping(int i, time_t now) connection_t *conn = smartlist_get(connection_array, i); const or_options_t *options = get_options(); or_connection_t *or_conn; + channel_t *chan = NULL; + int have_any_circuits; int past_keepalive = now >= conn->timestamp_lastwritten + options->KeepalivePeriod; @@ -1069,8 +1062,18 @@ run_connection_housekeeping(int i, time_t now) tor_assert(conn->outbuf); #endif + chan = TLS_CHAN_TO_BASE(or_conn->chan); + tor_assert(chan); + + if (channel_num_circuits(chan) != 0) { + have_any_circuits = 1; + chan->timestamp_last_had_circuits = now; + } else { + have_any_circuits = 0; + } + if (channel_is_bad_for_new_circs(TLS_CHAN_TO_BASE(or_conn->chan)) && - !connection_or_get_num_circuits(or_conn)) { + ! have_any_circuits) { /* It's bad for new circuits, and has no unmarked circuits on it: * mark it now. */ log_info(LD_OR, @@ -1089,19 +1092,22 @@ run_connection_housekeeping(int i, time_t now) connection_or_close_normally(TO_OR_CONN(conn), 0); } } else if (we_are_hibernating() && - !connection_or_get_num_circuits(or_conn) && + ! have_any_circuits && !connection_get_outbuf_len(conn)) { /* We're hibernating, there's no circuits, and nothing to flush.*/ log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) " "[Hibernating or exiting].", (int)conn->s,conn->address, conn->port); connection_or_close_normally(TO_OR_CONN(conn), 1); - } else if (!connection_or_get_num_circuits(or_conn) && - now >= or_conn->timestamp_last_added_nonpadding + - IDLE_OR_CONN_TIMEOUT) { + } else if (!have_any_circuits && + now - or_conn->idle_timeout >= + chan->timestamp_last_had_circuits) { log_info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) " - "[idle %d].", (int)conn->s,conn->address, conn->port, - (int)(now - or_conn->timestamp_last_added_nonpadding)); + "[no circuits for %d; timeout %d; %scanonical].", + (int)conn->s, conn->address, conn->port, + (int)(now - chan->timestamp_last_had_circuits), + or_conn->idle_timeout, + or_conn->is_canonical ? "" : "non"); connection_or_close_normally(TO_OR_CONN(conn), 0); } else if ( now >= or_conn->timestamp_lastempty + options->KeepalivePeriod*10 && @@ -2545,6 +2551,7 @@ tor_free_all(int postfork) microdesc_free_all(); ext_orport_free_all(); control_free_all(); + sandbox_free_getaddrinfo_cache(); if (!postfork) { config_free_all(); or_state_free_all(); @@ -2934,6 +2941,7 @@ sandbox_init_filter(void) sandbox_cfg_allow_stat_filename_array(&cfg, get_datadir_fname("keys"), + get_datadir_fname("stats"), get_datadir_fname2("stats", "dirreq-stats"), NULL, 0 ); diff --git a/src/or/or.h b/src/or/or.h index 6aa6b59e8..f1d68b766 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1488,13 +1488,14 @@ typedef struct or_connection_t { uint16_t link_proto; /**< What protocol version are we using? 0 for * "none negotiated yet." */ - + uint16_t idle_timeout; /**< How long can this connection sit with no + * circuits on it before we close it? Based on + * IDLE_CIRCUIT_TIMEOUT_{NON,}CANONICAL and + * on is_canonical, randomized. */ or_handshake_state_t *handshake_state; /**< If we are setting this connection * up, state information to do so. */ time_t timestamp_lastempty; /**< When was the outbuf last completely empty?*/ - time_t timestamp_last_added_nonpadding; /** When did we last add a - * non-padding cell to the outbuf? */ /* bandwidth* and *_bucket only used by ORs in OPEN state: */ int bandwidthrate; /**< Bytes/s added to the bucket. (OPEN ORs only.) */ diff --git a/src/or/relay.c b/src/or/relay.c index 58d6db2b2..f42602d41 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -1437,7 +1437,6 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, switch (rh.command) { case RELAY_COMMAND_BEGIN: case RELAY_COMMAND_CONNECTED: - case RELAY_COMMAND_DATA: case RELAY_COMMAND_END: case RELAY_COMMAND_RESOLVE: case RELAY_COMMAND_RESOLVED: @@ -1462,6 +1461,9 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, * EXIT_CONN_STATE_CONNECTING or EXIT_CONN_STATE_RESOLVING. * This speeds up HTTP, for example. */ optimistic_data = 1; + } else if (rh.stream_id == 0 && rh.command == RELAY_COMMAND_DATA) { + log_warn(LD_BUG, "Somehow I had a connection that matched a " + "data cell with stream ID 0."); } else { return connection_edge_process_relay_cell_not_open( &rh, cell, circ, conn, layer_hint); @@ -1522,7 +1524,11 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, circuit_consider_sending_sendme(circ, layer_hint); - if (!conn) { + if (rh.stream_id == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, "Relay data cell with zero " + "stream_id. Dropping."); + return 0; + } else if (!conn) { log_info(domain,"data cell dropped, unknown stream (streamid %d).", rh.stream_id); return 0; |