Merge branch 'maint-0.2.2' into release-0.2.2

12 files changed, 71 insertions, 16 deletions

diff --git a/changes/dhparam b/changes/dhparam
new file mode 100644
index 0000000000..cb31243ba9
--- /dev/null
+++ b/changes/dhparam
@@ -0,0 +1,3 @@
+  o Minor features
+    - Adjust our TLS Diffie-Hellman parameters to match those used by
+      Apache's mod_ssl.
diff --git a/changes/policy_summarize-assert b/changes/policy_summarize-assert
new file mode 100644
index 0000000000..619e8e7e42
--- /dev/null
+++ b/changes/policy_summarize-assert
@@ -0,0 +1,6 @@
+  o Major bugfixes (security)
+    - Fix a bounds-checking error that could allow an attacker to
+      remotely crash a directory authority.  Found by piebeer.
+      Bugfix on 0.2.1.5-alpha.
+
+
diff --git a/changes/routerparse_maxima b/changes/routerparse_maxima
new file mode 100644
index 0000000000..340f2c3c2d
--- /dev/null
+++ b/changes/routerparse_maxima
@@ -0,0 +1,4 @@
+  o Minor bugfixes
+    - Check for and reject overly long directory certificates and
+      directory tokens before they have a chance to hit any
+      assertions. Bugfix on 0.2.1.28. Found by doorss.
diff --git a/src/common/crypto.c b/src/common/crypto.c
index e847d8c033..5264fd8085 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -1685,8 +1685,10 @@ crypto_hmac_sha1(char *hmac_out,
 
 /* DH */
 
-/** Shared P parameter for our DH key exchanged. */
+/** Shared P parameter for our circuit-crypto DH key exchanges. */
 static BIGNUM *dh_param_p = NULL;
+/** Shared P parameter for our TLS DH key exchanges. */
+static BIGNUM *dh_param_p_tls = NULL;
 /** Shared G parameter for our DH key exchanges. */
 static BIGNUM *dh_param_g = NULL;
 
@@ -1695,14 +1697,16 @@ static BIGNUM *dh_param_g = NULL;
 static void
 init_dh_param(void)
 {
-  BIGNUM *p, *g;
+  BIGNUM *p, *p2, *g;
   int r;
-  if (dh_param_p && dh_param_g)
+  if (dh_param_p && dh_param_g && dh_param_p_tls)
     return;
 
   p = BN_new();
+  p2 = BN_new();
   g = BN_new();
   tor_assert(p);
+  tor_assert(p2);
   tor_assert(g);
 
   /* This is from rfc2409, section 6.2.  It's a safe prime, and
@@ -1716,10 +1720,20 @@ init_dh_param(void)
                 "A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE6"
                 "49286651ECE65381FFFFFFFFFFFFFFFF");
   tor_assert(r);
+  /* This is the 1024-bit safe prime that Apache uses for its DH stuff; see
+   * modules/ssl/ssl_engine_dh.c */
+  r = BN_hex2bn(&p2,
+                  "D67DE440CBBBDC1936D693D34AFD0AD50C84D239A45F520BB88174CB98"
+                "BCE951849F912E639C72FB13B4B4D7177E16D55AC179BA420B2A29FE324A"
+                "467A635E81FF5901377BEDDCFD33168A461AAD3B72DAE8860078045B07A7"
+                "DBCA7874087D1510EA9FCC9DDD330507DD62DB88AEAA747DE0F4D6E2BD68"
+                "B0E7393E0F24218EB3");
+  tor_assert(r);
 
   r = BN_set_word(g, 2);
   tor_assert(r);
   dh_param_p = p;
+  dh_param_p_tls = p2;
   dh_param_g = g;
 }
 
@@ -1728,18 +1742,26 @@ init_dh_param(void)
 /** Allocate and return a new DH object for a key exchange.
  */
 crypto_dh_env_t *
-crypto_dh_new(void)
+crypto_dh_new(int dh_type)
 {
   crypto_dh_env_t *res = tor_malloc_zero(sizeof(crypto_dh_env_t));
 
+  tor_assert(dh_type == DH_TYPE_CIRCUIT || dh_type == DH_TYPE_TLS ||
+             dh_type == DH_TYPE_REND);
+
   if (!dh_param_p)
     init_dh_param();
 
   if (!(res->dh = DH_new()))
     goto err;
 
-  if (!(res->dh->p = BN_dup(dh_param_p)))
-    goto err;
+  if (dh_type == DH_TYPE_TLS) {
+    if (!(res->dh->p = BN_dup(dh_param_p_tls)))
+      goto err;
+  } else {
+    if (!(res->dh->p = BN_dup(dh_param_p)))
+      goto err;
+  }
 
   if (!(res->dh->g = BN_dup(dh_param_g)))
     goto err;
diff --git a/src/common/crypto.h b/src/common/crypto.h
index c306bec276..7134956731 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -195,7 +195,10 @@ void crypto_hmac_sha1(char *hmac_out,
                       const char *msg, size_t msg_len);
 
 /* Key negotiation */
-crypto_dh_env_t *crypto_dh_new(void);
+#define DH_TYPE_CIRCUIT 1
+#define DH_TYPE_REND 2
+#define DH_TYPE_TLS 3
+crypto_dh_env_t *crypto_dh_new(int dh_type);
 int crypto_dh_get_bytes(crypto_dh_env_t *dh);
 int crypto_dh_generate_public(crypto_dh_env_t *dh);
 int crypto_dh_get_public(crypto_dh_env_t *dh, char *pubkey_out,
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 9d22657f6d..8ad0f2f310 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -807,7 +807,7 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime)
   if (!SSL_CTX_check_private_key(result->ctx))
     goto error;
   {
-    crypto_dh_env_t *dh = crypto_dh_new();
+    crypto_dh_env_t *dh = crypto_dh_new(DH_TYPE_TLS);
     SSL_CTX_set_tmp_dh(result->ctx, _crypto_dh_env_get_dh(dh));
     crypto_dh_free(dh);
   }
diff --git a/src/or/onion.c b/src/or/onion.c
index 323e0003e6..9aa16d2747 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -184,7 +184,7 @@ onion_skin_create(crypto_pk_env_t *dest_router_key,
   *handshake_state_out = NULL;
   memset(onion_skin_out, 0, ONIONSKIN_CHALLENGE_LEN);
 
-  if (!(dh = crypto_dh_new()))
+  if (!(dh = crypto_dh_new(DH_TYPE_CIRCUIT)))
     goto err;
 
   dhbytes = crypto_dh_get_bytes(dh);
@@ -258,7 +258,7 @@ onion_skin_server_handshake(const char *onion_skin, /*ONIONSKIN_CHALLENGE_LEN*/
     goto err;
   }
 
-  dh = crypto_dh_new();
+  dh = crypto_dh_new(DH_TYPE_CIRCUIT);
   if (crypto_dh_get_public(dh, handshake_reply_out, DH_KEY_LEN)) {
     log_info(LD_GENERAL, "crypto_dh_get_public failed.");
     goto err;
diff --git a/src/or/policies.c b/src/or/policies.c
index 6947222b2e..62e048cfc2 100644
--- a/src/or/policies.c
+++ b/src/or/policies.c
@@ -1243,8 +1243,8 @@ policy_summarize(smartlist_t *policy)
   accepts_str = smartlist_join_strings(accepts, ",", 0, &accepts_len);
   rejects_str = smartlist_join_strings(rejects, ",", 0, &rejects_len);
 
-  if (rejects_len > MAX_EXITPOLICY_SUMMARY_LEN &&
-      accepts_len > MAX_EXITPOLICY_SUMMARY_LEN) {
+  if (rejects_len > MAX_EXITPOLICY_SUMMARY_LEN-strlen("reject")-1 &&
+      accepts_len > MAX_EXITPOLICY_SUMMARY_LEN-strlen("accept")-1) {
     char *c;
     shorter_str = accepts_str;
     prefix = "accept";
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 1306fe0710..ba5987c698 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -121,7 +121,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc,
     cpath = rendcirc->build_state->pending_final_cpath =
       tor_malloc_zero(sizeof(crypt_path_t));
     cpath->magic = CRYPT_PATH_MAGIC;
-    if (!(cpath->dh_handshake_state = crypto_dh_new())) {
+    if (!(cpath->dh_handshake_state = crypto_dh_new(DH_TYPE_REND))) {
       log_warn(LD_BUG, "Internal error: couldn't allocate DH.");
       goto err;
     }
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index f1480e0b7f..45039822f8 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1100,7 +1100,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request,
   }
 
   /* Try DH handshake... */
-  dh = crypto_dh_new();
+  dh = crypto_dh_new(DH_TYPE_REND);
   if (!dh || crypto_dh_generate_public(dh)<0) {
     log_warn(LD_BUG,"Internal error: couldn't build DH state "
              "or generate public key.");
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 08f81d9f76..5ceb298b8b 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1720,6 +1720,10 @@ extrainfo_parse_entry_from_string(const char *s, const char *end,
 authority_cert_t *
 authority_cert_parse_from_string(const char *s, const char **end_of_string)
 {
+  /** Reject any certificate at least this big; it is probably an overflow, an
+   * attack, a bug, or some other nonsense. */
+#define MAX_CERT_SIZE (128*1024)
+
   authority_cert_t *cert = NULL, *old_cert;
   smartlist_t *tokens = NULL;
   char digest[DIGEST_LEN];
@@ -1747,6 +1751,12 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
   ++eos;
   len = eos - s;
 
+  if (len > MAX_CERT_SIZE) {
+    log_warn(LD_DIR, "Certificate is far too big (at %lu bytes long); "
+             "rejecting", (unsigned long)len);
+    return NULL;
+  }
+
   tokens = smartlist_create();
   area = memarea_new();
   if (tokenize_string(area,s, eos, tokens, dir_key_certificate_table, 0) < 0) {
@@ -3818,6 +3828,9 @@ get_next_token(memarea_t *area,
   /** Reject any object at least this big; it is probably an overflow, an
    * attack, a bug, or some other nonsense. */
 #define MAX_UNPARSED_OBJECT_SIZE (128*1024)
+  /** Reject any line at least this big; it is probably an overflow, an
+   * attack, a bug, or some other nonsense. */
+#define MAX_LINE_LENGTH (128*1024)
 
   const char *next, *eol, *obstart;
   size_t obname_len;
@@ -3837,6 +3850,10 @@ get_next_token(memarea_t *area,
   eol = memchr(*s, '\n', eos-*s);
   if (!eol)
     eol = eos;
+  if (eol - *s > MAX_LINE_LENGTH) {
+    RET_ERR("Line far too long");
+  }
+
   next = find_whitespace_eos(*s, eol);
 
   if (!strcmp_len(*s, "opt", next-*s)) {
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index 8093c9c898..6ea7f295ed 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -12,8 +12,8 @@
 static void
 test_crypto_dh(void)
 {
-  crypto_dh_env_t *dh1 = crypto_dh_new();
-  crypto_dh_env_t *dh2 = crypto_dh_new();
+  crypto_dh_env_t *dh1 = crypto_dh_new(DH_TYPE_CIRCUIT);
+  crypto_dh_env_t *dh2 = crypto_dh_new(DH_TYPE_CIRCUIT);
   char p1[DH_BYTES];
   char p2[DH_BYTES];
   char s1[DH_BYTES];