commit 701c2b69f52cb4db46aa7455904e518b35fafc1a Author: Isis Lovecruft isis@torproject.org Date: Wed Mar 21 02:22:54 2018 +0000
rust: Mirror TROVE-2018-005 fix in Rust protover implementation.
* REFACTORS `UnvalidatedProtoEntry::from_str` to place the bulk of the splitting/parsing logic in to a new `UnvalidatedProtoEntry::parse_protocol_and_version_str()` method (so that both `from_str()` and `from_str_any_len()` can call it.) * ADD a new `UnvalidatedProtoEntry::from_str_any_len()` method in order to maintain compatibility with consensus methods older than 29. * ADD a limit on the number of characters in a protocol name. * FIXES part of #25517: https://bugs.torproject.org/25517 --- src/rust/protover/ffi.rs | 14 +++++-- src/rust/protover/protover.rs | 93 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 91 insertions(+), 16 deletions(-)
diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 2dfeda87b..1e0d9d6bf 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -56,7 +56,8 @@ pub extern "C" fn protover_all_supported( Err(_) => return 1, };
- let relay_proto_entry: UnvalidatedProtoEntry = match relay_version.parse() { + let relay_proto_entry: UnvalidatedProtoEntry = + match UnvalidatedProtoEntry::from_str_any_len(relay_version) { Ok(n) => n, Err(_) => return 1, }; @@ -167,6 +168,7 @@ pub extern "C" fn protover_get_supported_protocols() -> *const c_char { pub extern "C" fn protover_compute_vote( list: *const Stringlist, threshold: c_int, + allow_long_proto_names: bool, ) -> *mut c_char {
if list.is_null() { @@ -181,9 +183,13 @@ pub extern "C" fn protover_compute_vote( let mut proto_entries: Vec<UnvalidatedProtoEntry> = Vec::new();
for datum in data { - let entry: UnvalidatedProtoEntry = match datum.parse() { - Ok(x) => x, - Err(_) => continue, + let entry: UnvalidatedProtoEntry = match allow_long_proto_names { + true => match UnvalidatedProtoEntry::from_str_any_len(datum.as_str()) { + Ok(n) => n, + Err(_) => continue}, + false => match datum.parse() { + Ok(n) => n, + Err(_) => continue}, }; proto_entries.push(entry); } diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs index 514aeffc5..d6ed2739f 100644 --- a/src/rust/protover/protover.rs +++ b/src/rust/protover/protover.rs @@ -28,6 +28,9 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha"; /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND` const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16);
+/// The maximum size an `UnknownProtocol`'s name may be. +pub(crate) const MAX_PROTOCOL_NAME_LENGTH: usize = 100; + /// Known subprotocols in Tor. Indicates which subprotocol a relay supports. /// /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t` @@ -90,6 +93,18 @@ impl FromStr for UnknownProtocol { type Err = ProtoverError;
fn from_str(s: &str) -> Result<Self, Self::Err> { + if s.len() <= MAX_PROTOCOL_NAME_LENGTH { + Ok(UnknownProtocol(s.to_string())) + } else { + Err(ProtoverError::ExceedsNameLimit) + } + } +} + +impl UnknownProtocol { + /// Create an `UnknownProtocol`, ignoring whether or not it + /// exceeds MAX_PROTOCOL_NAME_LENGTH. + fn from_str_any_len(s: &str) -> Result<Self, ProtoverError> { Ok(UnknownProtocol(s.to_string())) } } @@ -417,6 +432,49 @@ impl UnvalidatedProtoEntry { }; supported_versions.iter().any(|v| v.1 >= *vers) } + + /// Split a string containing (potentially) several protocols and their + /// versions into a `Vec` of tuples of string in `(protocol, versions)` + /// form. + /// + /// # Inputs + /// + /// A &str in the form `"Link=3-4 Cons=5"`. + /// + /// # Returns + /// + /// A `Result` whose `Ok` variant is a `Vec<(&str, &str)>` of `(protocol, + /// versions)`, or whose `Err` variant is a `ProtoverError`. + /// + /// # Errors + /// + /// This will error with a `ProtoverError::Unparseable` if any of the + /// following are true: + /// + /// * If a protocol name is an empty string, e.g. `"Cons=1,3 =3-5"`. + /// * If a protocol name cannot be parsed as utf-8. + /// * If the version numbers are an empty string, e.g. `"Cons="`. + fn parse_protocol_and_version_str<'a>(protocol_string: &'a str) + -> Result<Vec<(&'a str, &'a str)>, ProtoverError> + { + let mut protovers: Vec<(&str, &str)> = Vec::new(); + + for subproto in protocol_string.split(' ') { + let mut parts = subproto.splitn(2, '='); + + let name = match parts.next() { + Some("") => return Err(ProtoverError::Unparseable), + Some(n) => n, + None => return Err(ProtoverError::Unparseable), + }; + let vers = match parts.next() { + Some(n) => n, + None => return Err(ProtoverError::Unparseable), + }; + protovers.push((name, vers)); + } + Ok(protovers) + } }
impl FromStr for UnvalidatedProtoEntry { @@ -449,19 +507,10 @@ impl FromStr for UnvalidatedProtoEntry { /// * If the version string is malformed. See `impl FromStr for ProtoSet`. fn from_str(protocol_string: &str) -> Result<UnvalidatedProtoEntry, ProtoverError> { let mut parsed: UnvalidatedProtoEntry = UnvalidatedProtoEntry::default(); + let parts: Vec<(&str, &str)> = + UnvalidatedProtoEntry::parse_protocol_and_version_str(protocol_string)?;
- for subproto in protocol_string.split(' ') { - let mut parts = subproto.splitn(2, '='); - - let name = match parts.next() { - Some("") => return Err(ProtoverError::Unparseable), - Some(n) => n, - None => return Err(ProtoverError::Unparseable), - }; - let vers = match parts.next() { - Some(n) => n, - None => return Err(ProtoverError::Unparseable), - }; + for &(name, vers) in parts.iter() { let versions = ProtoSet::from_str(vers)?; let protocol = UnknownProtocol::from_str(name)?;
@@ -471,6 +520,26 @@ impl FromStr for UnvalidatedProtoEntry { } }
+impl UnvalidatedProtoEntry { + /// Create an `UnknownProtocol`, ignoring whether or not it + /// exceeds MAX_PROTOCOL_NAME_LENGTH. + pub(crate) fn from_str_any_len(protocol_string: &str) + -> Result<UnvalidatedProtoEntry, ProtoverError> + { + let mut parsed: UnvalidatedProtoEntry = UnvalidatedProtoEntry::default(); + let parts: Vec<(&str, &str)> = + UnvalidatedProtoEntry::parse_protocol_and_version_str(protocol_string)?; + + for &(name, vers) in parts.iter() { + let versions = ProtoSet::from_str(vers)?; + let protocol = UnknownProtocol::from_str_any_len(name)?; + + parsed.insert(protocol, versions); + } + Ok(parsed) + } +} + /// Pretend a `ProtoEntry` is actually an `UnvalidatedProtoEntry`. impl From<ProtoEntry> for UnvalidatedProtoEntry { fn from(proto_entry: ProtoEntry) -> UnvalidatedProtoEntry {