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<SocketAddr> is already a thing.

Closes #428.

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};

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;

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) {

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]

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;
 

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 {

crates/tor-guardmgr/semver.md

@@ -0,0 +1 @@
+BREAKING: Exposed objects implement the new versions of the linkspec traits

crates/tor-guardmgr/src/fallback.rs

@@ -114,10 +114,12 @@ pub(crate) fn default_fallbacks() -> Vec<FallbackDirBuilder> {
     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 {}

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() {

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();

crates/tor-linkspec/semver.md

@@ -0,0 +1,2 @@
+BREAKING: Split ChanTarget into HasAddrs and HasRelayIds
+

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};

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<T: HasRelayIds + ?Sized>(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<SocketAddr>,
-    /// 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

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<pk::ed25519::PublicKey> {
         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

crates/tor-netdir/semver.md

@@ -0,0 +1 @@
+BREAKING: Relay implements the new versions of the tor-linkspec traits.

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()

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;

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());
         });