[tor-commits] [tor/release-0.4.0] Make tor_addr_is_internal_() RFC6598 (Carrier Grade NAT) aware

[asn at torproject.org]

commit 63b404911441a7691949c475a374569d668a1b32
Author: Neel Chauhan <neel at neelc.org>
Date:   Sat Mar 2 21:25:35 2019 -0500

    Make tor_addr_is_internal_() RFC6598 (Carrier Grade NAT) aware
    
    Fixes 28525.
---
 changes/bug28525     |  8 ++++++++
 src/common/address.c | 19 +++++++++++++++----
 src/test/test_addr.c | 18 ++++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/changes/bug28525 b/changes/bug28525
new file mode 100644
index 000000000..392a9265e
--- /dev/null
+++ b/changes/bug28525
@@ -0,0 +1,8 @@
+  o Minor bugfixes (address selection):
+    - Make Tor aware of the RFC 6598 (Carrier Grade NAT) IP range, which is the
+      subnet 100.64.0.0/10. This is deployed by many ISPs as an alternative to
+      RFC 1918 that does not break existing internal networks. This patch fixes
+      security issues caused by RFC 6518 by blocking control ports on these
+      addresses and warns users if client ports or ExtORPorts are listening on
+      a RFC 6598 address. Fixes bug 28525; bugfix on 0.4.1.1-alpha. Patch by
+      Neel Chauhan.
diff --git a/src/common/address.c b/src/common/address.c
index 794345a13..71d380538 100644
--- a/src/common/address.c
+++ b/src/common/address.c
@@ -348,9 +348,18 @@ tor_addr_lookup(const char *name, uint16_t family, tor_addr_t *addr)
   }
 }
 
-/** Return true iff <b>ip</b> is an IP reserved to localhost or local networks
- * in RFC1918 or RFC4193 or RFC4291. (fec0::/10, deprecated by RFC3879, is
- * also treated as internal for now.)
+/** Return true iff <b>ip</b> is an IP reserved to localhost or local networks.
+ *
+ * If <b>ip</b> is in RFC1918 or RFC4193 or RFC4291, we will return true.
+ * (fec0::/10, deprecated by RFC3879, is also treated as internal for now
+ * and will return true.)
+ *
+ * If <b>ip</b> is 0.0.0.0 or 100.64.0.0/10 (RFC6598), we will act as:
+ *  - Internal if <b>for_listening</b> is 0, as these addresses are not
+ *    routable on the internet and we won't be publicly accessible to clients.
+ *  - External if <b>for_listening</b> is 1, as clients could connect to us
+ *    from the internet (in the case of 0.0.0.0) or a service provider's
+ *    internal network (in the case of RFC6598).
  */
 int
 tor_addr_is_internal_(const tor_addr_t *addr, int for_listening,
@@ -398,11 +407,13 @@ tor_addr_is_internal_(const tor_addr_t *addr, int for_listening,
 
     return 0;
   } else if (v_family == AF_INET) {
-    if (for_listening && !iph4) /* special case for binding to 0.0.0.0 */
+    /* special case for binding to 0.0.0.0 or 100.64/10 (RFC6598) */
+    if (for_listening && (!iph4 || ((iph4 & 0xffc00000) == 0x64400000)))
       return 0;
     if (((iph4 & 0xff000000) == 0x0a000000) || /*       10/8 */
         ((iph4 & 0xff000000) == 0x00000000) || /*        0/8 */
         ((iph4 & 0xff000000) == 0x7f000000) || /*      127/8 */
+        ((iph4 & 0xffc00000) == 0x64400000) || /*  100.64/10 */
         ((iph4 & 0xffff0000) == 0xa9fe0000) || /* 169.254/16 */
         ((iph4 & 0xfff00000) == 0xac100000) || /*  172.16/12 */
         ((iph4 & 0xffff0000) == 0xc0a80000))   /* 192.168/16 */
diff --git a/src/test/test_addr.c b/src/test/test_addr.c
index be440a092..5c4b6449c 100644
--- a/src/test/test_addr.c
+++ b/src/test/test_addr.c
@@ -1063,6 +1063,23 @@ test_addr_make_null(void *data)
   tor_free(zeros);
 }
 
+#define TEST_ADDR_INTERNAL(a, for_listening, rv) STMT_BEGIN \
+    tor_addr_t t; \
+    tt_int_op(tor_inet_pton(AF_INET, a, &t.addr.in_addr), OP_EQ, 1); \
+    t.family = AF_INET; \
+    tt_int_op(tor_addr_is_internal(&t, for_listening), OP_EQ, rv); \
+  STMT_END;
+
+static void
+test_addr_rfc6598(void *arg)
+{
+  (void)arg;
+  TEST_ADDR_INTERNAL("100.64.0.1", 0, 1);
+  TEST_ADDR_INTERNAL("100.64.0.1", 1, 0);
+ done:
+  ;
+}
+
 #define ADDR_LEGACY(name)                                               \
   { #name, test_addr_ ## name , 0, NULL, NULL }
 
@@ -1076,6 +1093,7 @@ struct testcase_t addr_tests[] = {
   { "sockaddr_to_str", test_addr_sockaddr_to_str, 0, NULL, NULL },
   { "is_loopback", test_addr_is_loopback, 0, NULL, NULL },
   { "make_null", test_addr_make_null, 0, NULL, NULL },
+  { "rfc6598", test_addr_rfc6598, 0, NULL, NULL },
   END_OF_TESTCASES
 };