diff --git a/crates/tor-circmgr/src/lib.rs b/crates/tor-circmgr/src/lib.rs index 48f915dc3e59fd3981fb92c1ffff08733c467798..b5617832e3cae51742b665f6760c42210f6b49c2 100644 --- a/crates/tor-circmgr/src/lib.rs +++ b/crates/tor-circmgr/src/lib.rs @@ -114,6 +114,9 @@ pub enum DirInfo<'a> { Fallbacks(&'a FallbackList), /// A complete network directory Directory(&'a NetDir), + /// No information: we can only build one-hop paths: and that, only if the + /// guard manager knows some guards or fallbacks. + Nothing, } impl<'a> From<&'a FallbackList> for DirInfo<'a> { @@ -144,8 +147,8 @@ impl<'a> DirInfo<'a> { } match self { - DirInfo::Fallbacks(_) => from_netparams(&NetParameters::default()), DirInfo::Directory(d) => from_netparams(d.params()), + _ => from_netparams(&NetParameters::default()), } } } diff --git a/crates/tor-circmgr/src/path.rs b/crates/tor-circmgr/src/path.rs index 58ce26266ad14a1113286e52852ff86bef76cd8a..b25d8b6dccb52f3272086fa1dc7c9b9214e5be21 100644 --- a/crates/tor-circmgr/src/path.rs +++ b/crates/tor-circmgr/src/path.rs @@ -32,6 +32,8 @@ enum TorPathInner<'a> { /// A single-hop path for use with a directory cache, when we don't have /// a consensus. FallbackOneHop(&'a FallbackDir), + /// A single-hop path taken from an OwnedChanTarget. + OwnedOneHop(OwnedChanTarget), /// A multi-hop path, containing one or more relays. Path(Vec<Relay<'a>>), } @@ -53,6 +55,14 @@ impl<'a> TorPath<'a> { } } + /// Construct a new one-hop path for directory use from an arbitrarily + /// chosen channel target. + pub fn new_one_hop_owned<T: tor_linkspec::ChanTarget>(target: &T) -> Self { + Self { + inner: TorPathInner::OwnedOneHop(OwnedChanTarget::from_chan_target(target)), + } + } + /// Create a new multi-hop path with a given number of ordered relays. pub fn new_multihop(relays: impl IntoIterator<Item = Relay<'a>>) -> Self { Self { @@ -82,6 +92,7 @@ impl<'a> TorPath<'a> { match &self.inner { OneHop(_) => 1, FallbackOneHop(_) => 1, + OwnedOneHop(_) => 1, Path(p) => p.len(), } } @@ -104,6 +115,7 @@ impl<'a> TryFrom<&TorPath<'a>> for OwnedPath { Ok(match &p.inner { FallbackOneHop(h) => OwnedPath::ChannelOnly(OwnedChanTarget::from_chan_target(*h)), OneHop(h) => OwnedPath::Normal(vec![OwnedCircTarget::from_circ_target(h)]), + OwnedOneHop(owned) => OwnedPath::ChannelOnly(owned.clone()), Path(p) if !p.is_empty() => { OwnedPath::Normal(p.iter().map(OwnedCircTarget::from_circ_target).collect()) } diff --git a/crates/tor-circmgr/src/path/dirpath.rs b/crates/tor-circmgr/src/path/dirpath.rs index 3be69f77775bc93e72d26f292d965c66dccf54e3..23b260412fdeed4ffbfb218e0806f005c950a01f 100644 --- a/crates/tor-circmgr/src/path/dirpath.rs +++ b/crates/tor-circmgr/src/path/dirpath.rs @@ -1,6 +1,7 @@ //! Code to construct paths to a directory for non-anonymous downloads use super::TorPath; use crate::{DirInfo, Error, Result}; +use tor_error::bad_api_usage; use tor_guardmgr::{GuardMgr, GuardMonitor, GuardUsable}; use tor_netdir::{Relay, WeightRole}; use tor_rtcompat::Runtime; @@ -42,18 +43,31 @@ impl DirPathBuilder { return Ok((TorPath::new_one_hop(r), None, None)); } } - (DirInfo::Directory(netdir), Some(guardmgr)) => { - // TODO: We might want to use the guardmgr even if - // we don't have a netdir. See arti#220. - guardmgr.update_network(netdir); // possibly unnecessary. + (DirInfo::Nothing, None) => { + return Err(bad_api_usage!( + "Tried to build a one hop path with no directory, fallbacks, or guard manager" + ) + .into()); + } + (dirinfo, Some(guardmgr)) => { + // We use a guardmgr whenever we have one, regardless of whether + // there's a netdir. + // + // That way, we prefer our guards (if they're up) before we default to the fallback directories. + let netdir = match dirinfo { + DirInfo::Directory(netdir) => { + guardmgr.update_network(netdir); // possibly unnecessary. + Some(netdir) + } + _ => None, + }; + let guard_usage = tor_guardmgr::GuardUsageBuilder::default() .kind(tor_guardmgr::GuardUsageKind::OneHopDirectory) .build() .expect("Unable to build directory guard usage"); - let (guard, mon, usable) = guardmgr.select_guard(guard_usage, Some(netdir))?; - if let Some(r) = guard.get_relay(netdir) { - return Ok((TorPath::new_one_hop(r), Some(mon), Some(usable))); - } + let (guard, mon, usable) = guardmgr.select_guard(guard_usage, netdir)?; + return Ok((TorPath::new_one_hop_owned(&guard), Some(mon), Some(usable))); } } Err(Error::NoPath( @@ -142,7 +156,13 @@ mod test { let guards: OptDummyGuardMgr<'_> = None; let err = DirPathBuilder::default().pick_path(&mut rng, dirinfo, guards); - assert!(matches!(err, Err(Error::NoPath(_)))); + dbg!(err.as_ref().err()); + assert!(matches!( + err, + Err(Error::Guard( + tor_guardmgr::PickGuardError::AllFallbacksDown { .. } + )) + )); } #[test] @@ -166,7 +186,7 @@ mod test { let (path, mon, usable) = DirPathBuilder::new() .pick_path(&mut rng, dirinfo, Some(&guards)) .unwrap(); - if let crate::path::TorPathInner::OneHop(relay) = path.inner { + if let crate::path::TorPathInner::OwnedOneHop(relay) = path.inner { distinct_guards.insert(relay.ed_identity().clone()); mon.unwrap().succeeded(); assert!(usable.unwrap().await.unwrap()); diff --git a/crates/tor-circmgr/src/path/exitpath.rs b/crates/tor-circmgr/src/path/exitpath.rs index 3e2746ded7d9850c64e3edb5eb209389c07dd8e0..df57dce9d869bf76beaeb1e2d2889065b5537f8b 100644 --- a/crates/tor-circmgr/src/path/exitpath.rs +++ b/crates/tor-circmgr/src/path/exitpath.rs @@ -125,13 +125,13 @@ impl<'a> ExitPathBuilder<'a> { now: SystemTime, ) -> Result<(TorPath<'a>, Option<GuardMonitor>, Option<GuardUsable>)> { let netdir = match netdir { - DirInfo::Fallbacks(_) => { + DirInfo::Directory(d) => d, + _ => { return Err(bad_api_usage!( - "Tried to build a multihop path using only a list of fallback caches" + "Tried to build a multihop path without a network directory" ) .into()) } - DirInfo::Directory(d) => d, }; let subnet_config = config.subnet_config(); let lifetime = netdir.lifetime();