September 2012 - tor-commits - lists.torproject.org

[tor/release-0.2.3] Address Nick's comments from code review.
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 4e42a8a2f20e501f9236452b41939964e6d942cf Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Thu Aug 16 16:29:19 2012 -0700 Address Nick's comments from code review. Also promote log messages to notice and rate-limit them. --- changes/bug6475 | 6 + src/or/circuitbuild.c | 284 ++++++++++++++++++++++++++++++++++--------------- src/or/or.h | 13 ++- 3 files changed, 217 insertions(+), 86 deletions(-) diff --git a/changes/bug6475 b/changes/bug6475 new file mode 100644 index 0000000..67bab99 --- /dev/null +++ b/changes/bug6475 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Add internal circuit construction state to protect against + the noisy warn message "Unexpectedly high circuit_successes". + Also add some additional rate-limited notice messages to help + determine the root cause of the warn. Fixes bug 6475. + Bugfix against 0.2.3.17-beta. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 7ac2b97..2fa3dd5 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -135,6 +135,9 @@ static entry_guard_t *entry_guard_get_by_id_digest(const char *digest); static void bridge_free(bridge_info_t *bridge); +static int entry_guard_inc_first_hop_count(entry_guard_t *guard); +static void pathbias_count_success(origin_circuit_t *circ); + /** * This function decides if CBT learning should be disabled. It returns * true if one or more of the following four conditions are met: @@ -2285,44 +2288,11 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) } log_info(LD_CIRC,"circuit built!"); circuit_reset_failure_count(0); - /* Don't count cannibalized or onehop circs for path bias */ + if (circ->build_state->onehop_tunnel || circ->has_opened) { control_event_bootstrap(BOOTSTRAP_STATUS_REQUESTING_STATUS, 0); - } else { - entry_guard_t *guard = - entry_guard_get_by_id_digest(circ->_base.n_conn->identity_digest); - - if (guard) { - if (circ->path_state == PATH_STATE_DID_FIRST_HOP) { - circ->path_state = PATH_STATE_SUCCEEDED; - guard->circuit_successes++; - - log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s", - guard->circuit_successes, guard->first_hops, - guard->nickname, hex_str(guard->identity, DIGEST_LEN)); - } else { - log_info(LD_BUG, - "Succeeded circuit has strange path state %d. " - "Circuit is a %s currently %s.", - circ->path_state, - circuit_purpose_to_string(circ->_base.purpose), - circuit_state_to_string(circ->_base.state)); - } - - if (guard->first_hops < guard->circuit_successes) { - log_warn(LD_BUG, "Unexpectedly high circuit_successes (%u/%u) " - "for guard %s=%s", - guard->circuit_successes, guard->first_hops, - guard->nickname, hex_str(guard->identity, DIGEST_LEN)); - } - } else { - log_info(LD_BUG, - "Completed circuit has no known guard. " - "Circuit is a %s currently %s.", - circuit_purpose_to_string(circ->_base.purpose), - circuit_state_to_string(circ->_base.state)); - } } + if (!can_complete_circuit && !circ->build_state->onehop_tunnel) { const or_options_t *options = get_options(); can_complete_circuit=1; @@ -2338,6 +2308,8 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) consider_testing_reachability(1, 1); } } + + pathbias_count_success(circ); circuit_rep_hist_note_result(circ); circuit_has_opened(circ); /* do other actions as necessary */ @@ -2637,6 +2609,194 @@ pathbias_get_scale_factor(const or_options_t *options) DFLT_PATH_BIAS_SCALE_THRESHOLD, 1, INT32_MAX); } +const char * +pathbias_state_to_string(path_state_t state) +{ + switch(state) { + case PATH_STATE_NEW_CIRC: + return "new"; + case PATH_STATE_DID_FIRST_HOP: + return "first hop"; + case PATH_STATE_SUCCEEDED: + return "succeeded"; + } + + return "unknown"; +} + + +/** + * Check our circuit state to see if this is a successful first hop. + * If so, record it in the current guard's path bias first_hop count. + * + * Also check for several potential error cases for bug #6475. + */ +static int +pathbias_count_first_hop(origin_circuit_t *circ) +{ +#define FIRST_HOP_NOTICE_INTERVAL (600) + static ratelim_t first_hop_notice_limit = RATELIM_INIT(FIRST_HOP_NOTICE_INTERVAL); + char *rate_msg = NULL; + + /* Completely ignore one hop circuits */ + if (circ->build_state->onehop_tunnel) { + tor_assert(circ->build_state->desired_path_len == 1); + return 0; + } + + if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { + /* Help track down the real cause of bug #6475: */ + if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) { + if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "Opened circuit is in strange path state %s. " + "Circuit is a %s currently %s. %s", + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + + /* Don't count cannibalized circs for path bias */ + if (!circ->has_opened) { + entry_guard_t *guard; + + guard = entry_guard_get_by_id_digest( + circ->_base.n_conn->identity_digest); + if (guard) { + if (circ->path_state == PATH_STATE_NEW_CIRC) { + circ->path_state = PATH_STATE_DID_FIRST_HOP; + + if (entry_guard_inc_first_hop_count(guard) < 0) { + /* Bogus guard; we already warned. */ + return -END_CIRC_REASON_TORPROTOCOL; + } + } else { + if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "Unopened circuit has strange path state %s. " + "Circuit is a %s currently %s. %s", + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + } else { + if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "Unopened circuit has no known guard. " + "Circuit is a %s currently %s. %s", + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + } + } else { + /* Help track down the real cause of bug #6475: */ + if (circ->path_state == PATH_STATE_NEW_CIRC) { + if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "A %s circuit is in cpath state %d (opened: %d). " + "Circuit is a %s currently %s. %s", + pathbias_state_to_string(circ->path_state), + circ->cpath->state, circ->has_opened, + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + } + + return 0; +} + +/** + * Check our circuit state to see if this is a successful circuit + * completion. If so, record it in the current guard's path bias + * success count. + * + * Also check for several potential error cases for bug #6475. + */ +static void +pathbias_count_success(origin_circuit_t *circ) +{ +#define SUCCESS_NOTICE_INTERVAL (600) + static ratelim_t success_notice_limit = RATELIM_INIT(SUCCESS_NOTICE_INTERVAL); + char *rate_msg = NULL; + + /* Ignore one hop circuits */ + if (circ->build_state->onehop_tunnel) { + tor_assert(circ->build_state->desired_path_len == 1); + return; + } + + /* Don't count cannibalized/reused circs for path bias */ + if (!circ->has_opened) { + entry_guard_t *guard = + entry_guard_get_by_id_digest(circ->_base.n_conn->identity_digest); + + if (guard) { + if (circ->path_state == PATH_STATE_DID_FIRST_HOP) { + circ->path_state = PATH_STATE_SUCCEEDED; + guard->circuit_successes++; + + log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s", + guard->circuit_successes, guard->first_hops, + guard->nickname, hex_str(guard->identity, DIGEST_LEN)); + } else { + if ((rate_msg = rate_limit_log(&success_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "Succeeded circuit is in strange path state %s. " + "Circuit is a %s currently %s. %s", + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + + if (guard->first_hops < guard->circuit_successes) { + log_warn(LD_BUG, "Unexpectedly high circuit_successes (%u/%u) " + "for guard %s=%s", + guard->circuit_successes, guard->first_hops, + guard->nickname, hex_str(guard->identity, DIGEST_LEN)); + } + } else { + if ((rate_msg = rate_limit_log(&success_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "Completed circuit has no known guard. " + "Circuit is a %s currently %s. %s", + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + } else { + if (circ->path_state != PATH_STATE_SUCCEEDED) { + if ((rate_msg = rate_limit_log(&success_notice_limit, + approx_time()))) { + log_notice(LD_BUG, + "Opened circuit is in strange path state %s. " + "Circuit is a %s currently %s. %s", + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->_base.purpose), + circuit_state_to_string(circ->_base.state), + rate_msg); + } + } + } +} + + /** Increment the number of times we successfully extended a circuit to * 'guard', first checking if the failure rate is high enough that we should * eliminate the guard. Return -1 if the guard looks no good; return 0 if the @@ -2704,60 +2864,14 @@ circuit_finish_handshake(origin_circuit_t *circ, uint8_t reply_type, { char keys[CPATH_KEY_MATERIAL_LEN]; crypt_path_t *hop; + int rv; + + if((rv = pathbias_count_first_hop(circ)) < 0) + return rv; if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { hop = circ->cpath; - - /* Help track down the real cause of bug #6475: */ - if (circ->has_opened && circ->path_state == PATH_STATE_NEW_CIRC) { - log_info(LD_BUG, - "Opened circuit seems new. " - "Circuit is a %s currently %s.", - circuit_purpose_to_string(circ->_base.purpose), - circuit_state_to_string(circ->_base.state)); - } - - /* Don't count cannibalized or onehop circs for path bias */ - if (!circ->has_opened && !circ->build_state->onehop_tunnel) { - entry_guard_t *guard; - - guard = entry_guard_get_by_id_digest( - circ->_base.n_conn->identity_digest); - if (guard) { - if (circ->path_state == PATH_STATE_NEW_CIRC) { - circ->path_state = PATH_STATE_DID_FIRST_HOP; - - if (entry_guard_inc_first_hop_count(guard) < 0) { - /* Bogus guard; we already warned. */ - return -END_CIRC_REASON_TORPROTOCOL; - } - } else { - log_info(LD_BUG, - "Unopened circuit has strange path state %d. " - "Circuit is a %s currently %s.", - circ->path_state, - circuit_purpose_to_string(circ->_base.purpose), - circuit_state_to_string(circ->_base.state)); - } - } else { - log_info(LD_BUG, - "Opened circuit has no known guard. " - "Circuit is a %s currently %s.", - circuit_purpose_to_string(circ->_base.purpose), - circuit_state_to_string(circ->_base.state)); - } - } } else { - /* Help track down the real cause of bug #6475: */ - if (circ->path_state == PATH_STATE_NEW_CIRC) { - log_info(LD_BUG, - "New circuit is in cpath state %d (opened: %d). " - "Circuit is a %s currently %s.", - circ->cpath->state, circ->has_opened, - circuit_purpose_to_string(circ->_base.purpose), - circuit_state_to_string(circ->_base.state)); - } - hop = onion_next_hop_in_cpath(circ->cpath); if (!hop) { /* got an extended when we're all done? */ log_warn(LD_PROTOCOL,"got extended when circ already built? Closing."); diff --git a/src/or/or.h b/src/or/or.h index 0db6137..d2cafb5 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2596,9 +2596,20 @@ typedef struct circuit_t { * circuit. */ #define MAX_RELAY_EARLY_CELLS_PER_CIRCUIT 8 +/** + * Describes the circuit building process in simplified terms based + * on the path bias accounting state for a circuit. Created to prevent + * overcounting due to unknown cases of circuit reuse. See Bug #6475. + */ typedef enum { + /** This circuit is "new". It has not yet completed a first hop + * or been counted by the path bias code. */ PATH_STATE_NEW_CIRC = 0, + /** This circuit has completed a first hop, and has been counted by + * the path bias logic. */ PATH_STATE_DID_FIRST_HOP = 1, + /** This circuit has been completely built, and has been counted as + * successful by the path bias logic. */ PATH_STATE_SUCCEEDED = 2, } path_state_t; @@ -2637,7 +2648,7 @@ typedef struct origin_circuit_t { /** Kludge to help us prevent the warn in bug #6475 and eventually * debug why we are not seeing first hops in some cases. */ - path_state_t path_state; + path_state_t path_state : 2; /** Set iff this is a hidden-service circuit which has timed out * according to our current circuit-build timeout, but which has

1 0

[tor/release-0.2.3] Merge remote-tracking branch 'public/bug6514' into maint-0.2.3
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit f25e8d034b78be01c9fbe659d4686fc370083572 Merge: 91b52a2 2d6d5db Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 11:53:06 2012 -0400 Merge remote-tracking branch 'public/bug6514' into maint-0.2.3 changes/bug6514 | 5 +++++ src/or/dirvote.c | 1 + 2 files changed, 6 insertions(+), 0 deletions(-)

1 0

[tor/release-0.2.3] Merge remote-tracking branch 'mikeperry/bug6475' into maint-0.2.3
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 3621f30ad44973acef1ae0dc2e723fd8c9f1dc0d Merge: f25e8d0 4e42a8a Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 12:08:42 2012 -0400 Merge remote-tracking branch 'mikeperry/bug6475' into maint-0.2.3 changes/bug6475 | 6 ++ src/or/circuitbuild.c | 236 ++++++++++++++++++++++++++++++++++++++++++------- src/or/or.h | 21 +++++ 3 files changed, 229 insertions(+), 34 deletions(-)

1 0

[tor/release-0.2.3] Merge branch 'bug6379' into maint-0.2.3
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 97602c9de432cc1d49d4441de858ab73b8bf41c0 Merge: e9172e5 4c8fcba Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 13:57:07 2012 -0400 Merge branch 'bug6379' into maint-0.2.3 changes/bug6379 | 6 +++++ configure.in | 1 + src/common/OpenBSD_malloc_Linux.c | 40 ++++++++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 9 deletions(-)

1 0

[tor/release-0.2.3] Merge remote-tracking branch 'public/bug6244_part_c' into maint-0.2.3
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit e9172e51fb13cfe5d4c70a98809a45e05b528482 Merge: 676f710 2ba52f4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 12:37:49 2012 -0400 Merge remote-tracking branch 'public/bug6244_part_c' into maint-0.2.3 changes/bug6244_part_c | 6 ++++ src/or/config.c | 61 ++++++++++++++++++++++++++++++----------------- src/or/config.h | 6 ++++ src/or/control.c | 15 +++++++++-- 4 files changed, 63 insertions(+), 25 deletions(-)

1 0

[tor/release-0.2.3] Merge remote-tracking branch 'public/bug6507' into maint-0.2.3
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 676f71054f11727fdfa8707979d8c6fe143c9333 Merge: a74d418 d993b04 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 12:33:17 2012 -0400 Merge remote-tracking branch 'public/bug6507' into maint-0.2.3 changes/bug6507 | 15 +++++ src/or/config.c | 161 ++++++++++++++++++++++++++++++++++---------------- src/or/directory.c | 2 +- src/or/directory.h | 2 +- src/or/dirserv.c | 12 ++-- src/or/or.h | 35 +++++++++-- src/or/router.c | 5 +- src/or/routerparse.c | 2 +- 8 files changed, 164 insertions(+), 70 deletions(-)

1 0

[tor/release-0.2.3] Whitespace and build fixes on 6475 patch
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit a74d4182f106b1b89367cf1a640b1140d3d35d2c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 12:10:31 2012 -0400 Whitespace and build fixes on 6475 patch --- src/or/circuitbuild.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 2fa3dd5..39a223b 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2609,14 +2609,14 @@ pathbias_get_scale_factor(const or_options_t *options) DFLT_PATH_BIAS_SCALE_THRESHOLD, 1, INT32_MAX); } -const char * +static const char * pathbias_state_to_string(path_state_t state) { - switch(state) { + switch (state) { case PATH_STATE_NEW_CIRC: - return "new"; + return "new"; case PATH_STATE_DID_FIRST_HOP: - return "first hop"; + return "first hop"; case PATH_STATE_SUCCEEDED: return "succeeded"; } @@ -2624,7 +2624,6 @@ pathbias_state_to_string(path_state_t state) return "unknown"; } - /** * Check our circuit state to see if this is a successful first hop. * If so, record it in the current guard's path bias first_hop count. @@ -2635,7 +2634,8 @@ static int pathbias_count_first_hop(origin_circuit_t *circ) { #define FIRST_HOP_NOTICE_INTERVAL (600) - static ratelim_t first_hop_notice_limit = RATELIM_INIT(FIRST_HOP_NOTICE_INTERVAL); + static ratelim_t first_hop_notice_limit = + RATELIM_INIT(FIRST_HOP_NOTICE_INTERVAL); char *rate_msg = NULL; /* Completely ignore one hop circuits */ @@ -2706,7 +2706,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) "A %s circuit is in cpath state %d (opened: %d). " "Circuit is a %s currently %s. %s", pathbias_state_to_string(circ->path_state), - circ->cpath->state, circ->has_opened, + circ->cpath->state, circ->has_opened, circuit_purpose_to_string(circ->_base.purpose), circuit_state_to_string(circ->_base.state), rate_msg); @@ -2728,7 +2728,8 @@ static void pathbias_count_success(origin_circuit_t *circ) { #define SUCCESS_NOTICE_INTERVAL (600) - static ratelim_t success_notice_limit = RATELIM_INIT(SUCCESS_NOTICE_INTERVAL); + static ratelim_t success_notice_limit = + RATELIM_INIT(SUCCESS_NOTICE_INTERVAL); char *rate_msg = NULL; /* Ignore one hop circuits */ @@ -2796,7 +2797,6 @@ pathbias_count_success(origin_circuit_t *circ) } } - /** Increment the number of times we successfully extended a circuit to * 'guard', first checking if the failure rate is high enough that we should * eliminate the guard. Return -1 if the guard looks no good; return 0 if the @@ -2866,7 +2866,7 @@ circuit_finish_handshake(origin_circuit_t *circ, uint8_t reply_type, crypt_path_t *hop; int rv; - if((rv = pathbias_count_first_hop(circ)) < 0) + if ((rv = pathbias_count_first_hop(circ)) < 0) return rv; if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) {

1 0

[tor/release-0.2.3] Consider IPv6 OR ports when deciding whether a routerinfo change is cosmetic.
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 9ed87b37d04705983b11cd995140e8864454bf5e Author: Linus Nordberg <linus(a)torproject.org> Date: Thu Jul 19 20:41:23 2012 +0200 Consider IPv6 OR ports when deciding whether a routerinfo change is cosmetic. Closes #6423. --- changes/bug6423 | 3 +++ src/or/routerlist.c | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/changes/bug6423 b/changes/bug6423 new file mode 100644 index 0000000..2ea4f14 --- /dev/null +++ b/changes/bug6423 @@ -0,0 +1,3 @@ + o Minor features: + - Consider new, removed or changed IPv6 OR ports a non cosmetic + change. diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 4979b93..3c39e36 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5160,6 +5160,8 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2) if (strcasecmp(r1->address, r2->address) || strcasecmp(r1->nickname, r2->nickname) || r1->or_port != r2->or_port || + !tor_addr_eq(&r1->ipv6_addr, &r2->ipv6_addr) || + r1->ipv6_orport != r2->ipv6_orport || r1->dir_port != r2->dir_port || r1->purpose != r2->purpose || crypto_pk_cmp_keys(r1->onion_pkey, r2->onion_pkey) ||

1 0

[tor/release-0.2.3] Fix more warnings from openbsd_malloc
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 4c8fcba86c35bd8e2cfde4c65f394c7837638ff0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 17 13:49:52 2012 -0400 Fix more warnings from openbsd_malloc Apparently, (void)writev is not enough to suppress the "you are ignoring the return value!" warnings on Linux. Instead, remove the whole warning/error logic when compiling openbsd_malloc for Tor: we can't use it. --- src/common/OpenBSD_malloc_Linux.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/common/OpenBSD_malloc_Linux.c b/src/common/OpenBSD_malloc_Linux.c index 39c8f8b..da82729 100644 --- a/src/common/OpenBSD_malloc_Linux.c +++ b/src/common/OpenBSD_malloc_Linux.c @@ -450,6 +450,7 @@ extern char *__progname; static void wrterror(const char *p) { +#ifndef BUILDING_FOR_TOR const char *q = " error: "; struct iovec iov[5]; @@ -463,8 +464,10 @@ wrterror(const char *p) iov[3].iov_len = strlen(p); iov[4].iov_base = (char*)"\n"; iov[4].iov_len = 1; - (void) writev(STDERR_FILENO, iov, 5); - + writev(STDERR_FILENO, iov, 5); +#else + (void)p; +#endif suicide = 1; #ifdef MALLOC_STATS if (malloc_stats) @@ -478,14 +481,17 @@ wrterror(const char *p) static void wrtwarning(const char *p) { +#ifndef BUILDING_FOR_TOR const char *q = " warning: "; struct iovec iov[5]; +#endif if (malloc_abort) wrterror(p); else if (malloc_silent) return; +#ifndef BUILDING_FOR_TOR iov[0].iov_base = __progname; iov[0].iov_len = strlen(__progname); iov[1].iov_base = (char*)malloc_func; @@ -498,6 +504,9 @@ wrtwarning(const char *p) iov[4].iov_len = 1; (void) writev(STDERR_FILENO, iov, 5); +#else + (void)p; +#endif } #ifdef MALLOC_STATS

1 0

[tor/release-0.2.3] Do not assert when comparing a null address/port against a policy
by arma＠torproject.org 04 Sep '12

04 Sep '12

commit 62d96284f7e0f81c40d5df7e53dd7b4dfe7e56a5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Aug 27 11:52:51 2012 -0400 Do not assert when comparing a null address/port against a policy This can create a remote crash opportunity for/against directory authorities. --- changes/bug6690 | 7 +++++++ src/or/policies.c | 6 +++++- 2 files changed, 12 insertions(+), 1 deletions(-) diff --git a/changes/bug6690 b/changes/bug6690 new file mode 100644 index 0000000..99d4297 --- /dev/null +++ b/changes/bug6690 @@ -0,0 +1,7 @@ + o Major bugfixes (security): + - Do not crash when comparing an address with port value 0 to an + address policy. This bug could have been used to cause a remote + assertion failure by or against directory authorities, or to + allow some applications to crash clients. Fixes bug 6690; bugfix + on 0.2.1.10-alpha. + diff --git a/src/or/policies.c b/src/or/policies.c index c870360..55d08af 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -685,7 +685,11 @@ compare_tor_addr_to_addr_policy(const tor_addr_t *addr, uint16_t port, /* no policy? accept all. */ return ADDR_POLICY_ACCEPTED; } else if (tor_addr_is_null(addr)) { - tor_assert(port != 0); + if (port == 0) { + log_info(LD_BUG, "Rejecting null address with 0 port (family %d)", + addr ? tor_addr_family(addr) : -1); + return ADDR_POLICY_REJECTED; + } return compare_unknown_tor_addr_to_addr_policy(port, policy); } else if (port == 0) { return compare_known_tor_addr_to_addr_policy_noport(addr, policy);

1 0