From f32a10865e89c8754bcc3e51938f710657996328 Mon Sep 17 00:00:00 2001 From: Neel Chauhan Date: Fri, 3 Dec 2021 10:28:42 -0800 Subject: [PATCH] Make DNS fields in arti-client/src/client.rs configurable --- Cargo.lock | 1 + crates/arti-client/Cargo.toml | 1 + crates/arti-client/src/client.rs | 21 +++---- crates/arti-client/src/config.rs | 76 +++++++++++++++++++++++ crates/arti-config/src/arti_defaults.toml | 12 ++++ crates/arti-config/src/options.rs | 20 +++++- 6 files changed, 116 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e1107d22..32e7c3509 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 e5c36bbbb..a0f43da61 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, ClientDNSConfig, 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 + dnscfg: ClientDNSConfig, } /// 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 dns_cfg = config.dns_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, + dnscfg: dns_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.dnscfg.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.dnscfg.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.dnscfg.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 257111385..0f89f6c3e 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,31 @@ pub struct ClientAddrConfig { pub(crate) allow_local_addrs: bool, } +/// Configuration for client behavior relating to DNS +/// +/// This type is immutable once constructed. To create an object of this type, +/// use [`ClientDNSConfigBuilder`]. +#[derive(Debug, Clone, Builder, Deserialize, Eq, PartialEq)] +#[builder(build_fn(error = "ConfigBuildError"))] +#[serde(deny_unknown_fields)] +#[non_exhaustive] +pub struct ClientDNSConfig { + /// The client DNS stream timeout + #[builder(default = "default_dns_stream_timeout()")] + #[serde(with = "humantime_serde", default = "default_dns_stream_timeout")] + pub stream_timeout: Duration, + + /// The client DNS resolve timeout + #[builder(default = "default_dns_resolve_timeout()")] + #[serde(with = "humantime_serde", default = "default_dns_resolve_timeout")] + pub resolve_timeout: Duration, + + /// The client DNS resolve timeout + #[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 +96,47 @@ impl ClientAddrConfig { } } +#[allow(clippy::unwrap_used)] +impl Default for ClientDNSConfig { + fn default() -> Self { + ClientDNSConfigBuilder::default().build().unwrap() + } +} + +impl From for ClientDNSConfigBuilder { + fn from(cfg: ClientDNSConfig) -> ClientDNSConfigBuilder { + let mut builder = ClientDNSConfigBuilder::default(); + builder + .stream_timeout(cfg.stream_timeout) + .resolve_timeout(cfg.resolve_timeout) + .resolve_ptr_timeout(cfg.resolve_ptr_timeout); + + builder + } +} + +impl ClientDNSConfig { + /// Return a new [`ClientDNSConfigBuilder`]. + pub fn builder() -> ClientDNSConfigBuilder { + ClientDNSConfigBuilder::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 @@ -190,6 +257,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) dns_rules: ClientDNSConfig, } impl Default for TorClientConfig { @@ -249,6 +319,8 @@ pub struct TorClientConfigBuilder { circuit_timing: circ::CircuitTimingBuilder, /// Inner builder for the `address_filter` section. address_filter: ClientAddrConfigBuilder, + /// Inner builder for the `dns_rules` section. + dns_rules: ClientDNSConfigBuilder, } impl TorClientConfigBuilder { @@ -276,6 +348,7 @@ impl TorClientConfigBuilder { .address_filter .build() .map_err(|e| e.within("address_filter"))?; + let dns_rules = self.dns_rules.build().map_err(|e| e.within("dns_rules"))?; Ok(TorClientConfig { tor_network, @@ -285,6 +358,7 @@ impl TorClientConfigBuilder { path_rules, circuit_timing, address_filter, + dns_rules, }) } @@ -384,6 +458,7 @@ impl From for TorClientConfigBuilder { path_rules, circuit_timing, address_filter, + dns_rules, } = cfg; TorClientConfigBuilder { @@ -394,6 +469,7 @@ impl From for TorClientConfigBuilder { path_rules: path_rules.into(), circuit_timing: circuit_timing.into(), address_filter: address_filter.into(), + dns_rules: dns_rules.into(), } } } diff --git a/crates/arti-config/src/arti_defaults.toml b/crates/arti-config/src/arti_defaults.toml index 0a5f8f6af..028142e51 100644 --- a/crates/arti-config/src/arti_defaults.toml +++ b/crates/arti-config/src/arti_defaults.toml @@ -110,3 +110,15 @@ request_loyalty = "50 msec" # Should we allow attempts to make Tor connections to local addresses? allow_local_addrs = false + +# Rules for a client's DNS resolution +[dns_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 0449ce505..04000f5cc 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, ClientDNSConfig, ClientDNSConfigBuilder, + StorageConfig, StorageConfigBuilder, TorClientConfig, TorClientConfigBuilder, }; use derive_builder::Builder; use serde::Deserialize; @@ -157,6 +157,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. + dns_rules: ClientDNSConfig, } impl From for TorClientConfigBuilder { @@ -232,6 +235,8 @@ pub struct ArtiConfigBuilder { circuit_timing: circ::CircuitTimingBuilder, /// Builder for the address_filter section. address_filter: ClientAddrConfigBuilder, + /// Builder for the DNS resolution rules. + dns_rules: ClientDNSConfigBuilder, } impl ArtiConfigBuilder { @@ -261,6 +266,7 @@ impl ArtiConfigBuilder { .address_filter .build() .map_err(|e| e.within("address_filter"))?; + let dns_rules = self.dns_rules.build().map_err(|e| e.within("dns_rules"))?; Ok(ArtiConfig { proxy, logging, @@ -271,6 +277,7 @@ impl ArtiConfigBuilder { path_rules, circuit_timing, address_filter, + dns_rules, }) } @@ -355,6 +362,14 @@ impl ArtiConfigBuilder { pub fn address_filter(&mut self) -> &mut ClientAddrConfigBuilder { &mut self.address_filter } + + /// Return a mutable reference to a [`ClientDNSConfigBuilder`]. + /// + /// This section controls how Arti should handle an exit relay's DNS + /// resolution. + pub fn dns_rules(&mut self) -> &mut ClientDNSConfigBuilder { + &mut self.dns_rules + } } impl From for ArtiConfigBuilder { @@ -369,6 +384,7 @@ impl From for ArtiConfigBuilder { path_rules: cfg.path_rules.into(), circuit_timing: cfg.circuit_timing.into(), address_filter: cfg.address_filter.into(), + dns_rules: cfg.dns_rules.into(), } } }