Merge branch 'maint-0.2.4' into release-0.2.4

changes/bug10485

@@ -0,0 +1,4 @@
+  o Minor bugfixes:
+    - Move message about circuit handshake counts into the heartbeat
+      message where it belongs, instead of logging it once per hour
+      unconditionally. Fixes bug 10485; bugfix on 0.2.4.17-rc.

changes/bug10793

@@ -0,0 +1,4 @@
+  o Minor features (security):
+    - Always clear OpenSSL bignums before freeing them--even bignums
+      that don't contain secrets. Resolves ticket 10793. Patch by
+      Florent Daigniere.

changes/bug10835

@@ -0,0 +1,4 @@
+  o Minor bugfixes (testing):
+    - Fix a segmentation fault in our benchmark code when running with
+      Fedora's OpenSSL package, or any other OpenSSL that provides
+      ECDH but not P224. Fixes bug 10835; bugfix on 0.2.4.8-alpha.

changes/bug9602

@@ -0,0 +1,5 @@
+ o Bugfixes
+   - Null out orconn->chan->conn when closing orconn in case orconn is freed
+     before channel_run_cleanup() gets to orconn->chan, and handle the null
+     conn edge case correctly in channel_tls_t methods.  Fixes bug #9602;
+     bugfix on 0.2.4.4-alpha.

changes/bug9716

@@ -0,0 +1,4 @@
+  o Bugfixes (performance):
+    - Set the listen() backlog limit to the largest actually supported
+      on the system, not to the value in a header file. Fixes bug 9716;
+      bugfix on every released Tor.

src/common/crypto.c

@@ -486,7 +486,7 @@ crypto_pk_generate_key_with_bits(crypto_pk_t *env, int bits)
     r = NULL;
   done:
     if (e)
-      BN_free(e);
+      BN_clear_free(e);
     if (r)
       RSA_free(r);
   }
@@ -1922,7 +1922,7 @@ crypto_set_tls_dh_prime(const char *dynamic_dh_modulus_fname)
 
   /* If the space is occupied, free the previous TLS DH prime */
   if (dh_param_p_tls) {
-    BN_free(dh_param_p_tls);
+    BN_clear_free(dh_param_p_tls);
     dh_param_p_tls = NULL;
   }
 
@@ -2084,8 +2084,8 @@ crypto_dh_generate_public(crypto_dh_t *dh)
     log_warn(LD_CRYPTO, "Weird! Our own DH key was invalid.  I guess once-in-"
              "the-universe chances really do happen.  Trying again.");
     /* Free and clear the keys, so OpenSSL will actually try again. */
-    BN_free(dh->dh->pub_key);
-    BN_free(dh->dh->priv_key);
+    BN_clear_free(dh->dh->pub_key);
+    BN_clear_free(dh->dh->priv_key);
     dh->dh->pub_key = dh->dh->priv_key = NULL;
     goto again;
   }
@@ -2147,10 +2147,10 @@ tor_check_dh_key(int severity, BIGNUM *bn)
     log_fn(severity, LD_CRYPTO, "DH key must be at most p-2.");
     goto err;
   }
-  BN_free(x);
+  BN_clear_free(x);
   return 0;
  err:
-  BN_free(x);
+  BN_clear_free(x);
   s = BN_bn2hex(bn);
   log_fn(severity, LD_CRYPTO, "Rejecting insecure DH key [%s]", s);
   OPENSSL_free(s);
