Merge branch 'tor-github/pr/709'

10 files changed, 1276 insertions, 364 deletions

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 ee91976066..ac79ed231b 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/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 <b>cert</b> */
-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 cdd032f78d..cbd5f2f91e 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);
@@ -194,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;
@@ -269,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 *
@@ -374,8 +377,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()));
@@ -1941,26 +1944,33 @@ 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.
+/** 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.
  */
-int
-router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
+MOCK_IMPL(STATIC int,
+router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out))
 {
-  routerinfo_t *ri;
-  extrainfo_t *ei;
+  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
@@ -2017,8 +2027,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
   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());
@@ -2057,85 +2067,258 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
 
   ri->declared_family = get_my_declared_family(options);
 
+  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;
+
+ err:
+  routerinfo_free(ri);
+  *ri_out = NULL;
+  return result;
+
+ done:
+  *ri_out = ri;
+  return 0;
+}
+
+/** 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_unsigned_extrainfo(const routerinfo_t *ri)
+{
+  extrainfo_t *ei = NULL;
+  const or_options_t *options = get_options();
+
+  if (BUG(!ri))
+    return NULL;
+
   /* 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());
 
   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;
+}
+
+/** 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_dump_and_sign_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) {
     log_warn(LD_BUG, "Couldn't generate extra-info descriptor.");
-    extrainfo_free(ei);
-    ei = NULL;
-  } 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 -1;
   }
 
-  /* 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. */
+  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;
+}
+
+/** 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_fresh_signed_extrainfo(const routerinfo_t *ri)
+{
+  int result = -1;
+  extrainfo_t *ei = NULL;
+
+  if (BUG(!ri))
+    return NULL;
+
+  ei = router_build_fresh_unsigned_extrainfo(ri);
+  /* router_build_fresh_unsigned_extrainfo() should not fail. */
+  if (BUG(!ei))
+    goto err;
+
+  result = router_dump_and_sign_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.
+ */
+STATIC void
+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. */
+    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);
+}
+
+/** 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_dump_and_sign_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);
 
-  ri->purpose =
-    options->BridgeRelay ? ROUTER_PURPOSE_BRIDGE : ROUTER_PURPOSE_GENERAL;
-  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;
-  }
-
   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;
+}
+
+/** Build a fresh routerinfo, signed server descriptor, and signed extrainfo
+ * document for this OR.
+ *
+ * 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.
+ * Caller is responsible for freeing generated documents on success.
+ */
+int
+router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
+{
+  int result = TOR_ROUTERINFO_ERROR_INTERNAL_BUG;
+  routerinfo_t *ri = NULL;
+  extrainfo_t *ei = NULL;
+
+  if (BUG(!r))
+    goto err;
+
+  if (BUG(!e))
+    goto err;
+
+  result = router_build_fresh_unsigned_routerinfo(&ri);
+  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;
+  }
+
+  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_dump_and_sign_routerinfo_descriptor_body(ri);
+  if (result < 0)
+    goto err;
+
   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;
@@ -2478,6 +2661,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,
@@ -2930,9 +3117,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,
@@ -2942,7 +3134,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 +3148,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 +3173,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 +3227,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 +3331,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;
 }
diff --git a/src/feature/relay/router.h b/src/feature/relay/router.h
index 60bc857ceb..55b9ef9e68 100644
--- a/src/feature/relay/router.h
+++ b/src/feature/relay/router.h
@@ -23,11 +23,12 @@ 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);
+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);
@@ -114,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);
@@ -123,8 +124,18 @@ 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);
-#endif
-
-#endif
+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));
+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 /* defined(TOR_UNIT_TESTS) */
+
+#endif /* defined(ROUTER_PRIVATE) */
 
 #endif /* !defined(TOR_ROUTER_H) */
diff --git a/src/feature/relay/routerkeys.c b/src/feature/relay/routerkeys.c
index 876f908d41..d965777ad6 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;
 }
@@ -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;
 
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 07a2641c9f..07362c2f60 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -162,6 +162,269 @@ 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_free(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;
+}
+
+/* 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 */
@@ -171,71 +434,510 @@ mock_get_configured_ports(void)
   return mocked_configured_ports;
 }
 
-/** Run unit tests for router descriptor generation logic. */
+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;
+}
+
+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;
+}
+
+/* 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
-test_dir_formats(void *arg)
+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);
+}
+
+/* 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.
+ */
+#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.
+ */
+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;
-  routerinfo_t *r1=NULL, *r2=NULL;
-  crypto_pk_t *pk1 = NULL, *pk2 = NULL;
-  routerinfo_t *rp1 = NULL, *rp2 = NULL;
-  addr_policy_t *ex1, *ex2;
-  routerlist_t *dir1 = NULL, *dir2 = NULL;
+  char *buf2 = NULL;
+  char *cp = 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];
 
-  (void)arg;
-  pk1 = pk_generate(0);
-  pk2 = pk_generate(1);
+  routerinfo_t *r1 = NULL;
+  extrainfo_t *e1 = NULL;
+  routerinfo_t *rp1 = NULL;
+  extrainfo_t *ep1 = NULL;
 
-  tt_assert(pk1 && pk2);
+  smartlist_t *chunks = NULL;
+  const char *msg = NULL;
+  int rv = -1;
+
+  or_options_t *options = get_options_mutable();
+  setup_dir_formats_options((const char *)arg, options);
 
   hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE);
 
