Merge branch 'ticket27246_035_01_squashed'

changes/ticket27246

@@ -0,0 +1,4 @@
+  o Minor features (memory usage):
+    - When not using them, store legacy TAP public onion keys in
+      DER-encoded format, rather than as expanded public keys. This should
+      save several megabytes on typical clients. Closes ticket 27246.

src/core/or/circuitbuild.c

@@ -2776,6 +2776,8 @@ extend_info_new(const char *nickname,
 extend_info_t *
 extend_info_from_node(const node_t *node, int for_direct_connect)
 {
+  crypto_pk_t *rsa_pubkey = NULL;
+  extend_info_t *info = NULL;
   tor_addr_port_t ap;
   int valid_addr = 0;
 
@@ -2823,25 +2825,28 @@ extend_info_from_node(const node_t *node, int for_direct_connect)
   /* Retrieve the curve25519 pubkey. */
   const curve25519_public_key_t *curve_pubkey =
     node_get_curve25519_onion_key(node);
+  rsa_pubkey = node_get_rsa_onion_key(node);
 
-  if (valid_addr && node->ri)
-    return extend_info_new(node->ri->nickname,
+  if (valid_addr && node->ri) {
+    info = extend_info_new(node->ri->nickname,
                            node->identity,
                            ed_pubkey,
-                           node->ri->onion_pkey,
+                           rsa_pubkey,
                            curve_pubkey,
                            &ap.addr,
                            ap.port);
-  else if (valid_addr && node->rs && node->md)
-    return extend_info_new(node->rs->nickname,
+  } else if (valid_addr && node->rs && node->md) {
+    info = extend_info_new(node->rs->nickname,
                            node->identity,
                            ed_pubkey,
-                           node->md->onion_pkey,
+                           rsa_pubkey,
                            curve_pubkey,
                            &ap.addr,
                            ap.port);
-  else
-    return NULL;
+  }
+
+  crypto_pk_free(rsa_pubkey);
+  return info;
 }
 
 /** Release storage held by an extend_info_t struct. */

src/feature/dirauth/dirvote.c

@@ -3754,8 +3754,10 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method)
   size_t keylen;
   smartlist_t *chunks = smartlist_new();
   char *output = NULL;
+  crypto_pk_t *rsa_pubkey = router_get_rsa_onion_pkey(ri->onion_pkey,
+                                                      ri->onion_pkey_len);
 
-  if (crypto_pk_write_public_key_to_string(ri->onion_pkey, &key, &keylen)<0)
+  if (crypto_pk_write_public_key_to_string(rsa_pubkey, &key, &keylen)<0)
     goto done;
   summary = policy_summarize(ri->exit_policy, AF_INET);
   if (ri->declared_family)
@@ -3826,6 +3828,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method)
   }
 
  done:
+  crypto_pk_free(rsa_pubkey);
   tor_free(output);
   tor_free(key);
   tor_free(summary);

src/feature/nodelist/microdesc.c

@@ -874,7 +874,7 @@ microdesc_free_(microdesc_t *md, const char *fname, int lineno)
   //tor_assert(md->held_by_nodes == 0);
 
   if (md->onion_pkey)
-    crypto_pk_free(md->onion_pkey);
+    tor_free(md->onion_pkey);
   tor_free(md->onion_curve25519_pkey);
   tor_free(md->ed25519_identity_pkey);
   if (md->body && md->saved_location != SAVED_IN_CACHE)

src/feature/nodelist/microdesc_st.h

@@ -53,8 +53,14 @@ struct microdesc_t {
 
   /* Fields in the microdescriptor. */
 
-  /** As routerinfo_t.onion_pkey */
-  crypto_pk_t *onion_pkey;
+  /**
+   * Public RSA TAP key for onions, ASN.1 encoded.  We store this
+   * in its encoded format since storing it as a crypto_pk_t uses
+   * significantly more memory. */
+  char *onion_pkey;
+  /** Length of onion_pkey, in bytes. */
+  size_t onion_pkey_len;
+
   /** As routerinfo_t.onion_curve25519_pkey */
   struct curve25519_public_key_t *onion_curve25519_pkey;
   /** Ed25519 identity key, if included. */

src/feature/nodelist/nodelist.c

@@ -1768,6 +1768,37 @@ node_get_curve25519_onion_key(const node_t *node)
     return NULL;
 }
 
