aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/bug16069-exit-policy-rule616
-rw-r--r--src/common/address.c4
-rw-r--r--src/or/policies.c19
-rw-r--r--src/or/routerparse.c51
-rw-r--r--src/or/routerparse.h2
-rw-r--r--src/or/routerset.c17
-rw-r--r--src/test/test_policy.c130
-rw-r--r--src/test/test_routerset.c7
8 files changed, 212 insertions, 34 deletions
diff --git a/changes/bug16069-exit-policy-rule6 b/changes/bug16069-exit-policy-rule6
new file mode 100644
index 0000000000..5b0560a2cf
--- /dev/null
+++ b/changes/bug16069-exit-policy-rule6
@@ -0,0 +1,16 @@
+ o Minor bug fixes (torrc exit policies):
+ - When parsing torrc ExitPolicies, we now warn if:
+ * an IPv4 address is used on an accept6 or reject6 line. The line is
+ ignored, but the rest of the policy items in the list are used.
+ (accept/reject continue to allow both IPv4 and IPv6 addresses in
+ torrcs.)
+ * a "private" address alias is used on an accept6 or reject6 line.
+ The line filters both IPv4 and IPv6 private addresses, disregarding
+ the 6 in accept6/reject6.
+ - When parsing torrc ExitPolicies, we now issue an info-level message:
+ * when expanding an accept/reject * line to include both IPv4 and IPv6
+ wildcard addresses.
+ - In each instance, usage advice is provided to avoid the message.
+ Partial fix for ticket 16069. Patch by "teor".
+ Patch on 2eb7eafc9d78 and a96c0affcb4c (25 Oct 2012),
+ released in 0.2.4.7-alpha.
diff --git a/src/common/address.c b/src/common/address.c
index dd336257ef..597f6990d3 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -690,6 +690,10 @@ tor_addr_parse_mask_ports(const char *s,
if (flags & TAPMP_EXTENDED_STAR) {
family = AF_UNSPEC;
tor_addr_make_unspec(addr_out);
+ log_info(LD_GENERAL,
+ "'%s' expands into rules which apply to all IPv4 and IPv6 "
+ "addresses. (Use accept/reject *4:* for IPv4 or "
+ "accept[6]/reject[6] *6:* for IPv6.)", s);
} else {
family = AF_INET;
tor_addr_from_ipv4h(addr_out, 0);
diff --git a/src/or/policies.c b/src/or/policies.c
index 07a3e18597..e1d7da1f5e 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -167,6 +167,7 @@ parse_addr_policy(config_line_t *cfg, smartlist_t **dest,
smartlist_t *result;
smartlist_t *entries;
addr_policy_t *item;
+ int malformed_list;
int r = 0;
if (!cfg)
@@ -179,12 +180,22 @@ parse_addr_policy(config_line_t *cfg, smartlist_t **dest,
SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0);
SMARTLIST_FOREACH_BEGIN(entries, const char *, ent) {
log_debug(LD_CONFIG,"Adding new entry '%s'",ent);
- item = router_parse_addr_policy_item_from_string(ent, assume_action);
+ malformed_list = 0;
+ item = router_parse_addr_policy_item_from_string(ent, assume_action,
+ &malformed_list);
if (item) {
smartlist_add(result, item);
- } else {
- log_warn(LD_CONFIG,"Malformed policy '%s'.", ent);
+ } else if (malformed_list) {
+ /* the error is so severe the entire list should be discarded */
+ log_warn(LD_CONFIG, "Malformed policy '%s'. Discarding entire policy "
+ "list.", ent);
r = -1;
+ } else {
+ /* the error is minor: don't add the item, but keep processing the
+ * rest of the policies in the list */
+ log_debug(LD_CONFIG, "Ignored policy '%s' due to non-fatal error. "
+ "The remainder of the policy list will be used.",
+ ent);
}
} SMARTLIST_FOREACH_END(ent);
SMARTLIST_FOREACH(entries, char *, ent, tor_free(ent));
@@ -568,6 +579,8 @@ cmp_single_addr_policy(addr_policy_t *a, addr_policy_t *b)
return r;
if ((r=((int)a->is_private - (int)b->is_private)))
return r;
+ /* refcnt and is_canonical are irrelevant to equality,
+ * they are hash table implementation details */
if ((r=tor_addr_compare(&a->addr, &b->addr, CMP_EXACT)))
return r;
if ((r=((int)a->maskbits - (int)b->maskbits)))
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index d0b2cba19f..2f7e50e60a 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -3666,12 +3666,20 @@ networkstatus_parse_detached_signatures(const char *s, const char *eos)
* assume_action is nonnegative, then insert its action (ADDR_POLICY_ACCEPT or
* ADDR_POLICY_REJECT) for items that specify no action.
*
+ * Returns NULL on policy errors.
+ *
+ * If there is a policy error, malformed_list is set to true if the entire
+ * policy list should be discarded. Otherwise, it is set to false, and only
+ * this item should be ignored - the rest of the policy list can continue to
+ * be processed and used.
+ *
* The addr_policy_t returned by this function can have its address set to
* AF_UNSPEC for '*'. Use policy_expand_unspec() to turn this into a pair
* of AF_INET and AF_INET6 items.
*/
MOCK_IMPL(addr_policy_t *,
-router_parse_addr_policy_item_from_string,(const char *s, int assume_action))
+router_parse_addr_policy_item_from_string,(const char *s, int assume_action,
+ int *malformed_list))
{
directory_token_t *tok = NULL;
const char *cp, *eos;
@@ -3688,6 +3696,8 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action))
addr_policy_t *r;
memarea_t *area = NULL;
+ tor_assert(malformed_list);
+
s = eat_whitespace(s);
if ((*s == '*' || TOR_ISDIGIT(*s)) && assume_action >= 0) {
if (tor_snprintf(line, sizeof(line), "%s %s",
@@ -3714,9 +3724,32 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action))
goto err;
}
+ /* Use the extended interpretation of accept/reject *,
+ * expanding it into an IPv4 wildcard and an IPv6 wildcard.
+ * Also permit *4 and *6 for IPv4 and IPv6 only wildcards. */
r = router_parse_addr_policy(tok, TAPMP_EXTENDED_STAR);
+ if (!r) {
+ goto err;
+ }
+
+ /* Ensure that accept6/reject6 fields are followed by IPv6 addresses.
+ * AF_UNSPEC addresses are only permitted on the accept/reject field type.
+ * Unlike descriptors, torrcs exit policy accept/reject can be followed by
+ * either an IPv4 or IPv6 address. */
+ if ((tok->tp == K_ACCEPT6 || tok->tp == K_REJECT6) &&
+ tor_addr_family(&r->addr) != AF_INET6) {
+ /* This is a non-fatal error, just ignore this one entry. */
+ *malformed_list = 0;
+ log_warn(LD_DIR, "IPv4 address '%s' with accept6/reject6 field type in "
+ "exit policy. Ignoring, but continuing to parse rules. (Use "
+ "accept/reject with IPv4 addresses.)",
+ tok->n_args == 1 ? tok->args[0] : "");
+ return NULL;
+ }
+
goto done;
err:
+ *malformed_list = 1;
r = NULL;
done:
token_clear(tok);
@@ -3733,19 +3766,26 @@ static int
router_add_exit_policy(routerinfo_t *router, directory_token_t *tok)
{
addr_policy_t *newe;
+ /* Use the standard interpretation of accept/reject *, an IPv4 wildcard. */
newe = router_parse_addr_policy(tok, 0);
if (!newe)
return -1;
if (! router->exit_policy)
router->exit_policy = smartlist_new();
+ /* Ensure that in descriptors, accept/reject fields are followed by
+ * IPv4 addresses, and accept6/reject6 fields are followed by
+ * IPv6 addresses. Unlike torrcs, descriptor exit policies do not permit
+ * accept/reject followed by IPv6. */
if (((tok->tp == K_ACCEPT6 || tok->tp == K_REJECT6) &&
tor_addr_family(&newe->addr) == AF_INET)
||
((tok->tp == K_ACCEPT || tok->tp == K_REJECT) &&
tor_addr_family(&newe->addr) == AF_INET6)) {
+ /* There's nothing the user can do about other relays' descriptors,
+ * so we don't provide usage advice here. */
log_warn(LD_DIR, "Mismatch between field type and address type in exit "
- "policy");
+ "policy '%s'. Ignoring.", tok->n_args == 1 ? tok->args[0] : "");
addr_policy_free(newe);
return -1;
}
@@ -3821,6 +3861,13 @@ router_parse_addr_policy_private(directory_token_t *tok)
result.prt_min = port_min;
result.prt_max = port_max;
+ if (tok->tp == K_ACCEPT6 || tok->tp == K_REJECT6) {
+ log_warn(LD_GENERAL,
+ "'%s' expands into rules which apply to all private IPv4 and "
+ "IPv6 addresses. (Use accept/reject private:* for IPv4 and "
+ "IPv6.)", tok->n_args == 1 ? tok->args[0] : "");
+ }
+
return addr_policy_get_canonical_entry(&result);
}
diff --git a/src/or/routerparse.h b/src/or/routerparse.h
index 85e4b7d88e..99fd52866c 100644
--- a/src/or/routerparse.h
+++ b/src/or/routerparse.h
@@ -41,7 +41,7 @@ extrainfo_t *extrainfo_parse_entry_from_string(const char *s, const char *end,
int cache_copy, struct digest_ri_map_t *routermap,
int *can_dl_again_out);
MOCK_DECL(addr_policy_t *, router_parse_addr_policy_item_from_string,
- (const char *s, int assume_action));
+ (const char *s, int assume_action, int *malformed_list));
version_status_t tor_version_is_obsolete(const char *myversion,
const char *versionlist);
int tor_version_as_new_as(const char *platform, const char *cutoff);
diff --git a/src/or/routerset.c b/src/or/routerset.c
index 9fe5dffdeb..3be55d3404 100644
--- a/src/or/routerset.c
+++ b/src/or/routerset.c
@@ -85,10 +85,13 @@ routerset_parse(routerset_t *target, const char *s, const char *description)
int added_countries = 0;
char *countryname;
smartlist_t *list = smartlist_new();
+ int malformed_list;
smartlist_split_string(list, s, ",",
SPLIT_SKIP_SPACE | SPLIT_IGNORE_BLANK, 0);
SMARTLIST_FOREACH_BEGIN(list, char *, nick) {
addr_policy_t *p;
+ /* if it doesn't pass our validation, assume it's malformed */
+ malformed_list = 1;
if (is_legal_hexdigest(nick)) {
char d[DIGEST_LEN];
if (*nick == '$')
@@ -106,15 +109,21 @@ routerset_parse(routerset_t *target, const char *s, const char *description)
added_countries = 1;
} else if ((strchr(nick,'.') || strchr(nick, '*')) &&
(p = router_parse_addr_policy_item_from_string(
- nick, ADDR_POLICY_REJECT))) {
+ nick, ADDR_POLICY_REJECT,
+ &malformed_list))) {
log_debug(LD_CONFIG, "Adding address %s to %s", nick, description);
smartlist_add(target->policies, p);
- } else {
- log_warn(LD_CONFIG, "Entry '%s' in %s is malformed.", nick,
- description);
+ } else if (malformed_list) {
+ log_warn(LD_CONFIG, "Entry '%s' in %s is malformed. Discarding entire"
+ " list.", nick, description);
r = -1;
tor_free(nick);
SMARTLIST_DEL_CURRENT(list, nick);
+ } else {
+ log_notice(LD_CONFIG, "Entry '%s' in %s is ignored. Using the"
+ " remainder of the list.", nick, description);
+ tor_free(nick);
+ SMARTLIST_DEL_CURRENT(list, nick);
}
} SMARTLIST_FOREACH_END(nick);
policy_expand_unspec(&target->policies);
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index 33f90c7da5..5e91468e8c 100644
--- a/src/test/test_policy.c
+++ b/src/test/test_policy.c
@@ -84,11 +84,13 @@ test_policies_general(void *arg)
smartlist_t *sm = NULL;
char *policy_str = NULL;
short_policy_t *short_parsed = NULL;
+ int malformed_list = -1;
(void)arg;
policy = smartlist_new();
- p = router_parse_addr_policy_item_from_string("reject 192.168.0.0/16:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 192.168.0.0/16:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
tt_int_op(ADDR_POLICY_REJECT,OP_EQ, p->policy_type);
tor_addr_from_ipv4h(&tar, 0xc0a80000u);
@@ -117,60 +119,76 @@ test_policies_general(void *arg)
tt_assert(policy2);
policy3 = smartlist_new();
- p = router_parse_addr_policy_item_from_string("reject *:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject *:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy3, p);
- p = router_parse_addr_policy_item_from_string("accept *:*",-1);
+ p = router_parse_addr_policy_item_from_string("accept *:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy3, p);
policy4 = smartlist_new();
- p = router_parse_addr_policy_item_from_string("accept *:443",-1);
+ p = router_parse_addr_policy_item_from_string("accept *:443", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy4, p);
- p = router_parse_addr_policy_item_from_string("accept *:443",-1);
+ p = router_parse_addr_policy_item_from_string("accept *:443", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy4, p);
policy5 = smartlist_new();
- p = router_parse_addr_policy_item_from_string("reject 0.0.0.0/8:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 0.0.0.0/8:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject 169.254.0.0/16:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 169.254.0.0/16:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject 127.0.0.0/8:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 127.0.0.0/8:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject 192.168.0.0/16:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 192.168.0.0/16:*",
+ -1, &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject 10.0.0.0/8:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 10.0.0.0/8:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject 172.16.0.0/12:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 172.16.0.0/12:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject 80.190.250.90:*",-1);
+ p = router_parse_addr_policy_item_from_string("reject 80.190.250.90:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject *:1-65534",-1);
+ p = router_parse_addr_policy_item_from_string("reject *:1-65534", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("reject *:65535",-1);
+ p = router_parse_addr_policy_item_from_string("reject *:65535", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
- p = router_parse_addr_policy_item_from_string("accept *:1-65535",-1);
+ p = router_parse_addr_policy_item_from_string("accept *:1-65535", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy5, p);
policy6 = smartlist_new();
- p = router_parse_addr_policy_item_from_string("accept 43.3.0.0/9:*",-1);
+ p = router_parse_addr_policy_item_from_string("accept 43.3.0.0/9:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy6, p);
policy7 = smartlist_new();
- p = router_parse_addr_policy_item_from_string("accept 0.0.0.0/8:*",-1);
+ p = router_parse_addr_policy_item_from_string("accept 0.0.0.0/8:*", -1,
+ &malformed_list);
tt_assert(p != NULL);
smartlist_add(policy7, p);
@@ -297,6 +315,68 @@ test_policies_general(void *arg)
TT_BAD_SHORT_POLICY("accept 1-,3");
TT_BAD_SHORT_POLICY("accept 1-,3");
+ /* Make sure that IPv4 addresses are ignored in accept6/reject6 lines. */
+ p = router_parse_addr_policy_item_from_string("accept6 1.2.3.4:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(!malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("reject6 2.4.6.0/24:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(!malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("accept6 *4:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(!malformed_list);
+
+ /* Make sure malformed policies are detected as such. */
+ p = router_parse_addr_policy_item_from_string("bad_token *4:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("accept6 **:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("accept */15:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("reject6 */:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("accept 127.0.0.1/33:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("accept6 [::1]/129:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("reject 8.8.8.8/-1:*", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("reject 8.8.4.4:10-5", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
+ p = router_parse_addr_policy_item_from_string("reject 1.2.3.4:-1", -1,
+ &malformed_list);
+ tt_assert(p == NULL);
+ tt_assert(malformed_list);
+
/* Test a too-long policy. */
{
int i;
@@ -360,6 +440,7 @@ test_dump_exit_policy_to_string(void *arg)
{
char *ep;
addr_policy_t *policy_entry;
+ int malformed_list = -1;
routerinfo_t *ri = tor_malloc_zero(sizeof(routerinfo_t));
@@ -376,7 +457,8 @@ test_dump_exit_policy_to_string(void *arg)
ri->exit_policy = smartlist_new();
ri->policy_is_reject_star = 0;
- policy_entry = router_parse_addr_policy_item_from_string("accept *:*",-1);
+ policy_entry = router_parse_addr_policy_item_from_string("accept *:*", -1,
+ &malformed_list);
smartlist_add(ri->exit_policy,policy_entry);
@@ -386,7 +468,8 @@ test_dump_exit_policy_to_string(void *arg)
tor_free(ep);
- policy_entry = router_parse_addr_policy_item_from_string("reject *:25",-1);
+ policy_entry = router_parse_addr_policy_item_from_string("reject *:25", -1,
+ &malformed_list);
smartlist_add(ri->exit_policy,policy_entry);
@@ -397,7 +480,8 @@ test_dump_exit_policy_to_string(void *arg)
tor_free(ep);
policy_entry =
- router_parse_addr_policy_item_from_string("reject 8.8.8.8:*",-1);
+ router_parse_addr_policy_item_from_string("reject 8.8.8.8:*", -1,
+ &malformed_list);
smartlist_add(ri->exit_policy,policy_entry);
@@ -407,7 +491,8 @@ test_dump_exit_policy_to_string(void *arg)
tor_free(ep);
policy_entry =
- router_parse_addr_policy_item_from_string("reject6 [FC00::]/7:*",-1);
+ router_parse_addr_policy_item_from_string("reject6 [FC00::]/7:*", -1,
+ &malformed_list);
smartlist_add(ri->exit_policy,policy_entry);
@@ -418,7 +503,8 @@ test_dump_exit_policy_to_string(void *arg)
tor_free(ep);
policy_entry =
- router_parse_addr_policy_item_from_string("accept6 [c000::]/3:*",-1);
+ router_parse_addr_policy_item_from_string("accept6 [c000::]/3:*", -1,
+ &malformed_list);
smartlist_add(ri->exit_policy,policy_entry);
diff --git a/src/test/test_routerset.c b/src/test/test_routerset.c
index 9bd0c125c3..90dfb28c6b 100644
--- a/src/test/test_routerset.c
+++ b/src/test/test_routerset.c
@@ -430,7 +430,7 @@ NS(test_main)(void *arg)
*/
NS_DECL(addr_policy_t *, router_parse_addr_policy_item_from_string,
- (const char *s, int assume_action));
+ (const char *s, int assume_action, int *malformed_list));
addr_policy_t *NS(mock_addr_policy);
@@ -457,10 +457,13 @@ NS(test_main)(void *arg)
}
addr_policy_t *
-NS(router_parse_addr_policy_item_from_string)(const char *s, int assume_action)
+NS(router_parse_addr_policy_item_from_string)(const char *s,
+ int assume_action,
+ int *malformed_list)
{
(void)s;
(void)assume_action;
+ (void)malformed_list;
CALLED(router_parse_addr_policy_item_from_string)++;
return NS(mock_addr_policy);
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion