[tor-commits] [tor/master] Add policy assume_action support for IPv6 addresses

[nickm at torproject.org]

commit ce92335214f4490f0e14487a99415c26777be2a8
Author: teor (Tim Wilson-Brown) <teor2345 at gmail.com>
Date:   Wed Dec 16 08:49:06 2015 +1100

    Add policy assume_action support for IPv6 addresses
    
    These IPv6 addresses must be quoted, because : is the port separator,
    and "acce" is a valid hex block.
    
    Add unit tests for assumed actions in IPv6 policies.
---
 src/or/routerparse.c   |    8 +++--
 src/test/test_policy.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 8170bbb..f5a003e 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -3684,8 +3684,8 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action,
   directory_token_t *tok = NULL;
   const char *cp, *eos;
   /* Longest possible policy is
-   * "accept6 ffff:ffff:..255/128:10000-65535",
-   * which contains a max-length IPv6 address, plus 24 characters.
+   * "accept6 [ffff:ffff:..255]/128:10000-65535",
+   * which contains a max-length IPv6 address, plus 26 characters.
    * But note that there can be an arbitrary amount of space between the
    * accept and the address:mask/port element.
    * We don't need to multiply TOR_ADDR_BUF_LEN by 2, as there is only one
@@ -3700,7 +3700,9 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action,
   *malformed_list = 0;
 
   s = eat_whitespace(s);
-  if ((*s == '*' || TOR_ISDIGIT(*s)) && assume_action >= 0) {
+  /* We can only do assume_action on []-quoted IPv6, as "a" (accept)
+   * and ":" (port separator) are ambiguous */
+  if ((*s == '*' || *s == '[' || TOR_ISDIGIT(*s)) && assume_action >= 0) {
     if (tor_snprintf(line, sizeof(line), "%s %s",
                assume_action == ADDR_POLICY_ACCEPT?"accept":"reject", s)<0) {
       log_warn(LD_DIR, "Policy %s is too long.", escaped(s));
diff --git a/src/test/test_policy.c b/src/test/test_policy.c
index b7d89a1..4f5565e 100644
--- a/src/test/test_policy.c
+++ b/src/test/test_policy.c
@@ -270,6 +270,93 @@ test_policies_general(void *arg)
   addr_policy_list_free(policy);
   policy = NULL;
 
+  /* make sure assume_action works */
+  malformed_list = 0;
+  p = router_parse_addr_policy_item_from_string("127.0.0.1",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("127.0.0.1:*",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("[::]",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("[::]:*",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("[face::b]",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("[b::aaaa]",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("*",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("*4",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  p = router_parse_addr_policy_item_from_string("*6",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(p);
+  addr_policy_free(p);
+  tt_assert(!malformed_list);
+
+  /* These are all ambiguous IPv6 addresses, it's good that we reject them */
+  p = router_parse_addr_policy_item_from_string("acce::abcd",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(!p);
+  tt_assert(malformed_list);
+  malformed_list = 0;
+
+  p = router_parse_addr_policy_item_from_string("7:1234",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(!p);
+  tt_assert(malformed_list);
+  malformed_list = 0;
+
+  p = router_parse_addr_policy_item_from_string("::",
+                                                ADDR_POLICY_ACCEPT,
+                                                &malformed_list);
+  tt_assert(!p);
+  tt_assert(malformed_list);
+  malformed_list = 0;
+
   /* make sure compacting logic works. */
   policy = NULL;
   line.key = (char*)"foo";