Implement proposal 318: Limit protovers to 0..63

In brief: we go through a lot of gymnastics to handle huge protover
numbers, but after years of development we're not even close to 10
for any of our current versions.  We also have a convenient
workaround available in case we ever run out of protocols: if (for
example) we someday need Link=64, we can just add Link2=0 or
something.

This patch is a minimal patch to change tor's behavior; it doesn't
take advantage of the new restrictions.

Implements #40133 and proposal 318.

1 files changed, 22 insertions, 69 deletions

diff --git a/src/test/test_protover.c b/src/test/test_protover.c
index 63c508bd13..b4689045cf 100644
--- a/src/test/test_protover.c
+++ b/src/test/test_protover.c
@@ -25,7 +25,7 @@ test_protover_parse(void *arg)
 #else
   char *re_encoded = NULL;
 
-  const char *orig = "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16,900";
+  const char *orig = "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16";
   smartlist_t *elts = parse_protocol_list(orig);
 
   tt_assert(elts);
@@ -61,7 +61,7 @@ test_protover_parse(void *arg)
 
   e = smartlist_get(elts, 3);
   tt_str_op(e->name, OP_EQ, "Quux");
-  tt_int_op(smartlist_len(e->ranges), OP_EQ, 4);
+  tt_int_op(smartlist_len(e->ranges), OP_EQ, 3);
   {
     r = smartlist_get(e->ranges, 0);
     tt_int_op(r->low, OP_EQ, 9);
@@ -74,10 +74,6 @@ test_protover_parse(void *arg)
     r = smartlist_get(e->ranges, 2);
     tt_int_op(r->low, OP_EQ, 15);
     tt_int_op(r->high, OP_EQ, 16);
-
-    r = smartlist_get(e->ranges, 3);
-    tt_int_op(r->low, OP_EQ, 900);
-    tt_int_op(r->high, OP_EQ, 900);
   }
 
   re_encoded = encode_protocol_list(elts);
@@ -149,14 +145,14 @@ test_protover_vote(void *arg)
   tt_str_op(result, OP_EQ, "");
   tor_free(result);
 
-  smartlist_add(lst, (void*) "Foo=1-10,500 Bar=1,3-7,8");
+  smartlist_add(lst, (void*) "Foo=1-10,63 Bar=1,3-7,8");
   result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "Bar=1,3-8 Foo=1-10,500");
+  tt_str_op(result, OP_EQ, "Bar=1,3-8 Foo=1-10,63");
   tor_free(result);
 
-  smartlist_add(lst, (void*) "Quux=123-456,78 Bar=2-6,8 Foo=9");
+  smartlist_add(lst, (void*) "Quux=12-45 Bar=2-6,8 Foo=9");
   result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "Bar=1-8 Foo=1-10,500 Quux=78,123-456");
+  tt_str_op(result, OP_EQ, "Bar=1-8 Foo=1-10,63 Quux=12-45");
   tor_free(result);
 
   result = protover_compute_vote(lst, 2);
@@ -194,45 +190,16 @@ test_protover_vote(void *arg)
 
   /* Just below the threshold: Rust */
   smartlist_clear(lst);
-  smartlist_add(lst, (void*) "Sleen=1-500");
+  smartlist_add(lst, (void*) "Sleen=1-50");
   result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "Sleen=1-500");
+  tt_str_op(result, OP_EQ, "Sleen=1-50");
   tor_free(result);
 
   /* Just below the threshold: C */
   smartlist_clear(lst);
-  smartlist_add(lst, (void*) "Sleen=1-65536");
-  result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "Sleen=1-65536");
-  tor_free(result);
-
-  /* Large protover lists that exceed the threshold */
-
-  /* By adding two votes, C allows us to exceed the limit */
-  smartlist_add(lst, (void*) "Sleen=1-65536");
-  smartlist_add(lst, (void*) "Sleen=100000");
-  result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "Sleen=1-65536,100000");
-  tor_free(result);
-
-  /* Large integers */
-  smartlist_clear(lst);
-  smartlist_add(lst, (void*) "Sleen=4294967294");
+  smartlist_add(lst, (void*) "Sleen=1-63");
   result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "Sleen=4294967294");
-  tor_free(result);
-
-  /* This parses, but fails at the vote stage */
-  smartlist_clear(lst);
-  smartlist_add(lst, (void*) "Sleen=4294967295");
-  result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "");
-  tor_free(result);
-
-  smartlist_clear(lst);
-  smartlist_add(lst, (void*) "Sleen=4294967296");
-  result = protover_compute_vote(lst, 1);
-  tt_str_op(result, OP_EQ, "");
+  tt_str_op(result, OP_EQ, "Sleen=1-63");
   tor_free(result);
 
   /* Protocol name too long */
