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.

4 files changed, 37 insertions, 55 deletions

diff --git a/src/rust/protover/errors.rs b/src/rust/protover/errors.rs
index dc0d8735f4..04397ac4fe 100644
--- a/src/rust/protover/errors.rs
+++ b/src/rust/protover/errors.rs
@@ -36,7 +36,7 @@ impl Display for ProtoverError {
             ProtoverError::Unparseable => write!(f, "The protover string was unparseable."),
             ProtoverError::ExceedsMax => write!(
                 f,
-                "The high in a (low, high) protover range exceeds u32::MAX."
+                "The high in a (low, high) protover range exceeds 63."
             ),
             ProtoverError::ExceedsExpansionLimit => write!(
                 f,
diff --git a/src/rust/protover/protoset.rs b/src/rust/protover/protoset.rs
index 3b283983c8..0ab94457c5 100644
--- a/src/rust/protover/protoset.rs
+++ b/src/rust/protover/protoset.rs
@@ -294,6 +294,10 @@ impl ProtoSet {
     }
 }
 
+/// Largest allowed protocol version.
+/// C_RUST_COUPLED: protover.c `MAX_PROTOCOL_VERSION`
+const MAX_PROTOCOL_VERSION: Version = 63;
+
 impl FromStr for ProtoSet {
     type Err = ProtoverError;
 
@@ -370,7 +374,7 @@ impl FromStr for ProtoSet {
         let pieces: ::std::str::Split<char> = version_string.split(',');
 
         for p in pieces {
-            if p.contains('-') {
+            let (lo,hi) = if p.contains('-') {
                 let mut pair = p.splitn(2, '-');
 
                 let low = pair.next().ok_or(ProtoverError::Unparseable)?;
@@ -379,12 +383,17 @@ impl FromStr for ProtoSet {
                 let lo: Version = low.parse().or(Err(ProtoverError::Unparseable))?;
                 let hi: Version = high.parse().or(Err(ProtoverError::Unparseable))?;
 
-                pairs.push((lo, hi));
+                (lo,hi)
             } else {
                 let v: u32 = p.parse().or(Err(ProtoverError::Unparseable))?;
 
-                pairs.push((v, v));
+                (v, v)
+            };
+
+            if lo > MAX_PROTOCOL_VERSION || hi > MAX_PROTOCOL_VERSION {
+                return Err(ProtoverError::ExceedsMax);
             }
+            pairs.push((lo, hi));
         }
 
         ProtoSet::from_slice(&pairs[..])
@@ -674,12 +683,11 @@ mod test {
 
     #[test]
     fn test_protoset_into_vec() {
-        let ps: ProtoSet = "1-13,42,9001,4294967294".parse().unwrap();
+        let ps: ProtoSet = "1-13,42".parse().unwrap();
         let v: Vec<Version> = ps.into();
 
         assert!(v.contains(&7));
-        assert!(v.contains(&9001));
-        assert!(v.contains(&4294967294));
+        assert!(v.contains(&42));
     }
 }
 
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index 06fdf56c69..536667f61b 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -866,12 +866,12 @@ mod test {
 
     #[test]
     fn test_protoentry_from_str_allowed_number_of_versions() {
-        assert_protoentry_is_parseable!("Desc=1-4294967294");
+        assert_protoentry_is_parseable!("Desc=1-63");
     }
 
     #[test]
     fn test_protoentry_from_str_too_many_versions() {
-        assert_protoentry_is_unparseable!("Desc=1-4294967295");
+        assert_protoentry_is_unparseable!("Desc=1-64");
     }
 
     #[test]
@@ -910,10 +910,10 @@ mod test {
 
     #[test]
     fn test_protoentry_all_supported_unsupported_high_version() {
-        let protocols: UnvalidatedProtoEntry = "HSDir=12-100".parse().unwrap();
+        let protocols: UnvalidatedProtoEntry = "HSDir=12-60".parse().unwrap();
         let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
         assert_eq!(true, unsupported.is_some());
-        assert_eq!("HSDir=12-100", &unsupported.unwrap().to_string());
+        assert_eq!("HSDir=12-60", &unsupported.unwrap().to_string());
     }
 
     #[test]
@@ -962,7 +962,7 @@ mod test {
             ProtoSet::from_str(&versions).unwrap().to_string()
         );
 
-        versions = "1-3,500";
+        versions = "1-3,50";
         assert_eq!(
             String::from(versions),
             ProtoSet::from_str(&versions).unwrap().to_string()
diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs
index 942fe3c6ab..d563202d87 100644
--- a/src/rust/protover/tests/protover.rs
+++ b/src/rust/protover/tests/protover.rs
@@ -98,10 +98,10 @@ fn protocol_all_supported_with_unsupported_protocol() {
 
 #[test]
 fn protocol_all_supported_with_unsupported_versions() {
-    let protocols: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap();
+    let protocols: UnvalidatedProtoEntry = "Link=3-63".parse().unwrap();
     let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
     assert_eq!(true, unsupported.is_some());
-    assert_eq!("Link=6-999", &unsupported.unwrap().to_string());
+    assert_eq!("Link=6-63", &unsupported.unwrap().to_string());
 }
 
 #[test]
@@ -114,10 +114,10 @@ fn protocol_all_supported_with_unsupported_low_version() {
 
 #[test]
 fn protocol_all_supported_with_unsupported_high_version() {
-    let protocols: UnvalidatedProtoEntry = "Cons=1-2,999".parse().unwrap();
+    let protocols: UnvalidatedProtoEntry = "Cons=1-2,60".parse().unwrap();
     let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
     assert_eq!(true, unsupported.is_some());
-    assert_eq!("Cons=999", &unsupported.unwrap().to_string());
+    assert_eq!("Cons=60", &unsupported.unwrap().to_string());
 }
 
 #[test]
@@ -195,27 +195,27 @@ fn protover_compute_vote_returns_protocols_that_it_doesnt_currently_support() {
 
 #[test]
 fn protover_compute_vote_returns_matching_for_mix() {
-    let protocols: &[UnvalidatedProtoEntry] = &["Link=1-10,500 Cons=1,3-7,8".parse().unwrap()];
+    let protocols: &[UnvalidatedProtoEntry] = &["Link=1-10,50 Cons=1,3-7,8".parse().unwrap()];
     let listed = ProtoverVote::compute(protocols, &1);
-    assert_eq!("Cons=1,3-8 Link=1-10,500", listed.to_string());
+    assert_eq!("Cons=1,3-8 Link=1-10,50", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_matching_for_longer_mix() {
     let protocols: &[UnvalidatedProtoEntry] = &[
-        "Desc=1-10,500 Cons=1,3-7,8".parse().unwrap(),
-        "Link=123-456,78 Cons=2-6,8 Desc=9".parse().unwrap(),
+        "Desc=1-10,50 Cons=1,3-7,8".parse().unwrap(),
+        "Link=12-45,8 Cons=2-6,8 Desc=9".parse().unwrap(),
     ];
 
     let listed = ProtoverVote::compute(protocols, &1);
-    assert_eq!("Cons=1-8 Desc=1-10,500 Link=78,123-456", listed.to_string());
+    assert_eq!("Cons=1-8 Desc=1-10,50 Link=8,12-45", listed.to_string());
 }
 
 #[test]
 fn protover_compute_vote_returns_matching_for_longer_mix_with_threshold_two() {
     let protocols: &[UnvalidatedProtoEntry] = &[
-        "Desc=1-10,500 Cons=1,3-7,8".parse().unwrap(),
-        "Link=123-456,78 Cons=2-6,8 Desc=9".parse().unwrap(),
+        "Desc=1-10,50 Cons=1,3-7,8".parse().unwrap(),
+        "Link=8,12-45 Cons=2-6,8 Desc=9".parse().unwrap(),
     ];
 
     let listed = ProtoverVote::compute(protocols, &2);
@@ -320,30 +320,20 @@ fn protocol_all_supported_with_single_protocol_and_protocol_range() {
     assert_eq!(true, unsupported.is_none());
 }
 
-// By allowing us to add to votes, the C implementation allows us to
-// exceed the limit.
-#[test]
-fn protover_compute_vote_may_exceed_limit() {
-    let proto1: UnvalidatedProtoEntry = "Sleen=1-65535".parse().unwrap();
-    let proto2: UnvalidatedProtoEntry = "Sleen=100000".parse().unwrap();
-
-    let _result: UnvalidatedProtoEntry = ProtoverVote::compute(&[proto1, proto2], &1);
-}
-
 #[test]
 fn protover_all_supported_should_exclude_versions_we_actually_do_support() {
-    let proto: UnvalidatedProtoEntry = "Link=3-999".parse().unwrap();
+    let proto: UnvalidatedProtoEntry = "Link=3-63".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();
 
-    assert_eq!(result, "Link=6-999".to_string());
+    assert_eq!(result, "Link=6-63".to_string());
 }
 
 #[test]
 fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex1() {
-    let proto: UnvalidatedProtoEntry = "Link=1-3,345-666".parse().unwrap();
+    let proto: UnvalidatedProtoEntry = "Link=1-3,30-63".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();
 
-    assert_eq!(result, "Link=345-666".to_string());
+    assert_eq!(result, "Link=30-63".to_string());
 }
 
 #[test]
@@ -356,26 +346,10 @@ fn protover_all_supported_should_exclude_versions_we_actually_do_support_complex
 
 #[test]
 fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() {
-    let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=9000-9001".parse().unwrap();
-    let result: String = proto.all_supported().unwrap().to_string();
-
-    assert_eq!(result, "Link=6-12 Quokka=9000-9001".to_string());
-}
-
-#[test]
-fn protover_all_supported_should_not_dos_anyones_computer() {
-    let proto: UnvalidatedProtoEntry = "Link=1-2147483648".parse().unwrap();
-    let result: String = proto.all_supported().unwrap().to_string();
-
-    assert_eq!(result, "Link=6-2147483648".to_string());
-}
-
-#[test]
-fn protover_all_supported_should_not_dos_anyones_computer_max_versions() {
-    let proto: UnvalidatedProtoEntry = "Link=1-4294967294".parse().unwrap();
+    let proto: UnvalidatedProtoEntry = "Link=1-3,5-12 Quokka=50-51".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();
 
-    assert_eq!(result, "Link=6-4294967294".to_string());
+    assert_eq!(result, "Link=6-12 Quokka=50-51".to_string());
 }
 
 #[test]