From 2253697a04c926edb1a5f18727a6cd24528730b1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 13 May 2011 16:18:53 -0400 Subject: [PATCH 1/7] New smartlist function to see if two lists of strings are equal. We'll use this to detect changes in CSV options. --- src/common/container.c | 19 +++++++++++++++++++ src/common/container.h | 2 ++ 2 files changed, 21 insertions(+) diff --git a/src/common/container.c b/src/common/container.c index 09d4bb131f..ca49cbb170 100644 --- a/src/common/container.c +++ b/src/common/container.c @@ -215,6 +215,25 @@ smartlist_string_num_isin(const smartlist_t *sl, int num) return smartlist_string_isin(sl, buf); } +/** Return true iff the two lists contain the same strings in the same + * order, or if they are both NULL. */ +int +smartlist_strings_eq(const smartlist_t *sl1, const smartlist_t *sl2) +{ + if (sl1 == NULL) + return sl2 == NULL; + if (sl2 == NULL) + return 0; + if (smartlist_len(sl1) != smartlist_len(sl2)) + return 0; + SMARTLIST_FOREACH(sl1, const char *, cp1, { + const char *cp2 = smartlist_get(sl2, cp1_sl_idx); + if (strcmp(cp1, cp2)) + return 0; + }); + return 1; +} + /** Return true iff sl has some element E such that * tor_memeq(E,element,DIGEST_LEN) */ diff --git a/src/common/container.h b/src/common/container.h index b39d4ca07e..f5e42de764 100644 --- a/src/common/container.h +++ b/src/common/container.h @@ -42,6 +42,8 @@ int smartlist_string_pos(const smartlist_t *, const char *elt) ATTR_PURE; int smartlist_string_isin_case(const smartlist_t *sl, const char *element) ATTR_PURE; int smartlist_string_num_isin(const smartlist_t *sl, int num) ATTR_PURE; +int smartlist_strings_eq(const smartlist_t *sl1, const smartlist_t *sl2) + ATTR_PURE; int smartlist_digest_isin(const smartlist_t *sl, const char *element) ATTR_PURE; int smartlist_overlap(const smartlist_t *sl1, const smartlist_t *sl2) From 09da83e1e8c1f1050fd65af86736abc1f4e5b530 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 13 May 2011 16:20:01 -0400 Subject: [PATCH 2/7] Don't clear out transient addressmap entries on HUP If you really want to purge the client DNS cache, the TrackHostExits mappings, and the virtual address mappings, you should be using NEWNYM instead. Fixes bug 1345; bugfix on Tor 0.1.0.1-rc. Note that this needs more work: now that we aren't nuking the transient addressmap entries on HUP, we need to make sure that configuration changes to VirtualAddressMap and TrackHostExits actually have a reasonable effect. --- changes/bug1345 | 4 ++++ src/or/main.c | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changes/bug1345 diff --git a/changes/bug1345 b/changes/bug1345 new file mode 100644 index 0000000000..6945b5fe17 --- /dev/null +++ b/changes/bug1345 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - On SIGHUP, do not clear out all TrackExitHost mappings, client DNS + cache entries, and virtual address mappings: that's what NEWNYM is + for. Bugfix on Tor 0.1.0.1-rc; fixes bug 1345. diff --git a/src/or/main.c b/src/or/main.c index 15682d5400..d700f0e7a8 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1417,7 +1417,6 @@ do_hup(void) router_reset_warnings(); routerlist_reset_warnings(); - addressmap_clear_transient(); /* first, reload config variables, in case they've changed */ if (options->ReloadTorrcOnSIGHUP) { /* no need to provide argc/v, they've been cached in init_from_config */ From ec81d17d0c03d73f4d4acd9102893113b2883d76 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 13 May 2011 16:22:10 -0400 Subject: [PATCH 3/7] Raise the TrackHostExits membership code into its own function --- src/or/circuituse.c | 35 ++++++++++++++++++++++------------- src/or/circuituse.h | 2 ++ 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index e68fb4fa82..6e069b1670 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1485,12 +1485,31 @@ link_apconn_to_circ(edge_connection_t *apconn, origin_circuit_t *circ, } } +/** Return true iff address is matched by one of the entries in + * TrackHostExits. */ +int +hostname_in_track_host_exits(or_options_t *options, const char *address) +{ + if (!options->TrackHostExits) + return 0; + SMARTLIST_FOREACH_BEGIN(options->TrackHostExits, const char *, cp) { + if (cp[0] == '.') { /* match end */ + if (cp[1] == '\0' || + !strcasecmpend(address, cp) || + !strcasecmp(address, &cp[1])) + return 1; + } else if (strcasecmp(cp, address) == 0) { + return 1; + } + } SMARTLIST_FOREACH_END(cp); + return 0; +} + /** If an exit wasn't specifically chosen, save the history for future * use. */ static void consider_recording_trackhost(edge_connection_t *conn, origin_circuit_t *circ) { - int found_needle = 0; or_options_t *options = get_options(); size_t len; char *new_address; @@ -1503,18 +1522,8 @@ consider_recording_trackhost(edge_connection_t *conn, origin_circuit_t *circ) options->TrackHostExitsExpire)) return; /* nothing to track, or already mapped */ - SMARTLIST_FOREACH(options->TrackHostExits, const char *, cp, { - if (cp[0] == '.') { /* match end */ - if (cp[1] == '\0' || - !strcasecmpend(conn->socks_request->address, cp) || - !strcasecmp(conn->socks_request->address, &cp[1])) - found_needle = 1; - } else if (strcasecmp(cp, conn->socks_request->address) == 0) { - found_needle = 1; - } - }); - - if (!found_needle || !circ->build_state->chosen_exit) + if (!hostname_in_track_host_exits(options, conn->socks_request->address) || + !circ->build_state->chosen_exit) return; /* write down the fingerprint of the chosen exit, not the nickname, diff --git a/src/or/circuituse.h b/src/or/circuituse.h index 9f393ab378..bfeaea20dc 100644 --- a/src/or/circuituse.h +++ b/src/or/circuituse.h @@ -51,5 +51,7 @@ int connection_ap_handshake_attach_chosen_circuit(edge_connection_t *conn, crypt_path_t *cpath); int connection_ap_handshake_attach_circuit(edge_connection_t *conn); +int hostname_in_track_host_exits(or_options_t *options, const char *address); + #endif From a3ae591115ba5c4a43ff4fa3839be274aac9e5c3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 13 May 2011 16:22:58 -0400 Subject: [PATCH 4/7] When TrackExitHosts changes, remove all no-longer-valid mappings This bug couldn't happen when TrackExitHosts changed in torrc, since the SIGHUP to reload the torrc would clear out all the transient addressmap entries before. But if you used SETCONF to change TrackExitHosts, old entries would be left alone: that's a bug, and so this is a bugfix on Tor 0.1.0.1-rc. --- changes/bug1345 | 5 ++++- src/or/config.c | 10 +++++++++- src/or/connection_edge.c | 6 ++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/changes/bug1345 b/changes/bug1345 index 6945b5fe17..b35e78f5e8 100644 --- a/changes/bug1345 +++ b/changes/bug1345 @@ -1,4 +1,7 @@ o Minor bugfixes: - - On SIGHUP, do not clear out all TrackExitHost mappings, client DNS + - On SIGHUP, do not clear out all TrackHostExits mappings, client DNS cache entries, and virtual address mappings: that's what NEWNYM is for. Bugfix on Tor 0.1.0.1-rc; fixes bug 1345. + - When TrackHostExits is changed from a controller, remove any + mappings for hosts that should no longer have their exits tracked. + Bugfix on Tor 0.1.0.1-rc. diff --git a/src/or/config.c b/src/or/config.c index a7ff28f462..147cc66b6b 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1264,6 +1264,7 @@ options_act(or_options_t *old_options) /* Check for transitions that need action. */ if (old_options) { + int revise_trackexithosts = 0; if ((options->UseEntryGuards && !old_options->UseEntryGuards) || !routerset_equal(old_options->ExcludeNodes,options->ExcludeNodes) || !routerset_equal(old_options->ExcludeExitNodes, @@ -1276,9 +1277,16 @@ options_act(or_options_t *old_options) "excluded node lists. Abandoning previous circuits."); circuit_mark_all_unused_circs(); circuit_expire_all_dirty_circs(); - addressmap_clear_excluded_trackexithosts(options); + revise_trackexithosts = 1; } + if (!smartlist_strings_eq(old_options->TrackHostExits, + options->TrackHostExits)) + revise_trackexithosts = 1; + + if (revise_trackexithosts) + addressmap_clear_excluded_trackexithosts(options); + /* How long should we delay counting bridge stats after becoming a bridge? * We use this so we don't count people who used our bridge thinking it is * a relay. If you change this, don't forget to change the log message diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 037920b688..5301471e91 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -810,7 +810,8 @@ clear_trackexithost_mappings(const char *exitname) } /** Remove all TRACKEXIT mappings from the addressmap for which the target - * host is unknown or no longer allowed. */ + * host is unknown or no longer allowed, or for which the source address + * is no longer in trackexithosts. */ void addressmap_clear_excluded_trackexithosts(or_options_t *options) { @@ -851,7 +852,8 @@ addressmap_clear_excluded_trackexithosts(or_options_t *options) tor_free(nodename); if (!ri || (allow_nodes && !routerset_contains_router(allow_nodes, ri)) || - routerset_contains_router(exclude_nodes, ri)) { + routerset_contains_router(exclude_nodes, ri) || + !hostname_in_track_host_exits(options, address)) { /* We don't know this one, or we want to be rid of it. */ addressmap_ent_remove(address, ent); MAP_DEL_CURRENT(address); From da8297dbcb6a44d2291878b01779500640e4d0b1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 13 May 2011 16:59:31 -0400 Subject: [PATCH 5/7] Handle transitions in Automap*, VirtualAddrNetwork correctly Previously, if they changed in torrc during a SIGHUP, all was well, since we would just clear all transient entries from the addrmap thanks to bug 1345. But if you changed them from the controller, Tor would leave old mappings in place. The VirtualAddrNetwork bug has been here since 0.1.1.19-rc; the AutomapHosts* bug has been here since 0.2.0.1-alpha. --- changes/bug1345 | 6 ++++++ src/or/config.c | 15 ++++++++++++++ src/or/connection_edge.c | 45 +++++++++++++++++++++++++++++++++++++++- src/or/connection_edge.h | 1 + src/or/or.h | 3 +++ 5 files changed, 69 insertions(+), 1 deletion(-) diff --git a/changes/bug1345 b/changes/bug1345 index b35e78f5e8..0c9375a35d 100644 --- a/changes/bug1345 +++ b/changes/bug1345 @@ -5,3 +5,9 @@ - When TrackHostExits is changed from a controller, remove any mappings for hosts that should no longer have their exits tracked. Bugfix on Tor 0.1.0.1-rc. + - When VirtualAddrNetwork option is changed from a controller, + remove any mappings for hosts that were automapped to + that network. Bugfix on 0.1.1.19-rc. + - When one of the AutomapHosts* options is changed from a + controller, remove any mappings for hosts that should no longer be + automapped. Bugfix on 0.2.0.1-alpha. diff --git a/src/or/config.c b/src/or/config.c index 147cc66b6b..9c68b6fa59 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1265,6 +1265,7 @@ options_act(or_options_t *old_options) /* Check for transitions that need action. */ if (old_options) { int revise_trackexithosts = 0; + int revise_automap_entries = 0; if ((options->UseEntryGuards && !old_options->UseEntryGuards) || !routerset_equal(old_options->ExcludeNodes,options->ExcludeNodes) || !routerset_equal(old_options->ExcludeExitNodes, @@ -1287,6 +1288,20 @@ options_act(or_options_t *old_options) if (revise_trackexithosts) addressmap_clear_excluded_trackexithosts(options); + if (old_options->AutomapHostsOnResolve && !options->AutomapHostsOnResolve) { + revise_automap_entries = 1; + } else if (options->AutomapHostsOnResolve) { + if (!smartlist_strings_eq(old_options->AutomapHostsSuffixes, + options->AutomapHostsSuffixes)) + revise_automap_entries = 1; + else if (!opt_streq(old_options->VirtualAddrNetwork, + options->VirtualAddrNetwork)) + revise_automap_entries = 1; + } + + if (revise_automap_entries) + addressmap_clear_invalid_automaps(options); + /* How long should we delay counting bridge stats after becoming a bridge? * We use this so we don't count people who used our bridge thinking it is * a relay. If you change this, don't forget to change the log message diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 5301471e91..7828f16386 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -861,6 +861,49 @@ addressmap_clear_excluded_trackexithosts(or_options_t *options) } STRMAP_FOREACH_END; } +/** Remove all AUTOMAP mappings from the addressmap for which the + * source address no longer matches AutomapHostsSuffixes, which is + * no longer allowed by AutomapHostsOnResolve, or for which the + * target address is no longer in the virtual network. */ +void +addressmap_clear_invalid_automaps(or_options_t *options) +{ + int clear_all = !options->AutomapHostsOnResolve; + const smartlist_t *suffixes = options->AutomapHostsSuffixes; + + if (!addressmap) + return; + + if (!suffixes) + clear_all = 1; /* This should be impossible, but let's be sure. */ + + STRMAP_FOREACH_MODIFY(addressmap, src_address, addressmap_entry_t *, ent) { + int remove = clear_all; + if (ent->source != ADDRMAPSRC_AUTOMAP) + continue; /* not an automap mapping. */ + + if (!remove) { + int suffix_found = 0; + SMARTLIST_FOREACH(suffixes, const char *, suffix, { + if (!strcasecmpend(src_address, suffix)) { + suffix_found = 1; + break; + } + }); + if (!suffix_found) + remove = 1; + } + + if (!remove && ! address_is_in_virtual_range(ent->new_address)) + remove = 1; + + if (remove) { + addressmap_ent_remove(src_address, ent); + MAP_DEL_CURRENT(src_address); + } + } STRMAP_FOREACH_END; +} + /** Remove all entries from the addressmap that were set via the * configuration file or the command line. */ void @@ -1372,7 +1415,7 @@ addressmap_register_virtual_address(int type, char *new_address) log_info(LD_APP, "Registering map from %s to %s", *addrp, new_address); if (vent_needs_to_be_added) strmap_set(virtaddress_reversemap, new_address, vent); - addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_CONTROLLER); + addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_AUTOMAP); #if 0 { diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 70d0dd2713..8ba2fafd08 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -62,6 +62,7 @@ int address_is_invalid_destination(const char *address, int client); void addressmap_init(void); void addressmap_clear_excluded_trackexithosts(or_options_t *options); +void addressmap_clear_invalid_automaps(or_options_t *options); void addressmap_clean(time_t now); void addressmap_clear_configured(void); void addressmap_clear_transient(void); diff --git a/src/or/or.h b/src/or/or.h index a73d98ab74..5647691550 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3150,6 +3150,9 @@ typedef enum setopt_err_t { typedef enum { /** We're remapping this address because the controller told us to. */ ADDRMAPSRC_CONTROLLER, + /** We're remapping this address because of an AutomapHostsOnResolve + * configuration. */ + ADDRMAPSRC_AUTOMAP, /** We're remapping this address because our configuration (via torrc, the * command line, or a SETCONF command) told us to. */ ADDRMAPSRC_TORRC, From 2bb6bdc3f9354a57b3daf0dbb494123cf39ba29f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 15 May 2011 11:37:33 -0400 Subject: [PATCH 6/7] Better doc for consider_recording_trackexithost --- src/or/circuituse.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 6e069b1670..86db7c3a02 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1505,8 +1505,10 @@ hostname_in_track_host_exits(or_options_t *options, const char *address) return 0; } -/** If an exit wasn't specifically chosen, save the history for future - * use. */ +/** If an exit wasn't explicitly specified for conn, consider saving + * the exit that we *did* choose for use by future connections to + * conn's destination. + */ static void consider_recording_trackhost(edge_connection_t *conn, origin_circuit_t *circ) { From f2871009346e0589455be14e9cef930c19082c0a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 15 May 2011 11:41:49 -0400 Subject: [PATCH 7/7] Replace a nasty add-malloc-snprintf with a nice clean asprintf --- src/or/circuituse.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 86db7c3a02..7289aa5c11 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1513,8 +1513,7 @@ static void consider_recording_trackhost(edge_connection_t *conn, origin_circuit_t *circ) { or_options_t *options = get_options(); - size_t len; - char *new_address; + char *new_address = NULL; char fp[HEX_DIGEST_LEN+1]; /* Search the addressmap for this conn's destination. */ @@ -1534,12 +1533,7 @@ consider_recording_trackhost(edge_connection_t *conn, origin_circuit_t *circ) circ->build_state->chosen_exit->identity_digest, DIGEST_LEN); /* Add this exit/hostname pair to the addressmap. */ - len = strlen(conn->socks_request->address) + 1 /* '.' */ + - strlen(fp) + 1 /* '.' */ + - strlen("exit") + 1 /* '\0' */; - new_address = tor_malloc(len); - - tor_snprintf(new_address, len, "%s.%s.exit", + tor_asprintf(&new_address, "%s.%s.exit", conn->socks_request->address, fp); addressmap_register(conn->socks_request->address, new_address,