From 1e95a4a1f6d074e5f5f0033f596dd5ee66eea58a Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Sun, 3 Jun 2012 00:21:49 +0300 Subject: [PATCH 1/3] Improve conflict resolution when adding new bridges. --- src/or/circuitbuild.c | 88 +++++++++++++++++++++++++++++++++++++++++-- 1 file 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 bridge has the same identity digest as + * digest. digest 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 bridge uses the same pluggable transport as + * transport_name. transport_name 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 addr:port, with + * optional digest and transport_name. 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 digest/transport_name, + 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 addr:port. If digest * 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); From 64167e1772b62a06fe8853b84c3cd027abcc7f74 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 5 Jun 2012 11:39:31 -0400 Subject: [PATCH 2/3] Minor changes to bug5603 * Minor stylistic changes to comments and doxygen * Use strcmp_opt; it already exists. * Tighten bridge_has_digest implementation a little. --- src/or/circuitbuild.c | 54 +++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 8e7948c6e0..b80c7130eb 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4912,52 +4912,34 @@ learned_router_identity(const tor_addr_t *addr, uint16_t port, } } -/** Returns true if bridge has the same identity digest as - * digest. digest is optional in which case it matches - * bridges without identity digests. */ +/** Return true if bridge has the same identity digest as + * digest. If digest is NULL, it matches + * bridges with unspecified 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; + if (digest) + return tor_memeq(digest, bridge->identity, DIGEST_LEN); + else + return tor_digest_is_zero(bridge->identity); } -/** Returns true if bridge uses the same pluggable transport as - * transport_name. transport_name 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 addr:port, with - * optional digest and transport_name. 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. +/** We are about to add a new bridge at addr:port, with optional + * digest and transport_name. Mark for removal any previously + * existing bridge with the same address and port, and warn the user as + * appropriate. */ 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: + /* 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 digest/transport_name, - it's probably a misconfiguration and we should warn the user. + 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 digest/transport_name, + 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) @@ -4968,7 +4950,7 @@ bridge_resolve_conflicts(const tor_addr_t *addr, uint16_t port, bridge->marked_for_removal = 1; if (!bridge_has_digest(bridge, digest) || - !bridge_has_transport_name(bridge, transport_name)) { + strcmp_opt(bridge->transport_name, transport_name)) { /* warn the user */ char *bridge_description_new, *bridge_description_old; tor_asprintf(&bridge_description_new, "%s:%u:%s:%s", From 7cad2e73481bdfa04c3d4a69f026784386440f0b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 5 Jun 2012 11:47:16 -0400 Subject: [PATCH 3/3] Changes file for bug 5603 --- changes/bug5603 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changes/bug5603 diff --git a/changes/bug5603 b/changes/bug5603 new file mode 100644 index 0000000000..97f8988239 --- /dev/null +++ b/changes/bug5603 @@ -0,0 +1,9 @@ + o Minor bugfixes: + - When told to add a bridge with the same addr:port as a + preexisting bridge but a different transport, change the + transport as requested. Previously we would not notice the + change. Fix for bug 5603; fix on 0.2.3.2-alpha. + - When told to add a bridge with the same digest as a + preexisting bridge but a different addr:port, change the + addr:port as requested. Previously we would not notice the + change. Fix for bug 5603; fix on 0.2.2.26-beta.