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

[tor/master] Have tor_parse_*long functions check for negative bases
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 56e0959d2ab9cae45d39c5b9d72b1bb1b8ad03b7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 30 10:34:05 2012 -0400 Have tor_parse_*long functions check for negative bases One of our unit tests checks that they behave correctly (giving an error) when the base is negative. But there isn't a guarantee that strtol and friends actually handle negative bases correctly. Found by Coverity Scan; fix for CID 504. --- src/common/util.c | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 1807ee7..266368c 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -906,6 +906,11 @@ tor_parse_long(const char *s, int base, long min, long max, char *endptr; long r; + if (base < 0) { + if (ok) + *ok = 0; + return 0; + } r = strtol(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -918,6 +923,11 @@ tor_parse_ulong(const char *s, int base, unsigned long min, char *endptr; unsigned long r; + if (base < 0) { + if (ok) + *ok = 0; + return 0; + } r = strtoul(s, &endptr, base); CHECK_STRTOX_RESULT(); } @@ -942,6 +952,12 @@ tor_parse_uint64(const char *s, int base, uint64_t min, char *endptr; uint64_t r; + if (base < 0) { + if (ok) + *ok = 0; + return 0; + } + #ifdef HAVE_STRTOULL r = (uint64_t)strtoull(s, &endptr, base); #elif defined(_WIN32)

1 0

[tor/master] Fix a couple of "unused assigned value" warnings in parse_config tests
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 88caa552cc3742189124c297e7cf65ac5e5ef332 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 30 10:26:34 2012 -0400 Fix a couple of "unused assigned value" warnings in parse_config tests Coverity doesn't like the fact that we were storing the value of parse_config_line_from_str() but not checking it in a couple of cases. Fixes CID 505 and 506. --- src/test/test_util.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 0f9d1a7..88f00e0 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -388,6 +388,8 @@ test_util_config_line_comment_character(void) test_streq(v, "some value"); tor_free(k); tor_free(v); + test_streq(str, "k3 /home/user/myTorNetwork#2\n"); + #if 0 str = parse_config_line_from_str(str, &k, &v); test_streq(k, "k3"); @@ -499,6 +501,7 @@ test_util_config_line_escaped_content(void) test_streq(k, "Mix"); test_streq(v, "This is a \"star\":\t'*'\nAnd second line"); tor_free(k); tor_free(v); + test_streq(str, ""); str = buf2;

1 0