@@ -2209,7 +2209,7 @@ crypto_dh_compute_secret(int severity, crypto_dh_t *dh,
  done:
   crypto_log_errors(LOG_WARN, "completing DH handshake");
   if (pubkey_bn)
-    BN_free(pubkey_bn);
+    BN_clear_free(pubkey_bn);
   if (secret_tmp) {
     memwipe(secret_tmp, 0, secret_tmp_len);
     tor_free(secret_tmp);
@@ -3118,11 +3118,11 @@ crypto_global_cleanup(void)
   ERR_free_strings();
 
   if (dh_param_p)
-    BN_free(dh_param_p);
+    BN_clear_free(dh_param_p);
   if (dh_param_p_tls)
-    BN_free(dh_param_p_tls);
+    BN_clear_free(dh_param_p_tls);
   if (dh_param_g)
-    BN_free(dh_param_g);
+    BN_clear_free(dh_param_g);
 
 #ifndef DISABLE_ENGINES
   ENGINE_cleanup();

src/common/tortls.c

@@ -694,7 +694,7 @@ tor_tls_create_certificate(crypto_pk_t *rsa,
   if (pkey)
     EVP_PKEY_free(pkey);
   if (serial_number)
-    BN_free(serial_number);
+    BN_clear_free(serial_number);
   if (name)
     X509_NAME_free(name);
   if (name_issuer)

src/or/channeltls.c

@@ -53,6 +53,7 @@ static void channel_tls_common_init(channel_tls_t *tlschan);
 
 static void channel_tls_close_method(channel_t *chan);
 static const char * channel_tls_describe_transport_method(channel_t *chan);
+static void channel_tls_free_method(channel_t *chan);
 static int
 channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out);
 static const char *
@@ -112,6 +113,7 @@ channel_tls_common_init(channel_tls_t *tlschan)
   chan->state = CHANNEL_STATE_OPENING;
   chan->close = channel_tls_close_method;
   chan->describe_transport = channel_tls_describe_transport_method;
+  chan->free = channel_tls_free_method;
   chan->get_remote_addr = channel_tls_get_remote_addr_method;
   chan->get_remote_descr = channel_tls_get_remote_descr_method;
   chan->has_queued_writes = channel_tls_has_queued_writes_method;
@@ -365,7 +367,7 @@ channel_tls_describe_transport_method(channel_t *chan)
 
   tor_assert(chan);
 
-   tlschan = BASE_CHAN_TO_TLS(chan);
+  tlschan = BASE_CHAN_TO_TLS(chan);
 
   if (tlschan->conn) {
     id = TO_CONN(tlschan->conn)->global_identifier;
@@ -383,6 +385,30 @@ channel_tls_describe_transport_method(channel_t *chan)
   return rv;
 }
 
+/**
+ * Free a channel_tls_t
+ *
+ * This is called by the generic channel layer when freeing a channel_tls_t;
+ * this happens either on a channel which has already reached
+ * CHANNEL_STATE_CLOSED or CHANNEL_STATE_ERROR from channel_run_cleanup() or
+ * on shutdown from channel_free_all().  In the latter case we might still
+ * have an orconn active (which connection_free_all() will get to later),
+ * so we should null out its channel pointer now.
+ */
+
+static void
+channel_tls_free_method(channel_t *chan)
+{
+  channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
+
+  tor_assert(tlschan);
+
+  if (tlschan->conn) {
+    tlschan->conn->chan = NULL;
+    tlschan->conn = NULL;
+  }
+}
+
 /**
  * Get the remote address of a channel_tls_t
  *
@@ -394,15 +420,18 @@ channel_tls_describe_transport_method(channel_t *chan)
 static int
 channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out)
 {
+  int rv = 0;
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
 
   tor_assert(tlschan);
   tor_assert(addr_out);
-  tor_assert(tlschan->conn);
 
-  tor_addr_copy(addr_out, &(TO_CONN(tlschan->conn)->addr));
+  if (tlschan->conn) {
+    tor_addr_copy(addr_out, &(TO_CONN(tlschan->conn)->addr));
+    rv = 1;
+  } else tor_addr_make_unspec(addr_out);
 
-  return 1;
+  return rv;
 }
 
 /**
@@ -426,41 +455,43 @@ channel_tls_get_remote_descr_method(channel_t *chan, int flags)
   char *addr_str;
 
   tor_assert(tlschan);
-  tor_assert(tlschan->conn);
 
-  conn = TO_CONN(tlschan->conn);
-
-  switch (flags) {
-    case 0:
-      /* Canonical address with port*/
-      tor_snprintf(buf, MAX_DESCR_LEN + 1,
-                   "%s:%u", conn->address, conn->port);
-      answer = buf;
-      break;
-    case GRD_FLAG_ORIGINAL:
-      /* Actual address with port */
-      addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
-      tor_snprintf(buf, MAX_DESCR_LEN + 1,
-                   "%s:%u", addr_str, conn->port);
-      tor_free(addr_str);
-      answer = buf;
-      break;
-    case GRD_FLAG_ADDR_ONLY:
-      /* Canonical address, no port */
-      strlcpy(buf, conn->address, sizeof(buf));
-      answer = buf;
-      break;
-    case GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY:
-      /* Actual address, no port */
-      addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
-      strlcpy(buf, addr_str, sizeof(buf));
-      tor_free(addr_str);
-      answer = buf;
-      break;
-
-    default:
-      /* Something's broken in channel.c */
-      tor_assert(1);
+  if (tlschan->conn) {
+    conn = TO_CONN(tlschan->conn);
+    switch (flags) {
+      case 0:
+        /* Canonical address with port*/
+        tor_snprintf(buf, MAX_DESCR_LEN + 1,
+                     "%s:%u", conn->address, conn->port);
+        answer = buf;
+        break;
+      case GRD_FLAG_ORIGINAL:
+        /* Actual address with port */
+        addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
+        tor_snprintf(buf, MAX_DESCR_LEN + 1,
+                     "%s:%u", addr_str, conn->port);
+        tor_free(addr_str);
+        answer = buf;
+        break;
+      case GRD_FLAG_ADDR_ONLY:
+        /* Canonical address, no port */
+        strlcpy(buf, conn->address, sizeof(buf));
+        answer = buf;
+        break;
+      case GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY:
+        /* Actual address, no port */
+        addr_str = tor_dup_addr(&(tlschan->conn->real_addr));
+        strlcpy(buf, addr_str, sizeof(buf));
+        tor_free(addr_str);
+        answer = buf;
+        break;
+      default:
+        /* Something's broken in channel.c */
+        tor_assert(1);
+    }
+  } else {
+    strlcpy(buf, "(No connection)", sizeof(buf));
+    answer = buf;
   }
 
   return answer;
@@ -480,9 +511,16 @@ channel_tls_has_queued_writes_method(channel_t *chan)
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
 
   tor_assert(tlschan);
-  tor_assert(tlschan->conn);
+  if (!(tlschan->conn)) {
+    log_info(LD_CHANNEL,
+             "something called has_queued_writes on a tlschan "
+             "(%p with ID " U64_FORMAT " but no conn",
+             chan, U64_PRINTF_ARG(chan->global_identifier));
+  }
 
-  outbuf_len = connection_get_outbuf_len(TO_CONN(tlschan->conn));
+  outbuf_len = (tlschan->conn != NULL) ?
+    connection_get_outbuf_len(TO_CONN(tlschan->conn)) :
+    0;
 
   return (outbuf_len > 0);
 }
@@ -502,24 +540,26 @@ channel_tls_is_canonical_method(channel_t *chan, int req)
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
 
   tor_assert(tlschan);
-  tor_assert(tlschan->conn);
 
-  switch (req) {
-    case 0:
-      answer = tlschan->conn->is_canonical;
-      break;
-    case 1:
-      /*
-       * Is the is_canonical bit reliable?  In protocols version 2 and up
-       * we get the canonical address from a NETINFO cell, but in older
-       * versions it might be based on an obsolete descriptor.
-       */
-      answer = (tlschan->conn->link_proto >= 2);
-      break;
-    default:
-      /* This shouldn't happen; channel.c is broken if it does */
-      tor_assert(1);
+  if (tlschan->conn) {
+    switch (req) {
+      case 0:
+        answer = tlschan->conn->is_canonical;
+        break;
+      case 1:
+        /*
+         * Is the is_canonical bit reliable?  In protocols version 2 and up
+         * we get the canonical address from a NETINFO cell, but in older
+         * versions it might be based on an obsolete descriptor.
+         */
+        answer = (tlschan->conn->link_proto >= 2);
+        break;
+      default:
+        /* This shouldn't happen; channel.c is broken if it does */
+        tor_assert(1);
+    }
   }
+  /* else return 0 for tlschan->conn == NULL */
 
   return answer;
 }
@@ -540,6 +580,15 @@ channel_tls_matches_extend_info_method(channel_t *chan,
   tor_assert(tlschan);
   tor_assert(extend_info);
 
+  /* Never match if we have no conn */
+  if (!(tlschan->conn)) {
+    log_info(LD_CHANNEL,
+             "something called matches_extend_info on a tlschan "
+             "(%p with ID " U64_FORMAT " but no conn",
+             chan, U64_PRINTF_ARG(chan->global_identifier));
+    return 0;
+  }
+
   return (tor_addr_eq(&(extend_info->addr),
                       &(TO_CONN(tlschan->conn)->addr)) &&
          (extend_info->port == TO_CONN(tlschan->conn)->port));
@@ -561,7 +610,15 @@ channel_tls_matches_target_method(channel_t *chan,
 
   tor_assert(tlschan);
   tor_assert(target);
-  tor_assert(tlschan->conn);
+
+  /* Never match if we have no conn */
+  if (!(tlschan->conn)) {
+    log_info(LD_CHANNEL,
+             "something called matches_target on a tlschan "
+             "(%p with ID " U64_FORMAT " but no conn",
+             chan, U64_PRINTF_ARG(chan->global_identifier));
+    return 0;
+  }
 
   return tor_addr_eq(&(tlschan->conn->real_addr), target);
 }
@@ -577,14 +634,22 @@ static int
 channel_tls_write_cell_method(channel_t *chan, cell_t *cell)
 {
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
+  int written = 0;
 
   tor_assert(tlschan);
   tor_assert(cell);
-  tor_assert(tlschan->conn);
 
-  connection_or_write_cell_to_buf(cell, tlschan->conn);
+  if (tlschan->conn) {
+    connection_or_write_cell_to_buf(cell, tlschan->conn);
+    ++written;
+  } else {
+    log_info(LD_CHANNEL,
+             "something called write_cell on a tlschan "
+             "(%p with ID " U64_FORMAT " but no conn",
+             chan, U64_PRINTF_ARG(chan->global_identifier));
+  }
 
-  return 1;
+  return written;
 }
 
 /**
@@ -600,18 +665,26 @@ channel_tls_write_packed_cell_method(channel_t *chan,
 {
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
   size_t cell_network_size = get_cell_network_size(chan->wide_circ_ids);
+  int written = 0;
 
   tor_assert(tlschan);
   tor_assert(packed_cell);
-  tor_assert(tlschan->conn);
 
-  connection_write_to_buf(packed_cell->body, cell_network_size,
-                          TO_CONN(tlschan->conn));
+  if (tlschan->conn) {
+    connection_write_to_buf(packed_cell->body, cell_network_size,
+                            TO_CONN(tlschan->conn));
 
-  /* This is where the cell is finished; used to be done from relay.c */
-  packed_cell_free(packed_cell);
+    /* This is where the cell is finished; used to be done from relay.c */
+    packed_cell_free(packed_cell);
+    ++written;
+  } else {
+    log_info(LD_CHANNEL,
+             "something called write_packed_cell on a tlschan "
+             "(%p with ID " U64_FORMAT " but no conn",
+             chan, U64_PRINTF_ARG(chan->global_identifier));
+  }
 
-  return 1;
+  return written;
 }
 
 /**
@@ -625,14 +698,22 @@ static int
 channel_tls_write_var_cell_method(channel_t *chan, var_cell_t *var_cell)
 {
   channel_tls_t *tlschan = BASE_CHAN_TO_TLS(chan);
+  int written = 0;
 
   tor_assert(tlschan);
   tor_assert(var_cell);
-  tor_assert(tlschan->conn);
 
-  connection_or_write_var_cell_to_buf(var_cell, tlschan->conn);
+  if (tlschan->conn) {
+    connection_or_write_var_cell_to_buf(var_cell, tlschan->conn);
+    ++written;
+  } else {
+    log_info(LD_CHANNEL,
+             "something called write_var_cell on a tlschan "
+             "(%p with ID " U64_FORMAT " but no conn",
+             chan, U64_PRINTF_ARG(chan->global_identifier));
+  }
 
-  return 1;
+  return written;
 }
 
 /*************************************************

src/or/connection.c

@@ -514,6 +514,22 @@ connection_free_(connection_t *conn)
     or_handshake_state_free(or_conn->handshake_state);
     or_conn->handshake_state = NULL;
     tor_free(or_conn->nickname);
+    if (or_conn->chan) {
+      /* Owww, this shouldn't happen, but... */
+      log_info(LD_CHANNEL,
+               "Freeing orconn at %p, saw channel %p with ID "
+               U64_FORMAT " left un-NULLed",
+               or_conn, TLS_CHAN_TO_BASE(or_conn->chan),
+               U64_PRINTF_ARG(
+                 TLS_CHAN_TO_BASE(or_conn->chan)->global_identifier));
+      if (!(TLS_CHAN_TO_BASE(or_conn->chan)->state == CHANNEL_STATE_CLOSED ||
+            TLS_CHAN_TO_BASE(or_conn->chan)->state == CHANNEL_STATE_ERROR)) {
+        channel_close_for_error(TLS_CHAN_TO_BASE(or_conn->chan));
+      }
+
+      or_conn->chan->conn = NULL;
+      or_conn->chan = NULL;
+    }
   }
   if (conn->type == CONN_TYPE_AP) {
     entry_connection_t *entry_conn = TO_ENTRY_CONN(conn);
@@ -926,6 +942,27 @@ make_socket_reuseable(tor_socket_t sock)
 #endif
 }
 