-  get_platform_str(platform, sizeof(platform));
-  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 */
+  /* 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);
+
+ /* 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);
 
-  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 */
+  /* XXXX+++ router_dump_to_string should really take this from ri. */
+  options->ContactInfo = tor_strdup("Magri White "
+                                    "<magri@elsewhere.example.com>");
+
+  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);
+
+  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 "
+                               "<magri@elsewhere.example.com>\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");
+
+  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);
+
+  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);
+
+  CHECK_ROUTERINFO_CONSISTENCY(r1, rp1);
+
+  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.
+   */
+
+  /* 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);
+
+  /* Fake just enough of an ORPort and DirPort to get by */
+  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);
+  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);
+  /* If they are incompatible, fail and show the msg string */
+  tt_str_op(msg, OP_EQ, "");
+  tt_assert(rv == 0);
+
+  /* Now cleanup */
+  cleanup_mocks_for_fresh_descriptor();
+
+  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);
+
+  CHECK_ROUTERINFO_CONSISTENCY(r1, rp1);
+
+  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);
+
+  CHECK_EXTRAINFO_CONSISTENCY(r1, ep1);
+
+  /* In future tests, we could check the actual extrainfo statistics. */
+
+  extrainfo_free(ep1);
+
+ 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);
+
+  extrainfo_free(e1);
+  extrainfo_free(ep1);
+
+  tor_free(rsa_cc);
+
+  tor_free(buf);
+  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.
+ */
+static void
+test_dir_formats_rsa_ed25519(void *arg)
+{
+  char *buf = NULL;
+  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;
+  routerinfo_t *r2_out = NULL;
+  routerinfo_t *rp2 = NULL;
+  extrainfo_t *ep2 = NULL;
+  addr_policy_t *ex1, *ex2;
+  const addr_policy_t *p;
+
+  smartlist_t *chunks = NULL;
+  int rv = -1;
+
+  or_options_t *options = get_options_mutable();
+  setup_dir_formats_options((const char *)arg, options);
+
+  hibernate_set_state_for_testing_(HIBERNATE_STATE_LIVE);
+
+  /* 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);
+
+  /* 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));
+
+  /* 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");
@@ -248,157 +950,80 @@ test_dir_formats(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));
-
-  /* XXXX+++ router_dump_to_string should really take this from ri.*/
-  options->ContactInfo = tor_strdup("Magri White "
-                                    "<magri@elsewhere.example.com>");
-  /* Skip reachability checks for DirPort 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();
-
-  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;
+  /* Fake just enough of an ORPort to get by */
+  setup_mock_configured_ports(r2->or_port, 0);
 
-  tor_free(options->ContactInfo);
+  buf = router_dump_router_to_string(r2,
+                                     r2->identity_pkey, r2_onion_pkey,
+                                     &r2_onion_keypair, &kp2);
   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 <magri@elsewhere.example.com>\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));
-  buf[strlen(buf2)] = '\0'; /* Don't compare the sig; it's never the same
-                             * twice */
+  cleanup_mock_configured_ports();
 
-  tt_str_op(buf,OP_EQ, buf2);
-  tor_free(buf);
+  chunks = smartlist_new();
 
-  buf = router_dump_router_to_string(r1, pk2, NULL, NULL, NULL);
-  tt_assert(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);
-  //tt_assert(rp1->exit_policy == NULL);
-  tor_free(buf);
+  /* Synthesise a router descriptor, without the signatures */
+  smartlist_add(chunks, get_new_router_line(r2));
 
-  strlcpy(buf2,
-          "router Fred 10.3.2.1 9005 0 0\n"
-          "identity-ed25519\n"
-          "-----BEGIN ED25519 CERT-----\n", sizeof(buf2));
+  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;
@@ -413,112 +1038,165 @@ test_dir_formats(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");
 
-  /* Fake just enough of an ORPort to get by */
-  MOCK(get_configured_ports, mock_get_configured_ports);
-  mocked_configured_ports = smartlist_new();
+  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");
 
-  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);
+  smartlist_add_strdup(chunks, "router-sig-ed25519 ");
 
-  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
+  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 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);
-  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);
-
-#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);
+
+  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.
+   */
+  mocked_routerinfo = r2;
+  MOCK(router_build_fresh_unsigned_routerinfo,
+       mock_router_build_fresh_unsigned_routerinfo);
+
+  /* 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 */
+  setup_mock_configured_ports(r2->or_port, 0);
+
+  /* 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 */
+  cleanup_mocks_for_fresh_descriptor();
+
+  mocked_routerinfo = NULL;
+  UNMOCK(router_build_fresh_unsigned_routerinfo);
+  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);
+
+  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);
 
-#endif /* 0 */
-  dirserv_free_fingerprint_list();
+  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);
+
+  CHECK_PARSED_EXIT_POLICY(rp2);
+
+  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);
+
+  CHECK_EXTRAINFO_CONSISTENCY(r2, ep2);
+
+  /* In future tests, we could check the actual extrainfo statistics. */
+
+  extrainfo_free(ep2);
 
  done:
-  if (r1)
-    routerinfo_free(r1);
-  if (r2)
-    routerinfo_free(r2);
-  if (rp2)
-    routerinfo_free(rp2);
+  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);
+
+  extrainfo_free(e2);
+  extrainfo_free(ep2);
 
   tor_free(rsa_cc);
+  crypto_pk_free(r2_onion_pkey);
+
   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);
-  tor_free(dir1); /* XXXX And more !*/
-  tor_free(dir2); /* And more !*/
+  tor_free(buf2);
 }
 
 #include "failing_routerdescs.inc"
@@ -6546,7 +7224,22 @@ test_dir_format_versions_list(void *arg)
 
 struct testcase_t dir_tests[] = {
   DIR_LEGACY(nicknames),
-  DIR_LEGACY(formats),
+  /* 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),
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