Merge branch 'tor-github/pr/709'

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.

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**::

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)

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);
 

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;
 }

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) */

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;
 

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);

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