From 9da43189f36b19681d10b78ef982c52c929b4573 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 25 Mar 2022 16:44:12 -0400 Subject: [PATCH] Turn FallbackList into a real type, and store one in GuardMgr. The guard manager is responsible for handing out the first hops of tor circuits, keeping track of their successes and failures, and remembering their states. Given that, it makes sense to store this information here. It is not yet used; I'll be fixing that in upcoming commits. Arguably, this information no longer belongs in the directory manager: I've added a todo about moving it. This commit will break compilation on its own in a couple of places; subsequent commits will fix it up. --- Cargo.lock | 2 + crates/arti-client/Cargo.toml | 1 + crates/arti-client/src/config.rs | 6 +++ crates/tor-circmgr/Cargo.toml | 1 + crates/tor-circmgr/src/config.rs | 2 + crates/tor-circmgr/src/lib.rs | 19 +++++---- crates/tor-circmgr/src/mgr.rs | 6 ++- crates/tor-circmgr/src/path/dirpath.rs | 24 +++++------ crates/tor-circmgr/src/path/exitpath.rs | 2 +- crates/tor-dirmgr/src/config.rs | 26 ++++++++---- crates/tor-guardmgr/src/err.rs | 15 ++++++- crates/tor-guardmgr/src/fallback.rs | 4 ++ crates/tor-guardmgr/src/fallback/set.rs | 55 +++++++++++++++++++++++++ crates/tor-guardmgr/src/lib.rs | 16 +++++-- 14 files changed, 142 insertions(+), 37 deletions(-) create mode 100644 crates/tor-guardmgr/src/fallback/set.rs diff --git a/Cargo.lock b/Cargo.lock index c4b5e39fe..99f4fbe53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,6 +148,7 @@ dependencies = [ "tor-config", "tor-dirmgr", "tor-error", + "tor-guardmgr", "tor-netdoc", "tor-persist", "tor-proto", @@ -3250,6 +3251,7 @@ dependencies = [ "futures-await-test", "humantime-serde", "itertools", + "once_cell", "pin-project", "rand 0.8.5", "retry-error", diff --git a/crates/arti-client/Cargo.toml b/crates/arti-client/Cargo.toml index 9a16b9265..49abaa364 100644 --- a/crates/arti-client/Cargo.toml +++ b/crates/arti-client/Cargo.toml @@ -36,6 +36,7 @@ tor-config = { path = "../tor-config", version = "0.1.0" } tor-chanmgr = { path = "../tor-chanmgr", version = "0.1.0" } tor-dirmgr = { path = "../tor-dirmgr", version = "0.1.0" } tor-error = { path = "../tor-error", version = "0.1.0" } +tor-guardmgr = { path = "../tor-guardmgr", version = "0.1.0" } tor-netdoc = { path = "../tor-netdoc", version = "0.1.0" } tor-persist = { path = "../tor-persist", version = "0.1.0" } tor-proto = { path = "../tor-proto", version = "0.1.0" } diff --git a/crates/arti-client/src/config.rs b/crates/arti-client/src/config.rs index 98fe32936..7997c27b3 100644 --- a/crates/arti-client/src/config.rs +++ b/crates/arti-client/src/config.rs @@ -275,6 +275,12 @@ pub struct TorClientConfig { impl tor_circmgr::CircMgrConfig for TorClientConfig {} +impl AsRef for TorClientConfig { + fn as_ref(&self) -> &tor_guardmgr::fallback::FallbackList { + self.tor_network.fallback_caches() + } +} + impl Default for TorClientConfig { fn default() -> Self { Self::builder() diff --git a/crates/tor-circmgr/Cargo.toml b/crates/tor-circmgr/Cargo.toml index 4df3ce9f4..9435fdf2b 100644 --- a/crates/tor-circmgr/Cargo.toml +++ b/crates/tor-circmgr/Cargo.toml @@ -40,6 +40,7 @@ educe = "0.4.6" futures = "0.3.14" humantime-serde = "1.1.1" itertools = "0.10.1" +once_cell = "1" tracing = "0.1.18" pin-project = "1" rand = "0.8" diff --git a/crates/tor-circmgr/src/config.rs b/crates/tor-circmgr/src/config.rs index ce715514f..2d50d0fc8 100644 --- a/crates/tor-circmgr/src/config.rs +++ b/crates/tor-circmgr/src/config.rs @@ -6,6 +6,7 @@ use tor_basic_utils::define_accessor_trait; use tor_config::ConfigBuildError; +use tor_guardmgr::fallback::FallbackList; use derive_builder::Builder; use serde::Deserialize; @@ -281,6 +282,7 @@ define_accessor_trait! { path_rules: PathConfig, circuit_timing: CircuitTiming, preemptive_circuits: PreemptiveCircuitConfig, + fallbacks: FallbackList, } } diff --git a/crates/tor-circmgr/src/lib.rs b/crates/tor-circmgr/src/lib.rs index 4ce22098c..48f915dc3 100644 --- a/crates/tor-circmgr/src/lib.rs +++ b/crates/tor-circmgr/src/lib.rs @@ -51,7 +51,6 @@ #![deny(clippy::unwrap_used)] use tor_chanmgr::ChanMgr; -use tor_guardmgr::fallback::FallbackDir; use tor_netdir::{DirEvent, NetDir, NetDirProvider}; use tor_proto::circuit::{CircParameters, ClientCirc, UniqId}; use tor_rtcompat::Runtime; @@ -75,8 +74,9 @@ mod timeouts; mod usage; pub use err::Error; -pub use isolation::IsolationToken; +pub use isolation::{IsolationToken}; pub use usage::{TargetPort, TargetPorts}; +use tor_guardmgr::fallback::FallbackList; pub use config::{ CircMgrConfig, CircuitTiming, CircuitTimingBuilder, PathConfig, PathConfigBuilder, @@ -111,13 +111,13 @@ const PARETO_TIMEOUT_DATA_KEY: &str = "circuit_timeouts"; #[non_exhaustive] pub enum DirInfo<'a> { /// A list of fallbacks, for use when we don't know a network directory. - Fallbacks(&'a [&'a FallbackDir]), + Fallbacks(&'a FallbackList), /// A complete network directory Directory(&'a NetDir), } -impl<'a> From<&'a [&'a FallbackDir]> for DirInfo<'a> { - fn from(v: &'a [&'a FallbackDir]) -> DirInfo<'a> { +impl<'a> From<&'a FallbackList> for DirInfo<'a> { + fn from(v: &'a FallbackList) -> DirInfo<'a> { DirInfo::Fallbacks(v) } } @@ -184,7 +184,11 @@ impl CircMgr { config.preemptive_circuits().clone(), ))); - let guardmgr = tor_guardmgr::GuardMgr::new(runtime.clone(), storage.clone())?; + let guardmgr = tor_guardmgr::GuardMgr::new( + runtime.clone(), + storage.clone(), + config.fallbacks().clone(), + )?; let storage_handle = storage.create_handle(PARETO_TIMEOUT_DATA_KEY); @@ -715,7 +719,8 @@ mod test { use tor_netdir::{MdReceiver, PartialNetDir}; use tor_netdoc::doc::netstatus::NetParams; // If it's just fallbackdir, we get the default parameters. - let di: DirInfo<'_> = (&[][..]).into(); + let fb = FallbackList::from([]); + let di: DirInfo<'_> = (&fb).into(); let p1 = di.circ_params(); assert!(!p1.extend_by_ed25519_id()); diff --git a/crates/tor-circmgr/src/mgr.rs b/crates/tor-circmgr/src/mgr.rs index 5b389d070..10e60614e 100644 --- a/crates/tor-circmgr/src/mgr.rs +++ b/crates/tor-circmgr/src/mgr.rs @@ -1335,9 +1335,11 @@ mod test { use crate::isolation::test::{assert_isoleq, IsolationTokenEq}; use crate::usage::{ExitPolicy, SupportedCircUsage}; use crate::{Error, StreamIsolation, TargetCircUsage, TargetPort}; + use once_cell::sync::Lazy; use std::collections::BTreeSet; use std::sync::atomic::{self, AtomicUsize}; use tor_error::bad_api_usage; + use tor_guardmgr::fallback::FallbackList; use tor_netdir::testnet; use tor_rtcompat::SleepProvider; use tor_rtmock::MockSleepRuntime; @@ -1463,10 +1465,10 @@ mod test { const FAKE_CIRC_DELAY: Duration = Duration::from_millis(30); - static DI_EMPTY: [&tor_guardmgr::fallback::FallbackDir; 0] = []; + static FALLBACKS_EMPTY: Lazy = Lazy::new(|| [].into()); fn di() -> DirInfo<'static> { - DI_EMPTY[..].into() + (&*FALLBACKS_EMPTY).into() } #[async_trait] diff --git a/crates/tor-circmgr/src/path/dirpath.rs b/crates/tor-circmgr/src/path/dirpath.rs index d153d53d3..3be69f777 100644 --- a/crates/tor-circmgr/src/path/dirpath.rs +++ b/crates/tor-circmgr/src/path/dirpath.rs @@ -5,7 +5,7 @@ use tor_guardmgr::{GuardMgr, GuardMonitor, GuardUsable}; use tor_netdir::{Relay, WeightRole}; use tor_rtcompat::Runtime; -use rand::{seq::SliceRandom, Rng}; +use rand::Rng; /// A PathBuilder that can connect to a directory. #[non_exhaustive] @@ -32,11 +32,9 @@ impl DirPathBuilder { guards: Option<&GuardMgr>, ) -> Result<(TorPath<'a>, Option, Option)> { match (netdir, guards) { - (DirInfo::Fallbacks(f), _) => { - let relay = f.choose(rng); - if let Some(r) = relay { - return Ok((TorPath::new_fallback_one_hop(r), None, None)); - } + (DirInfo::Fallbacks(f), None) => { + let relay = f.choose(rng)?; + return Ok((TorPath::new_fallback_one_hop(relay), None, None)); } (DirInfo::Directory(netdir), None) => { let relay = netdir.pick_relay(rng, WeightRole::BeginDir, Relay::is_dir_cache); @@ -72,7 +70,7 @@ mod test { use crate::path::assert_same_path_when_owned; use crate::test::OptDummyGuardMgr; use std::collections::HashSet; - use tor_guardmgr::fallback::FallbackDir; + use tor_guardmgr::fallback::{FallbackDir, FallbackList}; use tor_linkspec::ChanTarget; use tor_netdir::testnet; @@ -116,8 +114,8 @@ mod test { .build() .unwrap(), ]; - let fb: Vec<_> = fb_owned.iter().collect(); - let dirinfo = (&fb[..]).into(); + let fb: FallbackList = fb_owned.clone().into(); + let dirinfo = (&fb).into(); let mut rng = rand::thread_rng(); let guards: OptDummyGuardMgr<'_> = None; @@ -129,7 +127,7 @@ mod test { assert_same_path_when_owned(&p); if let crate::path::TorPathInner::FallbackOneHop(f) = p.inner { - assert!(std::ptr::eq(f, fb[0]) || std::ptr::eq(f, fb[1])); + assert!(f == &fb_owned[0] || f == &fb_owned[1]); } else { panic!("Generated the wrong kind of path."); } @@ -138,8 +136,8 @@ mod test { #[test] fn dirpath_no_fallbacks() { - let fb = vec![]; - let dirinfo = DirInfo::Fallbacks(&fb[..]); + let fb = FallbackList::from([]); + let dirinfo = DirInfo::Fallbacks(&fb); let mut rng = rand::thread_rng(); let guards: OptDummyGuardMgr<'_> = None; @@ -157,7 +155,7 @@ mod test { let mut rng = rand::thread_rng(); let dirinfo = (&netdir).into(); let statemgr = tor_persist::TestingStateMgr::new(); - let guards = tor_guardmgr::GuardMgr::new(rt.clone(), statemgr).unwrap(); + let guards = tor_guardmgr::GuardMgr::new(rt.clone(), statemgr, [].into()).unwrap(); guards.update_network(&netdir); let mut distinct_guards = HashSet::new(); diff --git a/crates/tor-circmgr/src/path/exitpath.rs b/crates/tor-circmgr/src/path/exitpath.rs index 17c2d50d7..3e2746ded 100644 --- a/crates/tor-circmgr/src/path/exitpath.rs +++ b/crates/tor-circmgr/src/path/exitpath.rs @@ -389,7 +389,7 @@ mod test { let mut rng = rand::thread_rng(); let dirinfo = (&netdir).into(); let statemgr = tor_persist::TestingStateMgr::new(); - let guards = tor_guardmgr::GuardMgr::new(rt.clone(), statemgr).unwrap(); + let guards = tor_guardmgr::GuardMgr::new(rt.clone(), statemgr, [].into()).unwrap(); let config = PathConfig::default(); guards.update_network(&netdir); let port443 = TargetPort::ipv4(443); diff --git a/crates/tor-dirmgr/src/config.rs b/crates/tor-dirmgr/src/config.rs index 5ddd81942..7be49434e 100644 --- a/crates/tor-dirmgr/src/config.rs +++ b/crates/tor-dirmgr/src/config.rs @@ -12,19 +12,20 @@ use crate::retry::DownloadSchedule; use crate::storage::DynStore; use crate::{Authority, Result}; use tor_config::ConfigBuildError; -use tor_guardmgr::fallback::FallbackDir; use tor_netdoc::doc::netstatus; use derive_builder::Builder; -use std::path::PathBuf; - use serde::Deserialize; +use std::path::PathBuf; /// Configuration information about the Tor network itself; used as /// part of Arti's configuration. /// /// This type is immutable once constructed. To make one, use /// [`NetworkConfigBuilder`], or deserialize it from a string. +// +// TODO: We should move this type around, since the fallbacks part will no longer be used in +// dirmgr, but only in guardmgr. Probably this type belongs in `arti-client`. #[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] #[serde(deny_unknown_fields)] #[builder(build_fn(validate = "Self::validate", error = "ConfigBuildError"))] @@ -42,8 +43,8 @@ pub struct NetworkConfig { #[builder(default = "fallbacks::default_fallbacks()")] #[serde(rename = "fallback_caches")] #[builder_field_attr(serde(rename = "fallback_caches"))] - #[builder(setter(name = "fallback_caches"))] - pub(crate) fallbacks: Vec, + #[builder(setter(into, name = "fallback_caches"))] + pub(crate) fallbacks: tor_guardmgr::fallback::FallbackList, /// List of directory authorities which we expect to sign consensus /// documents. @@ -71,6 +72,11 @@ impl NetworkConfig { pub fn builder() -> NetworkConfigBuilder { NetworkConfigBuilder::default() } + + /// Return the list of fallback directory caches from this configuration. + pub fn fallback_caches(&self) -> &tor_guardmgr::fallback::FallbackList { + &self.fallbacks + } } impl NetworkConfigBuilder { @@ -230,7 +236,7 @@ impl DirMgrConfig { } /// Return the configured set of fallback directories - pub fn fallbacks(&self) -> &[FallbackDir] { + pub fn fallbacks(&self) -> &tor_guardmgr::fallback::FallbackList { &self.network_config.fallbacks } @@ -306,11 +312,11 @@ impl DownloadScheduleConfig { /// Helpers for initializing the fallback list. mod fallbacks { - use tor_guardmgr::fallback::FallbackDir; + use tor_guardmgr::fallback::{FallbackDir, FallbackList}; use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity}; /// Return a list of the default fallback directories shipped with /// arti. - pub(crate) fn default_fallbacks() -> Vec { + pub(crate) fn default_fallbacks() -> FallbackList { /// Build a fallback directory; panic if input is bad. fn fallback(rsa: &str, ed: &str, ports: &[&str]) -> FallbackDir { let rsa = RsaIdentity::from_hex(rsa).expect("Bad hex in built-in fallback list"); @@ -331,7 +337,7 @@ mod fallbacks { bld.build() .expect("Unable to build default fallback directory!?") } - include!("fallback_dirs.inc") + include!("fallback_dirs.inc").into() } } @@ -361,6 +367,8 @@ mod test { #[test] fn build_network() -> Result<()> { + use tor_guardmgr::fallback::FallbackDir; + let dflt = NetworkConfig::default(); // with nothing set, we get the default. diff --git a/crates/tor-guardmgr/src/err.rs b/crates/tor-guardmgr/src/err.rs index b7d56c460..ca0ec0462 100644 --- a/crates/tor-guardmgr/src/err.rs +++ b/crates/tor-guardmgr/src/err.rs @@ -21,6 +21,17 @@ pub enum PickGuardError { /// out. #[error("No running guards were usable for the selected purpose")] NoGuardsUsable, + + /// We have no usable fallback directories. + #[error("All fallback directories are down")] + AllFallbacksDown { + /// The next time at which any fallback directory will back available. + retry_at: Option, + }, + + /// Tried to select guards or fallbacks from an empty list. + #[error("Tried to pick from an empty list")] + NoCandidatesAvailable, } impl tor_error::HasKind for PickGuardError { @@ -28,8 +39,8 @@ impl tor_error::HasKind for PickGuardError { use tor_error::ErrorKind as EK; use PickGuardError as E; match self { - E::AllGuardsDown { .. } => EK::TorAccessFailed, - E::NoGuardsUsable => EK::NoPath, + E::AllFallbacksDown { .. } | E::AllGuardsDown { .. } => EK::TorAccessFailed, + E::NoGuardsUsable | E::NoCandidatesAvailable => EK::NoPath, } } } diff --git a/crates/tor-guardmgr/src/fallback.rs b/crates/tor-guardmgr/src/fallback.rs index f4332b9f5..160e54452 100644 --- a/crates/tor-guardmgr/src/fallback.rs +++ b/crates/tor-guardmgr/src/fallback.rs @@ -10,6 +10,8 @@ //! The types in this module are re-exported from `arti-client` and //! `tor-dirmgr`: any changes here must be reflected there. +mod set; + use derive_builder::Builder; use tor_config::ConfigBuildError; use tor_llcrypto::pk::ed25519::Ed25519Identity; @@ -18,6 +20,8 @@ use tor_llcrypto::pk::rsa::RsaIdentity; use serde::Deserialize; use std::net::SocketAddr; +pub use set::FallbackList; + /// A directory whose location ships with Tor (or arti), and which we /// can use for bootstrapping when we don't know anything else about /// the network. diff --git a/crates/tor-guardmgr/src/fallback/set.rs b/crates/tor-guardmgr/src/fallback/set.rs new file mode 100644 index 000000000..7cb468f2e --- /dev/null +++ b/crates/tor-guardmgr/src/fallback/set.rs @@ -0,0 +1,55 @@ +//! Declare the [`FallbackSet`] type, which is used to store a set of FallbackDir. + +use rand::seq::IteratorRandom; +use std::iter::FromIterator; + +use super::FallbackDir; +use crate::PickGuardError; +use serde::Deserialize; + +/// A list of fallback directories. +/// +/// Fallback directories (represented by [`FallbackDir`]) are used by Tor +/// clients when they don't already have enough other directory information to +/// contact the network. +#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)] +#[serde(transparent)] +pub struct FallbackList { + /// The underlying fallbacks in this set. + fallbacks: Vec, +} + +impl FromIterator for FallbackList { + fn from_iter>(iter: T) -> Self { + FallbackList { + fallbacks: iter.into_iter().collect(), + } + } +} + +impl> From for FallbackList { + fn from(fallbacks: T) -> Self { + FallbackList { + fallbacks: fallbacks.into_iter().collect(), + } + } +} + +impl FallbackList { + /// Return the number of fallbacks in this list. + pub fn len(&self) -> usize { + self.fallbacks.len() + } + /// Return true if there are no fallbacks in this list. + pub fn is_empty(&self) -> bool { + self.fallbacks.is_empty() + } + /// Return a random member of this list. + pub fn choose(&self, rng: &mut R) -> Result<&FallbackDir, PickGuardError> { + // TODO: Return NoCandidatesAvailable when the fallback list is empty. + self.fallbacks + .iter() + .choose(rng) + .ok_or(PickGuardError::AllFallbacksDown { retry_at: None }) + } +} diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs index 26e11f6de..a71cc382a 100644 --- a/crates/tor-guardmgr/src/lib.rs +++ b/crates/tor-guardmgr/src/lib.rs @@ -229,6 +229,11 @@ struct GuardMgrInner { /// same guard. waiting: Vec, + /// A list of fallback directories used to access the directory system + /// when no other directory information is yet known. + // TODO: reconfigure when the configuration changes. + fallbacks: fallback::FallbackList, + /// Location in which to store persistent state. storage: DynStorageHandle, } @@ -261,7 +266,11 @@ impl GuardMgr { /// /// It won't be able to hand out any guards until /// [`GuardMgr::update_network`] has been called. - pub fn new(runtime: R, state_mgr: S) -> Result + pub fn new( + runtime: R, + state_mgr: S, + fallbacks: fallback::FallbackList, + ) -> Result where S: StateMgr + Send + Sync + 'static, { @@ -279,6 +288,7 @@ impl GuardMgr { ctrl, pending: HashMap::new(), waiting: Vec::new(), + fallbacks, storage, })); { @@ -1066,7 +1076,7 @@ mod test { let statemgr = TestingStateMgr::new(); let have_lock = statemgr.try_lock().unwrap(); assert!(have_lock.held()); - let guardmgr = GuardMgr::new(rt, statemgr.clone()).unwrap(); + let guardmgr = GuardMgr::new(rt, statemgr.clone(), [].into()).unwrap(); let (con, mds) = testnet::construct_network().unwrap(); let override_p = "guard-min-filtered-sample-size=5 guard-n-primary-guards=2" .parse() @@ -1103,7 +1113,7 @@ mod test { drop(guardmgr); // Try reloading from the state... - let guardmgr2 = GuardMgr::new(rt.clone(), statemgr.clone()).unwrap(); + let guardmgr2 = GuardMgr::new(rt.clone(), statemgr.clone(), [].into()).unwrap(); guardmgr2.update_network(&netdir); // Since the guard was confirmed, we should get the same one this time!