commit 1582adabbb13871bcf6f91be4fa8523aeb36f79a Author: Nick Mathewson nickm@torproject.org Date: Tue Feb 14 12:21:31 2017 -0500

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. --- changes/bug21027 | 8 ++++++++ src/or/bridges.c | 27 +++++++++++++++++++++++++++ src/or/bridges.h | 4 ++++ src/or/entrynodes.c | 29 +++++++++++++++-------------- 4 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/changes/bug21027 b/changes/bug21027 new file mode 100644 index 0000000..d20df87 --- /dev/null +++ b/changes/bug21027 @@ -0,0 +1,8 @@ + o Major bugfixes (bridges): + + - When the same bridge is configured multiple times at different + address:port combinations (but with the same identity), treat + those bridge instances as separate guards. This allows clients to + configure the same bridge with multiple pluggable transports, once + again. Fixes bug 21027; bugfix on 0.3.0.1-alpha. + diff --git a/src/or/bridges.c b/src/or/bridges.c index 8b37f41..f766931 100644 --- a/src/or/bridges.c +++ b/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 diff --git a/src/or/bridges.h b/src/or/bridges.h index de23fe6..27ea5e1 100644 --- a/src/or/bridges.h +++ b/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); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 93dd2bd..3e87147 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); }