[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: Patch: Adding country codes to *Nodes config options
On Monday 22 September 2008 21:02:41 Nick Mathewson wrote:
> {Please take pity on us old-school purists and wrap email to under 75
> colums, so that it doesn't overflow our terminals when we reply}
>
> Hi, Robert! This patch is a good start. I can clean it up if you
> would like, or you can do another version; let me know what you'd
> prefer.
>
OK, I think I've got my wrapping and most of the patch sorted. I've added XXXX
comments wherever I'm unsure of something. Hopefully won't require too much
cleaning up!
Index: src/or/config.c
===================================================================
--- src/or/config.c (revision 16958)
+++ src/or/config.c (working copy)
@@ -194,11 +194,11 @@
V(DNSListenAddress, LINELIST, NULL),
V(DownloadExtraInfo, BOOL, "0"),
V(EnforceDistinctSubnets, BOOL, "1"),
- V(EntryNodes, STRING, NULL),
+ V(EntryNodes, ROUTERSET, NULL),
V(TestingEstimatedDescriptorPropagationTime, INTERVAL, "10 minutes"),
V(ExcludeNodes, ROUTERSET, NULL),
V(ExcludeExitNodes, ROUTERSET, NULL),
- V(ExitNodes, STRING, NULL),
+ V(ExitNodes, ROUTERSET, NULL),
V(ExitPolicy, LINELIST, NULL),
V(ExitPolicyRejectPrivate, BOOL, "1"),
V(FallbackNetworkstatusFile, FILENAME,
@@ -817,13 +817,22 @@
return _version;
}
+/** Release additional memory allocated in options
+ */
+static void
+or_options_free(void)
+{
+ routerset_free(global_options->_ExcludeExitNodesUnion);
+ config_free(&options_format, global_options);
+}
+
/** Release all memory and resources held by global configuration structures.
*/
void
config_free_all(void)
{
if (global_options) {
- config_free(&options_format, global_options);
+ or_options_free();
global_options = NULL;
}
if (global_state) {
@@ -1336,11 +1345,21 @@
#endif
geoip_load_file(actual_fname, options);
tor_free(actual_fname);
+
+ /* XXXX Would iterating through all option_var's routersets be better? */
+ if (options->EntryNodes)
+ routerset_refresh_countries(options->EntryNodes);
+ if (options->ExitNodes)
+ routerset_refresh_countries(options->ExitNodes);
+ if (options->ExcludeNodes)
+ routerset_refresh_countries(options->ExcludeNodes);
+ if (options->ExcludeExitNodes)
+ routerset_refresh_countries(options->ExcludeExitNodes);
}
/* Check if we need to parse and add the EntryNodes config option. */
if (options->EntryNodes &&
(!old_options ||
- !opt_streq(old_options->EntryNodes, options->EntryNodes)))
+ (!routerset_equal(old_options->EntryNodes,options->EntryNodes))))
entry_nodes_should_be_added();
/* Since our options changed, we might need to regenerate and upload our
@@ -1701,7 +1720,6 @@
case CONFIG_TYPE_LINELIST_S:
config_line_append((config_line_t**)lvalue, c->key, c->value);
break;
-
case CONFIG_TYPE_OBSOLETE:
log_warn(LD_CONFIG, "Skipping obsolete configuration option '%s'",
c->key);
break;
@@ -2964,17 +2982,17 @@
}
if (options->StrictExitNodes &&
- (!options->ExitNodes || !strlen(options->ExitNodes)) &&
+ (!options->ExitNodes) &&
(!old_options ||
(old_options->StrictExitNodes != options->StrictExitNodes) ||
- (!opt_streq(old_options->ExitNodes, options->ExitNodes))))
+ (!routerset_equal(old_options->ExitNodes,options->ExitNodes))))
COMPLAIN("StrictExitNodes set, but no ExitNodes listed.");
if (options->StrictEntryNodes &&
- (!options->EntryNodes || !strlen(options->EntryNodes)) &&
+ (!options->EntryNodes) &&
(!old_options ||
(old_options->StrictEntryNodes != options->StrictEntryNodes) ||
- (!opt_streq(old_options->EntryNodes, options->EntryNodes))))
+ (!routerset_equal(old_options->EntryNodes,options->EntryNodes))))
COMPLAIN("StrictEntryNodes set, but no EntryNodes listed.");
if (options->AuthoritativeDir) {
@@ -3334,10 +3352,6 @@
if (options->UseEntryGuards && ! options->NumEntryGuards)
REJECT("Cannot enable UseEntryGuards with NumEntryGuards set to 0");
- if (check_nickname_list(options->ExitNodes, "ExitNodes", msg))
- return -1;
- if (check_nickname_list(options->EntryNodes, "EntryNodes", msg))
- return -1;
if (check_nickname_list(options->MyFamily, "MyFamily", msg))
return -1;
for (cl = options->NodeFamilies; cl; cl = cl->next) {
Index: src/or/routerlist.c
===================================================================
--- src/or/routerlist.c (revision 16958)
+++ src/or/routerlist.c (working copy)
@@ -4707,6 +4707,7 @@
/** An address policy for routers in the set. For implementation reasons,
* a router belongs to the set if it is _rejected_ by this policy. */
smartlist_t *policies;
+ bitarray_t *countries;
};
/** Return a new empty routerset. */
@@ -4718,9 +4719,62 @@
result->names = strmap_new();
result->digests = digestmap_new();
result->policies = smartlist_create();
+ result->countries = bitarray_init_zero(geoip_get_n_countries());
return result;
}
+/** Add the GeoIP database's integer index (+1) of a valid two-character
+ * country code to the routerset's <b>countries</b> bitarray. Return the
+ * integer index if the country code is valid, zero otherwise.*/
+static int
+routerset_add_country(const char *c)
+{
+ char *country;
+ int cc;
+
+ /* XXXX: Country codes must be of the form \{[a-z\?]{2}\} but this accepts
+ \{[.]{2}\}. Do we need to be strict? */
+ if (!(*c=='{') || !(*(c+3)=='}'))
+ return 0;
+
+ if (!geoip_is_loaded()) {
+ log(LOG_WARN, LD_CONFIG, "GeoIP Database Not Loaded: Cannot add country"
+ "entry %s, ignoring.", c);
+ return 0;
+ }
+
+ country=tor_strdup(c);
+ tor_strstrip(country,"}");
+ tor_strstrip(country,"{");
+ tor_strlower(country);
+
+ if ((cc=geoip_is_valid_country(country))==-1) {
+ log(LOG_WARN, LD_CONFIG, "Country Code '%s' is not valid, ignoring.",
+ country);
+ }
+ tor_free(country);
+ return cc;
+}
+
+/** Update the routerset's <b>countries</b> bitarray_t. Called whenever
+ * the GeoIP database is reloaded.
+ */
+void
+routerset_refresh_countries(routerset_t *target)
+{
+ int cc;
+ /* XXX: Is this the proper check for a bitarray_t that is empty? */
+ if (!target->countries)
+ return;
+ bitarray_free(target->countries);
+ target->countries=bitarray_init_zero(geoip_get_n_countries());
+ SMARTLIST_FOREACH(target->list, char *, nick, {
+ if ((cc=routerset_add_country(nick))!=-1) {
+ bitarray_set(target->countries,cc);
+ }
+ });
+}
+
/** Parse the string <b>s</b> to create a set of routerset entries, and add
* them to <b>target</b>. In log messages, refer to the string as
* <b>description</b>. Return 0 on success, -1 on failure.
@@ -4732,7 +4786,7 @@
int
routerset_parse(routerset_t *target, const char *s, const char *description)
{
- int r = 0;
+ int r = 0,cc=0;
smartlist_t *list = smartlist_create();
smartlist_split_string(list, s, ",",
SPLIT_SKIP_SPACE | SPLIT_IGNORE_BLANK, 0);
@@ -4748,6 +4802,10 @@
} else if (is_legal_nickname(nick)) {
log_debug(LD_CONFIG, "Adding nickname %s to %s", nick, description);
strmap_set_lc(target->names, nick, (void*)1);
+ } else if ((cc=routerset_add_country(nick))!=-1) {
+ log_debug(LD_CONFIG, "Adding country %s to %s", nick,
+ description);
+ bitarray_set(target->countries,cc);
} else if ((strchr(nick,'.') || strchr(nick, '*')) &&
(p = router_parse_addr_policy_item_from_string(
nick, ADDR_POLICY_REJECT))) {
@@ -4795,6 +4853,10 @@
if (addr && compare_tor_addr_to_addr_policy(addr, orport, set->policies)
== ADDR_POLICY_REJECTED)
return 1;
+ if (addr &&
+ bitarray_is_set(set->countries,
+ geoip_get_country_by_ip(tor_addr_to_ipv4h(addr))))
+ return 1;
return 0;
}
@@ -4856,14 +4918,16 @@
smartlist_add(out, router);
}
});
- if (smartlist_len(routerset->policies)) {
+ if (smartlist_len(routerset->policies) || (routerset->countries)) {
routerlist_t *rl = router_get_routerlist();
SMARTLIST_FOREACH(rl->routers, routerinfo_t *, router,
- if (compare_addr_to_addr_policy(router->addr, router->or_port,
- routerset->policies) == ADDR_POLICY_REJECT) {
+ if ((compare_addr_to_addr_policy(router->addr, router->or_port,
+ routerset->policies) == ADDR_POLICY_REJECT) ||
+ (bitarray_is_set(routerset->countries,
+ geoip_get_country_by_ip(router->addr)))) {
if (!running_only || router->is_running)
smartlist_add(out, router);
- });
+ });
}
}
@@ -4892,6 +4956,29 @@
return smartlist_join_strings(set->list, ",", 0, NULL);
}
+/** Helper: return true iff old and new are both NULL, or both non-NULL
+ * equal routersets. */
+int
+routerset_equal(const routerset_t *old, const routerset_t *new)
+{
+ /* XXXX: This won't work if the names/digests are identical but in a
+ different order. Checking for exact equality would be heavy going,
+ is it worth it? */
+ if (sizeof(old->names) != sizeof(new->names))
+ return 0;
+ if (memcmp(old->names,new->names,sizeof(new->names)))
+ return 0;
+ if (sizeof(old->digests) != sizeof(new->digests))
+ return 0;
+ if (memcmp(old->digests,new->digests,sizeof(new->digests)))
+ return 0;
+ if (sizeof(old->countries) != sizeof(new->countries))
+ return 0;
+ if (memcmp(old->countries,new->countries,sizeof(new->countries)))
+ return 0;
+ return 1;
+}
+
/** Free all storage held in <b>routerset</b>. */
void
routerset_free(routerset_t *routerset)
@@ -4904,7 +4991,7 @@
strmap_free(routerset->names, NULL);
digestmap_free(routerset->digests, NULL);
-
+ bitarray_free(routerset->countries);
tor_free(routerset);
}
Index: src/or/or.h
===================================================================
--- src/or/or.h (revision 16958)
+++ src/or/or.h (working copy)
@@ -1394,7 +1394,8 @@
time_t last_reachable;
/** When did we start testing reachability for this OR? */
time_t testing_since;
-
+ /** According to the geoip db what country is this router in? */
+ int country;
} routerinfo_t;
/** Information needed to keep and cache a signed extra-info document. */
@@ -2090,17 +2091,25 @@
char *Address; /**< OR only: configured address for this onion router. */
char *PidFile; /**< Where to store PID of Tor process. */
- char *ExitNodes; /**< Comma-separated list of nicknames of ORs to consider
- * as exits. */
- char *EntryNodes; /**< Comma-separated list of nicknames of ORs to consider
- * as entry points. */
+ struct routerset_t *ExitNodes; /**< Structure containing nicknames,
+ * digests, country codes and IP address
+ * patterns of ORs to consider as exits. */
+ struct routerset_t *EntryNodes;/**< Structure containing nicknames,
+ * digests, country codes and IP address
+ * patterns of ORs to consider as entry
+ * points. */
int StrictExitNodes; /**< Boolean: When none of our ExitNodes are up, do we
* stop building circuits? */
int StrictEntryNodes; /**< Boolean: When none of our EntryNodes are up, do
we
* stop building circuits? */
- struct routerset_t *ExcludeNodes; /**< Comma-separated list of nicknames of
- * ORs not to use in circuits. */
- struct routerset_t *ExcludeExitNodes; /**<DODOC */
+ struct routerset_t *ExcludeNodes;/**< Structure containing nicknames,
+ * digests, country codes and IP address
+ * patterns of ORs not to use in
+ * circuits. */
+ struct routerset_t *ExcludeExitNodes;/**< Structure containing nicknames,
+ * digests, country codes and IP address
+ * patterns of ORs not to consider as exits.
+ */
/** Union of ExcludeNodes and ExcludeExitNodes */
struct routerset_t *_ExcludeExitNodesUnion;
@@ -3468,6 +3477,7 @@
int geoip_get_n_countries(void);
const char *geoip_get_country_name(int num);
int geoip_is_loaded(void);
+int geoip_is_valid_country(const char *country);
/** Indicates an action that we might be noting geoip statistics on.
* Note that if we're noticing CONNECT, we're a bridge, and if we're noticing
* the others, we're not.
@@ -4292,6 +4302,8 @@
void routerset_subtract_routers(smartlist_t *out,
const routerset_t *routerset);
char *routerset_to_string(const routerset_t *routerset);
+void routerset_refresh_countries(routerset_t *target);
+int routerset_equal(const routerset_t *old, const routerset_t *new);
void routerset_free(routerset_t *routerset);
int hid_serv_get_responsible_directories(smartlist_t *responsible_dirs,
Index: src/or/geoip.c
===================================================================
--- src/or/geoip.c (revision 16958)
+++ src/or/geoip.c (working copy)
@@ -42,6 +42,23 @@
/** A list of all known geoip_entry_t, sorted by ip_low. */
static smartlist_t *geoip_entries = NULL;
+/** Return the index of the <b>country</b>'s entry in the GeoIP DB
+ * if it is a valid 2-letter country code, otherwise return zero.
+ */
+int
+geoip_is_valid_country(const char *country)
+{
+ void *_idxplus1;
+ intptr_t idx;
+
+ _idxplus1 = strmap_get_lc(country_idxplus1_by_lc_code, country);
+ if (!_idxplus1)
+ return -1;
+
+ idx = ((uintptr_t)_idxplus1)-1;
+ return (int)idx;
+}
+
/** Add an entry to the GeoIP table, mapping all IPs between <b>low</b> and
* <b>high</b>, inclusive, to the 2-letter country code <b>country</b>.
*/
Index: src/or/circuitbuild.c
===================================================================
--- src/or/circuitbuild.c (revision 16958)
+++ src/or/circuitbuild.c (working copy)
@@ -1278,7 +1278,7 @@
n_pending_connections);
preferredexits = smartlist_create();
- add_nickname_list_to_smartlist(preferredexits,options->ExitNodes,1);
+ routerset_get_all_routers(preferredexits,options->ExitNodes,1);
sl = smartlist_create();
@@ -1399,6 +1399,24 @@
return NULL;
}
+/** Log a warning if the user specified an exit for the circuit that
+ * has been excluded from use by ExcludeNodes or ExcludeExitNodes. */
+static void
+warn_if_router_excluded(const extend_info_t *exit)
+{
+ or_options_t *options = get_options();
+ routerinfo_t *ri = router_get_by_digest(exit->identity_digest);
+
+ if (!ri || !options->_ExcludeExitNodesUnion)
+ return;
+
+ if (routerset_contains_router(options->_ExcludeExitNodesUnion, ri))
+ log_warn(LD_CIRC,"Requested exit node '%s' is in ExcludeNodes, "
+ "or ExcludeExitNodes, using anyway.",exit->nickname);
+
+ return;
+}
+
/** Decide a suitable length for circ's cpath, and pick an exit
* router (or use <b>exit</b> if provided). Store these in the
* cpath. Return 0 if ok, -1 if circuit should be closed. */
@@ -1419,6 +1437,7 @@
}
if (exit) { /* the circuit-builder pre-requested one */
+ warn_if_router_excluded(exit);
log_info(LD_CIRC,"Using requested exit node '%s'", exit->nickname);
exit = extend_info_dup(exit);
} else { /* we have to decide one */
@@ -1832,7 +1851,7 @@
else if (options->UseBridges && ri->purpose != ROUTER_PURPOSE_BRIDGE)
*reason = "not a bridge";
else if (!options->UseBridges && !ri->is_possible_guard &&
- !router_nickname_is_in_list(ri, options->EntryNodes))
+ !routerset_contains_router(options->EntryNodes,ri))
*reason = "not recommended as a guard";
else if (routerset_contains_router(options->ExcludeNodes, ri))
*reason = "excluded";
@@ -1856,7 +1875,6 @@
control_event_guard(e->nickname, e->identity, "GOOD");
changed = 1;
}
-
return changed;
}
@@ -2346,8 +2364,9 @@
return;
}
- log_info(LD_CIRC,"Adding configured EntryNodes '%s'.",
- options->EntryNodes);
+ if (options->EntryNodes)
+ log_info(LD_CIRC,"Adding configured EntryNodes '%s'.",
+ routerset_to_string(options->EntryNodes));
entry_routers = smartlist_create();
entry_fps = smartlist_create();
@@ -2355,7 +2374,7 @@
old_entry_guards_not_on_list = smartlist_create();
/* Split entry guards into those on the list and those not. */
- add_nickname_list_to_smartlist(entry_routers, options->EntryNodes, 0);
+ routerset_get_all_routers(entry_routers, options->EntryNodes, 0);
SMARTLIST_FOREACH(entry_routers, routerinfo_t *, ri,
smartlist_add(entry_fps,ri->cache_info.identity_digest));
SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, {
Index: contrib/checkSpace.pl
===================================================================
--- contrib/checkSpace.pl (revision 16958)
+++ contrib/checkSpace.pl (working copy)
@@ -73,7 +73,7 @@
s!//.*!!;
}
## Warn about braces preceded by non-space.
- if (/([^\s])\{/) {
+ if (/([^\s'])\{/) {
print " $1\{:$fn:$.\n";
}
## Warn about multiple internal spaces.
Index: doc/tor.1.in
===================================================================
--- doc/tor.1.in (revision 16958)
+++ doc/tor.1.in (working copy)
@@ -422,28 +422,28 @@
.LP
.TP
\fBExcludeNodes \fR\fInode\fR,\fInode\fR,\fI...\fP
-A list of identity fingerprints, nicknames, and address patterns of
-nodes to never use when building a circuit. (Example: ExcludeNodes
-SlowServer, $ABCDEFFFFFFFFFFFFFFF, 255.254.0.0/8)
+A list of identity fingerprints, nicknames, country codes and address patterns
+of nodes to never use when building a circuit. (Example: ExcludeNodes
+SlowServer, $ABCDEFFFFFFFFFFFFFFF, {cc}, 255.254.0.0/8)
.LP
.TP
\fBExcludeExitNodes \fR\fInode\fR,\fInode\fR,\fI...\fP
-A list of identity fingerprints, nicknames, and address patterns of
-nodes to never use when picking an exit node. Note that any node
+A list of identity fingerprints, nicknames, country codes and address patterns
+of nodes to never use when picking an exit node. Note that any node
listed in ExcludeNodes is automatically considered to be part of this
list.
.LP
.TP
\fBEntryNodes \fR\fInode\fR,\fInode\fR,\fI...\fP
-A list of identity fingerprints or nicknames of preferred nodes to use for the
-first hop in the circuit.
+A list of identity fingerprints, nicknames, country codes and address patterns
+of nodes to use for the first hop in the circuit.
These are treated only as preferences unless StrictEntryNodes (see
below) is also set.
.LP
.TP
\fBExitNodes \fR\fInode\fR,\fInode\fR,\fI...\fP
-A list of identity fingerprints or nicknames of preferred nodes to use for the
-last hop in the circuit.
+A list of identity fingerprints, nicknames, country codes and address patterns
+of nodes to use for the last hop in the circuit.
These are treated only as preferences unless StrictExitNodes (see
below) is also set.
.LP