- tor-commits - lists.torproject.org

[tor/master] Limit IPv6 mask bits to 128
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit e54e71fb6bb0f76bf73b540580abd0a5c5c8539d Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Wed Dec 16 08:48:32 2015 +1100 Limit IPv6 mask bits to 128 --- src/common/address.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common/address.c b/src/common/address.c index 6277641..69a8098 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -1039,6 +1039,8 @@ tor_addr_compare_masked(const tor_addr_t *addr1, const tor_addr_t *addr2, return r; } case AF_INET6: { + if (mbits > 128) + mbits = 128; const uint8_t *a1 = tor_addr_to_in6_addr8(addr1); const uint8_t *a2 = tor_addr_to_in6_addr8(addr2); const int bytes = mbits >> 3;

1 0

[tor/master] Initialise malformed_list to 0 each time we parse a policy
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit cd0a5db5e9ffbe873fba1a0f4965cdd008824247 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Wed Dec 16 08:50:13 2015 +1100 Initialise malformed_list to 0 each time we parse a policy --- src/or/routerparse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index f6619cb..8170bbb 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3697,6 +3697,7 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action, memarea_t *area = NULL; tor_assert(malformed_list); + *malformed_list = 0; s = eat_whitespace(s); if ((*s == '*' || TOR_ISDIGIT(*s)) && assume_action >= 0) {

1 0

[tor/master] Add policy assume_action support for IPv6 addresses
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit ce92335214f4490f0e14487a99415c26777be2a8 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Wed Dec 16 08:49:06 2015 +1100 Add policy assume_action support for IPv6 addresses These IPv6 addresses must be quoted, because : is the port separator, and "acce" is a valid hex block. Add unit tests for assumed actions in IPv6 policies. --- src/or/routerparse.c | 8 +++-- src/test/test_policy.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 8170bbb..f5a003e 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3684,8 +3684,8 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action, directory_token_t *tok = NULL; const char *cp, *eos; /* Longest possible policy is - * "accept6 ffff:ffff:..255/128:10000-65535", - * which contains a max-length IPv6 address, plus 24 characters. + * "accept6 [ffff:ffff:..255]/128:10000-65535", + * which contains a max-length IPv6 address, plus 26 characters. * But note that there can be an arbitrary amount of space between the * accept and the address:mask/port element. * We don't need to multiply TOR_ADDR_BUF_LEN by 2, as there is only one @@ -3700,7 +3700,9 @@ router_parse_addr_policy_item_from_string,(const char *s, int assume_action, *malformed_list = 0; s = eat_whitespace(s); - if ((*s == '*' || TOR_ISDIGIT(*s)) && assume_action >= 0) { + /* We can only do assume_action on []-quoted IPv6, as "a" (accept) + * and ":" (port separator) are ambiguous */ + if ((*s == '*' || *s == '[' || TOR_ISDIGIT(*s)) && assume_action >= 0) { if (tor_snprintf(line, sizeof(line), "%s %s", assume_action == ADDR_POLICY_ACCEPT?"accept":"reject", s)<0) { log_warn(LD_DIR, "Policy %s is too long.", escaped(s)); diff --git a/src/test/test_policy.c b/src/test/test_policy.c index b7d89a1..4f5565e 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -270,6 +270,93 @@ test_policies_general(void *arg) addr_policy_list_free(policy); policy = NULL; + /* make sure assume_action works */ + malformed_list = 0; + p = router_parse_addr_policy_item_from_string("127.0.0.1", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("127.0.0.1:*", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("[::]", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("[::]:*", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("[face::b]", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("[b::aaaa]", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("*", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("*4", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + p = router_parse_addr_policy_item_from_string("*6", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(p); + addr_policy_free(p); + tt_assert(!malformed_list); + + /* These are all ambiguous IPv6 addresses, it's good that we reject them */ + p = router_parse_addr_policy_item_from_string("acce::abcd", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(!p); + tt_assert(malformed_list); + malformed_list = 0; + + p = router_parse_addr_policy_item_from_string("7:1234", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(!p); + tt_assert(malformed_list); + malformed_list = 0; + + p = router_parse_addr_policy_item_from_string("::", + ADDR_POLICY_ACCEPT, + &malformed_list); + tt_assert(!p); + tt_assert(malformed_list); + malformed_list = 0; + /* make sure compacting logic works. */ policy = NULL; line.key = (char*)"foo";

1 0

[tor/master] Warn when comparing against an AF_UNSPEC address in a policy
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit e2e09a2dbeae508ef8539e48eff1babb6aa0346e Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Wed Dec 16 08:47:47 2015 +1100 Warn when comparing against an AF_UNSPEC address in a policy It produces unexpected results, and it's most likely a bug. --- changes/feature17863 | 6 ++++++ src/or/policies.c | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/changes/feature17863 b/changes/feature17863 new file mode 100644 index 0000000..86c4e2c --- /dev/null +++ b/changes/feature17863 @@ -0,0 +1,6 @@ + o Minor feature (IPv6): + - Add address policy assume_action support for IPv6 addresses. + - Limit IPv6 mask bits to 128. + - Warn when comparing against an AF_UNSPEC address in a policy, + it's almost always a bug. + Closes ticket 17863; patch by "teor". diff --git a/src/or/policies.c b/src/or/policies.c index 32a7ec2..c9bce1b 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -696,6 +696,10 @@ compare_known_tor_addr_to_addr_policy(const tor_addr_t *addr, uint16_t port, /* We know the address and port, and we know the policy, so we can just * compute an exact match. */ SMARTLIST_FOREACH_BEGIN(policy, addr_policy_t *, tmpe) { + if (tmpe->addr.family == AF_UNSPEC) { + log_warn(LD_BUG, "Policy contains an AF_UNSPEC address, which only " + "matches other AF_UNSPEC addresses."); + } /* Address is known */ if (!tor_addr_compare_masked(addr, &tmpe->addr, tmpe->maskbits, CMP_EXACT)) { @@ -723,6 +727,10 @@ compare_known_tor_addr_to_addr_policy_noport(const tor_addr_t *addr, int maybe_accept = 0, maybe_reject = 0; SMARTLIST_FOREACH_BEGIN(policy, addr_policy_t *, tmpe) { + if (tmpe->addr.family == AF_UNSPEC) { + log_warn(LD_BUG, "Policy contains an AF_UNSPEC address, which only " + "matches other AF_UNSPEC addresses."); + } if (!tor_addr_compare_masked(addr, &tmpe->addr, tmpe->maskbits, CMP_EXACT)) { if (tmpe->prt_min <= 1 && tmpe->prt_max >= 65535) { @@ -762,6 +770,10 @@ compare_unknown_tor_addr_to_addr_policy(uint16_t port, int maybe_accept = 0, maybe_reject = 0; SMARTLIST_FOREACH_BEGIN(policy, addr_policy_t *, tmpe) { + if (tmpe->addr.family == AF_UNSPEC) { + log_warn(LD_BUG, "Policy contains an AF_UNSPEC address, which only " + "matches other AF_UNSPEC addresses."); + } if (tmpe->prt_min <= port && port <= tmpe->prt_max) { if (tmpe->maskbits == 0) { /* Definitely matches, since it covers all addresses. */

1 0

[tor/master] Merge remote-tracking branch 'teor/feature17863'
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit bb23ad3e47d2953079fb1d8d7c02db939a30633b Merge: 10e442b e2e09a2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 16 08:48:28 2015 -0500 Merge remote-tracking branch 'teor/feature17863' changes/feature17863 | 6 ++++ src/common/address.c | 2 ++ src/or/policies.c | 12 +++++++ src/or/routerparse.c | 9 +++-- src/test/test_policy.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 3 deletions(-)

1 0

[tor/master] tweak router_parse_addr_policy_item_from_string docs
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit 9d17d10b36129305ef0f851ade67bd087f25ac0e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 16 08:49:32 2015 -0500 tweak router_parse_addr_policy_item_from_string docs --- src/or/routerparse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index f5a003e..3f794ad 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3668,10 +3668,10 @@ networkstatus_parse_detached_signatures(const char *s, const char *eos) * * Returns NULL on policy errors. * - * If there is a policy error, malformed_list is set to true if the entire - * policy list should be discarded. Otherwise, it is set to false, and only - * this item should be ignored - the rest of the policy list can continue to - * be processed and used. + * Set *<b>malformed_list>/b> to true if the entire policy list should be + * discarded. Otherwise, set it to false, and only this item should be ignored + * on error - the rest of the policy list can continue to be processed and + * used. * * The addr_policy_t returned by this function can have its address set to * AF_UNSPEC for '*'. Use policy_expand_unspec() to turn this into a pair

1 0

[tor/master] Merge remote-tracking branch 'teor/feature17864'
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit 10e442ba93be0c41daec10319943aa19c616b281 Merge: a03469a 978210d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 16 08:41:20 2015 -0500 Merge remote-tracking branch 'teor/feature17864' changes/feature17864 | 5 +++++ src/or/routerlist.c | 10 ++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)

1 0

[tor/master] Wait for busy authorities/fallbacks rather than ignoring excluded nodes
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit 978210d5a868e8b0f5bd5f83804117ae46954f5f Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Mon Dec 14 20:20:52 2015 +1100 Wait for busy authorities/fallbacks rather than ignoring excluded nodes Applies the 6c443e987d fix to router_pick_directory_server_impl. 6c443e987d applied to directory servers chosen from the consensus, and was: "Tweak the 9969 fix a little If we have busy nodes and excluded nodes, then don't retry with the excluded ones enabled. Instead, wait for the busy ones to be nonbusy." --- changes/feature17864 | 5 +++++ src/or/routerlist.c | 10 ++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/changes/feature17864 b/changes/feature17864 new file mode 100644 index 0000000..1cf1562 --- /dev/null +++ b/changes/feature17864 @@ -0,0 +1,5 @@ + o Minor feature (directory downloads): + - Wait for busy authorities and fallbacks to become non-busy when + bootstrapping. (A similar change was made in 6c443e987d for + directory servers chosen from the consensus.) + Closes ticket 17864; patch by "teor". diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 051bac5..c45854c 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1739,22 +1739,24 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist, result = &selection->fake_status; } - if (n_busy_out) - *n_busy_out = n_busy; - smartlist_free(direct); smartlist_free(tunnel); smartlist_free(overloaded_direct); smartlist_free(overloaded_tunnel); - if (result == NULL && try_excluding && !options->StrictNodes && n_excluded) { + if (result == NULL && try_excluding && !options->StrictNodes && n_excluded + && !n_busy) { /* If we got no result, and we are excluding nodes, and StrictNodes is * not set, try again without excluding nodes. */ try_excluding = 0; n_excluded = 0; + n_busy = 0; goto retry_without_exclude; } + if (n_busy_out) + *n_busy_out = n_busy; + return result; }

1 0

[tor/master] More debugging code to try to track down #17659
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit a03469aa851bf6e658d539e95b5f1e786f9622bc Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 16 08:37:40 2015 -0500 More debugging code to try to track down #17659 --- src/or/connection_edge.c | 31 ++++++++++++++++++++++++++++--- src/or/or.h | 7 +++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 7a2de29..961e49a 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -780,6 +780,15 @@ connection_ap_rescan_and_attach_pending(void) connection_ap_attach_pending(1); } +#ifdef DEBUGGING_17659 +#define UNMARK() do { \ + entry_conn->marked_pending_circ_line = 0; \ + entry_conn->marked_pending_circ_file = 0; \ + } while (0) +#else +#define UNMARK() do { } while (0) +#endif + /** Tell any AP streams that are listed as waiting for a new circuit to try * again, either attaching to an available circ or launching a new one. * @@ -801,12 +810,14 @@ connection_ap_attach_pending(int retry) connection_t *conn = ENTRY_TO_CONN(entry_conn); tor_assert(conn && entry_conn); if (conn->marked_for_close) { + UNMARK(); SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); continue; } if (conn->magic != ENTRY_CONNECTION_MAGIC) { - log_warn(LD_BUG, "%p has impossible magic value %u", + log_warn(LD_BUG, "%p has impossible magic value %u.", entry_conn, (unsigned)conn->magic); + UNMARK(); SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); continue; } @@ -815,6 +826,7 @@ connection_ap_attach_pending(int retry) "is %s. Why is it on pending_entry_connections?", entry_conn, conn_state_to_string(conn->type, conn->state)); + UNMARK(); SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); continue; } @@ -828,6 +840,7 @@ connection_ap_attach_pending(int retry) if (conn->marked_for_close || conn->type != CONN_TYPE_AP || conn->state != AP_CONN_STATE_CIRCUIT_WAIT) { + UNMARK(); SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); continue; } @@ -862,12 +875,23 @@ connection_ap_mark_as_pending_circuit_(entry_connection_t *entry_conn, if (PREDICT_UNLIKELY(smartlist_contains(pending_entry_connections, entry_conn))) { log_warn(LD_BUG, "What?? pending_entry_connections already contains %p! " - "(called from %s:%d)", + "(Called from %s:%d.)", entry_conn, fname, lineno); - log_backtrace(LOG_WARN, LD_BUG, "To debug, this may help."); +#ifdef DEBUGGING_17659 + const char *f2 = entry_conn->marked_pending_circ_file; + log_warn(LD_BUG, "(Previously called from %s:%d.)\n", + f2 ? f2 : "<NULL>", + entry_conn->marked_pending_circ_line); +#endif + log_backtrace(LOG_WARN, LD_BUG, "To debug, this may help"); return; } +#ifdef DEBUGGING_17659 + entry_conn->marked_pending_circ_line = (uint16_t) lineno; + entry_conn->marked_pending_circ_file = fname; +#endif + untried_pending_connections = 1; smartlist_add(pending_entry_connections, entry_conn); } @@ -878,6 +902,7 @@ connection_ap_mark_as_non_pending_circuit(entry_connection_t *entry_conn) { if (PREDICT_UNLIKELY(NULL == pending_entry_connections)) return; + UNMARK(); smartlist_remove(pending_entry_connections, entry_conn); } diff --git a/src/or/or.h b/src/or/or.h index e92a4bb..e621fe9 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1646,6 +1646,13 @@ typedef struct entry_connection_t { * request that we're going to try to answer. */ struct evdns_server_request *dns_server_request; +#define DEBUGGING_17659 + +#ifdef DEBUGGING_17659 + uint16_t marked_pending_circ_line; + const char *marked_pending_circ_file; +#endif + #define NUM_CIRCUITS_LAUNCHED_THRESHOLD 10 /** Number of times we've launched a circuit to handle this stream. If * it gets too high, that could indicate an inconsistency between our

1 0

[tor/master] Add some assertions to try to catch #17752
by nickm＠torproject.org 16 Dec '15

16 Dec '15

commit a4ca2ef1fffec0181b18f24a4aaaa71856108ce2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Dec 16 08:24:54 2015 -0500 Add some assertions to try to catch #17752 --- src/or/connection_edge.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 30dcd13..7a2de29 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -765,6 +765,7 @@ connection_ap_rescan_and_attach_pending(void) continue; entry_conn = TO_ENTRY_CONN(conn); + tor_assert(entry_conn); if (! smartlist_contains(pending_entry_connections, entry_conn)) { log_warn(LD_BUG, "Found a connection %p that was supposed to be " "in pending_entry_connections, but wasn't. No worries; " @@ -798,6 +799,7 @@ connection_ap_attach_pending(int retry) SMARTLIST_FOREACH_BEGIN(pending_entry_connections, entry_connection_t *, entry_conn) { connection_t *conn = ENTRY_TO_CONN(entry_conn); + tor_assert(conn && entry_conn); if (conn->marked_for_close) { SMARTLIST_DEL_CURRENT(pending_entry_connections, entry_conn); continue;

1 0