Refactor dirobj signature generation

Now we can compute the hash and signature of a dirobj before
concatenating the smartlist, and we don't need to play silly games
with sigbuf and realloc any more.

7 files changed, 122 insertions, 85 deletions

diff --git a/src/common/crypto.c b/src/common/crypto.c
index 22d57c7c8..aeef1e3c6 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -1631,6 +1631,29 @@ crypto_digest_assign(crypto_digest_t *into,
   memcpy(into,from,sizeof(crypto_digest_t));
 }
 
+
+/** Given a list of strings in <b>lst</b>, set the <b>len_out</b>-byte digest
+ * at <b>digest_out</b> to the hash of the concatenation of those strings,
+ * plus the optional string <b>append</b>, computed with the algorithm
+ * <b>alg</b>. */
+void
+crypto_digest_smartlist(char *digest_out, size_t len_out,
+                        const smartlist_t *lst, const char *append,
+                        digest_algorithm_t alg)
+{
+  crypto_digest_t *d;
+  if (alg == DIGEST_SHA1)
+    d = crypto_digest_new();
+  else
+    d = crypto_digest256_new(alg);
+  SMARTLIST_FOREACH(lst, const char *, cp,
+                    crypto_digest_add_bytes(d, cp, strlen(cp)));
+  if (append)
+    crypto_digest_add_bytes(d, append, strlen(append));
+  crypto_digest_get_digest(d, digest_out, len_out);
+  crypto_digest_free(d);
+}
+
 /** Compute the HMAC-SHA-1 of the <b>msg_len</b> bytes in <b>msg</b>, using
  * the <b>key</b> of length <b>key_len</b>.  Store the DIGEST_LEN-byte result
  * in <b>hmac_out</b>.
diff --git a/src/common/crypto.h b/src/common/crypto.h
index 12fcfae27..e8f6eace1 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -206,6 +206,10 @@ int crypto_digest(char *digest, const char *m, size_t len);
 int crypto_digest256(char *digest, const char *m, size_t len,
                      digest_algorithm_t algorithm);
 int crypto_digest_all(digests_t *ds_out, const char *m, size_t len);
+struct smartlist_t;
+void crypto_digest_smartlist(char *digest_out, size_t len_out,
+                             const struct smartlist_t *lst, const char *append,
+                             digest_algorithm_t alg);
 const char *crypto_digest_algorithm_get_name(digest_algorithm_t alg);
 int crypto_digest_algorithm_parse_name(const char *name);
 crypto_digest_t *crypto_digest_new(void);
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index d5b90b936..2bbfc9a7c 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -2911,7 +2911,6 @@ generate_v2_networkstatus_opinion(void)
   size_t identity_pkey_len;
   char *status = NULL, *client_versions = NULL, *server_versions = NULL,
     *identity_pkey = NULL, *hostname = NULL;
-  size_t status_len;
   const or_options_t *options = get_options();
   char fingerprint[FINGERPRINT_LEN+1];
   char published[ISO_TIME_LEN+1];
@@ -3032,23 +3031,21 @@ generate_v2_networkstatus_opinion(void)
   smartlist_add_asprintf(chunks, "directory-signature %s\n",
                          options->Nickname);
 
-  status = smartlist_join_strings(chunks, "", 0, NULL);
-#define MAX_V2_OPINION_SIGNATURE_LEN 4096
-  status_len = strlen(status) + MAX_V2_OPINION_SIGNATURE_LEN + 1;
-  status = tor_realloc(status, status_len);
-
-  if (router_get_networkstatus_v2_hash(status, digest)<0) {
-    log_warn(LD_BUG, "Unable to hash network status");
-    goto done;
-  }
+  crypto_digest_smartlist(digest, DIGEST_LEN, chunks, "", DIGEST_SHA1);
 
   note_crypto_pk_op(SIGN_DIR);
-  if (router_append_dirobj_signature(status, status_len,digest,DIGEST_LEN,
-                                     private_key)<0) {
-    log_warn(LD_BUG, "Unable to sign router status.");
-    goto done;
+  {
+    char *sig;
+    if (!(sig = router_get_dirobj_signature(digest,DIGEST_LEN,
+                                            private_key))) {
+      log_warn(LD_BUG, "Unable to sign router status.");
+      goto done;
+    }
+    smartlist_add(chunks, sig);
   }
 
+  status = smartlist_join_strings(chunks, "", 0, NULL);
+
   {
     networkstatus_v2_t *ns;
     if (!(ns = networkstatus_v2_parse_from_string(status))) {
diff --git a/src/or/dirvote.c b/src/or/dirvote.c
index a7876701a..6ae494436 100644
--- a/src/or/dirvote.c
+++ b/src/or/dirvote.c
@@ -75,7 +75,6 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
   char *client_versions_line = NULL, *server_versions_line = NULL;
   networkstatus_voter_info_t *voter;
   char *status = NULL;
-  size_t status_len;
 
   tor_assert(private_signing_key);
   tor_assert(v3_ns->type == NS_TYPE_VOTE || v3_ns->type == NS_TYPE_OPINION);
@@ -185,6 +184,11 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
 
   smartlist_add(chunks, tor_strdup("directory-footer\n"));
 
+  /* The digest includes everything up through the space after
+   * directory-signature.  (Yuck.) */
+  crypto_digest_smartlist(digest, DIGEST_LEN, chunks,
+                          "directory-signature ", DIGEST_SHA1);
+
   {
     char signing_key_fingerprint[FINGERPRINT_LEN+1];
     if (crypto_pk_get_fingerprint(private_signing_key,
@@ -197,22 +201,19 @@ format_networkstatus_vote(crypto_pk_t *private_signing_key,
                            signing_key_fingerprint);
   }
 
-  status = smartlist_join_strings(chunks, "", 0, NULL);
-#define MAX_VOTE_SIGNATURE_LEN 4096
-  status_len = strlen(status) + MAX_VOTE_SIGNATURE_LEN + 1;
-  status = tor_realloc(status, status_len);
-
-  if (router_get_networkstatus_v3_hash(status, digest, DIGEST_SHA1)<0)
-    goto err;
   note_crypto_pk_op(SIGN_DIR);
-  if (router_append_dirobj_signature(status+strlen(status),
-                                     status_len,
-                                     digest, DIGEST_LEN,
-                                     private_signing_key)<0) {
-    log_warn(LD_BUG, "Unable to sign networkstatus vote.");
-    goto err;
+  {
+    char *sig = router_get_dirobj_signature(digest, DIGEST_LEN,
+                                            private_signing_key);
+    if (!sig) {
+      log_warn(LD_BUG, "Unable to sign networkstatus vote.");
+      goto err;
+    }
+    smartlist_add(chunks, sig);
   }
 
+  status = smartlist_join_strings(chunks, "", 0, NULL);
+
   {
     networkstatus_t *v;
     if (!(v = networkstatus_parse_vote_from_string(status, NULL,
@@ -479,24 +480,6 @@ compute_routerstatus_consensus(smartlist_t *votes, int consensus_method,
   return most;
 }
 
-/** Given a list of strings in <b>lst</b>, set the <b>len_out</b>-byte digest
- * at <b>digest_out</b> to the hash of the concatenation of those strings,
- * computed with the algorithm <b>alg</b>. */
-static void
-hash_list_members(char *digest_out, size_t len_out,
-                  smartlist_t *lst, digest_algorithm_t alg)
-{
-  crypto_digest_t *d;
-  if (alg == DIGEST_SHA1)
-    d = crypto_digest_new();
-  else
-    d = crypto_digest256_new(alg);
-  SMARTLIST_FOREACH(lst, const char *, cp,
-                    crypto_digest_add_bytes(d, cp, strlen(cp)));
-  crypto_digest_get_digest(d, digest_out, len_out);
-  crypto_digest_free(d);
-}
-
 /** Sorting helper: compare two strings based on their values as base-ten
  * positive integers. (Non-integers are treated as prior to all integers, and
  * compared lexically.) */
@@ -2095,12 +2078,12 @@ networkstatus_compute_consensus(smartlist_t *votes,
     size_t digest_len =
       flavor == FLAV_NS ? DIGEST_LEN : DIGEST256_LEN;
     const char *algname = crypto_digest_algorithm_get_name(digest_alg);
-    char sigbuf[4096];
+    char *signature;
 
     smartlist_add(chunks, tor_strdup("directory-signature "));
 
     /* Compute the hash of the chunks. */
-    hash_list_members(digest, digest_len, chunks, digest_alg);
+    crypto_digest_smartlist(digest, digest_len, chunks, "", digest_alg);
 
     /* Get the fingerprints */
     crypto_pk_get_fingerprint(identity_key, fingerprint, 0);
@@ -2116,14 +2099,12 @@ networkstatus_compute_consensus(smartlist_t *votes,
                    signing_key_fingerprint);
     }
     /* And the signature. */
-    sigbuf[0] = '\0';
-    if (router_append_dirobj_signature(sigbuf, sizeof(sigbuf),
-                                       digest, digest_len,
-                                       signing_key)) {
+    if (!(signature = router_get_dirobj_signature(digest, digest_len,
+                                                  signing_key))) {
       log_warn(LD_BUG, "Couldn't sign consensus networkstatus.");
       goto done;
     }
-    smartlist_add(chunks, tor_strdup(sigbuf));
+    smartlist_add(chunks, signature);
 
     if (legacy_id_key_digest && legacy_signing_key && consensus_method >= 3) {
       smartlist_add(chunks, tor_strdup("directory-signature "));
@@ -2139,14 +2120,13 @@ networkstatus_compute_consensus(smartlist_t *votes,
                      algname, fingerprint,
                      signing_key_fingerprint);
       }
-      sigbuf[0] = '\0';
-      if (router_append_dirobj_signature(sigbuf, sizeof(sigbuf),
-                                         digest, digest_len,
-                                         legacy_signing_key)) {
+
+      if (!(signature = router_get_dirobj_signature(digest, digest_len,
+                                                    legacy_signing_key))) {
         log_warn(LD_BUG, "Couldn't sign consensus networkstatus.");
         goto done;
       }
-      smartlist_add(chunks, tor_strdup(sigbuf));
+      smartlist_add(chunks, signature);
     }
   }
 
diff --git a/src/or/router.c b/src/or/router.c
index 1b5909eb1..a391cddec 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -2248,7 +2248,6 @@ router_dump_router_to_string(routerinfo_t *router,
   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)) {
@@ -2395,24 +2394,22 @@ router_dump_router_to_string(routerinfo_t *router,
   /* Sign the descriptor */
   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;
-  }
+  crypto_digest_smartlist(digest, DIGEST_LEN, chunks, "", DIGEST_SHA1);
 
   note_crypto_pk_op(SIGN_RTR);
-  if (router_append_dirobj_signature(output, output_len,
-                                     digest,DIGEST_LEN,ident_key)<0) {
-    log_warn(LD_BUG, "Couldn't sign router descriptor");
-    goto err;
+  {
+    char *sig;
+    if (!(sig = router_get_dirobj_signature(digest, DIGEST_LEN, ident_key))) {
+      log_warn(LD_BUG, "Couldn't sign router descriptor");
+      goto err;
+    }
+    smartlist_add(chunks, sig);
   }
 
   /* include a last '\n' */
-  strlcat(output, "\n", output_len);
+  smartlist_add(chunks, tor_strdup("\n"));
+
+  output = smartlist_join_strings(chunks, "", 0, NULL);
 
 #ifdef DEBUG_ROUTER_DUMP_ROUTER_TO_STRING
   {
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index ce2cd5c51..63f8fab10 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -683,20 +683,19 @@ router_get_extrainfo_hash(const char *s, size_t s_len, char *digest)
                               "\nrouter-signature",'\n', DIGEST_SHA1);
 }
 
-/** Helper: used to generate signatures for routers, directories and
- * network-status objects.  Given a digest in <b>digest</b> and a secret
- * <b>private_key</b>, generate an PKCS1-padded signature, BASE64-encode it,
- * surround it with -----BEGIN/END----- pairs, and write it to the
- * <b>buf_len</b>-byte buffer at <b>buf</b>.  Return 0 on success, -1 on
- * failure.
- */
-int
-router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest,
-                               size_t digest_len, crypto_pk_t *private_key)
+/** DOCDOC */
+char *
+router_get_dirobj_signature(const char *digest,
+                            size_t digest_len,
+                            crypto_pk_t *private_key)
 {
   char *signature;
   size_t i, keysize;
   int siglen;
+  char *buf = NULL;
+  size_t buf_len;
+  /* overestimate of BEGIN/END lines total len. */
+#define BEGIN_END_OVERHEAD_LEN 64
 
   keysize = crypto_pk_keysize(private_key);
   signature = tor_malloc(keysize);
@@ -706,7 +705,12 @@ router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest,
     log_warn(LD_BUG,"Couldn't sign digest.");
     goto err;
   }
-  if (strlcat(buf, "-----BEGIN SIGNATURE-----\n", buf_len) >= buf_len)
+
+  /* The *2 here is a ridiculous overestimate of base-64 overhead. */
+  buf_len = (siglen * 2) + BEGIN_END_OVERHEAD_LEN;
+  buf = tor_malloc(buf_len);
+
+  if (strlcpy(buf, "-----BEGIN SIGNATURE-----\n", buf_len) >= buf_len)
     goto truncated;
 
   i = strlen(buf);
@@ -719,13 +723,42 @@ router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest,
     goto truncated;
 
   tor_free(signature);
-  return 0;
+  return buf;
 
  truncated:
   log_warn(LD_BUG,"tried to exceed string length.");
  err:
   tor_free(signature);
-  return -1;
+  tor_free(buf);
+  return NULL;
+}
+
+/** Helper: used to generate signatures for routers, directories and
+ * network-status objects.  Given a digest in <b>digest</b> and a secret
+ * <b>private_key</b>, generate an PKCS1-padded signature, BASE64-encode it,
+ * surround it with -----BEGIN/END----- pairs, and write it to the
+ * <b>buf_len</b>-byte buffer at <b>buf</b>.  Return 0 on success, -1 on
+ * failure.
+ */
+int
+router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest,
+                               size_t digest_len, crypto_pk_t *private_key)
+{
+  size_t sig_len, s_len;
+  char *sig = router_get_dirobj_signature(digest, digest_len, private_key);
+  if (!sig) {
+    log_warn(LD_BUG, "No signature generated");
+    return -1;
+  }
+  sig_len = strlen(sig);
+  s_len = strlen(buf);
+  if (sig_len + s_len + 1 > buf_len) {
+    log_warn(LD_BUG, "Not enough room for signature");
+    tor_free(sig);
+    return -1;
+  }
+  memcpy(buf+s_len, sig, sig_len+1);
+  return 0;
 }
 
 /** Return VS_RECOMMENDED if <b>myversion</b> is contained in
diff --git a/src/or/routerparse.h b/src/or/routerparse.h
index 859a691e2..8eac3a2aa 100644
--- a/src/or/routerparse.h
+++ b/src/or/routerparse.h
@@ -21,6 +21,9 @@ int router_get_networkstatus_v3_hash(const char *s, char *digest,
 int router_get_networkstatus_v3_hashes(const char *s, digests_t *digests);
 int router_get_extrainfo_hash(const char *s, size_t s_len, char *digest);
 #define DIROBJ_MAX_SIG_LEN 256
+char *router_get_dirobj_signature(const char *digest,
+                                  size_t digest_len,
+                                  crypto_pk_t *private_key);
 int router_append_dirobj_signature(char *buf, size_t buf_len,
                                    const char *digest,
                                    size_t digest_len,