Add a set of Identity-related types and accessors.

I wonder if these types are correct.  I think it makes sense to have
a Ref type like this, rather than just using `&RelayId`, but it
doesn't seems that I can make `RelayId` and `RelayIdRef` implement
Borrow and ToOwned for one another, so maybe I've messed up.

Cargo.lock

@@ -3717,8 +3717,10 @@ dependencies = [
 name = "tor-linkspec"
 version = "0.4.0"
 dependencies = [
+ "derive_more",
  "hex-literal",
  "serde",
+ "strum",
  "tor-bytes",
  "tor-llcrypto",
  "tor-protover",

crates/tor-linkspec/Cargo.toml

@@ -12,7 +12,9 @@ categories = ["network-programming"]
 repository = "https://gitlab.torproject.org/tpo/core/arti.git/"
 
 [dependencies]
+derive_more = "0.99"
 serde = { version = "1.0.103", features = ["derive"] }
+strum = { version = "0.24", features = ["derive"] }
 tor-bytes = { path = "../tor-bytes", version = "0.5.0" }
 tor-llcrypto = { path = "../tor-llcrypto", version = "0.3.3" }
 tor-protover = { path = "../tor-protover", version = "0.3.0" }

crates/tor-linkspec/src/ids.rs

@@ -0,0 +1,105 @@
+//! Code to abstract over the notion of relays having one or more identities.
+//!
+//! Currently (2022), every Tor relay has exactly two identities: A legacy
+//! identity that is based on the SHA-1 hash of an RSA-1024 public key, and a
+//! modern identity that is an Ed25519 public key.  This code lets us abstract
+//! over those types, and over other new types that may exist in the future.
+
+use derive_more::{Display, From};
+use tor_llcrypto::pk::{ed25519::Ed25519Identity, rsa::RsaIdentity};
+
+/// The type of a relay identity.
+///
+#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Ord, PartialOrd, Display, strum::EnumIter)]
+#[non_exhaustive]
+pub enum RelayIdType {
+    /// An Ed25519 identity.
+    ///
+    /// Every relay (currently) has one of these identities. It is the same
+    /// as the encoding of the relay's public Ed25519 identity key.
+    #[display(fmt = "Ed25519")]
+    Ed25519,
+    /// An RSA identity.
+    ///
+    /// Every relay (currently) has one of these identities.  It is computed as
+    /// a SHA-1 digest of the DER encoding of the relay's public RSA 1024-bit
+    /// identity key.  Because of short key length, this type of identity should
+    /// not be considered secure on its own.
+    #[display(fmt = "RSA (legacy)")]
+    Rsa,
+}
+
+/// A single relay identity.
+#[derive(Debug, Clone, Copy, Eq, PartialEq, Display, From, Hash)]
+#[non_exhaustive]
+pub enum RelayId {
+    /// An Ed25519 identity.
+    #[display(fmt = "{}", _0)]
+    Ed25519(Ed25519Identity),
+    /// An RSA identity.
+    #[display(fmt = "{}", _0)]
+    Rsa(RsaIdentity),
+}
+
+/// A reference to a single relay identity.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Display, From, derive_more::TryInto)]
+#[non_exhaustive]
+pub enum RelayIdRef<'a> {
+    /// An Ed25519 identity.
+    #[display(fmt = "{}", _0)]
+    Ed25519(&'a Ed25519Identity),
+    /// An RSA identity.
+    #[display(fmt = "{}", _0)]
+    Rsa(&'a RsaIdentity),
+}
+
+impl RelayIdType {
+    /// Return an iterator over all
+    pub fn all_types() -> RelayIdTypeIter {
+        use strum::IntoEnumIterator;
+        Self::iter()
+    }
+}
+
+impl RelayId {
+    /// Return a [`RelayIdRef`] pointing to the contents of this identity.
+    pub fn as_ref(&self) -> RelayIdRef<'_> {
+        match self {
+            RelayId::Ed25519(key) => key.into(),
+
+            RelayId::Rsa(key) => key.into(),
+        }
+    }
+}
+
+impl<'a> RelayIdRef<'a> {
+    /// Copy this reference into a new [`RelayId`] object.
+    //
+    // TODO(nickm): I wish I could make this a proper `ToOwned` implementation,
+    // but I see no way to do as long as RelayIdRef<'a> implements Clone too.
+    pub fn to_owned(&self) -> RelayId {
+        match *self {
+            RelayIdRef::Ed25519(key) => (*key).into(),
+            RelayIdRef::Rsa(key) => (*key).into(),
+        }
+    }
+}
+
+/// Expand to an implementation for PartialEq for a given key type.
+macro_rules! impl_eq_variant {
+    { $var:ident($type:ty) } => {
+        impl<'a> PartialEq<$type> for RelayIdRef<'a> {
+            fn eq(&self, other: &$type) -> bool {
+                matches!(self, RelayIdRef::$var(this) if this == &other)
+            }
+        }
+        impl PartialEq<$type> for RelayId {
+            fn eq(&self, other: &$type) -> bool {
+                matches!(&self, RelayId::$var(this) if this == other)
+            }
+        }
+    }
+}
+
+impl_eq_variant! { Rsa(RsaIdentity) }
+impl_eq_variant! { Ed25519(Ed25519Identity) }

crates/tor-linkspec/src/lib.rs

@@ -72,10 +72,12 @@
 #![allow(clippy::significant_drop_in_scrutinee)] // arti/-/merge_requests/588/#note_2812945
 //! <!-- @@ end lint list maintained by maint/add_warning @@ -->
 
+mod ids;
 mod ls;
 mod owned;
 mod traits;
 
+pub use ids::{RelayId, RelayIdRef, RelayIdType, RelayIdTypeIter};
 pub use ls::LinkSpec;
 pub use owned::{OwnedChanTarget, OwnedCircTarget, RelayIds};
 pub use traits::{ChanTarget, CircTarget, HasAddrs, HasRelayIds};

crates/tor-linkspec/src/ls.rs

@@ -9,6 +9,8 @@ use tor_bytes::{EncodeResult, Error, Readable, Reader, Result, Writeable, Writer
 use tor_llcrypto::pk::ed25519;
 use tor_llcrypto::pk::rsa::RsaIdentity;
 
+use crate::RelayId;
+
 /// A piece of information about a relay and how to connect to it.
 #[non_exhaustive]
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -131,6 +133,14 @@ impl From<ed25519::PublicKey> for LinkSpec {
         LinkSpec::Ed25519Id(pk.into())
     }
 }
+impl From<RelayId> for LinkSpec {
+    fn from(id: RelayId) -> Self {
+        match id {
+            RelayId::Ed25519(key) => LinkSpec::Ed25519Id(key),
+            RelayId::Rsa(key) => LinkSpec::RsaId(key),
+        }
+    }
+}
 
 impl LinkSpec {
     /// Helper: return the position in the list of identifiers

crates/tor-linkspec/src/traits.rs

@@ -1,11 +1,32 @@
 //! Declare traits to be implemented by types that describe a place
 //! that Tor can connect to, directly or indirectly.
 
-use std::net::SocketAddr;
+use std::{iter::FusedIterator, net::SocketAddr};
 use tor_llcrypto::pk;
 
+use crate::{RelayIdRef, RelayIdType, RelayIdTypeIter};
+
 /// An object containing information about a relay's identity keys.
 pub trait HasRelayIds {
+    /// Return the identity of this relay whose type is `key_type`, or None if
+    /// the relay has no such identity.
+    ///
+    /// (Currently all relays have all recognized identity types, but we might
+    /// implement or deprecate an identity type in the future.)
+    fn identity(&self, key_type: RelayIdType) -> Option<RelayIdRef<'_>> {
+        match key_type {
+            RelayIdType::Rsa => Some(self.rsa_identity().into()),
+            RelayIdType::Ed25519 => Some(self.ed_identity().into()),
+        }
+    }
+    /// Return an iterator over all of the identities held by this object.
+    fn identities(&self) -> RelayIdIter<'_, Self> {
+        RelayIdIter {
+            info: self,
+            next_key: RelayIdType::all_types(),
+        }
+    }
+
     /// Return the ed25519 identity for this relay.
     fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity;
     /// Return the RSA identity for this relay.
@@ -28,6 +49,30 @@ pub trait HasRelayIds {
     }
 }
 
+/// An iterator over all of the relay identities held by a [`HasRelayIds`]
+#[derive(Clone)]
+pub struct RelayIdIter<'a, T: HasRelayIds + ?Sized> {
+    /// The object holding the keys
+    info: &'a T,
+    /// The next key type to yield
+    next_key: RelayIdTypeIter,
+}
+
+impl<'a, T: HasRelayIds + ?Sized> Iterator for RelayIdIter<'a, T> {
+    type Item = RelayIdRef<'a>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        for key_type in &mut self.next_key {
+            if let Some(key) = self.info.identity(key_type) {
+                return Some(key);
+            }
+        }
+        None
+    }
+}
+// RelayIdIter is fused since next_key is fused.
+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.
@@ -54,7 +99,7 @@ pub trait CircTarget: ChanTarget {
     // of link specifiers, but that's not so easy to do, since it seems
     // doing so correctly would require default associated types.
     fn linkspecs(&self) -> Vec<crate::LinkSpec> {
-        let mut result = vec![(*self.ed_identity()).into(), (*self.rsa_identity()).into()];
+        let mut result: Vec<_> = self.identities().map(|id| id.to_owned().into()).collect();
         for addr in self.addrs().iter() {
             result.push(addr.into());
         }