Merge branch 'ct_key_zero' into 'main'

Add a new constant-time is_zero() check for RsaIdentity

See merge request tpo/core/arti!735

crates/tor-cell/src/relaycell/onion_service.rs

@@ -7,6 +7,7 @@ use super::msg;
 use caret::caret_int;
 use tor_bytes::{EncodeError, EncodeResult, Error as BytesError, Readable, Result, Writeable};
 use tor_bytes::{Reader, Writer};
+use tor_llcrypto::pk::rsa::RsaIdentity;
 use tor_units::BoundedInt32;
 
 caret_int! {
@@ -269,8 +270,8 @@ impl msg::Body for Introduce1 {
         msg::RelayMsg::Introduce1(self)
     }
     fn decode_from_reader(r: &mut Reader<'_>) -> Result<Self> {
-        let legacy_key_id: [u8; 20] = r.extract()?;
-        if legacy_key_id.iter().any(|b| *b != 0_u8) {
+        let legacy_key_id: RsaIdentity = r.extract()?;
+        if !legacy_key_id.is_zero() {
             return Err(BytesError::BadMessage("legacy key id in Introduce1."));
         }
         let auth_key_type = r.take_u8()?.into();

crates/tor-llcrypto/semver.md

@@ -0,0 +1,2 @@
+MODIFIED: New is_zero API on RsaIdentity.
+

crates/tor-llcrypto/src/pk/rsa.rs

@@ -159,6 +159,17 @@ impl RsaIdentity {
             Ok(()) => Some(RsaIdentity::from(array)),
         }
     }
+
+    /// Return true if this `RsaIdentity` is composed entirely of zero-valued
+    /// bytes.
+    ///
+    /// Such all-zero values should not be used internally, since they are not
+    /// the ID of any valid key.  Instead, they are used in some places in the
+    /// Tor protocols.
+    pub fn is_zero(&self) -> bool {
+        // We do a constant-time comparison to avoid side-channels.
+        self.id.ct_eq(&[0; RSA_ID_LEN]).into()
+    }
 }
 
 impl From<[u8; 20]> for RsaIdentity {

crates/tor-llcrypto/tests/misc.rs

@@ -63,6 +63,26 @@ fn test_wrong_hex_rsa_ids() {
     assert!(RsaIdentity::from_hex("listen carefully, spider of destiny  -FZ").is_none());
 }
 
+#[test]
+fn test_rsa_is_zero() {
+    use ll::pk::rsa::RsaIdentity;
+    assert!(
+        RsaIdentity::from_hex("0000000000000000000000000000000000000000")
+            .unwrap()
+            .is_zero()
+    );
+    assert!(
+        !RsaIdentity::from_hex("000000000000000000000000000000000000000F")
+            .unwrap()
+            .is_zero()
+    );
+    assert!(
+        !RsaIdentity::from_hex("F000000000000000000000000000000000000000")
+            .unwrap()
+            .is_zero()
+    );
+}
+
 // TODO: Proper tests for RSA keys
 
 #[test]