From bfebc942eaf7098e8d35f1de9159da450d5e3969 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 11 Apr 2011 02:26:07 -0700 Subject: [PATCH 1/7] Correct the warning emitted when rejecting an oversized HS desc --- changes/bug2750 | 6 ++++++ src/or/routerparse.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changes/bug2750 diff --git a/changes/bug2750 b/changes/bug2750 new file mode 100644 index 0000000000..4371a0a4ee --- /dev/null +++ b/changes/bug2750 @@ -0,0 +1,6 @@ + o Minor bugfixes + - Correct the warning displayed when a rendezvous descriptor exceeds + the maximum size. Fixes bug 2750; bugfix on 0.2.1.5-alpha. Found + by John Brooks. + + diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 1faa177edf..28ecaf0ad7 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3629,7 +3629,7 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out, /* Check length. */ if (strlen(desc) > REND_DESC_MAX_SIZE) { log_warn(LD_REND, "Descriptor length is %i which exceeds " - "maximum rendezvous descriptor size of %i kilobytes.", + "maximum rendezvous descriptor size of %i bytes.", (int)strlen(desc), REND_DESC_MAX_SIZE); goto err; } From adc31001c2f9923f8fcee2141d233200c95373cf Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 18 Apr 2011 13:25:48 -0700 Subject: [PATCH 2/7] Add an XXX --- src/or/routerparse.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 28ecaf0ad7..3d73f8de68 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -3628,6 +3628,8 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out, eos = eos + 1; /* Check length. */ if (strlen(desc) > REND_DESC_MAX_SIZE) { + /* XXX023 If we are parsing this descriptor as a server, this + * should be a protocol warning. */ log_warn(LD_REND, "Descriptor length is %i which exceeds " "maximum rendezvous descriptor size of %i bytes.", (int)strlen(desc), REND_DESC_MAX_SIZE); From 48bdc2f729cba1a22305f6150d230cf0334ebd55 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 18 Apr 2011 13:53:13 -0700 Subject: [PATCH 3/7] Correct HS descriptor length check Fixes bug 2948. --- changes/bug2948 | 7 +++++++ src/or/routerparse.c | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 changes/bug2948 diff --git a/changes/bug2948 b/changes/bug2948 new file mode 100644 index 0000000000..640ef625d9 --- /dev/null +++ b/changes/bug2948 @@ -0,0 +1,7 @@ + o Minor bugfixes + - Only limit the lengths of single HS descriptors, even when + multiple HS descriptors are published to an HSDir relay in a + single POST operation. Fixes bug 2948; bugfix on 0.2.1.5-alpha. + Found by hsdir. + + diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 8456a0a02d..dd72eb6bb4 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4638,12 +4638,12 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out, else eos = eos + 1; /* Check length. */ - if (strlen(desc) > REND_DESC_MAX_SIZE) { + if (eos-desc > REND_DESC_MAX_SIZE) { /* XXX023 If we are parsing this descriptor as a server, this * should be a protocol warning. */ log_warn(LD_REND, "Descriptor length is %i which exceeds " "maximum rendezvous descriptor size of %i bytes.", - (int)strlen(desc), REND_DESC_MAX_SIZE); + (int)(eos-desc), REND_DESC_MAX_SIZE); goto err; } /* Tokenize descriptor. */ From cfd7b118a700f8dba53915118b1be4ac267db23b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 19 Apr 2011 12:29:20 -0400 Subject: [PATCH 4/7] Ouch: correctly tabify the micro-revision.i target in Makefile.am (0.2.1 only) --- src/or/Makefile.am | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/or/Makefile.am b/src/or/Makefile.am index 51c1b0fb06..b56d496431 100644 --- a/src/or/Makefile.am +++ b/src/or/Makefile.am @@ -55,19 +55,19 @@ noinst_HEADERS = or.h eventdns.h eventdns_tor.h micro-revision.i tor_main.o: micro-revision.i micro-revision.i: FORCE - @rm -f micro-revision.tmp; \ - if test -d ../../.git && test -x "`which git 2>&1;true`"; then \ - HASH="`git rev-parse --short=16 HEAD`"; \ - echo \"$$HASH\" > micro-revision.tmp; \ - fi; \ - if test ! -f micro-revision.tmp ; then \ - if test ! -f micro-revision.i ; then \ - echo '""' > micro-revision.i; \ - fi; \ - elif test ! -f micro-revision.i || \ - test x"`cat micro-revision.tmp`" != x"`cat micro-revision.i`"; then \ - mv micro-revision.tmp micro-revision.i; \ - fi; true + @rm -f micro-revision.tmp; \ + if test -d ../../.git && test -x "`which git 2>&1;true`"; then \ + HASH="`git rev-parse --short=16 HEAD`"; \ + echo \"$$HASH\" > micro-revision.tmp; \ + fi; \ + if test ! -f micro-revision.tmp ; then \ + if test ! -f micro-revision.i ; then \ + echo '""' > micro-revision.i; \ + fi; \ + elif test ! -f micro-revision.i || \ + test x"`cat micro-revision.tmp`" != x"`cat micro-revision.i`"; then \ + mv micro-revision.tmp micro-revision.i; \ + fi; true #Dummy target to ensure that micro-revision.i _always_ gets built. FORCE: From 5cc322e5472996fda6095e54de302c0f5c68ea92 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 19 Apr 2011 12:40:26 -0400 Subject: [PATCH 5/7] Standardize our printf code on %d, not %i. --- src/common/address.c | 2 +- src/or/routerparse.c | 4 ++-- src/test/test_addr.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common/address.c b/src/common/address.c index adc0ef0f7c..aff517ca51 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -604,7 +604,7 @@ tor_addr_parse_mask_ports(const char *s, tor_addr_t *addr_out, if (family == AF_INET6 && v4map) { if (bits > 32 && bits < 96) { /* Crazy */ log_warn(LD_GENERAL, - "Bad mask bits %i for V4-mapped V6 address; rejecting.", + "Bad mask bits %d for V4-mapped V6 address; rejecting.", bits); goto err; } diff --git a/src/or/routerparse.c b/src/or/routerparse.c index dd72eb6bb4..ba29f056f1 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4641,8 +4641,8 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out, if (eos-desc > REND_DESC_MAX_SIZE) { /* XXX023 If we are parsing this descriptor as a server, this * should be a protocol warning. */ - log_warn(LD_REND, "Descriptor length is %i which exceeds " - "maximum rendezvous descriptor size of %i bytes.", + log_warn(LD_REND, "Descriptor length is %d which exceeds " + "maximum rendezvous descriptor size of %d bytes.", (int)(eos-desc), REND_DESC_MAX_SIZE); goto err; } diff --git a/src/test/test_addr.c b/src/test/test_addr.c index 20ffaa0c52..6db4ee2483 100644 --- a/src/test/test_addr.c +++ b/src/test/test_addr.c @@ -477,9 +477,9 @@ test_addr_ip6_helpers(void) i = get_interface_address6(LOG_DEBUG, AF_INET6, &t2); #if 0 tor_inet_ntop(AF_INET, &t1.sa.sin_addr, buf, sizeof(buf)); - printf("\nv4 address: %s (family=%i)", buf, IN_FAMILY(&t1)); + printf("\nv4 address: %s (family=%d)", buf, IN_FAMILY(&t1)); tor_inet_ntop(AF_INET6, &t2.sa6.sin6_addr, buf, sizeof(buf)); - printf("\nv6 address: %s (family=%i)", buf, IN_FAMILY(&t2)); + printf("\nv6 address: %s (family=%d)", buf, IN_FAMILY(&t2)); #endif done: From 13c3884ff60aa0e58464f379f988683f1c30438a Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Tue, 19 Apr 2011 17:12:27 +0200 Subject: [PATCH 6/7] Don't sometimes undercount bw average This fixes the first part of bug 2704. It should occur only rarely when no bw maxima are known. Bugfix on 0.2.2.23-alpha. Fixes bug 2704. --- changes/bug2704_part1 | 5 +++++ src/or/rephist.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changes/bug2704_part1 diff --git a/changes/bug2704_part1 b/changes/bug2704_part1 new file mode 100644 index 0000000000..eaf22812cc --- /dev/null +++ b/changes/bug2704_part1 @@ -0,0 +1,5 @@ + o Minor bugfixes: + - Fix an issue causing calculation of Tor's average bandwidth as saved + in the state file to be 10 times smaller than it should be. Fixes the + first part of bug 2704, bugfix on tor-0.2.2.23-alpha. + diff --git a/src/or/rephist.c b/src/or/rephist.c index 9b7eefecf2..6034bbcb2b 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1673,7 +1673,7 @@ rep_hist_load_bwhist_state_section(bw_array_t *b, mv *= NUM_SECS_ROLLING_MEASURE; } else { /* No maxima known; guess average rate to be conservative. */ - mv = v / s_interval; + mv = (v / s_interval) * NUM_SECS_ROLLING_MEASURE; } if (!ok) { retval = -1; From 3f7f96d9e7aa2254d62d63530c37ee96d4b3abc8 Mon Sep 17 00:00:00 2001 From: Sebastian Hahn Date: Tue, 19 Apr 2011 16:00:41 +0200 Subject: [PATCH 7/7] Prevent hugely inflated observed bandwidth values When reading the bw history from the state file, we'd add the 900-second value as traffic that occured during one second. Fix that by adding the average value to each second. This bug was present since 0.2.0.5-alpha, but was hidden until 0.2.23-alpha when we started using the saved values. --- changes/bug2704_part2 | 5 +++++ src/or/rephist.c | 19 ++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 changes/bug2704_part2 diff --git a/changes/bug2704_part2 b/changes/bug2704_part2 new file mode 100644 index 0000000000..962c8b7099 --- /dev/null +++ b/changes/bug2704_part2 @@ -0,0 +1,5 @@ + o Major bugfixes: + - Prevent relays that read their bandwidth history from their state file + from arbitrarily inflating that value. Fixes the second half of bug + 2704, bugfix on tor-0.2.2.23-alpha. + diff --git a/src/or/rephist.c b/src/or/rephist.c index 6034bbcb2b..fb091d5adf 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1686,11 +1686,24 @@ rep_hist_load_bwhist_state_section(bw_array_t *b, } if (start < now) { - add_obs(b, start, v); + time_t cur_start = start; + time_t actual_interval_len = s_interval; + uint64_t cur_val = 0; + /* Calculate the average per second. This is the best we can do + * because our state file doesn't have per-second resolution. */ + if (start + s_interval > now) + actual_interval_len = now - start; + cur_val = v / actual_interval_len; + /* This is potentially inefficient, but since we don't do it very + * often it should be ok. */ + while (cur_start < start + actual_interval_len) { + add_obs(b, cur_start, cur_val); + ++cur_start; + } b->max_total = mv; /* This will result in some fairly choppy history if s_interval - * is notthe same as NUM_SECS_BW_SUM_INTERVAL. XXXX */ - start += s_interval; + * is not the same as NUM_SECS_BW_SUM_INTERVAL. XXXX */ + start += actual_interval_len; } } SMARTLIST_FOREACH_END(cp); }