Actually allow unrecognized address types in NETINFO cell

Ignore the address value instead of failing with error condition in case
unrecognized address type is found.

src/test/include.am

@@ -147,6 +147,7 @@ src_test_test_SOURCES += \
 	src/test/test_logging.c \
 	src/test/test_mainloop.c \
 	src/test/test_microdesc.c \
+	src/test/test_netinfo.c \
 	src/test/test_nodelist.c \
 	src/test/test_oom.c \
 	src/test/test_oos.c \

src/test/test.c

@@ -890,6 +890,7 @@ struct testgroup_t testgroups[] = {
   { "legacy_hs/", hs_tests },
   { "link-handshake/", link_handshake_tests },
   { "mainloop/", mainloop_tests },
+  { "netinfo/", netinfo_tests },
   { "nodelist/", nodelist_tests },
   { "oom/", oom_tests },
   { "oos/", oos_tests },

src/test/test.h

@@ -232,6 +232,7 @@ extern struct testcase_t link_handshake_tests[];
 extern struct testcase_t logging_tests[];
 extern struct testcase_t mainloop_tests[];
 extern struct testcase_t microdesc_tests[];
+extern struct testcase_t netinfo_tests[];
 extern struct testcase_t nodelist_tests[];
 extern struct testcase_t oom_tests[];
 extern struct testcase_t oos_tests[];

src/test/test_netinfo.c

@@ -0,0 +1,48 @@
+/* Copyright (c) 2007-2018, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+#include "orconfig.h"
+#include "core/or/or.h" // XXX: is this needed?
+#include "trunnel/netinfo.h"
+#include "test/test.h"
+
+static void
+test_netinfo_unsupported_addr(void *arg)
+{
+  const uint8_t wire_data[] =
+    { // TIME
+      0x00, 0x00, 0x00, 0x01,
+      // OTHERADDR
+      0x04, // ATYPE
+      0x04, // ALEN
+      0x08, 0x08, 0x08, 0x08, // AVAL
+      0x01, // NMYADDR
+      0x03, // ATYPE (unsupported)
+      0x05, // ALEN
+      'a', 'd', 'r', 'r', '!' // AVAL (unsupported)
+    };
+
+  (void)arg;
+
+  netinfo_cell_t *parsed_cell = NULL;
+
+  ssize_t parsed = netinfo_cell_parse(&parsed_cell, wire_data,
+                  sizeof(wire_data));
+
+  tt_assert(parsed == sizeof(wire_data));
+
+  netinfo_addr_t *addr = netinfo_cell_get_my_addrs(parsed_cell, 0);
+  tt_assert(addr);
+
+  tt_int_op(3, OP_EQ, netinfo_addr_get_addr_type(addr));
+  tt_int_op(5, OP_EQ, netinfo_addr_get_len(addr));
+
+ done:
+  netinfo_cell_free(parsed_cell);
+}
+
+struct testcase_t netinfo_tests[] = {
+  { "unsupported_addr", test_netinfo_unsupported_addr, 0, NULL, NULL },
+  END_OF_TESTCASES
+};
+

src/trunnel/netinfo.c

@@ -140,7 +140,6 @@ netinfo_addr_check(const netinfo_addr_t *obj)
       break;
 
     default:
-        return "Bad tag for union";
       break;
   }
   return NULL;
@@ -175,7 +174,6 @@ netinfo_addr_encoded_len(const netinfo_addr_t *obj)
       break;
 
     default:
-      trunnel_assert(0);
       break;
   }
   return result;
@@ -198,6 +196,8 @@ netinfo_addr_encode(uint8_t *output, const size_t avail, const netinfo_addr_t *o
   const ssize_t encoded_len = netinfo_addr_encoded_len(obj);
 #endif
 
+  uint8_t *backptr_len = NULL;
+
   if (NULL != (msg = netinfo_addr_check(obj)))
     goto check_failed;
 
@@ -213,11 +213,14 @@ netinfo_addr_encode(uint8_t *output, const size_t avail, const netinfo_addr_t *o
   written += 1; ptr += 1;
 
   /* Encode u8 len */
+  backptr_len = ptr;
   trunnel_assert(written <= avail);
   if (avail - written < 1)
     goto truncated;
   trunnel_set_uint8(ptr, (obj->len));
   written += 1; ptr += 1;
+  {
+    size_t written_before_union = written;
 
     /* Encode union addr[addr_type] */
     trunnel_assert(written <= avail);
@@ -244,9 +247,16 @@ netinfo_addr_encode(uint8_t *output, const size_t avail, const netinfo_addr_t *o
         break;
 
       default:
-      trunnel_assert(0);
         break;
     }
+    /* Write the length field back to len */
+    trunnel_assert(written >= written_before_union);
+#if UINT8_MAX < SIZE_MAX
+    if (written - written_before_union > UINT8_MAX)
+      goto check_failed;
+#endif
+    trunnel_set_uint8(backptr_len, (written - written_before_union));
+  }
 
 
   trunnel_assert(ptr == output + written);
@@ -291,6 +301,11 @@ netinfo_addr_parse_into(netinfo_addr_t *obj, const uint8_t *input, const size_t
   CHECK_REMAINING(1, truncated);
   obj->len = (trunnel_get_uint8(ptr));
   remaining -= 1; ptr += 1;
+  {
+    size_t remaining_after;
+    CHECK_REMAINING(obj->len, truncated);
+    remaining_after = remaining - obj->len;
+    remaining = obj->len;
 
     /* Parse union addr[addr_type] */
     switch (obj->addr_type) {
@@ -298,7 +313,7 @@ netinfo_addr_parse_into(netinfo_addr_t *obj, const uint8_t *input, const size_t
       case NETINFO_ADDR_TYPE_IPV4:
 
         /* Parse u32 addr_ipv4 */
-      CHECK_REMAINING(4, truncated);
+        CHECK_REMAINING(4, fail);
         obj->addr_ipv4 = trunnel_ntohl(trunnel_get_uint32(ptr));
         remaining -= 4; ptr += 4;
         break;
@@ -306,15 +321,20 @@ netinfo_addr_parse_into(netinfo_addr_t *obj, const uint8_t *input, const size_t
       case NETINFO_ADDR_TYPE_IPV6:
 
         /* Parse u8 addr_ipv6[16] */
-      CHECK_REMAINING(16, truncated);
+        CHECK_REMAINING(16, fail);
         memcpy(obj->addr_ipv6, ptr, 16);
         remaining -= 16; ptr += 16;
         break;
 
       default:
-      goto fail;
+        /* Skip to end of union */
+        ptr += remaining; remaining = 0;
         break;
     }
+    if (remaining != 0)
+      goto fail;
+    remaining = remaining_after;
+  }
   trunnel_assert(ptr + remaining == input + len_in);
   return len_in - remaining;
 

src/trunnel/netinfo.trunnel

@@ -7,10 +7,10 @@ const NETINFO_ADDR_TYPE_IPV6 = 6;
 struct netinfo_addr {
   u8 addr_type;
   u8 len;
-  union addr[addr_type] {
+  union addr[addr_type] with length len {
     NETINFO_ADDR_TYPE_IPV4: u32 ipv4;
     NETINFO_ADDR_TYPE_IPV6: u8 ipv6[16];
-    default: fail;
+    default: ignore;
   };
 
 }