+/* Return a newly allocacted RSA onion public key taken from the given node.
+ *
+ * Return NULL if node is NULL or no RSA onion public key can be found. It is
+ * the caller responsability to free the returned object. */
+crypto_pk_t *
+node_get_rsa_onion_key(const node_t *node)
+{
+  crypto_pk_t *pk = NULL;
+  const char *onion_pkey;
+  size_t onion_pkey_len;
+
+  if (!node) {
+    goto end;
+  }
+
+  if (node->ri) {
+    onion_pkey = node->ri->onion_pkey;
+    onion_pkey_len = node->ri->onion_pkey_len;
+  } else if (node->rs && node->md) {
+    onion_pkey = node->md->onion_pkey;
+    onion_pkey_len = node->md->onion_pkey_len;
+  } else {
+    /* No descriptor or microdescriptor. */
+    goto end;
+  }
+  pk = router_get_rsa_onion_pkey(onion_pkey, onion_pkey_len);
+
+ end:
+  return pk;
+}
+
 /** Refresh the country code of <b>ri</b>.  This function MUST be called on
  * each router when the GeoIP database is reloaded, and on all new routers. */
 void

src/feature/nodelist/nodelist.h

@@ -95,6 +95,7 @@ void node_get_pref_ipv6_dirport(const node_t *node, tor_addr_port_t *ap_out);
 int node_has_curve25519_onion_key(const node_t *node);
 const struct curve25519_public_key_t *node_get_curve25519_onion_key(
                                   const node_t *node);
+crypto_pk_t *node_get_rsa_onion_key(const node_t *node);
 
 MOCK_DECL(smartlist_t *, nodelist_get_list, (void));
 

src/feature/nodelist/routerinfo_st.h

