commit 6c5a506cdb887a655d8a4654fba5f6ea466aaeae Author: teor teor@torproject.org Date: Thu Jan 10 17:11:26 2019 +1000
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 93bc8c96c..e57e9fb8d 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(!