+/** Max backlog to pass to listen.  We start at */
+static int listen_limit = INT_MAX;
+
+/* Listen on <b>fd</b> with appropriate backlog. Return as for listen. */
+static int
+tor_listen(tor_socket_t fd)
+{
+  int r;
+
+  if ((r = listen(fd, listen_limit)) < 0) {
+    if (listen_limit == SOMAXCONN)
+      return r;
+    if ((r = listen(fd, SOMAXCONN)) == 0) {
+      listen_limit = SOMAXCONN;
+      log_warn(LD_NET, "Setting listen backlog to INT_MAX connections "
+               "didn't work, but SOMAXCONN did. Lowering backlog limit.");
+    }
+  }
+  return r;
+}
+
 /** Bind a new non-blocking socket listening to the socket described
  * by <b>listensockaddr</b>.
  *
@@ -1009,7 +1046,7 @@ connection_listener_new(const struct sockaddr *listensockaddr,
     }
 
     if (is_tcp) {
-      if (listen(s,SOMAXCONN) < 0) {
+      if (tor_listen(s) < 0) {
         log_warn(LD_NET, "Could not listen on %s:%u: %s", address, usePort,
                  tor_socket_strerror(tor_socket_errno(s)));
         tor_close_socket(s);

src/or/connection_or.c

@@ -622,6 +622,11 @@ connection_or_about_to_close(or_connection_t *or_conn)
   /* Tell the controlling channel we're closed */
   if (or_conn->chan) {
     channel_closed(TLS_CHAN_TO_BASE(or_conn->chan));
+    /*
+     * NULL this out because the channel might hang around a little
+     * longer before channel_run_cleanup() gets it.
+     */
+    or_conn->chan->conn = NULL;
     or_conn->chan = NULL;
   }
 