@@ -27,7 +27,14 @@ struct routerinfo_t {
   tor_addr_t ipv6_addr;
   uint16_t ipv6_orport;
 
-  crypto_pk_t *onion_pkey; /**< Public RSA key for onions. */
+  /**
+   * Public RSA TAP key for onions, ASN.1 encoded.  We store this
+   * in its encoded format since storing it as a crypto_pk_t uses
+   * significantly more memory. */
+  char *onion_pkey;
+  /** Length of onion_pkey, in bytes. */
+  size_t onion_pkey_len;
+
   crypto_pk_t *identity_pkey;  /**< Public RSA key for signing. */
   /** Public curve25519 key for onions */
   struct curve25519_public_key_t *onion_curve25519_pkey;

src/feature/nodelist/routerlist.c

@@ -3217,7 +3217,7 @@ routerinfo_free_(routerinfo_t *router)
   tor_free(router->protocol_list);
   tor_free(router->contact_info);
   if (router->onion_pkey)
-    crypto_pk_free(router->onion_pkey);
+    tor_free(router->onion_pkey);
   tor_free(router->onion_curve25519_pkey);
   if (router->identity_pkey)
     crypto_pk_free(router->identity_pkey);
@@ -5498,7 +5498,8 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
       r1->ipv6_orport != r2->ipv6_orport ||
       r1->dir_port != r2->dir_port ||
       r1->purpose != r2->purpose ||
-      !crypto_pk_eq_keys(r1->onion_pkey, r2->onion_pkey) ||
+      r1->onion_pkey_len != r2->onion_pkey_len ||
+      !tor_memeq(r1->onion_pkey, r2->onion_pkey, r1->onion_pkey_len) ||
       !crypto_pk_eq_keys(r1->identity_pkey, r2->identity_pkey) ||
       strcasecmp(r1->platform, r2->platform) ||
       (r1->contact_info && !r2->contact_info) || /* contact_info is optional */

src/feature/nodelist/routerparse.c

@@ -1535,6 +1535,7 @@ router_parse_entry_from_string(const char *s, const char *end,
   /* Do not set this to '1' until we have parsed everything that we intend to
    * parse that's covered by the hash. */
   int can_dl_again = 0;
+  crypto_pk_t *rsa_pubkey = NULL;
 
   tor_assert(!allow_annotations || !prepend_annotations);
 
@@ -1717,8 +1718,9 @@ router_parse_entry_from_string(const char *s, const char *end,
              "Relay's onion key had invalid exponent.");
     goto err;
   }
-  router->onion_pkey = tok->key;
-  tok->key = NULL; /* Prevent free */
+  router_set_rsa_onion_pkey(tok->key, &router->onion_pkey,
+                            &router->onion_pkey_len);
+  crypto_pk_free(tok->key);
 
   if ((tok = find_opt_by_keyword(tokens, K_ONION_KEY_NTOR))) {
     curve25519_public_key_t k;
@@ -1890,10 +1892,12 @@ router_parse_entry_from_string(const char *s, const char *end,
         goto err;
       }
 
+      rsa_pubkey = router_get_rsa_onion_pkey(router->onion_pkey,
+                                             router->onion_pkey_len);
       if (check_tap_onion_key_crosscert(
                       (const uint8_t*)cc_tap_tok->object_body,
                       (int)cc_tap_tok->object_size,
-                      router->onion_pkey,
+                      rsa_pubkey,
                       &cert->signing_key,
                       (const uint8_t*)router->cache_info.identity_digest)<0) {
         log_warn(LD_DIR, "Incorrect TAP cross-verification");
@@ -2058,6 +2062,7 @@ router_parse_entry_from_string(const char *s, const char *end,
   routerinfo_free(router);
   router = NULL;
  done:
+  crypto_pk_free(rsa_pubkey);
   tor_cert_free(ntor_cc_cert);
   if (tokens) {
     SMARTLIST_FOREACH(tokens, directory_token_t *, t, token_clear(t));
@@ -4752,8 +4757,9 @@ microdescs_parse_from_string(const char *s, const char *eos,
                "Relay's onion key had invalid exponent.");
       goto next;
     }
-    md->onion_pkey = tok->key;
-    tok->key = NULL;
+    router_set_rsa_onion_pkey(tok->key, &md->onion_pkey,
+                              &md->onion_pkey_len);
+    crypto_pk_free(tok->key);
 
     if ((tok = find_opt_by_keyword(tokens, K_ONION_KEY_NTOR))) {
       curve25519_public_key_t k;

src/feature/relay/router.c

@@ -1464,6 +1464,8 @@ router_should_advertise_begindir(const or_options_t *options,
 static extend_info_t *
 extend_info_from_router(const routerinfo_t *r)
 {
+  crypto_pk_t *rsa_pubkey;
+  extend_info_t *info;
   tor_addr_port_t ap;
   tor_assert(r);
 
@@ -1477,10 +1479,13 @@ extend_info_from_router(const routerinfo_t *r)
     ed_id_key = NULL;
 
   router_get_prim_orport(r, &ap);
-  return extend_info_new(r->nickname, r->cache_info.identity_digest,
+  rsa_pubkey = router_get_rsa_onion_pkey(r->onion_pkey, r->onion_pkey_len);
+  info = extend_info_new(r->nickname, r->cache_info.identity_digest,
                          ed_id_key,
-                         r->onion_pkey, r->onion_curve25519_pkey,
+                         rsa_pubkey, r->onion_curve25519_pkey,
                          &ap.addr, ap.port);
+  crypto_pk_free(rsa_pubkey);
+  return info;
 }
 
 /**See if we currently believe our ORPort or DirPort to be
@@ -2313,8 +2318,10 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e)
   ri->supports_tunnelled_dir_requests =
     directory_permits_begindir_requests(options);
   ri->cache_info.published_on = time(NULL);
-  ri->onion_pkey = crypto_pk_dup_key(get_onion_key()); /* must invoke from
-                                                        * main thread */
+  /* get_onion_key() must invoke from main thread */
+  router_set_rsa_onion_pkey(get_onion_key(), &ri->onion_pkey,
+                            &ri->onion_pkey_len);
+
   ri->onion_curve25519_pkey =
     tor_memdup(&get_current_curve25519_keypair()->pubkey,
                sizeof(curve25519_public_key_t));
@@ -2849,6 +2856,7 @@ router_dump_router_to_string(routerinfo_t *router,
 {
   char *address = NULL;
   char *onion_pkey = NULL; /* Onion key, PEM-encoded. */
+  crypto_pk_t *rsa_pubkey = NULL;
   char *identity_pkey = NULL; /* Identity key, PEM-encoded. */
   char digest[DIGEST256_LEN];
   char published[ISO_TIME_LEN+1];
@@ -2915,7 +2923,9 @@ router_dump_router_to_string(routerinfo_t *router,
   }
 
   /* PEM-encode the onion key */
-  if (crypto_pk_write_public_key_to_string(router->onion_pkey,
+  rsa_pubkey = router_get_rsa_onion_pkey(router->onion_pkey,
+                                         router->onion_pkey_len);
+  if (crypto_pk_write_public_key_to_string(rsa_pubkey,
                                            &onion_pkey,&onion_pkeylen)<0) {
     log_warn(LD_BUG,"write onion_pkey to string failed!");
     goto err;
@@ -3200,6 +3210,7 @@ router_dump_router_to_string(routerinfo_t *router,
     SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
     smartlist_free(chunks);
   }
+  crypto_pk_free(rsa_pubkey);
   tor_free(address);
   tor_free(family_line);
   tor_free(onion_pkey);
@@ -3827,3 +3838,42 @@ router_get_all_orports(const routerinfo_t *ri)
   fake_node.ri = (routerinfo_t *)ri;
   return node_get_all_orports(&fake_node);
 }
+
+/* From the given RSA key object, convert it to ASN-1 encoded format and set
+ * the newly allocated object in onion_pkey_out. The length of the key is set
+ * in onion_pkey_len_out. */
+void
+router_set_rsa_onion_pkey(const crypto_pk_t *pk, char **onion_pkey_out,
+                          size_t *onion_pkey_len_out)
+{
+  int len;
+  char buf[1024];
+
+  tor_assert(pk);
+  tor_assert(onion_pkey_out);
+  tor_assert(onion_pkey_len_out);
+
+  len = crypto_pk_asn1_encode(pk, buf, sizeof(buf));
+  if (BUG(len < 0)) {
+    goto done;
+  }
+
+  *onion_pkey_out = tor_memdup(buf, len);
+  *onion_pkey_len_out = len;
+
+ done:
+  return;
+}
+
+/* From an ASN-1 encoded onion pkey, return a newly allocated RSA key object.
+ * It is the caller responsability to free the returned object.
+ *
+ * Return NULL if the pkey is NULL, malformed or if the length is 0. */
+crypto_pk_t *
+router_get_rsa_onion_pkey(const char *pkey, size_t pkey_len)
+{
+  if (!pkey || pkey_len == 0) {
+    return NULL;
+  }
+  return crypto_pk_asn1_decode(pkey, pkey_len);
+}

src/feature/relay/router.h

@@ -45,6 +45,10 @@ void v3_authority_check_key_expiry(void);
 int get_onion_key_lifetime(void);
 int get_onion_key_grace_period(void);
 
+crypto_pk_t *router_get_rsa_onion_pkey(const char *pkey, size_t pkey_len);
+void router_set_rsa_onion_pkey(const crypto_pk_t *pk, char **onion_pkey_out,
+                               size_t *onion_pkey_len);
+
 di_digest256_map_t *construct_ntor_key_map(void);
 void ntor_key_map_free_(di_digest256_map_t *map);
 #define ntor_key_map_free(map) \

src/test/test_dir.c

@@ -166,7 +166,7 @@ test_dir_formats(void *arg)
   r1->supports_tunnelled_dir_requests = 1;
   tor_addr_parse(&r1->ipv6_addr, "1:2:3:4::");
   r1->ipv6_orport = 9999;
-  r1->onion_pkey = crypto_pk_dup_key(pk1);
+  router_set_rsa_onion_pkey(pk1, &r1->onion_pkey, &r1->onion_pkey_len);
   /* Fake just enough of an ntor key to get by */
   curve25519_keypair_t r1_onion_keypair;
   curve25519_keypair_generate(&r1_onion_keypair, 0);
@@ -209,7 +209,7 @@ test_dir_formats(void *arg)
   r2->or_port = 9005;
   r2->dir_port = 0;
   r2->supports_tunnelled_dir_requests = 1;
-  r2->onion_pkey = crypto_pk_dup_key(pk2);
+  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,
@@ -302,7 +302,10 @@ test_dir_formats(void *arg)
   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);
-  tt_int_op(crypto_pk_cmp_keys(rp1->onion_pkey, pk1), OP_EQ, 0);
+  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);
@@ -419,7 +422,10 @@ test_dir_formats(void *arg)
   tt_mem_op(rp2->onion_curve25519_pkey->public_key,OP_EQ,
              r2->onion_curve25519_pkey->public_key,
              CURVE25519_PUBKEY_LEN);
-  tt_int_op(crypto_pk_cmp_keys(rp2->onion_pkey, pk2), OP_EQ, 0);
+  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);
 

src/test/test_hs_service.c

@@ -1238,7 +1238,7 @@ test_build_update_descriptors(void *arg)
     tt_int_op(ret, OP_EQ, 0);
     ri.onion_curve25519_pkey =
       tor_malloc_zero(sizeof(curve25519_public_key_t));
-    ri.onion_pkey = crypto_pk_new();
+    ri.onion_pkey = tor_malloc_zero(140);
     curve25519_public_key_generate(ri.onion_curve25519_pkey,
                                    &curve25519_secret_key);
     memset(ri.cache_info.identity_digest, 'A', DIGEST_LEN);
@@ -1264,7 +1264,7 @@ test_build_update_descriptors(void *arg)
   update_all_descriptors(now);
   tor_free(node->ri->onion_curve25519_pkey); /* Avoid memleak. */
   tor_free(node->ri->cache_info.signing_key_cert);
-  crypto_pk_free(node->ri->onion_pkey);
+  tor_free(node->ri->onion_pkey);
   expect_log_msg_containing("just picked 1 intro points and wanted 3 for next "
                             "descriptor. It currently has 0 intro points. "
                             "Launching ESTABLISH_INTRO circuit shortly.");

src/test/test_router.c

@@ -49,7 +49,8 @@ NS(router_get_my_routerinfo)(void)
     mock_routerinfo->platform = tor_strdup("unittest");
     mock_routerinfo->cache_info.published_on = now;
     mock_routerinfo->identity_pkey = crypto_pk_dup_key(ident_key);
-    mock_routerinfo->onion_pkey = crypto_pk_dup_key(tap_key);
+    router_set_rsa_onion_pkey(tap_key, &mock_routerinfo->onion_pkey,
+                              &mock_routerinfo->onion_pkey_len);
     mock_routerinfo->bandwidthrate = 9001;
     mock_routerinfo->bandwidthburst = 9002;
   }
@@ -89,11 +90,14 @@ test_router_dump_router_to_string_no_bridge_distribution_method(void *arg)
 
   /* Generate our server descriptor and ensure that the substring
    * "bridge-distribution-request any" occurs somewhere within it. */
+  crypto_pk_t *onion_pkey = router_get_rsa_onion_pkey(router->onion_pkey,
+                                                      router->onion_pkey_len);
   desc = router_dump_router_to_string(router,
                                       router->identity_pkey,
-                                      router->onion_pkey,
+                                      onion_pkey,
                                       &ntor_keypair,
                                       &signing_keypair);
+  crypto_pk_free(onion_pkey);
   tt_ptr_op(desc, !=, NULL);
   found = strstr(desc, needle);
   tt_ptr_op(found, !=, NULL);