From ce09b7c96f445def056a5718024e30d663195ac1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 5 Oct 2022 11:25:19 -0400 Subject: [PATCH] Begin revising HasAddr and its relationship to ChanTarget HasAddr used to mean "Here are addresses that I have, at which I can be contacted." But "Where (and how) can I be contacted?" is now a question for HasChannelMethod to answer. (We still need to have "HasAddr", though, so we can answer things like "what country is this relay in" and "are these relays in the same /8?") So this commit introduces: * A new trait for adding an implementation of HasChannelMethod in terms of HasAddr. * A requirement on ChanTarget that it needs to implement HasChannelMethod. There is some temporary breakage here, marked with "TODO pt-client", that I'll fix later in this branch. --- crates/tor-guardmgr/src/bridge/relay.rs | 16 ++++++++++- crates/tor-guardmgr/src/fallback.rs | 3 +++ crates/tor-guardmgr/src/guard.rs | 2 ++ crates/tor-guardmgr/src/lib.rs | 1 + crates/tor-linkspec/semver.md | 2 ++ crates/tor-linkspec/src/lib.rs | 3 ++- crates/tor-linkspec/src/owned.rs | 36 ++++++++++++++++++++++--- crates/tor-linkspec/src/traits.rs | 36 ++++++++++++++++++------- crates/tor-netdir/src/lib.rs | 5 +++- 9 files changed, 87 insertions(+), 17 deletions(-) diff --git a/crates/tor-guardmgr/src/bridge/relay.rs b/crates/tor-guardmgr/src/bridge/relay.rs index 6da421c40..042606000 100644 --- a/crates/tor-guardmgr/src/bridge/relay.rs +++ b/crates/tor-guardmgr/src/bridge/relay.rs @@ -2,7 +2,9 @@ use std::sync::Arc; -use tor_linkspec::{ChanTarget, CircTarget, HasAddrs, HasRelayIds, RelayIdRef, RelayIdType}; +use tor_linkspec::{ + ChanTarget, CircTarget, HasAddrs, HasChanMethods, HasRelayIds, RelayIdRef, RelayIdType, +}; use super::{Bridge, BridgeDesc}; @@ -63,6 +65,12 @@ impl HasAddrs for BridgeRelay { } } +impl HasChanMethods for BridgeRelay { + fn chan_methods(&self) -> Vec { + todo!() + } +} + impl ChanTarget for BridgeRelay {} impl<'a> HasRelayIds for BridgeRelayWithDesc<'a> { @@ -84,6 +92,12 @@ impl<'a> HasAddrs for BridgeRelayWithDesc<'a> { &[] } } +impl<'a> HasChanMethods for BridgeRelayWithDesc<'a> { + fn chan_methods(&self) -> Vec { + todo!() + } +} + impl<'a> ChanTarget for BridgeRelayWithDesc<'a> {} impl<'a> BridgeRelayWithDesc<'a> { diff --git a/crates/tor-guardmgr/src/fallback.rs b/crates/tor-guardmgr/src/fallback.rs index 904357a2a..877923ed5 100644 --- a/crates/tor-guardmgr/src/fallback.rs +++ b/crates/tor-guardmgr/src/fallback.rs @@ -16,6 +16,7 @@ use crate::ids::FallbackId; use derive_builder::Builder; use tor_config::ConfigBuildError; use tor_config::{define_list_builder_accessors, impl_standard_builder, list_builder::VecBuilder}; +use tor_linkspec::DirectChanMethodsHelper; use tor_llcrypto::pk::ed25519::Ed25519Identity; use tor_llcrypto::pk::rsa::RsaIdentity; @@ -128,4 +129,6 @@ impl tor_linkspec::HasRelayIdsLegacy for FallbackDir { } } +impl DirectChanMethodsHelper for FallbackDir {} + impl tor_linkspec::ChanTarget for FallbackDir {} diff --git a/crates/tor-guardmgr/src/guard.rs b/crates/tor-guardmgr/src/guard.rs index 353c12fda..19cba5cf6 100644 --- a/crates/tor-guardmgr/src/guard.rs +++ b/crates/tor-guardmgr/src/guard.rs @@ -702,6 +702,8 @@ impl tor_linkspec::HasRelayIds for Guard { } } +impl tor_linkspec::DirectChanMethodsHelper for Guard {} + impl tor_linkspec::ChanTarget for Guard {} /// A reason for permanently disabling a guard. diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs index 83be4a8aa..169f92887 100644 --- a/crates/tor-guardmgr/src/lib.rs +++ b/crates/tor-guardmgr/src/lib.rs @@ -1378,6 +1378,7 @@ impl tor_linkspec::HasRelayIds for FirstHop { self.id.identity(key_type) } } +impl tor_linkspec::DirectChanMethodsHelper for FirstHop {} impl tor_linkspec::ChanTarget for FirstHop {} /// The purpose for which we plan to use a guard. diff --git a/crates/tor-linkspec/semver.md b/crates/tor-linkspec/semver.md index 236b2c509..56933df04 100644 --- a/crates/tor-linkspec/semver.md +++ b/crates/tor-linkspec/semver.md @@ -1 +1,3 @@ MODIFIED: New ByRelayIds type. +BREAKING: Changed the semantics of HasAddrs +BREAKING: ChanTarget now requires HasChanMethod diff --git a/crates/tor-linkspec/src/lib.rs b/crates/tor-linkspec/src/lib.rs index e9cbf3dff..942c86139 100644 --- a/crates/tor-linkspec/src/lib.rs +++ b/crates/tor-linkspec/src/lib.rs @@ -105,7 +105,8 @@ pub use ids::{ pub use ls::LinkSpec; pub use owned::{OwnedChanTarget, OwnedCircTarget, RelayIds}; pub use traits::{ - ChanTarget, CircTarget, HasAddrs, HasChanMethods, HasRelayIds, HasRelayIdsLegacy, + ChanTarget, CircTarget, DirectChanMethodsHelper, HasAddrs, HasChanMethods, HasRelayIds, + HasRelayIdsLegacy, }; pub use transport::{ChannelMethod, PtAddrError, PtTargetAddr, TransportId, TransportIdError}; diff --git a/crates/tor-linkspec/src/owned.rs b/crates/tor-linkspec/src/owned.rs index 21efb91da..6d1e92b2a 100644 --- a/crates/tor-linkspec/src/owned.rs +++ b/crates/tor-linkspec/src/owned.rs @@ -5,7 +5,10 @@ use std::fmt::{self, Display}; use std::net::SocketAddr; use tor_llcrypto::pk; -use crate::{ChanTarget, CircTarget, HasAddrs, HasRelayIds, RelayIdRef, RelayIdType}; +use crate::{ + ChanTarget, ChannelMethod, CircTarget, HasAddrs, HasChanMethods, HasRelayIds, RelayIdRef, + RelayIdType, +}; /// RelayIds is an owned copy of the set of known identities of a relay. /// @@ -60,11 +63,15 @@ impl RelayIds { /// OwnedChanTarget is a summary of a [`ChanTarget`] that owns all of its /// members. -// TODO pt-client: I believe that this should also implement HasChanMethods. #[derive(Debug, Clone)] pub struct OwnedChanTarget { /// Copy of the addresses from the underlying ChanTarget. addrs: Vec, + /// Copy of the channel methods from the underlying ChanTarget. + // + // TODO: in many cases this will be redundant with addrs; if we allocate a + // lot of these objects, we might want to handle that. + methods: Vec, /// Identities that this relay provides. ids: RelayIds, } @@ -75,6 +82,12 @@ impl HasAddrs for OwnedChanTarget { } } +impl HasChanMethods for OwnedChanTarget { + fn chan_methods(&self) -> Vec { + self.methods.clone() + } +} + impl HasRelayIds for OwnedChanTarget { fn identity(&self, key_type: RelayIdType) -> Option> { self.ids.identity(key_type) @@ -85,7 +98,9 @@ impl ChanTarget for OwnedChanTarget {} impl OwnedChanTarget { /// Construct a new OwnedChanTarget from its parts. - // TODO: Put this function behind a feature. + // + // TODO pt-client: Eliminate all non-testing uses of this function, and make + // it experimental-api. pub fn new( addrs: Vec, ed_identity: pk::ed25519::Ed25519Identity, @@ -93,6 +108,7 @@ impl OwnedChanTarget { ) -> Self { Self { addrs, + methods: Vec::new(), // this is incorrect for actual use! TODO pt-client. ids: RelayIds::new(ed_identity, rsa_identity), } } @@ -104,6 +120,7 @@ impl OwnedChanTarget { { OwnedChanTarget { addrs: target.addrs().to_vec(), + methods: target.chan_methods(), ids: RelayIds::from_relay_ids(target), } } @@ -111,10 +128,14 @@ impl OwnedChanTarget { /// Construct a new OwnedChanTarget containing _only_ the provided `addr`. /// /// If `addr` is not an address of this `ChanTarget`, return the original OwnedChanTarget. + // + // TODO pt-client: this method no longer makes any sense, and needs to get replaced with one + // that works by ChanMethod. pub fn restrict_addr(&self, addr: &SocketAddr) -> Result { if self.addrs.contains(addr) { Ok(OwnedChanTarget { addrs: vec![*addr], + methods: self.methods.clone(), ids: self.ids.clone(), }) } else { @@ -154,7 +175,9 @@ pub struct OwnedCircTarget { impl OwnedCircTarget { /// Construct a new OwnedCircTarget from its parts. - // TODO: Put this function behind a feature. + // + // TODO pt-client: Eliminate all non-testing uses of this function, and make + // it experimental-api. pub fn new( chan_target: OwnedChanTarget, ntor_onion_key: pk::curve25519::PublicKey, @@ -200,6 +223,11 @@ impl HasRelayIds for OwnedCircTarget { self.chan_target.identity(key_type) } } +impl HasChanMethods for OwnedCircTarget { + fn chan_methods(&self) -> Vec { + self.chan_target.chan_methods() + } +} impl ChanTarget for OwnedCircTarget {} diff --git a/crates/tor-linkspec/src/traits.rs b/crates/tor-linkspec/src/traits.rs index f656f3c28..3da7d6c23 100644 --- a/crates/tor-linkspec/src/traits.rs +++ b/crates/tor-linkspec/src/traits.rs @@ -128,7 +128,15 @@ impl<'a, T: HasRelayIds + ?Sized> FusedIterator for RelayIdIter<'a, T> {} /// 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. + /// Return the addresses listed for this server. + /// + /// NOTE that these addresses are not necessarily ones that we should + /// connect to directly! They can be useful for telling where a server is + /// located, or whether it is "close" to another server, but without knowing + /// the associated protocols you cannot use these to launch a connection. + /// + /// To see how to _connect_ to a relay, use [`HasChanMethods::chan_methods`] + // // 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. @@ -144,22 +152,29 @@ pub trait HasChanMethods { fn chan_methods(&self) -> Vec; } -// TODO pt-client: _possibly_ we should have a blanket implementation of -// HasChanMethods for T: HasAddrs, or vice versa. +/// Implement `HasChanMethods` for an object with `HasAddr` whose addresses +/// _all_ represent a host we can connect to by a direct Tor connection at its +/// IP addresses. // -// Alternatively, we could merge HasAddrs and HasChanMethods into one trait, and -// have default implementatoins for addrs() and chan_methods(), each -// implemented in terms of the other. +// TODO pt-client: We should decide whether this is a good name. Maybe the name +// should instead suggest that we _only_ have direct methods? +pub trait DirectChanMethodsHelper: HasAddrs {} + +impl HasChanMethods for D { + fn chan_methods(&self) -> Vec { + self.addrs() + .iter() + .map(|a| ChannelMethod::Direct(*a)) + .collect() + } +} /// 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. -// -// TODO pt-client: I believe that this should also implement HasChanMethods, or -// possibly implement HasChanMethods _in place of_ HasAddrs. -nickm -pub trait ChanTarget: HasRelayIds + HasAddrs {} +pub trait ChanTarget: HasRelayIds + HasAddrs + HasChanMethods {} /// Information about a Tor relay used to extend a circuit to it. /// @@ -203,6 +218,7 @@ mod test { &self.addrs[..] } } + impl DirectChanMethodsHelper for Example {} impl HasRelayIdsLegacy for Example { fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity { &self.ed_id diff --git a/crates/tor-netdir/src/lib.rs b/crates/tor-netdir/src/lib.rs index 7991fd42d..bc3b93bb3 100644 --- a/crates/tor-netdir/src/lib.rs +++ b/crates/tor-netdir/src/lib.rs @@ -70,7 +70,9 @@ mod weight; pub mod testnet; use static_assertions::const_assert; -use tor_linkspec::{ChanTarget, HasAddrs, HasRelayIds, RelayIdRef, RelayIdType}; +use tor_linkspec::{ + ChanTarget, DirectChanMethodsHelper, HasAddrs, HasRelayIds, RelayIdRef, RelayIdType, +}; use tor_llcrypto as ll; use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity}; use tor_netdoc::doc::microdesc::{MdDigest, Microdesc}; @@ -1198,6 +1200,7 @@ impl<'a> HasRelayIds for UncheckedRelay<'a> { } } +impl<'a> DirectChanMethodsHelper for Relay<'a> {} impl<'a> ChanTarget for Relay<'a> {} impl<'a> tor_linkspec::CircTarget for Relay<'a> {