@@ -272,8 +239,8 @@ test_protover_all_supported(void *arg)
   tt_assert(! protover_all_supported("Wombat=9", &msg));
   tt_str_op(msg, OP_EQ, "Wombat=9");
   tor_free(msg);
-  tt_assert(! protover_all_supported("Link=999", &msg));
-  tt_str_op(msg, OP_EQ, "Link=999");
+  tt_assert(! protover_all_supported("Link=60", &msg));
+  tt_str_op(msg, OP_EQ, "Link=60");
   tor_free(msg);
 
   // Mix of things we support and things we don't
@@ -283,11 +250,11 @@ test_protover_all_supported(void *arg)
 
   /* Mix of things we support and don't support within a single protocol
    * which we do support */
-  tt_assert(! protover_all_supported("Link=3-999", &msg));
-  tt_str_op(msg, OP_EQ, "Link=6-999");
+  tt_assert(! protover_all_supported("Link=3-60", &msg));
+  tt_str_op(msg, OP_EQ, "Link=6-60");
   tor_free(msg);
-  tt_assert(! protover_all_supported("Link=1-3,345-666", &msg));
-  tt_str_op(msg, OP_EQ, "Link=345-666");
+  tt_assert(! protover_all_supported("Link=1-3,50-63", &msg));
+  tt_str_op(msg, OP_EQ, "Link=50-63");
   tor_free(msg);
   tt_assert(! protover_all_supported("Link=1-3,5-12", &msg));
   tt_str_op(msg, OP_EQ, "Link=6-12");
@@ -295,18 +262,8 @@ test_protover_all_supported(void *arg)
 
   /* Mix of protocols we do support and some we don't, where the protocols
    * we do support have some versions we don't support. */
-  tt_assert(! protover_all_supported("Link=1-3,5-12 Quokka=9000-9001", &msg));
-  tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=9000-9001");
-  tor_free(msg);
-
-  /* We shouldn't be able to DoS ourselves parsing a large range. */
-  tt_assert(! protover_all_supported("Sleen=1-2147483648", &msg));
-  tt_str_op(msg, OP_EQ, "Sleen=1-2147483648");
-  tor_free(msg);
-
-  /* This case is allowed. */
-  tt_assert(! protover_all_supported("Sleen=1-4294967294", &msg));
-  tt_str_op(msg, OP_EQ, "Sleen=1-4294967294");
+  tt_assert(! protover_all_supported("Link=1-3,5-12 Quokka=40-41", &msg));
+  tt_str_op(msg, OP_EQ, "Link=6-12 Quokka=40-41");
   tor_free(msg);
 
   /* If we get a (barely) valid (but unsupported list, we say "yes, that's
@@ -566,9 +523,9 @@ test_protover_vote_roundtrip(void *args)
     /* Will fail because of 4294967295. */
     { "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16,900 Zn=1,4294967295",
        NULL },
-    { "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16,900 Zn=1,4294967294",
-      "Bar=3 Foo=1,3 Quux=9-12,14-16,900 Zn=1,4294967294" },
-    { "Zu16=1,65536", "Zu16=1,65536" },
+    { "Foo=1,3 Bar=3 Baz= Quux=9-12,14,15-16,50 Zn=1,42",
+      "Bar=3 Foo=1,3 Quux=9-12,14-16,50 Zn=1,42" },
+    { "Zu16=1,63", "Zu16=1,63" },
     { "N-1=1,2", "N-1=1-2" },
     { "-1=4294967295", NULL },
     { "-1=3", "-1=3" },
@@ -602,12 +559,8 @@ test_protover_vote_roundtrip(void *args)
     /* Large integers */
     { "Link=4294967296", NULL },
     /* Large range */
-    { "Sleen=1-501", "Sleen=1-501" },
+    { "Sleen=1-63", "Sleen=1-63" },
     { "Sleen=1-65537", NULL },
-    /* Both C/Rust implementations should be able to handle this mild DoS. */
-    { "Sleen=1-2147483648", NULL },
-    /* Rust tests are built in debug mode, so ints are bounds-checked. */
-    { "Sleen=1-4294967295", NULL },
   };
   unsigned u;
   smartlist_t *votes = smartlist_new();