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.

1 files changed, 38 insertions, 42 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 <b>extrainfo</b> and aggregated statistics to
- * *<b>s_out</b>, signing them with <b>ident_key</b>. Return 0 on
- * success, negative on failure. */
+/** Write the contents of <b>extrainfo</b>, to * *<b>s_out</b>, signing them
+ * with <b>ident_key</b>.
+ *
+ * If ExtraInfoStatistics is 1, also write aggregated statistics and related
+ * configuration data before signing. Most statistics also have an option that
+ * enables or disables that particular statistic.
+ *
+ * Return 0 on success, negative on failure. */
 int
 extrainfo_dump_to_string(char **s_out, extrainfo_t *extrainfo,
                          crypto_pk_t *ident_key,