- tor-commits - lists.torproject.org

[tor/release-0.2.3] add a unit test to expose bug 7192
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 4c8b58f9005b7a187878273b8ff0f8ec23e86326 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Oct 22 17:09:43 2012 -0400 add a unit test to expose bug 7192 --- src/or/policies.c | 27 +++++++++++++++++++++++++++ src/or/policies.h | 1 + src/test/test.c | 4 ++++ 3 files changed, 32 insertions(+), 0 deletions(-) diff --git a/src/or/policies.c b/src/or/policies.c index 6e98421..486c264 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -1413,6 +1413,33 @@ parse_short_policy(const char *summary) return result; } +/** Write policy back out into a string. Used only for unit tests + * currently. */ +const char * +write_short_policy(const short_policy_t *policy) +{ + int i; + char *answer; + smartlist_t *sl = smartlist_new(); + + smartlist_add_asprintf(sl, "%s", policy->is_accept ? "accept " : "reject "); + + for(i=0; i < policy->n_entries; i++) { + const short_policy_entry_t *e = &policy->entries[i]; + if (e->min_port == e->max_port) { + smartlist_add_asprintf(sl, "%d", e->min_port); + } else { + smartlist_add_asprintf(sl, "%d-%d", e->min_port, e->max_port); + } + if (i < policy->n_entries-1) + smartlist_add(sl, tor_strdup(",")); + } + answer = smartlist_join_strings(sl, "", 0, NULL); + SMARTLIST_FOREACH(sl, char *, a, tor_free(a)); + smartlist_free(sl); + return answer; +} + /** Release all storage held in policy. */ void short_policy_free(short_policy_t *policy) diff --git a/src/or/policies.h b/src/or/policies.h index 31f3f06..b385d8e 100644 --- a/src/or/policies.h +++ b/src/or/policies.h @@ -61,6 +61,7 @@ void policies_free_all(void); char *policy_summarize(smartlist_t *policy); short_policy_t *parse_short_policy(const char *summary); +const char *write_short_policy(const short_policy_t *policy); void short_policy_free(short_policy_t *policy); int short_policy_is_reject_star(const short_policy_t *policy); addr_policy_result_t compare_tor_addr_to_short_policy( diff --git a/src/test/test.c b/src/test/test.c index 6bf2d28..9b510d2 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1014,6 +1014,7 @@ test_policy_summary_helper(const char *policy_str, config_line_t line; smartlist_t *policy = smartlist_new(); char *summary = NULL; + const char *summary_after; int r; short_policy_t *short_policy = NULL; @@ -1030,8 +1031,11 @@ test_policy_summary_helper(const char *policy_str, short_policy = parse_short_policy(summary); tt_assert(short_policy); + summary_after = write_short_policy(short_policy); + test_streq(summary, summary_after); done: + tor_free(summary_after); tor_free(summary); if (policy) addr_policy_list_free(policy);

1 0

[tor/release-0.2.3] Fix parse_short_policy (bug 7192.)
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 85659d3964669f9f419123c648e517f4ba539462 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 22 17:34:05 2012 -0400 Fix parse_short_policy (bug 7192.) Our implementation of parse_short_policy was screwed up: it would ignore the last character of every short policy. Obviously, that's broken. This patch fixes the busted behavior, and adds a bunch of unit tests to make sure the rest of that function is okay. Fixes bug 7192; fix on 0.2.3.1-alpha. --- changes/bug7192 | 10 ++++++++ src/or/policies.c | 18 +++++++++----- src/or/policies.h | 2 +- src/test/test.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/changes/bug7192 b/changes/bug7192 new file mode 100644 index 0000000..10cbc24 --- /dev/null +++ b/changes/bug7192 @@ -0,0 +1,10 @@ + o Major bugfixes: + - When parsing exit policy summaries from microdescriptors, we had + previously been ignoring the last character in each one, so that + "accept 80,443,8080" would be treated by clients as indicating a + node that allows access to ports 80, 443, and 808. That would lead + to clients attempting connections that could never work, and + ignoring exit nodes that would support their connections. Now clients + parse these exit policy summaries correctly. Fixes bug 7192; + bugfix on 0.2.3.1-alpha. + diff --git a/src/or/policies.c b/src/or/policies.c index 486c264..bbd6816 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -1348,8 +1348,10 @@ parse_short_policy(const char *summary) unsigned low, high; char dummy; char ent_buf[32]; + size_t len; next = comma ? comma+1 : strchr(summary, '\0'); + len = comma ? (size_t)(comma - summary) : strlen(summary); if (n_entries == MAX_EXITPOLICY_SUMMARY_LEN) { log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Impossibly long policy summary %s", @@ -1357,20 +1359,22 @@ parse_short_policy(const char *summary) return NULL; } - if (! TOR_ISDIGIT(*summary) || next-summary > (int)(sizeof(ent_buf)-1)) { + if (! TOR_ISDIGIT(*summary) || len > (sizeof(ent_buf)-1)) { /* unrecognized entry format. skip it. */ continue; } - if (next-summary < 2) { + if (len < 1) { /* empty; skip it. */ + /* XXX This happens to be unreachable, since if len==0, then *summary is + * ',' or '\0', and the TOR_ISDIGIT test above would have failed. */ continue; } - memcpy(ent_buf, summary, next-summary-1); - ent_buf[next-summary-1] = '\0'; + memcpy(ent_buf, summary, len); + ent_buf[len] = '\0'; if (tor_sscanf(ent_buf, "%u-%u%c", &low, &high, &dummy) == 2) { - if (low<1 || low>65535 || high<1 || high>65535) { + if (low<1 || low>65535 || high<1 || high>65535 || low>high) { log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Found bad entry in policy summary %s", escaped(orig_summary)); return NULL; @@ -1415,7 +1419,7 @@ parse_short_policy(const char *summary) /** Write policy back out into a string. Used only for unit tests * currently. */ -const char * +char * write_short_policy(const short_policy_t *policy) { int i; @@ -1424,7 +1428,7 @@ write_short_policy(const short_policy_t *policy) smartlist_add_asprintf(sl, "%s", policy->is_accept ? "accept " : "reject "); - for(i=0; i < policy->n_entries; i++) { + for (i=0; i < policy->n_entries; i++) { const short_policy_entry_t *e = &policy->entries[i]; if (e->min_port == e->max_port) { smartlist_add_asprintf(sl, "%d", e->min_port); diff --git a/src/or/policies.h b/src/or/policies.h index b385d8e..f00d829 100644 --- a/src/or/policies.h +++ b/src/or/policies.h @@ -61,7 +61,7 @@ void policies_free_all(void); char *policy_summarize(smartlist_t *policy); short_policy_t *parse_short_policy(const char *summary); -const char *write_short_policy(const short_policy_t *policy); +char *write_short_policy(const short_policy_t *policy); void short_policy_free(short_policy_t *policy); int short_policy_is_reject_star(const short_policy_t *policy); addr_policy_result_t compare_tor_addr_to_short_policy( diff --git a/src/test/test.c b/src/test/test.c index 9b510d2..ddfd633 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1004,6 +1004,28 @@ test_circuit_timeout(void) return; } +/* Helper: assert that short_policy parses and writes back out as itself, + or as expected if that's provided. */ +static void +test_short_policy_parse(const char *input, + const char *expected) +{ + short_policy_t *short_policy = NULL; + char *out = NULL; + + if (expected == NULL) + expected = input; + + short_policy = parse_short_policy(input); + tt_assert(short_policy); + out = write_short_policy(short_policy); + tt_str_op(out, ==, expected); + + done: + tor_free(out); + short_policy_free(short_policy); +} + /** Helper: Parse the exit policy string in policy_str, and make sure * that policies_summarize() produces the string expected_summary from * it. */ @@ -1014,7 +1036,7 @@ test_policy_summary_helper(const char *policy_str, config_line_t line; smartlist_t *policy = smartlist_new(); char *summary = NULL; - const char *summary_after; + char *summary_after = NULL; int r; short_policy_t *short_policy = NULL; @@ -1231,6 +1253,46 @@ test_policies(void) "accept *:*", "reject 1,3,5,7"); + /* Short policies with unrecognized formats should get accepted. */ + test_short_policy_parse("accept fred,2,3-5", "accept 2,3-5"); + test_short_policy_parse("accept 2,fred,3", "accept 2,3"); + test_short_policy_parse("accept 2,fred,3,bob", "accept 2,3"); + test_short_policy_parse("accept 2,-3,500-600", "accept 2,500-600"); + /* Short policies with nil entries are accepted too. */ + test_short_policy_parse("accept 1,,3", "accept 1,3"); + test_short_policy_parse("accept 100-200,,", "accept 100-200"); + test_short_policy_parse("reject ,1-10,,,,30-40", "reject 1-10,30-40"); + + /* Try parsing various broken short policies */ + tt_ptr_op(NULL, ==, parse_short_policy("accept 200-199")); + tt_ptr_op(NULL, ==, parse_short_policy("")); + tt_ptr_op(NULL, ==, parse_short_policy("rejekt 1,2,3")); + tt_ptr_op(NULL, ==, parse_short_policy("reject ")); + tt_ptr_op(NULL, ==, parse_short_policy("reject")); + tt_ptr_op(NULL, ==, parse_short_policy("rej")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 2,3,100000")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 2,3x,4")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 2,3x,4")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 2-")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 2-x")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 1-,3")); + tt_ptr_op(NULL, ==, parse_short_policy("accept 1-,3")); + /* Test a too-long policy. */ + { + int i; + char *policy = NULL; + smartlist_t *chunks = smartlist_new(); + smartlist_add(chunks, tor_strdup("accept ")); + for (i=1; i<10000; ++i) + smartlist_add_asprintf(chunks, "%d,", i); + smartlist_add(chunks, tor_strdup("20000")); + policy = smartlist_join_strings(chunks, "", 0, NULL); + SMARTLIST_FOREACH(chunks, char *, ch, tor_free(ch)); + smartlist_free(chunks); + tt_ptr_op(NULL, ==, parse_short_policy(policy));/* shouldn't be accepted */ + tor_free(policy); /* could leak. */ + } + /* truncation ports */ sm = smartlist_new(); for (i=1; i<2000; i+=2) {

1 0

[tor/release-0.2.3] Let 0.2.3 clients exit to internal addresses if they want
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 2ecee3fce2179c6fe9f9c748522e417b887ee021 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Oct 22 15:45:39 2012 -0400 Let 0.2.3 clients exit to internal addresses if they want Clients now consider the ClientRejectInternalAddresses config option when using a microdescriptor consensus stanza to decide whether an exit relay would allow exiting to an internal address. Fixes bug 7190; bugfix on 0.2.3.1-alpha. --- changes/bug7190 | 6 ++++++ src/or/policies.c | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/changes/bug7190 b/changes/bug7190 new file mode 100644 index 0000000..1607f79 --- /dev/null +++ b/changes/bug7190 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Clients now consider the ClientRejectInternalAddresses config option + when using a microdescriptor consensus stanza to decide whether + an exit relay would allow exiting to an internal address. Fixes + bug 7190; bugfix on 0.2.3.1-alpha. + diff --git a/src/or/policies.c b/src/or/policies.c index bbd6816..81e4809 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -1462,15 +1462,14 @@ compare_tor_addr_to_short_policy(const tor_addr_t *addr, uint16_t port, int i; int found_match = 0; int accept; - (void)addr; tor_assert(port != 0); if (addr && tor_addr_is_null(addr)) addr = NULL; /* Unspec means 'no address at all,' in this context. */ - if (addr && (tor_addr_is_internal(addr, 0) || - tor_addr_is_loopback(addr))) + if (addr && get_options()->ClientRejectInternalAddresses && + (tor_addr_is_internal(addr, 0) || tor_addr_is_loopback(addr))) return ADDR_POLICY_REJECTED; for (i=0; i < policy->n_entries; ++i) {

1 0

[tor/release-0.2.3] Merge branch 'maint-0.2.3' into release-0.2.3
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 36801ba3c0c6a08e138cb09ecd7720a0ff711483 Merge: 3a2b86e 2ecee3f Author: Roger Dingledine <arma(a)torproject.org> Date: Tue Oct 23 17:26:21 2012 -0400 Merge branch 'maint-0.2.3' into release-0.2.3 changes/bug7190 | 6 +++++ changes/bug7192 | 10 ++++++++ src/or/policies.c | 46 ++++++++++++++++++++++++++++++------ src/or/policies.h | 1 + src/test/test.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 8 deletions(-)

1 0

[tor/master] Let 0.2.3 clients exit to internal addresses if they want
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 2ecee3fce2179c6fe9f9c748522e417b887ee021 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Oct 22 15:45:39 2012 -0400 Let 0.2.3 clients exit to internal addresses if they want Clients now consider the ClientRejectInternalAddresses config option when using a microdescriptor consensus stanza to decide whether an exit relay would allow exiting to an internal address. Fixes bug 7190; bugfix on 0.2.3.1-alpha. --- changes/bug7190 | 6 ++++++ src/or/policies.c | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/changes/bug7190 b/changes/bug7190 new file mode 100644 index 0000000..1607f79 --- /dev/null +++ b/changes/bug7190 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Clients now consider the ClientRejectInternalAddresses config option + when using a microdescriptor consensus stanza to decide whether + an exit relay would allow exiting to an internal address. Fixes + bug 7190; bugfix on 0.2.3.1-alpha. + diff --git a/src/or/policies.c b/src/or/policies.c index bbd6816..81e4809 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -1462,15 +1462,14 @@ compare_tor_addr_to_short_policy(const tor_addr_t *addr, uint16_t port, int i; int found_match = 0; int accept; - (void)addr; tor_assert(port != 0); if (addr && tor_addr_is_null(addr)) addr = NULL; /* Unspec means 'no address at all,' in this context. */ - if (addr && (tor_addr_is_internal(addr, 0) || - tor_addr_is_loopback(addr))) + if (addr && get_options()->ClientRejectInternalAddresses && + (tor_addr_is_internal(addr, 0) || tor_addr_is_loopback(addr))) return ADDR_POLICY_REJECTED; for (i=0; i < policy->n_entries; ++i) {

1 0

[tor/master] Merge branch 'maint-0.2.3'
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 4c06a804d946de238e2d6a0b1a7dd655b0d82ccc Merge: 0712d60 2ecee3f Author: Roger Dingledine <arma(a)torproject.org> Date: Tue Oct 23 17:26:07 2012 -0400 Merge branch 'maint-0.2.3' changes/bug7190 | 6 ++++++ src/or/policies.c | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-)

1 0

[tor/master] Add changes file for ticket 6876 missing from 0.2.4.4-alpha ChangeLog.
by nickm＠torproject.org 23 Oct '12

23 Oct '12

commit 478644e905462a9a5d0567e77ee93af6334feb50 Author: Linus Nordberg <linus(a)torproject.org> Date: Fri Oct 19 23:23:04 2012 +0200 Add changes file for ticket 6876 missing from 0.2.4.4-alpha ChangeLog. --- changes/enh6876 | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/changes/enh6876 b/changes/enh6876 new file mode 100644 index 0000000..9019fa4 --- /dev/null +++ b/changes/enh6876 @@ -0,0 +1,3 @@ + o Minor features: + - Option OutboundBindAddress can be specified multiple times and + accepts IPv6 addresses. Resolves ticket 6876.

1 0

[tor/master] Merge remote-tracking branch 'linus/enh6876_changes_file'
by nickm＠torproject.org 23 Oct '12

23 Oct '12

commit f80c992d7f6b0633596efb70c5281595090b4361 Merge: e17fd57 478644e Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 23 17:22:37 2012 -0400 Merge remote-tracking branch 'linus/enh6876_changes_file' changes/enh6876 | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)

1 0

[tor/master] Roll enh6876 into Changelog for 0.2.4.4 alpha, which had the change but not the entry.
by nickm＠torproject.org 23 Oct '12

23 Oct '12

commit 0712d606c6c3111f939d07f48c9c361dd116f553 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 23 17:23:48 2012 -0400 Roll enh6876 into Changelog for 0.2.4.4 alpha, which had the change but not the entry. --- ChangeLog | 2 ++ changes/enh6876 | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index d234807..f45ae9c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -52,6 +52,8 @@ Changes in version 0.2.4.4-alpha - 2012-10-20 place). Resolves ticket 6889. - Tor now builds correctly on Bitrig, an OpenBSD fork. Patch from dhill. Resolves ticket 6982. + - Option OutboundBindAddress can be specified multiple times and + accepts IPv6 addresses. Resolves ticket 6876. o Minor bugfixes (also in 0.2.3.23-rc): - Don't serve or accept v2 hidden service descriptors over a diff --git a/changes/enh6876 b/changes/enh6876 deleted file mode 100644 index 9019fa4..0000000 --- a/changes/enh6876 +++ /dev/null @@ -1,3 +0,0 @@ - o Minor features: - - Option OutboundBindAddress can be specified multiple times and - accepts IPv6 addresses. Resolves ticket 6876.

1 0

[tor/maint-0.2.3] Let 0.2.3 clients exit to internal addresses if they want
by arma＠torproject.org 23 Oct '12

23 Oct '12

commit 2ecee3fce2179c6fe9f9c748522e417b887ee021 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Oct 22 15:45:39 2012 -0400 Let 0.2.3 clients exit to internal addresses if they want Clients now consider the ClientRejectInternalAddresses config option when using a microdescriptor consensus stanza to decide whether an exit relay would allow exiting to an internal address. Fixes bug 7190; bugfix on 0.2.3.1-alpha. --- changes/bug7190 | 6 ++++++ src/or/policies.c | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/changes/bug7190 b/changes/bug7190 new file mode 100644 index 0000000..1607f79 --- /dev/null +++ b/changes/bug7190 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Clients now consider the ClientRejectInternalAddresses config option + when using a microdescriptor consensus stanza to decide whether + an exit relay would allow exiting to an internal address. Fixes + bug 7190; bugfix on 0.2.3.1-alpha. + diff --git a/src/or/policies.c b/src/or/policies.c index bbd6816..81e4809 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -1462,15 +1462,14 @@ compare_tor_addr_to_short_policy(const tor_addr_t *addr, uint16_t port, int i; int found_match = 0; int accept; - (void)addr; tor_assert(port != 0); if (addr && tor_addr_is_null(addr)) addr = NULL; /* Unspec means 'no address at all,' in this context. */ - if (addr && (tor_addr_is_internal(addr, 0) || - tor_addr_is_loopback(addr))) + if (addr && get_options()->ClientRejectInternalAddresses && + (tor_addr_is_internal(addr, 0) || tor_addr_is_loopback(addr))) return ADDR_POLICY_REJECTED; for (i=0; i < policy->n_entries; ++i) {

1 0