commit 45be44ed9c1b3ddbe2ad4caaebc3ae0076bdbd07 Author: teor teor@torproject.org Date: Mon Jun 24 20:32:38 2019 +1000
stats: Split extrainfo_dump_to_string() into smaller functions.
Closes ticket 30956. --- changes/ticket30956_refactor | 3 + scripts/maint/practracker/exceptions.txt | 3 +- src/feature/relay/router.c | 186 +++++++++++++++++++++++-------- 3 files changed, 146 insertions(+), 46 deletions(-)
diff --git a/changes/ticket30956_refactor b/changes/ticket30956_refactor new file mode 100644 index 000000000..81151c6cc --- /dev/null +++ b/changes/ticket30956_refactor @@ -0,0 +1,3 @@ + o Code simplification and refactoring: + - Split extrainfo_dump_to_string() into smaller functions. + Closes ticket 30956. diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 815d0ed09..4db452b89 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -225,13 +225,12 @@ problem function-size /src/feature/nodelist/routerlist.c:update_extrainfo_downlo problem function-size /src/feature/relay/dns.c:dns_resolve_impl() 134 problem function-size /src/feature/relay/dns.c:configure_nameservers() 161 problem function-size /src/feature/relay/dns.c:evdns_callback() 109 -problem file-size /src/feature/relay/router.c 3407 +problem file-size /src/feature/relay/router.c 3510 problem include-count /src/feature/relay/router.c 56 problem function-size /src/feature/relay/router.c:init_keys() 252 problem function-size /src/feature/relay/router.c:get_my_declared_family() 114 problem function-size /src/feature/relay/router.c:router_build_fresh_unsigned_routerinfo() 136 problem function-size /src/feature/relay/router.c:router_dump_router_to_string() 371 -problem function-size /src/feature/relay/router.c:extrainfo_dump_to_string() 206 problem function-size /src/feature/relay/routerkeys.c:load_ed_keys() 294 problem function-size /src/feature/rend/rendcache.c:rend_cache_store_v2_desc_as_client() 193 problem function-size /src/feature/rend/rendclient.c:rend_client_send_introduction() 220 diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 25bb1835c..6b3326529 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -3113,33 +3113,22 @@ load_stats_file(const char *filename, const char *end_line, time_t now, return r; }
-/** Write the contents of <b>extrainfo</b>, to * *<b>s_out</b>, signing them - * with <b>ident_key</b>. - * - * If ExtraInfoStatistics is 1, also write aggregated statistics and related - * configuration data before signing. Most statistics also have an option that - * enables or disables that particular statistic. - * - * Return 0 on success, negative on failure. */ -int -extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, - crypto_pk_t *ident_key, - const ed25519_keypair_t *signing_keypair) +/** Add header strings to chunks, based on the extrainfo object extrainfo, + * and ed25519 keypair signing_keypair, if emit_ed_sigs is true. + * Helper for extrainfo_dump_to_string(). + * Returns 0 on success, negative on failure. */ +static int +extrainfo_dump_to_string_header_helper( + smartlist_t *chunks, + const extrainfo_t *extrainfo, + const ed25519_keypair_t *signing_keypair, + int emit_ed_sigs) { - const or_options_t *options = get_options(); char identity[HEX_DIGEST_LEN+1]; char published[ISO_TIME_LEN+1]; - char digest[DIGEST_LEN]; - int result; - static int write_stats_to_extrainfo = 1; - char sig[DIROBJ_MAX_SIG_LEN+1]; - char *s = NULL, *pre, *contents, *cp, *s_dup = NULL; - time_t now = time(NULL); - smartlist_t *chunks = smartlist_new(); - extrainfo_t *ei_tmp = NULL; - const int emit_ed_sigs = signing_keypair && - extrainfo->cache_info.signing_key_cert; char *ed_cert_line = NULL; + char *pre = NULL; + int rv = -1;
base16_encode(identity, sizeof(identity), extrainfo->cache_info.identity_digest, DIGEST_LEN); @@ -3175,6 +3164,29 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, published); smartlist_add(chunks, pre);
+ rv = 0; + goto done; + + err: + rv = -1; + + done: + tor_free(ed_cert_line); + return rv; +} + +/** Add pluggable transport and statistics strings to chunks, skipping + * statistics if write_stats_to_extrainfo is false. + * Helper for extrainfo_dump_to_string(). + * Can not fail. */ +static void +extrainfo_dump_to_string_stats_helper(smartlist_t *chunks, + int write_stats_to_extrainfo) +{ + const or_options_t *options = get_options(); + char *contents = NULL; + time_t now = time(NULL); + /* Add information about the pluggable transports we support, even if we * are not publishing statistics. This information is needed by BridgeDB * to distribute bridges. */ @@ -3241,21 +3253,113 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, } } } +} + +/** Add an ed25519 signature of chunks to chunks, using the ed25519 keypair + * signing_keypair. + * Helper for extrainfo_dump_to_string(). + * Returns 0 on success, negative on failure. */ +static int +extrainfo_dump_to_string_ed_sig_helper( + smartlist_t *chunks, + const ed25519_keypair_t *signing_keypair) +{ + char sha256_digest[DIGEST256_LEN]; + ed25519_signature_t ed_sig; + char buf[ED25519_SIG_BASE64_LEN+1]; + int rv = -1; + + smartlist_add_strdup(chunks, "router-sig-ed25519 "); + crypto_digest_smartlist_prefix(sha256_digest, DIGEST256_LEN, + ED_DESC_SIGNATURE_PREFIX, + chunks, "", DIGEST_SHA256); + if (ed25519_sign(&ed_sig, (const uint8_t*)sha256_digest, DIGEST256_LEN, + signing_keypair) < 0) + goto err; + ed25519_signature_to_base64(buf, &ed_sig); + + smartlist_add_asprintf(chunks, "%s\n", buf); + + rv = 0; + goto done; + + err: + rv = -1; + + done: + return rv; +} + +/** Add an RSA signature of extrainfo_string to chunks, using the RSA key + * ident_key. + * Helper for extrainfo_dump_to_string(). + * Returns 0 on success, negative on failure. */ +static int +extrainfo_dump_to_string_rsa_sig_helper(smartlist_t *chunks, + crypto_pk_t *ident_key, + const char *extrainfo_string) +{ + char sig[DIROBJ_MAX_SIG_LEN+1]; + char digest[DIGEST_LEN]; + int rv = -1; + + memset(sig, 0, sizeof(sig)); + if (router_get_extrainfo_hash(extrainfo_string, strlen(extrainfo_string), + 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."); + goto err; + } + smartlist_add_strdup(chunks, sig); + + rv = 0; + goto done; + + err: + rv = -1; + + done: + return rv; +} + +/** Write the contents of <b>extrainfo</b>, to * *<b>s_out</b>, signing them + * with <b>ident_key</b>. + * + * If ExtraInfoStatistics is 1, also write aggregated statistics and related + * configuration data before signing. Most statistics also have an option that + * enables or disables that particular statistic. + * + * Always write pluggable transport lines. + * + * Return 0 on success, negative on failure. */ +int +extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, + crypto_pk_t *ident_key, + const ed25519_keypair_t *signing_keypair) +{ + int result; + static int write_stats_to_extrainfo = 1; + char *s = NULL, *cp, *s_dup = NULL; + smartlist_t *chunks = smartlist_new(); + extrainfo_t *ei_tmp = NULL; + const int emit_ed_sigs = signing_keypair && + extrainfo->cache_info.signing_key_cert; + int rv = 0; + + rv = extrainfo_dump_to_string_header_helper(chunks, extrainfo, + signing_keypair, + emit_ed_sigs); + if (rv < 0) + goto err; + + extrainfo_dump_to_string_stats_helper(chunks, write_stats_to_extrainfo);
if (emit_ed_sigs) { - char sha256_digest[DIGEST256_LEN]; - smartlist_add_strdup(chunks, "router-sig-ed25519 "); - crypto_digest_smartlist_prefix(sha256_digest, DIGEST256_LEN, - ED_DESC_SIGNATURE_PREFIX, - chunks, "", DIGEST_SHA256); - ed25519_signature_t ed_sig; - char buf[ED25519_SIG_BASE64_LEN+1]; - if (ed25519_sign(&ed_sig, (const uint8_t*)sha256_digest, DIGEST256_LEN, - signing_keypair) < 0) + rv = extrainfo_dump_to_string_ed_sig_helper(chunks, signing_keypair); + if (rv < 0) goto err; - ed25519_signature_to_base64(buf, &ed_sig); - - smartlist_add_asprintf(chunks, "%s\n", buf); }
smartlist_add_strdup(chunks, "router-signature\n"); @@ -3285,15 +3389,10 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, } }
- memset(sig, 0, sizeof(sig)); - if (router_get_extrainfo_hash(s, strlen(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."); + rv = extrainfo_dump_to_string_rsa_sig_helper(chunks, ident_key, s); + if (rv < 0) goto err; - } - smartlist_add_strdup(chunks, sig); + tor_free(s); s = smartlist_join_strings(chunks, "", 0, NULL);
@@ -3329,7 +3428,6 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, SMARTLIST_FOREACH(chunks, char *, chunk, tor_free(chunk)); smartlist_free(chunks); tor_free(s_dup); - tor_free(ed_cert_line); extrainfo_free(ei_tmp);
return result;