diff --git a/changes/bug10485 b/changes/bug10485
new file mode 100644
index 000000000..7e5fa530e
--- /dev/null
+++ b/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.
diff --git a/changes/bug10793 b/changes/bug10793
new file mode 100644
index 000000000..24c4025dd
--- /dev/null
+++ b/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.
diff --git a/changes/bug10835 b/changes/bug10835
new file mode 100644
index 000000000..9df7bdd27
--- /dev/null
+++ b/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.
diff --git a/changes/bug9602 b/changes/bug9602
new file mode 100644
index 000000000..2dc13c4c0
--- /dev/null
+++ b/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.
diff --git a/changes/bug9716 b/changes/bug9716
new file mode 100644
index 000000000..5e3907717
--- /dev/null
+++ b/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.
diff --git a/src/common/crypto.c b/src/common/crypto.c
index 940a756f6..925beb352 100644
--- a/src/common/crypto.c
+++ b/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();
diff --git a/src/common/tortls.c b/src/common/tortls.c
index 60444f1b8..886ee0dda 100644
--- a/src/common/tortls.c
+++ b/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)
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index f751c0da9..d5428c1ab 100644
--- a/src/or/channeltls.c
+++ b/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;
}
/*************************************************
diff --git a/src/or/connection.c b/src/or/connection.c
index 78cc31e89..4f74a1d04 100644
--- a/src/or/connection.c
+++ b/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 fd 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 listensockaddr.
*
@@ -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);
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index 3d16e1453..8e7cd9ea5 100644
--- a/src/or/connection_or.c
+++ b/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;
}
diff --git a/src/or/main.c b/src/or/main.c
index deed798e8..bd23141b9 100644
--- a/src/or/main.c
+++ b/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.
diff --git a/src/or/rephist.c b/src/or/rephist.c
index 131e531b1..2948bf8f0 100644
--- a/src/or/rephist.c
+++ b/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],
diff --git a/src/or/status.c b/src/or/status.c
index d239e6ee7..69f92ed09 100644
--- a/src/or/status.c
+++ b/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);
diff --git a/src/test/bench.c b/src/test/bench.c
index 5a8d21d17..706b8bc7f 100644
--- a/src/test/bench.c
+++ b/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);