guard sample: Change semantics of `contains` to handle ambiguity.

This is necessary for the (somewhat undesirable) lookup_ids function
to return an ID that the dirmgr can actually use to report successes
and failures.

As noted, lookup_ids will create problems down the road when we
implement relays. We should refactor it out before then.

crates/tor-guardmgr/src/lib.rs

@@ -1055,7 +1055,11 @@ impl GuardMgrInner {
     }
 
     /// Return every currently extant FirstHopId for a guard or fallback
-    /// directory matching the provided keys.
+    /// directory matching (or possibly matching) the provided keys.
+    ///
+    /// An identity is _possibly matching_ if it contains some of the IDs in the
+    /// provided identity, and it has no _contradictory_ identities, but it does
+    /// not necessarily contain _all_ of those identities.
     ///
     /// # TODO
     ///
@@ -1064,6 +1068,10 @@ impl GuardMgrInner {
     /// doesn't know whether its circuit came from a guard or a fallback.  To
     /// solve that, we'll need CircMgr to record and report which one it was
     /// using, which will take some more plumbing.
+    ///
+    /// TODO relay: we will have to make the change above when we implement
+    /// relays; otherwise, it would be possible for an attacker to exploit it to
+    /// mislead us about our guard status.
     fn lookup_ids<T>(&self, identity: &T) -> Vec<FirstHopId>
     where
         T: tor_linkspec::HasRelayIds + ?Sized,
@@ -1073,9 +1081,12 @@ impl GuardMgrInner {
 
         let id = ids::GuardId::from_relay_ids(identity);
         for sample in GuardSetSelector::iter() {
-            if self.guards.guards(&sample).contains(&id) {
-                vec.push(FirstHopId(FirstHopIdInner::Guard(sample, id.clone())));
-            }
+            let guard_id = match self.guards.guards(&sample).contains(&id) {
+                Ok(true) => &id,
+                Err(other) => other,
+                Ok(false) => continue,
+            };
+            vec.push(FirstHopId(FirstHopIdInner::Guard(sample, guard_id.clone())));
         }
 
         let id = ids::FallbackId::from_relay_ids(identity);

crates/tor-guardmgr/src/sample.rs

@@ -278,12 +278,24 @@ impl GuardSet {
         self.guards.all_overlapping(relay).is_empty()
     }
 
-    /// Return true if `id` is definitely a member of this set.
-    pub(crate) fn contains(&self, id: &GuardId) -> bool {
-        // XXXX TODO pt-client: fix next.
-
-        // (This could be yes/no/maybe.)
-        self.guards.by_all_ids(id).is_some()
+    /// Return `Ok(true)` if `id` is definitely a member of this set, and
+    /// `Ok(false)` if it is definitely not a member.  
+    ///
+    /// If we cannot tell, it's because there is a guard in this sample that has
+    /// a _subset_ of the IDs in `id`. In that case, we return
+    /// `Err(guard_ident)`, where `guard_ident`  is the identity of that guard.
+    pub(crate) fn contains(&self, id: &GuardId) -> Result<bool, &GuardId> {
+        let overlapping = self.guards.all_overlapping(id);
+        match &overlapping[..] {
+            [singleton] => {
+                if singleton.has_all_relay_ids_from(id) {
+                    Ok(true)
+                } else {
+                    Err(singleton.guard_id())
+                }
+            }
+            _ => Ok(false),
+        }
     }
 
     /// If there are not enough filter-permitted usable guards in this