February 2017 - tor-commits - lists.torproject.org

[tor/maint-0.3.0] Also allow C_MEASURE_TIMEOUT circuits to lack guard state.
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 5e08fc85570173200935b62de2bffee81fed2fc1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Feb 24 11:12:21 2017 -0500 Also allow C_MEASURE_TIMEOUT circuits to lack guard state. Fixes a case of 21007; bugfix on 0.3.0.1-alpha when prop271 was implemented. Found by toralf. --- changes/bug21007_case2 | 4 ++++ src/or/circuitbuild.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/changes/bug21007_case2 b/changes/bug21007_case2 new file mode 100644 index 0000000..4334444 --- /dev/null +++ b/changes/bug21007_case2 @@ -0,0 +1,4 @@ + o Minor bugfixes (guards): + - Don't warn about a missing guard state on timeout-measurement + circuits: they aren't supposed to be using guards. Fixes an + instance of bug 21007; bugfix on 0.3.0.1-alpha. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index cd00034..79962e8 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -892,6 +892,27 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out, } } +/** + * Return true iff purpose is a purpose for a circuit which is + * allowed to have no guard configured, even if the circuit is multihop + * and guards are enabled. + */ +static int +circuit_purpose_may_omit_guard(int purpose) +{ + switch (purpose) { + case CIRCUIT_PURPOSE_TESTING: + case CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT: + /* Testing circuits may omit guards because they're measuring + * liveness or performance, and don't want guards to interfere. */ + return 1; + default: + /* All other multihop circuits should use guards if guards are + * enabled. */ + return 0; + } +} + /** This is the backbone function for building circuits. * * If circ's first hop is closed, then we need to build a create @@ -969,7 +990,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) guard_usable_t r; if (! circ->guard_state) { if (circuit_get_cpath_len(circ) != 1 && - circ->base_.purpose != CIRCUIT_PURPOSE_TESTING && + ! circuit_purpose_may_omit_guard(circ->base_.purpose) && get_options()->UseEntryGuards) { log_warn(LD_BUG, "%d-hop circuit %p with purpose %d has no " "guard state",

1 0

[tor/master] Also allow C_MEASURE_TIMEOUT circuits to lack guard state.
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 5e08fc85570173200935b62de2bffee81fed2fc1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Feb 24 11:12:21 2017 -0500 Also allow C_MEASURE_TIMEOUT circuits to lack guard state. Fixes a case of 21007; bugfix on 0.3.0.1-alpha when prop271 was implemented. Found by toralf. --- changes/bug21007_case2 | 4 ++++ src/or/circuitbuild.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/changes/bug21007_case2 b/changes/bug21007_case2 new file mode 100644 index 0000000..4334444 --- /dev/null +++ b/changes/bug21007_case2 @@ -0,0 +1,4 @@ + o Minor bugfixes (guards): + - Don't warn about a missing guard state on timeout-measurement + circuits: they aren't supposed to be using guards. Fixes an + instance of bug 21007; bugfix on 0.3.0.1-alpha. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index cd00034..79962e8 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -892,6 +892,27 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out, } } +/** + * Return true iff purpose is a purpose for a circuit which is + * allowed to have no guard configured, even if the circuit is multihop + * and guards are enabled. + */ +static int +circuit_purpose_may_omit_guard(int purpose) +{ + switch (purpose) { + case CIRCUIT_PURPOSE_TESTING: + case CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT: + /* Testing circuits may omit guards because they're measuring + * liveness or performance, and don't want guards to interfere. */ + return 1; + default: + /* All other multihop circuits should use guards if guards are + * enabled. */ + return 0; + } +} + /** This is the backbone function for building circuits. * * If circ's first hop is closed, then we need to build a create @@ -969,7 +990,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) guard_usable_t r; if (! circ->guard_state) { if (circuit_get_cpath_len(circ) != 1 && - circ->base_.purpose != CIRCUIT_PURPOSE_TESTING && + ! circuit_purpose_may_omit_guard(circ->base_.purpose) && get_options()->UseEntryGuards) { log_warn(LD_BUG, "%d-hop circuit %p with purpose %d has no " "guard state",

1 0

[tor/master] Merge remote-tracking branch 'public/bug21007_case2_030' into maint-0.3.0
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 3a60214f32bc6e1ba6d8c6dd6c6abda0c5be1f1e Merge: 16f337e 5e08fc8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 28 08:28:46 2017 -0500 Merge remote-tracking branch 'public/bug21007_case2_030' into maint-0.3.0 changes/bug21007_case2 | 4 ++++ src/or/circuitbuild.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.0] Merge remote-tracking branch 'public/bug21007_case2_030' into maint-0.3.0
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 3a60214f32bc6e1ba6d8c6dd6c6abda0c5be1f1e Merge: 16f337e 5e08fc8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 28 08:28:46 2017 -0500 Merge remote-tracking branch 'public/bug21007_case2_030' into maint-0.3.0 changes/bug21007_case2 | 4 ++++ src/or/circuitbuild.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)

1 0

[tor/master] Merge branch 'maint-0.3.0'
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 811280013848536e6907b5880f30c75202170044 Merge: 9282355 3a60214 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 28 08:28:55 2017 -0500 Merge branch 'maint-0.3.0' changes/bug21007_case2 | 4 ++++ src/or/circuitbuild.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.0] Also allow C_MEASURE_TIMEOUT circuits to lack guard state.
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 5e08fc85570173200935b62de2bffee81fed2fc1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Feb 24 11:12:21 2017 -0500 Also allow C_MEASURE_TIMEOUT circuits to lack guard state. Fixes a case of 21007; bugfix on 0.3.0.1-alpha when prop271 was implemented. Found by toralf. --- changes/bug21007_case2 | 4 ++++ src/or/circuitbuild.c | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/changes/bug21007_case2 b/changes/bug21007_case2 new file mode 100644 index 0000000..4334444 --- /dev/null +++ b/changes/bug21007_case2 @@ -0,0 +1,4 @@ + o Minor bugfixes (guards): + - Don't warn about a missing guard state on timeout-measurement + circuits: they aren't supposed to be using guards. Fixes an + instance of bug 21007; bugfix on 0.3.0.1-alpha. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index cd00034..79962e8 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -892,6 +892,27 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out, } } +/** + * Return true iff purpose is a purpose for a circuit which is + * allowed to have no guard configured, even if the circuit is multihop + * and guards are enabled. + */ +static int +circuit_purpose_may_omit_guard(int purpose) +{ + switch (purpose) { + case CIRCUIT_PURPOSE_TESTING: + case CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT: + /* Testing circuits may omit guards because they're measuring + * liveness or performance, and don't want guards to interfere. */ + return 1; + default: + /* All other multihop circuits should use guards if guards are + * enabled. */ + return 0; + } +} + /** This is the backbone function for building circuits. * * If circ's first hop is closed, then we need to build a create @@ -969,7 +990,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) guard_usable_t r; if (! circ->guard_state) { if (circuit_get_cpath_len(circ) != 1 && - circ->base_.purpose != CIRCUIT_PURPOSE_TESTING && + ! circuit_purpose_may_omit_guard(circ->base_.purpose) && get_options()->UseEntryGuards) { log_warn(LD_BUG, "%d-hop circuit %p with purpose %d has no " "guard state",

1 0

[tor/maint-0.3.0] Replace entry_guard_get_by_id_digest_for_guard_selection impl.
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit f5995692dadf9bc038455d7e701be84937f28d82 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 14 10:28:54 2017 -0500 Replace entry_guard_get_by_id_digest_for_guard_selection impl. We already implemented this whole function somewhere else; no need to have the same code twice. --- src/or/entrynodes.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 3bb5169..93dd2bd 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2901,13 +2901,7 @@ entry_guard_t * entry_guard_get_by_id_digest_for_guard_selection(guard_selection_t *gs, const char *digest) { - tor_assert(gs != NULL); - - SMARTLIST_FOREACH(gs->sampled_entry_guards, entry_guard_t *, entry, - if (tor_memeq(digest, entry->identity, DIGEST_LEN)) - return entry; - ); - return NULL; + return get_sampled_guard_with_id(gs, (const uint8_t*)digest); } /** Return the node_t associated with a single entry_guard_t. May

1 0

[tor/master] Replace entry_guard_get_by_id_digest_for_guard_selection impl.
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit f5995692dadf9bc038455d7e701be84937f28d82 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 14 10:28:54 2017 -0500 Replace entry_guard_get_by_id_digest_for_guard_selection impl. We already implemented this whole function somewhere else; no need to have the same code twice. --- src/or/entrynodes.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 3bb5169..93dd2bd 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2901,13 +2901,7 @@ entry_guard_t * entry_guard_get_by_id_digest_for_guard_selection(guard_selection_t *gs, const char *digest) { - tor_assert(gs != NULL); - - SMARTLIST_FOREACH(gs->sampled_entry_guards, entry_guard_t *, entry, - if (tor_memeq(digest, entry->identity, DIGEST_LEN)) - return entry; - ); - return NULL; + return get_sampled_guard_with_id(gs, (const uint8_t*)digest); } /** Return the node_t associated with a single entry_guard_t. May

1 0

[tor/maint-0.3.0] Merge branch 'bug21027_v2_squashed' into maint-0.3.0
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 16f337e7639c42b5c6106d6bc9828cb9e6614054 Merge: eef8bd4 1582ada Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Feb 28 08:16:43 2017 -0500 Merge branch 'bug21027_v2_squashed' into maint-0.3.0 changes/bug21027 | 8 ++++++++ src/or/bridges.c | 27 +++++++++++++++++++++++++++ src/or/bridges.h | 4 ++++ src/or/entrynodes.c | 37 ++++++++++++++++--------------------- 4 files changed, 55 insertions(+), 21 deletions(-)

1 0

[tor/maint-0.3.0] Change approach to preventing duplicate guards.
by nickm＠torproject.org 28 Feb '17

28 Feb '17

commit 1582adabbb13871bcf6f91be4fa8523aeb36f79a Author: Nick Mathewson <nickm(a)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 addr:port, and that the ID digest match + * digest. (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 digest, or a * bridge with no known digest whose address matches addr:port, * return 1. Else return 0. If digest 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); }

1 0