diff --git a/Cargo.lock b/Cargo.lock index e1a5d4429..c1e12c31f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -123,6 +123,7 @@ version = "0.0.1" dependencies = [ "arti-client", "config", + "derive_builder", "once_cell", "regex", "serde", diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index a7d5594cc..a986a7b11 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -159,16 +159,10 @@ impl TorClient { /// /// Return a client once there is enough directory material to /// connect safely over the Tor network. - pub async fn bootstrap( - runtime: R, - TorClientConfig { - state_cfg, - dir_cfg, - circ_cfg, - addr_cfg, - }: TorClientConfig, - ) -> Result> { - let statemgr = FsStateMgr::from_path(state_cfg)?; + pub async fn bootstrap(runtime: R, config: TorClientConfig) -> Result> { + let circ_cfg = config.get_circmgr_config()?; + let dir_cfg = config.get_dirmgr_config()?; + let statemgr = FsStateMgr::from_path(config.storage.expand_state_dir()?)?; if statemgr.try_lock()?.held() { debug!("It appears we have the lock on our state files."); } else { @@ -176,6 +170,7 @@ impl TorClient { "Another process has the lock on our state files. We'll proceed in read-only mode." ); } + let addr_cfg = config.address_filter.clone(); let chanmgr = Arc::new(tor_chanmgr::ChanMgr::new(runtime.clone())); let circmgr = tor_circmgr::CircMgr::new(circ_cfg, statemgr.clone(), &runtime, Arc::clone(&chanmgr))?; diff --git a/crates/arti-client/src/config.rs b/crates/arti-client/src/config.rs index 0a3caf2fd..5d1566294 100644 --- a/crates/arti-client/src/config.rs +++ b/crates/arti-client/src/config.rs @@ -2,10 +2,12 @@ //! //! Some of these are re-exported from lower-level crates. -use crate::{Error, Result}; use derive_builder::Builder; use serde::Deserialize; +use std::collections::HashMap; +use std::path::Path; use std::path::PathBuf; +use tor_config::CfgPath; pub use tor_config::ConfigBuildError; @@ -30,7 +32,7 @@ pub mod dir { /// /// This type is immutable once constructed. To create an object of this type, /// use [`ClientAddrConfigBuilder`]. -#[derive(Debug, Clone, Builder, Deserialize)] +#[derive(Debug, Clone, Builder, Deserialize, Eq, PartialEq)] #[builder(build_fn(error = "ConfigBuildError"))] pub struct ClientAddrConfig { /// Should we allow attempts to make Tor connections to local addresses? @@ -38,6 +40,7 @@ pub struct ClientAddrConfig { /// This option is off by default, since (by default) Tor exits will /// always reject connections to such addresses. #[builder(default)] + #[serde(default)] pub(crate) allow_local_addrs: bool, } @@ -51,6 +54,72 @@ impl Default for ClientAddrConfig { } } +impl From for ClientAddrConfigBuilder { + fn from(cfg: ClientAddrConfig) -> ClientAddrConfigBuilder { + let mut builder = ClientAddrConfigBuilder::default(); + builder.allow_local_addrs(cfg.allow_local_addrs); + builder + } +} + +impl ClientAddrConfig { + /// Return a new [`ClientAddrConfigBuilder`]. + pub fn builder() -> ClientAddrConfigBuilder { + ClientAddrConfigBuilder::default() + } +} + +/// Configuration for where information should be stored on disk. +/// +/// This section is for read/write storage. +#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] +#[serde(deny_unknown_fields)] +#[builder(build_fn(error = "ConfigBuildError"))] +pub struct StorageConfig { + /// Location on disk for cached directory information + #[builder(setter(into))] + cache_dir: CfgPath, + #[builder(setter(into))] + /// Location on disk for less-sensitive persistent state information. + state_dir: CfgPath, +} + +impl StorageConfig { + /// Return a new StorageConfigBuilder. + pub fn builder() -> StorageConfigBuilder { + StorageConfigBuilder::default() + } + + /// Try to expand `state_dir` to be a path buffer. + // TODO(nickm): This won't be public once we're done. + pub fn expand_state_dir(&self) -> Result { + self.state_dir + .path() + .map_err(|e| ConfigBuildError::Invalid { + field: "state_dir".to_owned(), + problem: e.to_string(), + }) + } + /// Try to expand `cache_dir` to be a path buffer. + // TODO(nickm): This won't be public once we're done. + pub fn expand_cache_dir(&self) -> Result { + self.state_dir + .path() + .map_err(|e| ConfigBuildError::Invalid { + field: "cache_dir".to_owned(), + problem: e.to_string(), + }) + } +} + +impl From for StorageConfigBuilder { + fn from(cfg: StorageConfig) -> StorageConfigBuilder { + let mut builder = StorageConfigBuilder::default(); + builder.state_dir(cfg.state_dir).cache_dir(cfg.cache_dir); + builder + } +} + /// A configuration used to bootstrap a [`TorClient`](crate::TorClient). /// /// In order to connect to the Tor network, Arti needs to know a few @@ -66,50 +135,55 @@ impl Default for ClientAddrConfig { /// /// Finally, you can get fine-grained control over the members of a a /// TorClientConfig using [`TorClientConfigBuilder`]. -#[derive(Clone, Debug, Builder)] -#[builder(build_fn(error = "ConfigBuildError"))] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct TorClientConfig { - /// A directory suitable for storing persistent Tor state in. - /// - /// This is distinct from the cache directory set in `dir_cfg`: - /// it is _not_ safe to delete this information regularly. - /// - /// Multiple instances of Arti may share the same state directory. - pub(crate) state_cfg: PathBuf, + /// Information about the Tor network we want to connect to. + tor_network: dir::NetworkConfig, - /// Configuration for the network directory manager. - /// - /// This includes information on how to find and authenticate the - /// Tor network, how to frequently to retry directory downloads, - /// and where to store cached directory information. - pub(crate) dir_cfg: dir::DirMgrConfig, + /// Directories for storing information on disk + pub(crate) storage: StorageConfig, - /// Configuration for the network circuit manager. - /// - /// This includes information about how to build paths through the - /// Tor network, and how to retry failed circuits. - pub(crate) circ_cfg: circ::CircMgrConfig, + /// Information about when and how often to download directory information + download_schedule: dir::DownloadScheduleConfig, - /// Configures how the client interprets addresses on the network. - pub(crate) addr_cfg: ClientAddrConfig, + /// Facility to override network parameters from the values set in the + /// consensus. + override_net_params: HashMap, + + /// Information about how to build paths through the network. + path_rules: circ::PathConfig, + + /// Information about how to retry and expire circuits and request for circuits. + circuit_timing: circ::CircuitTiming, + + /// Rules about which addresses the client is willing to connect to. + pub(crate) address_filter: ClientAddrConfig, } impl TorClientConfig { + /// Return a new TorClientConfigBuilder. + pub fn builder() -> TorClientConfigBuilder { + TorClientConfigBuilder::default() + } + /// Returns a `TorClientConfig` using reasonably sane defaults. /// /// This gies the same result as using `tor_config`'s definitions - /// for `APP_LOCAL_DATA` and `APP_CACHE` for the state and cache + /// for `ARTI_LOCAL_DATA` and `ARTI_CACHE` for the state and cache /// directories respectively. /// /// (On unix, this usually works out to `~/.local/share/arti` and /// `~/.cache/arti`, depending on your environment. We use the /// `directories` crate for reasonable defaults on other platforms.) - pub fn sane_defaults() -> Result { + pub fn sane_defaults() -> Result { // Note: this must stay in sync with project_dirs() in the // tor-config crate. let dirs = directories::ProjectDirs::from("org", "torproject", "Arti").ok_or_else(|| { - Error::Configuration("Could not determine default directories".to_string()) + ConfigBuildError::Invalid { + field: "directories".to_string(), + problem: "Could not determine default directories".to_string(), + } })?; let state_dir = dirs.data_local_dir(); @@ -121,26 +195,191 @@ impl TorClientConfig { /// Returns a `TorClientConfig` using the specified state and cache directories. /// /// All other configuration options are set to their defaults. - pub fn with_directories(state_dir: P, cache_dir: Q) -> Result + pub fn with_directories(state_dir: P, cache_dir: Q) -> Result where - P: Into, - Q: Into, + P: AsRef, + Q: AsRef, { - Ok(Self { - state_cfg: state_dir.into(), - dir_cfg: dir::DirMgrConfig::builder() - .cache_path(cache_dir.into()) - .build() - .map_err(|e| { - Error::Configuration(format!("failed to build DirMgrConfig: {}", e)) - })?, - circ_cfg: Default::default(), - addr_cfg: Default::default(), + let mut builder = Self::builder(); + builder + .storage() + .cache_dir(CfgPath::from_path(cache_dir)) + .state_dir(CfgPath::from_path(state_dir)); + builder.build() + } + + /// Build a DirMgrConfig from this configuration. + pub(crate) fn get_dirmgr_config(&self) -> Result { + let mut dircfg = dir::DirMgrConfigBuilder::default(); + dircfg.network_config(self.tor_network.clone()); + dircfg.schedule_config(self.download_schedule.clone()); + dircfg.cache_path(self.storage.expand_cache_dir()?); + for (k, v) in self.override_net_params.iter() { + dircfg.override_net_param(k.clone(), *v); + } + dircfg.build() + } + + /// Return a [`CircMgrConfig`] object based on the user's selected + /// configuration. + pub(crate) fn get_circmgr_config(&self) -> Result { + let mut builder = circ::CircMgrConfigBuilder::default(); + builder + .path_rules(self.path_rules.clone()) + .circuit_timing(self.circuit_timing.clone()) + .build() + } +} + +/// Builder object used to construct a [`TorClientConfig`]. +/// +/// Unlike other builder types in Arti, this builder works by exposing an +/// inner builder for each section in the [`TorClientConfig`]. +#[derive(Clone, Default)] +pub struct TorClientConfigBuilder { + /// Inner builder for the `tor_network` section. + tor_network: dir::NetworkConfigBuilder, + /// Inner builder for the `storage` section. + storage: StorageConfigBuilder, + /// Inner builder for the `download_schedule` section. + download_schedule: dir::DownloadScheduleConfigBuilder, + /// Inner builder for the `override_net_params` section. + override_net_params: HashMap, + /// Inner builder for the `path_rules` section. + path_rules: circ::PathConfigBuilder, + /// Inner builder for the `circuit_timing` section. + circuit_timing: circ::CircuitTimingBuilder, + /// Inner builder for the `address_filter` section. + address_filter: ClientAddrConfigBuilder, +} + +impl TorClientConfigBuilder { + /// Construct a [`TorClientConfig`] from this builder. + pub fn build(&self) -> Result { + let tor_network = self + .tor_network + .build() + .map_err(|e| e.within("tor_network"))?; + let storage = self.storage.build().map_err(|e| e.within("storage"))?; + let download_schedule = self + .download_schedule + .build() + .map_err(|e| e.within("download_schedule"))?; + let override_net_params = self.override_net_params.clone(); + let path_rules = self + .path_rules + .build() + .map_err(|e| e.within("path_rules"))?; + let circuit_timing = self + .circuit_timing + .build() + .map_err(|e| e.within("circuit_timing"))?; + let address_filter = self + .address_filter + .build() + .map_err(|e| e.within("address_filter"))?; + + Ok(TorClientConfig { + tor_network, + storage, + download_schedule, + override_net_params, + path_rules, + circuit_timing, + address_filter, }) } - /// Return a new builder to construct a `TorClientConfig`. - pub fn builder() -> TorClientConfigBuilder { - TorClientConfigBuilder::default() + /// Return a mutable reference to a + /// [`NetworkConfigBuilder`](dir::NetworkConfigBuilder) + /// to use in configuring the underlying Tor network. + /// + /// Most programs shouldn't need to alter this configuration: it's only for + /// cases when you need to use a nonstandard set of Tor directory authorities + /// and fallback caches. + pub fn tor_network(&mut self) -> &mut dir::NetworkConfigBuilder { + &mut self.tor_network + } + + /// Return a mutable reference to a [`StorageConfigBuilder`]. + /// + /// This section is used to configure the locations where Arti should + /// store files on disk. + pub fn storage(&mut self) -> &mut StorageConfigBuilder { + &mut self.storage + } + + /// Return a mutable reference to a + /// [`DowloadScheduleConfigBuilder`](dir::DownloadScheduleConfigBuilder). + /// + /// This section is used to override Arti's schedule when attempting and + /// retrying to download directory objects. + pub fn download_schedule(&mut self) -> &mut dir::DownloadScheduleConfigBuilder { + &mut self.download_schedule + } + + /// Return a mutable reference to a [`HashMap`] of network parameters + /// that should be used to override those specified in the consensus + /// directory. + /// + /// This section should not usually be used for anything but testing: + /// if you find yourself needing to configure an override here for + /// production use, please consider opening a feature request for it + /// instead. + /// + /// For a complete list of Tor's defined network parameters (not all of + /// which are yet supported by Arti), see + /// [`path-spec.txt`](https://gitlab.torproject.org/tpo/core/torspec/-/blob/main/param-spec.txt). + pub fn override_net_params(&mut self) -> &mut HashMap { + &mut self.override_net_params + } + + /// Return a mutable reference to a [`PathConfigBuilder`](circ::PathConfigBuilder). + /// + /// This section is used to override Arti's rules for selecting which + /// relays should be used in a given circuit. + pub fn path_rules(&mut self) -> &mut circ::PathConfigBuilder { + &mut self.path_rules + } + + /// Return a mutable reference to a [`CircuitTimingBuilder`](circ::CircuitTimingBuilder). + /// + /// This section overrides Arti's rules for deciding how long to use + /// circuits, and when to give up on attempts to launch them. + pub fn circuit_timing(&mut self) -> &mut circ::CircuitTimingBuilder { + &mut self.circuit_timing + } + + /// Return a mutable reference to a [`ClientAddrConfigBuilder`]. + /// + /// This section controls which addresses Arti is willing to launch connections + /// to over the Tor network. Any addresses rejected by this section cause + /// stream attempts to fail before any traffic is sent over the network. + pub fn address_filter(&mut self) -> &mut ClientAddrConfigBuilder { + &mut self.address_filter + } +} + +impl From for TorClientConfigBuilder { + fn from(cfg: TorClientConfig) -> TorClientConfigBuilder { + let TorClientConfig { + tor_network, + storage, + download_schedule, + override_net_params, + path_rules, + circuit_timing, + address_filter, + } = cfg; + + TorClientConfigBuilder { + tor_network: tor_network.into(), + storage: storage.into(), + download_schedule: download_schedule.into(), + override_net_params, + path_rules: path_rules.into(), + circuit_timing: circuit_timing.into(), + address_filter: address_filter.into(), + } } } diff --git a/crates/arti-client/src/err.rs b/crates/arti-client/src/err.rs index 569da14d7..e319b0c08 100644 --- a/crates/arti-client/src/err.rs +++ b/crates/arti-client/src/err.rs @@ -49,7 +49,7 @@ pub enum Error { /// Building configuration for the client failed. #[error("Configuration failed: {0}")] - Configuration(String), + Configuration(#[from] tor_config::ConfigBuildError), } impl From for Error { diff --git a/crates/arti-config/Cargo.toml b/crates/arti-config/Cargo.toml index 463e0a82a..4a113fc2b 100644 --- a/crates/arti-config/Cargo.toml +++ b/crates/arti-config/Cargo.toml @@ -19,6 +19,7 @@ serde = { version = "1.0.124", features = ["derive"] } toml = "0.5.8" regex = { version = "1.5.3", default-features = false, features = ["std"] } thiserror = "1.0.24" +derive_builder = "0.10.2" [dev-dependencies] tempfile = "3.2.0" diff --git a/crates/arti-config/src/arti_defaults.toml b/crates/arti-config/src/arti_defaults.toml index 2c41542ac..0a5f8f6af 100644 --- a/crates/arti-config/src/arti_defaults.toml +++ b/crates/arti-config/src/arti_defaults.toml @@ -32,16 +32,16 @@ trace_filter = "debug" # These paths can use ~ to indicate the user's home directory, or a set # of shell-style variables to indicate platform-specific paths. # -# Supported variables are APP_CACHE, APP_CONFIG, APP_SHARED_DATA, -# APP_LOCAL_DATA, and USER_HOME. +# Supported variables are ARTI_CACHE, ARTI_CONFIG, ARTI_SHARED_DATA, +# ARTI_LOCAL_DATA, and USER_HOME. # # Multiple processes can share the same cache_dir. If they do, one of them # will download directory information for all of the others. # # The state directory is not yet used. [storage] -cache_dir = "${APP_CACHE}" -state_dir = "${APP_LOCAL_DATA}" +cache_dir = "${ARTI_CACHE}" +state_dir = "${ARTI_LOCAL_DATA}" # Replacement values for consensus parameters. This is an advanced option # and you probably should leave it alone. Not all parameters are supported. diff --git a/crates/arti-config/src/lib.rs b/crates/arti-config/src/lib.rs index da1dd2ca8..74a4e364d 100644 --- a/crates/arti-config/src/lib.rs +++ b/crates/arti-config/src/lib.rs @@ -42,7 +42,7 @@ mod cmdline; mod options; pub use cmdline::CmdLine; -pub use options::{ArtiConfig, LoggingConfig, ProxyConfig, StorageConfig}; +pub use options::{ArtiConfig, LoggingConfig, ProxyConfig}; use tor_config::CfgPath; use std::path::{Path, PathBuf}; @@ -91,7 +91,7 @@ fn load_mut>( /// Return a filename for the default user configuration file. pub fn default_config_file() -> Option { - CfgPath::new("${APP_CONFIG}/arti.toml".into()).path().ok() + CfgPath::new("${ARTI_CONFIG}/arti.toml".into()).path().ok() } #[cfg(test)] diff --git a/crates/arti-config/src/options.rs b/crates/arti-config/src/options.rs index 6c01ab655..1e12778d0 100644 --- a/crates/arti-config/src/options.rs +++ b/crates/arti-config/src/options.rs @@ -1,22 +1,22 @@ //! Handling for arti's configuration formats. -use crate::CfgPath; use arti_client::config::{ - circ::{CircMgrConfig, CircMgrConfigBuilder}, - dir::{DirMgrConfig, DirMgrConfigBuilder, DownloadScheduleConfig, NetworkConfig}, - ConfigBuildError, TorClientConfig, + dir::{DownloadScheduleConfig, NetworkConfig}, + StorageConfig, TorClientConfig, TorClientConfigBuilder, }; +use derive_builder::Builder; use serde::Deserialize; use std::collections::HashMap; -use std::path::PathBuf; +use tor_config::ConfigBuildError; /// Default options to use for our configuration. pub(crate) const ARTI_DEFAULTS: &str = concat!(include_str!("./arti_defaults.toml"),); /// Structure to hold our logging configuration options -#[derive(Deserialize, Debug, Clone)] +#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] #[serde(deny_unknown_fields)] #[non_exhaustive] // TODO(nickm) remove public elements when I revise this. +#[builder(build_fn(error = "ConfigBuildError"))] pub struct LoggingConfig { /// Filtering directives that determine tracing levels as described at /// @@ -25,23 +25,63 @@ pub struct LoggingConfig { /// /// Example: "info,tor_proto::channel=trace" // TODO(nickm) remove public elements when I revise this. + #[serde(default = "default_trace_filter")] + #[builder(default = "default_trace_filter()")] pub trace_filter: String, /// Whether to log to journald // TODO(nickm) remove public elements when I revise this. + #[serde(default)] + #[builder(default)] pub journald: bool, } +/// Return a default value for `trace_filter`. +fn default_trace_filter() -> String { + "debug".to_owned() +} + +impl LoggingConfig { + /// Return a new LoggingConfigBuilder + pub fn builder() -> LoggingConfigBuilder { + LoggingConfigBuilder::default() + } +} + +impl From for LoggingConfigBuilder { + fn from(cfg: LoggingConfig) -> LoggingConfigBuilder { + let mut builder = LoggingConfigBuilder::default(); + builder + .trace_filter(cfg.trace_filter) + .journald(cfg.journald); + builder + } +} + /// Configuration for one or more proxy listeners. -#[derive(Deserialize, Debug, Clone)] +#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] #[serde(deny_unknown_fields)] +#[builder(build_fn(error = "ConfigBuildError"))] pub struct ProxyConfig { /// Port to listen on (at localhost) for incoming SOCKS /// connections. + #[serde(default = "default_socks_port")] + #[builder(default = "default_socks_port()")] socks_port: Option, } +/// Return the default value for `socks_port` +#[allow(clippy::unnecessary_wraps)] +fn default_socks_port() -> Option { + Some(9150) +} + impl ProxyConfig { + /// Return a new [`ProxyConfigBuilder`]. + pub fn builder() -> ProxyConfigBuilder { + ProxyConfigBuilder::default() + } + /// Return the configured SOCKS port for this proxy configuration, /// if one is enabled. pub fn socks_port(&self) -> Option { @@ -49,6 +89,14 @@ impl ProxyConfig { } } +impl From for ProxyConfigBuilder { + fn from(cfg: ProxyConfig) -> ProxyConfigBuilder { + let mut builder = ProxyConfigBuilder::default(); + builder.socks_port(cfg.socks_port); + builder + } +} + /// Structure to hold Arti's configuration options, whether from a /// configuration file or the command line. // @@ -60,7 +108,7 @@ impl ProxyConfig { /// /// NOTE: These are NOT the final options or their final layout. /// Expect NO stability here. -#[derive(Deserialize, Debug, Clone)] +#[derive(Deserialize, Debug, Clone, Eq, PartialEq)] #[serde(deny_unknown_fields)] pub struct ArtiConfig { /// Configuration for proxy listeners @@ -94,76 +142,35 @@ pub struct ArtiConfig { address_filter: arti_client::config::ClientAddrConfig, } -/// Configuration for where information should be stored on disk. -/// -/// This section is for read/write storage. -#[derive(Deserialize, Debug, Clone)] -#[serde(deny_unknown_fields)] -pub struct StorageConfig { - /// Location on disk for cached directory information - cache_dir: CfgPath, - /// Location on disk for less-sensitive persistent state information. - state_dir: CfgPath, -} - -impl StorageConfig { - /// Try to expand `state_dir` to be a path buffer. - fn expand_state_dir(&self) -> Result { - self.state_dir - .path() - .map_err(|e| ConfigBuildError::Invalid { - field: "state_dir".to_owned(), - problem: e.to_string(), - }) - } - /// Try to expand `cache_dir` to be a path buffer. - fn expand_cache_dir(&self) -> Result { - self.state_dir - .path() - .map_err(|e| ConfigBuildError::Invalid { - field: "cache_dir".to_owned(), - problem: e.to_string(), - }) +impl From for TorClientConfigBuilder { + fn from(cfg: ArtiConfig) -> TorClientConfigBuilder { + let mut builder = TorClientConfig::builder(); + let ArtiConfig { + storage, + address_filter, + path_rules, + circuit_timing, + override_net_params, + download_schedule, + tor_network, + .. + } = cfg; + *builder.storage() = storage.into(); + *builder.address_filter() = address_filter.into(); + *builder.path_rules() = path_rules.into(); + *builder.circuit_timing() = circuit_timing.into(); + *builder.override_net_params() = override_net_params; + *builder.download_schedule() = download_schedule.into(); + *builder.tor_network() = tor_network.into(); + builder } } impl ArtiConfig { - /// Return a [`DirMgrConfig`] object based on the user's selected - /// configuration. - fn get_dir_config(&self) -> Result { - let mut dircfg = DirMgrConfigBuilder::default(); - dircfg.network_config(self.tor_network.clone()); - dircfg.schedule_config(self.download_schedule.clone()); - dircfg.cache_path(self.storage.expand_cache_dir()?); - for (k, v) in self.override_net_params.iter() { - dircfg.override_net_param(k.clone(), *v); - } - dircfg.build() - } - - /// Return a [`CircMgrConfig`] object based on the user's selected - /// configuration. - fn get_circ_config(&self) -> Result { - let mut builder = CircMgrConfigBuilder::default(); - builder - .path_rules(self.path_rules.clone()) - .circuit_timing(self.circuit_timing.clone()) - .build() - } - /// Construct a [`TorClientConfig`] based on this configuration. pub fn tor_client_config(&self) -> Result { - let statecfg = self.storage.expand_state_dir()?; - let dircfg = self.get_dir_config()?; - let circcfg = self.get_circ_config()?; - let addrcfg = self.address_filter.clone(); - - TorClientConfig::builder() - .state_cfg(statecfg) - .dir_cfg(dircfg) - .circ_cfg(circcfg) - .addr_cfg(addrcfg) - .build() + let builder: TorClientConfigBuilder = self.clone().into(); + builder.build() } /// Return the [`LoggingConfig`] for this configuration. diff --git a/crates/tor-circmgr/src/config.rs b/crates/tor-circmgr/src/config.rs index d51987fee..337e90b54 100644 --- a/crates/tor-circmgr/src/config.rs +++ b/crates/tor-circmgr/src/config.rs @@ -15,7 +15,7 @@ use std::time::Duration; /// /// This type is immutable once constructed. To build one, use /// [`PathConfigBuilder`], or deserialize it from a string. -#[derive(Debug, Clone, Builder, Deserialize)] +#[derive(Debug, Clone, Builder, Deserialize, Eq, PartialEq)] #[builder(build_fn(error = "ConfigBuildError"))] pub struct PathConfig { /// Set the length of a bit-prefix for a default IPv4 subnet-family. @@ -45,6 +45,10 @@ fn ipv6_prefix_default() -> u8 { } impl PathConfig { + /// Return a new [`PathConfigBuilder`]. + pub fn builder(&self) -> PathConfigBuilder { + PathConfigBuilder::default() + } /// Return a subnet configuration based on these rules. pub fn subnet_config(&self) -> tor_netdir::SubnetConfig { tor_netdir::SubnetConfig::new( @@ -62,40 +66,71 @@ impl Default for PathConfig { } } +impl From for PathConfigBuilder { + fn from(cfg: PathConfig) -> PathConfigBuilder { + let mut builder = PathConfigBuilder::default(); + builder + .ipv4_subnet_family_prefix(cfg.ipv4_subnet_family_prefix) + .ipv6_subnet_family_prefix(cfg.ipv6_subnet_family_prefix); + builder + } +} + /// Configuration for circuit timeouts, expiration, and so on. /// /// This type is immutable once constructd. To create an object of this /// type, use [`CircuitTimingBuilder`]. -#[derive(Debug, Clone, Builder, Deserialize)] +#[derive(Debug, Clone, Builder, Deserialize, Eq, PartialEq)] #[builder(build_fn(error = "ConfigBuildError"))] pub struct CircuitTiming { /// How long after a circuit has first been used should we give /// it out for new requests? - #[builder(default = "Duration::from_secs(60 * 10)")] - #[serde(with = "humantime_serde")] + #[builder(default = "default_max_dirtiness()")] + #[serde(with = "humantime_serde", default = "default_max_dirtiness")] pub(crate) max_dirtiness: Duration, /// When a circuit is requested, we stop retrying new circuits /// after this much time. // TODO: Impose a maximum or minimum? - #[builder(default = "Duration::from_secs(60)")] - #[serde(with = "humantime_serde")] + #[builder(default = "default_request_timeout()")] + #[serde(with = "humantime_serde", default = "default_request_timeout")] pub(crate) request_timeout: Duration, /// When a circuit is requested, we stop retrying new circuits after /// this many attempts. // TODO: Impose a maximum or minimum? - #[builder(default = "32")] + #[builder(default = "default_request_max_retries()")] + #[serde(default = "default_request_max_retries")] pub(crate) request_max_retries: u32, /// When waiting for requested circuits, wait at least this long /// before using a suitable-looking circuit launched by some other /// request. - #[builder(default = "Duration::from_millis(50)")] - #[serde(with = "humantime_serde")] + #[builder(default = "default_request_loyalty()")] + #[serde(with = "humantime_serde", default = "default_request_loyalty")] pub(crate) request_loyalty: Duration, } +/// Return the default value for `max_dirtiness`. +fn default_max_dirtiness() -> Duration { + Duration::from_secs(60 * 10) +} + +/// Return the default value for `request_timeout`. +fn default_request_timeout() -> Duration { + Duration::from_secs(60) +} + +/// Return the default value for `request_max_retries`. +fn default_request_max_retries() -> u32 { + 32 +} + +/// Return the default request loyalty timeout. +fn default_request_loyalty() -> Duration { + Duration::from_millis(50) +} + // NOTE: it seems that `unwrap` may be safe because of builder defaults // check `derive_builder` documentation for details // https://docs.rs/derive_builder/0.10.2/derive_builder/#default-values @@ -106,6 +141,25 @@ impl Default for CircuitTiming { } } +impl CircuitTiming { + /// Return a new [`CircuitTimingBuilder`] + pub fn builder() -> CircuitTimingBuilder { + CircuitTimingBuilder::default() + } +} + +impl From for CircuitTimingBuilder { + fn from(cfg: CircuitTiming) -> CircuitTimingBuilder { + let mut builder = CircuitTimingBuilder::default(); + builder + .max_dirtiness(cfg.max_dirtiness) + .request_timeout(cfg.request_timeout) + .request_max_retries(cfg.request_max_retries) + .request_loyalty(cfg.request_loyalty); + builder + } +} + /// Configuration for a circuit manager. /// /// This configuration includes information about how to build paths @@ -116,7 +170,7 @@ impl Default for CircuitTiming { /// this type, use [`CircMgrConfigBuilder`], or deserialize it from a /// string. (Arti generally uses Toml for configuration, but you can /// use other formats if you prefer.) -#[derive(Debug, Clone, Builder, Default)] +#[derive(Debug, Clone, Builder, Default, Eq, PartialEq)] #[builder(build_fn(error = "ConfigBuildError"))] pub struct CircMgrConfig { /// Override the default required distance for two relays to share @@ -128,3 +182,10 @@ pub struct CircMgrConfig { #[builder(default)] pub(crate) circuit_timing: CircuitTiming, } + +impl CircMgrConfig { + /// Return a new [`CircMgrConfigBuilder`]. + pub fn builder() -> CircMgrConfigBuilder { + CircMgrConfigBuilder::default() + } +} diff --git a/crates/tor-config/src/path.rs b/crates/tor-config/src/path.rs index b0a65da34..1251bafcd 100644 --- a/crates/tor-config/src/path.rs +++ b/crates/tor-config/src/path.rs @@ -13,20 +13,30 @@ use serde::Deserialize; /// with expansion of certain variables. /// /// The supported variables are: -/// * `APP_CACHE`: an arti-specific cache directory. -/// * `APP_CONFIG`: an arti-specific configuration directory. -/// * `APP_SHARED_DATA`: an arti-specific directory in the user's "shared +/// * `ARTI_CACHE`: an arti-specific cache directory. +/// * `ARTI_CONFIG`: an arti-specific configuration directory. +/// * `ARTI_SHARED_DATA`: an arti-specific directory in the user's "shared /// data" space. -/// * `APP_LOCAL_DATA`: an arti-specific directory in the user's "local +/// * `ARTI_LOCAL_DATA`: an arti-specific directory in the user's "local /// data" space. /// * `USER_HOME`: the user's home directory. /// /// These variables are implemented using the `directories` crate, and /// so should use appropriate system-specific overrides under the /// hood. -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] #[serde(transparent)] -pub struct CfgPath(String); +pub struct CfgPath(PathInner); + +/// Inner implementation of CfgPath +#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] +#[serde(untagged)] +enum PathInner { + /// A path that should be expanded from a string using ShellExpand. + Shell(String), + /// A path that should be used literally, with no expansion. + Literal(PathBuf), +} /// An error that has occurred while expanding a path. #[derive(thiserror::Error, Debug, Clone)] @@ -47,30 +57,47 @@ pub enum CfgPathError { /// be fixed in the future.) #[error("can't convert value of {0} to UTF-8")] BadUtf8(String), + /// We couldn't convert a string to a valid path on the OS. + #[error("invalid path string: {0:?}")] + InvalidString(String), } impl CfgPath { /// Create a new configuration path pub fn new(s: String) -> Self { - CfgPath(s) + CfgPath(PathInner::Shell(s)) } /// Return the path on disk designated by this `CfgPath`. - #[cfg(feature = "expand-paths")] pub fn path(&self) -> Result { - Ok(shellexpand::full_with_context(&self.0, get_home, get_env) - .map_err(|e| e.cause)? - .into_owned() - .into()) + match &self.0 { + PathInner::Shell(s) => expand(s), + PathInner::Literal(p) => Ok(p.clone()), + } } - /// Return the path on disk designated by this `CfgPath`. - #[cfg(not(feature = "expand-paths"))] - pub fn path(&self) -> Result { - Ok(self.0.into()) + /// Construct a new `CfgPath` from a system path. + pub fn from_path>(path: P) -> Self { + CfgPath(PathInner::Literal(path.as_ref().to_owned())) } } +/// Helper: expand a directory given as a string. +#[cfg(feature = "expand-paths")] +fn expand(s: &str) -> Result { + Ok(shellexpand::full_with_context(s, get_home, get_env) + .map_err(|e| e.cause)? + .into_owned() + .into()) +} + +/// Helper: convert a string to a path without expansion. +#[cfg(not(feature = "expand-paths"))] +fn expand(s: &str) -> Result { + s.try_into() + .map_err(|_| CfgPathError::InvalidString(s.to_owned())) +} + /// Shellexpand helper: return the user's home directory if we can. #[cfg(feature = "expand-paths")] fn get_home() -> Option<&'static Path> { @@ -81,10 +108,10 @@ fn get_home() -> Option<&'static Path> { #[cfg(feature = "expand-paths")] fn get_env(var: &str) -> Result, CfgPathError> { let path = match var { - "APP_CACHE" => project_dirs()?.cache_dir(), - "APP_CONFIG" => project_dirs()?.config_dir(), - "APP_SHARED_DATA" => project_dirs()?.data_dir(), - "APP_LOCAL_DATA" => project_dirs()?.data_local_dir(), + "ARTI_CACHE" => project_dirs()?.cache_dir(), + "ARTI_CONFIG" => project_dirs()?.config_dir(), + "ARTI_SHARED_DATA" => project_dirs()?.data_dir(), + "ARTI_LOCAL_DATA" => project_dirs()?.data_local_dir(), "USER_HOME" => base_dirs()?.home_dir(), _ => return Err(CfgPathError::UnknownVar(var.to_owned())), }; @@ -102,7 +129,10 @@ fn get_env(var: &str) -> Result, CfgPathError> { impl std::fmt::Display for CfgPath { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.fmt(fmt) + match &self.0 { + PathInner::Literal(p) => write!(fmt, "{:?}", p), + PathInner::Shell(s) => s.fmt(fmt), + } } } @@ -158,8 +188,8 @@ mod test { #[test] fn expand_cache() { - let p = CfgPath::new("${APP_CACHE}/example".to_string()); - assert_eq!(p.to_string(), "${APP_CACHE}/example".to_string()); + let p = CfgPath::new("${ARTI_CACHE}/example".to_string()); + assert_eq!(p.to_string(), "${ARTI_CACHE}/example".to_string()); let expected = project_dirs().unwrap().cache_dir().join("example"); assert_eq!(p.path().unwrap().to_str(), expected.to_str()); @@ -167,8 +197,8 @@ mod test { #[test] fn expand_bogus() { - let p = CfgPath::new("${APP_WOMBAT}/example".to_string()); - assert_eq!(p.to_string(), "${APP_WOMBAT}/example".to_string()); + let p = CfgPath::new("${ARTI_WOMBAT}/example".to_string()); + assert_eq!(p.to_string(), "${ARTI_WOMBAT}/example".to_string()); assert!(p.path().is_err()); } diff --git a/crates/tor-dirmgr/src/authority.rs b/crates/tor-dirmgr/src/authority.rs index 2343cc327..8c5b432fa 100644 --- a/crates/tor-dirmgr/src/authority.rs +++ b/crates/tor-dirmgr/src/authority.rs @@ -13,7 +13,7 @@ use tor_netdoc::doc::authcert::{AuthCert, AuthCertKeyIds}; // Note that we do *not* set serde(deny_unknown_fields)] on this structure: // we want our authorities format to be future-proof against adding new info // about each authority. -#[derive(Deserialize, Debug, Clone, Builder)] +#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] pub struct Authority { /// A memorable nickname for this authority. #[builder(setter(into))] diff --git a/crates/tor-dirmgr/src/config.rs b/crates/tor-dirmgr/src/config.rs index 3b0d182ee..03205809b 100644 --- a/crates/tor-dirmgr/src/config.rs +++ b/crates/tor-dirmgr/src/config.rs @@ -26,7 +26,7 @@ use serde::Deserialize; /// /// This type is immutable once constructed. To make one, use /// [`NetworkConfigBuilder`], or deserialize it from a string. -#[derive(Deserialize, Debug, Clone, Builder)] +#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] #[serde(deny_unknown_fields)] #[builder(build_fn(validate = "Self::validate", error = "ConfigBuildError"))] pub struct NetworkConfig { @@ -58,6 +58,16 @@ impl Default for NetworkConfig { } } +impl From for NetworkConfigBuilder { + fn from(cfg: NetworkConfig) -> NetworkConfigBuilder { + let mut builder = NetworkConfigBuilder::default(); + builder + .fallback_caches(cfg.fallback_caches) + .authorities(cfg.authorities); + builder + } +} + impl NetworkConfig { /// Return a new builder to construct a NetworkConfig. pub fn builder() -> NetworkConfigBuilder { @@ -93,7 +103,7 @@ impl NetworkConfigBuilder { /// /// This type is immutable once constructed. To make one, use /// [`DownloadScheduleConfigBuilder`], or deserialize it from a string. -#[derive(Deserialize, Debug, Clone, Builder)] +#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)] #[serde(deny_unknown_fields)] #[builder(build_fn(error = "ConfigBuildError"))] pub struct DownloadScheduleConfig { @@ -130,12 +140,9 @@ fn default_microdesc_schedule() -> DownloadSchedule { impl Default for DownloadScheduleConfig { fn default() -> Self { - DownloadScheduleConfig { - retry_bootstrap: default_retry_bootstrap(), - retry_consensus: Default::default(), - retry_certs: Default::default(), - retry_microdescs: Default::default(), - } + Self::builder() + .build() + .expect("default builder setting didn't work") } } @@ -146,6 +153,18 @@ impl DownloadScheduleConfig { } } +impl From for DownloadScheduleConfigBuilder { + fn from(cfg: DownloadScheduleConfig) -> DownloadScheduleConfigBuilder { + let mut builder = DownloadScheduleConfigBuilder::default(); + builder + .retry_bootstrap(cfg.retry_bootstrap) + .retry_consensus(cfg.retry_consensus) + .retry_certs(cfg.retry_certs) + .retry_microdescs(cfg.retry_microdescs); + builder + } +} + /// Configuration type for network directory operations. /// /// This type is immutable once constructed. @@ -153,7 +172,7 @@ impl DownloadScheduleConfig { /// To create an object of this type, use [`DirMgrConfigBuilder`], or /// deserialize it from a string. (Arti generally uses Toml for /// configuration, but you can use other formats if you prefer.) -#[derive(Debug, Clone, Builder)] +#[derive(Debug, Clone, Builder, Eq, PartialEq)] #[builder(build_fn(error = "ConfigBuildError"))] pub struct DirMgrConfig { /// Location to use for storing and reading current-format diff --git a/crates/tor-netdir/src/fallback.rs b/crates/tor-netdir/src/fallback.rs index adf03b520..3de0d9747 100644 --- a/crates/tor-netdir/src/fallback.rs +++ b/crates/tor-netdir/src/fallback.rs @@ -25,7 +25,7 @@ use std::net::SocketAddr; // Note that we do *not* set serde(deny_unknown_fields) on this // structure: we want our fallback directory configuration format to // be future-proof against adding new info about each fallback. -#[derive(Debug, Clone, Deserialize, Builder)] +#[derive(Debug, Clone, Deserialize, Builder, Eq, PartialEq)] #[builder(build_fn(validate = "FallbackDirBuilder::validate", error = "ConfigBuildError"))] pub struct FallbackDir { /// RSA identity for the directory relay diff --git a/crates/tor-netdoc/src/doc/netstatus.rs b/crates/tor-netdoc/src/doc/netstatus.rs index 17c2e0891..7615fc325 100644 --- a/crates/tor-netdoc/src/doc/netstatus.rs +++ b/crates/tor-netdoc/src/doc/netstatus.rs @@ -142,7 +142,7 @@ impl Lifetime { /// These are used to describe current settings for the Tor network, /// current weighting parameters for path selection, and so on. They're /// encoded with a space-separated K=V format. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone, Default, Eq, PartialEq)] pub struct NetParams { /// Map from keys to values. params: HashMap,