Teach guard restrictions about RelayId.

This implementation is (sadly) too copy-heavy or now, because
HashSet<RelayId> can't be indexed with RelayIdRef.

crates/tor-circmgr/src/path/exitpath.rs

@@ -7,6 +7,7 @@ use std::time::SystemTime;
 use tor_basic_utils::iter::FilterCount;
 use tor_error::{bad_api_usage, internal};
 use tor_guardmgr::{GuardMgr, GuardMonitor, GuardUsable};
+use tor_linkspec::RelayId;
 use tor_netdir::{NetDir, Relay, SubnetConfig, WeightRole};
 use tor_rtcompat::Runtime;
 
@@ -166,9 +167,13 @@ impl<'a> ExitPathBuilder<'a> {
                 guardmgr.update_network(netdir); // possibly unnecessary.
                 if let Some(exit_relay) = chosen_exit {
                     let mut family = std::collections::HashSet::new();
-                    family.insert(*exit_relay.id());
+                    family.insert(RelayId::from(*exit_relay.id()));
                     // TODO(nickm): See "limitations" note on `known_family_members`.
-                    family.extend(netdir.known_family_members(exit_relay).map(|r| *r.id()));
+                    family.extend(
+                        netdir
+                            .known_family_members(exit_relay)
+                            .map(|r| RelayId::from(*r.id())),
+                    );
                     b.restrictions()
                         .push(tor_guardmgr::GuardRestriction::AvoidAllIds(family));
                 }

crates/tor-guardmgr/src/guard.rs

@@ -393,8 +393,15 @@ impl Guard {
     /// Return true if this guard obeys a single restriction.
     fn obeys_restriction(&self, r: &GuardRestriction) -> bool {
         match r {
-            GuardRestriction::AvoidId(ed) => self.id.0.ed_identity() != ed,
-            GuardRestriction::AvoidAllIds(ids) => !ids.contains(self.id.0.ed_identity()),
+            GuardRestriction::AvoidId(avoid_id) => !self.id.0.has_identity(avoid_id.as_ref()),
+            GuardRestriction::AvoidAllIds(avoid_ids) => {
+                // TODO(nickm): This copies all of our IDs!
+                // We should use a contains method on a RelayIdSet or something.
+                self.id
+                    .0
+                    .identities()
+                    .all(|id| !avoid_ids.contains(&id.to_owned()))
+            }
         }
     }
 
@@ -788,7 +795,7 @@ impl CircHistory {
 mod test {
     #![allow(clippy::unwrap_used)]
     use super::*;
-    use tor_linkspec::HasRelayIds;
+    use tor_linkspec::{HasRelayIds, RelayId};
 
     #[test]
     fn crate_id() {
@@ -809,6 +816,9 @@ mod test {
 
     #[test]
     fn simple_accessors() {
+        fn ed(id: [u8; 32]) -> RelayId {
+            RelayId::Ed25519(id.into())
+        }
         let id = basic_id();
         let g = basic_guard();
 
@@ -820,34 +830,33 @@ mod test {
 
         use crate::GuardUsageBuilder;
         let mut usage1 = GuardUsageBuilder::new();
+
         usage1
             .restrictions()
-            .push(GuardRestriction::AvoidId([22; 32].into()));
+            .push(GuardRestriction::AvoidId(ed([22; 32])));
         let usage1 = usage1.build().unwrap();
         let mut usage2 = GuardUsageBuilder::new();
         usage2
             .restrictions()
-            .push(GuardRestriction::AvoidId([13; 32].into()));
+            .push(GuardRestriction::AvoidId(ed([13; 32])));
         let usage2 = usage2.build().unwrap();
         let usage3 = GuardUsage::default();
         let mut usage4 = GuardUsageBuilder::new();
         usage4
             .restrictions()
-            .push(GuardRestriction::AvoidId([22; 32].into()));
+            .push(GuardRestriction::AvoidId(ed([22; 32])));
         usage4
             .restrictions()
-            .push(GuardRestriction::AvoidId([13; 32].into()));
+            .push(GuardRestriction::AvoidId(ed([13; 32])));
         let usage4 = usage4.build().unwrap();
         let mut usage5 = GuardUsageBuilder::new();
         usage5.restrictions().push(GuardRestriction::AvoidAllIds(
-            vec![[22; 32].into(), [13; 32].into()].into_iter().collect(),
+            vec![ed([22; 32]), ed([13; 32])].into_iter().collect(),
         ));
         let usage5 = usage5.build().unwrap();
         let mut usage6 = GuardUsageBuilder::new();
         usage6.restrictions().push(GuardRestriction::AvoidAllIds(
-            vec![[99; 32].into(), [100; 32].into()]
-                .into_iter()
-                .collect(),
+            vec![ed([99; 32]), ed([100; 32])].into_iter().collect(),
         ));
         let usage6 = usage6.build().unwrap();
 

crates/tor-guardmgr/src/lib.rs

@@ -141,6 +141,7 @@ use std::collections::{HashMap, HashSet};
 use std::net::SocketAddr;
 use std::sync::{Arc, Mutex, Weak};
 use std::time::{Duration, Instant, SystemTime};
+use tor_linkspec::RelayId;
 use tor_netdir::NetDirProvider;
 use tor_proto::ClockSkew;
 use tracing::{debug, info, trace, warn};
@@ -1410,10 +1411,12 @@ impl GuardUsageBuilder {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 #[non_exhaustive]
 pub enum GuardRestriction {
-    /// Don't pick a guard with the provided Ed25519 identity.
-    AvoidId(pk::ed25519::Ed25519Identity),
+    /// Don't pick a guard with the provided identity.
+    AvoidId(RelayId),
     /// Don't pick a guard with any of the provided Ed25519 identities.
-    AvoidAllIds(HashSet<pk::ed25519::Ed25519Identity>),
+    //
+    // TODO(nickm): Switch this to a type that can actually work well with RelayId.
+    AvoidAllIds(HashSet<RelayId>),
 }
 
 #[cfg(test)]

crates/tor-linkspec/src/ids.rs

@@ -94,6 +94,14 @@ impl RelayId {
                 .into(),
         })
     }
+
+    /// Return the type of this relay identity.
+    pub fn id_type(&self) -> RelayIdType {
+        match self {
+            RelayId::Ed25519(_) => RelayIdType::Ed25519,
+            RelayId::Rsa(_) => RelayIdType::Rsa,
+        }
+    }
 }
 
 impl<'a> RelayIdRef<'a> {
@@ -107,6 +115,14 @@ impl<'a> RelayIdRef<'a> {
             RelayIdRef::Rsa(key) => (*key).into(),
         }
     }
+
+    /// Return the type of this relay identity.
+    pub fn id_type(&self) -> RelayIdType {
+        match self {
+            RelayIdRef::Ed25519(_) => RelayIdType::Ed25519,
+            RelayIdRef::Rsa(_) => RelayIdType::Rsa,
+        }
+    }
 }
 
 /// Expand to an implementation for PartialEq for a given key type.

crates/tor-linkspec/src/traits.rs

@@ -26,6 +26,17 @@ pub trait HasRelayIds {
             next_key: RelayIdType::all_types(),
         }
     }
+    /// Check whether the provided Id is a known identity of this relay.
+    ///
+    /// Remember that a given set of identity keys may be incomplete: some
+    /// objects that represent a relay have only a subset of the relay's
+    /// identities. Therefore, a "true" answer means that the relay has this
+    /// identity,  but a "false" answer could mean that the relay has a
+    /// different identity of this type, or that it has _no_ known identity of
+    /// this type.
+    fn has_identity(&self, id: RelayIdRef<'_>) -> bool {
+        self.identity(id.id_type()).map(|my_id| my_id == id) == Some(true)
+    }
 
     /// Return the ed25519 identity for this relay.
     fn ed_identity(&self) -> &pk::ed25519::Ed25519Identity;