diff --git a/Cargo.lock b/Cargo.lock index b3642816c..de3de1811 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -91,6 +91,7 @@ dependencies = [ "derive_builder", "directories", "futures", + "humantime-serde", "hyper", "pin-project", "serde", diff --git a/crates/arti-client/Cargo.toml b/crates/arti-client/Cargo.toml index 632ce5d49..147c7eb4c 100644 --- a/crates/arti-client/Cargo.toml +++ b/crates/arti-client/Cargo.toml @@ -31,6 +31,7 @@ tor-persist = { path="../tor-persist", version = "0.0.2"} tor-proto = { path="../tor-proto", version = "0.0.2"} tor-rtcompat = { path="../tor-rtcompat", version = "0.0.2"} +humantime-serde = "1.0.1" derive_builder = "0.10.2" directories = "4.0.1" futures = "0.3.13" diff --git a/crates/arti-client/src/client.rs b/crates/arti-client/src/client.rs index b7379a853..6439e7b86 100644 --- a/crates/arti-client/src/client.rs +++ b/crates/arti-client/src/client.rs @@ -6,7 +6,7 @@ //! `TorClient::connect()`. use crate::address::IntoTorAddr; -use crate::config::{ClientAddrConfig, TorClientConfig}; +use crate::config::{ClientAddrConfig, ClientTimeoutConfig, TorClientConfig}; use tor_circmgr::{DirInfo, IsolationToken, StreamIsolationBuilder, TargetPort}; use tor_dirmgr::DirEvent; use tor_persist::{FsStateMgr, StateMgr}; @@ -45,6 +45,8 @@ pub struct TorClient { dirmgr: Arc>, /// Client address configuration addrcfg: ClientAddrConfig, + /// Client DNS configuration + timeoutcfg: ClientTimeoutConfig, } /// Preferences for how to route a stream over the Tor network. @@ -171,6 +173,7 @@ impl TorClient { ); } let addr_cfg = config.address_filter.clone(); + let timeout_cfg = config.timeout_rules.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))?; @@ -217,6 +220,7 @@ impl TorClient { circmgr, dirmgr, addrcfg: addr_cfg, + timeoutcfg: timeout_cfg, }) } @@ -257,14 +261,11 @@ impl TorClient { let circ = self.get_or_launch_exit_circ(&exit_ports, &flags).await?; info!("Got a circuit for {}:{}", addr, port); - // TODO: make this configurable. - let stream_timeout = Duration::new(10, 0); - let stream_future = circ.begin_stream(&addr, port, Some(flags.stream_parameters())); // This timeout is needless but harmless for optimistic streams. let stream = self .runtime - .timeout(stream_timeout, stream_future) + .timeout(self.timeoutcfg.stream_timeout, stream_future) .await??; Ok(stream) @@ -282,13 +283,10 @@ impl TorClient { let flags = flags.unwrap_or_default(); let circ = self.get_or_launch_exit_circ(&[], &flags).await?; - // TODO: make this configurable. - let resolve_timeout = Duration::new(10, 0); - let resolve_future = circ.resolve(hostname); let addrs = self .runtime - .timeout(resolve_timeout, resolve_future) + .timeout(self.timeoutcfg.resolve_timeout, resolve_future) .await??; Ok(addrs) @@ -305,13 +303,10 @@ impl TorClient { let flags = flags.unwrap_or_default(); let circ = self.get_or_launch_exit_circ(&[], &flags).await?; - // TODO: make this configurable. - let resolve_ptr_timeout = Duration::new(10, 0); - let resolve_ptr_future = circ.resolve_ptr(addr); let hostnames = self .runtime - .timeout(resolve_ptr_timeout, resolve_ptr_future) + .timeout(self.timeoutcfg.resolve_ptr_timeout, resolve_ptr_future) .await??; Ok(hostnames) diff --git a/crates/arti-client/src/config.rs b/crates/arti-client/src/config.rs index e5a961ab1..7460d2631 100644 --- a/crates/arti-client/src/config.rs +++ b/crates/arti-client/src/config.rs @@ -7,6 +7,7 @@ use serde::Deserialize; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; +use std::time::Duration; use tor_config::CfgPath; pub use tor_config::ConfigBuildError; @@ -45,6 +46,33 @@ pub struct ClientAddrConfig { pub(crate) allow_local_addrs: bool, } +/// Configuration for client behavior relating to connection timeouts +/// +/// This type is immutable once constructed. To create an object of this type, +/// use [`ClientTimeoutConfigBuilder`]. +#[derive(Debug, Clone, Builder, Deserialize, Eq, PartialEq)] +#[builder(build_fn(error = "ConfigBuildError"))] +#[serde(deny_unknown_fields)] +#[non_exhaustive] +pub struct ClientTimeoutConfig { + /// How long should we wait before timing out a stream when connecting + /// to a host? + #[builder(default = "default_dns_stream_timeout()")] + #[serde(with = "humantime_serde", default = "default_dns_stream_timeout")] + pub stream_timeout: Duration, + + /// How long should we wait before timing out when resolving a DNS record? + #[builder(default = "default_dns_resolve_timeout()")] + #[serde(with = "humantime_serde", default = "default_dns_resolve_timeout")] + pub resolve_timeout: Duration, + + /// How long should we wait before timing out when resolving a DNS + /// PTR record? + #[builder(default = "default_dns_resolve_ptr_timeout()")] + #[serde(with = "humantime_serde", default = "default_dns_resolve_ptr_timeout")] + pub resolve_ptr_timeout: Duration, +} + // 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 @@ -70,6 +98,47 @@ impl ClientAddrConfig { } } +#[allow(clippy::unwrap_used)] +impl Default for ClientTimeoutConfig { + fn default() -> Self { + ClientTimeoutConfigBuilder::default().build().unwrap() + } +} + +impl From for ClientTimeoutConfigBuilder { + fn from(cfg: ClientTimeoutConfig) -> ClientTimeoutConfigBuilder { + let mut builder = ClientTimeoutConfigBuilder::default(); + builder + .stream_timeout(cfg.stream_timeout) + .resolve_timeout(cfg.resolve_timeout) + .resolve_ptr_timeout(cfg.resolve_ptr_timeout); + + builder + } +} + +impl ClientTimeoutConfig { + /// Return a new [`ClientTimeoutConfigBuilder`]. + pub fn builder() -> ClientTimeoutConfigBuilder { + ClientTimeoutConfigBuilder::default() + } +} + +/// Return the default stream timeout +fn default_dns_stream_timeout() -> Duration { + Duration::new(10, 0) +} + +/// Return the default resolve timeout +fn default_dns_resolve_timeout() -> Duration { + Duration::new(10, 0) +} + +/// Return the default PTR resolve timeout +fn default_dns_resolve_ptr_timeout() -> Duration { + Duration::new(10, 0) +} + /// Configuration for where information should be stored on disk. /// /// By default, cache information will be stored in `${ARTI_CACHE}`, and @@ -193,6 +262,9 @@ pub struct TorClientConfig { /// Rules about which addresses the client is willing to connect to. pub(crate) address_filter: ClientAddrConfig, + + /// Rules about client DNS configuration + pub(crate) timeout_rules: ClientTimeoutConfig, } impl Default for TorClientConfig { @@ -254,6 +326,8 @@ pub struct TorClientConfigBuilder { circuit_timing: circ::CircuitTimingBuilder, /// Inner builder for the `address_filter` section. address_filter: ClientAddrConfigBuilder, + /// Inner builder for the `timeout_rules` section. + timeout_rules: ClientTimeoutConfigBuilder, } impl TorClientConfigBuilder { @@ -285,6 +359,10 @@ impl TorClientConfigBuilder { .address_filter .build() .map_err(|e| e.within("address_filter"))?; + let timeout_rules = self + .timeout_rules + .build() + .map_err(|e| e.within("timeout_rules"))?; Ok(TorClientConfig { tor_network, @@ -295,6 +373,7 @@ impl TorClientConfigBuilder { circuit_preemptive, circuit_timing, address_filter, + timeout_rules, }) } @@ -402,6 +481,7 @@ impl From for TorClientConfigBuilder { circuit_preemptive, circuit_timing, address_filter, + timeout_rules, } = cfg; TorClientConfigBuilder { @@ -413,6 +493,7 @@ impl From for TorClientConfigBuilder { circuit_preemptive: circuit_preemptive.into(), circuit_timing: circuit_timing.into(), address_filter: address_filter.into(), + timeout_rules: timeout_rules.into(), } } } diff --git a/crates/arti-config/src/arti_defaults.toml b/crates/arti-config/src/arti_defaults.toml index b5cce4413..107d709ae 100644 --- a/crates/arti-config/src/arti_defaults.toml +++ b/crates/arti-config/src/arti_defaults.toml @@ -126,3 +126,15 @@ request_loyalty = "50 msec" # Should we allow attempts to make Tor connections to local addresses? allow_local_addrs = false + +# Rules for a circuit timeouts +[timeout_rules] + +# How long should we wait before timing out a stream when connecting to a host? +stream_timeout = "10 sec" + +# How long should we wait before timing out when resolving a DNS record? +resolve_timeout = "10 sec" + +# How long should we wait before timing out when resolving a DNS PTR record? +resolve_ptr_timeout = "10 sec" diff --git a/crates/arti-config/src/options.rs b/crates/arti-config/src/options.rs index 860ecde62..4295df87a 100644 --- a/crates/arti-config/src/options.rs +++ b/crates/arti-config/src/options.rs @@ -3,8 +3,8 @@ use arti_client::config::{ circ, dir::{self, DownloadScheduleConfig, NetworkConfig}, - ClientAddrConfig, ClientAddrConfigBuilder, StorageConfig, StorageConfigBuilder, - TorClientConfig, TorClientConfigBuilder, + ClientAddrConfig, ClientAddrConfigBuilder, ClientTimeoutConfig, ClientTimeoutConfigBuilder, + StorageConfig, StorageConfigBuilder, TorClientConfig, TorClientConfigBuilder, }; use derive_builder::Builder; use serde::Deserialize; @@ -160,6 +160,9 @@ pub struct ArtiConfig { /// Rules about which addresses the client is willing to connect to. address_filter: ClientAddrConfig, + + /// Rules about a client's DNS resolution. + timeout_rules: ClientTimeoutConfig, } impl From for TorClientConfigBuilder { @@ -239,6 +242,8 @@ pub struct ArtiConfigBuilder { circuit_timing: circ::CircuitTimingBuilder, /// Builder for the address_filter section. address_filter: ClientAddrConfigBuilder, + /// Builder for the DNS resolution rules. + timeout_rules: ClientTimeoutConfigBuilder, } impl ArtiConfigBuilder { @@ -272,6 +277,10 @@ impl ArtiConfigBuilder { .address_filter .build() .map_err(|e| e.within("address_filter"))?; + let timeout_rules = self + .timeout_rules + .build() + .map_err(|e| e.within("timeout_rules"))?; Ok(ArtiConfig { proxy, logging, @@ -283,6 +292,7 @@ impl ArtiConfigBuilder { circuit_preemptive, circuit_timing, address_filter, + timeout_rules, }) } @@ -374,6 +384,14 @@ impl ArtiConfigBuilder { pub fn address_filter(&mut self) -> &mut ClientAddrConfigBuilder { &mut self.address_filter } + + /// Return a mutable reference to a [`ClientTimeoutConfigBuilder`]. + /// + /// This section controls how Arti should handle an exit relay's DNS + /// resolution. + pub fn timeout_rules(&mut self) -> &mut ClientTimeoutConfigBuilder { + &mut self.timeout_rules + } } impl From for ArtiConfigBuilder { @@ -389,6 +407,7 @@ impl From for ArtiConfigBuilder { circuit_preemptive: cfg.circuit_preemptive.into(), circuit_timing: cfg.circuit_timing.into(), address_filter: cfg.address_filter.into(), + timeout_rules: cfg.timeout_rules.into(), } } }