From 962b6c32e153ad6bb1a2b9014266aa32c319144e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 22 Apr 2022 17:22:24 +0100 Subject: [PATCH] fallback list: Introduce and use FallbackListBuilder Now the network fallbacks configuration wants to Deserialize a Vec, rather than validated Vec. Methods on FallbackListBuilder are as per https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/462#note_2797697 mutatis mutandi for the fact that this struct has only fallbacks in it. --- crates/arti-client/src/config.rs | 6 +-- crates/arti/src/cfg.rs | 6 +-- crates/tor-dirmgr/src/config.rs | 23 +++++------ crates/tor-dirmgr/src/state.rs | 2 +- crates/tor-guardmgr/src/fallback.rs | 13 +++--- crates/tor-guardmgr/src/fallback/set.rs | 54 ++++++++++++++++++++++++- doc/semver_status.md | 4 ++ 7 files changed, 79 insertions(+), 29 deletions(-) diff --git a/crates/arti-client/src/config.rs b/crates/arti-client/src/config.rs index 7997c27b3..c3782a9d0 100644 --- a/crates/arti-client/src/config.rs +++ b/crates/arti-client/src/config.rs @@ -526,13 +526,13 @@ mod test { .rsa_identity([23; 20].into()) .ed_identity([99; 32].into()) .orports(vec!["127.0.0.7:7".parse().unwrap()]) - .build() - .unwrap(); + .clone(); let mut bld = TorClientConfig::builder(); bld.tor_network() .authorities(vec![auth]) - .fallback_caches(vec![fallback]); + .fallback_caches() + .set(vec![fallback]); bld.storage() .cache_dir(CfgPath::new("/var/tmp/foo".to_owned())) .state_dir(CfgPath::new("/var/tmp/bar".to_owned())); diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs index ad7ea9476..f3813ca44 100644 --- a/crates/arti/src/cfg.rs +++ b/crates/arti/src/cfg.rs @@ -309,8 +309,7 @@ mod test { .rsa_identity([23; 20].into()) .ed_identity([99; 32].into()) .orports(vec!["127.0.0.7:7".parse().unwrap()]) - .build() - .unwrap(); + .clone(); let mut bld = ArtiConfig::builder(); bld.proxy().socks_port(Some(9999)); @@ -318,7 +317,8 @@ mod test { bld.tor() .tor_network() .authorities(vec![auth]) - .fallback_caches(vec![fallback]); + .fallback_caches() + .set(vec![fallback]); bld.tor() .storage() .cache_dir(CfgPath::new("/var/tmp/foo".to_owned())) diff --git a/crates/tor-dirmgr/src/config.rs b/crates/tor-dirmgr/src/config.rs index d63754e72..59033663a 100644 --- a/crates/tor-dirmgr/src/config.rs +++ b/crates/tor-dirmgr/src/config.rs @@ -12,6 +12,7 @@ use crate::retry::DownloadSchedule; use crate::storage::DynStore; use crate::{Authority, Result}; use tor_config::ConfigBuildError; +use tor_guardmgr::fallback::FallbackListBuilder; use tor_netdoc::doc::netstatus; use derive_builder::Builder; @@ -39,11 +40,11 @@ pub struct NetworkConfig { /// /// This section can be changed in a running Arti client. Doing so will /// affect future download attempts only. - #[serde(default = "fallbacks::default_fallbacks")] - #[builder(default = "fallbacks::default_fallbacks()")] + #[builder(sub_builder)] + #[serde(default)] #[serde(rename = "fallback_caches")] #[builder_field_attr(serde(rename = "fallback_caches"))] - #[builder(setter(into, name = "fallback_caches"))] + #[builder(setter(name = "fallback_caches"))] pub(crate) fallbacks: tor_guardmgr::fallback::FallbackList, /// List of directory authorities which we expect to sign consensus @@ -61,7 +62,9 @@ pub struct NetworkConfig { impl Default for NetworkConfig { fn default() -> Self { NetworkConfig { - fallbacks: fallbacks::default_fallbacks(), + fallbacks: FallbackListBuilder::default() + .build() + .expect("build default fallbacks"), authorities: crate::authority::default_authorities(), } } @@ -82,7 +85,7 @@ impl NetworkConfig { impl NetworkConfigBuilder { /// Check that this builder will give a reasonable network. fn validate(&self) -> std::result::Result<(), ConfigBuildError> { - if self.authorities.is_some() && self.fallbacks.is_none() { + if self.authorities.is_some() && self.fallbacks.is_unmodified_default() { return Err(ConfigBuildError::Inconsistent { fields: vec!["authorities".to_owned(), "fallbacks".to_owned()], problem: "Non-default authorities are use, but the fallback list is not overridden" @@ -310,11 +313,6 @@ impl DownloadScheduleConfig { } } -/// Compatibility alias, will go away in a moment -mod fallbacks { - pub(crate) use tor_guardmgr::fallback::default_fallbacks; -} - #[cfg(test)] mod test { #![allow(clippy::unwrap_used)] @@ -367,13 +365,12 @@ mod test { ]); assert!(bld.build().is_err()); - bld.fallback_caches(vec![FallbackDir::builder() + bld.fallback_caches().set(vec![FallbackDir::builder() .rsa_identity([b'x'; 20].into()) .ed_identity([b'y'; 32].into()) .orport("127.0.0.1:99".parse().unwrap()) .orport("[::]:99".parse().unwrap()) - .build() - .unwrap()]); + .clone()]); let cfg = bld.build().unwrap(); assert_eq!(cfg.authorities.len(), 2); assert_eq!(cfg.fallbacks.len(), 1); diff --git a/crates/tor-dirmgr/src/state.rs b/crates/tor-dirmgr/src/state.rs index 00df1a9e7..3081ced23 100644 --- a/crates/tor-dirmgr/src/state.rs +++ b/crates/tor-dirmgr/src/state.rs @@ -1012,7 +1012,7 @@ mod test { impl DirRcv { fn new(now: SystemTime, authorities: Option>) -> Self { let mut netcfg = crate::NetworkConfig::builder(); - netcfg.fallback_caches(vec![]); + netcfg.fallback_caches().set(vec![]); if let Some(a) = authorities { netcfg.authorities(a); } diff --git a/crates/tor-guardmgr/src/fallback.rs b/crates/tor-guardmgr/src/fallback.rs index 32bfa4d07..97d785354 100644 --- a/crates/tor-guardmgr/src/fallback.rs +++ b/crates/tor-guardmgr/src/fallback.rs @@ -22,8 +22,8 @@ use serde::Deserialize; use std::net::SocketAddr; use crate::dirstatus::DirStatus; -pub use set::FallbackList; pub(crate) use set::FallbackState; +pub use set::{FallbackList, FallbackListBuilder}; /// A directory whose location ships with Tor (or arti), and which we /// can use for bootstrapping when we don't know anything else about @@ -90,11 +90,9 @@ impl FallbackDirBuilder { /// Return a list of the default fallback directories shipped with /// arti. -// -// TODO: this shouldn't be exposed in the public API; we will deal with that in a moment -pub fn default_fallbacks() -> FallbackList { +pub(crate) fn default_fallbacks() -> Vec { /// Build a fallback directory; panic if input is bad. - fn fallback(rsa: &str, ed: &str, ports: &[&str]) -> FallbackDir { + fn fallback(rsa: &str, ed: &str, ports: &[&str]) -> FallbackDirBuilder { let rsa = RsaIdentity::from_hex(rsa).expect("Bad hex in built-in fallback list"); let ed = base64::decode_config(ed, base64::STANDARD_NO_PAD) .expect("Bad hex in built-in fallback list"); @@ -109,10 +107,9 @@ pub fn default_fallbacks() -> FallbackList { bld.orport(p); }); - bld.build() - .expect("Unable to build default fallback directory!?") + bld } - include!("fallback_dirs.inc").into() + include!("fallback_dirs.inc") } impl tor_linkspec::ChanTarget for FallbackDir { diff --git a/crates/tor-guardmgr/src/fallback/set.rs b/crates/tor-guardmgr/src/fallback/set.rs index 67c950cb2..c7cdb1ca1 100644 --- a/crates/tor-guardmgr/src/fallback/set.rs +++ b/crates/tor-guardmgr/src/fallback/set.rs @@ -4,9 +4,11 @@ use crate::skew::SkewObservation; use rand::seq::IteratorRandom; use std::time::{Duration, Instant}; -use super::{DirStatus, FallbackDir}; +use super::{DirStatus, FallbackDir, FallbackDirBuilder}; +use crate::fallback::default_fallbacks; use crate::{ids::FallbackId, PickGuardError}; use serde::Deserialize; +use tor_config::ConfigBuildError; /// A list of fallback directories. /// @@ -28,6 +30,56 @@ impl> From for FallbackList { } } +/// List of fallback directories, being built as part of the configuration +/// +/// Fallback directories (represented by [`FallbackDir`]) are used by Tor +/// clients when they don't already have enough other directory information to +/// contact the network. +/// +/// This is a builder pattern struct which will be resolved into a `FallbackList` +/// during configuration resolution, via the [`build`](FallbackListBuilder::build) method. +#[derive(Default, Clone, Deserialize)] +#[serde(transparent)] +pub struct FallbackListBuilder { + /// The fallbacks, as overridden + pub(crate) fallbacks: Option>, +} + +impl FallbackListBuilder { + /// Append `fallback` to the list + pub fn append(&mut self, fallback: FallbackDirBuilder) { + self.fallbacks + .get_or_insert_with(default_fallbacks) + .push(fallback); + } + + /// Replace the list with the supplied one + pub fn set(&mut self, fallbacks: impl IntoIterator) { + self.fallbacks = Some(fallbacks.into_iter().collect()); + } + + /// Checks whether any calls have been made to set or adjust the set + pub fn is_unmodified_default(&self) -> bool { + self.fallbacks.is_none() + } + + /// Resolve into a `FallbackList` + pub fn build(&self) -> Result { + let default_buffer; + let fallbacks = if let Some(f) = &self.fallbacks { + f + } else { + default_buffer = default_fallbacks(); + &default_buffer + }; + let fallbacks = fallbacks + .iter() + .map(|item| item.build()) + .collect::>()?; + Ok(FallbackList { fallbacks }) + } +} + impl FallbackList { /// Return the number of fallbacks in this list. pub fn len(&self) -> usize { diff --git a/doc/semver_status.md b/doc/semver_status.md index 5520e632b..bc8f95132 100644 --- a/doc/semver_status.md +++ b/doc/semver_status.md @@ -18,6 +18,10 @@ We can delete older sections here after we bump the releases. ## Since Arti 0.2.0 +### configuration (affecting arti, arti-client, tor-dirmgr, tor-guardmgr + +BREAKING: Configuration of fallback directories overhauled; now uses FalllbadkDirBuilder more. + ### tor-basic-util MODIFIED: Added `reset()` method to RetrySchedule.