From cec21652a7c939d1112591347e35c0815de63f93 Mon Sep 17 00:00:00 2001 From: Karsten Loesing Date: Sat, 13 Nov 2010 22:25:19 +0100 Subject: [PATCH 1/8] Try harder not to exceed the 50 KB extra-info descriptor limit. Our checks that we don't exceed the 50 KB size limit of extra-info descriptors apparently failed. This patch fixes these checks and reserves another 250 bytes for appending the signature. Fixes bug 2183. --- changes/bug2183 | 6 ++ src/or/rephist.c | 7 +- src/or/rephist.h | 2 +- src/or/router.c | 193 +++++++++++++++++++++-------------------------- src/or/router.h | 2 +- 5 files changed, 99 insertions(+), 111 deletions(-) create mode 100644 changes/bug2183 diff --git a/changes/bug2183 b/changes/bug2183 new file mode 100644 index 0000000000..01a8d76aca --- /dev/null +++ b/changes/bug2183 @@ -0,0 +1,6 @@ + o Minor bugfixes: + - Try harder not to exceed the maximum length of 50 KB when writing + statistics to extra-info descriptors. This bug was triggered by very + fast relays reporting exit-port, entry, and dirreq statistics. + Reported by Olaf Selke. Bugfix on 0.2.2.1-alpha. Fixes bug 2183. + diff --git a/src/or/rephist.c b/src/or/rephist.c index 056fc5cc12..22b3ec5217 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1440,7 +1440,7 @@ rep_hist_fill_bandwidth_history(char *buf, size_t len, bw_array_t *b) * history in its descriptor. */ char * -rep_hist_get_bandwidth_lines(int for_extrainfo) +rep_hist_get_bandwidth_lines(void) { char *buf, *cp; char t[ISO_TIME_LEN+1]; @@ -1474,9 +1474,8 @@ rep_hist_get_bandwidth_lines(int for_extrainfo) } tor_assert(b); format_iso_time(t, b->next_period-NUM_SECS_BW_SUM_INTERVAL); - tor_snprintf(cp, len-(cp-buf), "%s%s %s (%d s) ", - for_extrainfo ? "" : "opt ", desc, t, - NUM_SECS_BW_SUM_INTERVAL); + tor_snprintf(cp, len-(cp-buf), "%s %s (%d s) ", + desc, t, NUM_SECS_BW_SUM_INTERVAL); cp += strlen(cp); cp += rep_hist_fill_bandwidth_history(cp, len-(cp-buf), b); strlcat(cp, "\n", len-(cp-buf)); diff --git a/src/or/rephist.h b/src/or/rephist.h index c3914dcaf2..8f5a34dacf 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -28,7 +28,7 @@ void rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when); void rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when); int rep_hist_bandwidth_assess(void); -char *rep_hist_get_bandwidth_lines(int for_extrainfo); +char *rep_hist_get_bandwidth_lines(void); void rep_hist_update_state(or_state_t *state); int rep_hist_load_state(or_state_t *state, char **err); void rep_history_clean(time_t before); diff --git a/src/or/router.c b/src/or/router.c index 725a515e0c..5c137aa77e 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1370,7 +1370,6 @@ router_rebuild_descriptor(int force) uint32_t addr; char platform[256]; int hibernating = we_are_hibernating(); - size_t ei_size; or_options_t *options = get_options(); if (desc_clean_since && !force) @@ -1486,11 +1485,8 @@ router_rebuild_descriptor(int force) ei->cache_info.published_on = ri->cache_info.published_on; memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest, DIGEST_LEN); - ei_size = options->ExtraInfoStatistics ? MAX_EXTRAINFO_UPLOAD_SIZE : 8192; - ei->cache_info.signed_descriptor_body = tor_malloc(ei_size); - if (extrainfo_dump_to_string(ei->cache_info.signed_descriptor_body, - ei_size, ei, - get_server_identity_key()) < 0) { + if (extrainfo_dump_to_string(&ei->cache_info.signed_descriptor_body, + ei, get_server_identity_key()) < 0) { log_warn(LD_BUG, "Couldn't generate extra-info descriptor."); routerinfo_free(ri); extrainfo_free(ei); @@ -1981,11 +1977,11 @@ load_stats_file(const char *filename, const char *end_line, time_t now, return r; } -/** Write the contents of extrainfo to the maxlen-byte string - * s, signing them with ident_key. Return 0 on success, - * negative on failure. */ +/** Write the contents of extrainfo and aggregated statistics to + * *s_out, signing them with ident_key. Return 0 on + * success, negative on failure. */ int -extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo, +extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, crypto_pk_env_t *ident_key) { or_options_t *options = get_options(); @@ -1994,140 +1990,127 @@ extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo, char digest[DIGEST_LEN]; char *bandwidth_usage; int result; - size_t len; static int write_stats_to_extrainfo = 1; +#define SIG_LEN 250 + char sig[SIG_LEN+1]; + char *s, *pre, *contents, *cp, *s_dup = NULL; time_t now = time(NULL); + const char *bridge_stats = NULL; + smartlist_t *chunks = smartlist_create(); + extrainfo_t *ei_tmp = NULL; base16_encode(identity, sizeof(identity), extrainfo->cache_info.identity_digest, DIGEST_LEN); format_iso_time(published, extrainfo->cache_info.published_on); - bandwidth_usage = rep_hist_get_bandwidth_lines(1); - - result = tor_snprintf(s, maxlen, - "extra-info %s %s\n" - "published %s\n%s", - extrainfo->nickname, identity, - published, bandwidth_usage); + bandwidth_usage = rep_hist_get_bandwidth_lines(); + tor_asprintf(&pre, "extra-info %s %s\npublished %s\n%s", + extrainfo->nickname, identity, + published, bandwidth_usage); tor_free(bandwidth_usage); - if (result<0) - return -1; + smartlist_add(chunks, pre); if (options->ExtraInfoStatistics && write_stats_to_extrainfo) { - char *contents = NULL; log_info(LD_GENERAL, "Adding stats to extra-info descriptor."); if (options->DirReqStatistics && load_stats_file("stats"PATH_SEPARATOR"dirreq-stats", "dirreq-stats-end", now, &contents) > 0) { - size_t pos = strlen(s); - if (strlcpy(s + pos, contents, maxlen - strlen(s)) != - strlen(contents)) { - log_warn(LD_DIR, "Could not write dirreq-stats to extra-info " - "descriptor."); - s[pos] = '\0'; - write_stats_to_extrainfo = 0; - } - tor_free(contents); + smartlist_add(chunks, contents); } if (options->EntryStatistics && load_stats_file("stats"PATH_SEPARATOR"entry-stats", "entry-stats-end", now, &contents) > 0) { - size_t pos = strlen(s); - if (strlcpy(s + pos, contents, maxlen - strlen(s)) != - strlen(contents)) { - log_warn(LD_DIR, "Could not write entry-stats to extra-info " - "descriptor."); - s[pos] = '\0'; - write_stats_to_extrainfo = 0; - } - tor_free(contents); + smartlist_add(chunks, contents); } if (options->CellStatistics && load_stats_file("stats"PATH_SEPARATOR"buffer-stats", "cell-stats-end", now, &contents) > 0) { - size_t pos = strlen(s); - if (strlcpy(s + pos, contents, maxlen - strlen(s)) != - strlen(contents)) { - log_warn(LD_DIR, "Could not write buffer-stats to extra-info " - "descriptor."); - s[pos] = '\0'; - write_stats_to_extrainfo = 0; - } - tor_free(contents); + smartlist_add(chunks, contents); } if (options->ExitPortStatistics && load_stats_file("stats"PATH_SEPARATOR"exit-stats", "exit-stats-end", now, &contents) > 0) { - size_t pos = strlen(s); - if (strlcpy(s + pos, contents, maxlen - strlen(s)) != - strlen(contents)) { - log_warn(LD_DIR, "Could not write exit-stats to extra-info " - "descriptor."); - s[pos] = '\0'; - write_stats_to_extrainfo = 0; - } - tor_free(contents); + smartlist_add(chunks, contents); } } if (should_record_bridge_info(options) && write_stats_to_extrainfo) { - const char *bridge_stats = geoip_get_bridge_stats_extrainfo(now); + bridge_stats = geoip_get_bridge_stats_extrainfo(now); if (bridge_stats) { - size_t pos = strlen(s); - if (strlcpy(s + pos, bridge_stats, maxlen - strlen(s)) != - strlen(bridge_stats)) { - log_warn(LD_DIR, "Could not write bridge-stats to extra-info " - "descriptor."); - s[pos] = '\0'; - write_stats_to_extrainfo = 0; - } + smartlist_add(chunks, tor_strdup(bridge_stats)); } } - len = strlen(s); - strlcat(s+len, "router-signature\n", maxlen-len); - len += strlen(s+len); - if (router_get_extrainfo_hash(s, digest)<0) - return -1; - if (router_append_dirobj_signature(s+len, maxlen-len, digest, DIGEST_LEN, - ident_key)<0) - return -1; + smartlist_add(chunks, tor_strdup("router-signature\n")); + s = smartlist_join_strings(chunks, "", 0, NULL); - { - char *cp, *s_dup; - extrainfo_t *ei_tmp; - cp = s_dup = tor_strdup(s); - ei_tmp = extrainfo_parse_entry_from_string(cp, NULL, 1, NULL); - if (!ei_tmp) { - log_err(LD_BUG, - "We just generated an extrainfo descriptor we can't parse."); - log_err(LD_BUG, "Descriptor was: <<%s>>", s); - tor_free(s_dup); - return -1; + if (strlen(s) > MAX_EXTRAINFO_UPLOAD_SIZE - SIG_LEN) { + if (write_stats_to_extrainfo) { + log_warn(LD_GENERAL, "We just generated an extra-info descriptor " + "with statistics that exceeds the 50 KB " + "upload limit. Not adding statistics to this " + "or any future extra-info descriptors. " + "Descriptor was: <<%s>>", s); + goto nostats; + } else { + log_err(LD_BUG, "We just generated an extra-info descriptors that " + "exceeds the 50 KB upload limit. Descriptor was: " + "<<%s>>", s); + goto err; + } + } +#undef SIG_LEN + + memset(sig, 0, sizeof(sig)); + if (router_get_extrainfo_hash(s, digest) < 0 || + router_append_dirobj_signature(sig, sizeof(sig), digest, DIGEST_LEN, + ident_key) < 0) { + log_err(LD_BUG, "Could not append signature to extra-info " + "descriptor. Descriptor was: <<%s>>", s); + goto err; + } + smartlist_add(chunks, tor_strdup(sig)); + tor_free(s); + s = smartlist_join_strings(chunks, "", 0, NULL); + + cp = s_dup = tor_strdup(s); + ei_tmp = extrainfo_parse_entry_from_string(cp, NULL, 1, NULL); + if (!ei_tmp) { + if (write_stats_to_extrainfo) { + log_warn(LD_GENERAL, "We just generated an extra-info descriptor " + "with statistics that we can't parse. Not " + "adding statistics to this or any future " + "extra-info descriptors. Descriptor was: " + "<<%s>>", s); + goto nostats; + } else { + log_err(LD_BUG, "We just generated an extrainfo descriptor we " + "can't parse. Descriptor was: <<%s>>", s); + goto err; } - tor_free(s_dup); - extrainfo_free(ei_tmp); } - if (options->ExtraInfoStatistics && write_stats_to_extrainfo) { - char *cp, *s_dup; - extrainfo_t *ei_tmp; - cp = s_dup = tor_strdup(s); - ei_tmp = extrainfo_parse_entry_from_string(cp, NULL, 1, NULL); - if (!ei_tmp) { - log_warn(LD_GENERAL, - "We just generated an extra-info descriptor with " - "statistics that we can't parse. Not adding statistics to " - "this or any future extra-info descriptors. Descriptor " - "was:\n%s", s); - write_stats_to_extrainfo = 0; - extrainfo_dump_to_string(s, maxlen, extrainfo, ident_key); - } - tor_free(s_dup); - extrainfo_free(ei_tmp); - } + *s_out = s; + s = NULL; /* prevent free */ + result = 0; + goto done; - return (int)strlen(s)+1; + nostats: + write_stats_to_extrainfo = 0; + result = extrainfo_dump_to_string(s_out, extrainfo, ident_key); + goto done; + + err: + result = -1; + + done: + tor_free(s); + SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp)); + smartlist_free(chunks); + tor_free(s_dup); + extrainfo_free(ei_tmp); + + return result; } /** Return true iff s is a legally valid server nickname. */ diff --git a/src/or/router.h b/src/or/router.h index c5e7987dd6..0bfa2d5545 100644 --- a/src/or/router.h +++ b/src/or/router.h @@ -77,7 +77,7 @@ int router_pick_published_address(or_options_t *options, uint32_t *addr); int router_rebuild_descriptor(int force); int router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, crypto_pk_env_t *ident_key); -int extrainfo_dump_to_string(char *s, size_t maxlen, extrainfo_t *extrainfo, +int extrainfo_dump_to_string(char **s, extrainfo_t *extrainfo, crypto_pk_env_t *ident_key); int is_legal_nickname(const char *s); int is_legal_nickname_or_hexdigest(const char *s); From ff1cf35442e0859574a2c3be725b6387dbfac64a Mon Sep 17 00:00:00 2001 From: Karsten Loesing Date: Mon, 15 Nov 2010 13:39:53 +0100 Subject: [PATCH 2/8] Don't use log_err for non-criticial warnings. --- src/or/router.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 5c137aa77e..72c32d5eae 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2053,9 +2053,9 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, "Descriptor was: <<%s>>", s); goto nostats; } else { - log_err(LD_BUG, "We just generated an extra-info descriptors that " - "exceeds the 50 KB upload limit. Descriptor was: " - "<<%s>>", s); + log_warn(LD_BUG, "We just generated an extra-info descriptors that " + "exceeds the 50 KB upload limit. Descriptor was: " + "<<%s>>", s); goto err; } } @@ -2065,8 +2065,8 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, if (router_get_extrainfo_hash(s, digest) < 0 || router_append_dirobj_signature(sig, sizeof(sig), digest, DIGEST_LEN, ident_key) < 0) { - log_err(LD_BUG, "Could not append signature to extra-info " - "descriptor. Descriptor was: <<%s>>", s); + log_warn(LD_BUG, "Could not append signature to extra-info descriptor. " + "Descriptor was: <<%s>>", s); goto err; } smartlist_add(chunks, tor_strdup(sig)); @@ -2084,8 +2084,8 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, "<<%s>>", s); goto nostats; } else { - log_err(LD_BUG, "We just generated an extrainfo descriptor we " - "can't parse. Descriptor was: <<%s>>", s); + log_warn(LD_BUG, "We just generated an extrainfo descriptor we can't " + "parse. Descriptor was: <<%s>>", s); goto err; } } From 0f1afaf5955e5905b7fd6fcf27d7c9958782bd06 Mon Sep 17 00:00:00 2001 From: Karsten Loesing Date: Wed, 17 Nov 2010 10:26:34 +0100 Subject: [PATCH 3/8] Tweak the bugfix for 2183 a bit more. --- src/or/router.c | 49 +++++++++++++++++++++----------------------- src/or/routerparse.h | 1 + 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 72c32d5eae..38f0a2f47d 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1876,7 +1876,8 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, } } - if (written+256 > maxlen) { /* Not enough room for signature. */ + if (written + DIROBJ_MAX_SIG_LEN > maxlen) { + /* Not enough room for signature. */ log_warn(LD_BUG,"not enough room left in descriptor for signature!"); return -1; } @@ -1991,11 +1992,9 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, char *bandwidth_usage; int result; static int write_stats_to_extrainfo = 1; -#define SIG_LEN 250 - char sig[SIG_LEN+1]; + char sig[DIROBJ_MAX_SIG_LEN+1]; char *s, *pre, *contents, *cp, *s_dup = NULL; time_t now = time(NULL); - const char *bridge_stats = NULL; smartlist_t *chunks = smartlist_create(); extrainfo_t *ei_tmp = NULL; @@ -2035,7 +2034,7 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, } if (should_record_bridge_info(options) && write_stats_to_extrainfo) { - bridge_stats = geoip_get_bridge_stats_extrainfo(now); + const char *bridge_stats = geoip_get_bridge_stats_extrainfo(now); if (bridge_stats) { smartlist_add(chunks, tor_strdup(bridge_stats)); } @@ -2044,29 +2043,31 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, smartlist_add(chunks, tor_strdup("router-signature\n")); s = smartlist_join_strings(chunks, "", 0, NULL); - if (strlen(s) > MAX_EXTRAINFO_UPLOAD_SIZE - SIG_LEN) { - if (write_stats_to_extrainfo) { + while (strlen(s) > MAX_EXTRAINFO_UPLOAD_SIZE - DIROBJ_MAX_SIG_LEN) { + if (smartlist_len(chunks) > 2) { + int idx = smartlist_len(chunks) - 2; + char *e = smartlist_get(chunks, idx); + smartlist_del_keeporder(chunks, idx); log_warn(LD_GENERAL, "We just generated an extra-info descriptor " "with statistics that exceeds the 50 KB " - "upload limit. Not adding statistics to this " - "or any future extra-info descriptors. " - "Descriptor was: <<%s>>", s); - goto nostats; + "upload limit. Removing last added " + "statistics."); + tor_free(e); + tor_free(s); + s = smartlist_join_strings(chunks, "", 0, NULL); } else { log_warn(LD_BUG, "We just generated an extra-info descriptors that " - "exceeds the 50 KB upload limit. Descriptor was: " - "<<%s>>", s); + "exceeds the 50 KB upload limit."); goto err; } } -#undef SIG_LEN memset(sig, 0, sizeof(sig)); if (router_get_extrainfo_hash(s, digest) < 0 || router_append_dirobj_signature(sig, sizeof(sig), digest, DIGEST_LEN, ident_key) < 0) { - log_warn(LD_BUG, "Could not append signature to extra-info descriptor. " - "Descriptor was: <<%s>>", s); + log_warn(LD_BUG, "Could not append signature to extra-info " + "descriptor."); goto err; } smartlist_add(chunks, tor_strdup(sig)); @@ -2080,12 +2081,13 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, log_warn(LD_GENERAL, "We just generated an extra-info descriptor " "with statistics that we can't parse. Not " "adding statistics to this or any future " - "extra-info descriptors. Descriptor was: " - "<<%s>>", s); - goto nostats; + "extra-info descriptors."); + write_stats_to_extrainfo = 0; + result = extrainfo_dump_to_string(s_out, extrainfo, ident_key); + goto done; } else { - log_warn(LD_BUG, "We just generated an extrainfo descriptor we can't " - "parse. Descriptor was: <<%s>>", s); + log_warn(LD_BUG, "We just generated an extrainfo descriptor we " + "can't parse."); goto err; } } @@ -2095,11 +2097,6 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, result = 0; goto done; - nostats: - write_stats_to_extrainfo = 0; - result = extrainfo_dump_to_string(s_out, extrainfo, ident_key); - goto done; - err: result = -1; diff --git a/src/or/routerparse.h b/src/or/routerparse.h index e5ebf07615..f41743bcfb 100644 --- a/src/or/routerparse.h +++ b/src/or/routerparse.h @@ -20,6 +20,7 @@ int router_get_networkstatus_v3_hash(const char *s, char *digest, digest_algorithm_t algorithm); int router_get_networkstatus_v3_hashes(const char *s, digests_t *digests); int router_get_extrainfo_hash(const char *s, char *digest); +#define DIROBJ_MAX_SIG_LEN 256 int router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest, size_t digest_len, From 213bcb3c4022ef3535a50f2d9375ba79eccf11b4 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 14 Nov 2010 22:17:32 -0800 Subject: [PATCH 4/8] Do not emit an extra-info-digest descriptor line if the digest is zero. --- src/or/router.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 725a515e0c..2705a78c54 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1746,6 +1746,7 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, char digest[DIGEST_LEN]; char published[ISO_TIME_LEN+1]; char fingerprint[FINGERPRINT_LEN+1]; + int has_extra_info_digest; char extra_info_digest[HEX_DIGEST_LEN+1]; size_t onion_pkeylen, identity_pkeylen; size_t written; @@ -1796,8 +1797,13 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, family_line = tor_strdup(""); } - base16_encode(extra_info_digest, sizeof(extra_info_digest), - router->cache_info.extra_info_digest, DIGEST_LEN); + has_extra_info_digest = + tor_digest_is_zero(router->cache_info.extra_info_digest); + + if (has_extra_info_digest) { + base16_encode(extra_info_digest, sizeof(extra_info_digest), + router->cache_info.extra_info_digest, DIGEST_LEN); + } /* Generate the easy portion of the router descriptor. */ result = tor_snprintf(s, maxlen, @@ -1808,7 +1814,7 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, "opt fingerprint %s\n" "uptime %ld\n" "bandwidth %d %d %d\n" - "opt extra-info-digest %s\n%s" + "%s%s%s%s" "onion-key\n%s" "signing-key\n%s" "%s%s%s%s", @@ -1823,7 +1829,9 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, (int) router->bandwidthrate, (int) router->bandwidthburst, (int) router->bandwidthcapacity, - extra_info_digest, + has_extra_info_digest ? "opt extra-info-digest " : "", + has_extra_info_digest ? extra_info_digest : "", + has_extra_info_digest ? "\n" : "", options->DownloadExtraInfo ? "opt caches-extra-info\n" : "", onion_pkey, identity_pkey, family_line, From 780b8f4287432c51e822b72952f097a1dbb5e0d2 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Mon, 15 Nov 2010 01:36:08 -0800 Subject: [PATCH 5/8] Generate a router descriptor even if generating an extra-info descriptor fails. Fixes bug #2195. --- src/or/router.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 2705a78c54..17c20eddfd 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1492,19 +1492,24 @@ router_rebuild_descriptor(int force) ei_size, ei, get_server_identity_key()) < 0) { log_warn(LD_BUG, "Couldn't generate extra-info descriptor."); - routerinfo_free(ri); extrainfo_free(ei); - return -1; + ei = NULL; + } else { + ei->cache_info.signed_descriptor_len = + strlen(ei->cache_info.signed_descriptor_body); + router_get_extrainfo_hash(ei->cache_info.signed_descriptor_body, + ei->cache_info.signed_descriptor_digest); } - ei->cache_info.signed_descriptor_len = - strlen(ei->cache_info.signed_descriptor_body); - router_get_extrainfo_hash(ei->cache_info.signed_descriptor_body, - ei->cache_info.signed_descriptor_digest); /* Now finish the router descriptor. */ - memcpy(ri->cache_info.extra_info_digest, - ei->cache_info.signed_descriptor_digest, - DIGEST_LEN); + if (ei) { + memcpy(ri->cache_info.extra_info_digest, + ei->cache_info.signed_descriptor_digest, + DIGEST_LEN); + } else { + /* ri was allocated with tor_malloc_zero, so there is no need to + * zero ri->cache_info.extra_info_digest here. */ + } ri->cache_info.signed_descriptor_body = tor_malloc(8192); if (router_dump_router_to_string(ri->cache_info.signed_descriptor_body, 8192, ri, get_server_identity_key()) < 0) { @@ -1531,7 +1536,9 @@ router_rebuild_descriptor(int force) routerinfo_set_country(ri); - tor_assert(! routerinfo_incompatible_with_extrainfo(ri, ei, NULL, NULL)); + if (ei) { + tor_assert(! routerinfo_incompatible_with_extrainfo(ri, ei, NULL, NULL)); + } routerinfo_free(desc_routerinfo); desc_routerinfo = ri; From e88486bc9d2f4fd6982a00f5ccf498a4f809cfd2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 19 Nov 2010 16:13:11 -0500 Subject: [PATCH 6/8] Add comments to some of the bug2183 fix code --- src/or/router.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/or/router.c b/src/or/router.c index 38f0a2f47d..9b80247f19 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2044,7 +2044,12 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, s = smartlist_join_strings(chunks, "", 0, NULL); while (strlen(s) > MAX_EXTRAINFO_UPLOAD_SIZE - DIROBJ_MAX_SIG_LEN) { + /* So long as there are at least two chunks (one for the initial + * extra-info line and one for the router-signature), we can keep removing + * things. */ if (smartlist_len(chunks) > 2) { + /* We remove the next-to-last element (remember, len-1 is the last + element), since we need to keep the router-signature element. */ int idx = smartlist_len(chunks) - 2; char *e = smartlist_get(chunks, idx); smartlist_del_keeporder(chunks, idx); From 69472ca4210183ec11a632abd952b6060a086258 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Fri, 19 Nov 2010 13:23:45 -0800 Subject: [PATCH 7/8] Fix logic error in router_dump_router_to_string. Spotted by Nick Mathewson. --- src/or/router.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/router.c b/src/or/router.c index 17c20eddfd..e82945e64c 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1805,7 +1805,7 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router, } has_extra_info_digest = - tor_digest_is_zero(router->cache_info.extra_info_digest); + ! tor_digest_is_zero(router->cache_info.extra_info_digest); if (has_extra_info_digest) { base16_encode(extra_info_digest, sizeof(extra_info_digest), From b3470ce95740b968dc53a3e43208a1ed02138a28 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Fri, 19 Nov 2010 13:46:09 -0800 Subject: [PATCH 8/8] Add changes file for bug2195. --- changes/bug2195 | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changes/bug2195 diff --git a/changes/bug2195 b/changes/bug2195 new file mode 100644 index 0000000000..1724fd0e21 --- /dev/null +++ b/changes/bug2195 @@ -0,0 +1,7 @@ + o Minor bugfixes + - Publish a router descriptor even if generating an extra-info + descriptor fails. Previously we would not publish a router + descriptor without an extra-info descriptor; this can cause fast + exit relays collecting exit-port statistics to drop from the + consensus. Bugfix on 0.1.2.9-rc; fixes bug 2195. +