[or-cvs] [tor/master 01/11] Try harder not to exceed the 50 KB extra-info descriptor limit.
nickm at torproject.org
nickm at torproject.org
Fri Nov 19 21:58:38 UTC 2010
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date: Sat, 13 Nov 2010 22:25:19 +0100
Subject: Try harder not to exceed the 50 KB extra-info descriptor limit.
Commit: cec21652a7c939d1112591347e35c0815de63f93
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 | 191 ++++++++++++++++++++++++-----------------------------
src/or/router.h | 2 +-
5 files changed, 98 insertions(+), 110 deletions(-)
create mode 100644 changes/bug2183
diff --git a/changes/bug2183 b/changes/bug2183
new file mode 100644
index 0000000..01a8d76
--- /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 056fc5c..22b3ec5 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 c3914dc..8f5a34d 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 725a515..5c137aa 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 <b>extrainfo</b> to the <b>maxlen</b>-byte string
- * <b>s</b>, signing them with <b>ident_key</b>. Return 0 on success,
- * negative on failure. */
+/** Write the contents of <b>extrainfo</b> and aggregated statistics to
+ * *<b>s_out</b>, signing them with <b>ident_key</b>. 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;
}
- 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);
+#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);
}
- return (int)strlen(s)+1;
+ *s_out = s;
+ s = NULL; /* prevent free */
+ 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;
+
+ 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 <b>s</b> is a legally valid server nickname. */
diff --git a/src/or/router.h b/src/or/router.h
index c5e7987..0bfa2d5 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);
--
1.7.1
More information about the tor-commits
mailing list