[tor/master] simplify further
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 5cb82e44d185fff4586ca2b92039e5686b9e753b Author: Roger Dingledine <arma(a)torproject.org> Date: Thu Mar 29 16:37:50 2012 -0400 simplify further --- src/or/connection_or.c | 7 ++----- src/or/dirserv.c | 9 ++------- src/or/dirserv.h | 3 +-- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 30d92b2..0d3f9d8 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1453,7 +1453,6 @@ int connection_or_client_learned_peer_id(or_connection_t *conn, const uint8_t *peer_id) { - int as_expected = 1; const or_options_t *options = get_options(); int severity = server_mode(options) ? LOG_PROTOCOL_WARN : LOG_WARN; @@ -1492,14 +1491,12 @@ connection_or_client_learned_peer_id(or_connection_t *conn, control_event_bootstrap_problem( "Unexpected identity in router certificate", END_OR_CONN_REASON_OR_IDENTITY); - as_expected = 0; + return -1; } if (authdir_mode_tests_reachability(options)) { dirserv_orconn_tls_done(conn->_base.address, conn->_base.port, - (const char*)peer_id, as_expected); + (const char*)peer_id); } - if (!as_expected) - return -1; return 0; } diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 01a0833..467129c 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3264,14 +3264,12 @@ dirserv_get_routerdescs(smartlist_t *descs_out, const char *key, * router listening at address:or_port, and has yielded * a certificate with digest digest_rcvd. * - * If as_advertised is 1, then inform the reachability checker that we - * could get to this guy. + * Inform the reachability checker that we could get to this guy. */ void dirserv_orconn_tls_done(const char *address, uint16_t or_port, - const char *digest_rcvd, - int as_advertised) + const char *digest_rcvd) { routerinfo_t *ri = NULL; time_t now = time(NULL); @@ -3282,9 +3280,6 @@ dirserv_orconn_tls_done(const char *address, * addr/port but with nonmatching keys, but instead of dumping, we should * skip testing. */ - if (!as_advertised) - return; - ri = router_get_mutable_by_digest(digest_rcvd); if (ri == NULL) return; diff --git a/src/or/dirserv.h b/src/or/dirserv.h index 6a86b94..fc48e48 100644 --- a/src/or/dirserv.h +++ b/src/or/dirserv.h @@ -108,8 +108,7 @@ int dirserv_get_routerdescs(smartlist_t *descs_out, const char *key, const char **msg); void dirserv_orconn_tls_done(const char *address, uint16_t or_port, - const char *digest_rcvd, - int as_advertised); + const char *digest_rcvd); int dirserv_should_launch_reachability_test(const routerinfo_t *ri, const routerinfo_t *ri_old); void dirserv_single_reachability_test(time_t now, routerinfo_t *router);

1 0

[tor/master] Refactor dirserv_orconn_tls_done().
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 20eb38a588313bba2fd35c32dc0e1c63ef4b8497 Author: Linus Nordberg <linus(a)nordberg.se> Date: Tue Mar 27 15:00:34 2012 +0200 Refactor dirserv_orconn_tls_done(). Look up the router using the digest instead of looping over all routers. --- src/or/dirserv.c | 55 ++++++++++++++++++++++++++++------------------------- 1 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index a7bbe98..01a0833 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3264,8 +3264,8 @@ dirserv_get_routerdescs(smartlist_t *descs_out, const char *key, * router listening at address:or_port, and has yielded * a certificate with digest digest_rcvd. * - * Also, if as_advertised is 1, then inform the reachability checker - * that we could get to this guy. + * If as_advertised is 1, then inform the reachability checker that we + * could get to this guy. */ void dirserv_orconn_tls_done(const char *address, @@ -3273,37 +3273,40 @@ dirserv_orconn_tls_done(const char *address, const char *digest_rcvd, int as_advertised) { - routerlist_t *rl = router_get_routerlist(); + routerinfo_t *ri = NULL; time_t now = time(NULL); - int bridge_auth = authdir_mode_bridge(get_options()); tor_assert(address); tor_assert(digest_rcvd); - /* XXX023 Doing a loop like this is stupid. We should just look up the - * router by digest_rcvd, and see if address, orport, and as_advertised - * match up. -NM */ - SMARTLIST_FOREACH_BEGIN(rl->routers, routerinfo_t *, ri) { - if (!strcasecmp(address, ri->address) && or_port == ri->or_port && - as_advertised && - fast_memeq(ri->cache_info.identity_digest, digest_rcvd, DIGEST_LEN)) { - /* correct digest. mark this router reachable! */ - if (!bridge_auth || ri->purpose == ROUTER_PURPOSE_BRIDGE) { - tor_addr_t addr, *addrp=NULL; - log_info(LD_DIRSERV, "Found router %s to be reachable at %s:%d. Yay.", - router_describe(ri), - address, ri->or_port); - if (tor_addr_parse(&addr, ri->address) != -1) - addrp = &addr; - else - log_warn(LD_BUG, "Couldn't parse IP address \"%s\"", ri->address); - rep_hist_note_router_reachable(digest_rcvd, addrp, or_port, now); - ri->last_reachable = now; - } - } - } SMARTLIST_FOREACH_END(ri); /* FFFF Maybe we should reinstate the code that dumps routers with the same * addr/port but with nonmatching keys, but instead of dumping, we should * skip testing. */ + + if (!as_advertised) + return; + + ri = router_get_mutable_by_digest(digest_rcvd); + if (ri == NULL) + return; + + if (!strcasecmp(address, ri->address) && or_port == ri->or_port) { + /* Found the right router. */ + if (!authdir_mode_bridge(get_options()) || + ri->purpose == ROUTER_PURPOSE_BRIDGE) { + /* This is a bridge or we're not a bridge authorititative -- + mark it as reachable. */ + tor_addr_t addr, *addrp=NULL; + log_info(LD_DIRSERV, "Found router %s to be reachable at %s:%d. Yay.", + router_describe(ri), + address, ri->or_port); + if (tor_addr_parse(&addr, ri->address) != -1) + addrp = &addr; + else + log_warn(LD_BUG, "Couldn't parse IP address \"%s\"", ri->address); + rep_hist_note_router_reachable(digest_rcvd, addrp, or_port, now); + ri->last_reachable = now; + } + } } /** Called when we, as an authority, receive a new router descriptor either as

1 0

[tor/master] Merge branch 'bug5527'
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 1da5223e893fd61ed171782809ed9a55eb3503c7 Merge: 545cb5f 491ffa5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 30 10:15:35 2012 -0400 Merge branch 'bug5527' src/or/connection_or.c | 7 +---- src/or/dirserv.c | 55 +++++++++++++++++++++-------------------------- src/or/dirserv.h | 3 +- 3 files changed, 28 insertions(+), 37 deletions(-)

1 0

[tor/master] checking "same addr/port but with nonmatching keys" is obsolete
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 3031def7268ce0cc6f3ab015c0df382295494bb9 Author: Roger Dingledine <arma(a)torproject.org> Date: Thu Mar 29 16:45:25 2012 -0400 checking "same addr/port but with nonmatching keys" is obsolete Specifically, I believe it dates back to when extend cells had address:port but no digest in them. The special edge case is certainly not worth the complexity these days. --- src/or/dirserv.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 467129c..dfecbc8 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3271,16 +3271,11 @@ dirserv_orconn_tls_done(const char *address, uint16_t or_port, const char *digest_rcvd) { - routerinfo_t *ri = NULL; + routerinfo_t *ri = router_get_mutable_by_digest(digest_rcvd); time_t now = time(NULL); tor_assert(address); tor_assert(digest_rcvd); - /* FFFF Maybe we should reinstate the code that dumps routers with the same - * addr/port but with nonmatching keys, but instead of dumping, we should - * skip testing. */ - - ri = router_get_mutable_by_digest(digest_rcvd); if (ri == NULL) return;

1 0

[tor/master] Move router lookup to _after_ we assert that its argument is set
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 491ffa540fda6a9cd8d75aad7d20945a3ae1df65 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 30 10:14:31 2012 -0400 Move router lookup to _after_ we assert that its argument is set A previous commit in the 5527 branch had moved router_get_mutable_by_digest(digest_rcvd) to happen before we did tor_assert(digest_rcvd), which would have defeated the purpose of the assert. --- src/or/dirserv.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/or/dirserv.c b/src/or/dirserv.c index dfecbc8..11f235c 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3271,11 +3271,13 @@ dirserv_orconn_tls_done(const char *address, uint16_t or_port, const char *digest_rcvd) { - routerinfo_t *ri = router_get_mutable_by_digest(digest_rcvd); + routerinfo_t *ri; time_t now = time(NULL); tor_assert(address); tor_assert(digest_rcvd); + ri = router_get_mutable_by_digest(digest_rcvd); + if (ri == NULL) return;

1 0

[tor/master] Merge remote-tracking branch 'linus/empty_desc_stats'
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 545cb5f34ea5eba4209650833634611160ab7389 Merge: f348daa d2cf90d Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 30 10:06:21 2012 -0400 Merge remote-tracking branch 'linus/empty_desc_stats' changes/bug5508 | 4 ++++ src/or/rephist.c | 2 ++ 2 files changed, 6 insertions(+), 0 deletions(-)

1 0

[tor/master] Add changes file.
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit d2cf90dc8886aafc1ea16220324473697682c94f Author: Linus Nordberg <linus(a)nordberg.se> Date: Tue Mar 27 15:00:34 2012 +0200 Add changes file. --- changes/bug5508 | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/changes/bug5508 b/changes/bug5508 new file mode 100644 index 0000000..6606f82 --- /dev/null +++ b/changes/bug5508 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - Fix a bug where a bridge authority crashes (on a failed assert) + if it has seen no directory requests when it's time to write + statistics to disk.

1 0

[tor/master] Don't try to generate stats from an empty served_descs.
by nickm＠torproject.org 30 Mar '12

30 Mar '12

commit 1b6f6bfda59989389983602c57ef0028dd736908 Author: Linus Nordberg <linus(a)nordberg.se> Date: Tue Mar 27 15:00:34 2012 +0200 Don't try to generate stats from an empty served_descs. --- src/or/rephist.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/or/rephist.c b/src/or/rephist.c index 3d5000c..0cd60ee 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -2657,6 +2657,8 @@ rep_hist_format_desc_stats(time_t now) return NULL; size = digestmap_size(served_descs); + if (size == 0) + return NULL; vals = tor_malloc(size * sizeof(int)); for (iter = digestmap_iter_init(served_descs); !digestmap_iter_done(iter);

1 0