From b6452b481215ce0963123630234b01be0c440092 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 4 Mar 2022 08:55:34 -0500 Subject: [PATCH 1/3] Add a from_hex method for RsaIdentity. We perform this operation in a bunch of places, and most of them use hex::decode(). That's not great, since hex::decode() has to do heap allocation. This implementation uses hex::decode_to_slice(), which should be faster. (In the future we might choose to use one of the faster hex implementations, but I'm hoping that this change will be sufficient to get hex decoding out of our profiles.) Part of #377. --- crates/tor-llcrypto/src/pk/rsa.rs | 15 ++++++++++++--- crates/tor-llcrypto/tests/misc.rs | 15 +++++++++++++-- doc/semver_status.md | 4 ++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/tor-llcrypto/src/pk/rsa.rs b/crates/tor-llcrypto/src/pk/rsa.rs index cf0e8454e..4afa1dca1 100644 --- a/crates/tor-llcrypto/src/pk/rsa.rs +++ b/crates/tor-llcrypto/src/pk/rsa.rs @@ -94,9 +94,8 @@ impl<'de> serde::Deserialize<'de> for RsaIdentity { where E: serde::de::Error, { - let bytes = hex::decode(s).map_err(E::custom)?; - RsaIdentity::from_bytes(&bytes) - .ok_or_else(|| E::custom("wrong length for RSA identity")) + RsaIdentity::from_hex(s) + .ok_or_else(|| E::custom("wrong encoding for RSA identity")) } } @@ -151,6 +150,16 @@ impl RsaIdentity { None } } + /// Decode an `RsaIdentity` from a hexadecimal string. + /// + /// The string must have no spaces, or any extra characters. + pub fn from_hex(s: &str) -> Option { + let mut array = [0_u8; 20]; + match hex::decode_to_slice(s, &mut array) { + Err(_) => None, + Ok(()) => Some(RsaIdentity::from(array)), + } + } } impl From<[u8; 20]> for RsaIdentity { diff --git a/crates/tor-llcrypto/tests/misc.rs b/crates/tor-llcrypto/tests/misc.rs index ff63791d0..4caf7dc14 100644 --- a/crates/tor-llcrypto/tests/misc.rs +++ b/crates/tor-llcrypto/tests/misc.rs @@ -41,8 +41,7 @@ fn test_ed25519_identity() { fn test_rsa_formatting() { use ll::pk::rsa::RsaIdentity; - let id = hex::decode("5696ab38cb3852afa476a5c07b2d4788963d5567").unwrap(); - let id = RsaIdentity::from_bytes(&id).unwrap(); + let id = RsaIdentity::from_hex("5696ab38cb3852afa476a5c07b2d4788963d5567").unwrap(); assert_eq!( format!("<<{}>>", id), @@ -54,6 +53,18 @@ fn test_rsa_formatting() { ); } +#[test] +fn test_wrong_hex_rsa_ids() { + use ll::pk::rsa::RsaIdentity; + assert!(RsaIdentity::from_hex("5696ab38cb3852afa476a5c07b2d4788963d5567").is_some()); + assert!(RsaIdentity::from_hex("5696AB38CB3852AFA476A5C07b2d4788963d5567").is_some()); + assert!(RsaIdentity::from_hex("5696ab38cb3852afa476a5c07b2d4788963d").is_none()); + assert!(RsaIdentity::from_hex("5696ab38cb3852afa476a5c07b2d4788963d5567A").is_none()); + assert!(RsaIdentity::from_hex("5696ab38cb3852afa476a5c07b2d4788963d5567AB").is_none()); + assert!(RsaIdentity::from_hex("").is_none()); + assert!(RsaIdentity::from_hex("listen carefully, spider of destiny -FZ").is_none()); +} + // TODO: Proper tests for RSA keys #[test] diff --git a/doc/semver_status.md b/doc/semver_status.md index f25ea8112..26568c441 100644 --- a/doc/semver_status.md +++ b/doc/semver_status.md @@ -19,6 +19,10 @@ We can delete older sections here after we bump the releases. ## Since Arti 0.1.0 +tor-llcrypto: + + new-api: Added RsaIdentity::from_hex(). + arti-client: api-break (experimental only): changed circmgr() and dirmgr() to return From caf372ac0f653617070c635f7daf20c8f070b25a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 4 Mar 2022 09:05:04 -0500 Subject: [PATCH 2/3] tor-netdoc: Use RsaIdentity::from_hex() in critical path. This commit changes the main parsing code for RsaIdentity in tor-netdoc, and . Previously, parse_hex_ident was something like 10% of our startup CPU time; now it's only like ~2%. (Still not perfect, but way better.) Closes #377. --- crates/tor-netdoc/src/types/misc.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/tor-netdoc/src/types/misc.rs b/crates/tor-netdoc/src/types/misc.rs index 69bbaa550..9375ac217 100644 --- a/crates/tor-netdoc/src/types/misc.rs +++ b/crates/tor-netdoc/src/types/misc.rs @@ -353,12 +353,7 @@ mod fingerprint { /// Helper: parse an identity from a hexadecimal string fn parse_hex_ident(s: &str) -> Result { - let bytes = hex::decode(s).map_err(|_| { - EK::BadArgument - .at_pos(Pos::at(s)) - .with_msg("invalid hexadecimal in fingerprint") - })?; - RsaIdentity::from_bytes(&bytes).ok_or_else(|| { + RsaIdentity::from_hex(s).ok_or_else(|| { EK::BadArgument .at_pos(Pos::at(s)) .with_msg("wrong length on fingerprint") From c23dad8c2d0eb69016995d4f4246dfd6aed5db1e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 4 Mar 2022 09:23:26 -0500 Subject: [PATCH 3/3] Use RsaIdentity::from_hex() and hex::decode_to_slice in more places These aren't critical-path, but they do make the code a little nicer. --- crates/tor-dirmgr/src/authority.rs | 5 ++--- crates/tor-dirmgr/src/config.rs | 4 +--- crates/tor-dirmgr/src/state.rs | 3 +-- crates/tor-dirmgr/src/storage/sqlite.rs | 12 ++++-------- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/crates/tor-dirmgr/src/authority.rs b/crates/tor-dirmgr/src/authority.rs index 8c5b432fa..475bce84c 100644 --- a/crates/tor-dirmgr/src/authority.rs +++ b/crates/tor-dirmgr/src/authority.rs @@ -59,9 +59,8 @@ impl Authority { pub(crate) fn default_authorities() -> Vec { /// Build an authority; panic if input is bad. fn auth(name: &str, key: &str) -> Authority { - let v3ident = hex::decode(key).expect("Built-in authority identity had bad hex!?"); - let v3ident = RsaIdentity::from_bytes(&v3ident) - .expect("Built-in authority identity had wrong length!?"); + let v3ident = + RsaIdentity::from_hex(key).expect("Built-in authority identity had bad hex!?"); AuthorityBuilder::new() .name(name) .v3ident(v3ident) diff --git a/crates/tor-dirmgr/src/config.rs b/crates/tor-dirmgr/src/config.rs index aab3cac75..536d6bf4d 100644 --- a/crates/tor-dirmgr/src/config.rs +++ b/crates/tor-dirmgr/src/config.rs @@ -331,9 +331,7 @@ mod fallbacks { pub(crate) fn default_fallbacks() -> Vec { /// Build a fallback directory; panic if input is bad. fn fallback(rsa: &str, ed: &str, ports: &[&str]) -> FallbackDir { - let rsa = hex::decode(rsa).expect("Bad hex in built-in fallback list"); - let rsa = - RsaIdentity::from_bytes(&rsa).expect("Wrong length in built-in fallback list"); + let rsa = RsaIdentity::from_hex(rsa).expect("Bad hex in built-in fallback list"); let ed = base64::decode_config(ed, base64::STANDARD_NO_PAD) .expect("Bad hex in built-in fallback list"); let ed = diff --git a/crates/tor-dirmgr/src/state.rs b/crates/tor-dirmgr/src/state.rs index a00d3f6b3..dcd18d5d9 100644 --- a/crates/tor-dirmgr/src/state.rs +++ b/crates/tor-dirmgr/src/state.rs @@ -1005,8 +1005,7 @@ mod test { datetime!(2020-08-07 12:42:45 UTC).into() } fn rsa(s: &str) -> RsaIdentity { - let k = hex::decode(s).unwrap(); - RsaIdentity::from_bytes(&k[..]).unwrap() + RsaIdentity::from_hex(s).unwrap() } fn test_authorities() -> Vec { fn a(s: &str) -> Authority { diff --git a/crates/tor-dirmgr/src/storage/sqlite.rs b/crates/tor-dirmgr/src/storage/sqlite.rs index 2c4242929..0648b614e 100644 --- a/crates/tor-dirmgr/src/storage/sqlite.rs +++ b/crates/tor-dirmgr/src/storage/sqlite.rs @@ -608,20 +608,16 @@ impl Drop for Unlinker { /// Convert a hexadecimal sha3-256 digest from the database into an array. fn digest_from_hex(s: &str) -> Result<[u8; 32]> { - hex::decode(s) - .map_err(Error::BadHexInCache)? - .try_into() - .map_err(|_| Error::CacheCorruption("Invalid digest in database")) + let mut bytes = [0_u8; 32]; + hex::decode_to_slice(s, &mut bytes[..]).map_err(Error::BadHexInCache)?; + Ok(bytes) } /// Convert a hexadecimal sha3-256 "digest string" as used in the /// digest column from the database into an array. fn digest_from_dstr(s: &str) -> Result<[u8; 32]> { if let Some(stripped) = s.strip_prefix("sha3-256-") { - hex::decode(stripped) - .map_err(Error::BadHexInCache)? - .try_into() - .map_err(|_| Error::CacheCorruption("Invalid digest in database")) + digest_from_hex(stripped) } else { Err(Error::CacheCorruption("Invalid digest in database")) }