Use chunks, not buffers, for router descriptors

3 files changed, 67 insertions, 92 deletions

diff --git a/src/or/router.c b/src/or/router.c
index 95aa70a9c..1b5909eb1 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1927,9 +1927,8 @@ router_rebuild_descriptor(int force)
     /* ri was allocated with tor_malloc_zero, so there is no need to
      * zero ri->cache_info.extra_info_digest here. */
   }
-  ri->cache_info.signed_descriptor_body = tor_malloc(8192);
-  if (router_dump_router_to_string(ri->cache_info.signed_descriptor_body, 8192,
-                                   ri, get_server_identity_key()) < 0) {
+  if (! (ri->cache_info.signed_descriptor_body = router_dump_router_to_string(
+                                           ri, get_server_identity_key()))) {
     log_warn(LD_BUG, "Couldn't generate router descriptor.");
     routerinfo_free(ri);
     extrainfo_free(ei);
@@ -2229,54 +2228,53 @@ get_platform_str(char *platform, size_t len)
 #define DEBUG_ROUTER_DUMP_ROUTER_TO_STRING
 
 /** OR only: Given a routerinfo for this router, and an identity key to sign
- * with, encode the routerinfo as a signed server descriptor and write the
- * result into <b>s</b>, using at most <b>maxlen</b> bytes.  Return -1 on
- * failure, and the number of bytes used on success.
+ * with, encode the routerinfo as a signed server descriptor and return a new
+ * string encoding the result, or NULL on failure.
  */
-int
-router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
+char *
+router_dump_router_to_string(routerinfo_t *router,
                              crypto_pk_t *ident_key)
 {
-  char *onion_pkey; /* Onion key, PEM-encoded. */
-  char *identity_pkey; /* Identity key, PEM-encoded. */
+  char *onion_pkey = NULL; /* Onion key, PEM-encoded. */
+  char *identity_pkey = NULL; /* Identity key, PEM-encoded. */
   char digest[DIGEST_LEN];
   char published[ISO_TIME_LEN+1];
   char fingerprint[FINGERPRINT_LEN+1];
   int has_extra_info_digest;
   char extra_info_digest[HEX_DIGEST_LEN+1];
   size_t onion_pkeylen, identity_pkeylen;
-  size_t written;
-  int result=0;
-  char *family_line;
+  char *family_line = NULL;
   char *extra_or_address = NULL;
   const or_options_t *options = get_options();
+  smartlist_t *chunks = NULL;
+  char *output = NULL;
+  size_t output_len;
 
   /* Make sure the identity key matches the one in the routerinfo. */
   if (!crypto_pk_eq_keys(ident_key, router->identity_pkey)) {
     log_warn(LD_BUG,"Tried to sign a router with a private key that didn't "
              "match router's public key!");
-    return -1;
+    goto err;
   }
 
   /* record our fingerprint, so we can include it in the descriptor */
   if (crypto_pk_get_fingerprint(router->identity_pkey, fingerprint, 1)<0) {
     log_err(LD_BUG,"Error computing fingerprint");
-    return -1;
+    goto err;
   }
 
   /* PEM-encode the onion key */
   if (crypto_pk_write_public_key_to_string(router->onion_pkey,
                                            &onion_pkey,&onion_pkeylen)<0) {
     log_warn(LD_BUG,"write onion_pkey to string failed!");
-    return -1;
+    goto err;
   }
 
   /* PEM-encode the identity key */
   if (crypto_pk_write_public_key_to_string(router->identity_pkey,
                                         &identity_pkey,&identity_pkeylen)<0) {
     log_warn(LD_BUG,"write identity_pkey to string failed!");
-    tor_free(onion_pkey);
-    return -1;
+    goto err;
   }
 
   /* Encode the publication time. */
@@ -2311,8 +2309,9 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
     }
   }
 
+  chunks = smartlist_new();
   /* Generate the easy portion of the router descriptor. */
-  result = tor_snprintf(s, maxlen,
+  smartlist_add_asprintf(chunks,
                     "router %s %s %d 0 %d\n"
                     "%s"
                     "platform %s\n"
@@ -2347,28 +2346,11 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
     options->HidServDirectoryV2 ? "hidden-service-dir\n" : "",
     options->AllowSingleHopExits ? "allow-single-hop-exits\n" : "");
 
-  tor_free(family_line);
-  tor_free(onion_pkey);
-  tor_free(identity_pkey);
-  tor_free(extra_or_address);
-
-  if (result < 0) {
-    log_warn(LD_BUG,"descriptor snprintf #1 ran out of room!");
-    return -1;
-  }
-  /* From now on, we use 'written' to remember the current length of 's'. */
-  written = result;
-
   if (options->ContactInfo && strlen(options->ContactInfo)) {
     const char *ci = options->ContactInfo;
     if (strchr(ci, '\n') || strchr(ci, '\r'))
       ci = escaped(ci);
-    result = tor_snprintf(s+written,maxlen-written, "contact %s\n", ci);
-    if (result<0) {
-      log_warn(LD_BUG,"descriptor snprintf #2 ran out of room!");
-      return -1;
-    }
-    written += result;
+    smartlist_add_asprintf(chunks, "contact %s\n", ci);
   }
 
 #ifdef CURVE25519_ENABLED
@@ -2377,105 +2359,94 @@ router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
     base64_encode(kbuf, sizeof(kbuf),
                   (const char *)router->onion_curve25519_pkey->public_key,
                   CURVE25519_PUBKEY_LEN);
-    result = tor_snprintf(s+written,maxlen-written, "ntor-onion-key %s",
-                          kbuf);
-    if (result<0) {
-      log_warn(LD_BUG,"descriptor snprintf ran out of room!");
-      return -1;
-    }
-    written += result;
+    smartlist_add_asprintf(chunks, "ntor-onion-key %s", kbuf);
   }
 #endif
 
   /* Write the exit policy to the end of 's'. */
   if (!router->exit_policy || !smartlist_len(router->exit_policy)) {
-    strlcat(s+written, "reject *:*\n", maxlen-written);
-    written += strlen("reject *:*\n");
+    smartlist_add(chunks, tor_strdup("reject *:*\n"));
   } else if (router->exit_policy) {
     int i;
     for (i = 0; i < smartlist_len(router->exit_policy); ++i) {
+      char pbuf[POLICY_BUF_LEN];
       addr_policy_t *tmpe = smartlist_get(router->exit_policy, i);
+      int result;
       if (tor_addr_family(&tmpe->addr) == AF_INET6)
         continue; /* Don't include IPv6 parts of address policy */
-      result = policy_write_item(s+written, maxlen-written, tmpe, 1);
+      result = policy_write_item(pbuf, POLICY_BUF_LEN, tmpe, 1);
       if (result < 0) {
         log_warn(LD_BUG,"descriptor policy_write_item ran out of room!");
-        return -1;
+        goto err;
       }
-      tor_assert(result == (int)strlen(s+written));
-      written += result;
-      if (written+2 > maxlen) {
-        log_warn(LD_BUG,"descriptor policy_write_item ran out of room (2)!");
-        return -1;
-      }
-      s[written++] = '\n';
+      smartlist_add_asprintf(chunks, "%s\n", pbuf);
     }
   }
 
   if (router->ipv6_exit_policy) {
     char *p6 = write_short_policy(router->ipv6_exit_policy);
     if (p6 && strcmp(p6, "reject 1-65535")) {
-      result = tor_snprintf(s+written, maxlen-written,
+      smartlist_add_asprintf(chunks,
                             "ipv6-policy %s\n", p6);
-      if (result<0) {
-        log_warn(LD_BUG,"Descriptor printf of policy ran out of room");
-        tor_free(p6);
-        return -1;
-      }
-      written += result;
     }
     tor_free(p6);
   }
 
-  if (written + DIROBJ_MAX_SIG_LEN > maxlen) {
-    /* Not enough room for signature. */
-    log_warn(LD_BUG,"not enough room left in descriptor for signature!");
-    return -1;
-  }
-
   /* Sign the descriptor */
-  strlcpy(s+written, "router-signature\n", maxlen-written);
-  written += strlen(s+written);
-  s[written] = '\0';
-  if (router_get_router_hash(s, strlen(s), digest) < 0) {
-    return -1;
+  smartlist_add(chunks, tor_strdup("router-signature\n"));
+
+  output = smartlist_join_strings(chunks, "", 0, NULL);
+#define MAX_DESC_SIGNATURE_LEN 4096
+  output_len = strlen(output) + MAX_DESC_SIGNATURE_LEN + 1;
+  output = tor_realloc(output, output_len);
+
+  if (router_get_router_hash(output, strlen(output), digest) < 0) {
+    goto err;
   }
 
   note_crypto_pk_op(SIGN_RTR);
-  if (router_append_dirobj_signature(s+written,maxlen-written,
+  if (router_append_dirobj_signature(output, output_len,
                                      digest,DIGEST_LEN,ident_key)<0) {
     log_warn(LD_BUG, "Couldn't sign router descriptor");
-    return -1;
+    goto err;
   }
-  written += strlen(s+written);
 
-  if (written+2 > maxlen) {
-    log_warn(LD_BUG,"Not enough room to finish descriptor.");
-    return -1;
-  }
   /* include a last '\n' */
-  s[written] = '\n';
-  s[written+1] = 0;
+  strlcat(output, "\n", output_len);
 
 #ifdef DEBUG_ROUTER_DUMP_ROUTER_TO_STRING
   {
     char *s_dup;
     const char *cp;
     routerinfo_t *ri_tmp;
-    cp = s_dup = tor_strdup(s);
+    cp = s_dup = tor_strdup(output);
     ri_tmp = router_parse_entry_from_string(cp, NULL, 1, 0, NULL);
     if (!ri_tmp) {
       log_err(LD_BUG,
               "We just generated a router descriptor we can't parse.");
-      log_err(LD_BUG, "Descriptor was: <<%s>>", s);
-      return -1;
+      log_err(LD_BUG, "Descriptor was: <<%s>>", output);
+      goto err;
     }
     tor_free(s_dup);
     routerinfo_free(ri_tmp);
   }
 #endif
 
-  return (int)written+1;
+  goto done;
+
+ err:
+  tor_free(output); /* sets output to NULL */
+ done:
+  if (chunks) {
+    SMARTLIST_FOREACH(chunks, char *, cp, tor_free(cp));
+    smartlist_free(chunks);
+  }
+  tor_free(family_line);
+  tor_free(onion_pkey);
+  tor_free(identity_pkey);
+  tor_free(extra_or_address);
+
+  return output;
 }
 
 /** Copy the primary (IPv4) OR port (IP address and TCP port) for
diff --git a/src/or/router.h b/src/or/router.h
index fd2076af0..7a093019f 100644
--- a/src/or/router.h
+++ b/src/or/router.h
@@ -90,8 +90,8 @@ int router_is_me(const routerinfo_t *router);
 int router_fingerprint_is_me(const char *fp);
 int router_pick_published_address(const or_options_t *options, uint32_t *addr);
 int router_rebuild_descriptor(int force);
-int router_dump_router_to_string(char *s, size_t maxlen, routerinfo_t *router,
-                                 crypto_pk_t *ident_key);
+char *router_dump_router_to_string(routerinfo_t *router,
+                                   crypto_pk_t *ident_key);
 void router_get_prim_orport(const routerinfo_t *router,
                             tor_addr_port_t *addr_port_out);
 void router_get_pref_orport(const routerinfo_t *router,
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index fbd49b710..ea0011a6d 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -73,7 +73,8 @@ test_dir_nicknames(void)
 static void
 test_dir_formats(void)
 {
-  char buf[8192], buf2[8192];
+  char *buf = NULL;
+  char buf2[8192];
   char platform[256];
   char fingerprint[FINGERPRINT_LEN+1];
   char *pk1_str = NULL, *pk2_str = NULL, *pk3_str = NULL, *cp;
@@ -142,8 +143,8 @@ test_dir_formats(void)
   test_assert(!crypto_pk_write_public_key_to_string(pk3 , &pk3_str,
                                                     &pk3_str_len));
 
-  memset(buf, 0, 2048);
-  test_assert(router_dump_router_to_string(buf, 2048, r1, pk2)>0);
+  buf = router_dump_router_to_string(r1, pk2);
+  test_assert(buf);
 
   strlcpy(buf2, "router Magri 18.244.0.1 9000 0 9003\n"
           "or-address [1:2:3:4::]:9999\n"
@@ -170,8 +171,10 @@ test_dir_formats(void)
                              * twice */
 
   test_streq(buf, buf2);
+  tor_free(buf);
 
-  test_assert(router_dump_router_to_string(buf, 2048, r1, pk2)>0);
+  buf = router_dump_router_to_string(r1, pk2);
+  test_assert(buf);
   cp = buf;
   rp1 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL);
   test_assert(rp1);
@@ -232,6 +235,7 @@ test_dir_formats(void)
   if (r2)
     routerinfo_free(r2);
 
+  tor_free(buf);
   tor_free(pk1_str);
   tor_free(pk2_str);
   tor_free(pk3_str);