diff --git a/Cargo.lock b/Cargo.lock index eeb5b2b1e..3ecca9915 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4586,6 +4586,7 @@ dependencies = [ "serde_with", "signature 1.6.4", "smallvec", + "subtle", "thiserror", "time", "tinystr", diff --git a/crates/tor-hsclient/src/connect.rs b/crates/tor-hsclient/src/connect.rs index c9e3f459a..39dcfa71b 100644 --- a/crates/tor-hsclient/src/connect.rs +++ b/crates/tor-hsclient/src/connect.rs @@ -420,6 +420,8 @@ impl<'c, R: Runtime, M: MocksForConnect> Context<'c, R, M> { /// Returns an error if no valid descriptor could be found. async fn descriptor_ensure<'d>(&self, data: &'d mut DataHsDesc) -> Result<&'d HsDesc, CE> { // TODO HS are these right? make configurable? get from netdir? + // TODO HS: we should check the revision counter on the HSDesc before + // replacing it. // TODO HS should we even have MAX_TOTAL_ATTEMPTS or should we just try each one once? /// Maxmimum number of hsdir connection and retrieval attempts we'll make const MAX_TOTAL_ATTEMPTS: usize = 6; diff --git a/crates/tor-llcrypto/semver.md b/crates/tor-llcrypto/semver.md new file mode 100644 index 000000000..ddfcd0316 --- /dev/null +++ b/crates/tor-llcrypto/semver.md @@ -0,0 +1 @@ +ADDED: ct_lookup diff --git a/crates/tor-llcrypto/src/util/ct.rs b/crates/tor-llcrypto/src/util/ct.rs index 01c8adddb..1c8a764d8 100644 --- a/crates/tor-llcrypto/src/util/ct.rs +++ b/crates/tor-llcrypto/src/util/ct.rs @@ -87,8 +87,56 @@ impl AsMut<[u8; N]> for CtByteArray { } } +/// Try to find an item in a slice without leaking where and whether the +/// item was found. +/// +/// If there is any item `x` in the `array` for which `matches(x)` +/// is true, this function will return a reference to one such +/// item. (We don't specify which.) +/// +/// Otherwise, this function returns none. +/// +/// We evaluate `matches` on every item of the array, and try not to +/// leak by timing which element (if any) matched. Note that if +/// `matches` itself has side channels, this function can't hide them. +/// +/// Note that this doesn't necessarily do a constant-time comparison, +/// and that it is not constant-time for the found/not-found case. +pub fn ct_lookup(array: &[T], matches: F) -> Option<&T> +where + F: Fn(&T) -> Choice, +{ + // ConditionallySelectable isn't implemented for usize, so we need + // to use u64. + let mut idx: u64 = 0; + let mut found: Choice = 0.into(); + + for (i, x) in array.iter().enumerate() { + let equal = matches(x); + idx.conditional_assign(&(i as u64), equal); + found.conditional_assign(&equal, equal); + } + + if found.into() { + Some(&array[idx as usize]) + } else { + None + } +} + #[cfg(test)] mod test { + // @@ begin test lint list maintained by maint/add_warning @@ + #![allow(clippy::bool_assert_comparison)] + #![allow(clippy::clone_on_copy)] + #![allow(clippy::dbg_macro)] + #![allow(clippy::print_stderr)] + #![allow(clippy::print_stdout)] + #![allow(clippy::single_char_pattern)] + #![allow(clippy::unwrap_used)] + #![allow(clippy::unchecked_duration_subtraction)] + //! + use super::*; use rand::Rng; use tor_basic_utils::test_rng; @@ -122,4 +170,23 @@ mod test { } } } + + #[test] + fn test_lookup() { + use super::ct_lookup as lookup; + use subtle::ConstantTimeEq; + let items = vec![ + "One".to_string(), + "word".to_string(), + "of".to_string(), + "every".to_string(), + "length".to_string(), + ]; + let of_word = lookup(&items[..], |i| i.len().ct_eq(&2)); + let every_word = lookup(&items[..], |i| i.len().ct_eq(&5)); + let no_word = lookup(&items[..], |i| i.len().ct_eq(&99)); + assert_eq!(of_word.unwrap(), "of"); + assert_eq!(every_word.unwrap(), "every"); + assert_eq!(no_word, None); + } } diff --git a/crates/tor-netdoc/Cargo.toml b/crates/tor-netdoc/Cargo.toml index 3c29a3f23..afc4823c9 100644 --- a/crates/tor-netdoc/Cargo.toml +++ b/crates/tor-netdoc/Cargo.toml @@ -96,6 +96,7 @@ serde = "1.0.103" serde_with = "3.0.0" signature = "1" smallvec = "1.10" +subtle = "2" thiserror = "1" time = { version = "0.3", features = ["std", "parsing", "macros"] } tinystr = "0.7.0" diff --git a/crates/tor-netdoc/src/build.rs b/crates/tor-netdoc/src/build.rs index 26c64f2bd..fa81a4cd2 100644 --- a/crates/tor-netdoc/src/build.rs +++ b/crates/tor-netdoc/src/build.rs @@ -13,49 +13,22 @@ //! It is the caller's responsibility to call `.item()` in the right order, //! with the right keywords and arguments. -#![allow(unused_variables)] // TODO hs -#![allow(unused_imports)] // TODO hs -#![allow(dead_code)] // TODO hs -#![allow(clippy::missing_docs_in_private_items)] // TODO hs -#![allow(clippy::needless_pass_by_value)] // TODO hs - -use std::fmt::{self, Display, Write}; -use std::marker::PhantomData; -use std::ops::Deref; +use std::fmt::{Display, Write}; use std::time::SystemTime; use base64ct::{Base64, Encoding}; use humantime::format_rfc3339; use rand::{CryptoRng, RngCore}; use tor_bytes::EncodeError; -use tor_cert::Ed25519Cert; -use tor_error::{internal, into_internal, Bug}; -use tor_llcrypto::pk::ed25519; +use tor_error::{internal, Bug}; use crate::parse::keyword::Keyword; use crate::parse::tokenize::tag_keywords_ok; -/// Encoder, representing a partially-built document +/// Encoder, representing a partially-built document. /// -/// # Example -/// -/// # TODO hs, actually fix and test this example -/// ```rust,ignore -/// use OnionServiceKeyword as K; -/// -/// let mut document = NetDocEncoder::new(); -/// let beginning = document.marker(); -/// document.item(K::HsDescriptor).arg(3); -/// document.item(K::DescriptorLifetime).arg(&self.lifetime); -/// document.item(K::DescriptorSigningKeyCert).object("ED25519 CERT", &self.cert[..]); -/// document.item(K::RevisionCounter).arg(&self.counter); -/// document.item(K::Superencrypted).object("MESSAGE", inner_text); -/// let end = document.marker(); -/// let signature = key.sign(document.slice(beginning, end)); -/// document.item(K::Signature).arg(B64(signature)); -/// -/// let text = document.finish()?; -/// ``` +/// For example usage, see the tests in this module, or a descriptor building +/// function in tor-netdoc (such as `hsdesc::build::inner::HsDescInner::build_sign`). #[derive(Debug, Clone)] pub(crate) struct NetdocEncoder { /// The being-built document, with everything accumulated so far @@ -165,6 +138,7 @@ impl NetdocEncoder { /// In particular, `s` should end with a newline. /// No checks are performed. /// Incorrect use might lead to malformed documents, or later errors. + #[allow(dead_code)] // TODO: We should remove this if it never used. pub(crate) fn push_raw_string(&mut self, s: &dyn Display) { self.raw(s); } @@ -283,6 +257,7 @@ impl<'n> ItemEncoder<'n> { /// separated by (single) spaces. /// This is not (properly) checked. /// Incorrect use might lead to malformed documents, or later errors. + #[allow(unused)] // TODO: We should eventually remove this if nothing starts to use it. pub(crate) fn args_raw_string(mut self, args: &dyn Display) -> Self { let args = args.to_string(); if !args.is_empty() { diff --git a/crates/tor-netdoc/src/doc/hsdesc.rs b/crates/tor-netdoc/src/doc/hsdesc.rs index b979cce77..49c9ca7d3 100644 --- a/crates/tor-netdoc/src/doc/hsdesc.rs +++ b/crates/tor-netdoc/src/doc/hsdesc.rs @@ -8,7 +8,7 @@ //! An onion service descriptor is more complicated than most other //! documentation types, because it is partially encrypted. -#![allow(dead_code, unused_variables, clippy::missing_panics_doc)] // TODO hs: remove. +#![allow(dead_code)] // TODO hs: remove. mod desc_enc; #[cfg(feature = "hs-service")] @@ -385,11 +385,11 @@ impl EncryptedHsDesc { let kp_desc_sign = self.outer_doc.desc_sign_key_id(); // Decrypt the superencryption layer; parse the middle document. - let middle = self.outer_doc.decrypt_body(subcredential).map_err(|e| { + let middle = self.outer_doc.decrypt_body(subcredential).map_err(|_| { EK::BadObjectVal.with_msg("onion service descriptor superencryption failed.") })?; let middle = std::str::from_utf8(&middle[..]) - .map_err(|e| EK::BadObjectVal.with_msg("Bad utf-8 in middle document"))?; + .map_err(|_| EK::BadObjectVal.with_msg("Bad utf-8 in middle document"))?; let middle = middle::HsDescMiddle::parse(middle)?; // Decrypt the encryption layer and parse the inner document. @@ -400,11 +400,11 @@ impl EncryptedHsDesc { subcredential, hsc_desc_enc.map(|keys| keys.1), ) - .map_err(|e| { + .map_err(|_| { EK::BadObjectVal.with_msg("onion service descriptor encryption failed.") })?; let inner = std::str::from_utf8(&inner[..]) - .map_err(|e| EK::BadObjectVal.with_msg("Bad utf-8 in inner document"))?; + .map_err(|_| EK::BadObjectVal.with_msg("Bad utf-8 in inner document"))?; let (cert_signing_key, time_bound) = inner::HsDescInner::parse(inner)?; if cert_signing_key.as_ref() != Some(kp_desc_sign) { diff --git a/crates/tor-netdoc/src/doc/hsdesc/build.rs b/crates/tor-netdoc/src/doc/hsdesc/build.rs index d325a9d54..a034e5eb4 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/build.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/build.rs @@ -27,9 +27,12 @@ use self::outer::HsDescOuter; use super::desc_enc::{HsDescEncNonce, HsDescEncryption, HS_DESC_ENC_NONCE_LEN}; -/// A builder for encoding hidden service descriptors. +/// An intermediary type for encoding hidden service descriptors. /// -/// TODO hs: a comprehensive usage example. +/// This object is constructed via [`HsDescBuilder`], and then turned into a +/// signed document using [`HsDescBuilder::build_sign()`]. +/// +/// TODO HSS: Add an example for using this API. #[derive(Builder)] #[builder(public, derive(Debug, Clone), pattern = "owned", build_fn(vis = ""))] struct HsDesc<'a> { @@ -41,7 +44,7 @@ struct HsDesc<'a> { /// The expiration time of the descriptor signing key certificate. hs_desc_sign_cert_expiry: SystemTime, /// A list of recognized CREATE handshakes that this onion service supports. - // TODO hs: this should probably be a caret enum, not an integer + // TODO HSS: this should probably be a caret enum, not an integer create2_formats: &'a [u32], /// A list of authentication types that this onion service supports. auth_required: Option>, @@ -343,11 +346,10 @@ mod test { humantime::parse_duration("12 hours").unwrap(), ) .unwrap(); - let (public, blinded_id, subcredential) = HsIdKeypair::from(ExpandedKeypair::from(&hs_id)) + let (_, blinded_id, subcredential) = HsIdKeypair::from(ExpandedKeypair::from(&hs_id)) .compute_blinded_key(period) .unwrap(); - let id = ed25519::Ed25519Identity::from(blinded_id.as_ref().public); let expiry = SystemTime::now() + Duration::from_secs(CERT_EXPIRY_SECS); let mut rng = Config::Deterministic.into_rng().rng_compat(); let intro_points = vec![IntroPointDesc { diff --git a/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs b/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs index 9a5b9c2b7..eb38d6a9e 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/build/inner.rs @@ -34,7 +34,7 @@ pub(super) struct HsDescInner<'a> { /// The descriptor signing key. pub(super) hs_desc_sign: &'a ed25519::Keypair, /// A list of recognized CREATE handshakes that this onion service supports. - // TODO hs: this should probably be a caret enum, not an integer + // TODO hss: this should probably be a caret enum, not an integer pub(super) create2_formats: &'a [u32], /// A list of authentication types that this onion service supports. pub(super) auth_required: Option<&'a SmallVec<[IntroAuthType; 2]>>, @@ -50,7 +50,7 @@ pub(super) struct HsDescInner<'a> { /// Information in an onion service descriptor about a single introduction point. /// -/// TODO hs: Move out of tor-netdoc: this is a general-purpose representation of an introduction +/// TODO HSS: Move out of tor-netdoc: this is a general-purpose representation of an introduction /// point, not merely an intermediate representation for decoding/encoding. There may be other /// types that need to be factored out tor-netdoc for the same reason. #[derive(Debug, Clone)] @@ -151,7 +151,7 @@ impl<'a> NetdocBuilder for HsDescInner<'a> { // "The key is a base64 encoded curve25519 public key used to encrypt the introduction // request to service. (`KP_hss_ntor`)" // - // TODO hs: The spec allows for multiple enc-key lines, but we currently only ever encode + // TODO hss: The spec allows for multiple enc-key lines, but we currently only ever encode // a single one. encoder .item(ENC_KEY) @@ -163,7 +163,7 @@ impl<'a> NetdocBuilder for HsDescInner<'a> { // The subject key is the the ed25519 equivalent of the svc_ntor_key curve25519 public // encryption key. - // TODO hs: should the sign bit be 0 or 1? + // TODO hss: should the sign bit be 0 or 1? let signbit = 0; let ed_svc_ntor_key = convert_curve25519_to_ed25519_public(&intro_point.svc_ntor_key, signbit) @@ -338,9 +338,6 @@ eetKn+yDC5Q3eo/hJLDBGAQNOX7jFMdr9HjotjXIt6/Khfmg58CZC/gKhAw= #[test] fn inner_hsdesc_too_many_link_specifiers() { - let hs_desc_sign = - ed25519::Keypair::generate(&mut Config::Deterministic.into_rng().rng_compat()); - let link_spec = LinkSpec::OrPort(Ipv4Addr::LOCALHOST.into(), 9999); let link_specifiers = std::iter::repeat(link_spec) .take(u8::MAX as usize + 1) @@ -367,7 +364,6 @@ eetKn+yDC5Q3eo/hJLDBGAQNOX7jFMdr9HjotjXIt6/Khfmg58CZC/gKhAw= #[test] fn inner_hsdesc_intro_auth() { let mut rng = Config::Deterministic.into_rng().rng_compat(); - let hs_desc_sign = ed25519::Keypair::generate(&mut rng); let link_specs = vec![LinkSpec::OrPort(Ipv4Addr::LOCALHOST.into(), 8080)]; let intros = &[create_intro_point_descriptor(&mut rng, link_specs)]; let auth = SmallVec::from([IntroAuthType::Ed25519, IntroAuthType::Ed25519]); @@ -387,19 +383,19 @@ eetKn+yDC5Q3eo/hJLDBGAQNOX7jFMdr9HjotjXIt6/Khfmg58CZC/gKhAw= r#"create2-formats 1234 intro-auth-required ed25519 ed25519 introduction-point AQAGfwAAAR+Q -onion-key ntor pVDg7+MoXDE57TaedKLUKQ6OSUYduWcW/8eikjmR9RA= +onion-key ntor HWIigEAdcOgqgHPDFmzhhkeqvYP/GcMT2fKb5JY6ey8= auth-key -----BEGIN ED25519 CERT----- -AQkAAAAAAec/Z6WU0POJ2wsAAJD/erWpQWczFK7ouc8t2RWPD8OhAQAgBACQKRtN -eNThmyleMYdmFucrbgPcZNDO6S81MZD1r7q61Lte7Exhx6mxnXB+XmxoSqV2IQij -UdgEfu8viEqFaAdC8b/ffdmeXrRf4OQXJYd562M8Vtxih6CVVp2Bmu9jpwo= +AQkAAAAAAZZVJwNlzVw1ZQGO7MTzC5MsySASd+fswAcjdTJJOifXAQAgBACQKRtN +eNThmyleMYdmFucrbgPcZNDO6S81MZD1r7q61IVW0XivcAKhvUvNUsU1CFznk3Mz +KSsp/mBoKi2iY4f4eN2SXx8U6pmnxnXFxYP6obi+tc5QWj1Jbfl1Aci3TAA= -----END ED25519 CERT----- -enc-key ntor x/stThC6cVWJJUR7WERZj5VYVPTAOA/UDjHdtprJkiE= +enc-key ntor 9Upi9XNWyqx3ZwHeQ5r3+Dh116k+C4yHeE9BcM68HDc= enc-key-cert -----BEGIN ED25519 CERT----- -AQsAAAAAASPKuL+ddCmgEToN22Ig0Ja1i3RAvLK2y20ragaqGTRMAQAgBACQKRtN -eNThmyleMYdmFucrbgPcZNDO6S81MZD1r7q61LOY8CatszBdKADp+/LBEHuC2QiE -zkV7qcj2hWvbquRKigbpsXWa7atUoygiXJnrtVTbN9Q9O5VCEukdXkUEoQk= +AQsAAAAAAcH+1K5m7pRnMc01mPp5AYVnJK1iZ/fKHwK0tVR/jtBvAQAgBACQKRtN +eNThmyleMYdmFucrbgPcZNDO6S81MZD1r7q61Hectpha37ioha85fpNt+/yDfebh +6BKUUQ0jf3SMXuNgX8SV9NSabn14WCSdKG/8RoYBCTR+yRJX0dy55mjg+go= -----END ED25519 CERT----- "# ); diff --git a/crates/tor-netdoc/src/doc/hsdesc/build/outer.rs b/crates/tor-netdoc/src/doc/hsdesc/build/outer.rs index 323fdb555..10740267d 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/build/outer.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/build/outer.rs @@ -139,7 +139,7 @@ mod test { humantime::parse_duration("12 hours").unwrap(), ) .unwrap(); - let (public, blinded_id, _) = HsIdKeypair::from(ExpandedKeypair::from(&hs_id)) + let (_public, blinded_id, _) = HsIdKeypair::from(ExpandedKeypair::from(&hs_id)) .compute_blinded_key(period) .unwrap(); diff --git a/crates/tor-netdoc/src/doc/hsdesc/desc_enc.rs b/crates/tor-netdoc/src/doc/hsdesc/desc_enc.rs index 580bc5f1b..740443610 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/desc_enc.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/desc_enc.rs @@ -1,6 +1,4 @@ //! Types and functions for onion service descriptor encryption. -//! -//! TODO hs: It's possible that this should move to tor-netdoc. use tor_hscrypto::{pk::HsBlindId, RevisionCounter, Subcredential}; use tor_llcrypto::cipher::aes::Aes256Ctr as Cipher; @@ -87,7 +85,7 @@ impl<'a> HsDescEncryption<'a> { cipher.apply_keystream(&mut output[SALT_LEN..]); mac.update(&output[SALT_LEN..]); let mut mac_val = Default::default(); - let mac = mac.finalize_into(&mut mac_val); + mac.finalize_into(&mut mac_val); output.extend_from_slice(&mac_val); debug_assert_eq!(output.len(), output_len); @@ -141,7 +139,7 @@ impl<'a> HsDescEncryption<'a> { let mut key = Z::new([0_u8; Self::CIPHER_KEY_LEN]); let mut iv = Z::new([0_u8; Self::IV_LEN]); - let mut mac_key = Z::new([0_u8; Self::MAC_KEY_LEN]); // TODO HS conjectural! + let mut mac_key = Z::new([0_u8; Self::MAC_KEY_LEN]); key_stream.read(&mut key[..]); key_stream.read(&mut iv[..]); key_stream.read(&mut mac_key[..]); diff --git a/crates/tor-netdoc/src/doc/hsdesc/inner.rs b/crates/tor-netdoc/src/doc/hsdesc/inner.rs index 456972d15..335383a84 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/inner.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/inner.rs @@ -432,7 +432,7 @@ mod test { .decrypt_inner(&desc.blinded_id(), desc.revision_counter(), &subcred, None) .unwrap(); let inner_body = std::str::from_utf8(&inner_body).unwrap(); - let inner = HsDescInner::parse(inner_body)?; + let _inner = HsDescInner::parse(inner_body)?; // TODO hs: validate the expected contents of this part of the // descriptor. diff --git a/crates/tor-netdoc/src/doc/hsdesc/middle.rs b/crates/tor-netdoc/src/doc/hsdesc/middle.rs index cdae94636..79c3b2336 100644 --- a/crates/tor-netdoc/src/doc/hsdesc/middle.rs +++ b/crates/tor-netdoc/src/doc/hsdesc/middle.rs @@ -1,6 +1,7 @@ //! Handle the middle document of an onion service descriptor. use once_cell::sync::Lazy; +use subtle::ConstantTimeEq; use tor_hscrypto::pk::{HsBlindId, HsClientDescEncSecretKey, HsSvcDescEncKey}; use tor_hscrypto::{RevisionCounter, Subcredential}; use tor_llcrypto::pk::curve25519; @@ -17,8 +18,9 @@ use super::desc_enc::{ }; use super::DecryptionError; -/// TODO hs: This should be an enum. /// The only currently recognized `desc-auth-type`. +// +// TODO: In theory this should be an enum, if we ever add a second value here. pub(super) const HS_DESC_AUTH_TYPE: &str = "x25519"; /// A more-or-less verbatim representation of the middle document of an onion @@ -89,6 +91,7 @@ impl HsDescMiddle { ) -> Option { use cipher::{KeyIvInit, StreamCipher}; use tor_llcrypto::cipher::aes::Aes256Ctr as Cipher; + use tor_llcrypto::util::ct::ct_lookup; let (client_id, cookie_key) = build_descriptor_cookie_key( ks_hsc_desc_enc.as_ref(), @@ -96,12 +99,7 @@ impl HsDescMiddle { subcredential, ); // See whether there is any matching client_id in self.auth_ids. - // TODO HS: Perhaps we should use `tor_proto::util::ct::lookup`. We would - // have to put it in a lower level module. - let auth_client = self - .auth_clients - .iter() - .find(|c| c.client_id == client_id)?; + let auth_client = ct_lookup(&self.auth_clients, |c| c.client_id.ct_eq(&client_id))?; // We found an auth client entry: Take and decrypt the cookie `N_hs_desc_enc` at last. let mut cookie = auth_client.encrypted_cookie; @@ -198,7 +196,7 @@ impl HsDescMiddle { // Check for the only currently recognized `desc-auth-type` { let auth_type = body.required(DESC_AUTH_TYPE)?.required_arg(0)?; - if auth_type != "x25519" { + if auth_type != HS_DESC_AUTH_TYPE { return Err(EK::BadDocumentVersion .at_pos(Pos::at(auth_type)) .with_msg(format!("Unrecognized desc-auth-type {auth_type:?}"))); @@ -265,7 +263,7 @@ mod test { // TODO hs: assert that the fields here are expected. // TODO hs: write a test for the case where we _do_ have an encryption key. - let inner_body = middle + let _inner_body = middle .decrypt_inner(&desc.blinded_id(), desc.revision_counter(), &subcred, None) .unwrap(); diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 0e61653fe..c4e798eb1 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -299,14 +299,12 @@ pub struct SignatureGroup { #[derive( Debug, Clone, Copy, Eq, PartialEq, derive_more::From, derive_more::Into, derive_more::AsRef, )] -// TODO hs: Use CtBytes for this. I don't think it actually matters, but it -// seems like a good idea. +// (This doesn't need to use CtByteArray; we don't really need to compare these.) pub struct SharedRandVal([u8; 32]); /// A shared-random value produced by the directory authorities, /// along with meta-information about that value. #[allow(dead_code)] -// TODO hs: This should have real accessors, not this 'visible/visibility' hack. #[cfg_attr( feature = "dangerous-expose-struct-fields", visible::StructFields(pub), diff --git a/crates/tor-proto/src/crypto/handshake/ntor.rs b/crates/tor-proto/src/crypto/handshake/ntor.rs index e682dc8a0..c8ac66269 100644 --- a/crates/tor-proto/src/crypto/handshake/ntor.rs +++ b/crates/tor-proto/src/crypto/handshake/ntor.rs @@ -8,6 +8,7 @@ use tor_error::into_internal; use tor_llcrypto::d; use tor_llcrypto::pk::curve25519::*; use tor_llcrypto::pk::rsa::RsaIdentity; +use tor_llcrypto::util::ct::ct_lookup; use digest::Mac; use rand_core::{CryptoRng, RngCore}; @@ -285,7 +286,7 @@ where let my_key: PublicKey = cur.extract()?; let their_pk: PublicKey = cur.extract()?; - let keypair = ct::lookup(keys, |key| key.matches_pk(&my_key)); + let keypair = ct_lookup(keys, |key| key.matches_pk(&my_key)); let keypair = match keypair { Some(k) => k, None => return Err(RelayHandshakeError::MissingKey), diff --git a/crates/tor-proto/src/crypto/handshake/ntor_v3.rs b/crates/tor-proto/src/crypto/handshake/ntor_v3.rs index ca1ecc65f..88a6dac1b 100644 --- a/crates/tor-proto/src/crypto/handshake/ntor_v3.rs +++ b/crates/tor-proto/src/crypto/handshake/ntor_v3.rs @@ -19,7 +19,7 @@ use tor_bytes::{EncodeResult, Reader, SecretBuf, Writeable, Writer}; use tor_error::into_internal; use tor_llcrypto::d::{Sha3_256, Shake256}; use tor_llcrypto::pk::{curve25519, ed25519::Ed25519Identity}; -use tor_llcrypto::util::rand_compat::RngCompatExt; +use tor_llcrypto::util::{ct::ct_lookup, rand_compat::RngCompatExt}; use cipher::{KeyIvInit, StreamCipher}; @@ -444,7 +444,7 @@ fn server_handshake_ntor_v3_no_keygen( r.should_be_exhausted()?; // See if we recognize the provided (id,requested_pk) pair. - let keypair = ct::lookup(keys, |key| key.matches(id, requested_pk)); + let keypair = ct_lookup(keys, |key| key.matches(id, requested_pk)); let keypair = match keypair { Some(k) => k, None => return Err(RelayHandshakeError::MissingKey), diff --git a/crates/tor-proto/src/util/ct.rs b/crates/tor-proto/src/util/ct.rs index 01de676f8..3c9921327 100644 --- a/crates/tor-proto/src/util/ct.rs +++ b/crates/tor-proto/src/util/ct.rs @@ -1,41 +1,5 @@ //! Constant-time utilities. -use subtle::{Choice, ConditionallySelectable, ConstantTimeEq}; - -/// Try to find an item in a slice without leaking where and whether the -/// item was found. -/// -/// If there is any item `x` in the `array` for which `matches(x)` -/// is true, this function will return a reference to one such -/// item. (We don't specify which.) -/// -/// Otherwise, this function returns none. -/// -/// We evaluate `matches` on every item of the array, and try not to -/// leak by timing which element (if any) matched. -/// -/// Note that this doesn't necessarily do a constant-time comparison, -/// and that it is not constant-time for found/not-found case. -pub(crate) fn lookup(array: &[T], matches: F) -> Option<&T> -where - F: Fn(&T) -> Choice, -{ - // ConditionallySelectable isn't implemented for usize, so we need - // to use u64. - let mut idx: u64 = 0; - let mut found: Choice = 0.into(); - - for (i, x) in array.iter().enumerate() { - let equal = matches(x); - idx.conditional_assign(&(i as u64), equal); - found.conditional_assign(&equal, equal); - } - - if found.into() { - Some(&array[idx as usize]) - } else { - None - } -} +use subtle::{Choice, ConstantTimeEq}; /// Convert a boolean into a Choice. /// @@ -72,23 +36,4 @@ mod test { assert!(!bytes_eq(&b"hi"[..], &b"45"[..])); assert!(bytes_eq(&b""[..], &b""[..])); } - - #[test] - fn test_lookup() { - use super::lookup; - use subtle::ConstantTimeEq; - let items = vec![ - "One".to_string(), - "word".to_string(), - "of".to_string(), - "every".to_string(), - "length".to_string(), - ]; - let of_word = lookup(&items[..], |i| i.len().ct_eq(&2)); - let every_word = lookup(&items[..], |i| i.len().ct_eq(&5)); - let no_word = lookup(&items[..], |i| i.len().ct_eq(&99)); - assert_eq!(of_word.unwrap(), "of"); - assert_eq!(every_word.unwrap(), "every"); - assert_eq!(no_word, None); - } }