From 589f74a78f05f0d9fbdc9844d6686b0cbb60878d Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 18 Nov 2022 16:32:11 +0000 Subject: [PATCH] BridgeConfigBuilder: Introduce the struct with the dictionary repr This struct is going to be the principal "dictionary-style" serde representation for a bridge, and the builder, making this all in keeping with our usual approach. In this commit: * Introduce the struct (defining the serialisation) * Provide the setters (defining the Rust API) * Add success test cases (not all of the data in which is used yet) --- Cargo.lock | 1 + crates/tor-guardmgr/Cargo.toml | 2 + crates/tor-guardmgr/src/bridge.rs | 2 +- crates/tor-guardmgr/src/bridge/config.rs | 127 ++++++++++++++++++++++- crates/tor-guardmgr/src/lib.rs | 10 ++ 5 files changed, 136 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa512f53f..013d0933b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3853,6 +3853,7 @@ dependencies = [ "retain_mut", "safelog", "serde", + "serde_json", "strum", "thiserror", "tor-basic-utils", diff --git a/crates/tor-guardmgr/Cargo.toml b/crates/tor-guardmgr/Cargo.toml index 5d5386088..dd1e26441 100644 --- a/crates/tor-guardmgr/Cargo.toml +++ b/crates/tor-guardmgr/Cargo.toml @@ -62,10 +62,12 @@ tracing = "0.1.18" [dev-dependencies] float_eq = "1.0.0" +serde_json = "1.0.50" tor-netdir = { path = "../tor-netdir", version = "0.6.0", features = ["testing"] } tor-netdoc = { path = "../tor-netdoc", version = "0.5.2" } tor-persist = { path = "../tor-persist", version = "0.5.1", features = ["testing"] } tor-rtcompat = { path = "../tor-rtcompat", version = "0.7.0", features = ["tokio", "native-tls"] } + [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/crates/tor-guardmgr/src/bridge.rs b/crates/tor-guardmgr/src/bridge.rs index 148990cc0..b9e6dc480 100644 --- a/crates/tor-guardmgr/src/bridge.rs +++ b/crates/tor-guardmgr/src/bridge.rs @@ -11,7 +11,7 @@ mod config; mod descs; mod relay; -pub use config::{BridgeConfig, BridgeParseError}; +pub use config::{BridgeConfig, BridgeConfigBuilder, BridgeParseError}; pub use descs::{BridgeDesc, BridgeDescError, BridgeDescEvent, BridgeDescList, BridgeDescProvider}; pub use relay::BridgeRelay; diff --git a/crates/tor-guardmgr/src/bridge/config.rs b/crates/tor-guardmgr/src/bridge/config.rs index 37a426609..0802bfcf4 100644 --- a/crates/tor-guardmgr/src/bridge/config.rs +++ b/crates/tor-guardmgr/src/bridge/config.rs @@ -4,15 +4,19 @@ use std::fmt::{self, Display}; use std::net::SocketAddr; use std::str::FromStr; +use serde::{Deserialize, Serialize}; use thiserror::Error; +use tor_config::define_list_builder_accessors; use tor_linkspec::{ChanTarget, ChannelMethod, HasChanMethod}; use tor_linkspec::{HasAddrs, HasRelayIds, RelayIdRef, RelayIdType}; use tor_linkspec::{RelayId, RelayIdError, TransportIdError}; use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity}; +use tor_linkspec::BridgeAddr; + #[cfg(feature = "pt-client")] -use tor_linkspec::{BridgeAddr, PtAddrError, PtTarget, PtTargetInvalidSetting}; +use tor_linkspec::{PtAddrError, PtTarget, PtTargetInvalidSetting}; /// A relay not listed on the main tor network, used for anticensorship. /// @@ -68,10 +72,6 @@ pub struct BridgeConfig { /// The Ed25519 identity of the bridge. ed_id: Option, } -// TODO pt-client: when implementing deserialization for this type, make sure -// that it can accommodate a large variety of possible configurations methods, -// and check that the toml looks okay. For discussion see -// https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/704/diffs#note_2835271 impl HasRelayIds for BridgeConfig { fn identity(&self, key_type: RelayIdType) -> Option> { @@ -97,6 +97,72 @@ impl HasAddrs for BridgeConfig { impl ChanTarget for BridgeConfig {} +/// Builder for a `BridgeConfig`. +/// +/// Construct this with [`BridgeConfigBuilder::default()`] or [`BridgeConfig::builder()`], +/// call setter methods, and then call `build().` +// +// `BridgeConfig` contains a `ChannelMethod`. This is convenient for its users, +// but means we can't use `#[derive(Builder)]` to autogenerate this. +#[derive(Deserialize, Serialize, Default, Clone, Debug)] +#[cfg_attr(test, derive(Eq, PartialEq))] +pub struct BridgeConfigBuilder { + /// The `PtTransportName`, but not yet parsed or checked. + /// + /// `""` and `"-"` and `"bridge"` all mean "do not use a pluggable transport". + transport: Option, + + /// Host:ORPort + addrs: Option>, + + /// IDs + ids: Option>, + + /// Settings (for the transport) + settings: Option>, +} + +impl BridgeConfig { + /// Make a builder, for constructing a `BridgeConfig` + pub fn builder() -> BridgeConfigBuilder { + BridgeConfigBuilder::default() + } +} + +impl BridgeConfigBuilder { + /// Set the transport protocol name (eg, a pluggable transport) to use. + /// + /// The empty string `""`, a single hyphen `"-"`, and the word `"bridge"`, + /// all mean to connect directly; + /// i.e., passing one of this is equivalent to + /// calling [`direct()`](BridgeConfigBuilder::direct). + /// + /// The value is not checked at this point. + pub fn transport(&mut self, transport: impl Into) -> &mut Self { + self.transport = Some(transport.into()); + self + } + + /// Specify to use a direct connection. + pub fn direct(&mut self) -> &mut Self { + self.transport("") + } + + /// Add a pluggable transport setting + pub fn push_setting(&mut self, k: impl Into, v: impl Into) -> &mut Self { + self.settings().push((k.into(), v.into())); + self + } +} + +define_list_builder_accessors! { + struct BridgeConfigBuilder { + pub addrs: [BridgeAddr], + pub ids: [RelayId], + pub settings: [(String,String)], + } +} + /// Error when parsing a bridge line from a string #[derive(Error, Clone, Debug)] #[non_exhaustive] @@ -574,4 +640,55 @@ mod test { "More than one identity of the same type specified", ); } + + #[test] + fn config_api() { + let chk_bridgeline = |line: &str, jsons: &[&str], f: &dyn Fn(&mut BridgeConfigBuilder)| { + eprintln!(" ---- chk_bridgeline ----\n{}", line); + + let mut bcb = BridgeConfigBuilder::default(); + f(&mut bcb); + + // TODO: Test building (when that is implemente)d + // TODO: Test parsing bridge lines into BridgeConfigBuilder (when that is implemented) + // TODO: Test reserialsation (when that is implemented) + + for json in jsons { + let from_dict: BridgeConfigBuilder = serde_json::from_str(json).unwrap(); + assert_eq!(&from_dict, &bcb); + } + }; + + chk_bridgeline( + "38.229.33.83:80 $0bac39417268b96b9f514e7f63fa6fba1a788955 ed25519:dGhpcyBpcyBpbmNyZWRpYmx5IHNpbGx5ISEhISEhISE", + &[r#"{ + "addrs": ["38.229.33.83:80"], + "ids": ["ed25519:dGhpcyBpcyBpbmNyZWRpYmx5IHNpbGx5ISEhISEhISE", + "$0bac39417268b96b9f514e7f63fa6fba1a788955"] + }"#], + &|bcb| { + bcb.addrs().push("38.229.33.83:80".parse().unwrap()); + bcb.ids().push("ed25519:dGhpcyBpcyBpbmNyZWRpYmx5IHNpbGx5ISEhISEhISE".parse().unwrap()); + bcb.ids().push("$0bac39417268b96b9f514e7f63fa6fba1a788955".parse().unwrap()); + } + ); + + #[cfg(feature = "pt-client")] + chk_bridgeline( + "obfs4 some-host:80 $0bac39417268b96b9f514e7f63fa6fba1a788955 iat-mode=1", + &[r#"{ + "transport": "obfs4", + "addrs": ["some-host:80"], + "ids": ["$0bac39417268b96b9f514e7f63fa6fba1a788955"], + "settings": [["iat-mode", "1"]] + }"#], + &|bcb| { + bcb.transport("obfs4"); + bcb.addrs().push("some-host:80".parse().unwrap()); + bcb.ids() + .push("$0bac39417268b96b9f514e7f63fa6fba1a788955".parse().unwrap()); + bcb.push_setting("iat-mode", "1"); + }, + ); + } } diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs index 7d5f7a6dc..d2224a8f4 100644 --- a/crates/tor-guardmgr/src/lib.rs +++ b/crates/tor-guardmgr/src/lib.rs @@ -93,6 +93,16 @@ pub mod bridge { #[derive(Debug, Clone, Eq, PartialEq, Hash)] #[non_exhaustive] pub enum BridgeConfig {} + + /// Configuration builder for a bridge - uninhabited placeholder type + /// + /// This type appears in configuration APIs as a stand-in, + /// when the `bridge-client` cargo feature is not enabled. + /// + /// The type is uninhabited: without this feature, you cannot create a `BridgeConfigBuilder`. + #[derive(Debug, Clone, Eq, PartialEq, Hash)] + #[non_exhaustive] + pub enum BridgeConfigBuilder {} } #[cfg(feature = "testing")]