Change approach to preventing duplicate guards.

Previously I'd made a bad assumption in the implementation of
prop271 in 0.3.0.1-alpha: I'd assumed that there couldn't be two
guards with the same identity.  That's true for non-bridges, but in
the bridge case, we allow two bridges to have the same ID if they
have different addr:port combinations -- in order to have the same
bridge ID running multiple PTs.

Fortunately, this assumption wasn't deeply ingrained: we stop
enforcing the "one guard per ID" rule in the bridge case, and
instead enforce "one guard per <id,addr,port>".

We also needed to tweak our implementation of
get_bridge_info_for_guard, since it made the same incorrect
assumption.

Fixes bug 21027; bugfix on 0.3.0.1-alpha.

1 files changed, 15 insertions, 14 deletions

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index 93dd2bd59c..3e871477b4 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -768,11 +768,6 @@ get_sampled_guard_for_bridge(guard_selection_t *gs,
   const uint8_t *id = bridge_get_rsa_id_digest(bridge);
   const tor_addr_port_t *addrport = bridge_get_addr_port(bridge);
   entry_guard_t *guard;
-  if (id) {
-    guard = get_sampled_guard_with_id(gs, id);
-    if (guard)
-      return guard;
-  }
   if (BUG(!addrport))
     return NULL; // LCOV_EXCL_LINE
   guard = get_sampled_guard_by_bridge_addr(gs, addrport);
@@ -787,16 +782,17 @@ get_sampled_guard_for_bridge(guard_selection_t *gs,
 static bridge_info_t *
 get_bridge_info_for_guard(const entry_guard_t *guard)
 {
+  const uint8_t *identity = NULL;
   if (! tor_digest_is_zero(guard->identity)) {
-    bridge_info_t *bridge = find_bridge_by_digest(guard->identity);
-    if (bridge)
-      return bridge;
+    identity = (const uint8_t *)guard->identity;
   }
   if (BUG(guard->bridge_addr == NULL))
     return NULL;
-  return get_configured_bridge_by_addr_port_digest(&guard->bridge_addr->addr,
-                                                     guard->bridge_addr->port,
-                                                     NULL);
+
+  return get_configured_bridge_by_exact_addr_port_digest(
+                                                 &guard->bridge_addr->addr,
+                                                 guard->bridge_addr->port,
+                                                 (const char*)identity);
 }
 
 /**
@@ -820,6 +816,10 @@ entry_guard_add_to_sample(guard_selection_t *gs,
   log_info(LD_GUARD, "Adding %s as to the entry guard sample set.",
            node_describe(node));
 
+  /* make sure that the guard is not already sampled. */
+  if (BUG(have_sampled_guard_with_id(gs, (const uint8_t*)node->identity)))
+    return NULL; // LCOV_EXCL_LINE
+
   return entry_guard_add_to_sample_impl(gs,
                                         (const uint8_t*)node->identity,
                                         node_get_nickname(node),
@@ -843,9 +843,6 @@ entry_guard_add_to_sample_impl(guard_selection_t *gs,
 
   // XXXX #20827 take ed25519 identity here too.
 
-  /* make sure that the guard is not already sampled. */
-  if (rsa_id_digest && BUG(have_sampled_guard_with_id(gs, rsa_id_digest)))
-    return NULL; // LCOV_EXCL_LINE
   /* Make sure we can actually identify the guard. */
   if (BUG(!rsa_id_digest && !bridge_addrport))
     return NULL; // LCOV_EXCL_LINE
@@ -890,6 +887,10 @@ entry_guard_add_bridge_to_sample(guard_selection_t *gs,
 
   tor_assert(addrport);
 
+  /* make sure that the guard is not already sampled. */
+  if (BUG(get_sampled_guard_for_bridge(gs, bridge)))
+    return NULL; // LCOV_EXCL_LINE
+
   return entry_guard_add_to_sample_impl(gs, id_digest, NULL, addrport);
 }