From c90097e74a21ef70d9df2ab70e5b211d7a26dc6d Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 13 Mar 2011 21:10:32 -0700 Subject: [PATCH 01/15] Remove dead code from rend_cache_lookup_v2_desc_as_dir hid_serv_responsible_for_desc_id's return value is never negative, and there is no need to search through the consensus to find out whether we are responsible for a descriptor ID before we look in our cache for a descriptor. --- changes/bug2748 | 5 +++++ src/or/rendcommon.c | 7 ------- 2 files changed, 5 insertions(+), 7 deletions(-) create mode 100644 changes/bug2748 diff --git a/changes/bug2748 b/changes/bug2748 new file mode 100644 index 0000000000..647000d5f4 --- /dev/null +++ b/changes/bug2748 @@ -0,0 +1,5 @@ + o Minor bugfixes + - Remove dead code from rend_cache_lookup_v2_desc_as_dir. Fixes + part of bug 2748; bugfix on 0.2.0.10-alpha. + + diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index d6f5443815..ff3a4014a8 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -1008,13 +1008,6 @@ rend_cache_lookup_v2_desc_as_dir(const char *desc_id, const char **desc) safe_str(desc_id)); return -1; } - /* Determine if we are responsible. */ - if (hid_serv_responsible_for_desc_id(desc_id_digest) < 0) { - log_info(LD_REND, "Could not answer fetch request for v2 descriptor; " - "either we are no hidden service directory, or we are " - "not responsible for the requested ID."); - return -1; - } /* Lookup descriptor and return. */ e = digestmap_get(rend_cache_v2_dir, desc_id_digest); if (e) { From 96b929e743dc68f706f374c3448ed27c54a9cead Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 14 Mar 2011 01:11:30 -0700 Subject: [PATCH 02/15] Log malformed HS descriptor requests at the proper level This log message should be a 'protocol warning', not a 'warning'. --- changes/bug2748 | 5 +++++ src/or/rendcommon.c | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/changes/bug2748 b/changes/bug2748 index 647000d5f4..b522560a92 100644 --- a/changes/bug2748 +++ b/changes/bug2748 @@ -1,5 +1,10 @@ o Minor bugfixes - Remove dead code from rend_cache_lookup_v2_desc_as_dir. Fixes part of bug 2748; bugfix on 0.2.0.10-alpha. + - Log malformed requests for rendezvous descriptors as protocol + warnings, not warnings. Also, use a more informative log + message in case someone sees it at log level warning without + prior info-level messages. Fixes the other part of bug 2748; + bugfix on 0.2.0.10-alpha. diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index ff3a4014a8..9acc641026 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -1004,8 +1004,10 @@ rend_cache_lookup_v2_desc_as_dir(const char *desc_id, const char **desc) tor_assert(rend_cache_v2_dir); if (base32_decode(desc_id_digest, DIGEST_LEN, desc_id, REND_DESC_ID_V2_LEN_BASE32) < 0) { - log_warn(LD_REND, "Descriptor ID contains illegal characters: %s", - safe_str(desc_id)); + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Rejecting v2 rendezvous descriptor request -- descriptor ID " + "contains illegal characters: %s", + safe_str(desc_id)); return -1; } /* Lookup descriptor and return. */ From 56771f392e68eb2f78180daab0d8f17c9284ad11 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Mon, 30 May 2011 23:50:37 -0400 Subject: [PATCH 03/15] stop asserting at boot The patch for 3228 made us try to run init_keys() before we had loaded our state file, resulting in an assert inside init_keys. We had moved it too early in the function. Now it's later in the function, but still above the accounting calls. --- src/or/config.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index f97e9b1bea..6635cac5d6 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1180,18 +1180,6 @@ options_act(or_options_t *old_options) return -1; } - /* We want to reinit keys as needed before we do much of anything else: - keys are important, and other things can depend on them. */ - if (running_tor && - (transition_affects_workers || - (options->V3AuthoritativeDir && (!old_options || - !old_options->V3AuthoritativeDir)))) { - if (init_keys() < 0) { - log_warn(LD_BUG,"Error initializing keys; exiting"); - return -1; - } - } - if (consider_adding_dir_authorities(options, old_options) < 0) return -1; @@ -1237,6 +1225,17 @@ options_act(or_options_t *old_options) finish_daemon(options->DataDirectory); } + /* We want to reinit keys as needed before we do much of anything else: + keys are important, and other things can depend on them. */ + if (transition_affects_workers || + (options->V3AuthoritativeDir && (!old_options || + !old_options->V3AuthoritativeDir))) { + if (init_keys() < 0) { + log_warn(LD_BUG,"Error initializing keys; exiting"); + return -1; + } + } + /* Write our PID to the PID file. If we do not have write permissions we * will log a warning */ if (options->PidFile) From 7039c34519e29a18081a9bc7f3c957c473c70bc4 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Tue, 31 May 2011 20:43:55 -0400 Subject: [PATCH 04/15] fix a bridge edge case similar to 2511 If you had configured a bridge but then switched to a different bridge via the controller, you would still be willing to use the old one. --- changes/bug3321 | 7 +++++++ src/or/circuitbuild.c | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 changes/bug3321 diff --git a/changes/bug3321 b/changes/bug3321 new file mode 100644 index 0000000000..3605efce2d --- /dev/null +++ b/changes/bug3321 @@ -0,0 +1,7 @@ + o Minor bugfixes: + - In bug 2511 we fixed a case where you could use an unconfigured + bridge if you had configured it as a bridge the last time you ran + Tor. Now fix another edge case: if you had configured it as a bridge + but then switched to a different bridge via the controller, you + would still be willing to use the old one. Bugfix on 0.2.0.1-alpha; + fixes bug 3321. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 2f70b67d23..3f08448159 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3383,6 +3383,8 @@ entry_guard_set_status(entry_guard_t *e, routerinfo_t *ri, *reason = "down"; else if (options->UseBridges && ri->purpose != ROUTER_PURPOSE_BRIDGE) *reason = "not a bridge"; + else if (options->UseBridges && !routerinfo_is_a_configured_bridge(ri)) + *reason = "not a configured bridge"; else if (!options->UseBridges && !ri->is_possible_guard && !routerset_contains_router(options->EntryNodes,ri)) *reason = "not recommended as a guard"; @@ -3467,11 +3469,16 @@ entry_is_live(entry_guard_t *e, int need_uptime, int need_capacity, *msg = "no descriptor"; return NULL; } - if (get_options()->UseBridges && r->purpose != ROUTER_PURPOSE_BRIDGE) { - *msg = "not a bridge"; - return NULL; - } - if (!get_options()->UseBridges && r->purpose != ROUTER_PURPOSE_GENERAL) { + if (options->UseBridges) { + if (r->purpose != ROUTER_PURPOSE_BRIDGE) { + *msg = "not a bridge"; + return NULL; + } + if (!routerinfo_is_a_configured_bridge(r)) { + *msg = "not a configured bridge"; + return NULL; + } + } else if (r->purpose != ROUTER_PURPOSE_GENERAL) { *msg = "not general-purpose"; return NULL; } From 0fd3ad75daf925e8192aa1d44b229b3b7c29829d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 1 Jun 2011 11:07:08 -0400 Subject: [PATCH 05/15] Report wrong key sizes correctly When we introduced NEED_KEY_1024 in routerparse.c back in 0.2.0.1-alpha, I forgot to add a *8 when logging the length of a bad-length key. Bugfix for 3318 on 0.2.0.1-alpha. --- changes/bug3318 | 3 +++ src/or/routerparse.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changes/bug3318 diff --git a/changes/bug3318 b/changes/bug3318 new file mode 100644 index 0000000000..38991c4b1d --- /dev/null +++ b/changes/bug3318 @@ -0,0 +1,3 @@ + o Minor bugfixes: + - Fix a log message that said "bits" while displaying a value in + bytes. Fixes bug 3318; bugfix on 0.2.0.1-alpha. diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 1dcbc6a184..3728e9932b 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3767,7 +3767,7 @@ token_check_object(memarea_t *area, const char *kwd, case NEED_SKEY_1024: /* There must be a 1024-bit private key. */ if (tok->key && crypto_pk_keysize(tok->key) != PK_BYTES) { tor_snprintf(ebuf, sizeof(ebuf), "Wrong size on key for %s: %d bits", - kwd, (int)crypto_pk_keysize(tok->key)); + kwd, (int)crypto_pk_keysize(tok->key)*8); RET_ERR(ebuf); } /* fall through */ From 1d8bcba067ef8d96ebe022f06459d55c308343ec Mon Sep 17 00:00:00 2001 From: Gisle Date: Wed, 1 Jun 2011 11:11:12 -0400 Subject: [PATCH 06/15] Fix compile error in procmon.c An elusive compile-error (MingW-gcc v4.50 on Win_XP); a missing comma (!) and a typo ('err_msg' at line 277 changed to 'errmsg'). Aso changed the format for 'err_code' at line 293 into a "%ld" to suppress a warning. How did this go unnoticed for ~1 month? Btw. This is my 1st ever 'git commit', so it better work. --- src/common/procmon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/procmon.c b/src/common/procmon.c index 8fcc1afb7c..5c10e9a22b 100644 --- a/src/common/procmon.c +++ b/src/common/procmon.c @@ -252,7 +252,7 @@ tor_process_monitor_poll_cb(evutil_socket_t unused1, short unused2, if (!GetExitCodeProcess(procmon->hproc, &exit_code)) { char *errmsg = format_win32_error(GetLastError()); log_warn(procmon->log_domain, "Error \"%s\" occurred while polling " - "handle for monitored process %d; assuming it's dead." + "handle for monitored process %d; assuming it's dead.", errmsg, procmon->pid); tor_free(errmsg); its_dead_jim = 1; @@ -287,12 +287,12 @@ tor_process_monitor_poll_cb(evutil_socket_t unused1, short unused2, if (!its_dead_jim) log_info(procmon->log_domain, "Failed to open handle to monitored " - "process %d, and error code %d (%s) is not 'invalid " + "process %d, and error code %lu (%s) is not 'invalid " "parameter' -- assuming the process is still alive.", procmon->pid, - err_code, err_msg); + err_code, errmsg); - tor_free(err_msg); + tor_free(errmsg); } } #else From a1d866edc9af82c2134590f7ee3c4e18156a655c Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 2 Jun 2011 02:24:18 -0700 Subject: [PATCH 07/15] Make last_hid_serv_requests functions less fragile Previously, Tor would dereference a NULL pointer and crash if lookup_last_hid_serv_request were called before the first call to directory_clean_last_hid_serv_requests. As far as I can tell, that's currently impossible, but I want that undocumented invariant to go away in case I^Wwe break it someday. --- src/or/rendclient.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 29b9d260ed..ec6e3f2bed 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -377,7 +377,17 @@ rend_client_introduction_acked(origin_circuit_t *circ, * certain queries; keys are strings consisting of base32-encoded * hidden service directory identities and base32-encoded descriptor IDs; * values are pointers to timestamps of the last requests. */ -static strmap_t *last_hid_serv_requests = NULL; +static strmap_t *last_hid_serv_requests_ = NULL; + +/** Returns last_hid_serv_requests_, initializing it to a new strmap if + * necessary. */ +static strmap_t * +get_last_hid_serv_requests(void) +{ + if (!last_hid_serv_requests_) + last_hid_serv_requests_ = strmap_new(); + return last_hid_serv_requests_; +} /** Look up the last request time to hidden service directory hs_dir * for descriptor ID desc_id_base32. If set is non-zero, @@ -391,6 +401,7 @@ lookup_last_hid_serv_request(routerstatus_t *hs_dir, char hsdir_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1]; char hsdir_desc_comb_id[2 * REND_DESC_ID_V2_LEN_BASE32 + 1]; time_t *last_request_ptr; + strmap_t *last_hid_serv_requests = get_last_hid_serv_requests(); base32_encode(hsdir_id_base32, sizeof(hsdir_id_base32), hs_dir->identity_digest, DIGEST_LEN); tor_snprintf(hsdir_desc_comb_id, sizeof(hsdir_desc_comb_id), "%s%s", @@ -416,8 +427,7 @@ directory_clean_last_hid_serv_requests(void) { strmap_iter_t *iter; time_t cutoff = time(NULL) - REND_HID_SERV_DIR_REQUERY_PERIOD; - if (!last_hid_serv_requests) - last_hid_serv_requests = strmap_new(); + strmap_t *last_hid_serv_requests = get_last_hid_serv_requests(); for (iter = strmap_iter_init(last_hid_serv_requests); !strmap_iter_done(iter); ) { const char *key; From b0e7925c0205a68d730025cc8832110c1675cfd7 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 2 Jun 2011 02:46:04 -0700 Subject: [PATCH 08/15] Clear last_hid_serv_requests on SIGNAL NEWNYM Fixes bug #3309. --- changes/bug3309 | 9 +++++++++ src/or/main.c | 1 + src/or/rendclient.c | 19 +++++++++++++++++++ src/or/rendclient.h | 1 + 4 files changed, 30 insertions(+) create mode 100644 changes/bug3309 diff --git a/changes/bug3309 b/changes/bug3309 new file mode 100644 index 0000000000..5df0d9d348 --- /dev/null +++ b/changes/bug3309 @@ -0,0 +1,9 @@ + o Minor bugfixes: + - Clear the table recording the time of the last request for each + hidden service descriptor from each HS directory on SIGNAL + NEWNYM. Previously, we would clear our HS descriptor cache on + SIGNAL NEWNYM, but if we had previously retrieved a descriptor + (or tried to) from every directory responsible for it, we would + refuse to fetch it again for up to 15 minutes. Bugfix on + 0.2.2.25-alpha; fixes bug 3309. + diff --git a/src/or/main.c b/src/or/main.c index adbde9044f..3c84ddaa64 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -855,6 +855,7 @@ signewnym_impl(time_t now) addressmap_clear_transient(); rend_cache_purge(); rend_client_cancel_descriptor_fetches(); + rend_client_purge_last_hid_serv_requests(); time_of_last_signewnym = now; signewnym_is_pending = 0; } diff --git a/src/or/rendclient.c b/src/or/rendclient.c index ec6e3f2bed..12b54df0d6 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -444,6 +444,25 @@ directory_clean_last_hid_serv_requests(void) } } +/** Purge the history of request times to hidden service directories, + * so that future lookups of an HS descriptor will not fail because we + * accessed all of the HSDir relays responsible for the descriptor + * recently. */ +void +rend_client_purge_last_hid_serv_requests(void) +{ + /* Don't create the table if it doesn't exist yet (and it may very + * well not exist if the user hasn't accessed any HSes)... */ + strmap_t *old_last_hid_serv_requests = last_hid_serv_requests_; + /* ... and let get_last_hid_serv_requests re-create it for us if + * necessary. */ + last_hid_serv_requests_ = NULL; + + if (old_last_hid_serv_requests != NULL) { + strmap_free(old_last_hid_serv_requests, _tor_free); + } +} + /** Determine the responsible hidden service directories for desc_id * and fetch the descriptor belonging to that ID from one of them. Only * send a request to hidden service directories that we did not try within diff --git a/src/or/rendclient.h b/src/or/rendclient.h index 6910c1a97b..2bfc850ad5 100644 --- a/src/or/rendclient.h +++ b/src/or/rendclient.h @@ -19,6 +19,7 @@ int rend_client_introduction_acked(origin_circuit_t *circ, size_t request_len); void rend_client_refetch_v2_renddesc(const rend_data_t *rend_query); void rend_client_cancel_descriptor_fetches(void); +void rend_client_purge_last_hid_serv_requests(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, From d7af8a2f076a38d8be32798d34049ce150c8dda0 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 2 Jun 2011 02:57:29 -0700 Subject: [PATCH 09/15] Refactor HS client state-clearing code into a separate function --- src/or/main.c | 4 +--- src/or/rendclient.c | 10 ++++++++++ src/or/rendclient.h | 2 ++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 3c84ddaa64..bc639dbdd8 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -853,9 +853,7 @@ signewnym_impl(time_t now) circuit_expire_all_dirty_circs(); addressmap_clear_transient(); - rend_cache_purge(); - rend_client_cancel_descriptor_fetches(); - rend_client_purge_last_hid_serv_requests(); + rend_client_purge_state(); time_of_last_signewnym = now; signewnym_is_pending = 0; } diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 12b54df0d6..329b2567d6 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -27,6 +27,16 @@ static extend_info_t *rend_client_get_random_intro_impl( const rend_cache_entry_t *rend_query, const int strict, const int warnings); +/** Purge all potentially remotely-detectable state held in the hidden + * service client code. Called on SIGNAL NEWNYM. */ +void +rend_client_purge_state(void) +{ + rend_cache_purge(); + rend_client_cancel_descriptor_fetches(); + rend_client_purge_last_hid_serv_requests(); +} + /** Called when we've established a circuit to an introduction point: * send the introduction request. */ void diff --git a/src/or/rendclient.h b/src/or/rendclient.h index 2bfc850ad5..c6cf82b3dd 100644 --- a/src/or/rendclient.h +++ b/src/or/rendclient.h @@ -12,6 +12,8 @@ #ifndef _TOR_RENDCLIENT_H #define _TOR_RENDCLIENT_H +void rend_client_purge_state(void); + void rend_client_introcirc_has_opened(origin_circuit_t *circ); void rend_client_rendcirc_has_opened(origin_circuit_t *circ); int rend_client_introduction_acked(origin_circuit_t *circ, From fc4158dad706a76acbfd3adae8b505495f1004e5 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 2 Jun 2011 03:07:09 -0700 Subject: [PATCH 10/15] Add info-level log messages during HS-client-state purge I hope these will never be useful, but having them and not needing them is better than needing them and not having them. --- changes/bug3309 | 4 ++++ src/or/rendclient.c | 1 + src/or/rendcommon.c | 4 +++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/changes/bug3309 b/changes/bug3309 index 5df0d9d348..104056d8e3 100644 --- a/changes/bug3309 +++ b/changes/bug3309 @@ -7,3 +7,7 @@ refuse to fetch it again for up to 15 minutes. Bugfix on 0.2.2.25-alpha; fixes bug 3309. + o Minor features: + - Log (at info level) when purging pieces of hidden-service-client + state on SIGNAL NEWNYM. + diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 329b2567d6..533dfb8a97 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -469,6 +469,7 @@ rend_client_purge_last_hid_serv_requests(void) last_hid_serv_requests_ = NULL; if (old_last_hid_serv_requests != NULL) { + log_info(LD_REND, "Purging client last-HS-desc-request-time table"); strmap_free(old_last_hid_serv_requests, _tor_free); } } diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 4d4a90f61a..683e11ad24 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -839,8 +839,10 @@ rend_cache_clean(void) void rend_cache_purge(void) { - if (rend_cache) + if (rend_cache) { + log_info(LD_REND, "Purging client/v0-HS-authority HS descriptor cache"); strmap_free(rend_cache, _rend_cache_entry_free); + } rend_cache = strmap_new(); } From df42eb0a18a9f6fe4b729a1c9a63bb6ab247f631 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Thu, 2 Jun 2011 13:30:32 +0200 Subject: [PATCH 11/15] Fix unit test failure in dir/formats options->DirPort is 0 in the unit tests, so router_get_advertised_dir_port() would return 0 so we wouldn't pick a dirport. This isn't what we want for the unit tests. Fixes bug introduced in 95ac3ea5946. --- src/or/connection.c | 4 ++-- src/or/dirserv.c | 5 +++-- src/or/router.c | 18 +++++++++++------- src/or/router.h | 3 ++- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 3dcb573759..3f4ca1db4b 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1952,7 +1952,7 @@ retry_all_listeners(smartlist_t *replaced_conns, or_options_t *options = get_options(); int retval = 0; const uint16_t old_or_port = router_get_advertised_or_port(options); - const uint16_t old_dir_port = router_get_advertised_dir_port(options); + const uint16_t old_dir_port = router_get_advertised_dir_port(options, 0); if (retry_listeners(CONN_TYPE_OR_LISTENER, options->ORListenAddress, options->ORPort, "0.0.0.0", @@ -1998,7 +1998,7 @@ retry_all_listeners(smartlist_t *replaced_conns, return -1; if (old_or_port != router_get_advertised_or_port(options) || - old_dir_port != router_get_advertised_dir_port(options)) { + old_dir_port != router_get_advertised_dir_port(options, 0)) { /* Our chosen ORPort or DirPort is not what it used to be: the * descriptor we had (if any) should be regenerated. (We won't * automatically notice this because of changes in the option, diff --git a/src/or/dirserv.c b/src/or/dirserv.c index e9355fedb4..d114d8654e 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2705,7 +2705,7 @@ dirserv_generate_networkstatus_vote_obj(crypto_pk_env_t *private_key, voter->sigs = smartlist_create(); voter->address = hostname; voter->addr = addr; - voter->dir_port = router_get_advertised_dir_port(options); + voter->dir_port = router_get_advertised_dir_port(options, 0); voter->or_port = router_get_advertised_or_port(options); voter->contact = tor_strdup(contact); if (options->V3AuthUseLegacyKey) { @@ -2812,7 +2812,8 @@ generate_v2_networkstatus_opinion(void) "dir-options%s%s%s%s\n" "%s" /* client version line, server version line. */ "dir-signing-key\n%s", - hostname, ipaddr, (int)router_get_advertised_dir_port(options), + hostname, ipaddr, + (int)router_get_advertised_dir_port(options, 0), fingerprint, contact, published, diff --git a/src/or/router.c b/src/or/router.c index 30a340a05f..68e29bb4c8 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -704,7 +704,7 @@ init_keys(void) ds = router_get_trusteddirserver_by_digest(digest); if (!ds) { ds = add_trusted_dir_server(options->Nickname, NULL, - router_get_advertised_dir_port(options), + router_get_advertised_dir_port(options, 0), router_get_advertised_or_port(options), digest, v3_digest, @@ -802,7 +802,7 @@ decide_to_advertise_dirport(or_options_t *options, uint16_t dir_port) return 0; if (!check_whether_dirport_reachable()) return 0; - if (!router_get_advertised_dir_port(options)) + if (!router_get_advertised_dir_port(options, dir_port)) return 0; /* Section two: reasons to publish or not publish that the user @@ -1184,12 +1184,16 @@ router_get_advertised_or_port(or_options_t *options) return options->ORPort; } -/** Return the port that we should advertise as our DirPort; this is either - * the one configured in the DirPort option, or the one we actually bound to - * if DirPort is "auto". */ +/** Return the port that we should advertise as our DirPort; + * this is one of three possibilities: + * The one that is passed as dirport if the DirPort option is 0, or + * the one configured in the DirPort option, + * or the one we actually bound to if DirPort is "auto". */ uint16_t -router_get_advertised_dir_port(or_options_t *options) +router_get_advertised_dir_port(or_options_t *options, uint16_t dirport) { + if (!options->DirPort) + return dirport; if (options->DirPort == CFG_AUTO_PORT) { connection_t *c = connection_get_by_type(CONN_TYPE_DIR_LISTENER); if (c) @@ -1440,7 +1444,7 @@ router_rebuild_descriptor(int force) ri->nickname = tor_strdup(options->Nickname); ri->addr = addr; ri->or_port = router_get_advertised_or_port(options); - ri->dir_port = router_get_advertised_dir_port(options); + ri->dir_port = router_get_advertised_dir_port(options, 0); ri->cache_info.published_on = time(NULL); ri->onion_pkey = crypto_pk_dup_key(get_onion_key()); /* must invoke from * main thread */ diff --git a/src/or/router.h b/src/or/router.h index a27c1d92c5..3733099f93 100644 --- a/src/or/router.h +++ b/src/or/router.h @@ -51,7 +51,8 @@ int authdir_mode_tests_reachability(or_options_t *options); int authdir_mode_bridge(or_options_t *options); uint16_t router_get_advertised_or_port(or_options_t *options); -uint16_t router_get_advertised_dir_port(or_options_t *options); +uint16_t router_get_advertised_dir_port(or_options_t *options, + uint16_t dirport); int server_mode(or_options_t *options); int public_server_mode(or_options_t *options); From 507c1257a4d9c629fefc2adbad8db73607749734 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 May 2011 23:32:38 -0400 Subject: [PATCH 12/15] Add an "auto" option to UseBridges UseBridges 1 now means "connect only to bridges; if you know no bridges, don't make connections." UseBridges auto means "Use bridges if they are known, and we have no EntryNodes set, and we aren't a server." UseBridges 0 means "don't use bridges." --- src/or/config.c | 19 +++++++++++++++---- src/or/or.h | 12 +++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 6635cac5d6..44cecf353b 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -376,7 +376,7 @@ static config_var_t _option_vars[] = { V(TransPort, PORT, "0"), V(TunnelDirConns, BOOL, "1"), V(UpdateBridgesFromAuthority, BOOL, "0"), - V(UseBridges, BOOL, "0"), + VAR("UseBridges", STRING, UseBridges_, "auto"), V(UseEntryGuards, BOOL, "1"), V(User, STRING, NULL), VAR("V1AuthoritativeDirectory",BOOL, V1AuthoritativeDir, "0"), @@ -3232,6 +3232,19 @@ options_validate(or_options_t *old_options, or_options_t *options, "of the Internet, so they must not set Reachable*Addresses " "or FascistFirewall."); + /* XXX023 use autobool instead. */ + if (!strcmp(options->UseBridges_, "auto")) { + options->UseBridges = (options->Bridges && + !server_mode(options) && + !options->EntryNodes); + } else if (!strcmp(options->UseBridges_, "0")) { + options->UseBridges = 0; + } else if (!strcmp(options->UseBridges_, "1")) { + options->UseBridges = 1; + } else { + REJECT("UseBridges must be 0, 1, or auto"); + } + if (options->UseBridges && server_mode(options)) REJECT("Servers must be able to freely connect to the rest " @@ -3566,10 +3579,8 @@ options_validate(or_options_t *old_options, or_options_t *options, if (validate_dir_authorities(options, old_options) < 0) REJECT("Directory authority line did not parse. See logs for details."); - if (options->UseBridges && !options->Bridges) - REJECT("If you set UseBridges, you must specify at least one bridge."); if (options->UseBridges && !options->TunnelDirConns) - REJECT("If you set UseBridges, you must set TunnelDirConns."); + REJECT("TunnelDirConns set to 0 only works with UseBridges set to 0"); if (options->Bridges) { for (cl = options->Bridges; cl; cl = cl->next) { if (parse_bridge_line(cl->value, 1)<0) diff --git a/src/or/or.h b/src/or/or.h index 97fecd1500..456dce2be4 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2480,7 +2480,17 @@ typedef struct { * when doing so. */ char *BridgePassword; - int UseBridges; /**< Boolean: should we start all circuits with a bridge? */ + /** Whether we should start all circuits with a bridge. "1" means strictly + * yes, "0" means strictly no, and "auto" means that we do iff any bridges + * are configured, we are not running a server and have not specified a list + * of entry nodes. */ + char *UseBridges_; + /** Effective value of UseBridges. Will be set equally for UseBridges set to + * 1 or 0, but for 'auto' it will be set to 1 iff any bridges are + * configured, we are not running a server and have not specified a list of + * entry nodes. */ + int UseBridges; + config_line_t *Bridges; /**< List of bootstrap bridge addresses. */ int BridgeRelay; /**< Boolean: are we acting as a bridge relay? We make From 40cfad1b5ae90b06eb74861a4fdc1310f8611111 Mon Sep 17 00:00:00 2001 From: anonym Date: Mon, 30 May 2011 23:52:02 +0200 Subject: [PATCH 13/15] Update man page for new UseBridges tristate behaviour. --- doc/tor.1.txt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 1815a8d963..8aa32e82a0 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -708,10 +708,14 @@ The following options are useful only for clients (that is, if from the configured bridge authorities when feasible. It will fall back to a direct request if the authority responds with a 404. (Default: 0) -**UseBridges** **0**|**1**:: - When set, Tor will fetch descriptors for each bridge listed in the "Bridge" +**UseBridges** **0**|**1**|**auto**:: + Make Tor fetch descriptors for each bridge listed in the "Bridge" config lines, and use these relays as both entry guards and directory - guards. (Default: 0) + guards. If the option is 1, bridges must be used and if no bridges are + configured Tor will not make any connections until a bridge is configured; + if it's "auto", Tor will use bridges if any are configured, otherwise it + will connect directly to the Tor network; if it's 0, bridges are not used + at all. (Defaults to auto) **UseEntryGuards** **0**|**1**:: If this option is set to 1, we pick a few long-term entry servers, and try From b79d50dfcc77013329b2fbfd4a84af29a5965b27 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 30 May 2011 23:49:16 -0400 Subject: [PATCH 14/15] Changes file for bug2355. --- changes/bug2355 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changes/bug2355 diff --git a/changes/bug2355 b/changes/bug2355 new file mode 100644 index 0000000000..ee0ae4b96a --- /dev/null +++ b/changes/bug2355 @@ -0,0 +1,8 @@ + o Major features: + - If "UseBridges 1" is set and no bridges are configured, Tor will + now refuse to build any circuits until some bridges are set. + If "UseBridges auto" is set, Tor will use bridges if they are + configured and we are not running as a server, but otherwise + will make circuits as usual. The new default is "auto". Patch + by anonym. + From bbf2fee8ff7bbb8f645b7d973cd84bc97e93ae54 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 2 Jun 2011 12:32:59 -0400 Subject: [PATCH 15/15] Reject 128-byte keys that are not 1024-bit When we added the check for key size, we required that the keys be 128 bytes. But RSA_size (which defers to BN_num_bytes) will return 128 for keys of length 1017..1024. This patch adds a new crypto_pk_num_bits() that returns the actual number of significant bits in the modulus, and uses that to enforce key sizes. Also, credit the original bug3318 in the changes file. --- changes/bug3318 | 6 +++++- src/common/crypto.c | 11 +++++++++++ src/common/crypto.h | 1 + src/or/routerparse.c | 4 ++-- src/test/test_crypto.c | 2 ++ 5 files changed, 21 insertions(+), 3 deletions(-) diff --git a/changes/bug3318 b/changes/bug3318 index 38991c4b1d..8a3c27825f 100644 --- a/changes/bug3318 +++ b/changes/bug3318 @@ -1,3 +1,7 @@ o Minor bugfixes: - Fix a log message that said "bits" while displaying a value in - bytes. Fixes bug 3318; bugfix on 0.2.0.1-alpha. + bytes. Found by wanoskarnet. Fixes bug 3318; bugfix on + 0.2.0.1-alpha. + - When checking for 1024-bit keys, check for 1024 bits, not 128 + bytes. This allows Tor to correctly discard keys of length + 1017 through 1023. Bugfix on 0.0.9pre5. diff --git a/src/common/crypto.c b/src/common/crypto.c index 1ecc24ce23..d8e6619c9f 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -777,6 +777,17 @@ crypto_pk_keysize(crypto_pk_env_t *env) return (size_t) RSA_size(env->key); } +/** Return the size of the public key modulus of env, in bits. */ +int +crypto_pk_num_bits(crypto_pk_env_t *env) +{ + tor_assert(env); + tor_assert(env->key); + tor_assert(env->key->n); + + return BN_num_bits(env->key->n); +} + /** Increase the reference count of env, and return it. */ crypto_pk_env_t * diff --git a/src/common/crypto.h b/src/common/crypto.h index 54c7a67a3b..1a8c81f837 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -119,6 +119,7 @@ int crypto_pk_write_private_key_to_filename(crypto_pk_env_t *env, int crypto_pk_check_key(crypto_pk_env_t *env); int crypto_pk_cmp_keys(crypto_pk_env_t *a, crypto_pk_env_t *b); size_t crypto_pk_keysize(crypto_pk_env_t *env); +int crypto_pk_num_bits(crypto_pk_env_t *env); crypto_pk_env_t *crypto_pk_dup_key(crypto_pk_env_t *orig); crypto_pk_env_t *crypto_pk_copy_full(crypto_pk_env_t *orig); int crypto_pk_key_is_private(const crypto_pk_env_t *key); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 3728e9932b..f855f9d027 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3765,9 +3765,9 @@ token_check_object(memarea_t *area, const char *kwd, break; case NEED_KEY_1024: /* There must be a 1024-bit public key. */ case NEED_SKEY_1024: /* There must be a 1024-bit private key. */ - if (tok->key && crypto_pk_keysize(tok->key) != PK_BYTES) { + if (tok->key && crypto_pk_num_bits(tok->key) != PK_BYTES*8) { tor_snprintf(ebuf, sizeof(ebuf), "Wrong size on key for %s: %d bits", - kwd, (int)crypto_pk_keysize(tok->key)*8); + kwd, crypto_pk_num_bits(tok->key)); RET_ERR(ebuf); } /* fall through */ diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index bf2cc48174..121af279c7 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -343,7 +343,9 @@ test_crypto_pk(void) test_eq(0, crypto_pk_cmp_keys(pk1, pk2)); test_eq(128, crypto_pk_keysize(pk1)); + test_eq(1024, crypto_pk_num_bits(pk1)); test_eq(128, crypto_pk_keysize(pk2)); + test_eq(1024, crypto_pk_num_bits(pk2)); test_eq(128, crypto_pk_public_encrypt(pk2, data1, sizeof(data1), "Hello whirled.", 15,