From b68a3ed5e55c4f04c534fc4c643ad1178848d4f7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 26 Jul 2022 10:03:50 -0400 Subject: [PATCH] tor-linkspec: Refactor out traits to represent a relay's ID set. We want the set of identities supported by a relay to be extensible in the future with minimal fuss; we'd also like to make working with these ID sets more convenient. To handle that, this commit adds a new trait for "Something that has the same IDs as a relay" and a new object for "an owned representation of a relay's IDs." This commit introduces a similar trait for "Something with a list of SocketAddr, like a relay has." There's no owned equivelent for that, since Vec is already a thing. Closes #428. --- crates/tor-chanmgr/src/builder.rs | 2 +- crates/tor-circmgr/src/build.rs | 1 + crates/tor-circmgr/src/path.rs | 2 +- crates/tor-circmgr/src/path/dirpath.rs | 2 +- crates/tor-circmgr/src/path/exitpath.rs | 2 +- crates/tor-circmgr/src/usage.rs | 2 +- crates/tor-guardmgr/semver.md | 1 + crates/tor-guardmgr/src/fallback.rs | 6 +- crates/tor-guardmgr/src/guard.rs | 10 +++- crates/tor-guardmgr/src/lib.rs | 11 ++-- crates/tor-linkspec/semver.md | 2 + crates/tor-linkspec/src/lib.rs | 4 +- crates/tor-linkspec/src/owned.rs | 75 ++++++++++++++++++++----- crates/tor-linkspec/src/traits.rs | 38 ++++++++----- crates/tor-netdir/semver.md | 1 + crates/tor-netdir/src/lib.rs | 8 ++- crates/tor-proto/src/channel.rs | 2 +- crates/tor-proto/src/circuit.rs | 2 +- 18 files changed, 123 insertions(+), 48 deletions(-) create mode 100644 crates/tor-guardmgr/semver.md create mode 100644 crates/tor-linkspec/semver.md create mode 100644 crates/tor-netdir/semver.md diff --git a/crates/tor-chanmgr/src/builder.rs b/crates/tor-chanmgr/src/builder.rs index 854078a97..54e610106 100644 --- a/crates/tor-chanmgr/src/builder.rs +++ b/crates/tor-chanmgr/src/builder.rs @@ -9,7 +9,7 @@ use crate::{event::ChanMgrEventSender, Error}; use std::result::Result as StdResult; use std::time::Duration; use tor_error::{bad_api_usage, internal}; -use tor_linkspec::{ChanTarget, OwnedChanTarget}; +use tor_linkspec::{HasAddrs, OwnedChanTarget}; use tor_llcrypto::pk; use tor_proto::channel::params::ChannelsParamsUpdates; use tor_rtcompat::{tls::TlsConnector, Runtime, TcpProvider, TlsProvider}; diff --git a/crates/tor-circmgr/src/build.rs b/crates/tor-circmgr/src/build.rs index 35d8882a3..121e9cb10 100644 --- a/crates/tor-circmgr/src/build.rs +++ b/crates/tor-circmgr/src/build.rs @@ -482,6 +482,7 @@ mod test { use crate::timeouts::TimeoutEstimator; use futures::channel::oneshot; use std::sync::Mutex; + use tor_linkspec::HasRelayIds; use tor_llcrypto::pk::ed25519::Ed25519Identity; use tor_rtcompat::{test_with_all_runtimes, SleepProvider}; use tracing::trace; diff --git a/crates/tor-circmgr/src/path.rs b/crates/tor-circmgr/src/path.rs index 05996d3d3..4db5958ff 100644 --- a/crates/tor-circmgr/src/path.rs +++ b/crates/tor-circmgr/src/path.rs @@ -140,7 +140,7 @@ impl OwnedPath { #[cfg(test)] fn assert_same_path_when_owned(path: &TorPath<'_>) { #![allow(clippy::unwrap_used)] - use tor_linkspec::ChanTarget; + use tor_linkspec::HasRelayIds; let owned: OwnedPath = path.try_into().unwrap(); match (&owned, &path.inner) { diff --git a/crates/tor-circmgr/src/path/dirpath.rs b/crates/tor-circmgr/src/path/dirpath.rs index 7386d4609..144cb7d52 100644 --- a/crates/tor-circmgr/src/path/dirpath.rs +++ b/crates/tor-circmgr/src/path/dirpath.rs @@ -100,7 +100,7 @@ mod test { use std::collections::HashSet; use tor_basic_utils::test_rng::testing_rng; use tor_guardmgr::fallback::{FallbackDir, FallbackList}; - use tor_linkspec::ChanTarget; + use tor_linkspec::HasRelayIds; use tor_netdir::testnet; #[test] diff --git a/crates/tor-circmgr/src/path/exitpath.rs b/crates/tor-circmgr/src/path/exitpath.rs index 5631d5799..340cc8bb6 100644 --- a/crates/tor-circmgr/src/path/exitpath.rs +++ b/crates/tor-circmgr/src/path/exitpath.rs @@ -255,7 +255,7 @@ mod test { use crate::test::OptDummyGuardMgr; use std::collections::HashSet; use tor_basic_utils::test_rng::testing_rng; - use tor_linkspec::ChanTarget; + use tor_linkspec::HasRelayIds; use tor_netdir::testnet; use tor_rtcompat::SleepProvider; diff --git a/crates/tor-circmgr/src/usage.rs b/crates/tor-circmgr/src/usage.rs index 1a015b056..4665a3180 100644 --- a/crates/tor-circmgr/src/usage.rs +++ b/crates/tor-circmgr/src/usage.rs @@ -366,7 +366,7 @@ pub(crate) mod test { use crate::path::OwnedPath; use crate::test::OptDummyGuardMgr; use tor_basic_utils::test_rng::testing_rng; - use tor_linkspec::ChanTarget; + use tor_linkspec::HasRelayIds; use tor_netdir::testnet; impl IsolationTokenEq for TargetCircUsage { diff --git a/crates/tor-guardmgr/semver.md b/crates/tor-guardmgr/semver.md new file mode 100644 index 000000000..6bc27db9e --- /dev/null +++ b/crates/tor-guardmgr/semver.md @@ -0,0 +1 @@ +BREAKING: Exposed objects implement the new versions of the linkspec traits diff --git a/crates/tor-guardmgr/src/fallback.rs b/crates/tor-guardmgr/src/fallback.rs index 84da72486..816ef1708 100644 --- a/crates/tor-guardmgr/src/fallback.rs +++ b/crates/tor-guardmgr/src/fallback.rs @@ -114,10 +114,12 @@ pub(crate) fn default_fallbacks() -> Vec { include!("fallback_dirs.inc") } -impl tor_linkspec::ChanTarget for FallbackDir { +impl tor_linkspec::HasAddrs for FallbackDir { fn addrs(&self) -> &[SocketAddr] { &self.orports[..] } +} +impl tor_linkspec::HasRelayIds for FallbackDir { fn ed_identity(&self) -> &Ed25519Identity { &self.ed_identity } @@ -125,3 +127,5 @@ impl tor_linkspec::ChanTarget for FallbackDir { &self.rsa_identity } } + +impl tor_linkspec::ChanTarget for FallbackDir {} diff --git a/crates/tor-guardmgr/src/guard.rs b/crates/tor-guardmgr/src/guard.rs index 7609f225b..488c6fe79 100644 --- a/crates/tor-guardmgr/src/guard.rs +++ b/crates/tor-guardmgr/src/guard.rs @@ -1,7 +1,6 @@ //! Code to represent its single guard node and track its status. use tor_basic_utils::retry::RetryDelay; -use tor_linkspec::ChanTarget; use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity}; use tor_netdir::{NetDir, Relay, RelayWeight}; @@ -17,6 +16,7 @@ use crate::skew::SkewObservation; use crate::util::randomize_time; use crate::{ids::GuardId, GuardParams, GuardRestriction, GuardUsage}; use crate::{ExternalActivity, FirstHopId, GuardUsageKind}; +use tor_linkspec::HasAddrs; use tor_persist::{Futureproof, JsonValue}; /// Tri-state to represent whether a guard is believed to be reachable or not. @@ -686,10 +686,13 @@ impl Guard { } } -impl tor_linkspec::ChanTarget for Guard { +impl tor_linkspec::HasAddrs for Guard { fn addrs(&self) -> &[SocketAddr] { &self.orports[..] } +} + +impl tor_linkspec::HasRelayIds for Guard { fn ed_identity(&self) -> &Ed25519Identity { &self.id.0.ed25519 } @@ -698,6 +701,8 @@ impl tor_linkspec::ChanTarget for Guard { } } +impl tor_linkspec::ChanTarget for Guard {} + /// A reason for permanently disabling a guard. #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(tag = "type")] @@ -783,6 +788,7 @@ impl CircHistory { mod test { #![allow(clippy::unwrap_used)] use super::*; + use tor_linkspec::HasRelayIds; #[test] fn crate_id() { diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs index 7e6ad9966..b45b629e1 100644 --- a/crates/tor-guardmgr/src/lib.rs +++ b/crates/tor-guardmgr/src/lib.rs @@ -1318,11 +1318,13 @@ impl FirstHop { } } -// This is somewhat redundant with the implementation in crate::guard::Guard. -impl tor_linkspec::ChanTarget for FirstHop { +// This is somewhat redundant with the implementations in crate::guard::Guard. +impl tor_linkspec::HasAddrs for FirstHop { fn addrs(&self) -> &[SocketAddr] { &self.orports[..] } +} +impl tor_linkspec::HasRelayIds for FirstHop { fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity { &self.id.as_ref().ed25519 } @@ -1330,6 +1332,7 @@ impl tor_linkspec::ChanTarget for FirstHop { &self.id.as_ref().rsa } } +impl tor_linkspec::ChanTarget for FirstHop {} /// The purpose for which we plan to use a guard. /// @@ -1414,6 +1417,7 @@ pub enum GuardRestriction { mod test { #![allow(clippy::unwrap_used)] use super::*; + use tor_linkspec::{HasAddrs, HasRelayIds}; use tor_persist::TestingStateMgr; use tor_rtcompat::test_with_all_runtimes; @@ -1536,8 +1540,6 @@ mod test { #[test] fn filtering_basics() { test_with_all_runtimes!(|rt| async move { - use tor_linkspec::ChanTarget; - let (guardmgr, _statemgr, netdir) = init(rt); let u = GuardUsage::default(); let filter = { @@ -1558,7 +1560,6 @@ mod test { #[test] fn external_status() { - use tor_linkspec::ChanTarget; test_with_all_runtimes!(|rt| async move { let (guardmgr, _statemgr, netdir) = init(rt); let data_usage = GuardUsage::default(); diff --git a/crates/tor-linkspec/semver.md b/crates/tor-linkspec/semver.md new file mode 100644 index 000000000..dafa40b9b --- /dev/null +++ b/crates/tor-linkspec/semver.md @@ -0,0 +1,2 @@ +BREAKING: Split ChanTarget into HasAddrs and HasRelayIds + diff --git a/crates/tor-linkspec/src/lib.rs b/crates/tor-linkspec/src/lib.rs index 249f5468d..676c62527 100644 --- a/crates/tor-linkspec/src/lib.rs +++ b/crates/tor-linkspec/src/lib.rs @@ -77,5 +77,5 @@ mod owned; mod traits; pub use ls::LinkSpec; -pub use owned::{OwnedChanTarget, OwnedCircTarget}; -pub use traits::{ChanTarget, CircTarget}; +pub use owned::{OwnedChanTarget, OwnedCircTarget, RelayIds}; +pub use traits::{ChanTarget, CircTarget, HasAddrs, HasRelayIds}; diff --git a/crates/tor-linkspec/src/owned.rs b/crates/tor-linkspec/src/owned.rs index 27b9b03e5..17022ef31 100644 --- a/crates/tor-linkspec/src/owned.rs +++ b/crates/tor-linkspec/src/owned.rs @@ -4,7 +4,45 @@ use std::fmt::{self, Display}; use std::net::SocketAddr; use tor_llcrypto::pk; -use crate::{ChanTarget, CircTarget}; +use crate::{ChanTarget, CircTarget, HasAddrs, HasRelayIds}; + +/// RelayIds is an owned copy of the set of identities owned by a relay. +#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)] +pub struct RelayIds { + /// Copy of the ed25519 id from the underlying ChanTarget. + ed_identity: pk::ed25519::Ed25519Identity, + /// Copy of the rsa id from the underlying ChanTarget. + rsa_identity: pk::rsa::RsaIdentity, +} + +impl HasRelayIds for RelayIds { + fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity { + &self.ed_identity + } + + fn rsa_identity(&self) -> &pk::rsa::RsaIdentity { + &self.rsa_identity + } +} + +impl RelayIds { + /// Construct a new RelayIds object with a given pair of identity keys. + pub fn new( + ed_identity: pk::ed25519::Ed25519Identity, + rsa_identity: pk::rsa::RsaIdentity, + ) -> Self { + Self { + ed_identity, + rsa_identity, + } + } + + /// Construct a new RelayIds object from another object that impements + /// [`HasRelayIds`] + pub fn from_relay_ids(other: &T) -> Self { + Self::new(*other.ed_identity(), *other.rsa_identity()) + } +} /// OwnedChanTarget is a summary of a [`ChanTarget`] that owns all of its /// members. @@ -12,24 +50,28 @@ use crate::{ChanTarget, CircTarget}; pub struct OwnedChanTarget { /// Copy of the addresses from the underlying ChanTarget. addrs: Vec, - /// Copy of the ed25519 id from the underlying ChanTarget. - ed_identity: pk::ed25519::Ed25519Identity, - /// Copy of the rsa id from the underlying ChanTarget. - rsa_identity: pk::rsa::RsaIdentity, + /// Identities that this relay provides. + ids: RelayIds, } -impl ChanTarget for OwnedChanTarget { +impl HasAddrs for OwnedChanTarget { fn addrs(&self) -> &[SocketAddr] { &self.addrs[..] } +} + +impl HasRelayIds for OwnedChanTarget { fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity { - &self.ed_identity + self.ids.ed_identity() } + fn rsa_identity(&self) -> &pk::rsa::RsaIdentity { - &self.rsa_identity + self.ids.rsa_identity() } } +impl ChanTarget for OwnedChanTarget {} + impl OwnedChanTarget { /// Construct a new OwnedChanTarget from its parts. // TODO: Put this function behind a feature. @@ -40,8 +82,7 @@ impl OwnedChanTarget { ) -> Self { Self { addrs, - ed_identity, - rsa_identity, + ids: RelayIds::new(ed_identity, rsa_identity), } } @@ -52,8 +93,7 @@ impl OwnedChanTarget { { OwnedChanTarget { addrs: target.addrs().to_vec(), - ed_identity: *target.ed_identity(), - rsa_identity: *target.rsa_identity(), + ids: RelayIds::from_relay_ids(target), } } @@ -64,7 +104,7 @@ impl OwnedChanTarget { if self.addrs.contains(addr) { Ok(OwnedChanTarget { addrs: vec![*addr], - ..*self + ids: self.ids.clone(), }) } else { Err(self.clone()) @@ -81,7 +121,7 @@ impl Display for OwnedChanTarget { [a] => write!(f, "{}", a)?, [a, ..] => write!(f, "{}+", a)?, }; - write!(f, "{}", &self.ed_identity)?; // short enough to print + write!(f, "{}", self.ed_identity())?; // short enough to print write!(f, "]")?; Ok(()) } @@ -136,10 +176,13 @@ impl Display for OwnedCircTarget { } } -impl ChanTarget for OwnedCircTarget { +impl HasAddrs for OwnedCircTarget { fn addrs(&self) -> &[SocketAddr] { self.chan_target.addrs() } +} + +impl HasRelayIds for OwnedCircTarget { fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity { self.chan_target.ed_identity() } @@ -148,6 +191,8 @@ impl ChanTarget for OwnedCircTarget { } } +impl ChanTarget for OwnedCircTarget {} + impl CircTarget for OwnedCircTarget { fn ntor_onion_key(&self) -> &pk::curve25519::PublicKey { &self.ntor_onion_key diff --git a/crates/tor-linkspec/src/traits.rs b/crates/tor-linkspec/src/traits.rs index eb31a973c..814566024 100644 --- a/crates/tor-linkspec/src/traits.rs +++ b/crates/tor-linkspec/src/traits.rs @@ -4,29 +4,36 @@ use std::net::SocketAddr; use tor_llcrypto::pk; -/// Information about a Tor relay used to connect to it. -/// -/// Anything that implements 'ChanTarget' can be used as the -/// identity of a relay for the purposes of launching a new -/// channel. -pub trait ChanTarget { - /// Return the addresses at which you can connect to this relay - // TODO: This is a questionable API. I'd rather return an iterator - // of addresses or references to addresses, but both of those options - // make defining the right associated types rather tricky. - fn addrs(&self) -> &[SocketAddr]; +/// An object containing information about a relay's identity keys. +pub trait HasRelayIds { /// Return the ed25519 identity for this relay. fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity; + /// Return the RSA identity for this relay. + fn rsa_identity(&self) -> &pk::rsa::RsaIdentity; /// Return the ed25519 identity key for this relay, if it is valid. /// /// This can be costly. fn ed_identity_key(&self) -> Option { self.ed_identity().try_into().ok() } - /// Return the RSA identity for this relay. - fn rsa_identity(&self) -> &pk::rsa::RsaIdentity; } +/// An object that represents a host on the network with known IP addresses. +pub trait HasAddrs { + /// Return the addresses at which you can connect to this server. + // TODO: This is a questionable API. I'd rather return an iterator + // of addresses or references to addresses, but both of those options + // make defining the right associated types rather tricky. + fn addrs(&self) -> &[SocketAddr]; +} + +/// Information about a Tor relay used to connect to it. +/// +/// Anything that implements 'ChanTarget' can be used as the +/// identity of a relay for the purposes of launching a new +/// channel. +pub trait ChanTarget: HasRelayIds + HasAddrs {} + /// Information about a Tor relay used to extend a circuit to it. /// /// Anything that implements 'CircTarget' can be used as the @@ -64,10 +71,12 @@ mod test { ntor: pk::curve25519::PublicKey, pv: tor_protover::Protocols, } - impl ChanTarget for Example { + impl HasAddrs for Example { fn addrs(&self) -> &[SocketAddr] { &self.addrs[..] } + } + impl HasRelayIds for Example { fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity { &self.ed_id } @@ -75,6 +84,7 @@ mod test { &self.rsa_id } } + impl ChanTarget for Example {} impl CircTarget for Example { fn ntor_onion_key(&self) -> &pk::curve25519::PublicKey { &self.ntor diff --git a/crates/tor-netdir/semver.md b/crates/tor-netdir/semver.md new file mode 100644 index 000000000..e0dbd4dab --- /dev/null +++ b/crates/tor-netdir/semver.md @@ -0,0 +1 @@ +BREAKING: Relay implements the new versions of the tor-linkspec traits. diff --git a/crates/tor-netdir/src/lib.rs b/crates/tor-netdir/src/lib.rs index 6fdf88dc6..8495c9c9f 100644 --- a/crates/tor-netdir/src/lib.rs +++ b/crates/tor-netdir/src/lib.rs @@ -68,7 +68,7 @@ mod weight; #[cfg(any(test, feature = "testing"))] pub mod testnet; -use tor_linkspec::ChanTarget; +use tor_linkspec::{ChanTarget, HasAddrs, HasRelayIds}; use tor_llcrypto as ll; use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity}; use tor_netdoc::doc::microdesc::{MdDigest, Microdesc}; @@ -1112,10 +1112,12 @@ impl<'a> Relay<'a> { } } -impl<'a> ChanTarget for Relay<'a> { +impl<'a> HasAddrs for Relay<'a> { fn addrs(&self) -> &[std::net::SocketAddr] { self.rs.addrs() } +} +impl<'a> HasRelayIds for Relay<'a> { fn ed_identity(&self) -> &Ed25519Identity { self.id() } @@ -1124,6 +1126,8 @@ impl<'a> ChanTarget for Relay<'a> { } } +impl<'a> ChanTarget for Relay<'a> {} + impl<'a> tor_linkspec::CircTarget for Relay<'a> { fn ntor_onion_key(&self) -> &ll::pk::curve25519::PublicKey { self.md.ntor_key() diff --git a/crates/tor-proto/src/channel.rs b/crates/tor-proto/src/channel.rs index 47cc9685c..92f9ef739 100644 --- a/crates/tor-proto/src/channel.rs +++ b/crates/tor-proto/src/channel.rs @@ -77,7 +77,7 @@ use std::result::Result as StdResult; use std::time::Duration; use tor_cell::chancell::{msg, ChanCell, CircId}; use tor_error::internal; -use tor_linkspec::{ChanTarget, OwnedChanTarget}; +use tor_linkspec::{ChanTarget, HasRelayIds, OwnedChanTarget}; use tor_llcrypto::pk::ed25519::Ed25519Identity; use tor_llcrypto::pk::rsa::RsaIdentity; use tor_rtcompat::SleepProvider; diff --git a/crates/tor-proto/src/circuit.rs b/crates/tor-proto/src/circuit.rs index aae7b2657..5b3133e35 100644 --- a/crates/tor-proto/src/circuit.rs +++ b/crates/tor-proto/src/circuit.rs @@ -1040,7 +1040,7 @@ mod test { // Do the path accessors report a reasonable outcome? let path = circ.path(); assert_eq!(path.len(), 4); - use tor_linkspec::ChanTarget; + use tor_linkspec::HasRelayIds; assert_eq!(path[3].ed_identity(), example_target().ed_identity()); assert_ne!(path[0].ed_identity(), example_target().ed_identity()); });