diff --git a/Cargo.lock b/Cargo.lock index 8733ff615..81392ca7c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4165,8 +4165,8 @@ dependencies = [ "either", "futures", "humantime 2.1.0", - "itertools", "postage", + "rand 0.8.5", "rand_core 0.6.4", "retry-error", "safelog", @@ -4175,6 +4175,7 @@ dependencies = [ "thiserror", "tokio", "tor-async-utils", + "tor-basic-utils", "tor-chanmgr", "tor-checkable", "tor-circmgr", diff --git a/crates/tor-basic-utils/src/test_rng.rs b/crates/tor-basic-utils/src/test_rng.rs index 5d9b3bfb6..5b85b06a4 100644 --- a/crates/tor-basic-utils/src/test_rng.rs +++ b/crates/tor-basic-utils/src/test_rng.rs @@ -64,7 +64,7 @@ use rand::{RngCore, SeedableRng}; // We'll use the same PRNG as the (current) standard. We specify it here rather // than using StdRng, since we want determinism in the future. -use rand_chacha::ChaCha12Rng as TestingRng; +pub use rand_chacha::ChaCha12Rng as TestingRng; /// The seed type for the RNG we're returning. type Seed = ::Seed; diff --git a/crates/tor-hsclient/Cargo.toml b/crates/tor-hsclient/Cargo.toml index 301236712..e1d8f49d3 100644 --- a/crates/tor-hsclient/Cargo.toml +++ b/crates/tor-hsclient/Cargo.toml @@ -20,8 +20,8 @@ derive_more = "0.99.3" educe = "0.4.6" either = "1" futures = "0.3.14" -itertools = "0.10.1" postage = { version = "0.5.0", default-features = false, features = ["futures-traits"] } +rand = "0.8" rand_core = "0.6.2" retry-error = { path = "../retry-error", version = "0.4.0" } safelog = { path = "../safelog", version = "0.3.0" } @@ -46,6 +46,7 @@ tracing = "0.1.36" humantime = "2" tokio-crate = { package = "tokio", version = "1.7", features = ["full"] } tor-async-utils = { path = "../tor-async-utils", version = "0.1.0" } +tor-basic-utils = { path = "../tor-basic-utils", version = "0.7.0" } tor-chanmgr = { path = "../tor-chanmgr", version = "0.9.0" } tor-circmgr = { version = "0.8.0", path = "../tor-circmgr", features = ["hs-client", "testing"] } tor-guardmgr = { path = "../tor-guardmgr", version = "0.9.0", features = ["testing"] } diff --git a/crates/tor-hsclient/src/connect.rs b/crates/tor-hsclient/src/connect.rs index f04b0e35e..7bd86f784 100644 --- a/crates/tor-hsclient/src/connect.rs +++ b/crates/tor-hsclient/src/connect.rs @@ -10,7 +10,6 @@ use std::time::SystemTime; use async_trait::async_trait; use educe::Educe; use futures::{AsyncRead, AsyncWrite}; -use itertools::Itertools; use tor_hscrypto::Subcredential; use tracing::{debug, trace}; @@ -212,10 +211,12 @@ impl<'c, 'd, R: Runtime, M: MocksForConnect> Context<'c, 'd, R, M> { // Seems to be not valid now. Try to fetch a fresh one. } - let hs_dirs = self - .netdir - .hs_dirs(&self.hs_blind_id, HsDirOp::Download) - .collect_vec(); + let hs_dirs = self.netdir.hs_dirs( + &self.hs_blind_id, + HsDirOp::Download, + &mut self.mocks.thread_rng(), + ); + trace!( "HS desc fetch for {}, using {} hsdirs", &self.hsid, @@ -357,10 +358,17 @@ impl<'c, 'd, R: Runtime, M: MocksForConnect> Context<'c, 'd, R, M> { trait MocksForConnect: Clone { /// HS circuit pool type HsCircPool: MockableCircPool; + + /// A random number generator + type Rng: rand::Rng; + /// Tell tests we got this descriptor text fn test_got_desc(&self, desc: &HsDesc) { eprintln!("HS DESC:\n{:?}\n", &desc); // TODO HS remove } + + /// Return a random number generator + fn thread_rng(&self) -> Self::Rng; } /// Mock for `HsCircPool` #[async_trait] @@ -384,6 +392,11 @@ trait MockableClientCirc { impl MocksForConnect for () { type HsCircPool = HsCircPool; + type Rng = rand::rngs::ThreadRng; + + fn thread_rng(&self) -> Self::Rng { + rand::thread_rng() + } } #[async_trait] impl MockableCircPool for HsCircPool { @@ -444,6 +457,7 @@ mod test { use std::{iter, panic::AssertUnwindSafe}; use tokio_crate as tokio; use tor_async_utils::JoinReadWrite; + use tor_basic_utils::test_rng::{testing_rng, TestingRng}; use tor_llcrypto::pk::curve25519; use tor_netdoc::doc::{hsdesc::test_data, netstatus::Lifetime}; use tor_rtcompat::{tokio::TokioNativeTlsRuntime, CompoundRuntime}; @@ -473,9 +487,15 @@ mod test { impl MocksForConnect for Mocks<()> { type HsCircPool = Mocks<()>; + type Rng = TestingRng; + fn test_got_desc(&self, desc: &HsDesc) { self.mglobal.lock().unwrap().got_desc = Some(desc.clone()); } + + fn thread_rng(&self) -> Self::Rng { + testing_rng() + } } #[async_trait] impl MockableCircPool for Mocks<()> { diff --git a/crates/tor-netdir/src/lib.rs b/crates/tor-netdir/src/lib.rs index ab65166d3..630bc5061 100644 --- a/crates/tor-netdir/src/lib.rs +++ b/crates/tor-netdir/src/lib.rs @@ -68,6 +68,7 @@ use {hsdir_params::HsDirParams, hsdir_ring::HsDirRing, std::iter}; use derive_more::{From, Into}; use futures::stream::BoxStream; use num_enum::{IntoPrimitive, TryFromPrimitive}; +use rand::seq::SliceRandom; use serde::Deserialize; use std::collections::HashMap; use std::net::IpAddr; @@ -258,7 +259,7 @@ impl From for RelayWeight { } /// An operation for which we might be requesting a hidden service directory. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] #[non_exhaustive] pub enum HsDirOp { /// Uploading an onion service descriptor. @@ -1106,7 +1107,6 @@ impl NetDir { R: rand::Rng, P: FnMut(&Relay<'a>) -> bool, { - use rand::seq::SliceRandom; let relays: Vec<_> = self.relays().filter(usable).collect(); // This algorithm uses rand::distributions::WeightedIndex, and uses // gives O(n) time and space to build the index, plus O(log n) @@ -1159,7 +1159,6 @@ impl NetDir { R: rand::Rng, P: FnMut(&Relay<'a>) -> bool, { - use rand::seq::SliceRandom; let relays: Vec<_> = self.relays().filter(usable).collect(); // NOTE: See discussion in pick_relay(). let mut relays = match relays[..].choose_multiple_weighted(rng, n, |r| { @@ -1272,18 +1271,16 @@ impl NetDir { /// given onion service's descriptor at a given time period. /// /// When `op` is `Download`, the order is random. - // TODO HS ^ this is not in fact true right now. /// When `op` is `Upload`, the order is not specified. /// /// Return an error if the time period is not one returned by /// `onion_service_time_period` or `onion_service_secondary_time_periods`. #[cfg(feature = "hs-common")] #[allow(unused, clippy::missing_panics_doc)] // TODO hs: remove. - pub fn hs_dirs<'r>( - &'r self, - hsid: &'r HsBlindId, - op: HsDirOp, - ) -> impl Iterator> + 'r { + pub fn hs_dirs<'r, R>(&'r self, hsid: &HsBlindId, op: HsDirOp, rng: &mut R) -> Vec> + where + R: rand::Rng, + { // Algorithm: // // 1. Determine which HsDirRing to use, based on the time period. @@ -1310,7 +1307,8 @@ impl NetDir { // service, any nodes already chosen are disregarded (i.e. skipped over) // when choosing a replica's hsdir_spread_store nodes. - self.hsdir_rings + let mut hs_dirs = self + .hsdir_rings .iter_for_op(op) .cartesian_product(1..=n_replicas) // 1-indexed ! .flat_map(move |(ring, replica): (&HsDirRing, u8)| { @@ -1321,6 +1319,20 @@ impl NetDir { // This ought not to be None but let's not panic or bail if it is self.relay_by_rs_idx(*rs_idx) }) + .collect_vec(); + + match op { + HsDirOp::Download => { + // When `op` is `Download`, the order is random. + hs_dirs.shuffle(rng); + } + #[cfg(feature = "hs-service")] + HsDirOp::Upload => { + // When `op` is `Upload`, the order is not specified. + } + } + + hs_dirs } }