From a6cc15e2aedfc370fc0328edd375a869338ee4f1 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sat, 12 Mar 2011 07:17:26 -0800 Subject: [PATCH 01/23] Revert "If we are not using BEGIN_DIR cells, don't attempt to contact hidden service directories with non-open dir port." This reverts commit 9a7098487b2c25f36112b3521758f42621dcd6af. Conflicts: ChangeLog (left unchanged by this commit) --- src/or/routerlist.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/or/routerlist.c b/src/or/routerlist.c index fb8fb8815..5e9c82a1d 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5335,7 +5335,6 @@ hid_serv_get_responsible_directories(smartlist_t *responsible_dirs, { int start, found, n_added = 0, i; networkstatus_t *c = networkstatus_get_latest_consensus(); - int use_begindir = get_options()->TunnelDirConns; if (!c || !smartlist_len(c->routerstatus_list)) { log_warn(LD_REND, "We don't have a consensus, so we can't perform v2 " "rendezvous operations."); @@ -5348,14 +5347,9 @@ hid_serv_get_responsible_directories(smartlist_t *responsible_dirs, do { routerstatus_t *r = smartlist_get(c->routerstatus_list, i); if (r->is_hs_dir) { - if (r->dir_port || use_begindir) - smartlist_add(responsible_dirs, r); - else - log_info(LD_REND, "Not adding router '%s' to list of responsible " - "hidden service directories, because we have no way of " - "reaching it.", r->nickname); + smartlist_add(responsible_dirs, r); if (++n_added == REND_NUMBER_OF_CONSECUTIVE_REPLICAS) - break; + return 0; } if (++i == smartlist_len(c->routerstatus_list)) i = 0; From eb50e3d6bf6eb9880f92050cb0db849394bd6cb0 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sat, 12 Mar 2011 07:29:04 -0800 Subject: [PATCH 02/23] Add changes file for previous commit --- changes/bug2722 | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 changes/bug2722 diff --git a/changes/bug2722 b/changes/bug2722 new file mode 100644 index 000000000..ed132fc89 --- /dev/null +++ b/changes/bug2722 @@ -0,0 +1,11 @@ + o Minor bugfixes + - Ignore the TunnelDirConns option when determining which HSDir + relays are responsible for a hidden service descriptor ID. + Currently, clients and hidden services with TunnelDirConns off + will skip over HSDir relays which do not advertise a DirPort + when making a list of HSDirs responsible for a descriptor ID, + even though they would never try to use a HSDir's DirPort to + upload or fetch a hidden service descriptor. Fixes bug 2722; + bugfix on 0.2.1.6-alpha. + + From 134da2fbcf865418863bda8d4c479a20f6f6651d Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 18 Apr 2011 12:00:48 -0700 Subject: [PATCH 03/23] Add an XXX to the DA code regarding bug 2722 --- src/or/dirserv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index f65f25811..525524c42 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1811,6 +1811,8 @@ dirserv_thinks_router_is_hs_dir(routerinfo_t *router, time_t now) * bug 1693. In the future, once relays set wants_to_be_hs_dir * correctly, we can revert to only checking dir_port if router's * version is too old. */ + /* XXX Unfortunately, we need to keep checking dir_port until all + * *clients* suffering from bug 2722 are obsolete. */ return (router->wants_to_be_hs_dir && router->dir_port && uptime > get_options()->MinUptimeHidServDirectoryV2 && router->is_running); From c1927d7d5f5dff5b8d7da5bd4e7a743eb06bcbb3 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Sat, 16 Apr 2011 16:01:36 +0200 Subject: [PATCH 04/23] Don't report empty bw-history lines in extrainfo Some tor relays would report lines like these in their extrainfo documents: dirreq-write-history 2011-03-14 16:46:44 (900 s) This was confusing to some people who look at the stats. It would happen whenever a relay first starts up, or when a relay has dirport disabled. Change this so that lines without actual bw entries are omitted. Implements ticket 2497. --- changes/ticket2497 | 4 ++++ src/or/rephist.c | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 changes/ticket2497 diff --git a/changes/ticket2497 b/changes/ticket2497 new file mode 100644 index 000000000..51171412b --- /dev/null +++ b/changes/ticket2497 @@ -0,0 +1,4 @@ + o Minor features: + - Ensure that no empty [dirreq-](read|write)-history lines are added + to an extrainfo document. Implements ticket 2497. + diff --git a/src/or/rephist.c b/src/or/rephist.c index 9b7eefecf..b55797ac9 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1524,10 +1524,15 @@ rep_hist_get_bandwidth_lines(void) size_t len; /* opt [dirreq-](read|write)-history yyyy-mm-dd HH:MM:SS (n s) n,n,n... */ - len = (67+21*NUM_TOTALS)*4; +/* The n,n,n part above. Largest representation of a uint64_t is 20 chars + * long, plus the comma. */ +#define MAX_HIST_VALUE_LEN 21*NUM_TOTALS + len = (67+MAX_HIST_VALUE_LEN)*4; buf = tor_malloc_zero(len); cp = buf; for (r=0;r<4;++r) { + char tmp[MAX_HIST_VALUE_LEN]; + size_t slen; switch (r) { case 0: b = write_array; @@ -1547,11 +1552,16 @@ rep_hist_get_bandwidth_lines(void) break; } tor_assert(b); + slen = rep_hist_fill_bandwidth_history(tmp, MAX_HIST_VALUE_LEN, b); + /* If we don't have anything to write, skip to the next entry. */ + if (slen == 0) + continue; format_iso_time(t, b->next_period-NUM_SECS_BW_SUM_INTERVAL); tor_snprintf(cp, len-(cp-buf), "%s %s (%d s) ", desc, t, NUM_SECS_BW_SUM_INTERVAL); cp += strlen(cp); - cp += rep_hist_fill_bandwidth_history(cp, len-(cp-buf), b); + strlcat(cp, tmp, len-(cp-buf)); + cp += slen; strlcat(cp, "\n", len-(cp-buf)); ++cp; } From 43ffd023e9267927539dc9c12bee86199cd1c800 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 26 Apr 2011 13:00:46 -0400 Subject: [PATCH 05/23] Make SIZE_T_CEILING unsigned; add a signed SSIZE_T_CEILING None of the comparisons were _broken_ previously, but avoiding signed/unsigned comparisons makes everybody happier. Fixes bug2475. --- changes/bug2475 | 5 +++++ src/common/crypto.c | 2 +- src/common/torint.h | 4 +++- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 changes/bug2475 diff --git a/changes/bug2475 b/changes/bug2475 new file mode 100644 index 000000000..d6f0595a5 --- /dev/null +++ b/changes/bug2475 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Avoid signed/unsigned comparisons by making SIZE_T_CEILING unsigned. + (None of the cases where we did this before were wrong, but by making + this change we can avoid warnings.) Fixes bug2475; bugfix on + Tor 0.2.1.28. diff --git a/src/common/crypto.c b/src/common/crypto.c index 48c8dea08..838347e20 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -452,7 +452,7 @@ crypto_pk_read_private_key_from_string(crypto_pk_env_t *env, tor_assert(env); tor_assert(s); - tor_assert(len < INT_MAX && len < SIZE_T_CEILING); + tor_assert(len < INT_MAX && len < SSIZE_T_CEILING); /* Create a read-only memory BIO, backed by the string 's' */ b = BIO_new_mem_buf((char*)s, (int)len); diff --git a/src/common/torint.h b/src/common/torint.h index 2a9fba6fc..d48968465 100644 --- a/src/common/torint.h +++ b/src/common/torint.h @@ -330,8 +330,10 @@ typedef uint32_t uintptr_t; #endif #endif +/* Any ssize_t larger than this amount is likely to be an underflow. */ +#define SSIZE_T_CEILING ((ssize_t)(SSIZE_T_MAX-16)) /* Any size_t larger than this amount is likely to be an underflow. */ -#define SIZE_T_CEILING (SSIZE_T_MAX-16) +#define SIZE_T_CEILING ((size_t)(SSIZE_T_MAX-16)) #endif /* __TORINT_H */ From 34510f9278675e6e041503e425ca19a1fa0f3616 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 27 Apr 2011 17:21:41 -0400 Subject: [PATCH 06/23] Fix clear_trackhostexits_mapping() to actually work as advertised Previously, it would remove every trackhostexits-derived mapping *from* xyz..exit; it was supposed to remove every trackhostexits-derived mapping *to* xyz..exit. Bugfix on 0.2.0.20-rc: fixes an XXX020 added while staring at bug-1090 issues. --- changes/clear_trackexithost | 5 +++++ src/or/connection_edge.c | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changes/clear_trackexithost diff --git a/changes/clear_trackexithost b/changes/clear_trackexithost new file mode 100644 index 000000000..701d369df --- /dev/null +++ b/changes/clear_trackexithost @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Fix a bug in the code where we could keep trying to use a + TrackHostExits-based mapping after we failed to reach the intended + destination node. Bugfix on 0.2.0.20-rc. + diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 082cd5f1d..2c1196c0c 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -799,8 +799,8 @@ clear_trackexithost_mappings(const char *exitname) tor_strlower(suffix); STRMAP_FOREACH_MODIFY(addressmap, address, addressmap_entry_t *, ent) { - /* XXXX022 HEY! Shouldn't this look at ent->new_address? */ - if (ent->source == ADDRMAPSRC_TRACKEXIT && !strcmpend(address, suffix)) { + if (ent->source == ADDRMAPSRC_TRACKEXIT && + !strcmpend(ent->new_address, suffix)) { addressmap_ent_remove(address, ent); MAP_DEL_CURRENT(address); } From 7f85509a59b88173ac6060cd5925e28415f016b4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 27 Apr 2011 17:26:28 -0400 Subject: [PATCH 07/23] I guess that had a bug number: add it to the changes file. --- changes/clear_trackexithost | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/clear_trackexithost b/changes/clear_trackexithost index 701d369df..b9ac6fec4 100644 --- a/changes/clear_trackexithost +++ b/changes/clear_trackexithost @@ -1,5 +1,5 @@ o Minor bugfixes: - Fix a bug in the code where we could keep trying to use a TrackHostExits-based mapping after we failed to reach the intended - destination node. Bugfix on 0.2.0.20-rc. + destination node. Fixes bug 2999. Bugfix on 0.2.0.20-rc. From 440e48ddf27094e48c401c68c9014eca43b6867e Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 20 Apr 2011 02:27:58 -0700 Subject: [PATCH 08/23] Forget all rendezvous client state on SIGNAL NEWNYM --- changes/forget-rend-descs-on-newnym | 5 ++++ src/or/main.c | 2 ++ src/or/or.h | 2 ++ src/or/rendclient.c | 37 +++++++++++++++++++++++++++++ src/or/rendcommon.c | 10 ++++++++ 5 files changed, 56 insertions(+) create mode 100644 changes/forget-rend-descs-on-newnym diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym new file mode 100644 index 000000000..c086973d2 --- /dev/null +++ b/changes/forget-rend-descs-on-newnym @@ -0,0 +1,5 @@ + o Security fixes: + - Forget all hidden service descriptors cached as a client when + processing a SIGNAL NEWNYM command. Fixes bug 3000. Bugfix on + 0.0.6. + diff --git a/src/or/main.c b/src/or/main.c index 96b7b456e..e44fd4946 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -785,6 +785,8 @@ signewnym_impl(time_t now) { circuit_expire_all_dirty_circs(); addressmap_clear_transient(); + rend_cache_purge(); + rend_client_cancel_descriptor_fetches(); time_of_last_signewnym = now; signewnym_is_pending = 0; } diff --git a/src/or/or.h b/src/or/or.h index 57e091e9e..897ad32a4 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4039,6 +4039,7 @@ int rend_client_introduction_acked(origin_circuit_t *circ, size_t request_len); void rend_client_refetch_renddesc(const char *query); void rend_client_refetch_v2_renddesc(const rend_data_t *rend_query); +void rend_client_cancel_descriptor_fetches(void); int rend_client_remove_intro_point(extend_info_t *failed_intro, const rend_data_t *rend_query); int rend_client_rendezvous_acked(origin_circuit_t *circ, @@ -4137,6 +4138,7 @@ typedef struct rend_cache_entry_t { void rend_cache_init(void); void rend_cache_clean(void); void rend_cache_clean_v2_descs_as_dir(void); +void rend_cache_purge(void); void rend_cache_free_all(void); int rend_valid_service_id(const char *query); int rend_cache_lookup_desc(const char *query, int version, const char **desc, diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 51306b3dc..37cca1420 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -557,8 +557,45 @@ rend_client_refetch_v2_renddesc(const rend_data_t *rend_query) return; } +/** Cancel all rendezvous descriptor fetches currently in progress. + */ +void +rend_client_cancel_descriptor_fetches(void) +{ + smartlist_t *connection_array = get_connection_array(); + + SMARTLIST_FOREACH_BEGIN(connection_array, connection_t *, conn) { + if (conn->type == CONN_TYPE_DIR && + (conn->purpose == DIR_PURPOSE_FETCH_RENDDESC || + conn->purpose == DIR_PURPOSE_FETCH_RENDDESC_V2)) { + /* It's a rendezvous descriptor fetch in progress -- cancel it + * by marking the connection for close. + * + * Even if this connection has already reached EOF, this is + * enough to make sure that if the descriptor hasn't been + * processed yet, it won't be. See the end of + * connection_handle_read; connection_reached_eof (indirectly) + * processes whatever response the connection received. */ + + const rend_data_t *rd = (TO_DIR_CONN(conn))->rend_data; + if (!rd) { + log_warn(LD_BUG | LD_REND, + "Marking for close dir conn fetching rendezvous " + "descriptor for unknown service!"); + } else { + log_debug(LD_REND, "Marking for close dir conn fetching v%d " + "rendezvous descriptor for service %s", + (int)(rd->rend_desc_version), + safe_str(rd->onion_address)); + } + connection_mark_for_close(conn); + } + } SMARTLIST_FOREACH_END(conn); +} + /** Remove failed_intro from ent. If ent now has no intro points, or * service is unrecognized, then launch a new renddesc fetch. + * * Return -1 if error, 0 if no intro points remain or service * unrecognized, 1 if recognized and some intro points remain. diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index d6f544381..ba28ccabd 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -871,6 +871,16 @@ rend_cache_clean(void) } } +/** Remove ALL entries from the rendezvous service descriptor cache. + */ +void +rend_cache_purge(void) +{ + if (rend_cache) + strmap_free(rend_cache, _rend_cache_entry_free); + rend_cache = strmap_new(); +} + /** Remove all old v2 descriptors and those for which this hidden service * directory is not responsible for any more. */ void From 2ad18ae7366ce4979b44fa53d0105940005489c0 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 25 Apr 2011 08:36:02 -0700 Subject: [PATCH 09/23] Allow rend_client_send_introduction to fail transiently i.e. without closing the AP connection. --- changes/forget-rend-descs-on-newnym | 4 ++++ src/or/circuituse.c | 18 ++++++++++++------ src/or/rendclient.c | 20 ++++++++++---------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index c086973d2..ab2fd61f3 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -2,4 +2,8 @@ - Forget all hidden service descriptors cached as a client when processing a SIGNAL NEWNYM command. Fixes bug 3000. Bugfix on 0.0.6. + o Code simplifications and refactoring: + - Allow rend_client_send_introduction to fail without closing the + AP connection permanently. + diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 996c99cef..247aca7e0 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1560,14 +1560,20 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn) "introduction. (stream %d sec old)", introcirc->_base.n_circ_id, rendcirc->_base.n_circ_id, conn_age); - if (rend_client_send_introduction(introcirc, rendcirc) < 0) { + switch (rend_client_send_introduction(introcirc, rendcirc)) { + case 0: /* success */ + rendcirc->_base.timestamp_dirty = time(NULL); + introcirc->_base.timestamp_dirty = time(NULL); + assert_circuit_ok(TO_CIRCUIT(rendcirc)); + assert_circuit_ok(TO_CIRCUIT(introcirc)); + return 0; + case -1: /* transient error */ + return 0; + case -2: /* permanent error */ return -1; + default: /* oops */ + tor_fragile_assert(); } - rendcirc->_base.timestamp_dirty = time(NULL); - introcirc->_base.timestamp_dirty = time(NULL); - assert_circuit_ok(TO_CIRCUIT(rendcirc)); - assert_circuit_ok(TO_CIRCUIT(introcirc)); - return 0; } } diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 37cca1420..ca846d9b4 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -77,7 +77,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc, log_warn(LD_REND, "query %s didn't have valid rend desc in cache. Failing.", escaped_safe_str(introcirc->rend_data->onion_address)); - goto err; + goto perm_err; } /* first 20 bytes of payload are the hash of Bob's pk */ @@ -115,13 +115,13 @@ rend_client_send_introduction(origin_circuit_t *introcirc, log_info(LD_REND, "Internal error: could not find intro key; we " "only have a v2 rend desc with %d intro points.", num_intro_points); - goto err; + goto perm_err; } } } if (crypto_pk_get_digest(intro_key, payload)<0) { log_warn(LD_BUG, "Internal error: couldn't hash public key."); - goto err; + goto perm_err; } /* Initialize the pending_final_cpath and start the DH handshake. */ @@ -132,11 +132,11 @@ rend_client_send_introduction(origin_circuit_t *introcirc, cpath->magic = CRYPT_PATH_MAGIC; if (!(cpath->dh_handshake_state = crypto_dh_new(DH_TYPE_REND))) { log_warn(LD_BUG, "Internal error: couldn't allocate DH."); - goto err; + goto perm_err; } if (crypto_dh_generate_public(cpath->dh_handshake_state)<0) { log_warn(LD_BUG, "Internal error: couldn't generate g^x."); - goto err; + goto perm_err; } } @@ -186,7 +186,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc, if (crypto_dh_get_public(cpath->dh_handshake_state, tmp+dh_offset, DH_KEY_LEN)<0) { log_warn(LD_BUG, "Internal error: couldn't extract g^x."); - goto err; + goto perm_err; } note_crypto_pk_op(REND_CLIENT); @@ -199,7 +199,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc, PK_PKCS1_OAEP_PADDING, 0); if (r<0) { log_warn(LD_BUG,"Internal error: hybrid pk encrypt failed."); - goto err; + goto perm_err; } payload_len = DIGEST_LEN + r; @@ -212,17 +212,17 @@ rend_client_send_introduction(origin_circuit_t *introcirc, introcirc->cpath->prev)<0) { /* introcirc is already marked for close. leave rendcirc alone. */ log_warn(LD_BUG, "Couldn't send INTRODUCE1 cell"); - return -1; + return -2; } /* Now, we wait for an ACK or NAK on this circuit. */ introcirc->_base.purpose = CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT; return 0; -err: +perm_err: circuit_mark_for_close(TO_CIRCUIT(introcirc), END_CIRC_REASON_INTERNAL); circuit_mark_for_close(TO_CIRCUIT(rendcirc), END_CIRC_REASON_INTERNAL); - return -1; + return -2; } /** Called when a rendezvous circuit is open; sends a establish From f1cf9bd74d225e90ca123eb664c1c5538eedaa03 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 25 Apr 2011 06:38:35 -0700 Subject: [PATCH 10/23] Fix a bug introduced by purging rend_cache on NEWNYM If the user sent a SIGNAL NEWNYM command after we fetched a rendezvous descriptor, while we were building the introduction-point circuit, we would give up entirely on trying to connect to the hidden service. Original patch by rransom slightly edited to go into 0.2.1 --- src/or/rendclient.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index ca846d9b4..fb95efbdc 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -74,10 +74,27 @@ rend_client_send_introduction(origin_circuit_t *introcirc, if (rend_cache_lookup_entry(introcirc->rend_data->onion_address, -1, &entry) < 1) { - log_warn(LD_REND, - "query %s didn't have valid rend desc in cache. Failing.", - escaped_safe_str(introcirc->rend_data->onion_address)); - goto perm_err; + log_info(LD_REND, + "query %s didn't have valid rend desc in cache. " + "Refetching descriptor.", + safe_str(introcirc->rend_data->onion_address)); + /* Fetch both v0 and v2 rend descriptors in parallel. Use whichever + * arrives first. Exception: When using client authorization, only + * fetch v2 descriptors.*/ + rend_client_refetch_v2_renddesc(introcirc->rend_data); + if (introcirc->rend_data->auth_type == REND_NO_AUTH) + rend_client_refetch_renddesc(introcirc->rend_data->onion_address); + { + connection_t *conn; + + while ((conn = connection_get_by_type_state_rendquery(CONN_TYPE_AP, + AP_CONN_STATE_CIRCUIT_WAIT, + introcirc->rend_data->onion_address, -1))) { + conn->state = AP_CONN_STATE_RENDDESC_WAIT; + } + } + + return -1; } /* first 20 bytes of payload are the hash of Bob's pk */ From 8a36f2125137dc31a0771a8eeac0f2bb8c1343d0 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Thu, 28 Apr 2011 01:48:25 +0200 Subject: [PATCH 11/23] Fix a failure case of connection_ap_handshake_attach_circuit() tor_fragile_assert() might be a no-op, so we have to return something here to indicate failure to the caller. --- src/or/circuituse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 247aca7e0..6a9c3975c 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1573,6 +1573,7 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn) return -1; default: /* oops */ tor_fragile_assert(); + return -1; } } } From 2c0258b69a232a7b11ecc999bee74dac1c1b1495 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 28 Apr 2011 10:37:35 -0700 Subject: [PATCH 12/23] Clean up merge of bug3k_021 --- src/or/rendclient.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 97345bf6a..88038755e 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -94,7 +94,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc, log_info(LD_REND, "query %s didn't have valid rend desc in cache. " "Refetching descriptor.", - safe_str(introcirc->rend_data->onion_address)); + safe_str_client(introcirc->rend_data->onion_address)); rend_client_refetch_v2_renddesc(introcirc->rend_data); { connection_t *conn; @@ -120,8 +120,9 @@ rend_client_send_introduction(origin_circuit_t *introcirc, } }); if (!intro_key) { - log_info(LD_REND, "Internal error: could not find intro key; we " - "only have a v2 rend desc with %d intro points.", + log_info(LD_REND, "Our introduction point knowledge changed in " + "mid-connect! Could not find intro key; we only have a " + "v2 rend desc with %d intro points. Giving up.", smartlist_len(entry->parsed->intro_nodes)); goto perm_err; } From 51e551d3837990b7c4491253a88bedd2513fe1de Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 7 Dec 2010 14:37:50 -0500 Subject: [PATCH 13/23] Detect and handle NULL returns from (gm/local)time_r These functions can return NULL for otherwise-valid values of time_t. Notably, the glibc gmtime manpage says it can return NULL if the year if greater than INT_MAX, and the windows MSDN gmtime page says it can return NULL for negative time_t values. Also, our formatting code is not guaranteed to correctly handle years after 9999 CE. This patch tries to correct this by detecting NULL values from gmtime/localtime_r, and trying to clip them to a reasonable end of the scale. If they are in the middle of the scale, we call it a downright error. Arguably, it's a bug to get out-of-bounds dates like this to begin with. But we've had bugs of this kind in the past, and warning when we see a bug is much kinder than doing a NULL-pointer dereference. Boboper found this one too. --- changes/gmtime_null | 6 +++ src/common/compat.c | 104 +++++++++++++++++++++++++++++++++++++++----- src/common/compat.h | 9 ---- 3 files changed, 98 insertions(+), 21 deletions(-) create mode 100644 changes/gmtime_null diff --git a/changes/gmtime_null b/changes/gmtime_null new file mode 100644 index 000000000..16a25408b --- /dev/null +++ b/changes/gmtime_null @@ -0,0 +1,6 @@ + o Minor bugfixes + - On some platforms, gmtime and localtime can return NULL under + certain circumstances even for well-defined values of time_t. + Try to detect and make up for this deficiency. Possible fix for + bug 2077. Bugfix on all versions of Tor. Found by boboper. + diff --git a/src/common/compat.c b/src/common/compat.c index 27489e568..9b7c0b782 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -1996,14 +1996,88 @@ tor_gettimeofday(struct timeval *timeval) #define TIME_FNS_NEED_LOCKS #endif +static struct tm * +correct_tm(int islocal, const time_t *timep, struct tm *resultbuf, + struct tm *r) +{ + const char *outcome; + + if (PREDICT_LIKELY(r)) { + if (r->tm_year > 8099) { /* We can't strftime dates after 9999 CE. */ + r->tm_year = 8099; + r->tm_mon = 11; + r->tm_mday = 31; + r->tm_yday = 365; + r->tm_hour = 23; + r->tm_min = 59; + r->tm_sec = 59; + } + return r; + } + + /* If we get here, gmtime or localtime returned NULL. It might have done + * this because of overrun or underrun, or it might have done it because of + * some other weird issue. */ + if (timep) { + if (*timep < 0) { + r = resultbuf; + r->tm_year = 70; /* 1970 CE */ + r->tm_mon = 0; + r->tm_mday = 1; + r->tm_yday = 1; + r->tm_hour = 0; + r->tm_min = 0 ; + r->tm_sec = 0; + outcome = "Rounding up to 1970"; + goto done; + } else if (*timep >= INT32_MAX) { + /* Rounding down to INT32_MAX isn't so great, but keep in mind that we + * only do it if gmtime/localtime tells us NULL. */ + r = resultbuf; + r->tm_year = 137; /* 2037 CE */ + r->tm_mon = 11; + r->tm_mday = 31; + r->tm_yday = 365; + r->tm_hour = 23; + r->tm_min = 59; + r->tm_sec = 59; + outcome = "Rounding down to 2037"; + goto done; + } + } + + /* If we get here, then gmtime/localtime failed without getting an extreme + * value for *timep */ + + tor_fragile_assert(); + r = resultbuf; + memset(resultbuf, 0, sizeof(struct tm)); + outcome="can't recover"; + done: + log_warn(LD_BUG, "%s("I64_FORMAT") failed with error %s: %s", + islocal?"localtime":"gmtime", + timep?I64_PRINTF_ARG(*timep):0, + strerror(errno), + outcome); + return r; +} + + /** @{ */ /** As localtime_r, but defined for platforms that don't have it: * * Convert *timep to a struct tm in local time, and store the value in * *result. Return the result on success, or NULL on failure. */ -#ifndef HAVE_LOCALTIME_R -#ifdef TIME_FNS_NEED_LOCKS +#ifdef HAVE_LOCALTIME_R +struct tm * +tor_localtime_r(const time_t *timep, struct tm *result) +{ + struct tm *r; + r = localtime_r(timep, result); + return correct_tm(1, timep, result, r); +} +#elif defined(TIME_FNS_NEED_LOCKS) struct tm * tor_localtime_r(const time_t *timep, struct tm *result) { @@ -2013,9 +2087,10 @@ tor_localtime_r(const time_t *timep, struct tm *result) tor_assert(result); tor_mutex_acquire(m); r = localtime(timep); - memcpy(result, r, sizeof(struct tm)); + if (r) + memcpy(result, r, sizeof(struct tm)); tor_mutex_release(m); - return result; + return correct_tm(1, timep, result, r); } #else struct tm * @@ -2024,11 +2099,11 @@ tor_localtime_r(const time_t *timep, struct tm *result) struct tm *r; tor_assert(result); r = localtime(timep); - memcpy(result, r, sizeof(struct tm)); - return result; + if (r) + memcpy(result, r, sizeof(struct tm)); + return correct_tm(1, timep, result, r); } #endif -#endif /** @} */ /** @{ */ @@ -2038,7 +2113,14 @@ tor_localtime_r(const time_t *timep, struct tm *result) * *result. Return the result on success, or NULL on failure. */ #ifndef HAVE_GMTIME_R -#ifdef TIME_FNS_NEED_LOCKS +struct tm * +tor_gmtime_r(const time_t *timep, struct tm *result) +{ + struct tm *r; + r = gmtime_r(timep, result); + return correct_tm(0, timep, result, r); +} +#elif defined(TIME_FNS_NEED_LOCKS) struct tm * tor_gmtime_r(const time_t *timep, struct tm *result) { @@ -2050,7 +2132,7 @@ tor_gmtime_r(const time_t *timep, struct tm *result) r = gmtime(timep); memcpy(result, r, sizeof(struct tm)); tor_mutex_release(m); - return result; + return correct_tm(0, timep, result, r); } #else struct tm * @@ -2060,11 +2142,9 @@ tor_gmtime_r(const time_t *timep, struct tm *result) tor_assert(result); r = gmtime(timep); memcpy(result, r, sizeof(struct tm)); - return result; + return correct_tm(0, timep, result, r); } #endif -#endif -/** @} */ #if defined(USE_WIN32_THREADS) void diff --git a/src/common/compat.h b/src/common/compat.h index 550c08b53..af795ffba 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -321,17 +321,8 @@ struct timeval { void tor_gettimeofday(struct timeval *timeval); -#ifdef HAVE_LOCALTIME_R -#define tor_localtime_r localtime_r -#else struct tm *tor_localtime_r(const time_t *timep, struct tm *result); -#endif - -#ifdef HAVE_GMTIME_R -#define tor_gmtime_r gmtime_r -#else struct tm *tor_gmtime_r(const time_t *timep, struct tm *result); -#endif #ifndef timeradd /** Replacement for timeradd on platforms that do not have it: sets tvout to From 2dc9546eef6d748245d90b288f28ace1aa9b6f14 Mon Sep 17 00:00:00 2001 From: John Brooks Date: Mon, 3 Jan 2011 21:36:09 -0700 Subject: [PATCH 14/23] Correct the logic from f14754fbd for tor_gmtime_r --- src/common/compat.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/common/compat.c b/src/common/compat.c index 9b7c0b782..3644bd999 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -2112,7 +2112,7 @@ tor_localtime_r(const time_t *timep, struct tm *result) * Convert *timep to a struct tm in UTC, and store the value in * *result. Return the result on success, or NULL on failure. */ -#ifndef HAVE_GMTIME_R +#ifdef HAVE_GMTIME_R struct tm * tor_gmtime_r(const time_t *timep, struct tm *result) { @@ -2130,7 +2130,8 @@ tor_gmtime_r(const time_t *timep, struct tm *result) tor_assert(result); tor_mutex_acquire(m); r = gmtime(timep); - memcpy(result, r, sizeof(struct tm)); + if (r) + memcpy(result, r, sizeof(struct tm)); tor_mutex_release(m); return correct_tm(0, timep, result, r); } @@ -2141,7 +2142,8 @@ tor_gmtime_r(const time_t *timep, struct tm *result) struct tm *r; tor_assert(result); r = gmtime(timep); - memcpy(result, r, sizeof(struct tm)); + if (r) + memcpy(result, r, sizeof(struct tm)); return correct_tm(0, timep, result, r); } #endif From df5c7fedbd938c5d4634eadcf53693e07d2c8182 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 20 Apr 2011 15:20:10 -0700 Subject: [PATCH 15/23] Don't allow v0 HS auths to act as clients A v0 HS authority stores v0 HS descriptors in the same descriptor cache that its HS client functionality uses. Thus, if the HS authority operator clears its client HS descriptor cache, ALL v0 HS descriptors will be lost. That would be bad. --- changes/forget-rend-descs-on-newnym | 3 +++ src/or/config.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index ab2fd61f3..f8758c2d5 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -2,6 +2,9 @@ - Forget all hidden service descriptors cached as a client when processing a SIGNAL NEWNYM command. Fixes bug 3000. Bugfix on 0.0.6. + o Minor bugfixes: + - Don't allow v0 hidden service authorities to act as clients. + Required by fix for bug 3000. o Code simplifications and refactoring: - Allow rend_client_send_introduction to fail without closing the AP connection permanently. diff --git a/src/or/config.c b/src/or/config.c index f003e4d29..9384b3a68 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3078,6 +3078,10 @@ options_validate(or_options_t *old_options, or_options_t *options, REJECT("FetchDirInfoExtraEarly requires that you also set " "FetchDirInfoEarly"); + if (options->HSAuthoritativeDir && proxy_mode(options)) + REJECT("Running as authoritative v0 HS directory, but also configured " + "as a client."); + if (options->ConnLimit <= 0) { tor_asprintf(msg, "ConnLimit must be greater than 0, but was set to %d", From ddd1b7be2dce0845e6c08dcb5404ae65940b9edb Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 20 Apr 2011 17:01:41 -0700 Subject: [PATCH 16/23] Ignore SIGNAL NEWNYM on relay-only Tor instances --- changes/forget-rend-descs-on-newnym | 2 ++ src/or/main.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index f8758c2d5..0ea09e080 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -5,6 +5,8 @@ o Minor bugfixes: - Don't allow v0 hidden service authorities to act as clients. Required by fix for bug 3000. + - Ignore SIGNAL NEWNYM commands on relay-only Tor instances. + Required by fix for bug 3000. o Code simplifications and refactoring: - Allow rend_client_send_introduction to fail without closing the AP connection permanently. diff --git a/src/or/main.c b/src/or/main.c index 3bf21693f..a26be39fd 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -842,6 +842,13 @@ run_connection_housekeeping(int i, time_t now) static void signewnym_impl(time_t now) { + or_options_t *options = get_options(); + if (!proxy_mode(options)) { + log_info(LD_CONTROL, "Ignoring SIGNAL NEWNYM because client functionality " + "is disabled."); + return; + } + circuit_expire_all_dirty_circs(); addressmap_clear_transient(); rend_cache_purge(); From b8708b5bd30f5ef6dd34f7199a5588627486bcd2 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Tue, 26 Apr 2011 02:21:25 -0700 Subject: [PATCH 17/23] Fix bug 1930 --- changes/forget-rend-descs-on-newnym | 7 ++ src/or/rendclient.c | 103 +++++++++++++++++----------- 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index 0ea09e080..da7afbe20 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -2,6 +2,13 @@ - Forget all hidden service descriptors cached as a client when processing a SIGNAL NEWNYM command. Fixes bug 3000. Bugfix on 0.0.6. + o Major bugfixes: + - When we find that we have extended a hidden service's introduction + circuit to a relay which isn't listed as an introduction point in + the HS descriptor we currently have for the service, we now retry + one of the introduction points in the current HS descriptor. + Previously we would just give up. Bugfix on 0.2.0.10-alpha; fixes + bugs 1024 and 1930. o Minor bugfixes: - Don't allow v0 hidden service authorities to act as clients. Required by fix for bug 3000. diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 88038755e..8d024d8eb 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -66,6 +66,50 @@ rend_client_send_establish_rendezvous(origin_circuit_t *circ) return 0; } +/** Extend the introduction circuit circ to another valid + * introduction point for the hidden service it is trying to connect + * to, or mark it and launch a new circuit if we can't extend it. + * Return 0 on success. Return -1 and mark the introduction + * circuit on failure. + * + * On failure, the caller is responsible for marking the associated + * rendezvous circuit for close. */ +static int +rend_client_reextend_intro_circuit(origin_circuit_t *circ) +{ + extend_info_t *extend_info; + int result; + extend_info = rend_client_get_random_intro(circ->rend_data); + if (!extend_info) { + log_warn(LD_REND, + "No usable introduction points left for %s. Closing.", + safe_str_client(circ->rend_data->onion_address)); + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); + return -1; + } + if (circ->remaining_relay_early_cells) { + log_info(LD_REND, + "Re-extending circ %d, this time to %s.", + circ->_base.n_circ_id, extend_info->nickname); + result = circuit_extend_to_new_exit(circ, extend_info); + } else { + log_info(LD_REND, + "Building a new introduction circuit, this time to %s.", + extend_info->nickname); + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); + if (!circuit_launch_by_extend_info(CIRCUIT_PURPOSE_C_INTRODUCING, + extend_info, + CIRCLAUNCH_IS_INTERNAL)) { + log_warn(LD_REND, "Building introduction circuit failed."); + result = -1; + } else { + result = 0; + } + } + extend_info_free(extend_info); + return result; +} + /** Called when we're trying to connect an ap conn; sends an INTRODUCE1 cell * down introcirc if possible. */ @@ -120,11 +164,18 @@ rend_client_send_introduction(origin_circuit_t *introcirc, } }); if (!intro_key) { - log_info(LD_REND, "Our introduction point knowledge changed in " - "mid-connect! Could not find intro key; we only have a " - "v2 rend desc with %d intro points. Giving up.", + log_info(LD_REND, "Could not find intro key for %s at %s; we " + "have a v2 rend desc with %d intro points. " + "Trying a different intro point...", + safe_str_client(introcirc->rend_data->onion_address), + introcirc->build_state->chosen_exit->nickname, smartlist_len(entry->parsed->intro_nodes)); - goto perm_err; + + if (rend_client_reextend_intro_circuit(introcirc)) { + goto perm_err; + } else { + return -1; + } } if (crypto_pk_get_digest(intro_key, payload)<0) { log_warn(LD_BUG, "Internal error: couldn't hash public key."); @@ -227,7 +278,8 @@ rend_client_send_introduction(origin_circuit_t *introcirc, return 0; perm_err: - circuit_mark_for_close(TO_CIRCUIT(introcirc), END_CIRC_REASON_INTERNAL); + if (!introcirc->_base.marked_for_close) + circuit_mark_for_close(TO_CIRCUIT(introcirc), END_CIRC_REASON_INTERNAL); circuit_mark_for_close(TO_CIRCUIT(rendcirc), END_CIRC_REASON_INTERNAL); return -2; } @@ -290,45 +342,16 @@ rend_client_introduction_acked(origin_circuit_t *circ, * points. If any remain, extend to a new one and try again. * If none remain, refetch the service descriptor. */ + log_info(LD_REND, "Got nack for %s from %s...", + safe_str_client(circ->rend_data->onion_address), + circ->build_state->chosen_exit->nickname); if (rend_client_remove_intro_point(circ->build_state->chosen_exit, circ->rend_data) > 0) { /* There are introduction points left. Re-extend the circuit to * another intro point and try again. */ - extend_info_t *extend_info; - int result; - extend_info = rend_client_get_random_intro(circ->rend_data); - if (!extend_info) { - log_warn(LD_REND, "No introduction points left for %s. Closing.", - escaped_safe_str_client(circ->rend_data->onion_address)); - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); - return -1; - } - if (circ->remaining_relay_early_cells) { - log_info(LD_REND, - "Got nack for %s from %s. Re-extending circ %d, " - "this time to %s.", - escaped_safe_str_client(circ->rend_data->onion_address), - circ->build_state->chosen_exit->nickname, - circ->_base.n_circ_id, extend_info->nickname); - result = circuit_extend_to_new_exit(circ, extend_info); - } else { - log_info(LD_REND, - "Got nack for %s from %s. Building a new introduction " - "circuit, this time to %s.", - escaped_safe_str_client(circ->rend_data->onion_address), - circ->build_state->chosen_exit->nickname, - extend_info->nickname); - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); - if (!circuit_launch_by_extend_info(CIRCUIT_PURPOSE_C_INTRODUCING, - extend_info, - CIRCLAUNCH_IS_INTERNAL)) { - log_warn(LD_REND, "Building introduction circuit failed."); - result = -1; - } else { - result = 0; - } - } - extend_info_free(extend_info); + int result = rend_client_reextend_intro_circuit(circ); + /* XXXX If that call failed, should we close the rend circuit, + * too? */ return result; } } From 6dfc0d530113e055d91b68969c81595ddc749f07 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 28 Apr 2011 17:45:41 -0400 Subject: [PATCH 18/23] Avoid false positives from proxy_mode() Previously it would erroneously return true if ListenAddr was set for a client port, even if that port itself was 0. This would give false positives, which were not previously harmful... but which were about to become. --- src/or/router.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 0ef4728a0..65afd49f7 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1107,10 +1107,10 @@ set_server_advertised(int s) int proxy_mode(or_options_t *options) { - return (options->SocksPort != 0 || options->SocksListenAddress || - options->TransPort != 0 || options->TransListenAddress || - options->NATDPort != 0 || options->NATDListenAddress || - options->DNSPort != 0 || options->DNSListenAddress); + return (options->SocksPort != 0 || + options->TransPort != 0 || + options->NATDPort != 0 || + options->DNSPort != 0); } /** Decide if we're a publishable server. We are a publishable server if: From 525d2700dd8b6c794a69ea0be63864baeff27617 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Fri, 29 Apr 2011 01:18:32 +0200 Subject: [PATCH 19/23] Correctly check elapsed time in last hibernation period Fix bug 3020. --- changes/bug3020 | 7 +++++++ src/or/hibernate.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changes/bug3020 diff --git a/changes/bug3020 b/changes/bug3020 new file mode 100644 index 000000000..b98716122 --- /dev/null +++ b/changes/bug3020 @@ -0,0 +1,7 @@ + o Minor bugfixes: + - When checking whether a hibernation period has fully elapsed, use + the amount of seconds we expect for that period instead of using + the new period that just started. This would cause an issue because + February is a really short month. Bugfix on 0.2.2.17-alpha; + fixes bug 3020. + diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 1878d5d52..aebce4cc8 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -378,7 +378,8 @@ configure_accounting(time_t now) /* We are in the interval we thought we were in. Do nothing.*/ interval_end_time = start_of_accounting_period_after(interval_start_time); } else { - long duration = length_of_accounting_period_containing(now); + long duration = + length_of_accounting_period_containing(interval_start_time); double delta = ((double)(s_now - interval_start_time)) / duration; if (-0.50 <= delta && delta <= 0.50) { /* The start of the period is now a little later or earlier than we From 710227a77f3d47fec78497b0fabce13b4e3ce728 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Thu, 28 Apr 2011 19:19:04 -0400 Subject: [PATCH 20/23] fix a function comment --- src/or/circuitbuild.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 90572d57c..fe9426455 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4584,8 +4584,7 @@ find_bridge_by_digest(const char *digest) return NULL; } -/** We need to ask bridge for its server descriptor. address - * is a helpful string describing this bridge. */ +/** We need to ask bridge for its server descriptor. */ static void launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) { From 5693fedb60ee19048d45ed892edb07925b52678b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 28 Apr 2011 20:38:15 -0400 Subject: [PATCH 21/23] Clarify comment to say which version fixed 2722 --- src/or/dirserv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 525524c42..c8dda665e 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1812,7 +1812,8 @@ dirserv_thinks_router_is_hs_dir(routerinfo_t *router, time_t now) * correctly, we can revert to only checking dir_port if router's * version is too old. */ /* XXX Unfortunately, we need to keep checking dir_port until all - * *clients* suffering from bug 2722 are obsolete. */ + * *clients* suffering from bug 2722 are obsolete. The first version + * to fix the bug was 0.2.2.25-alpha. */ return (router->wants_to_be_hs_dir && router->dir_port && uptime > get_options()->MinUptimeHidServDirectoryV2 && router->is_running); From df3cf881d1d217b6ff3757bf95f8c2784584fec8 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Thu, 28 Apr 2011 20:40:15 -0400 Subject: [PATCH 22/23] stop putting wacky values into state->lastwritten --- changes/bug3039 | 5 +++++ src/or/circuitbuild.c | 2 +- src/or/config.c | 20 +++++++++++++++++--- src/or/config.h | 1 + 4 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 changes/bug3039 diff --git a/changes/bug3039 b/changes/bug3039 new file mode 100644 index 000000000..7347ee38e --- /dev/null +++ b/changes/bug3039 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Write the current time into the LastWritten line in our state file, + rather than the time from the previous write attempt. Also, stop + trying to use a time of -1 in our log statements. Fixes bug 3039; + bugfix on 0.2.2.14-alpha. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index fe9426455..85765e3e6 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -119,7 +119,7 @@ circuit_build_times_disabled(void) 0, 0, 1); int config_disabled = !get_options()->LearnCircuitBuildTimeout; int dirauth_disabled = get_options()->AuthoritativeDir; - int state_disabled = (get_or_state()->LastWritten == -1); + int state_disabled = did_last_state_file_write_fail() ? 1 : 0; if (consensus_disabled || config_disabled || dirauth_disabled || state_disabled) { diff --git a/src/or/config.c b/src/or/config.c index 9384b3a68..7534cc7f6 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -5119,6 +5119,18 @@ or_state_load(void) return r; } +/** Did the last time we tried to write the state file fail? If so, we + * should consider disabling such features as preemptive circuit generation + * to compute circuit-build-time. */ +static int last_state_file_write_failed = 0; + +/** Return whether the state file failed to write last time we tried. */ +int +did_last_state_file_write_fail(void) +{ + return last_state_file_write_failed; +} + /** If writing the state to disk fails, try again after this many seconds. */ #define STATE_WRITE_RETRY_INTERVAL 3600 @@ -5143,11 +5155,13 @@ or_state_save(time_t now) if (accounting_is_enabled(get_options())) accounting_run_housekeeping(now); + global_state->LastWritten = now; + tor_free(global_state->TorVersion); tor_asprintf(&global_state->TorVersion, "Tor %s", get_version()); state = config_dump(&state_format, global_state, 1, 0); - format_local_iso_time(tbuf, time(NULL)); + format_local_iso_time(tbuf, now); tor_asprintf(&contents, "# Tor state file last generated on %s local time\n" "# Other times below are in GMT\n" @@ -5158,7 +5172,7 @@ or_state_save(time_t now) if (write_str_to_file(fname, contents, 0)<0) { log_warn(LD_FS, "Unable to write state to file \"%s\"; " "will try again later", fname); - global_state->LastWritten = -1; + last_state_file_write_failed = 1; tor_free(fname); tor_free(contents); /* Try again after STATE_WRITE_RETRY_INTERVAL (or sooner, if the state @@ -5167,7 +5181,7 @@ or_state_save(time_t now) return -1; } - global_state->LastWritten = time(NULL); + last_state_file_write_failed = 0; log_info(LD_GENERAL, "Saved state to \"%s\"", fname); tor_free(fname); tor_free(contents); diff --git a/src/or/config.h b/src/or/config.h index defda35e0..78a67dddf 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -58,6 +58,7 @@ char *options_get_datadir_fname2_suffix(or_options_t *options, get_datadir_fname2_suffix((sub1), NULL, (suffix)) or_state_t *get_or_state(void); +int did_last_state_file_write_fail(void); int or_state_save(time_t now); int options_need_geoip_info(or_options_t *options, const char **reason_out); From 66de6f7eb8e2948f6c3849dbca20c7b31969b5b7 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Thu, 28 Apr 2011 21:06:25 -0400 Subject: [PATCH 23/23] relays checkpoint their state file twice a day --- changes/bug3012 | 5 +++++ src/or/config.c | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 changes/bug3012 diff --git a/changes/bug3012 b/changes/bug3012 new file mode 100644 index 000000000..dfde5fa90 --- /dev/null +++ b/changes/bug3012 @@ -0,0 +1,5 @@ + o Minor features: + - Relays can go for weeks without writing out their state file. A + relay that crashes would lose its bandwidth history (including + capacity estimate), client country statistics, and so on. Now relays + checkpoint the file at least every 12 hours. Addresses bug 3012. diff --git a/src/or/config.c b/src/or/config.c index 9384b3a68..dc2414048 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -5122,6 +5122,11 @@ or_state_load(void) /** If writing the state to disk fails, try again after this many seconds. */ #define STATE_WRITE_RETRY_INTERVAL 3600 +/** If we're a relay, how often should we checkpoint our state file even + * if nothing else dirties it? This will checkpoint ongoing stats like + * bandwidth used, per-country user stats, etc. */ +#define STATE_RELAY_CHECKPOINT_INTERVAL (12*60*60) + /** Write the persistent state to disk. Return 0 for success, <0 on failure. */ int or_state_save(time_t now) @@ -5172,7 +5177,11 @@ or_state_save(time_t now) tor_free(fname); tor_free(contents); - global_state->next_write = TIME_MAX; + if (server_mode(get_options())) + global_state->next_write = now + STATE_RELAY_CHECKPOINT_INTERVAL; + else + global_state->next_write = TIME_MAX; + return 0; }