rust: Fix ProtoSet and ProtoEntry to use the same DoS limits as C.

Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied
in Rust to the maximum number of version (total, for all subprotocols).
Whereas in C, it was being applied to the number of subprotocols that were
allowed.  This changes the Rust to match C's behaviour.

src/rust/protover/protoset.rs

@@ -8,7 +8,6 @@ use std::slice;
 use std::str::FromStr;
 use std::u32;
 
-use protover::MAX_PROTOCOLS_TO_EXPAND;
 use errors::ProtoverError;
 
 /// A single version number.
@@ -183,10 +182,6 @@ impl ProtoSet {
             last_high = high;
         }
 
-        if self.len() > MAX_PROTOCOLS_TO_EXPAND {
-            return Err(ProtoverError::ExceedsMax);
-        }
-
         Ok(self)
     }
 
@@ -317,9 +312,15 @@ impl FromStr for ProtoSet {
     /// assert!(protoset.contains(&5));
     /// assert!(!protoset.contains(&10));
     ///
-    /// // We can also equivalently call `ProtoSet::from_str` by doing:
+    /// // We can also equivalently call `ProtoSet::from_str` by doing (all
+    /// // implementations of `FromStr` can be called this way, this one isn't
+    /// // special):
     /// let protoset: ProtoSet = "4-6,12".parse()?;
     ///
+    /// // Calling it (either way) can take really large ranges (up to `u32::MAX`):
+    /// let protoset: ProtoSet = "1-70000".parse()?;
+    /// let protoset: ProtoSet = "1-4294967296".parse()?;
+    ///
     /// // There are lots of ways to get an `Err` from this function.  Here are
     /// // a few:
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("="));
@@ -327,7 +328,6 @@ impl FromStr for ProtoSet {
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("not_an_int"));
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("3-"));
     /// assert_eq!(Err(ProtoverError::Unparseable), ProtoSet::from_str("1-,4"));
-    /// assert_eq!(Err(ProtoverError::ExceedsMax), ProtoSet::from_str("1-70000"));
     ///
     /// // Things which would get parsed into an _empty_ `ProtoSet` are,
     /// // however, legal, and result in an empty `ProtoSet`:

src/rust/protover/protover.rs

@@ -26,7 +26,7 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha";
 /// before concluding that someone is trying to DoS us
 ///
 /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND`
-pub(crate) const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16);
+const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16);
 
 /// Known subprotocols in Tor. Indicates which subprotocol a relay supports.
 ///
@@ -155,6 +155,10 @@ impl ProtoEntry {
         supported.parse()
     }
 
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
     pub fn get(&self, protocol: &Protocol) -> Option<&ProtoSet> {
         self.0.get(protocol)
     }
@@ -209,8 +213,11 @@ impl FromStr for ProtoEntry {
             let proto_name: Protocol = proto.parse()?;
 
             proto_entry.insert(proto_name, versions);
-        }
 
+            if proto_entry.len() > MAX_PROTOCOLS_TO_EXPAND {
+                return Err(ProtoverError::ExceedsMax);
+            }
+        }
         Ok(proto_entry)
     }
 }
@@ -722,9 +729,14 @@ mod test {
         assert_protoentry_is_unparseable!("Ducks=5-7,8");
     }
 
+    #[test]
+    fn test_protoentry_from_str_allowed_number_of_versions() {
+        assert_protoentry_is_parseable!("Desc=1-4294967294");
+    }
+
     #[test]
     fn test_protoentry_from_str_too_many_versions() {
-        assert_protoentry_is_unparseable!("Desc=1-65537");
+        assert_protoentry_is_unparseable!("Desc=1-4294967295");
     }
 
     #[test]

src/rust/protover/tests/protover.rs

@@ -353,13 +353,6 @@ fn protover_all_supported_should_exclude_some_versions_and_entire_protocols() {
 }
 
 #[test]
-#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")]
-// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an
-// UnvalidatedProtoEntry is defined as a Hashmap<UnknownProtocol, ProtoSet>.
-// Because it contains the ProtoSet part, there's still *some* validation
-// happening, so in this case the DoS protections in the Rust code are kicking
-// in because the range here is exceeds the maximum, so it returns an
-// Err(ProtoverError::ExceedsMax).  The C, however seems to parse it anyway.
 fn protover_all_supported_should_not_dos_anyones_computer() {
     let proto: UnvalidatedProtoEntry = "Sleen=0-2147483648".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();
@@ -368,13 +361,6 @@ fn protover_all_supported_should_not_dos_anyones_computer() {
 }
 
 #[test]
-#[should_panic(expected = "called `Result::unwrap()` on an `Err` value: ExceedsMax")]
-// C_RUST_DIFFERS: This test fails in Rust (but not in C) because an
-// UnvalidatedProtoEntry is defined as a Hashmap<UnknownProtocol, ProtoSet>.
-// Because it contains the ProtoSet part, there's still *some* validation
-// happening, so in this case the DoS protections in the Rust code are kicking
-// in because the range here is exceeds the maximum, so it returns an
-// Err(ProtoverError::ExceedsMax).  The C, however seems to parse it anyway.
 fn protover_all_supported_should_not_dos_anyones_computer_max_versions() {
     let proto: UnvalidatedProtoEntry = "Sleen=0-4294967294".parse().unwrap();
     let result: String = proto.all_supported().unwrap().to_string();