From a798bd40fb108f83bcd3dea5c8fa8a60dbbb9fe2 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 8 Jan 2019 18:40:03 +1000 Subject: [PATCH 01/17] stats: Stop reporting statistics when ExtraInfoStatistics is 0 When ExtraInfoStatistics is 0, stop including bandwidth usage statistics, GeoIPFile hashes, ServerTransportPlugin lines, and bridge statistics by country in extra-info documents. Fixes bug 29018; bugfix on 0.2.4.1-alpha (and earlier versions). --- changes/bug29018 | 5 ++++ doc/tor.1.txt | 5 +++- src/feature/relay/router.c | 49 +++++++++++++++++++------------------- 3 files changed, 33 insertions(+), 26 deletions(-) create mode 100644 changes/bug29018 diff --git a/changes/bug29018 b/changes/bug29018 new file mode 100644 index 0000000000..b006ae36a7 --- /dev/null +++ b/changes/bug29018 @@ -0,0 +1,5 @@ + o Minor bugfixes (stats): + - When ExtraInfoStatistics is 0, stop including bandwidth usage statistics, + GeoIPFile hashes, ServerTransportPlugin lines, and bridge statistics + by country in extra-info documents. Fixes bug 29018; + bugfix on 0.2.4.1-alpha. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 52f5bfa0c3..d94e0dcac2 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -2313,7 +2313,8 @@ is non-zero): When this option is enabled and BridgeRelay is also enabled, and we have GeoIP data, Tor keeps a per-country count of how many client addresses have contacted it so that it can help the bridge authority guess - which countries have blocked access to it. (Default: 1) + which countries have blocked access to it. If ExtraInfoStatistics is + enabled, it will be published as part of extra-info document. (Default: 1) [[ServerDNSRandomizeCase]] **ServerDNSRandomizeCase** **0**|**1**:: When this option is set, Tor sets the case of each character randomly in @@ -2395,6 +2396,8 @@ 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. (Default: 1) [[ExtendAllowPrivateAddresses]] **ExtendAllowPrivateAddresses** **0**|**1**:: diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index cdd032f78d..93bc8c96cb 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2942,7 +2942,6 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, char identity[HEX_DIGEST_LEN+1]; char published[ISO_TIME_LEN+1]; char digest[DIGEST_LEN]; - char *bandwidth_usage; int result; static int write_stats_to_extrainfo = 1; char sig[DIROBJ_MAX_SIG_LEN+1]; @@ -2957,7 +2956,6 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, 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(); if (emit_ed_sigs) { if (!extrainfo->cache_info.signing_key_cert->signing_key_included || !ed25519_pubkey_eq(&extrainfo->cache_info.signing_key_cert->signed_key, @@ -2983,21 +2981,25 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, ed_cert_line = tor_strdup(""); } - tor_asprintf(&pre, "extra-info %s %s\n%spublished %s\n%s", + tor_asprintf(&pre, "extra-info %s %s\n%spublished %s\n", extrainfo->nickname, identity, ed_cert_line, - published, bandwidth_usage); + published); smartlist_add(chunks, pre); - if (geoip_is_loaded(AF_INET)) - smartlist_add_asprintf(chunks, "geoip-db-digest %s\n", - geoip_db_digest(AF_INET)); - if (geoip_is_loaded(AF_INET6)) - smartlist_add_asprintf(chunks, "geoip6-db-digest %s\n", - geoip_db_digest(AF_INET6)); - 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 */ + { + contents = rep_hist_get_bandwidth_lines(); + smartlist_add(chunks, contents); + } + if (geoip_is_loaded(AF_INET)) + smartlist_add_asprintf(chunks, "geoip-db-digest %s\n", + geoip_db_digest(AF_INET)); + if (geoip_is_loaded(AF_INET6)) + smartlist_add_asprintf(chunks, "geoip6-db-digest %s\n", + geoip_db_digest(AF_INET6)); if (options->DirReqStatistics && load_stats_file("stats"PATH_SEPARATOR"dirreq-stats", "dirreq-stats-end", now, &contents) > 0) { @@ -3033,19 +3035,17 @@ 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); - } - - if (should_record_bridge_info(options) && write_stats_to_extrainfo) { - const char *bridge_stats = geoip_get_bridge_stats_extrainfo(now); - if (bridge_stats) { - smartlist_add_strdup(chunks, bridge_stats); + /* 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); + } + if (should_record_bridge_info(options)) { + const char *bridge_stats = geoip_get_bridge_stats_extrainfo(now); + if (bridge_stats) { + smartlist_add_strdup(chunks, bridge_stats); + } } } @@ -3139,7 +3139,6 @@ extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo, tor_free(s_dup); tor_free(ed_cert_line); extrainfo_free(ei_tmp); - tor_free(bandwidth_usage); return result; } From 6c5a506cdb887a655d8a4654fba5f6ea466aaeae Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 17:11:26 +1000 Subject: [PATCH 02/17] router: split router_build_fresh_descriptor() into static functions Split the body of router_build_fresh_descriptor() into static functions, by inserting function prologues and epilogues between existing sections. Write a new body for router_build_fresh_descriptor() that calls the new static functions. Initial refactor with no changes to the body of the old router_build_fresh_descriptor(), except for the split. Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 133 ++++++++++++++++++++++++++++++++++--- 1 file changed, 124 insertions(+), 9 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 93bc8c96cb..e57e9fb8d9 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -1941,18 +1941,15 @@ get_my_declared_family(const or_options_t *options) return result; } -/** Build a fresh routerinfo, signed server descriptor, and extra-info document - * for this OR. Set r to the generated routerinfo, e to the generated - * extra-info document. Return 0 on success, -1 on temporary error. Failure to - * generate an extra-info document is not an error and is indicated by setting - * e to NULL. Caller is responsible for freeing generated documents if 0 is - * returned. +/** Build a fresh routerinfo for this OR, without any of the fields that depend + * on the corresponding extrainfo. Set r to the generated routerinfo. + * Return 0 on success, -1 on temporary error. Caller is responsible for + * freeing the generated routerinfo if 0 is returned. */ -int -router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) +static int +router_build_fresh_routerinfo(routerinfo_t **r) { routerinfo_t *ri; - extrainfo_t *ei; uint32_t addr; char platform[256]; int hibernating = we_are_hibernating(); @@ -2057,6 +2054,21 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) ri->declared_family = get_my_declared_family(options); + *r = ri; + return 0; +} + +/** Build an extrainfo for this OR, based on the routerinfo ri. Set e to the + * generated extrainfo. Return 0 on success, -1 on temporary error. Failure to + * generate an extrainfo is not an error and is indicated by setting e to + * NULL. Caller is responsible for freeing the generated extrainfo if 0 is + * returned. + */ +static int +router_build_fresh_extrainfo(const routerinfo_t *ri, extrainfo_t **e) +{ + extrainfo_t *ei; + /* Now generate the extrainfo. */ ei = tor_malloc_zero(sizeof(extrainfo_t)); ei->cache_info.is_extrainfo = 1; @@ -2067,6 +2079,18 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest, DIGEST_LEN); + *e = ei; + return 0; +} + +/** Create a signed descriptor for ei, and add it to ei->cache_info. + * Return ei on success, free ei and return NULL on temporary error. + * Caller is responsible for freeing the returned extrainfo + * (if it is not NULL), including any extra fields set in ei->cache_info. + */ +static extrainfo_t * +router_update_extrainfo_descriptor_body(extrainfo_t *ei) +{ if (extrainfo_dump_to_string(&ei->cache_info.signed_descriptor_body, ei, get_server_identity_key(), get_master_signing_keypair()) < 0) { @@ -2085,6 +2109,15 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) DIGEST_SHA256); } + return ei; +} + +/** If ei is not NULL, set the fields in ri that depend on ei. + */ +static void +router_update_routerinfo_from_extrainfo(routerinfo_t *ri, + const extrainfo_t *ei) +{ /* Now finish the router descriptor. */ if (ei) { memcpy(ri->cache_info.extra_info_digest, @@ -2097,6 +2130,17 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) /* ri was allocated with tor_malloc_zero, so there is no need to * zero ri->cache_info.extra_info_digest here. */ } +} + +/** Create a signed descriptor for ri, and add it to ri->cache_info. + * Return 0 on success, free ri and ei and return -1 on temporary error. + * TODO: freeing ri and ei, but leaving dangling pointers, is a bad interface. + * Caller is responsible for freeing the generated ri and ei if 0 is returned, + * including any extra fields set in ri->cache_info. + */ +static int +router_update_routerinfo_descriptor_body(routerinfo_t *ri, extrainfo_t *ei) +{ if (! (ri->cache_info.signed_descriptor_body = router_dump_router_to_string(ri, get_server_identity_key(), get_onion_key(), @@ -2110,8 +2154,27 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) ri->cache_info.signed_descriptor_len = strlen(ri->cache_info.signed_descriptor_body); + return 0; +} + +/** Set the purpose field in ri. + */ +static void +router_update_routerinfo_purpose(routerinfo_t *ri) +{ + const or_options_t *options = get_options(); + ri->purpose = options->BridgeRelay ? ROUTER_PURPOSE_BRIDGE : ROUTER_PURPOSE_GENERAL; +} + +/** Set the cache_info.send_unencrypted fields in ri and ei. + */ +static void +router_update_info_send_unencrypted(routerinfo_t *ri, extrainfo_t *ei) +{ + const or_options_t *options = get_options(); + if (options->BridgeRelay) { /* Bridges shouldn't be able to send their descriptors unencrypted, anyway, since they don't have a DirPort, and always connect to the @@ -2125,10 +2188,62 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (ei) ei->cache_info.send_unencrypted = 1; } +} +/** Set signed_descriptor_digest in ri->cache_info. + */ +static void +router_update_routerinfo_digest(routerinfo_t *ri) +{ router_get_router_hash(ri->cache_info.signed_descriptor_body, strlen(ri->cache_info.signed_descriptor_body), ri->cache_info.signed_descriptor_digest); +} + +/** Build a fresh routerinfo, signed server descriptor, and extra-info document + * for this OR. Set r to the generated routerinfo, e to the generated + * extra-info document. Return 0 on success, -1 on temporary error. Failure to + * generate an extra-info document is not an error and is indicated by setting + * e to NULL. Caller is responsible for freeing generated documents if 0 is + * returned. + */ +int +router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) +{ + int result = -1; + routerinfo_t *ri = NULL; + extrainfo_t *ei = NULL; + + /* TODO: return ri */ + result = router_build_fresh_routerinfo(&ri); + if (result < 0) + return result; + + /* TODO: return ei */ + result = router_build_fresh_extrainfo(ri, &ei); + if (result < 0) + return result; + + /* TODO: this function frees ei on failure, instead, goto err */ + ei = router_update_extrainfo_descriptor_body(ei); + + /* TODO: don't rely on tor_malloc_zero */ + router_update_routerinfo_from_extrainfo(ri, ei); + + /* TODO: this function frees ri and ei on failure, instead, goto err */ + result = router_update_routerinfo_descriptor_body(ri, ei); + if (result < 0) + return result; + + /* TODO: fold into router_build_fresh_routerinfo() */ + router_update_routerinfo_purpose(ri); + + /* TODO: fold into router_update_extrainfo_descriptor_body() and + * router_update_routerinfo_descriptor_body() ? */ + router_update_info_send_unencrypted(ri, ei); + + /* TODO: fold into router_update_routerinfo_descriptor_body() */ + router_update_routerinfo_digest(ri); if (ei) { tor_assert(! From f19b64dce90c082b0e19f059b94c2d42b015a956 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 19:47:24 +1000 Subject: [PATCH 03/17] router: refactor router_build_fresh_descriptor() static function interfaces Tidy the arguments and return values of these functions, and clean up their memory management. Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 169 ++++++++++++++++++++++++------------- src/feature/relay/router.h | 1 + 2 files changed, 112 insertions(+), 58 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index e57e9fb8d9..d9242448c9 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -152,6 +152,8 @@ routerinfo_err_to_string(int err) return "Cannot generate descriptor"; case TOR_ROUTERINFO_ERROR_DESC_REBUILDING: return "Descriptor still rebuilding - not ready yet"; + case TOR_ROUTERINFO_ERROR_INTERNAL_BUG: + return "Internal bug, see logs for details"; } log_warn(LD_BUG, "unknown routerinfo error %d - shouldn't happen", err); @@ -1941,23 +1943,33 @@ get_my_declared_family(const or_options_t *options) return result; } -/** Build a fresh routerinfo for this OR, without any of the fields that depend - * on the corresponding extrainfo. Set r to the generated routerinfo. - * Return 0 on success, -1 on temporary error. Caller is responsible for - * freeing the generated routerinfo if 0 is returned. +/** Allocate a fresh, unsigned routerinfo for this OR, without any of the + * fields that depend on the corresponding extrainfo. + * + * On success, set ri_out to the new routerinfo, and return 0. + * Caller is responsible for freeing the generated routerinfo. + * + * Returns a negative value and sets ri_out to NULL on temporary error. */ static int -router_build_fresh_routerinfo(routerinfo_t **r) +router_build_fresh_routerinfo(routerinfo_t **ri_out) { - routerinfo_t *ri; + routerinfo_t *ri = NULL; uint32_t addr; char platform[256]; int hibernating = we_are_hibernating(); const or_options_t *options = get_options(); + int result = TOR_ROUTERINFO_ERROR_INTERNAL_BUG; + + if (BUG(!ri_out)) { + result = TOR_ROUTERINFO_ERROR_INTERNAL_BUG; + goto err; + } if (router_pick_published_address(options, &addr, 0) < 0) { log_warn(LD_CONFIG, "Don't know my address while generating descriptor"); - return TOR_ROUTERINFO_ERROR_NO_EXT_ADDR; + result = TOR_ROUTERINFO_ERROR_NO_EXT_ADDR; + goto err; } /* Log a message if the address in the descriptor doesn't match the ORPort @@ -2014,8 +2026,8 @@ router_build_fresh_routerinfo(routerinfo_t **r) ri->identity_pkey = crypto_pk_dup_key(get_server_identity_key()); if (BUG(crypto_pk_get_digest(ri->identity_pkey, ri->cache_info.identity_digest) < 0)) { - routerinfo_free(ri); - return TOR_ROUTERINFO_ERROR_DIGEST_FAILED; + result = TOR_ROUTERINFO_ERROR_DIGEST_FAILED; + goto err; } ri->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); @@ -2054,20 +2066,26 @@ router_build_fresh_routerinfo(routerinfo_t **r) ri->declared_family = get_my_declared_family(options); - *r = ri; + goto done; + + err: + routerinfo_free(ri); + *ri_out = NULL; + return result; + + done: + *ri_out = ri; return 0; } -/** Build an extrainfo for this OR, based on the routerinfo ri. Set e to the - * generated extrainfo. Return 0 on success, -1 on temporary error. Failure to - * generate an extrainfo is not an error and is indicated by setting e to - * NULL. Caller is responsible for freeing the generated extrainfo if 0 is - * returned. +/** Allocate and return an extrainfo for this OR, based on the routerinfo ri. + * + * Caller is responsible for freeing the generated extrainfo. */ -static int -router_build_fresh_extrainfo(const routerinfo_t *ri, extrainfo_t **e) +static extrainfo_t * +router_build_fresh_extrainfo(const routerinfo_t *ri) { - extrainfo_t *ei; + extrainfo_t *ei = NULL; /* Now generate the extrainfo. */ ei = tor_malloc_zero(sizeof(extrainfo_t)); @@ -2079,24 +2097,23 @@ router_build_fresh_extrainfo(const routerinfo_t *ri, extrainfo_t **e) memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest, DIGEST_LEN); - *e = ei; - return 0; + + return ei; } /** Create a signed descriptor for ei, and add it to ei->cache_info. - * Return ei on success, free ei and return NULL on temporary error. - * Caller is responsible for freeing the returned extrainfo - * (if it is not NULL), including any extra fields set in ei->cache_info. + * + * Return 0 on success, -1 on temporary error. + * On error, ei->cache_info is not modified. */ -static extrainfo_t * +static int router_update_extrainfo_descriptor_body(extrainfo_t *ei) { if (extrainfo_dump_to_string(&ei->cache_info.signed_descriptor_body, ei, get_server_identity_key(), get_master_signing_keypair()) < 0) { log_warn(LD_BUG, "Couldn't generate extra-info descriptor."); - extrainfo_free(ei); - ei = NULL; + return -1; } else { ei->cache_info.signed_descriptor_len = strlen(ei->cache_info.signed_descriptor_body); @@ -2107,12 +2124,11 @@ router_update_extrainfo_descriptor_body(extrainfo_t *ei) ei->cache_info.signed_descriptor_body, ei->cache_info.signed_descriptor_len, DIGEST_SHA256); + return 0; } - - return ei; } -/** If ei is not NULL, set the fields in ri that depend on ei. +/** Set the fields in ri that depend on ei. */ static void router_update_routerinfo_from_extrainfo(routerinfo_t *ri, @@ -2133,24 +2149,26 @@ router_update_routerinfo_from_extrainfo(routerinfo_t *ri, } /** Create a signed descriptor for ri, and add it to ri->cache_info. - * Return 0 on success, free ri and ei and return -1 on temporary error. - * TODO: freeing ri and ei, but leaving dangling pointers, is a bad interface. - * Caller is responsible for freeing the generated ri and ei if 0 is returned, - * including any extra fields set in ri->cache_info. + * + * Return 0 on success, and a negative value on temporary error. + * If ri is NULL, logs a BUG() warning and returns a negative value. + * On error, ri->cache_info is not modified. */ static int -router_update_routerinfo_descriptor_body(routerinfo_t *ri, extrainfo_t *ei) +router_update_routerinfo_descriptor_body(routerinfo_t *ri) { + if (BUG(!ri)) + return TOR_ROUTERINFO_ERROR_INTERNAL_BUG; + if (! (ri->cache_info.signed_descriptor_body = router_dump_router_to_string(ri, get_server_identity_key(), get_onion_key(), get_current_curve25519_keypair(), get_master_signing_keypair())) ) { log_warn(LD_BUG, "Couldn't generate router descriptor."); - routerinfo_free(ri); - extrainfo_free(ei); return TOR_ROUTERINFO_ERROR_CANNOT_GENERATE; } + ri->cache_info.signed_descriptor_len = strlen(ri->cache_info.signed_descriptor_body); @@ -2200,40 +2218,63 @@ router_update_routerinfo_digest(routerinfo_t *ri) ri->cache_info.signed_descriptor_digest); } -/** Build a fresh routerinfo, signed server descriptor, and extra-info document - * for this OR. Set r to the generated routerinfo, e to the generated - * extra-info document. Return 0 on success, -1 on temporary error. Failure to - * generate an extra-info document is not an error and is indicated by setting - * e to NULL. Caller is responsible for freeing generated documents if 0 is - * returned. +/** Build a fresh routerinfo, signed server descriptor, and signed extra-info + * document for this OR. + * + * Set r to the generated routerinfo, e to the generated extra-info document. + * Failure to generate an extra-info document is not an error and is indicated + * by setting e to NULL. + * Return 0 on success, and a negative value on temporary error. + * Caller is responsible for freeing generated documents on success. */ int router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) { - int result = -1; + int result = TOR_ROUTERINFO_ERROR_INTERNAL_BUG; routerinfo_t *ri = NULL; extrainfo_t *ei = NULL; - /* TODO: return ri */ + if (BUG(!r)) + goto err; + + if (BUG(!e)) + goto err; + result = router_build_fresh_routerinfo(&ri); - if (result < 0) - return result; + if (result < 0) { + goto err; + } + /* If ri is NULL, then result should be negative. So this check should be + * unreachable. */ + if (BUG(!ri)) { + result = TOR_ROUTERINFO_ERROR_INTERNAL_BUG; + goto err; + } - /* TODO: return ei */ - result = router_build_fresh_extrainfo(ri, &ei); - if (result < 0) - return result; + ei = router_build_fresh_extrainfo(ri); + /* Failing to create an ei is not an error, but at this stage, + * router_build_fresh_extrainfo() should not fail. */ + if (BUG(!ei)) + goto skip_ei; - /* TODO: this function frees ei on failure, instead, goto err */ - ei = router_update_extrainfo_descriptor_body(ei); + result = router_update_extrainfo_descriptor_body(ei); + if (result < 0) + goto skip_ei; /* TODO: don't rely on tor_malloc_zero */ router_update_routerinfo_from_extrainfo(ri, ei); - /* TODO: this function frees ri and ei on failure, instead, goto err */ - result = router_update_routerinfo_descriptor_body(ri, ei); + /* TODO: disentangle these GOTOs, or split into another function. */ + goto ei_ok; + + skip_ei: + extrainfo_free(ei); + ei = NULL; + + ei_ok: + result = router_update_routerinfo_descriptor_body(ri); if (result < 0) - return result; + goto err; /* TODO: fold into router_build_fresh_routerinfo() */ router_update_routerinfo_purpose(ri); @@ -2246,11 +2287,23 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) router_update_routerinfo_digest(ri); if (ei) { - tor_assert(! - routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, - &ri->cache_info, NULL)); + if (BUG(routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, + &ri->cache_info, NULL))) { + result = TOR_ROUTERINFO_ERROR_INTERNAL_BUG; + goto err; + } } + goto done; + + err: + routerinfo_free(ri); + extrainfo_free(ei); + *r = NULL; + *e = NULL; + return result; + + done: *r = ri; *e = ei; return 0; diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index 60bc857ceb..46364206ef 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -23,6 +23,7 @@ struct ed25519_keypair_t; #define TOR_ROUTERINFO_ERROR_DIGEST_FAILED (-4) #define TOR_ROUTERINFO_ERROR_CANNOT_GENERATE (-5) #define TOR_ROUTERINFO_ERROR_DESC_REBUILDING (-6) +#define TOR_ROUTERINFO_ERROR_INTERNAL_BUG (-7) crypto_pk_t *get_onion_key(void); time_t get_onion_key_set_at(void); From a65c101973f0b0dc7380470edff4f590b58c39d3 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 19:49:46 +1000 Subject: [PATCH 04/17] router: check for NULL in router_build_fresh_descriptor() static functions Make sure that these static functions aren't passed NULL. If they are, log a BUG() warning, and return an error. Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index d9242448c9..9aa4d56a4c 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2080,6 +2080,7 @@ router_build_fresh_routerinfo(routerinfo_t **ri_out) /** Allocate and return an extrainfo for this OR, based on the routerinfo ri. * + * If ri is NULL, logs a BUG() warning and returns NULL. * Caller is responsible for freeing the generated extrainfo. */ static extrainfo_t * @@ -2087,6 +2088,9 @@ router_build_fresh_extrainfo(const routerinfo_t *ri) { extrainfo_t *ei = NULL; + if (BUG(!ri)) + return NULL; + /* Now generate the extrainfo. */ ei = tor_malloc_zero(sizeof(extrainfo_t)); ei->cache_info.is_extrainfo = 1; @@ -2104,11 +2108,15 @@ router_build_fresh_extrainfo(const routerinfo_t *ri) /** Create a signed descriptor for ei, and add it to ei->cache_info. * * Return 0 on success, -1 on temporary error. + * If ei is NULL, logs a BUG() warning and returns -1. * On error, ei->cache_info is not modified. */ static int router_update_extrainfo_descriptor_body(extrainfo_t *ei) { + if (BUG(!ei)) + return -1; + if (extrainfo_dump_to_string(&ei->cache_info.signed_descriptor_body, ei, get_server_identity_key(), get_master_signing_keypair()) < 0) { @@ -2129,23 +2137,27 @@ router_update_extrainfo_descriptor_body(extrainfo_t *ei) } /** Set the fields in ri that depend on ei. + * + * If ei is NULL, logs a BUG() warning and zeroes the relevant fields. */ static void router_update_routerinfo_from_extrainfo(routerinfo_t *ri, const extrainfo_t *ei) { - /* Now finish the router descriptor. */ - if (ei) { - memcpy(ri->cache_info.extra_info_digest, - ei->cache_info.signed_descriptor_digest, - DIGEST_LEN); - memcpy(ri->cache_info.extra_info_digest256, - ei->digest256, - DIGEST256_LEN); - } else { - /* ri was allocated with tor_malloc_zero, so there is no need to - * zero ri->cache_info.extra_info_digest here. */ + if (BUG(!ei)) { + /* Just to be safe, zero ri->cache_info.extra_info_digest* here. */ + memset(ri->cache_info.extra_info_digest, 0, DIGEST_LEN); + memset(ri->cache_info.extra_info_digest256, 0, DIGEST256_LEN); + return; } + + /* Now finish the router descriptor. */ + memcpy(ri->cache_info.extra_info_digest, + ei->cache_info.signed_descriptor_digest, + DIGEST_LEN); + memcpy(ri->cache_info.extra_info_digest256, + ei->digest256, + DIGEST256_LEN); } /** Create a signed descriptor for ri, and add it to ri->cache_info. @@ -2261,7 +2273,6 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (result < 0) goto skip_ei; - /* TODO: don't rely on tor_malloc_zero */ router_update_routerinfo_from_extrainfo(ri, ei); /* TODO: disentangle these GOTOs, or split into another function. */ From af0a43be2cf3e96ee9ac8e1f92c11aa5d5f6290f Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 20:01:28 +1000 Subject: [PATCH 05/17] router: eliminate tiny router_build_fresh_descriptor() static functions Remove some tiny static functions called by router_build_fresh_descriptor(), and move their code into more relevant functions. Then, give router_update_{router,extra}info_descriptor_body identical layouts. Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 58 +++++++++++++------------------------- 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 9aa4d56a4c..9a5e8b74ac 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2066,6 +2066,9 @@ router_build_fresh_routerinfo(routerinfo_t **ri_out) ri->declared_family = get_my_declared_family(options); + ri->purpose = + options->BridgeRelay ? ROUTER_PURPOSE_BRIDGE : ROUTER_PURPOSE_GENERAL; + goto done; err: @@ -2122,18 +2125,20 @@ router_update_extrainfo_descriptor_body(extrainfo_t *ei) get_master_signing_keypair()) < 0) { log_warn(LD_BUG, "Couldn't generate extra-info descriptor."); return -1; - } 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_len, - ei->cache_info.signed_descriptor_digest); - crypto_digest256((char*) ei->digest256, - ei->cache_info.signed_descriptor_body, - ei->cache_info.signed_descriptor_len, - DIGEST_SHA256); - return 0; } + + 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_len, + ei->cache_info.signed_descriptor_digest); + crypto_digest256((char*) ei->digest256, + ei->cache_info.signed_descriptor_body, + ei->cache_info.signed_descriptor_len, + DIGEST_SHA256); + + return 0; } /** Set the fields in ri that depend on ei. @@ -2184,20 +2189,13 @@ router_update_routerinfo_descriptor_body(routerinfo_t *ri) ri->cache_info.signed_descriptor_len = strlen(ri->cache_info.signed_descriptor_body); + router_get_router_hash(ri->cache_info.signed_descriptor_body, + strlen(ri->cache_info.signed_descriptor_body), + ri->cache_info.signed_descriptor_digest); + return 0; } -/** Set the purpose field in ri. - */ -static void -router_update_routerinfo_purpose(routerinfo_t *ri) -{ - const or_options_t *options = get_options(); - - ri->purpose = - options->BridgeRelay ? ROUTER_PURPOSE_BRIDGE : ROUTER_PURPOSE_GENERAL; -} - /** Set the cache_info.send_unencrypted fields in ri and ei. */ static void @@ -2220,16 +2218,6 @@ router_update_info_send_unencrypted(routerinfo_t *ri, extrainfo_t *ei) } } -/** Set signed_descriptor_digest in ri->cache_info. - */ -static void -router_update_routerinfo_digest(routerinfo_t *ri) -{ - router_get_router_hash(ri->cache_info.signed_descriptor_body, - strlen(ri->cache_info.signed_descriptor_body), - ri->cache_info.signed_descriptor_digest); -} - /** Build a fresh routerinfo, signed server descriptor, and signed extra-info * document for this OR. * @@ -2287,16 +2275,10 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (result < 0) goto err; - /* TODO: fold into router_build_fresh_routerinfo() */ - router_update_routerinfo_purpose(ri); - /* TODO: fold into router_update_extrainfo_descriptor_body() and * router_update_routerinfo_descriptor_body() ? */ router_update_info_send_unencrypted(ri, ei); - /* TODO: fold into router_update_routerinfo_descriptor_body() */ - router_update_routerinfo_digest(ri); - if (ei) { if (BUG(routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, &ri->cache_info, NULL))) { From 9cab988696b6f86f9b3b744c5694bee7d2a6cf70 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 20:19:02 +1000 Subject: [PATCH 06/17] router: eliminate router_update_info_send_unencrypted() Remove router_update_info_send_unencrypted(), and move its code into the relevant functions. Then, re-use an options pointer. Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 9a5e8b74ac..03af9aacd4 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2066,8 +2066,17 @@ router_build_fresh_routerinfo(routerinfo_t **ri_out) ri->declared_family = get_my_declared_family(options); - ri->purpose = - options->BridgeRelay ? ROUTER_PURPOSE_BRIDGE : ROUTER_PURPOSE_GENERAL; + if (options->BridgeRelay) { + ri->purpose = ROUTER_PURPOSE_BRIDGE; + /* Bridges shouldn't be able to send their descriptors unencrypted, + anyway, since they don't have a DirPort, and always connect to the + bridge authority anonymously. But just in case they somehow think of + sending them on an unencrypted connection, don't allow them to try. */ + ri->cache_info.send_unencrypted = 0; + } else { + ri->purpose = ROUTER_PURPOSE_GENERAL; + ri->cache_info.send_unencrypted = 1; + } goto done; @@ -2090,6 +2099,7 @@ static extrainfo_t * router_build_fresh_extrainfo(const routerinfo_t *ri) { extrainfo_t *ei = NULL; + const or_options_t *options = get_options(); if (BUG(!ri)) return NULL; @@ -2097,7 +2107,7 @@ router_build_fresh_extrainfo(const routerinfo_t *ri) /* Now generate the extrainfo. */ ei = tor_malloc_zero(sizeof(extrainfo_t)); ei->cache_info.is_extrainfo = 1; - strlcpy(ei->nickname, get_options()->Nickname, sizeof(ei->nickname)); + strlcpy(ei->nickname, options->Nickname, sizeof(ei->nickname)); ei->cache_info.published_on = ri->cache_info.published_on; ei->cache_info.signing_key_cert = tor_cert_dup(get_master_signing_key_cert()); @@ -2105,6 +2115,13 @@ router_build_fresh_extrainfo(const routerinfo_t *ri) memcpy(ei->cache_info.identity_digest, ri->cache_info.identity_digest, DIGEST_LEN); + if (options->BridgeRelay) { + /* See note in router_build_fresh_routerinfo(). */ + ei->cache_info.send_unencrypted = 0; + } else { + ei->cache_info.send_unencrypted = 1; + } + return ei; } @@ -2275,10 +2292,6 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (result < 0) goto err; - /* TODO: fold into router_update_extrainfo_descriptor_body() and - * router_update_routerinfo_descriptor_body() ? */ - router_update_info_send_unencrypted(ri, ei); - if (ei) { if (BUG(routerinfo_incompatible_with_extrainfo(ri->identity_pkey, ei, &ri->cache_info, NULL))) { From a1f8558628881216917814989d293a028f9e48d7 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 20:39:10 +1000 Subject: [PATCH 07/17] router: Move extrainfo signing into its own function This refactoring improves the structure of router_build_fresh_descriptor(). Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 57 ++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 03af9aacd4..0e872663f8 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2158,6 +2158,40 @@ router_update_extrainfo_descriptor_body(extrainfo_t *ei) return 0; } +/** Allocate and return a fresh, signed extrainfo for this OR, based on the + * routerinfo ri. + * + * If ri is NULL, logs a BUG() warning and returns NULL. + * Caller is responsible for freeing the generated extrainfo. + */ +static extrainfo_t * +router_build_signed_extrainfo(const routerinfo_t *ri) +{ + int result = -1; + extrainfo_t *ei = NULL; + + if (BUG(!ri)) + return NULL; + + ei = router_build_fresh_extrainfo(ri); + /* router_build_fresh_extrainfo() should not fail. */ + if (BUG(!ei)) + goto err; + + result = router_update_extrainfo_descriptor_body(ei); + if (result < 0) + goto err; + + goto done; + + err: + extrainfo_free(ei); + return NULL; + + done: + return ei; +} + /** Set the fields in ri that depend on ei. * * If ei is NULL, logs a BUG() warning and zeroes the relevant fields. @@ -2268,26 +2302,13 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) goto err; } - ei = router_build_fresh_extrainfo(ri); - /* Failing to create an ei is not an error, but at this stage, - * router_build_fresh_extrainfo() should not fail. */ - if (BUG(!ei)) - goto skip_ei; + ei = router_build_signed_extrainfo(ri); - result = router_update_extrainfo_descriptor_body(ei); - if (result < 0) - goto skip_ei; + /* Failing to create an ei is not an error. */ + if (ei) { + router_update_routerinfo_from_extrainfo(ri, ei); + } - router_update_routerinfo_from_extrainfo(ri, ei); - - /* TODO: disentangle these GOTOs, or split into another function. */ - goto ei_ok; - - skip_ei: - extrainfo_free(ei); - ei = NULL; - - ei_ok: result = router_update_routerinfo_descriptor_body(ri); if (result < 0) goto err; From a9f852a0f65a24fc8c0e33caf82d1e3c845c23cf Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 20:47:37 +1000 Subject: [PATCH 08/17] router: Document the additional config and state used to dump descriptors Also, explicitly state when routerinfos and extra-infos are signed. And tidy up some other comments. Preparation for testing 29017 and 20918. --- src/feature/relay/router.c | 80 ++++++++++++++++++-------------------- src/feature/relay/router.h | 2 +- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 0e872663f8..7e2161ef31 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -1952,7 +1952,7 @@ get_my_declared_family(const or_options_t *options) * Returns a negative value and sets ri_out to NULL on temporary error. */ static int -router_build_fresh_routerinfo(routerinfo_t **ri_out) +router_build_fresh_unsigned_routerinfo(routerinfo_t **ri_out) { routerinfo_t *ri = NULL; uint32_t addr; @@ -2090,13 +2090,17 @@ router_build_fresh_routerinfo(routerinfo_t **ri_out) return 0; } -/** Allocate and return an extrainfo for this OR, based on the routerinfo ri. +/** Allocate and return a fresh, unsigned extrainfo for this OR, based on the + * routerinfo ri. + * + * Uses options->Nickname to set the nickname, and options->BridgeRelay to set + * ei->cache_info.send_unencrypted. * * If ri is NULL, logs a BUG() warning and returns NULL. * Caller is responsible for freeing the generated extrainfo. */ static extrainfo_t * -router_build_fresh_extrainfo(const routerinfo_t *ri) +router_build_fresh_unsigned_extrainfo(const routerinfo_t *ri) { extrainfo_t *ei = NULL; const or_options_t *options = get_options(); @@ -2125,14 +2129,17 @@ router_build_fresh_extrainfo(const routerinfo_t *ri) return ei; } -/** Create a signed descriptor for ei, and add it to ei->cache_info. +/** Dump the extrainfo descriptor body for ei, sign it, and add the body and + * signature to ei->cache_info. Note that the extrainfo body is determined by + * ei, and some additional config and statistics state: see + * extrainfo_dump_to_string() for details. * * Return 0 on success, -1 on temporary error. * If ei is NULL, logs a BUG() warning and returns -1. * On error, ei->cache_info is not modified. */ static int -router_update_extrainfo_descriptor_body(extrainfo_t *ei) +router_dump_and_sign_extrainfo_descriptor_body(extrainfo_t *ei) { if (BUG(!ei)) return -1; @@ -2165,7 +2172,7 @@ router_update_extrainfo_descriptor_body(extrainfo_t *ei) * Caller is responsible for freeing the generated extrainfo. */ static extrainfo_t * -router_build_signed_extrainfo(const routerinfo_t *ri) +router_build_fresh_signed_extrainfo(const routerinfo_t *ri) { int result = -1; extrainfo_t *ei = NULL; @@ -2173,12 +2180,12 @@ router_build_signed_extrainfo(const routerinfo_t *ri) if (BUG(!ri)) return NULL; - ei = router_build_fresh_extrainfo(ri); - /* router_build_fresh_extrainfo() should not fail. */ + ei = router_build_fresh_unsigned_extrainfo(ri); + /* router_build_fresh_unsigned_extrainfo() should not fail. */ if (BUG(!ei)) goto err; - result = router_update_extrainfo_descriptor_body(ei); + result = router_dump_and_sign_extrainfo_descriptor_body(ei); if (result < 0) goto err; @@ -2201,7 +2208,7 @@ router_update_routerinfo_from_extrainfo(routerinfo_t *ri, const extrainfo_t *ei) { if (BUG(!ei)) { - /* Just to be safe, zero ri->cache_info.extra_info_digest* here. */ + /* Just to be safe, zero ri->cache_info.extra_info_digest here. */ memset(ri->cache_info.extra_info_digest, 0, DIGEST_LEN); memset(ri->cache_info.extra_info_digest256, 0, DIGEST256_LEN); return; @@ -2216,14 +2223,16 @@ router_update_routerinfo_from_extrainfo(routerinfo_t *ri, DIGEST256_LEN); } -/** Create a signed descriptor for ri, and add it to ri->cache_info. +/** Dump the descriptor body for ri, sign it, and add the body and signature to + * ri->cache_info. Note that the descriptor body is determined by ri, and some + * additional config and state: see router_dump_router_to_string() for details. * * Return 0 on success, and a negative value on temporary error. * If ri is NULL, logs a BUG() warning and returns a negative value. * On error, ri->cache_info is not modified. */ static int -router_update_routerinfo_descriptor_body(routerinfo_t *ri) +router_dump_and_sign_routerinfo_descriptor_body(routerinfo_t *ri) { if (BUG(!ri)) return TOR_ROUTERINFO_ERROR_INTERNAL_BUG; @@ -2247,32 +2256,10 @@ router_update_routerinfo_descriptor_body(routerinfo_t *ri) return 0; } -/** Set the cache_info.send_unencrypted fields in ri and ei. - */ -static void -router_update_info_send_unencrypted(routerinfo_t *ri, extrainfo_t *ei) -{ - const or_options_t *options = get_options(); - - if (options->BridgeRelay) { - /* Bridges shouldn't be able to send their descriptors unencrypted, - anyway, since they don't have a DirPort, and always connect to the - bridge authority anonymously. But just in case they somehow think of - sending them on an unencrypted connection, don't allow them to try. */ - ri->cache_info.send_unencrypted = 0; - if (ei) - ei->cache_info.send_unencrypted = 0; - } else { - ri->cache_info.send_unencrypted = 1; - if (ei) - ei->cache_info.send_unencrypted = 1; - } -} - -/** Build a fresh routerinfo, signed server descriptor, and signed extra-info +/** Build a fresh routerinfo, signed server descriptor, and signed extrainfo * document for this OR. * - * Set r to the generated routerinfo, e to the generated extra-info document. + * Set r to the generated routerinfo, e to the generated extrainfo document. * Failure to generate an extra-info document is not an error and is indicated * by setting e to NULL. * Return 0 on success, and a negative value on temporary error. @@ -2291,7 +2278,7 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) if (BUG(!e)) goto err; - result = router_build_fresh_routerinfo(&ri); + result = router_build_fresh_unsigned_routerinfo(&ri); if (result < 0) { goto err; } @@ -2302,14 +2289,14 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) goto err; } - ei = router_build_signed_extrainfo(ri); + ei = router_build_fresh_signed_extrainfo(ri); /* Failing to create an ei is not an error. */ if (ei) { router_update_routerinfo_from_extrainfo(ri, ei); } - result = router_update_routerinfo_descriptor_body(ri); + result = router_dump_and_sign_routerinfo_descriptor_body(ri); if (result < 0) goto err; @@ -2673,6 +2660,10 @@ get_platform_str(char *platform, size_t len) /** OR only: Given a routerinfo for this router, and an identity key to sign * with, encode the routerinfo as a signed server descriptor and return a new * string encoding the result, or NULL on failure. + * + * In addition to the fields in router, this function calls + * onion_key_lifetime(), get_options(), and we_are_hibernating(), and uses the + * results to populate some fields in the descriptor. */ char * router_dump_router_to_string(routerinfo_t *router, @@ -3125,9 +3116,14 @@ load_stats_file(const char *filename, const char *end_line, time_t now, return r; } -/** Write the contents of extrainfo and aggregated statistics to - * *s_out, signing them with ident_key. Return 0 on - * success, negative on failure. */ +/** 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, diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index 46364206ef..497d8d243e 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -115,7 +115,7 @@ void router_reset_reachability(void); void router_free_all(void); #ifdef ROUTER_PRIVATE -/* Used only by router.c and test.c */ +/* Used only by router.c and the unit tests */ STATIC void get_platform_str(char *platform, size_t len); STATIC int router_write_fingerprint(int hashed); STATIC smartlist_t *get_my_declared_family(const or_options_t *options); From 7c9450fb073c8b5fb38dab826de7f0356c4828e2 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jan 2019 17:12:15 +1000 Subject: [PATCH 09/17] test_router: Add comment to explain mocking Add comment in test_router_dump_router_to_string_no_bridge_distribution_method to explain the effect of a mocked function. --- src/test/test_router.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/test_router.c b/src/test/test_router.c index ea0ee3e84c..5477ab51e9 100644 --- a/src/test/test_router.c +++ b/src/test/test_router.c @@ -100,6 +100,9 @@ test_router_dump_router_to_string_no_bridge_distribution_method(void *arg) router = (routerinfo_t*)router_get_my_routerinfo(); tt_ptr_op(router, !=, NULL); + /* The real router_get_my_routerinfo() looks up onion_curve25519_pkey using + * get_current_curve25519_keypair(), but we don't initialise static data in + * this test. */ router->onion_curve25519_pkey = &ntor_keypair.pubkey; /* Generate our server descriptor and ensure that the substring From 53b49d1a35d2e7abf1cc7aff15553c23dde0f352 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 18 Feb 2019 15:24:26 +1000 Subject: [PATCH 10/17] test_dir: Unit tests for RSA-only router and extrainfo descriptor creation Tests 29017 and 29018. --- src/feature/nodelist/torcert.c | 4 +- src/feature/nodelist/torcert.h | 2 +- src/feature/relay/router.c | 18 ++--- src/feature/relay/router.h | 12 ++- src/test/test_dir.c | 134 ++++++++++++++++++++++++++++++--- 5 files changed, 146 insertions(+), 24 deletions(-) diff --git a/src/feature/nodelist/torcert.c b/src/feature/nodelist/torcert.c index b0197e9f13..56f1a8ac9f 100644 --- a/src/feature/nodelist/torcert.c +++ b/src/feature/nodelist/torcert.c @@ -290,8 +290,8 @@ tor_cert_describe_signature_status(const tor_cert_t *cert) } /** Return a new copy of cert */ -tor_cert_t * -tor_cert_dup(const tor_cert_t *cert) +MOCK_IMPL(tor_cert_t *, +tor_cert_dup,(const tor_cert_t *cert)) { tor_cert_t *newcert = tor_memdup(cert, sizeof(tor_cert_t)); if (cert->encoded) diff --git a/src/feature/nodelist/torcert.h b/src/feature/nodelist/torcert.h index 492275b514..03d5bdca93 100644 --- a/src/feature/nodelist/torcert.h +++ b/src/feature/nodelist/torcert.h @@ -71,7 +71,7 @@ int tor_cert_checksig(tor_cert_t *cert, const ed25519_public_key_t *pubkey, time_t now); const char *tor_cert_describe_signature_status(const tor_cert_t *cert); -tor_cert_t *tor_cert_dup(const tor_cert_t *cert); +MOCK_DECL(tor_cert_t *,tor_cert_dup,(const tor_cert_t *cert)); int tor_cert_eq(const tor_cert_t *cert1, const tor_cert_t *cert2); int tor_cert_opt_eq(const tor_cert_t *cert1, const tor_cert_t *cert2); diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 7e2161ef31..18c8375299 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -196,8 +196,8 @@ set_onion_key(crypto_pk_t *k) /** Return the current onion key. Requires that the onion key has been * loaded or generated. */ -crypto_pk_t * -get_onion_key(void) +MOCK_IMPL(crypto_pk_t *, +get_onion_key,(void)) { tor_assert(onionkey); return onionkey; @@ -376,8 +376,8 @@ assert_identity_keys_ok(void) /** Returns the current server identity key; requires that the key has * been set, and that we are running as a Tor server. */ -crypto_pk_t * -get_server_identity_key(void) +MOCK_IMPL(crypto_pk_t *, +get_server_identity_key,(void)) { tor_assert(server_identitykey); tor_assert(server_mode(get_options())); @@ -1951,8 +1951,8 @@ get_my_declared_family(const or_options_t *options) * * Returns a negative value and sets ri_out to NULL on temporary error. */ -static int -router_build_fresh_unsigned_routerinfo(routerinfo_t **ri_out) +MOCK_IMPL(STATIC int, +router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out)) { routerinfo_t *ri = NULL; uint32_t addr; @@ -2171,7 +2171,7 @@ router_dump_and_sign_extrainfo_descriptor_body(extrainfo_t *ei) * If ri is NULL, logs a BUG() warning and returns NULL. * Caller is responsible for freeing the generated extrainfo. */ -static extrainfo_t * +STATIC extrainfo_t * router_build_fresh_signed_extrainfo(const routerinfo_t *ri) { int result = -1; @@ -2203,7 +2203,7 @@ router_build_fresh_signed_extrainfo(const routerinfo_t *ri) * * If ei is NULL, logs a BUG() warning and zeroes the relevant fields. */ -static void +STATIC void router_update_routerinfo_from_extrainfo(routerinfo_t *ri, const extrainfo_t *ei) { @@ -2231,7 +2231,7 @@ router_update_routerinfo_from_extrainfo(routerinfo_t *ri, * If ri is NULL, logs a BUG() warning and returns a negative value. * On error, ri->cache_info is not modified. */ -static int +STATIC int router_dump_and_sign_routerinfo_descriptor_body(routerinfo_t *ri) { if (BUG(!ri)) diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index 497d8d243e..d4ad52c9d9 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -25,10 +25,10 @@ struct ed25519_keypair_t; #define TOR_ROUTERINFO_ERROR_DESC_REBUILDING (-6) #define TOR_ROUTERINFO_ERROR_INTERNAL_BUG (-7) -crypto_pk_t *get_onion_key(void); +MOCK_DECL(crypto_pk_t *,get_onion_key,(void)); time_t get_onion_key_set_at(void); void set_server_identity_key(crypto_pk_t *k); -crypto_pk_t *get_server_identity_key(void); +MOCK_DECL(crypto_pk_t *,get_server_identity_key,(void)); int server_identity_key_is_set(void); void set_client_identity_key(crypto_pk_t *k); crypto_pk_t *get_tlsclient_identity_key(void); @@ -124,6 +124,14 @@ STATIC smartlist_t *get_my_declared_family(const or_options_t *options); extern time_t desc_clean_since; extern const char *desc_dirty_reason; void set_server_identity_key_digest_testing(const uint8_t *digest); + +MOCK_DECL(STATIC int, + router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out)); +STATIC extrainfo_t *router_build_fresh_signed_extrainfo( + const routerinfo_t *ri); +STATIC void router_update_routerinfo_from_extrainfo(routerinfo_t *ri, + const extrainfo_t *ei); +STATIC int router_dump_and_sign_routerinfo_descriptor_body(routerinfo_t *ri); #endif #endif diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 4132d42d12..57adee414c 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -171,6 +171,31 @@ mock_get_configured_ports(void) return mocked_configured_ports; } +static tor_cert_t * +mock_tor_cert_dup_null(const tor_cert_t *cert) +{ + (void)cert; + return NULL; +} + +static crypto_pk_t *mocked_server_identitykey = NULL; + +/* Returns mocked_server_identitykey with no checks. */ +static crypto_pk_t * +mock_get_server_identity_key(void) +{ + return mocked_server_identitykey; +} + +static crypto_pk_t *mocked_onionkey = NULL; + +/* Returns mocked_onionkey with no checks. */ +static crypto_pk_t * +mock_get_onion_key(void) +{ + return mocked_onionkey; +} + /** Run unit tests for router descriptor generation logic. */ static void test_dir_formats(void *arg) @@ -182,8 +207,11 @@ test_dir_formats(void *arg) char *pk1_str = NULL, *pk2_str = NULL, *cp; size_t pk1_str_len, pk2_str_len; routerinfo_t *r1=NULL, *r2=NULL; + extrainfo_t *e1 = NULL, *e2 = NULL; crypto_pk_t *pk1 = NULL, *pk2 = NULL; + routerinfo_t *r2_out = NULL; routerinfo_t *rp1 = NULL, *rp2 = NULL; + extrainfo_t *ep1 = NULL, *ep2 = NULL; addr_policy_t *ex1, *ex2; routerlist_t *dir1 = NULL, *dir2 = NULL; uint8_t *rsa_cc = NULL; @@ -192,6 +220,8 @@ test_dir_formats(void *arg) time_t now = time(NULL); port_cfg_t orport, dirport; char cert_buf[256]; + int rv = -1; + const char *msg = NULL; (void)arg; pk1 = pk_generate(0); @@ -202,6 +232,8 @@ test_dir_formats(void *arg) hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); get_platform_str(platform, sizeof(platform)); + + /* r1 is a minimal, RSA-only descriptor */ r1 = tor_malloc_zero(sizeof(routerinfo_t)); r1->addr = 0xc0a80001u; /* 192.168.0.1 */ r1->cache_info.published_on = 0; @@ -224,6 +256,7 @@ test_dir_formats(void *arg) r1->nickname = tor_strdup("Magri"); r1->platform = tor_strdup(platform); + /* r2 is a RSA + ed25519 descriptor, with an exit policy */ ex1 = tor_malloc_zero(sizeof(addr_policy_t)); ex2 = tor_malloc_zero(sizeof(addr_policy_t)); ex1->policy_type = ADDR_POLICY_ACCEPT; @@ -352,8 +385,86 @@ test_dir_formats(void *arg) crypto_pk_free(onion_pkey); tt_int_op(crypto_pk_cmp_keys(rp1->identity_pkey, pk2), OP_EQ, 0); tt_assert(rp1->supports_tunnelled_dir_requests); - //tt_assert(rp1->exit_policy == NULL); + tt_assert(rp1->policy_is_reject_star); + tor_free(buf); + routerinfo_free(rp1); + + /* Test extrainfo creation. + * We avoid calling router_build_fresh_unsigned_routerinfo(), because it's + * too complex. Instead, we re-use the manually-created routerinfos. + */ + + /* router_build_fresh_signed_extrainfo() requires options->Nickname */ + tor_free(options->Nickname); + options->Nickname = tor_strdup(r1->nickname); + /* router_build_fresh_signed_extrainfo() passes the result of + * get_master_signing_key_cert() directly to tor_cert_dup(), which fails on + * NULL. But we want a NULL ei->cache_info.signing_key_cert to test the + * non-ed key path. + */ + MOCK(tor_cert_dup, mock_tor_cert_dup_null); + /* router_build_fresh_signed_extrainfo() requires get_server_identity_key(). + * Use the same one as the call to router_dump_router_to_string() above. + */ + mocked_server_identitykey = pk2; + MOCK(get_server_identity_key, mock_get_server_identity_key); + /* router_dump_and_sign_routerinfo_descriptor_body() requires + * get_onion_key(). Use the same one as r1. + */ + mocked_onionkey = pk1; + MOCK(get_onion_key, mock_get_onion_key); + + /* Test some of the low-level static functions. */ + e1 = router_build_fresh_signed_extrainfo(r1); + tt_assert(e1); + router_update_routerinfo_from_extrainfo(r1, e1); + rv = router_dump_and_sign_routerinfo_descriptor_body(r1); + tt_assert(rv == 0); + msg = ""; + rv = routerinfo_incompatible_with_extrainfo(r1->identity_pkey, e1, + &r1->cache_info, &msg); + tt_str_op(msg, OP_EQ, ""); + tt_assert(rv == 0); + + /* Now cleanup */ + tor_free(options->Nickname); + UNMOCK(tor_cert_dup); + mocked_server_identitykey = NULL; + UNMOCK(get_server_identity_key); + mocked_onionkey = NULL; + UNMOCK(get_onion_key); + + /* Test that the signed ri is parseable */ + tt_assert(r1->cache_info.signed_descriptor_body); + cp = r1->cache_info.signed_descriptor_body; + rp1 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); + tt_assert(rp1); + tt_int_op(rp1->addr,OP_EQ, r1->addr); + tt_int_op(rp1->or_port,OP_EQ, r1->or_port); + tt_int_op(rp1->dir_port,OP_EQ, r1->dir_port); + tt_int_op(rp1->bandwidthrate,OP_EQ, r1->bandwidthrate); + tt_int_op(rp1->bandwidthburst,OP_EQ, r1->bandwidthburst); + tt_int_op(rp1->bandwidthcapacity,OP_EQ, r1->bandwidthcapacity); + onion_pkey = router_get_rsa_onion_pkey(rp1->onion_pkey, + rp1->onion_pkey_len); + tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk1), OP_EQ, 0); + crypto_pk_free(onion_pkey); + tt_int_op(crypto_pk_cmp_keys(rp1->identity_pkey, pk2), OP_EQ, 0); + tt_assert(rp1->supports_tunnelled_dir_requests); + tt_assert(rp1->policy_is_reject_star); + + routerinfo_free(rp1); + + /* Test that the signed ei is parseable */ + tt_assert(e1->cache_info.signed_descriptor_body); + cp = e1->cache_info.signed_descriptor_body; + ep1 = extrainfo_parse_entry_from_string((const char*)cp,NULL,1,NULL,NULL); + tt_assert(ep1); + tt_str_op(ep1->nickname, OP_EQ, r1->nickname); + /* In future tests, we could check the actual extrainfo statistics. */ + + extrainfo_free(ep1); strlcpy(buf2, "router Fred 10.3.2.1 9005 0 0\n" @@ -503,20 +614,23 @@ test_dir_formats(void *arg) dirserv_free_fingerprint_list(); done: - if (r1) - routerinfo_free(r1); - if (r2) - routerinfo_free(r2); - if (rp2) - routerinfo_free(rp2); + routerinfo_free(r1); + routerinfo_free(r2); + routerinfo_free(r2_out); + routerinfo_free(rp1); + routerinfo_free(rp2); + + extrainfo_free(e1); + extrainfo_free(e2); + extrainfo_free(ep1); + extrainfo_free(ep2); tor_free(rsa_cc); tor_free(buf); tor_free(pk1_str); tor_free(pk2_str); - if (pk1) crypto_pk_free(pk1); - if (pk2) crypto_pk_free(pk2); - if (rp1) routerinfo_free(rp1); + crypto_pk_free(pk1); + crypto_pk_free(pk2); tor_free(dir1); /* XXXX And more !*/ tor_free(dir2); /* And more !*/ } From 7a2c8dadedcb3b17195111fee7aa91695d6bf6bb Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 18 Feb 2019 17:03:33 +1000 Subject: [PATCH 11/17] test_dir: Split test_dir_formats into separate rsa and rsa_ed25519 tests --- src/test/test_dir.c | 186 +++++++++++++++++++++++++++++--------------- 1 file changed, 123 insertions(+), 63 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 57adee414c..785d114f77 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -196,9 +196,12 @@ mock_get_onion_key(void) return mocked_onionkey; } -/** Run unit tests for router descriptor generation logic. */ +/** Run unit tests for router descriptor generation logic for a RSA-only + * router. Tor versions without ed25519 (0.2.6 and earlier) are no longer + * officially supported, but the authorities still accept their descriptors. + */ static void -test_dir_formats(void *arg) +test_dir_formats_rsa(void *arg) { char *buf = NULL; char buf2[8192]; @@ -206,18 +209,14 @@ test_dir_formats(void *arg) char fingerprint[FINGERPRINT_LEN+1]; char *pk1_str = NULL, *pk2_str = NULL, *cp; size_t pk1_str_len, pk2_str_len; - routerinfo_t *r1=NULL, *r2=NULL; - extrainfo_t *e1 = NULL, *e2 = NULL; + routerinfo_t *r1 = NULL; + extrainfo_t *e1 = NULL; crypto_pk_t *pk1 = NULL, *pk2 = NULL; - routerinfo_t *r2_out = NULL; - routerinfo_t *rp1 = NULL, *rp2 = NULL; - extrainfo_t *ep1 = NULL, *ep2 = NULL; - addr_policy_t *ex1, *ex2; + routerinfo_t *rp1 = NULL; + extrainfo_t *ep1 = NULL; routerlist_t *dir1 = NULL, *dir2 = NULL; uint8_t *rsa_cc = NULL; or_options_t *options = get_options_mutable(); - const addr_policy_t *p; - time_t now = time(NULL); port_cfg_t orport, dirport; char cert_buf[256]; int rv = -1; @@ -256,57 +255,15 @@ test_dir_formats(void *arg) r1->nickname = tor_strdup("Magri"); r1->platform = tor_strdup(platform); - /* r2 is a RSA + ed25519 descriptor, with an exit policy */ - ex1 = tor_malloc_zero(sizeof(addr_policy_t)); - ex2 = tor_malloc_zero(sizeof(addr_policy_t)); - ex1->policy_type = ADDR_POLICY_ACCEPT; - tor_addr_from_ipv4h(&ex1->addr, 0); - ex1->maskbits = 0; - ex1->prt_min = ex1->prt_max = 80; - ex2->policy_type = ADDR_POLICY_REJECT; - tor_addr_from_ipv4h(&ex2->addr, 18<<24); - ex2->maskbits = 8; - ex2->prt_min = ex2->prt_max = 24; - r2 = tor_malloc_zero(sizeof(routerinfo_t)); - r2->addr = 0x0a030201u; /* 10.3.2.1 */ - ed25519_keypair_t kp1, kp2; - ed25519_secret_key_from_seed(&kp1.seckey, - (const uint8_t*)"YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"); - ed25519_public_key_generate(&kp1.pubkey, &kp1.seckey); - ed25519_secret_key_from_seed(&kp2.seckey, - (const uint8_t*)"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"); - ed25519_public_key_generate(&kp2.pubkey, &kp2.seckey); - r2->cache_info.signing_key_cert = tor_cert_create(&kp1, - CERT_TYPE_ID_SIGNING, - &kp2.pubkey, - now, 86400, - CERT_FLAG_INCLUDE_SIGNING_KEY); - r2->platform = tor_strdup(platform); - r2->cache_info.published_on = 5; - r2->or_port = 9005; - r2->dir_port = 0; - r2->supports_tunnelled_dir_requests = 1; - router_set_rsa_onion_pkey(pk2, &r2->onion_pkey, &r2->onion_pkey_len); - curve25519_keypair_t r2_onion_keypair; - curve25519_keypair_generate(&r2_onion_keypair, 0); - r2->onion_curve25519_pkey = tor_memdup(&r2_onion_keypair.pubkey, - sizeof(curve25519_public_key_t)); - r2->identity_pkey = crypto_pk_dup_key(pk1); - r2->bandwidthrate = r2->bandwidthburst = r2->bandwidthcapacity = 3000; - r2->exit_policy = smartlist_new(); - smartlist_add(r2->exit_policy, ex1); - smartlist_add(r2->exit_policy, ex2); - r2->nickname = tor_strdup("Fred"); - tt_assert(!crypto_pk_write_public_key_to_string(pk1, &pk1_str, - &pk1_str_len)); + &pk1_str_len)); tt_assert(!crypto_pk_write_public_key_to_string(pk2 , &pk2_str, - &pk2_str_len)); + &pk2_str_len)); /* XXXX+++ router_dump_to_string should really take this from ri.*/ options->ContactInfo = tor_strdup("Magri White " ""); - /* Skip reachability checks for DirPort and tunnelled-dir-server */ + /* Skip reachability checks for DirPort, ORPort, and tunnelled-dir-server */ options->AssumeReachable = 1; /* Fake just enough of an ORPort and DirPort to get by */ @@ -466,6 +423,108 @@ test_dir_formats(void *arg) extrainfo_free(ep1); + done: + dirserv_free_fingerprint_list(); + + routerinfo_free(r1); + routerinfo_free(rp1); + + extrainfo_free(e1); + extrainfo_free(ep1); + + tor_free(rsa_cc); + tor_free(buf); + tor_free(pk1_str); + tor_free(pk2_str); + crypto_pk_free(pk1); + crypto_pk_free(pk2); + tor_free(dir1); /* XXXX And more !*/ + tor_free(dir2); /* And more !*/ +} + +/** Run unit tests for router descriptor generation logic for a RSA + ed25519 + * router. + */ +static void +test_dir_formats_rsa_ed25519(void *arg) +{ + char *buf = NULL; + char buf2[8192]; + char platform[256]; + char fingerprint[FINGERPRINT_LEN+1]; + char *pk1_str = NULL, *pk2_str = NULL, *cp; + size_t pk1_str_len, pk2_str_len; + routerinfo_t *r2 = NULL; + extrainfo_t *e2 = NULL; + crypto_pk_t *pk1 = NULL, *pk2 = NULL; + routerinfo_t *r2_out = NULL; + routerinfo_t *rp2 = NULL; + extrainfo_t *ep2 = NULL; + addr_policy_t *ex1, *ex2; + routerlist_t *dir1 = NULL, *dir2 = NULL; + uint8_t *rsa_cc = NULL; + or_options_t *options = get_options_mutable(); + const addr_policy_t *p; + time_t now = time(NULL); + port_cfg_t orport; + char cert_buf[256]; + + (void)arg; + pk1 = pk_generate(0); + pk2 = pk_generate(1); + + tt_assert(pk1 && pk2); + + hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); + + get_platform_str(platform, sizeof(platform)); + /* r2 is a RSA + ed25519 descriptor, with an exit policy */ + ex1 = tor_malloc_zero(sizeof(addr_policy_t)); + ex2 = tor_malloc_zero(sizeof(addr_policy_t)); + ex1->policy_type = ADDR_POLICY_ACCEPT; + tor_addr_from_ipv4h(&ex1->addr, 0); + ex1->maskbits = 0; + ex1->prt_min = ex1->prt_max = 80; + ex2->policy_type = ADDR_POLICY_REJECT; + tor_addr_from_ipv4h(&ex2->addr, 18<<24); + ex2->maskbits = 8; + ex2->prt_min = ex2->prt_max = 24; + r2 = tor_malloc_zero(sizeof(routerinfo_t)); + r2->addr = 0x0a030201u; /* 10.3.2.1 */ + ed25519_keypair_t kp1, kp2; + ed25519_secret_key_from_seed(&kp1.seckey, + (const uint8_t*)"YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"); + ed25519_public_key_generate(&kp1.pubkey, &kp1.seckey); + ed25519_secret_key_from_seed(&kp2.seckey, + (const uint8_t*)"XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"); + ed25519_public_key_generate(&kp2.pubkey, &kp2.seckey); + r2->cache_info.signing_key_cert = tor_cert_create(&kp1, + CERT_TYPE_ID_SIGNING, + &kp2.pubkey, + now, 86400, + CERT_FLAG_INCLUDE_SIGNING_KEY); + r2->platform = tor_strdup(platform); + r2->cache_info.published_on = 5; + r2->or_port = 9005; + r2->dir_port = 0; + r2->supports_tunnelled_dir_requests = 1; + router_set_rsa_onion_pkey(pk2, &r2->onion_pkey, &r2->onion_pkey_len); + curve25519_keypair_t r2_onion_keypair; + curve25519_keypair_generate(&r2_onion_keypair, 0); + r2->onion_curve25519_pkey = tor_memdup(&r2_onion_keypair.pubkey, + sizeof(curve25519_public_key_t)); + r2->identity_pkey = crypto_pk_dup_key(pk1); + r2->bandwidthrate = r2->bandwidthburst = r2->bandwidthcapacity = 3000; + r2->exit_policy = smartlist_new(); + smartlist_add(r2->exit_policy, ex1); + smartlist_add(r2->exit_policy, ex2); + r2->nickname = tor_strdup("Fred"); + + tt_assert(!crypto_pk_write_public_key_to_string(pk1, &pk1_str, + &pk1_str_len)); + tt_assert(!crypto_pk_write_public_key_to_string(pk2 , &pk2_str, + &pk2_str_len)); + strlcpy(buf2, "router Fred 10.3.2.1 9005 0 0\n" "identity-ed25519\n" @@ -540,6 +599,9 @@ test_dir_formats(void *arg) strlcat(buf2, "tunnelled-dir-server\n", sizeof(buf2)); strlcat(buf2, "router-sig-ed25519 ", sizeof(buf2)); + /* Skip reachability checks for ORPort and tunnelled-dir-server */ + options->AssumeReachable = 1; + /* Fake just enough of an ORPort to get by */ MOCK(get_configured_ports, mock_get_configured_ports); mocked_configured_ports = smartlist_new(); @@ -577,8 +639,8 @@ test_dir_formats(void *arg) tt_mem_op(rp2->onion_curve25519_pkey->public_key,OP_EQ, r2->onion_curve25519_pkey->public_key, CURVE25519_PUBKEY_LEN); - onion_pkey = router_get_rsa_onion_pkey(rp2->onion_pkey, - rp2->onion_pkey_len); + crypto_pk_t *onion_pkey = router_get_rsa_onion_pkey(rp2->onion_pkey, + rp2->onion_pkey_len); tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk2), OP_EQ, 0); crypto_pk_free(onion_pkey); tt_int_op(crypto_pk_cmp_keys(rp2->identity_pkey, pk1), OP_EQ, 0); @@ -611,18 +673,15 @@ test_dir_formats(void *arg) } #endif /* 0 */ - dirserv_free_fingerprint_list(); done: - routerinfo_free(r1); + dirserv_free_fingerprint_list(); + routerinfo_free(r2); routerinfo_free(r2_out); - routerinfo_free(rp1); routerinfo_free(rp2); - extrainfo_free(e1); extrainfo_free(e2); - extrainfo_free(ep1); extrainfo_free(ep2); tor_free(rsa_cc); @@ -6601,7 +6660,8 @@ test_dir_format_versions_list(void *arg) struct testcase_t dir_tests[] = { DIR_LEGACY(nicknames), - DIR_LEGACY(formats), + DIR_LEGACY(formats_rsa), + DIR_LEGACY(formats_rsa_ed25519), DIR(routerinfo_parsing, 0), DIR(extrainfo_parsing, 0), DIR(parse_router_list, TT_FORK), From 8e5df40018acd0bd80626073b16b6cc070129109 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 18 Feb 2019 17:37:47 +1000 Subject: [PATCH 12/17] test_dir: Test rsa + ed25519 extrainfo creation and parsing Also fix a missing mock in rsa-only parsing. --- src/feature/relay/router.c | 5 +- src/feature/relay/router.h | 2 + src/feature/relay/routerkeys.c | 8 +- src/feature/relay/routerkeys.h | 4 +- src/test/test_dir.c | 196 ++++++++++++++++++++++++++++++++- 5 files changed, 206 insertions(+), 9 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 18c8375299..cbd5f2f91e 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -271,11 +271,12 @@ expire_old_onion_keys(void) /** Return the current secret onion key for the ntor handshake. Must only * be called from the main thread. */ -static const curve25519_keypair_t * -get_current_curve25519_keypair(void) +MOCK_IMPL(STATIC const struct curve25519_keypair_t *, +get_current_curve25519_keypair,(void)) { return &curve25519_onion_key; } + /** Return a map from KEYID (the key itself) to keypairs for use in the ntor * handshake. Must only be called from the main thread. */ di_digest256_map_t * diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index d4ad52c9d9..f17e6a9438 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -124,6 +124,8 @@ STATIC smartlist_t *get_my_declared_family(const or_options_t *options); extern time_t desc_clean_since; extern const char *desc_dirty_reason; void set_server_identity_key_digest_testing(const uint8_t *digest); +MOCK_DECL(STATIC const struct curve25519_keypair_t *, + get_current_curve25519_keypair,(void)); MOCK_DECL(STATIC int, router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out)); diff --git a/src/feature/relay/routerkeys.c b/src/feature/relay/routerkeys.c index 876f908d41..bdd7a82b58 100644 --- a/src/feature/relay/routerkeys.c +++ b/src/feature/relay/routerkeys.c @@ -631,14 +631,14 @@ get_master_identity_keypair(void) } #endif /* defined(TOR_UNIT_TESTS) */ -const ed25519_keypair_t * -get_master_signing_keypair(void) +MOCK_IMPL(const ed25519_keypair_t *, +get_master_signing_keypair,(void)) { return master_signing_key; } -const struct tor_cert_st * -get_master_signing_key_cert(void) +MOCK_IMPL(const struct tor_cert_st *, +get_master_signing_key_cert,(void)) { return signing_key_cert; } diff --git a/src/feature/relay/routerkeys.h b/src/feature/relay/routerkeys.h index 0badd34191..cde07b52c3 100644 --- a/src/feature/relay/routerkeys.h +++ b/src/feature/relay/routerkeys.h @@ -7,8 +7,8 @@ #include "lib/crypt_ops/crypto_ed25519.h" const ed25519_public_key_t *get_master_identity_key(void); -const ed25519_keypair_t *get_master_signing_keypair(void); -const struct tor_cert_st *get_master_signing_key_cert(void); +MOCK_DECL(const ed25519_keypair_t *, get_master_signing_keypair,(void)); +MOCK_DECL(const struct tor_cert_st *, get_master_signing_key_cert,(void)); const ed25519_keypair_t *get_current_auth_keypair(void); const struct tor_cert_st *get_current_link_cert_cert(void); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 785d114f77..ad8ab87e9f 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -196,6 +196,45 @@ mock_get_onion_key(void) return mocked_onionkey; } +static routerinfo_t *mocked_routerinfo = NULL; + +/* Returns 0 and sets ri_out to mocked_routerinfo. + * ri_out must not be NULL. There are no other checks. */ +static int +mock_router_build_fresh_unsigned_routerinfo(routerinfo_t **ri_out) +{ + tor_assert(ri_out); + *ri_out = mocked_routerinfo; + return 0; +} + +static ed25519_keypair_t *mocked_master_signing_key = NULL; + +/* Returns mocked_master_signing_key with no checks. */ +static const ed25519_keypair_t * +mock_get_master_signing_keypair(void) +{ + return mocked_master_signing_key; +} + +static struct tor_cert_st *mocked_signing_key_cert = NULL; + +/* Returns mocked_signing_key_cert with no checks. */ +static const struct tor_cert_st * +mock_get_master_signing_key_cert(void) +{ + return mocked_signing_key_cert; +} + +static curve25519_keypair_t *mocked_curve25519_onion_key = NULL; + +/* Returns mocked_curve25519_onion_key with no checks. */ +static const curve25519_keypair_t * +mock_get_current_curve25519_keypair(void) +{ + return mocked_curve25519_onion_key; +} + /** Run unit tests for router descriptor generation logic for a RSA-only * router. Tor versions without ed25519 (0.2.6 and earlier) are no longer * officially supported, but the authorities still accept their descriptors. @@ -372,6 +411,22 @@ test_dir_formats_rsa(void *arg) mocked_onionkey = pk1; MOCK(get_onion_key, mock_get_onion_key); + /* Fake just enough of an ORPort and DirPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + memset(&orport, 0, sizeof(orport)); + orport.type = CONN_TYPE_OR_LISTENER; + orport.addr.family = AF_INET; + orport.port = 9000; + smartlist_add(mocked_configured_ports, &orport); + + memset(&dirport, 0, sizeof(dirport)); + dirport.type = CONN_TYPE_DIR_LISTENER; + dirport.addr.family = AF_INET; + dirport.port = 9003; + smartlist_add(mocked_configured_ports, &dirport); + /* Test some of the low-level static functions. */ e1 = router_build_fresh_signed_extrainfo(r1); tt_assert(e1); @@ -392,6 +447,10 @@ test_dir_formats_rsa(void *arg) mocked_onionkey = NULL; UNMOCK(get_onion_key); + UNMOCK(get_configured_ports); + smartlist_free(mocked_configured_ports); + mocked_configured_ports = NULL; + /* Test that the signed ri is parseable */ tt_assert(r1->cache_info.signed_descriptor_body); cp = r1->cache_info.signed_descriptor_body; @@ -468,6 +527,7 @@ test_dir_formats_rsa_ed25519(void *arg) time_t now = time(NULL); port_cfg_t orport; char cert_buf[256]; + int rv = -1; (void)arg; pk1 = pk_generate(0); @@ -478,7 +538,10 @@ test_dir_formats_rsa_ed25519(void *arg) hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); get_platform_str(platform, sizeof(platform)); - /* r2 is a RSA + ed25519 descriptor, with an exit policy */ + + /* We can't use init_mock_ed_keys() here, because the keys are seeded */ + + /* r2 is a RSA + ed25519 descriptor, with an exit policy */ ex1 = tor_malloc_zero(sizeof(addr_policy_t)); ex2 = tor_malloc_zero(sizeof(addr_policy_t)); ex1->policy_type = ADDR_POLICY_ACCEPT; @@ -662,6 +725,137 @@ test_dir_formats_rsa_ed25519(void *arg) tt_int_op(p->prt_min,OP_EQ, 24); tt_int_op(p->prt_max,OP_EQ, 24); + routerinfo_free(rp2); + + /* Test extrainfo creation. */ + + /* router_build_fresh_descriptor() requires + * router_build_fresh_unsigned_routerinfo(), but the implementation is + * too complex. Instead, we re-use r2. + */ + mocked_routerinfo = r2; + MOCK(router_build_fresh_unsigned_routerinfo, + mock_router_build_fresh_unsigned_routerinfo); + + /* router_build_fresh_signed_extrainfo() requires options->Nickname */ + tor_free(options->Nickname); + options->Nickname = tor_strdup(r2->nickname); + /* router_build_fresh_signed_extrainfo() requires get_server_identity_key(). + * Use the same one as the call to router_dump_router_to_string() above. + * For the second router, the keys are swapped. + */ + mocked_server_identitykey = pk1; + MOCK(get_server_identity_key, mock_get_server_identity_key); + /* router_dump_and_sign_routerinfo_descriptor_body() requires + * get_onion_key(). Use the same one as r1. + */ + mocked_onionkey = pk2; + MOCK(get_onion_key, mock_get_onion_key); + + /* r2 uses ed25519, so we need to mock the ed key functions */ + mocked_master_signing_key = &kp2; + MOCK(get_master_signing_keypair, mock_get_master_signing_keypair); + + mocked_signing_key_cert = r2->cache_info.signing_key_cert; + MOCK(get_master_signing_key_cert, mock_get_master_signing_key_cert); + + mocked_curve25519_onion_key = &r2_onion_keypair; + MOCK(get_current_curve25519_keypair, mock_get_current_curve25519_keypair); + + /* Fake just enough of an ORPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + memset(&orport, 0, sizeof(orport)); + orport.type = CONN_TYPE_OR_LISTENER; + orport.addr.family = AF_INET; + orport.port = 9005; + smartlist_add(mocked_configured_ports, &orport); + + /* Test the high-level interface. */ + rv = router_build_fresh_descriptor(&r2_out, &e2); + if (rv < 0) { + /* router_build_fresh_descriptor() frees r2 on failure. */ + r2 = NULL; + /* Get rid of an alias to rp2 */ + r2_out = NULL; + } + tt_assert(rv == 0); + tt_assert(r2_out); + tt_assert(e2); + /* Guaranteed by mock_router_build_fresh_unsigned_routerinfo() */ + tt_ptr_op(r2_out, OP_EQ, r2); + /* Get rid of an alias to r2 */ + r2_out = NULL; + + /* Now cleanup */ + mocked_routerinfo = NULL; + UNMOCK(router_build_fresh_unsigned_routerinfo); + tor_free(options->Nickname); + mocked_server_identitykey = NULL; + UNMOCK(get_server_identity_key); + mocked_onionkey = NULL; + UNMOCK(get_onion_key); + mocked_master_signing_key = NULL; + UNMOCK(get_master_signing_keypair); + mocked_signing_key_cert = NULL; + UNMOCK(get_master_signing_key_cert); + mocked_curve25519_onion_key = NULL; + UNMOCK(get_current_curve25519_keypair); + + UNMOCK(get_configured_ports); + smartlist_free(mocked_configured_ports); + mocked_configured_ports = NULL; + + /* Test that the signed ri is parseable */ + tt_assert(r2->cache_info.signed_descriptor_body); + cp = r2->cache_info.signed_descriptor_body; + rp2 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); + tt_assert(rp2); + tt_int_op(rp2->addr,OP_EQ, r2->addr); + tt_int_op(rp2->or_port,OP_EQ, r2->or_port); + tt_int_op(rp2->dir_port,OP_EQ, r2->dir_port); + tt_int_op(rp2->bandwidthrate,OP_EQ, r2->bandwidthrate); + tt_int_op(rp2->bandwidthburst,OP_EQ, r2->bandwidthburst); + tt_int_op(rp2->bandwidthcapacity,OP_EQ, r2->bandwidthcapacity); + tt_mem_op(rp2->onion_curve25519_pkey->public_key,OP_EQ, + r2->onion_curve25519_pkey->public_key, + CURVE25519_PUBKEY_LEN); + onion_pkey = router_get_rsa_onion_pkey(rp2->onion_pkey, + rp2->onion_pkey_len); + tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk2), OP_EQ, 0); + crypto_pk_free(onion_pkey); + tt_int_op(crypto_pk_cmp_keys(rp2->identity_pkey, pk1), OP_EQ, 0); + tt_assert(rp2->supports_tunnelled_dir_requests); + + tt_int_op(smartlist_len(rp2->exit_policy),OP_EQ, 2); + + p = smartlist_get(rp2->exit_policy, 0); + tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_ACCEPT); + tt_assert(tor_addr_is_null(&p->addr)); + tt_int_op(p->maskbits,OP_EQ, 0); + tt_int_op(p->prt_min,OP_EQ, 80); + tt_int_op(p->prt_max,OP_EQ, 80); + + p = smartlist_get(rp2->exit_policy, 1); + tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_REJECT); + tt_assert(tor_addr_eq(&p->addr, &ex2->addr)); + tt_int_op(p->maskbits,OP_EQ, 8); + tt_int_op(p->prt_min,OP_EQ, 24); + tt_int_op(p->prt_max,OP_EQ, 24); + + routerinfo_free(rp2); + + /* Test that the signed ei is parseable */ + tt_assert(e2->cache_info.signed_descriptor_body); + cp = e2->cache_info.signed_descriptor_body; + ep2 = extrainfo_parse_entry_from_string((const char*)cp,NULL,1,NULL,NULL); + tt_assert(ep2); + tt_str_op(ep2->nickname, OP_EQ, r2->nickname); + /* In future tests, we could check the actual extrainfo statistics. */ + + extrainfo_free(ep2); + #if 0 /* Okay, now for the directories. */ { From 38fc52a50e12fbcf1b1451e5a3a3016d1ced9759 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 19 Feb 2019 11:19:44 +1000 Subject: [PATCH 13/17] test_dir: Refactor common code out of the dir_format unit tests Also: * delete some obsolete code that was #if 0 * improve cleanup on failure * make the dir_format tests more consistent with each other * construct the descriptors using smartlist chunks This refactor is incomplete, because removing the remaining duplicate code would be time-consuming. Part of 29017 and 29018. --- src/test/test_dir.c | 1009 +++++++++++++++++++++++++++---------------- 1 file changed, 637 insertions(+), 372 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index ad8ab87e9f..cc0ef07fbf 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -162,6 +162,255 @@ test_dir_nicknames(void *arg) ; } +/* Allocate and return a new routerinfo, with the fields set from the + * arguments to this function. + * + * Also sets: + * - random RSA identity and onion keys, + * - the platform field using get_platform_str(), and + * - supports_tunnelled_dir_requests to 1. + * + * If rsa_onion_keypair_out is not NULL, it is set to the onion keypair. + * The caller must free this keypair. + */ +static routerinfo_t * +basic_routerinfo_new(const char *nickname, uint32_t ipv4_addr, + uint16_t or_port, uint16_t dir_port, + uint32_t bandwidthrate, uint32_t bandwidthburst, + uint32_t bandwidthcapacity, + time_t published_on, + crypto_pk_t **rsa_onion_keypair_out) +{ + char platform[256]; + + tor_assert(nickname); + + crypto_pk_t *pk1 = NULL, *pk2 = NULL; + /* These keys are random: idx is ignored. */ + pk1 = pk_generate(0); + pk2 = pk_generate(1); + + tor_assert(pk1); + tor_assert(pk2); + + get_platform_str(platform, sizeof(platform)); + + routerinfo_t *r1 = tor_malloc_zero(sizeof(routerinfo_t)); + + r1->nickname = tor_strdup(nickname); + r1->platform = tor_strdup(platform); + + r1->addr = ipv4_addr; + r1->or_port = or_port; + r1->dir_port = dir_port; + r1->supports_tunnelled_dir_requests = 1; + + router_set_rsa_onion_pkey(pk1, &r1->onion_pkey, &r1->onion_pkey_len); + r1->identity_pkey = pk2; + + r1->bandwidthrate = bandwidthrate; + r1->bandwidthburst = bandwidthburst; + r1->bandwidthcapacity = bandwidthcapacity; + + r1->cache_info.published_on = published_on; + + if (rsa_onion_keypair_out) { + *rsa_onion_keypair_out = pk1; + } else { + crypto_pk_free(pk1); + } + + return r1; +} + +/* Allocate and return a new string containing a "router" line for r1. */ +static char * +get_new_router_line(const routerinfo_t *r1) +{ + char *line = NULL; + + tor_assert(r1); + + tor_asprintf(&line, + "router %s %s %d 0 %d\n", + r1->nickname, fmt_addr32(r1->addr), + r1->or_port, r1->dir_port); + tor_assert(line); + + return line; +} + +/* Allocate and return a new string containing a "platform" line for the + * current Tor version and OS. */ +static char * +get_new_platform_line(void) +{ + char *line = NULL; + + tor_asprintf(&line, + "platform Tor %s on %s\n", + VERSION, get_uname()); + tor_assert(line); + + return line; +} + +/* Allocate and return a new string containing a "published" line for r1. + * r1->cache_info.published_on must be between 0 and 59 seconds. */ +static char * +get_new_published_line(const routerinfo_t *r1) +{ + char *line = NULL; + + tor_assert(r1); + + tor_assert(r1->cache_info.published_on >= 0); + tor_assert(r1->cache_info.published_on <= 59); + + tor_asprintf(&line, + "published 1970-01-01 00:00:%02u\n", + (unsigned)r1->cache_info.published_on); + tor_assert(line); + + return line; +} + +/* Allocate and return a new string containing a "fingerprint" line for r1. */ +static char * +get_new_fingerprint_line(const routerinfo_t *r1) +{ + char *line = NULL; + char fingerprint[FINGERPRINT_LEN+1]; + + tor_assert(r1); + + tor_assert(!crypto_pk_get_fingerprint(r1->identity_pkey, fingerprint, 1)); + tor_assert(strlen(fingerprint) > 0); + + tor_asprintf(&line, + "fingerprint %s\n", + fingerprint); + tor_assert(line); + + return line; +} + +/* Allocate and return a new string containing an "uptime" line with uptime t. + * + * You should pass a hard-coded value to this function, because even if we made + * it reflect uptime, that still wouldn't make it right, because the two + * descriptors might be made on different seconds. + */ +static char * +get_new_uptime_line(time_t t) +{ + char *line = NULL; + + tor_asprintf(&line, + "uptime %u\n", + (unsigned)t); + tor_assert(line); + + return line; +} + +/* Allocate and return a new string containing an "bandwidth" line for r1. + */ +static char * +get_new_bandwidth_line(const routerinfo_t *r1) +{ + char *line = NULL; + + tor_assert(r1); + + tor_asprintf(&line, + "bandwidth %u %u %u\n", + r1->bandwidthrate, + r1->bandwidthburst, + r1->bandwidthcapacity); + tor_assert(line); + + return line; +} + +/* Allocate and return a new string containing a key_name block for the + * RSA key pk1. + */ +static char * +get_new_rsa_key_block(const char *key_name, crypto_pk_t *pk1) +{ + char *block = NULL; + char *pk1_str = NULL; + size_t pk1_str_len = 0; + + tor_assert(key_name); + tor_assert(pk1); + + tor_assert(!crypto_pk_write_public_key_to_string(pk1, &pk1_str, + &pk1_str_len)); + tor_assert(pk1_str); + tor_assert(pk1_str_len); + + tor_asprintf(&block, + "%s\n%s", + key_name, + pk1_str); + tor_assert(block); + + return block; +} + +/* Allocate and return a new string containing an "onion-key" block for the + * router r1. + */ +static char * +get_new_onion_key_block(const routerinfo_t *r1) +{ + char *block = NULL; + tor_assert(r1); + crypto_pk_t *pk_tmp = router_get_rsa_onion_pkey(r1->onion_pkey, + r1->onion_pkey_len); + block = get_new_rsa_key_block("onion-key", pk_tmp); + crypto_pk_free(pk_tmp); + return block; +} + +/* Allocate and return a new string containing an "signing-key" block for the + * router r1. + */ +static char * +get_new_signing_key_block(const routerinfo_t *r1) +{ + tor_assert(r1); + return get_new_rsa_key_block("signing-key", r1->identity_pkey); +} + +/* Allocate and return a new string containing an "ntor-onion-key" line for + * the curve25519 public key ntor_onion_pubkey. + */ +static char * +get_new_ntor_onion_key_line(const curve25519_public_key_t *ntor_onion_pubkey) +{ + char *line = NULL; + char cert_buf[256]; + int rv = 0; + + tor_assert(ntor_onion_pubkey); + + rv = base64_encode(cert_buf, sizeof(cert_buf), + (const char*)ntor_onion_pubkey->public_key, 32, + BASE64_ENCODE_MULTILINE); + tor_assert(rv > 0); + tor_assert(strlen(cert_buf) > 0); + + tor_asprintf(&line, + "ntor-onion-key %s", + cert_buf); + tor_assert(line); + + return line; +} + static smartlist_t *mocked_configured_ports = NULL; /** Returns mocked_configured_ports */ @@ -235,6 +484,137 @@ mock_get_current_curve25519_keypair(void) return mocked_curve25519_onion_key; } +/* Unmock get_configured_ports() and free mocked_configured_ports. */ +static void +cleanup_mock_configured_ports(void) +{ + UNMOCK(get_configured_ports); + + if (mocked_configured_ports) { + SMARTLIST_FOREACH(mocked_configured_ports, port_cfg_t *, p, tor_free(p)); + smartlist_free(mocked_configured_ports); + } +} + +/* Mock get_configured_ports() with a list containing or_port and dir_port. + * If a port is 0, don't set it. + * Only sets the minimal data required for the tests to pass. */ +static void +setup_mock_configured_ports(uint16_t or_port, uint16_t dir_port) +{ + cleanup_mock_configured_ports(); + + /* Fake just enough of an ORPort and DirPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + if (or_port) { + port_cfg_t *or_port_cfg = tor_malloc_zero(sizeof(*or_port_cfg)); + or_port_cfg->type = CONN_TYPE_OR_LISTENER; + or_port_cfg->addr.family = AF_INET; + or_port_cfg->port = or_port; + smartlist_add(mocked_configured_ports, or_port_cfg); + } + + if (dir_port) { + port_cfg_t *dir_port_cfg = tor_malloc_zero(sizeof(*dir_port_cfg)); + dir_port_cfg->type = CONN_TYPE_DIR_LISTENER; + dir_port_cfg->addr.family = AF_INET; + dir_port_cfg->port = dir_port; + smartlist_add(mocked_configured_ports, dir_port_cfg); + } +} + +/* Clean up the data structures and unmock the functions needed for generating + * a fresh descriptor. */ +static void +cleanup_mocks_for_fresh_descriptor(void) +{ + tor_free(get_options_mutable()->Nickname); + + mocked_server_identitykey = NULL; + UNMOCK(get_server_identity_key); + + crypto_pk_free(mocked_onionkey); + UNMOCK(get_onion_key); +} + +/* Mock the data structures and functions needed for generating a fresh + * descriptor. + * + * Sets options->Nickname from r1->nickname. + * Mocks get_server_identity_key() with r1->identity_pkey. + * + * If rsa_onion_keypair is not NULL, it is used to mock get_onion_key(). + * Otherwise, the public key in r1->onion_pkey is used to mock get_onion_key(). + */ +static void +setup_mocks_for_fresh_descriptor(const routerinfo_t *r1, + crypto_pk_t *rsa_onion_keypair) +{ + cleanup_mocks_for_fresh_descriptor(); + + tor_assert(r1); + + /* router_build_fresh_signed_extrainfo() requires options->Nickname */ + get_options_mutable()->Nickname = tor_strdup(r1->nickname); + + /* router_build_fresh_signed_extrainfo() requires get_server_identity_key(). + * Use the same one as the call to router_dump_router_to_string() above. + */ + mocked_server_identitykey = r1->identity_pkey; + MOCK(get_server_identity_key, mock_get_server_identity_key); + + /* router_dump_and_sign_routerinfo_descriptor_body() requires + * get_onion_key(). Use the same one as r1. + */ + if (rsa_onion_keypair) { + mocked_onionkey = crypto_pk_dup_key(rsa_onion_keypair); + } else { + mocked_onionkey = router_get_rsa_onion_pkey(r1->onion_pkey, + r1->onion_pkey_len); + } + MOCK(get_onion_key, mock_get_onion_key); +} + +/* Check that routerinfos r1 and rp1 are consistent. + * Only performs some basic checks. + */ +#define CHECK_ROUTERINFO_CONSISTENCY(r1, rp1) \ +STMT_BEGIN \ + tt_assert(r1); \ + tt_assert(rp1); \ +\ + tt_int_op(rp1->addr,OP_EQ, r1->addr); \ + tt_int_op(rp1->or_port,OP_EQ, r1->or_port); \ + tt_int_op(rp1->dir_port,OP_EQ, r1->dir_port); \ + tt_int_op(rp1->bandwidthrate,OP_EQ, r1->bandwidthrate); \ + tt_int_op(rp1->bandwidthburst,OP_EQ, r1->bandwidthburst); \ + tt_int_op(rp1->bandwidthcapacity,OP_EQ, r1->bandwidthcapacity); \ + crypto_pk_t *rp1_onion_pkey = router_get_rsa_onion_pkey(rp1->onion_pkey, \ + rp1->onion_pkey_len); \ + crypto_pk_t *r1_onion_pkey = router_get_rsa_onion_pkey(r1->onion_pkey, \ + r1->onion_pkey_len); \ + tt_int_op(crypto_pk_cmp_keys(rp1_onion_pkey, r1_onion_pkey), OP_EQ, 0); \ + crypto_pk_free(rp1_onion_pkey); \ + crypto_pk_free(r1_onion_pkey); \ + tt_int_op(crypto_pk_cmp_keys(rp1->identity_pkey, r1->identity_pkey), \ + OP_EQ, 0); \ + tt_int_op(rp1->supports_tunnelled_dir_requests, OP_EQ, \ + r1->supports_tunnelled_dir_requests); \ +STMT_END + +/* Check that routerinfo r1 and extrainfo e1 are consistent. + * Only performs some basic checks. + */ +#define CHECK_EXTRAINFO_CONSISTENCY(r1, e1) \ +STMT_BEGIN \ + tt_assert(r1); \ + tt_assert(e1); \ +\ + tt_str_op(e1->nickname, OP_EQ, r1->nickname); \ +STMT_END + /** Run unit tests for router descriptor generation logic for a RSA-only * router. Tor versions without ed25519 (0.2.6 and earlier) are no longer * officially supported, but the authorities still accept their descriptors. @@ -243,144 +623,112 @@ static void test_dir_formats_rsa(void *arg) { char *buf = NULL; - char buf2[8192]; - char platform[256]; - char fingerprint[FINGERPRINT_LEN+1]; - char *pk1_str = NULL, *pk2_str = NULL, *cp; - size_t pk1_str_len, pk2_str_len; + char *buf2 = NULL; + char *cp = NULL; + + uint8_t *rsa_cc = NULL; + routerinfo_t *r1 = NULL; extrainfo_t *e1 = NULL; - crypto_pk_t *pk1 = NULL, *pk2 = NULL; routerinfo_t *rp1 = NULL; extrainfo_t *ep1 = NULL; - routerlist_t *dir1 = NULL, *dir2 = NULL; - uint8_t *rsa_cc = NULL; - or_options_t *options = get_options_mutable(); - port_cfg_t orport, dirport; - char cert_buf[256]; - int rv = -1; + + smartlist_t *chunks = NULL; const char *msg = NULL; + int rv = -1; + + or_options_t *options = get_options_mutable(); (void)arg; - pk1 = pk_generate(0); - pk2 = pk_generate(1); - - tt_assert(pk1 && pk2); hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); - get_platform_str(platform, sizeof(platform)); + /* r1 is a minimal, RSA-only descriptor, with DirPort and IPv6 */ + r1 = basic_routerinfo_new("Magri", 0xc0a80001u /* 192.168.0.1 */, + 9000, 9003, + 1000, 5000, 10000, + 0, + NULL); - /* r1 is a minimal, RSA-only descriptor */ - r1 = tor_malloc_zero(sizeof(routerinfo_t)); - r1->addr = 0xc0a80001u; /* 192.168.0.1 */ - r1->cache_info.published_on = 0; - r1->or_port = 9000; - r1->dir_port = 9003; - r1->supports_tunnelled_dir_requests = 1; - tor_addr_parse(&r1->ipv6_addr, "1:2:3:4::"); - r1->ipv6_orport = 9999; - router_set_rsa_onion_pkey(pk1, &r1->onion_pkey, &r1->onion_pkey_len); - /* Fake just enough of an ntor key to get by */ + /* Fake just enough of an ntor key to get by */ curve25519_keypair_t r1_onion_keypair; curve25519_keypair_generate(&r1_onion_keypair, 0); r1->onion_curve25519_pkey = tor_memdup(&r1_onion_keypair.pubkey, sizeof(curve25519_public_key_t)); - r1->identity_pkey = crypto_pk_dup_key(pk2); - r1->bandwidthrate = 1000; - r1->bandwidthburst = 5000; - r1->bandwidthcapacity = 10000; + + /* Now add IPv6 */ + tor_addr_parse(&r1->ipv6_addr, "1:2:3:4::"); + r1->ipv6_orport = 9999; + r1->exit_policy = NULL; - r1->nickname = tor_strdup("Magri"); - r1->platform = tor_strdup(platform); - tt_assert(!crypto_pk_write_public_key_to_string(pk1, &pk1_str, - &pk1_str_len)); - tt_assert(!crypto_pk_write_public_key_to_string(pk2 , &pk2_str, - &pk2_str_len)); - - /* XXXX+++ router_dump_to_string should really take this from ri.*/ + /* XXXX+++ router_dump_to_string should really take this from ri. */ options->ContactInfo = tor_strdup("Magri White " ""); /* Skip reachability checks for DirPort, ORPort, and tunnelled-dir-server */ options->AssumeReachable = 1; - /* Fake just enough of an ORPort and DirPort to get by */ - MOCK(get_configured_ports, mock_get_configured_ports); - mocked_configured_ports = smartlist_new(); + setup_mock_configured_ports(r1->or_port, r1->dir_port); - memset(&orport, 0, sizeof(orport)); - orport.type = CONN_TYPE_OR_LISTENER; - orport.addr.family = AF_INET; - orport.port = 9000; - smartlist_add(mocked_configured_ports, &orport); - - memset(&dirport, 0, sizeof(dirport)); - dirport.type = CONN_TYPE_DIR_LISTENER; - dirport.addr.family = AF_INET; - dirport.port = 9003; - smartlist_add(mocked_configured_ports, &dirport); - - buf = router_dump_router_to_string(r1, pk2, NULL, NULL, NULL); - - UNMOCK(get_configured_ports); - smartlist_free(mocked_configured_ports); - mocked_configured_ports = NULL; - - tor_free(options->ContactInfo); + buf = router_dump_router_to_string(r1, r1->identity_pkey, NULL, NULL, NULL); tt_assert(buf); - strlcpy(buf2, "router Magri 192.168.0.1 9000 0 9003\n" - "or-address [1:2:3:4::]:9999\n" - "platform Tor "VERSION" on ", sizeof(buf2)); - strlcat(buf2, get_uname(), sizeof(buf2)); - strlcat(buf2, "\n" - "published 1970-01-01 00:00:00\n" - "fingerprint ", sizeof(buf2)); - tt_assert(!crypto_pk_get_fingerprint(pk2, fingerprint, 1)); - strlcat(buf2, fingerprint, sizeof(buf2)); - strlcat(buf2, "\nuptime 0\n" - /* XXX the "0" above is hard-coded, but even if we made it reflect - * uptime, that still wouldn't make it right, because the two - * descriptors might be made on different seconds... hm. */ - "bandwidth 1000 5000 10000\n" - "onion-key\n", sizeof(buf2)); - strlcat(buf2, pk1_str, sizeof(buf2)); - strlcat(buf2, "signing-key\n", sizeof(buf2)); - strlcat(buf2, pk2_str, sizeof(buf2)); - strlcat(buf2, "hidden-service-dir\n", sizeof(buf2)); - strlcat(buf2, "contact Magri White \n", - sizeof(buf2)); - strlcat(buf2, "ntor-onion-key ", sizeof(buf2)); - base64_encode(cert_buf, sizeof(cert_buf), - (const char*)r1_onion_keypair.pubkey.public_key, 32, - BASE64_ENCODE_MULTILINE); - strlcat(buf2, cert_buf, sizeof(buf2)); - strlcat(buf2, "reject *:*\n", sizeof(buf2)); - strlcat(buf2, "tunnelled-dir-server\nrouter-signature\n", sizeof(buf2)); + tor_free(options->ContactInfo); + cleanup_mock_configured_ports(); + + /* Synthesise a router descriptor, without the signature */ + chunks = smartlist_new(); + + smartlist_add(chunks, get_new_router_line(r1)); + smartlist_add_strdup(chunks, "or-address [1:2:3:4::]:9999\n"); + + smartlist_add(chunks, get_new_platform_line()); + smartlist_add(chunks, get_new_published_line(r1)); + smartlist_add(chunks, get_new_fingerprint_line(r1)); + + smartlist_add(chunks, get_new_uptime_line(0)); + smartlist_add(chunks, get_new_bandwidth_line(r1)); + + smartlist_add(chunks, get_new_onion_key_block(r1)); + smartlist_add(chunks, get_new_signing_key_block(r1)); + + smartlist_add_strdup(chunks, "hidden-service-dir\n"); + + smartlist_add_strdup(chunks, "contact Magri White " + "\n"); + + smartlist_add(chunks, get_new_ntor_onion_key_line(&r1_onion_keypair.pubkey)); + smartlist_add_strdup(chunks, "reject *:*\n"); + smartlist_add_strdup(chunks, "tunnelled-dir-server\n"); + + smartlist_add_strdup(chunks, "router-signature\n"); + + size_t len_out = 0; + buf2 = smartlist_join_strings(chunks, "", 0, &len_out); + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); + + tt_assert(len_out > 0); + buf[strlen(buf2)] = '\0'; /* Don't compare the sig; it's never the same * twice */ tt_str_op(buf,OP_EQ, buf2); tor_free(buf); - buf = router_dump_router_to_string(r1, pk2, NULL, NULL, NULL); + setup_mock_configured_ports(r1->or_port, r1->dir_port); + + buf = router_dump_router_to_string(r1, r1->identity_pkey, NULL, NULL, NULL); tt_assert(buf); + + cleanup_mock_configured_ports(); + + /* Now, try to parse buf */ cp = buf; rp1 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); - tt_assert(rp1); - tt_int_op(rp1->addr,OP_EQ, r1->addr); - tt_int_op(rp1->or_port,OP_EQ, r1->or_port); - tt_int_op(rp1->dir_port,OP_EQ, r1->dir_port); - tt_int_op(rp1->bandwidthrate,OP_EQ, r1->bandwidthrate); - tt_int_op(rp1->bandwidthburst,OP_EQ, r1->bandwidthburst); - tt_int_op(rp1->bandwidthcapacity,OP_EQ, r1->bandwidthcapacity); - crypto_pk_t *onion_pkey = router_get_rsa_onion_pkey(rp1->onion_pkey, - rp1->onion_pkey_len); - tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk1), OP_EQ, 0); - crypto_pk_free(onion_pkey); - tt_int_op(crypto_pk_cmp_keys(rp1->identity_pkey, pk2), OP_EQ, 0); - tt_assert(rp1->supports_tunnelled_dir_requests); + + CHECK_ROUTERINFO_CONSISTENCY(r1, rp1); + tt_assert(rp1->policy_is_reject_star); tor_free(buf); @@ -391,41 +739,18 @@ test_dir_formats_rsa(void *arg) * too complex. Instead, we re-use the manually-created routerinfos. */ - /* router_build_fresh_signed_extrainfo() requires options->Nickname */ - tor_free(options->Nickname); - options->Nickname = tor_strdup(r1->nickname); + /* Set up standard mocks and data */ + setup_mocks_for_fresh_descriptor(r1, NULL); + /* router_build_fresh_signed_extrainfo() passes the result of * get_master_signing_key_cert() directly to tor_cert_dup(), which fails on * NULL. But we want a NULL ei->cache_info.signing_key_cert to test the * non-ed key path. */ MOCK(tor_cert_dup, mock_tor_cert_dup_null); - /* router_build_fresh_signed_extrainfo() requires get_server_identity_key(). - * Use the same one as the call to router_dump_router_to_string() above. - */ - mocked_server_identitykey = pk2; - MOCK(get_server_identity_key, mock_get_server_identity_key); - /* router_dump_and_sign_routerinfo_descriptor_body() requires - * get_onion_key(). Use the same one as r1. - */ - mocked_onionkey = pk1; - MOCK(get_onion_key, mock_get_onion_key); /* Fake just enough of an ORPort and DirPort to get by */ - MOCK(get_configured_ports, mock_get_configured_ports); - mocked_configured_ports = smartlist_new(); - - memset(&orport, 0, sizeof(orport)); - orport.type = CONN_TYPE_OR_LISTENER; - orport.addr.family = AF_INET; - orport.port = 9000; - smartlist_add(mocked_configured_ports, &orport); - - memset(&dirport, 0, sizeof(dirport)); - dirport.type = CONN_TYPE_DIR_LISTENER; - dirport.addr.family = AF_INET; - dirport.port = 9003; - smartlist_add(mocked_configured_ports, &dirport); + setup_mock_configured_ports(r1->or_port, r1->dir_port); /* Test some of the low-level static functions. */ e1 = router_build_fresh_signed_extrainfo(r1); @@ -436,38 +761,26 @@ test_dir_formats_rsa(void *arg) msg = ""; rv = routerinfo_incompatible_with_extrainfo(r1->identity_pkey, e1, &r1->cache_info, &msg); + /* If they are incompatible, fail and show the msg string */ tt_str_op(msg, OP_EQ, ""); tt_assert(rv == 0); /* Now cleanup */ - tor_free(options->Nickname); - UNMOCK(tor_cert_dup); - mocked_server_identitykey = NULL; - UNMOCK(get_server_identity_key); - mocked_onionkey = NULL; - UNMOCK(get_onion_key); + cleanup_mocks_for_fresh_descriptor(); - UNMOCK(get_configured_ports); - smartlist_free(mocked_configured_ports); - mocked_configured_ports = NULL; + UNMOCK(tor_cert_dup); + + cleanup_mock_configured_ports(); + + CHECK_EXTRAINFO_CONSISTENCY(r1, e1); /* Test that the signed ri is parseable */ tt_assert(r1->cache_info.signed_descriptor_body); cp = r1->cache_info.signed_descriptor_body; rp1 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); - tt_assert(rp1); - tt_int_op(rp1->addr,OP_EQ, r1->addr); - tt_int_op(rp1->or_port,OP_EQ, r1->or_port); - tt_int_op(rp1->dir_port,OP_EQ, r1->dir_port); - tt_int_op(rp1->bandwidthrate,OP_EQ, r1->bandwidthrate); - tt_int_op(rp1->bandwidthburst,OP_EQ, r1->bandwidthburst); - tt_int_op(rp1->bandwidthcapacity,OP_EQ, r1->bandwidthcapacity); - onion_pkey = router_get_rsa_onion_pkey(rp1->onion_pkey, - rp1->onion_pkey_len); - tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk1), OP_EQ, 0); - crypto_pk_free(onion_pkey); - tt_int_op(crypto_pk_cmp_keys(rp1->identity_pkey, pk2), OP_EQ, 0); - tt_assert(rp1->supports_tunnelled_dir_requests); + + CHECK_ROUTERINFO_CONSISTENCY(r1, rp1); + tt_assert(rp1->policy_is_reject_star); routerinfo_free(rp1); @@ -476,8 +789,9 @@ test_dir_formats_rsa(void *arg) tt_assert(e1->cache_info.signed_descriptor_body); cp = e1->cache_info.signed_descriptor_body; ep1 = extrainfo_parse_entry_from_string((const char*)cp,NULL,1,NULL,NULL); - tt_assert(ep1); - tt_str_op(ep1->nickname, OP_EQ, r1->nickname); + + CHECK_EXTRAINFO_CONSISTENCY(r1, ep1); + /* In future tests, we could check the actual extrainfo statistics. */ extrainfo_free(ep1); @@ -485,6 +799,17 @@ test_dir_formats_rsa(void *arg) done: dirserv_free_fingerprint_list(); + tor_free(options->ContactInfo); + tor_free(options->Nickname); + + cleanup_mock_configured_ports(); + cleanup_mocks_for_fresh_descriptor(); + + if (chunks) { + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); + } + routerinfo_free(r1); routerinfo_free(rp1); @@ -492,15 +817,31 @@ test_dir_formats_rsa(void *arg) extrainfo_free(ep1); tor_free(rsa_cc); + tor_free(buf); - tor_free(pk1_str); - tor_free(pk2_str); - crypto_pk_free(pk1); - crypto_pk_free(pk2); - tor_free(dir1); /* XXXX And more !*/ - tor_free(dir2); /* And more !*/ + tor_free(buf2); } +/* Check that the exit policy in rp2 is as expected. */ +#define CHECK_PARSED_EXIT_POLICY(rp2) \ +STMT_BEGIN \ + tt_int_op(smartlist_len(rp2->exit_policy),OP_EQ, 2); \ + \ + p = smartlist_get(rp2->exit_policy, 0); \ + tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_ACCEPT); \ + tt_assert(tor_addr_is_null(&p->addr)); \ + tt_int_op(p->maskbits,OP_EQ, 0); \ + tt_int_op(p->prt_min,OP_EQ, 80); \ + tt_int_op(p->prt_max,OP_EQ, 80); \ + \ + p = smartlist_get(rp2->exit_policy, 1); \ + tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_REJECT); \ + tt_assert(tor_addr_eq(&p->addr, &ex2->addr)); \ + tt_int_op(p->maskbits,OP_EQ, 8); \ + tt_int_op(p->prt_min,OP_EQ, 24); \ + tt_int_op(p->prt_max,OP_EQ, 24); \ +STMT_END + /** Run unit tests for router descriptor generation logic for a RSA + ed25519 * router. */ @@ -508,52 +849,47 @@ static void test_dir_formats_rsa_ed25519(void *arg) { char *buf = NULL; - char buf2[8192]; - char platform[256]; - char fingerprint[FINGERPRINT_LEN+1]; - char *pk1_str = NULL, *pk2_str = NULL, *cp; - size_t pk1_str_len, pk2_str_len; + char *buf2 = NULL; + char *cp = NULL; + + crypto_pk_t *r2_onion_pkey = NULL; + char cert_buf[256]; + uint8_t *rsa_cc = NULL; + time_t now = time(NULL); + routerinfo_t *r2 = NULL; extrainfo_t *e2 = NULL; - crypto_pk_t *pk1 = NULL, *pk2 = NULL; routerinfo_t *r2_out = NULL; routerinfo_t *rp2 = NULL; extrainfo_t *ep2 = NULL; addr_policy_t *ex1, *ex2; - routerlist_t *dir1 = NULL, *dir2 = NULL; - uint8_t *rsa_cc = NULL; - or_options_t *options = get_options_mutable(); const addr_policy_t *p; - time_t now = time(NULL); - port_cfg_t orport; - char cert_buf[256]; + + smartlist_t *chunks = NULL; int rv = -1; - (void)arg; - pk1 = pk_generate(0); - pk2 = pk_generate(1); + or_options_t *options = get_options_mutable(); - tt_assert(pk1 && pk2); + (void)arg; hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); - get_platform_str(platform, sizeof(platform)); + /* r2 is a RSA + ed25519 descriptor, with an exit policy, but no DirPort or + * IPv6 */ + r2 = basic_routerinfo_new("Fred", 0x0a030201u /* 10.3.2.1 */, + 9005, 0, + 3000, 3000, 3000, + 5, + &r2_onion_pkey); - /* We can't use init_mock_ed_keys() here, because the keys are seeded */ + /* Fake just enough of an ntor key to get by */ + curve25519_keypair_t r2_onion_keypair; + curve25519_keypair_generate(&r2_onion_keypair, 0); + r2->onion_curve25519_pkey = tor_memdup(&r2_onion_keypair.pubkey, + sizeof(curve25519_public_key_t)); - /* r2 is a RSA + ed25519 descriptor, with an exit policy */ - ex1 = tor_malloc_zero(sizeof(addr_policy_t)); - ex2 = tor_malloc_zero(sizeof(addr_policy_t)); - ex1->policy_type = ADDR_POLICY_ACCEPT; - tor_addr_from_ipv4h(&ex1->addr, 0); - ex1->maskbits = 0; - ex1->prt_min = ex1->prt_max = 80; - ex2->policy_type = ADDR_POLICY_REJECT; - tor_addr_from_ipv4h(&ex2->addr, 18<<24); - ex2->maskbits = 8; - ex2->prt_min = ex2->prt_max = 24; - r2 = tor_malloc_zero(sizeof(routerinfo_t)); - r2->addr = 0x0a030201u; /* 10.3.2.1 */ + /* Now add relay ed25519 keys + * We can't use init_mock_ed_keys() here, because the keys are seeded */ ed25519_keypair_t kp1, kp2; ed25519_secret_key_from_seed(&kp1.seckey, (const uint8_t*)"YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY"); @@ -566,72 +902,83 @@ test_dir_formats_rsa_ed25519(void *arg) &kp2.pubkey, now, 86400, CERT_FLAG_INCLUDE_SIGNING_KEY); - r2->platform = tor_strdup(platform); - r2->cache_info.published_on = 5; - r2->or_port = 9005; - r2->dir_port = 0; - r2->supports_tunnelled_dir_requests = 1; - router_set_rsa_onion_pkey(pk2, &r2->onion_pkey, &r2->onion_pkey_len); - curve25519_keypair_t r2_onion_keypair; - curve25519_keypair_generate(&r2_onion_keypair, 0); - r2->onion_curve25519_pkey = tor_memdup(&r2_onion_keypair.pubkey, - sizeof(curve25519_public_key_t)); - r2->identity_pkey = crypto_pk_dup_key(pk1); - r2->bandwidthrate = r2->bandwidthburst = r2->bandwidthcapacity = 3000; + + /* Now add an exit policy */ + ex1 = tor_malloc_zero(sizeof(addr_policy_t)); + ex2 = tor_malloc_zero(sizeof(addr_policy_t)); + ex1->policy_type = ADDR_POLICY_ACCEPT; + tor_addr_from_ipv4h(&ex1->addr, 0); + ex1->maskbits = 0; + ex1->prt_min = ex1->prt_max = 80; + ex2->policy_type = ADDR_POLICY_REJECT; + tor_addr_from_ipv4h(&ex2->addr, 18<<24); + ex2->maskbits = 8; + ex2->prt_min = ex2->prt_max = 24; + r2->exit_policy = smartlist_new(); smartlist_add(r2->exit_policy, ex1); smartlist_add(r2->exit_policy, ex2); - r2->nickname = tor_strdup("Fred"); - tt_assert(!crypto_pk_write_public_key_to_string(pk1, &pk1_str, - &pk1_str_len)); - tt_assert(!crypto_pk_write_public_key_to_string(pk2 , &pk2_str, - &pk2_str_len)); + /* Skip reachability checks for ORPort and tunnelled-dir-server */ + options->AssumeReachable = 1; - strlcpy(buf2, - "router Fred 10.3.2.1 9005 0 0\n" - "identity-ed25519\n" - "-----BEGIN ED25519 CERT-----\n", sizeof(buf2)); + /* Fake just enough of an ORPort to get by */ + setup_mock_configured_ports(r2->or_port, 0); + + buf = router_dump_router_to_string(r2, + r2->identity_pkey, r2_onion_pkey, + &r2_onion_keypair, &kp2); + tt_assert(buf); + + cleanup_mock_configured_ports(); + + chunks = smartlist_new(); + + /* Synthesise a router descriptor, without the signatures */ + smartlist_add(chunks, get_new_router_line(r2)); + + smartlist_add_strdup(chunks, + "identity-ed25519\n" + "-----BEGIN ED25519 CERT-----\n"); base64_encode(cert_buf, sizeof(cert_buf), (const char*)r2->cache_info.signing_key_cert->encoded, r2->cache_info.signing_key_cert->encoded_len, BASE64_ENCODE_MULTILINE); - strlcat(buf2, cert_buf, sizeof(buf2)); - strlcat(buf2, "-----END ED25519 CERT-----\n", sizeof(buf2)); - strlcat(buf2, "master-key-ed25519 ", sizeof(buf2)); + smartlist_add_strdup(chunks, cert_buf); + smartlist_add_strdup(chunks, "-----END ED25519 CERT-----\n"); + + smartlist_add_strdup(chunks, "master-key-ed25519 "); { char k[ED25519_BASE64_LEN+1]; tt_int_op(ed25519_public_to_base64(k, &r2->cache_info.signing_key_cert->signing_key), OP_GE, 0); - strlcat(buf2, k, sizeof(buf2)); - strlcat(buf2, "\n", sizeof(buf2)); + smartlist_add_strdup(chunks, k); + smartlist_add_strdup(chunks, "\n"); } - strlcat(buf2, "platform Tor "VERSION" on ", sizeof(buf2)); - strlcat(buf2, get_uname(), sizeof(buf2)); - strlcat(buf2, "\n" - "published 1970-01-01 00:00:05\n" - "fingerprint ", sizeof(buf2)); - tt_assert(!crypto_pk_get_fingerprint(pk1, fingerprint, 1)); - strlcat(buf2, fingerprint, sizeof(buf2)); - strlcat(buf2, "\nuptime 0\n" - "bandwidth 3000 3000 3000\n", sizeof(buf2)); - strlcat(buf2, "onion-key\n", sizeof(buf2)); - strlcat(buf2, pk2_str, sizeof(buf2)); - strlcat(buf2, "signing-key\n", sizeof(buf2)); - strlcat(buf2, pk1_str, sizeof(buf2)); + + smartlist_add(chunks, get_new_platform_line()); + smartlist_add(chunks, get_new_published_line(r2)); + smartlist_add(chunks, get_new_fingerprint_line(r2)); + + smartlist_add(chunks, get_new_uptime_line(0)); + smartlist_add(chunks, get_new_bandwidth_line(r2)); + + smartlist_add(chunks, get_new_onion_key_block(r2)); + smartlist_add(chunks, get_new_signing_key_block(r2)); + int rsa_cc_len; - rsa_cc = make_tap_onion_key_crosscert(pk2, + rsa_cc = make_tap_onion_key_crosscert(r2_onion_pkey, &kp1.pubkey, - pk1, + r2->identity_pkey, &rsa_cc_len); tt_assert(rsa_cc); base64_encode(cert_buf, sizeof(cert_buf), (char*)rsa_cc, rsa_cc_len, BASE64_ENCODE_MULTILINE); - strlcat(buf2, "onion-key-crosscert\n" - "-----BEGIN CROSSCERT-----\n", sizeof(buf2)); - strlcat(buf2, cert_buf, sizeof(buf2)); - strlcat(buf2, "-----END CROSSCERT-----\n", sizeof(buf2)); + smartlist_add_strdup(chunks, "onion-key-crosscert\n" + "-----BEGIN CROSSCERT-----\n"); + smartlist_add_strdup(chunks, cert_buf); + smartlist_add_strdup(chunks, "-----END CROSSCERT-----\n"); int ntor_cc_sign; { tor_cert_t *ntor_cc = NULL; @@ -646,89 +993,60 @@ test_dir_formats_rsa_ed25519(void *arg) BASE64_ENCODE_MULTILINE); tor_cert_free(ntor_cc); } - tor_snprintf(buf2+strlen(buf2), sizeof(buf2)-strlen(buf2), + smartlist_add_asprintf(chunks, "ntor-onion-key-crosscert %d\n" "-----BEGIN ED25519 CERT-----\n" "%s" "-----END ED25519 CERT-----\n", ntor_cc_sign, cert_buf); - strlcat(buf2, "hidden-service-dir\n", sizeof(buf2)); - strlcat(buf2, "ntor-onion-key ", sizeof(buf2)); - base64_encode(cert_buf, sizeof(cert_buf), - (const char*)r2_onion_keypair.pubkey.public_key, 32, - BASE64_ENCODE_MULTILINE); - strlcat(buf2, cert_buf, sizeof(buf2)); - strlcat(buf2, "accept *:80\nreject 18.0.0.0/8:24\n", sizeof(buf2)); - strlcat(buf2, "tunnelled-dir-server\n", sizeof(buf2)); - strlcat(buf2, "router-sig-ed25519 ", sizeof(buf2)); + smartlist_add_strdup(chunks, "hidden-service-dir\n"); - /* Skip reachability checks for ORPort and tunnelled-dir-server */ - options->AssumeReachable = 1; + smartlist_add(chunks, get_new_ntor_onion_key_line(&r2_onion_keypair.pubkey)); + smartlist_add_strdup(chunks, "accept *:80\nreject 18.0.0.0/8:24\n"); + smartlist_add_strdup(chunks, "tunnelled-dir-server\n"); - /* Fake just enough of an ORPort to get by */ - MOCK(get_configured_ports, mock_get_configured_ports); - mocked_configured_ports = smartlist_new(); + smartlist_add_strdup(chunks, "router-sig-ed25519 "); - memset(&orport, 0, sizeof(orport)); - orport.type = CONN_TYPE_OR_LISTENER; - orport.addr.family = AF_INET; - orport.port = 9005; - smartlist_add(mocked_configured_ports, &orport); + size_t len_out = 0; + buf2 = smartlist_join_strings(chunks, "", 0, &len_out); + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); - buf = router_dump_router_to_string(r2, pk1, pk2, &r2_onion_keypair, &kp2); - tt_assert(buf); - buf[strlen(buf2)] = '\0'; /* Don't compare the sig; it's never the same + tt_assert(len_out > 0); + + buf[strlen(buf2)] = '\0'; /* Don't compare either sig; they're never the same * twice */ tt_str_op(buf, OP_EQ, buf2); tor_free(buf); - buf = router_dump_router_to_string(r2, pk1, NULL, NULL, NULL); + setup_mock_configured_ports(r2->or_port, 0); - UNMOCK(get_configured_ports); - smartlist_free(mocked_configured_ports); - mocked_configured_ports = NULL; + buf = router_dump_router_to_string(r2, r2->identity_pkey, NULL, NULL, NULL); + tt_assert(buf); - /* Reset for later */ + cleanup_mock_configured_ports(); + + /* Now, try to parse buf */ cp = buf; rp2 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); - tt_assert(rp2); - tt_int_op(rp2->addr,OP_EQ, r2->addr); - tt_int_op(rp2->or_port,OP_EQ, r2->or_port); - tt_int_op(rp2->dir_port,OP_EQ, r2->dir_port); - tt_int_op(rp2->bandwidthrate,OP_EQ, r2->bandwidthrate); - tt_int_op(rp2->bandwidthburst,OP_EQ, r2->bandwidthburst); - tt_int_op(rp2->bandwidthcapacity,OP_EQ, r2->bandwidthcapacity); + + CHECK_ROUTERINFO_CONSISTENCY(r2, rp2); + tt_mem_op(rp2->onion_curve25519_pkey->public_key,OP_EQ, r2->onion_curve25519_pkey->public_key, CURVE25519_PUBKEY_LEN); - crypto_pk_t *onion_pkey = router_get_rsa_onion_pkey(rp2->onion_pkey, - rp2->onion_pkey_len); - tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk2), OP_EQ, 0); - crypto_pk_free(onion_pkey); - tt_int_op(crypto_pk_cmp_keys(rp2->identity_pkey, pk1), OP_EQ, 0); - tt_assert(rp2->supports_tunnelled_dir_requests); - tt_int_op(smartlist_len(rp2->exit_policy),OP_EQ, 2); - - p = smartlist_get(rp2->exit_policy, 0); - tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_ACCEPT); - tt_assert(tor_addr_is_null(&p->addr)); - tt_int_op(p->maskbits,OP_EQ, 0); - tt_int_op(p->prt_min,OP_EQ, 80); - tt_int_op(p->prt_max,OP_EQ, 80); - - p = smartlist_get(rp2->exit_policy, 1); - tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_REJECT); - tt_assert(tor_addr_eq(&p->addr, &ex2->addr)); - tt_int_op(p->maskbits,OP_EQ, 8); - tt_int_op(p->prt_min,OP_EQ, 24); - tt_int_op(p->prt_max,OP_EQ, 24); + CHECK_PARSED_EXIT_POLICY(rp2); + tor_free(buf); routerinfo_free(rp2); /* Test extrainfo creation. */ + /* Set up standard mocks and data */ + setup_mocks_for_fresh_descriptor(r2, r2_onion_pkey); + /* router_build_fresh_descriptor() requires * router_build_fresh_unsigned_routerinfo(), but the implementation is * too complex. Instead, we re-use r2. @@ -737,21 +1055,6 @@ test_dir_formats_rsa_ed25519(void *arg) MOCK(router_build_fresh_unsigned_routerinfo, mock_router_build_fresh_unsigned_routerinfo); - /* router_build_fresh_signed_extrainfo() requires options->Nickname */ - tor_free(options->Nickname); - options->Nickname = tor_strdup(r2->nickname); - /* router_build_fresh_signed_extrainfo() requires get_server_identity_key(). - * Use the same one as the call to router_dump_router_to_string() above. - * For the second router, the keys are swapped. - */ - mocked_server_identitykey = pk1; - MOCK(get_server_identity_key, mock_get_server_identity_key); - /* router_dump_and_sign_routerinfo_descriptor_body() requires - * get_onion_key(). Use the same one as r1. - */ - mocked_onionkey = pk2; - MOCK(get_onion_key, mock_get_onion_key); - /* r2 uses ed25519, so we need to mock the ed key functions */ mocked_master_signing_key = &kp2; MOCK(get_master_signing_keypair, mock_get_master_signing_keypair); @@ -763,14 +1066,7 @@ test_dir_formats_rsa_ed25519(void *arg) MOCK(get_current_curve25519_keypair, mock_get_current_curve25519_keypair); /* Fake just enough of an ORPort to get by */ - MOCK(get_configured_ports, mock_get_configured_ports); - mocked_configured_ports = smartlist_new(); - - memset(&orport, 0, sizeof(orport)); - orport.type = CONN_TYPE_OR_LISTENER; - orport.addr.family = AF_INET; - orport.port = 9005; - smartlist_add(mocked_configured_ports, &orport); + setup_mock_configured_ports(r2->or_port, 0); /* Test the high-level interface. */ rv = router_build_fresh_descriptor(&r2_out, &e2); @@ -789,13 +1085,10 @@ test_dir_formats_rsa_ed25519(void *arg) r2_out = NULL; /* Now cleanup */ + cleanup_mocks_for_fresh_descriptor(); + mocked_routerinfo = NULL; UNMOCK(router_build_fresh_unsigned_routerinfo); - tor_free(options->Nickname); - mocked_server_identitykey = NULL; - UNMOCK(get_server_identity_key); - mocked_onionkey = NULL; - UNMOCK(get_onion_key); mocked_master_signing_key = NULL; UNMOCK(get_master_signing_keypair); mocked_signing_key_cert = NULL; @@ -803,46 +1096,22 @@ test_dir_formats_rsa_ed25519(void *arg) mocked_curve25519_onion_key = NULL; UNMOCK(get_current_curve25519_keypair); - UNMOCK(get_configured_ports); - smartlist_free(mocked_configured_ports); - mocked_configured_ports = NULL; + cleanup_mock_configured_ports(); + + CHECK_EXTRAINFO_CONSISTENCY(r2, e2); /* Test that the signed ri is parseable */ tt_assert(r2->cache_info.signed_descriptor_body); cp = r2->cache_info.signed_descriptor_body; rp2 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL); - tt_assert(rp2); - tt_int_op(rp2->addr,OP_EQ, r2->addr); - tt_int_op(rp2->or_port,OP_EQ, r2->or_port); - tt_int_op(rp2->dir_port,OP_EQ, r2->dir_port); - tt_int_op(rp2->bandwidthrate,OP_EQ, r2->bandwidthrate); - tt_int_op(rp2->bandwidthburst,OP_EQ, r2->bandwidthburst); - tt_int_op(rp2->bandwidthcapacity,OP_EQ, r2->bandwidthcapacity); + + CHECK_ROUTERINFO_CONSISTENCY(r2, rp2); + tt_mem_op(rp2->onion_curve25519_pkey->public_key,OP_EQ, r2->onion_curve25519_pkey->public_key, CURVE25519_PUBKEY_LEN); - onion_pkey = router_get_rsa_onion_pkey(rp2->onion_pkey, - rp2->onion_pkey_len); - tt_int_op(crypto_pk_cmp_keys(onion_pkey, pk2), OP_EQ, 0); - crypto_pk_free(onion_pkey); - tt_int_op(crypto_pk_cmp_keys(rp2->identity_pkey, pk1), OP_EQ, 0); - tt_assert(rp2->supports_tunnelled_dir_requests); - tt_int_op(smartlist_len(rp2->exit_policy),OP_EQ, 2); - - p = smartlist_get(rp2->exit_policy, 0); - tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_ACCEPT); - tt_assert(tor_addr_is_null(&p->addr)); - tt_int_op(p->maskbits,OP_EQ, 0); - tt_int_op(p->prt_min,OP_EQ, 80); - tt_int_op(p->prt_max,OP_EQ, 80); - - p = smartlist_get(rp2->exit_policy, 1); - tt_int_op(p->policy_type,OP_EQ, ADDR_POLICY_REJECT); - tt_assert(tor_addr_eq(&p->addr, &ex2->addr)); - tt_int_op(p->maskbits,OP_EQ, 8); - tt_int_op(p->prt_min,OP_EQ, 24); - tt_int_op(p->prt_max,OP_EQ, 24); + CHECK_PARSED_EXIT_POLICY(rp2); routerinfo_free(rp2); @@ -850,27 +1119,26 @@ test_dir_formats_rsa_ed25519(void *arg) tt_assert(e2->cache_info.signed_descriptor_body); cp = e2->cache_info.signed_descriptor_body; ep2 = extrainfo_parse_entry_from_string((const char*)cp,NULL,1,NULL,NULL); - tt_assert(ep2); - tt_str_op(ep2->nickname, OP_EQ, r2->nickname); + + CHECK_EXTRAINFO_CONSISTENCY(r2, ep2); + /* In future tests, we could check the actual extrainfo statistics. */ extrainfo_free(ep2); -#if 0 - /* Okay, now for the directories. */ - { - fingerprint_list = smartlist_new(); - crypto_pk_get_fingerprint(pk2, buf, 1); - add_fingerprint_to_dir(buf, fingerprint_list, 0); - crypto_pk_get_fingerprint(pk1, buf, 1); - add_fingerprint_to_dir(buf, fingerprint_list, 0); - } - -#endif /* 0 */ - done: dirserv_free_fingerprint_list(); + tor_free(options->Nickname); + + cleanup_mock_configured_ports(); + cleanup_mocks_for_fresh_descriptor(); + + if (chunks) { + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); + } + routerinfo_free(r2); routerinfo_free(r2_out); routerinfo_free(rp2); @@ -879,13 +1147,10 @@ test_dir_formats_rsa_ed25519(void *arg) extrainfo_free(ep2); tor_free(rsa_cc); + crypto_pk_free(r2_onion_pkey); + tor_free(buf); - tor_free(pk1_str); - tor_free(pk2_str); - crypto_pk_free(pk1); - crypto_pk_free(pk2); - tor_free(dir1); /* XXXX And more !*/ - tor_free(dir2); /* And more !*/ + tor_free(buf2); } #include "failing_routerdescs.inc" From 39ab6c9f7360b77901efefae7dd4b18d5df47b90 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 19 Feb 2019 15:56:09 +1000 Subject: [PATCH 14/17] test_dir: Test descriptor variants Including: * relays and bridges, * no stats, basic stats, and all stats Part of 29017 and 29018. --- src/test/test_dir.c | 81 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index cc0ef07fbf..210b09a4e7 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -411,6 +411,19 @@ get_new_ntor_onion_key_line(const curve25519_public_key_t *ntor_onion_pubkey) return line; } +/* Allocate and return a new string containing a "bridge-distribution-request" + * line for options. + */ +static char * +get_new_bridge_distribution_request_line(const or_options_t *options) +{ + if (options->BridgeRelay) { + return tor_strdup("bridge-distribution-request any\n"); + } else { + return tor_strdup(""); + } +} + static smartlist_t *mocked_configured_ports = NULL; /** Returns mocked_configured_ports */ @@ -577,6 +590,43 @@ setup_mocks_for_fresh_descriptor(const routerinfo_t *r1, MOCK(get_onion_key, mock_get_onion_key); } +/* Set options based on arg. + * + * b: BridgeRelay 1 + * e: ExtraInfoStatistics 1 + * s: sets all the individual statistics options to 1 + * + * Always sets AssumeReachable to 1. + * + * Does not set ServerTransportPlugin, because it's parsed before use. + * + * Does not set BridgeRecordUsageByCountry, because the tests don't have access + * to a GeoIPFile or GeoIPv6File. */ +static void +setup_dir_formats_options(const char *arg, or_options_t *options) +{ + /* Skip reachability checks for DirPort, ORPort, and tunnelled-dir-server */ + options->AssumeReachable = 1; + + if (strchr(arg, 'b')) { + options->BridgeRelay = 1; + } + + if (strchr(arg, 'e')) { + options->ExtraInfoStatistics = 1; + } + + if (strchr(arg, 's')) { + options->DirReqStatistics = 1; + options->HiddenServiceStatistics = 1; + options->EntryStatistics = 1; + options->CellStatistics = 1; + options->ExitPortStatistics = 1; + options->ConnDirectionStatistics = 1; + options->PaddingStatistics = 1; + } +} + /* Check that routerinfos r1 and rp1 are consistent. * Only performs some basic checks. */ @@ -638,8 +688,7 @@ test_dir_formats_rsa(void *arg) int rv = -1; or_options_t *options = get_options_mutable(); - - (void)arg; + setup_dir_formats_options((const char *)arg, options); hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); @@ -665,8 +714,6 @@ test_dir_formats_rsa(void *arg) /* XXXX+++ router_dump_to_string should really take this from ri. */ options->ContactInfo = tor_strdup("Magri White " ""); - /* Skip reachability checks for DirPort, ORPort, and tunnelled-dir-server */ - options->AssumeReachable = 1; setup_mock_configured_ports(r1->or_port, r1->dir_port); @@ -697,6 +744,7 @@ test_dir_formats_rsa(void *arg) smartlist_add_strdup(chunks, "contact Magri White " "\n"); + smartlist_add(chunks, get_new_bridge_distribution_request_line(options)); smartlist_add(chunks, get_new_ntor_onion_key_line(&r1_onion_keypair.pubkey)); smartlist_add_strdup(chunks, "reject *:*\n"); smartlist_add_strdup(chunks, "tunnelled-dir-server\n"); @@ -869,8 +917,7 @@ test_dir_formats_rsa_ed25519(void *arg) int rv = -1; or_options_t *options = get_options_mutable(); - - (void)arg; + setup_dir_formats_options((const char *)arg, options); hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE); @@ -919,9 +966,6 @@ test_dir_formats_rsa_ed25519(void *arg) smartlist_add(r2->exit_policy, ex1); smartlist_add(r2->exit_policy, ex2); - /* Skip reachability checks for ORPort and tunnelled-dir-server */ - options->AssumeReachable = 1; - /* Fake just enough of an ORPort to get by */ setup_mock_configured_ports(r2->or_port, 0); @@ -1001,6 +1045,7 @@ test_dir_formats_rsa_ed25519(void *arg) smartlist_add_strdup(chunks, "hidden-service-dir\n"); + smartlist_add(chunks, get_new_bridge_distribution_request_line(options)); smartlist_add(chunks, get_new_ntor_onion_key_line(&r2_onion_keypair.pubkey)); smartlist_add_strdup(chunks, "accept *:80\nreject 18.0.0.0/8:24\n"); smartlist_add_strdup(chunks, "tunnelled-dir-server\n"); @@ -7119,8 +7164,22 @@ test_dir_format_versions_list(void *arg) struct testcase_t dir_tests[] = { DIR_LEGACY(nicknames), - DIR_LEGACY(formats_rsa), - DIR_LEGACY(formats_rsa_ed25519), + /* extrainfo without any stats */ + DIR_ARG(formats_rsa, TT_FORK, ""), + DIR_ARG(formats_rsa_ed25519, TT_FORK, ""), + /* on a bridge */ + DIR_ARG(formats_rsa, TT_FORK, "b"), + DIR_ARG(formats_rsa_ed25519, TT_FORK, "b"), + /* extrainfo with basic stats */ + DIR_ARG(formats_rsa, TT_FORK, "e"), + DIR_ARG(formats_rsa_ed25519, TT_FORK, "e"), + DIR_ARG(formats_rsa, TT_FORK, "be"), + DIR_ARG(formats_rsa_ed25519, TT_FORK, "be"), + /* extrainfo with all stats */ + DIR_ARG(formats_rsa, TT_FORK, "es"), + DIR_ARG(formats_rsa_ed25519, TT_FORK, "es"), + DIR_ARG(formats_rsa, TT_FORK, "bes"), + DIR_ARG(formats_rsa_ed25519, TT_FORK, "bes"), DIR(routerinfo_parsing, 0), DIR(extrainfo_parsing, 0), DIR(parse_router_list, TT_FORK), From 0c0f21582285bec0fe68f71e293158eeedaeeaa3 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 19 Feb 2019 18:52:54 +1000 Subject: [PATCH 15/17] routerkeys: Log failures at info-level in make_tap_onion_key_crosscert() --- src/feature/relay/routerkeys.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/feature/relay/routerkeys.c b/src/feature/relay/routerkeys.c index bdd7a82b58..d965777ad6 100644 --- a/src/feature/relay/routerkeys.c +++ b/src/feature/relay/routerkeys.c @@ -706,6 +706,8 @@ make_tap_onion_key_crosscert(const crypto_pk_t *onion_key, *len_out = 0; if (crypto_pk_get_digest(rsa_id_key, (char*)signed_data) < 0) { + log_info(LD_OR, "crypto_pk_get_digest failed in " + "make_tap_onion_key_crosscert!"); return NULL; } memcpy(signed_data + DIGEST_LEN, master_id_key->pubkey, ED25519_PUBKEY_LEN); @@ -713,8 +715,12 @@ make_tap_onion_key_crosscert(const crypto_pk_t *onion_key, int r = crypto_pk_private_sign(onion_key, (char*)signature, sizeof(signature), (const char*)signed_data, sizeof(signed_data)); - if (r < 0) + if (r < 0) { + /* It's probably missing the private key */ + log_info(LD_OR, "crypto_pk_private_sign failed in " + "make_tap_onion_key_crosscert!"); return NULL; + } *len_out = r; From 51f59f213e3850d1d77f0e60355d914c6583aeb4 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 19 Feb 2019 21:23:27 +1000 Subject: [PATCH 16/17] router: Add some missing #endif comments --- src/feature/relay/router.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h index f17e6a9438..55b9ef9e68 100644 --- a/src/feature/relay/router.h +++ b/src/feature/relay/router.h @@ -134,8 +134,8 @@ STATIC extrainfo_t *router_build_fresh_signed_extrainfo( STATIC void router_update_routerinfo_from_extrainfo(routerinfo_t *ri, const extrainfo_t *ei); STATIC int router_dump_and_sign_routerinfo_descriptor_body(routerinfo_t *ri); -#endif +#endif /* defined(TOR_UNIT_TESTS) */ -#endif +#endif /* defined(ROUTER_PRIVATE) */ #endif /* !defined(TOR_ROUTER_H) */ From 6c652eae0a95cae095f5adad9cc51e1f1a183245 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 20 Feb 2019 00:40:18 +1000 Subject: [PATCH 17/17] fixup! test_dir: Refactor common code out of the dir_format unit tests --- src/test/test_dir.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 210b09a4e7..31471fb7d6 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -355,8 +355,9 @@ get_new_rsa_key_block(const char *key_name, crypto_pk_t *pk1) "%s\n%s", key_name, pk1_str); - tor_assert(block); + tor_free(pk1_str); + tor_assert(block); return block; }