src/or/main.c

@@ -1353,11 +1353,6 @@ run_scheduled_events(time_t now)
         next_time_to_write_stats_files = next_write;
     }
     time_to_write_stats_files = next_time_to_write_stats_files;
-
-    /* Also commandeer this opportunity to log how our circuit handshake
-     * stats have been doing. */
-    if (public_server_mode(options))
-      rep_hist_log_circuit_handshake_stats(now);
   }
 
   /* 1h. Check whether we should write bridge statistics to disk.

src/or/rephist.c

@@ -3041,7 +3041,6 @@ void
 rep_hist_log_circuit_handshake_stats(time_t now)
 {
   (void)now;
-  /* XXX024 maybe quiet this log message before 0.2.4 goes stable for real */
   log_notice(LD_HIST, "Circuit handshake stats since last time: "
              "%d/%d TAP, %d/%d NTor.",
              onion_handshakes_completed[ONION_HANDSHAKE_TYPE_TAP],

src/or/status.c

@@ -15,6 +15,7 @@
 #include "circuitlist.h"
 #include "main.h"
 #include "hibernate.h"
+#include "rephist.h"
 
 /** Return the total number of circuits. */
 static int
@@ -121,6 +122,9 @@ log_heartbeat(time_t now)
     log_notice(LD_HEARTBEAT, "TLS write overhead: %.f%%", overhead);
   }
 
+  if (public_server_mode(options))
+    rep_hist_log_circuit_handshake_stats(now);
+
   tor_free(uptime);
   tor_free(bw_sent);
   tor_free(bw_rcvd);

src/test/bench.c

@@ -440,6 +440,10 @@ bench_ecdh_impl(int nid, const char *name)
     ssize_t slen_a, slen_b;
     EC_KEY *dh_a = EC_KEY_new_by_curve_name(nid);
     EC_KEY *dh_b = EC_KEY_new_by_curve_name(nid);
+    if (!dh_a || !dh_b) {
+      puts("Skipping.  (No implementation?)");
+      return;
+    }
 
     EC_KEY_generate_key(dh_a);
     EC_KEY_generate_key(dh_b);