From 8356cc5b514a5d6a0170927d7c3730e2e1164298 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 24 Jun 2019 19:44:24 +1000 Subject: [PATCH 1/2] stats: Always publish pluggable transports in extra info documents Always publish bridge pluggable transport information in the extra info descriptor, even if ExtraInfoStatistics is 0. This information is needed by BridgeDB. Fixes bug 30956; bugfix on 0.4.1.1-alpha. --- changes/bug30956 | 4 ++++ doc/tor.1.txt | 6 ++++-- src/feature/relay/router.c | 17 +++++++++++------ 3 files changed, 19 insertions(+), 8 deletions(-) create mode 100644 changes/bug30956 diff --git a/changes/bug30956 b/changes/bug30956 new file mode 100644 index 0000000000..8f52a81de3 --- /dev/null +++ b/changes/bug30956 @@ -0,0 +1,4 @@ + o Minor bugfixes (pluggable transports): + - Always publish bridge pluggable transport information in the extra info + descriptor, even if ExtraInfoStatistics is 0. This information is + needed by BridgeDB. Fixes bug 30956; bugfix on 0.4.1.1-alpha. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 4bd365c774..95d56c6dbd 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2451,8 +2451,10 @@ is non-zero): [[ExtraInfoStatistics]] **ExtraInfoStatistics** **0**|**1**:: When this option is enabled, Tor includes previously gathered statistics in its extra-info documents that it uploads to the directory authorities. - Disabling this option also disables bandwidth usage statistics, GeoIPFile - hashes, and ServerTransportPlugin lists in the extra-info file. + Disabling this option also removes bandwidth usage statistics, and + GeoIPFile and GeoIPv6File hashes from the extra-info file. Bridge + ServerTransportPlugin lines are always includes in the extra-info file, + because they are required by BridgeDB. (Default: 1) [[ExtendAllowPrivateAddresses]] **ExtendAllowPrivateAddresses** **0**|**1**:: diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index e0daf34db2..25bb1835c2 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -3175,6 +3175,15 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, published); smartlist_add(chunks, pre); + /* Add information about the pluggable transports we support, even if we + * are not publishing statistics. This information is needed by BridgeDB + * to distribute bridges. */ + if (options->ServerTransportPlugin) { + char *pluggable_transports = pt_get_extra_info_descriptor_string(); + if (pluggable_transports) + smartlist_add(chunks, pluggable_transports); + } + if (options->ExtraInfoStatistics && write_stats_to_extrainfo) { log_info(LD_GENERAL, "Adding stats to extra-info descriptor."); /* Bandwidth usage stats don't have their own option */ @@ -3182,6 +3191,7 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, contents = rep_hist_get_bandwidth_lines(); smartlist_add(chunks, contents); } + /* geoip hashes aren't useful unless we are publishing other stats */ if (geoip_is_loaded(AF_INET)) smartlist_add_asprintf(chunks, "geoip-db-digest %s\n", geoip_db_digest(AF_INET)); @@ -3223,12 +3233,7 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, if (contents) smartlist_add(chunks, contents); } - /* Add information about the pluggable transports we support. */ - if (options->ServerTransportPlugin) { - char *pluggable_transports = pt_get_extra_info_descriptor_string(); - if (pluggable_transports) - smartlist_add(chunks, pluggable_transports); - } + /* bridge statistics */ if (should_record_bridge_info(options)) { const char *bridge_stats = geoip_get_bridge_stats_extrainfo(now); if (bridge_stats) { From 45be44ed9c1b3ddbe2ad4caaebc3ae0076bdbd07 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 24 Jun 2019 20:32:38 +1000 Subject: [PATCH 2/2] 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(-) create mode 100644 changes/ticket30956_refactor diff --git a/changes/ticket30956_refactor b/changes/ticket30956_refactor new file mode 100644 index 0000000000..81151c6cc9 --- /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 815d0ed097..4db452b897 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 25bb1835c2..6b33265294 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 extrainfo, to * *s_out, signing them - * with ident_key. - * - * 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 extrainfo, to * *s_out, signing them + * with ident_key. + * + * 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;