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.

src/or/bridges.c

@@ -199,6 +199,33 @@ get_configured_bridge_by_addr_port_digest(const tor_addr_t *addr,
   return NULL;
 }
 
+/**
+ * As get_configured_bridge_by_addr_port, but require that the
+ * address match <b>addr</b>:<b>port</b>, and that the ID digest match
+ * <b>digest</b>.  (The other function will ignore the address if the
+ * digest matches.)
+ */
+bridge_info_t *
+get_configured_bridge_by_exact_addr_port_digest(const tor_addr_t *addr,
+                                                uint16_t port,
+                                                const char *digest)
+{
+  if (!bridge_list)
+    return NULL;
+  SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge) {
+    if (!tor_addr_compare(&bridge->addr, addr, CMP_EXACT) &&
+        bridge->port == port) {
+
+      if (digest && tor_memeq(bridge->identity, digest, DIGEST_LEN))
+        return bridge;
+      else if (!digest)
+        return bridge;
+    }
+
+  } SMARTLIST_FOREACH_END(bridge);
+  return NULL;
+}
+
 /** If we have a bridge configured whose digest matches <b>digest</b>, or a
  * bridge with no known digest whose address matches <b>addr</b>:<b>port</b>,
  * return 1.  Else return 0. If <b>digest</b> is NULL, check for

src/or/bridges.h

@@ -27,6 +27,10 @@ bridge_info_t *get_configured_bridge_by_addr_port_digest(
                                           const tor_addr_t *addr,
                                           uint16_t port,
                                           const char *digest);
+bridge_info_t *get_configured_bridge_by_exact_addr_port_digest(
+                                          const tor_addr_t *addr,
+                                          uint16_t port,
+                                          const char *digest);
 
 int addr_is_a_configured_bridge(const tor_addr_t *addr, uint16_t port,
                                 const char *digest);

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