diff --git a/crates/tor-bytes/semver.md b/crates/tor-bytes/semver.md new file mode 100644 index 000000000..2d979ebc8 --- /dev/null +++ b/crates/tor-bytes/semver.md @@ -0,0 +1 @@ +MODIFIED: Error::BadMessage is deprecated, Error::InvalidMessage is new. diff --git a/crates/tor-bytes/src/err.rs b/crates/tor-bytes/src/err.rs index e35097a48..daf0eab5b 100644 --- a/crates/tor-bytes/src/err.rs +++ b/crates/tor-bytes/src/err.rs @@ -1,5 +1,7 @@ //! Internal: Declare an Error type for tor-bytes +use std::borrow::Cow; + use thiserror::Error; use tor_error::{into_internal, Bug}; @@ -26,8 +28,13 @@ pub enum Error { BadLengthValue, /// An attempt to parse an object failed for some reason related to its /// contents. + #[deprecated(since = "0.6.2", note = "Use InvalidMessage instead.")] #[error("Bad object: {0}")] BadMessage(&'static str), + /// An attempt to parse an object failed for some reason related to its + /// contents. + #[error("Bad object: {0}")] + InvalidMessage(Cow<'static, str>), /// A parsing error that should never happen. /// /// We use this one in lieu of calling assert() and expect() and @@ -42,7 +49,9 @@ impl PartialEq for Error { match (self, other) { (Truncated, Truncated) => true, (ExtraneousBytes, ExtraneousBytes) => true, + #[allow(deprecated)] (BadMessage(a), BadMessage(b)) => a == b, + (InvalidMessage(a), InvalidMessage(b)) => a == b, (BadLengthValue, BadLengthValue) => true, // notably, this means that an internal error is equal to nothing, not even itself. (_, _) => false, diff --git a/crates/tor-bytes/src/impls.rs b/crates/tor-bytes/src/impls.rs index fa5104e97..ee9c8a413 100644 --- a/crates/tor-bytes/src/impls.rs +++ b/crates/tor-bytes/src/impls.rs @@ -182,7 +182,7 @@ mod ed25519_impls { fn take_from(b: &mut Reader<'_>) -> Result { let bytes = b.take(32)?; Self::from_bytes(array_ref![bytes, 0, 32]) - .map_err(|_| Error::BadMessage("Couldn't decode Ed25519 public key")) + .map_err(|_| Error::InvalidMessage("Couldn't decode Ed25519 public key".into())) } } @@ -208,7 +208,7 @@ mod ed25519_impls { fn take_from(b: &mut Reader<'_>) -> Result { let bytes = b.take(64)?; Self::from_bytes(array_ref![bytes, 0, 64]) - .map_err(|_| Error::BadMessage("Couldn't decode Ed25519 signature.")) + .map_err(|_| Error::InvalidMessage("Couldn't decode Ed25519 signature.".into())) } } } diff --git a/crates/tor-cell/src/chancell/msg.rs b/crates/tor-cell/src/chancell/msg.rs index 7943e7c4a..e4d10f7b8 100644 --- a/crates/tor-cell/src/chancell/msg.rs +++ b/crates/tor-cell/src/chancell/msg.rs @@ -128,7 +128,9 @@ impl Body for Vpadding { impl Readable for Vpadding { fn take_from(r: &mut Reader<'_>) -> Result { if r.remaining() > std::u16::MAX as usize { - return Err(Error::BadMessage("Too many bytes in VPADDING cell")); + return Err(Error::InvalidMessage( + "Too many bytes in VPADDING cell".into(), + )); } Ok(Vpadding { len: r.remaining() as u16, @@ -821,8 +823,8 @@ impl Readable for PaddingNegotiate { fn take_from(r: &mut Reader<'_>) -> Result { let v = r.take_u8()?; if v != 0 { - return Err(Error::BadMessage( - "Unrecognized padding negotiation version", + return Err(Error::InvalidMessage( + "Unrecognized padding negotiation version".into(), )); } let command = r.take_u8()?.into(); diff --git a/crates/tor-cell/src/relaycell.rs b/crates/tor-cell/src/relaycell.rs index 298178ec2..d6d53cd7e 100644 --- a/crates/tor-cell/src/relaycell.rs +++ b/crates/tor-cell/src/relaycell.rs @@ -293,7 +293,9 @@ impl RelayCell { r.advance(4)?; // digest let len = r.take_u16()? as usize; if r.remaining() < len { - return Err(Error::BadMessage("Insufficient data in relay cell")); + return Err(Error::InvalidMessage( + "Insufficient data in relay cell".into(), + )); } r.truncate(len); let msg = M::decode_from_reader(cmd, r)?; diff --git a/crates/tor-cell/src/relaycell/extend.rs b/crates/tor-cell/src/relaycell/extend.rs index f541f2f75..1041bbece 100644 --- a/crates/tor-cell/src/relaycell/extend.rs +++ b/crates/tor-cell/src/relaycell/extend.rs @@ -91,16 +91,16 @@ impl Readable for NtorV3Extension { Ok(match tag { NtorV3ExtensionType::CC_REQUEST => { if len != 0 { - return Err(tor_bytes::Error::BadMessage( - "invalid length for RequestCongestionControl", + return Err(tor_bytes::Error::InvalidMessage( + "invalid length for RequestCongestionControl".into(), )); } NtorV3Extension::RequestCongestionControl } NtorV3ExtensionType::CC_RESPONSE => { if len != 1 { - return Err(tor_bytes::Error::BadMessage( - "invalid length for AckCongestionControl", + return Err(tor_bytes::Error::InvalidMessage( + "invalid length for AckCongestionControl".into(), )); } let sendme_inc = reader.take_u8()?; diff --git a/crates/tor-cell/src/relaycell/msg.rs b/crates/tor-cell/src/relaycell/msg.rs index e32d6d8f1..f81bcf0d8 100644 --- a/crates/tor-cell/src/relaycell/msg.rs +++ b/crates/tor-cell/src/relaycell/msg.rs @@ -210,7 +210,7 @@ impl Body for Begin { let a = r.take_until(b']')?; let colon = r.take_u8()?; if colon != b':' { - return Err(Error::BadMessage("missing port in begin cell")); + return Err(Error::InvalidMessage("missing port in begin cell".into())); } a } else { @@ -222,15 +222,17 @@ impl Body for Begin { let flags = if r.remaining() >= 4 { r.take_u32()? } else { 0 }; if !addr.is_ascii() { - return Err(Error::BadMessage("target address in begin cell not ascii")); + return Err(Error::InvalidMessage( + "target address in begin cell not ascii".into(), + )); } let port = std::str::from_utf8(port) - .map_err(|_| Error::BadMessage("port in begin cell not utf8"))?; + .map_err(|_| Error::InvalidMessage("port in begin cell not utf8".into()))?; let port = port .parse() - .map_err(|_| Error::BadMessage("port in begin cell not a valid port"))?; + .map_err(|_| Error::InvalidMessage("port in begin cell not a valid port".into()))?; Ok(Begin { addr: addr.into(), @@ -519,7 +521,9 @@ impl Body for Connected { let ipv4 = r.take_u32()?; let addr = if ipv4 == 0 { if r.take_u8()? != 6 { - return Err(Error::BadMessage("Invalid address type in CONNECTED cell")); + return Err(Error::InvalidMessage( + "Invalid address type in CONNECTED cell".into(), + )); } IpAddr::V6(r.extract()?) } else { @@ -600,7 +604,7 @@ impl Body for Sendme { Some(r.take(dlen as usize)?.into()) } _ => { - return Err(Error::BadMessage("Unrecognized SENDME version.")); + return Err(Error::InvalidMessage("Unrecognized SENDME version.".into())); } } }; @@ -955,7 +959,9 @@ impl Readable for ResolvedVal { let len = r.take_u8()? as usize; if let Some(expected_len) = res_len(tp) { if len != expected_len { - return Err(Error::BadMessage("Wrong length for RESOLVED answer")); + return Err(Error::InvalidMessage( + "Wrong length for RESOLVED answer".into(), + )); } } Ok(match tp { diff --git a/crates/tor-cell/src/relaycell/onion_service.rs b/crates/tor-cell/src/relaycell/onion_service.rs index 18c029161..da9a495b3 100644 --- a/crates/tor-cell/src/relaycell/onion_service.rs +++ b/crates/tor-cell/src/relaycell/onion_service.rs @@ -331,7 +331,9 @@ impl Introduce { fn decode_from_reader(r: &mut Reader<'_>) -> Result { let legacy_key_id: RsaIdentity = r.extract()?; if !legacy_key_id.is_zero() { - return Err(BytesError::BadMessage("legacy key id in Introduce1.")); + return Err(BytesError::InvalidMessage( + "legacy key id in Introduce1.".into(), + )); } let auth_key_type = r.take_u8()?.into(); let auth_key_len = r.take_u16()?; diff --git a/crates/tor-cell/src/relaycell/udp.rs b/crates/tor-cell/src/relaycell/udp.rs index 11ff18792..eb2496a3c 100644 --- a/crates/tor-cell/src/relaycell/udp.rs +++ b/crates/tor-cell/src/relaycell/udp.rs @@ -98,7 +98,7 @@ impl Readable for Address { } T_IPV4 => Self::Ipv4(r.extract()?), T_IPV6 => Self::Ipv6(r.extract()?), - _ => return Err(Error::BadMessage("Invalid address type")), + _ => return Err(Error::InvalidMessage("Invalid address type".into())), }) }) } @@ -133,10 +133,10 @@ impl FromStr for Address { Ok(Self::Ipv6(ipv6)) } else { if s.len() > MAX_HOSTNAME_LEN { - return Err(Error::BadMessage("Hostname too long")); + return Err(Error::InvalidMessage("Hostname too long".into())); } if s.contains('\0') { - return Err(Error::BadMessage("Nul byte not permitted")); + return Err(Error::InvalidMessage("Nul byte not permitted".into())); } let mut addr = s.to_string(); @@ -229,11 +229,11 @@ impl msg::Body for ConnectedUdp { fn decode_from_reader(r: &mut Reader<'_>) -> Result { let our_address: AddressPort = r.extract()?; if our_address.addr.is_hostname() { - return Err(Error::BadMessage("Our address is a Hostname")); + return Err(Error::InvalidMessage("Our address is a Hostname".into())); } let their_address: AddressPort = r.extract()?; if their_address.addr.is_hostname() { - return Err(Error::BadMessage("Their address is a Hostname")); + return Err(Error::InvalidMessage("Their address is a Hostname".into())); } Ok(Self { diff --git a/crates/tor-cell/src/restrict.rs b/crates/tor-cell/src/restrict.rs index e7024dfd6..bd6b341b5 100644 --- a/crates/tor-cell/src/restrict.rs +++ b/crates/tor-cell/src/restrict.rs @@ -158,7 +158,7 @@ macro_rules! restricted_msg { )? // TODO: This message is too terse! This message type should maybe take a Cow? #[allow(unreachable_patterns)] // This is unreachable if we had an Unrecognized variant above. - _ => return Err($crate::restrict::tor_bytes::Error::BadMessage("Unexpected command")), + _ => return Err($crate::restrict::tor_bytes::Error::InvalidMessage("Unexpected command".into())), }) } } diff --git a/crates/tor-cell/tests/test_relaycell.rs b/crates/tor-cell/tests/test_relaycell.rs index 3e9e4b065..404056a72 100644 --- a/crates/tor-cell/tests/test_relaycell.rs +++ b/crates/tor-cell/tests/test_relaycell.rs @@ -91,7 +91,9 @@ fn test_cells() { let m = decode("02 0000 9999 12345678 01f3 6e6565642d746f2d6b6e6f77 00000000"); assert_eq!( AnyRelayCell::decode(m).err(), - Some(Error::BadMessage("Insufficient data in relay cell")) + Some(Error::InvalidMessage( + "Insufficient data in relay cell".into() + )) ); // check accessors. @@ -173,7 +175,10 @@ fn test_address() { let hostname = "a".repeat(256); let addr = Address::from_str(hostname.as_str()); assert!(addr.is_err()); - assert_eq!(addr.err(), Some(Error::BadMessage("Hostname too long"))); + assert_eq!( + addr.err(), + Some(Error::InvalidMessage("Hostname too long".into())) + ); // Some Unicode emojis (go Gen-Z!). let hostname = "👍️👍️👍️"; @@ -187,6 +192,6 @@ fn test_address() { assert!(addr.is_err()); assert_eq!( addr.err(), - Some(Error::BadMessage("Nul byte not permitted")) + Some(Error::InvalidMessage("Nul byte not permitted".into())) ); } diff --git a/crates/tor-cell/tests/testvec_chanmsg.rs b/crates/tor-cell/tests/testvec_chanmsg.rs index 51f17d1e7..4180f6d9b 100644 --- a/crates/tor-cell/tests/testvec_chanmsg.rs +++ b/crates/tor-cell/tests/testvec_chanmsg.rs @@ -406,6 +406,6 @@ fn test_padding_negotiate() { assert_eq!( decode_err(cmd, "90 0303", true), - BytesError::BadMessage("Unrecognized padding negotiation version") + BytesError::InvalidMessage("Unrecognized padding negotiation version".into()) ); } diff --git a/crates/tor-cell/tests/testvec_relaymsg.rs b/crates/tor-cell/tests/testvec_relaymsg.rs index b38ab500b..4b5062e67 100644 --- a/crates/tor-cell/tests/testvec_relaymsg.rs +++ b/crates/tor-cell/tests/testvec_relaymsg.rs @@ -101,14 +101,14 @@ fn test_begin() { msg_error( cmd, "5b3a3a5d21", // [::]! - BytesError::BadMessage("missing port in begin cell"), + BytesError::InvalidMessage("missing port in begin cell".into()), ); // hand-generated failure case: not ascii. msg_error( cmd, "746f7270726f6a656374e284a22e6f72673a34343300", // torproject™.org:443 - BytesError::BadMessage("target address in begin cell not ascii"), + BytesError::InvalidMessage("target address in begin cell not ascii".into()), ); // failure on construction: bad address. @@ -151,7 +151,7 @@ fn test_connected() { msg_error( cmd, "00000000 07 20010db8 00000000 00000000 00001122 00000E10", - BytesError::BadMessage("Invalid address type in CONNECTED cell"), + BytesError::InvalidMessage("Invalid address type in CONNECTED cell".into()), ); } @@ -418,7 +418,7 @@ fn test_resolved() { msg_error( cmd, "04 03 010203 00000001", - BytesError::BadMessage("Wrong length for RESOLVED answer"), + BytesError::InvalidMessage("Wrong length for RESOLVED answer".into()), ); } @@ -564,7 +564,7 @@ fn test_connect_udp() { msg_error( cmd, "00000000 07 04 01020304", - BytesError::BadMessage("Invalid address type"), + BytesError::InvalidMessage("Invalid address type".into()), ); // A zero length address with and without hostname payload. @@ -608,14 +608,14 @@ fn test_connected_udp() { cmd, "01 04 01020304 0050 04 04 05060708 0050", - BytesError::BadMessage("Our address is a Hostname"), + BytesError::InvalidMessage("Our address is a Hostname".into()), ); // Invalid their_address msg_error( cmd, "04 04 01020304 0050 01 04 05060708 0050", - BytesError::BadMessage("Their address is a Hostname"), + BytesError::InvalidMessage("Their address is a Hostname".into()), ); } @@ -773,7 +773,7 @@ fn test_introduce() { 02 0004 00010203 00 01090804", - BytesError::BadMessage("legacy key id in Introduce1."), + BytesError::InvalidMessage("legacy key id in Introduce1.".into()), ); } // TODO: need to add tests for: diff --git a/crates/tor-cert/src/lib.rs b/crates/tor-cert/src/lib.rs index 8175a5545..52aaa18e9 100644 --- a/crates/tor-cert/src/lib.rs +++ b/crates/tor-cert/src/lib.rs @@ -301,13 +301,15 @@ impl Readable for CertExt { Ok(match ext_type { ExtType::SIGNED_WITH_ED25519_KEY => CertExt::SignedWithEd25519(SignedWithEd25519Ext { - pk: ed25519::Ed25519Identity::from_bytes(body) - .ok_or(BytesError::BadMessage("wrong length on Ed25519 key"))?, + pk: ed25519::Ed25519Identity::from_bytes(body).ok_or( + BytesError::InvalidMessage("wrong length on Ed25519 key".into()), + )?, }), _ => { if (flags & 1) != 0 { - return Err(BytesError::BadMessage( - "unrecognized certificate extension, with 'affects_validation' flag set.", + return Err(BytesError::InvalidMessage( + "unrecognized certificate extension, with 'affects_validation' flag set." + .into(), )); } CertExt::Unrecognized(UnrecognizedExt { @@ -335,7 +337,9 @@ impl Ed25519Cert { if v != 1 { // This would be something other than a "v1" certificate. We don't // understand those. - return Err(BytesError::BadMessage("Unrecognized certificate version")); + return Err(BytesError::InvalidMessage( + "Unrecognized certificate version".into(), + )); } let cert_type = r.take_u8()?.into(); let exp_hours = r.take_u32()?; @@ -591,8 +595,8 @@ mod test { assert!(e.is_err()); assert_eq!( e.err().unwrap(), - BytesError::BadMessage( - "unrecognized certificate extension, with 'affects_validation' flag set." + BytesError::InvalidMessage( + "unrecognized certificate extension, with 'affects_validation' flag set.".into() ) ); diff --git a/crates/tor-cert/src/rsa.rs b/crates/tor-cert/src/rsa.rs index 29bb30bcf..7986d913f 100644 --- a/crates/tor-cert/src/rsa.rs +++ b/crates/tor-cert/src/rsa.rs @@ -82,7 +82,9 @@ impl ExternallySigned> for UncheckedRsaCrosscert { fn is_well_signed(&self, k: &Self::Key) -> Result<(), Self::Error> { k.verify(&self.0.digest[..], &self.0.signature[..]) .map_err(|_| { - tor_bytes::Error::BadMessage("Invalid signature on RSA->Ed identity crosscert") + tor_bytes::Error::InvalidMessage( + "Invalid signature on RSA->Ed identity crosscert".into(), + ) })?; Ok(()) } diff --git a/crates/tor-cert/tests/invalid_certs.rs b/crates/tor-cert/tests/invalid_certs.rs index ee9236d3a..72015036b 100644 --- a/crates/tor-cert/tests/invalid_certs.rs +++ b/crates/tor-cert/tests/invalid_certs.rs @@ -17,7 +17,7 @@ fn cant_parse() { assert_eq!( decode_err(&hex!("03")), - Error::BadMessage("Unrecognized certificate version") + Error::InvalidMessage("Unrecognized certificate version".into()) ); assert_eq!( @@ -30,7 +30,7 @@ fn cant_parse() { FF1A5203FA27F86EF7528D89A0845D2520166E340754FFEA2AAE0F612B7CE5DA 094A0236CDAC45034B0B6842C18E7F6B51B93A3CF7E60663B8AD061C30A62602" )), - Error::BadMessage("wrong length on Ed25519 key") + Error::InvalidMessage("wrong length on Ed25519 key".into()) ); assert_eq!( @@ -43,8 +43,8 @@ fn cant_parse() { FF1A5203FA27F86EF7528D89A0845D2520166E340754FFEA2AAE0F612B7CE5DA 094A0236CDAC45034B0B6842C18E7F6B51B93A3CF7E60663B8AD061C30A62602" )), - Error::BadMessage( - "unrecognized certificate extension, with 'affects_validation' flag set." + Error::InvalidMessage( + "unrecognized certificate extension, with 'affects_validation' flag set.".into() ) ); } diff --git a/crates/tor-socksproto/src/handshake.rs b/crates/tor-socksproto/src/handshake.rs index 8224e6ac5..b5d25e6df 100644 --- a/crates/tor-socksproto/src/handshake.rs +++ b/crates/tor-socksproto/src/handshake.rs @@ -41,18 +41,20 @@ impl Readable for SocksAddr { let hlen = r.take_u8()?; let hostname = r.take(hlen as usize)?; let hostname = std::str::from_utf8(hostname) - .map_err(|_| BytesError::BadMessage("bad utf8 on hostname"))? + .map_err(|_| BytesError::InvalidMessage("bad utf8 on hostname".into()))? .to_string(); let hostname = hostname .try_into() - .map_err(|_| BytesError::BadMessage("hostname too long"))?; + .map_err(|_| BytesError::InvalidMessage("hostname too long".into()))?; Ok(SocksAddr::Hostname(hostname)) } 4 => { let ip6: std::net::Ipv6Addr = r.extract()?; Ok(SocksAddr::Ip(ip6.into())) } - _ => Err(BytesError::BadMessage("unrecognized address type.")), + _ => Err(BytesError::InvalidMessage( + "unrecognized address type.".into(), + )), } } } diff --git a/crates/tor-socksproto/src/handshake/proxy.rs b/crates/tor-socksproto/src/handshake/proxy.rs index 43ff952d1..1a275fc40 100644 --- a/crates/tor-socksproto/src/handshake/proxy.rs +++ b/crates/tor-socksproto/src/handshake/proxy.rs @@ -124,7 +124,7 @@ impl SocksProxyHandshake { .to_string(); let hostname = hostname .try_into() - .map_err(|_| BytesError::BadMessage("hostname too long"))?; + .map_err(|_| BytesError::InvalidMessage("hostname too long".into()))?; SocksAddr::Hostname(hostname) } else { let ip4: std::net::Ipv4Addr = ip.into();