Improve conflict resolution when adding new bridges.

1 files changed, 84 insertions, 4 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 82ff327135..8e7948c6e0 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -4912,6 +4912,89 @@ learned_router_identity(const tor_addr_t *addr, uint16_t port,
   }
 }
 
+/** Returns true if <b>bridge</b> has the same identity digest as
+ *  <b>digest</b>. <b>digest</b> is optional in which case it matches
+ *  bridges without identity digests. */
+static int
+bridge_has_digest(const bridge_info_t *bridge, const char *digest)
+{
+  if (!digest && tor_digest_is_zero(bridge->identity))
+    return 1;
+  if (digest && tor_memeq(digest, bridge->identity, DIGEST_LEN))
+    return 1;
+  return 0;
+}
+
+/** Returns true if <b>bridge</b> uses the same pluggable transport as
+ *  <b>transport_name</b>. <b>transport_name</b> is optional in which
+ *  case it matches bridges without pluggable transports. */
+static int
+bridge_has_transport_name(const bridge_info_t *bridge,
+                          const char *transport_name)
+{
+  if (!transport_name && !bridge->transport_name)
+    return 1;
+  if (transport_name && !strcmp(transport_name, bridge->transport_name))
+    return 1;
+  return 0;
+}
+
+/** We want to add a new bridge at <b>addr</b>:<b>port</b>, with
+ *  optional <b>digest</b> and <b>transport_name</b>. See if this
+ *  generates any conflicts with already registered bridges, try to
+ *  resolve them and warn the user.
+ *
+ *  This function might end up marking already registered bridges to
+ *  be deleted.
+ */
+static void
+bridge_resolve_conflicts(const tor_addr_t *addr, uint16_t port,
+                         const char *digest, const char *transport_name)
+{
+  /** Iterate the already-registered bridge list:
+
+      If you find a bridge with the same adress and port, mark it for
+      removal. It doesn't make sense to have two active bridges with
+      the same IP:PORT. If the bridge in question has a different
+      digest or transport than <b>digest</b>/<b>transport_name</b>,
+      it's probably a misconfiguration and we should warn the user.
+  */
+  SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge) {
+    if (bridge->marked_for_removal)
+      continue;
+
+    if (tor_addr_eq(&bridge->addr, addr) && (bridge->port == port)) {
+
+      bridge->marked_for_removal = 1;
+
+      if (!bridge_has_digest(bridge, digest) ||
+          !bridge_has_transport_name(bridge, transport_name)) {
+        /* warn the user */
+        char *bridge_description_new, *bridge_description_old;
+        tor_asprintf(&bridge_description_new, "%s:%u:%s:%s",
+                     fmt_addr(addr), port,
+                     digest ? hex_str(digest, DIGEST_LEN) : "",
+                     transport_name ? transport_name : "");
+        tor_asprintf(&bridge_description_old, "%s:%u:%s:%s",
+                     fmt_addr(&bridge->addr), bridge->port,
+                     tor_digest_is_zero(bridge->identity) ?
+                     "" : hex_str(bridge->identity,DIGEST_LEN),
+                     bridge->transport_name ? bridge->transport_name : "");
+
+        log_warn(LD_GENERAL,"Tried to add bridge '%s', but we found a conflict"
+                 " with the already registered bridge '%s'. We will discard"
+                 " the old bridge and keep '%s'. If this is not what you"
+                 " wanted, please change your configuration file accordingly.",
+                 bridge_description_new, bridge_description_old,
+                 bridge_description_new);
+
+        tor_free(bridge_description_new);
+        tor_free(bridge_description_old);
+      }
+    }
+  } SMARTLIST_FOREACH_END(bridge);
+}
+
 /** Remember a new bridge at <b>addr</b>:<b>port</b>. If <b>digest</b>
  * is set, it tells us the identity key too.  If we already had the
  * bridge in our list, unmark it, and don't actually add anything new.
@@ -4923,10 +5006,7 @@ bridge_add_from_config(const tor_addr_t *addr, uint16_t port,
 {
   bridge_info_t *b;
 
-  if ((b = get_configured_bridge_by_addr_port_digest(addr, port, digest))) {
-    b->marked_for_removal = 0;
-    return;
-  }
+  bridge_resolve_conflicts(addr, port, digest, transport_name);
 
   b = tor_malloc_zero(sizeof(bridge_info_t));
   tor_addr_copy(&b->addr, addr);