- tor-commits - lists.torproject.org

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.2' into maint-0.2.3
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit a0e9dc9f55a452d78d9d16b4a2fc7d57dafa0409 Merge: e2549c3 8743080 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 19 00:58:33 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.2' into maint-0.2.3 changes/bug7139 | 9 +++++++++ src/common/tortls.c | 8 ++++++++ 2 files changed, 17 insertions(+), 0 deletions(-)

1 0

[tor/master] Disable TLS Session Tickets, which we were apparently getting for free
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 8743080a289a20bfaf0a67d6382ba0c2a6d6534d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 19:57:27 2012 -0400 Disable TLS Session Tickets, which we were apparently getting for free OpenSSL 1.0.0 added an implementation of TLS session tickets, a "feature" that let session resumption occur without server-side state by giving clients an encrypted "ticket" that the client could present later to get the session going again with the same keys as before. OpenSSL was giving the keys to decrypt these tickets the lifetime of the SSL contexts, which would have been terrible for PFS if we had long-lived SSL contexts. Fortunately, we don't. Still, it's pretty bad. We should also drop these, since our use of the extension stands out with our non-use of session cacheing. Found by nextgens. Bugfix on all versions of Tor when built with openssl 1.0.0 or later. Fixes bug 7139. --- changes/bug7139 | 9 +++++++++ src/common/tortls.c | 8 ++++++++ 2 files changed, 17 insertions(+), 0 deletions(-) diff --git a/changes/bug7139 b/changes/bug7139 new file mode 100644 index 0000000..dfb7d32 --- /dev/null +++ b/changes/bug7139 @@ -0,0 +1,9 @@ + o Major bugfixes (security): + + - Disable TLS session tickets. OpenSSL's implementation were giving + our TLS session keys the lifetime of our TLS context objects, when + perfect forward secrecy would want us to discard anything that + could decrypt a link connection as soon as the link connection was + closed. Fixes bug 7139; bugfix on all versions of Tor linked + against OpenSSL 1.0.0 or later. Found by "nextgens". + diff --git a/src/common/tortls.c b/src/common/tortls.c index c631612..fc0bcb9 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -804,6 +804,14 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, #ifdef SSL_OP_NO_TLSv1_1 SSL_CTX_set_options(result->ctx, SSL_OP_NO_TLSv1_1); #endif + /* Disable TLS tickets if they're supported. We never want to use them; + * using them can make our perfect forward secrecy a little worse, *and* + * create an opportunity to fingerprint us (since it's unusual to use them + * with TLS sessions turned off). + */ +#ifdef SSL_OP_NO_TICKET + SSL_CTX_set_options(result->ctx, SSL_OP_NO_TICKET); +#endif if ( #ifdef DISABLE_SSL3_HANDSHAKE

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.3'
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 9f1b1ef4fbc67b2d3f6e2f9b2dd4d8cd44dc49b7 Merge: 981f25a a0e9dc9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 19 01:01:26 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' changes/bug7139 | 9 +++++++++ src/common/tortls.c | 8 ++++++++ 2 files changed, 17 insertions(+), 0 deletions(-)

1 0

[tor/maint-0.2.3] Disable TLS Session Tickets, which we were apparently getting for free
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 8743080a289a20bfaf0a67d6382ba0c2a6d6534d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 19:57:27 2012 -0400 Disable TLS Session Tickets, which we were apparently getting for free OpenSSL 1.0.0 added an implementation of TLS session tickets, a "feature" that let session resumption occur without server-side state by giving clients an encrypted "ticket" that the client could present later to get the session going again with the same keys as before. OpenSSL was giving the keys to decrypt these tickets the lifetime of the SSL contexts, which would have been terrible for PFS if we had long-lived SSL contexts. Fortunately, we don't. Still, it's pretty bad. We should also drop these, since our use of the extension stands out with our non-use of session cacheing. Found by nextgens. Bugfix on all versions of Tor when built with openssl 1.0.0 or later. Fixes bug 7139. --- changes/bug7139 | 9 +++++++++ src/common/tortls.c | 8 ++++++++ 2 files changed, 17 insertions(+), 0 deletions(-) diff --git a/changes/bug7139 b/changes/bug7139 new file mode 100644 index 0000000..dfb7d32 --- /dev/null +++ b/changes/bug7139 @@ -0,0 +1,9 @@ + o Major bugfixes (security): + + - Disable TLS session tickets. OpenSSL's implementation were giving + our TLS session keys the lifetime of our TLS context objects, when + perfect forward secrecy would want us to discard anything that + could decrypt a link connection as soon as the link connection was + closed. Fixes bug 7139; bugfix on all versions of Tor linked + against OpenSSL 1.0.0 or later. Found by "nextgens". + diff --git a/src/common/tortls.c b/src/common/tortls.c index c631612..fc0bcb9 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -804,6 +804,14 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, #ifdef SSL_OP_NO_TLSv1_1 SSL_CTX_set_options(result->ctx, SSL_OP_NO_TLSv1_1); #endif + /* Disable TLS tickets if they're supported. We never want to use them; + * using them can make our perfect forward secrecy a little worse, *and* + * create an opportunity to fingerprint us (since it's unusual to use them + * with TLS sessions turned off). + */ +#ifdef SSL_OP_NO_TICKET + SSL_CTX_set_options(result->ctx, SSL_OP_NO_TICKET); +#endif if ( #ifdef DISABLE_SSL3_HANDSHAKE

1 0

[tor/maint-0.2.3] Merge remote-tracking branch 'origin/maint-0.2.2' into maint-0.2.3
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit a0e9dc9f55a452d78d9d16b4a2fc7d57dafa0409 Merge: e2549c3 8743080 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Oct 19 00:58:33 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.2' into maint-0.2.3 changes/bug7139 | 9 +++++++++ src/common/tortls.c | 8 ++++++++ 2 files changed, 17 insertions(+), 0 deletions(-)

1 0

[tor/maint-0.2.2] Disable TLS Session Tickets, which we were apparently getting for free
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 8743080a289a20bfaf0a67d6382ba0c2a6d6534d Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 19:57:27 2012 -0400 Disable TLS Session Tickets, which we were apparently getting for free OpenSSL 1.0.0 added an implementation of TLS session tickets, a "feature" that let session resumption occur without server-side state by giving clients an encrypted "ticket" that the client could present later to get the session going again with the same keys as before. OpenSSL was giving the keys to decrypt these tickets the lifetime of the SSL contexts, which would have been terrible for PFS if we had long-lived SSL contexts. Fortunately, we don't. Still, it's pretty bad. We should also drop these, since our use of the extension stands out with our non-use of session cacheing. Found by nextgens. Bugfix on all versions of Tor when built with openssl 1.0.0 or later. Fixes bug 7139. --- changes/bug7139 | 9 +++++++++ src/common/tortls.c | 8 ++++++++ 2 files changed, 17 insertions(+), 0 deletions(-) diff --git a/changes/bug7139 b/changes/bug7139 new file mode 100644 index 0000000..dfb7d32 --- /dev/null +++ b/changes/bug7139 @@ -0,0 +1,9 @@ + o Major bugfixes (security): + + - Disable TLS session tickets. OpenSSL's implementation were giving + our TLS session keys the lifetime of our TLS context objects, when + perfect forward secrecy would want us to discard anything that + could decrypt a link connection as soon as the link connection was + closed. Fixes bug 7139; bugfix on all versions of Tor linked + against OpenSSL 1.0.0 or later. Found by "nextgens". + diff --git a/src/common/tortls.c b/src/common/tortls.c index c631612..fc0bcb9 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -804,6 +804,14 @@ tor_tls_context_new(crypto_pk_env_t *identity, unsigned int key_lifetime, #ifdef SSL_OP_NO_TLSv1_1 SSL_CTX_set_options(result->ctx, SSL_OP_NO_TLSv1_1); #endif + /* Disable TLS tickets if they're supported. We never want to use them; + * using them can make our perfect forward secrecy a little worse, *and* + * create an opportunity to fingerprint us (since it's unusual to use them + * with TLS sessions turned off). + */ +#ifdef SSL_OP_NO_TICKET + SSL_CTX_set_options(result->ctx, SSL_OP_NO_TICKET); +#endif if ( #ifdef DISABLE_SSL3_HANDSHAKE

1 0

[tor/master] Document return value of channel_get_addr_if_possible
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit f633184af1f0dd4a0d2a4c3490d95a8aa6e964d1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 10:34:23 2012 -0400 Document return value of channel_get_addr_if_possible --- src/or/channel.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index 49ce129..10097ae 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -3506,12 +3506,12 @@ channel_get_canonical_remote_descr(channel_t *chan) } /** - * Get remote address if possible + * Get remote address if possible. * * Write the remote address out to a tor_addr_t if the underlying transport - * supports this operation. + * supports this operation, and return 1. Return 0 if the underlying transport + * doesn't let us do this. */ - int channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out) {

1 0

[tor/master] Fix for bug 7112 (spewing complaints from tor_addr_is_internal)
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 898bd1ae8f1eeb880eaaeba2caa1cda435a1ac5b Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 11:15:01 2012 -0400 Fix for bug 7112 (spewing complaints from tor_addr_is_internal) In 4768c0efe3e9471cc367c3740d1a4ba0ab79626c (not in any released version of Tor), we removed a little block of code that set the addr field of an exit connection used in making a tunneled directory request. Turns out that wasn't right. --- src/or/connection_edge.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index e7bc09a..53b57eb 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -3180,7 +3180,12 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) n_stream->on_circuit = circ; if (rh.command == RELAY_COMMAND_BEGIN_DIR) { + tor_addr_t tmp_addr; tor_assert(or_circ); + if (or_circ->p_chan && + channel_get_addr_if_possible(or_circ->p_chan, &tmp_addr)) { + tor_addr_copy(&n_stream->base_.addr, &tmp_addr); + } return connection_exit_connect_dir(n_stream); }

1 0

[tor/master] Restore the 'address' value of tunneled connections
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 26946c659b5abdc38fd4dc8cd4b4659aadcf05f0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 11:23:26 2012 -0400 Restore the 'address' value of tunneled connections When we merged the channel code, we made the 'address' field of linked directory connections created with begindir (and their associated edge connections) contain an address:port string, when they should only have contained the address part. This patch also tweaks the interface to the get_descr method of channels so that it takes a set of flags rather than a single flag. --- src/or/channel.c | 10 +++++++++- src/or/channel.h | 10 +++++++--- src/or/channeltls.c | 25 +++++++++++++++++++------ src/or/connection_edge.c | 2 +- 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index 10097ae..13aa007 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -3484,7 +3484,15 @@ channel_get_actual_remote_descr(channel_t *chan) tor_assert(chan->get_remote_descr); /* Param 1 indicates the actual description */ - return chan->get_remote_descr(chan, 1); + return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL); +} + +/** DOCDOC */ +const char * +channel_get_actual_remote_address(channel_t *chan) +{ + /* Param 1 indicates the actual description */ + return chan->get_remote_descr(chan, GRD_FLAG_ORIGINAL|GRD_FLAG_ADDR_ONLY); } /** diff --git a/src/or/channel.h b/src/or/channel.h index 33b7c8f..d90335c 100644 --- a/src/or/channel.h +++ b/src/or/channel.h @@ -79,10 +79,13 @@ struct channel_s { * available. */ int (*get_remote_addr)(channel_t *, tor_addr_t *); +#define GRD_FLAG_ORIGINAL 1 +#define GRD_FLAG_ADDR_ONLY 2 /* - * Get a text description of the remote endpoint; canonicalized if the - * arg is 0, or the one we originally connected to/received from if it's - * 1. + * Get a text description of the remote endpoint; canonicalized if the flag + * GRD_FLAG_ORIGINAL is not set, or the one we originally connected + * to/received from if it is. If GRD_FLAG_ADDR_ONLY is set, we return only + * the original address. */ const char * (*get_remote_descr)(channel_t *, int); /* Check if the lower layer has queued writes */ @@ -424,6 +427,7 @@ const char * channel_describe_transport(channel_t *chan); void channel_dump_statistics(channel_t *chan, int severity); void channel_dump_transport_statistics(channel_t *chan, int severity); const char * channel_get_actual_remote_descr(channel_t *chan); +const char * channel_get_actual_remote_address(channel_t *chan); int channel_get_addr_if_possible(channel_t *chan, tor_addr_t *addr_out); const char * channel_get_canonical_remote_descr(channel_t *chan); int channel_has_queued_writes(channel_t *chan); diff --git a/src/or/channeltls.c b/src/or/channeltls.c index 234aa31..a7ac83f 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -53,7 +53,7 @@ static const char * channel_tls_describe_transport_method(channel_t *chan); static int channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out); static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int req); +channel_tls_get_remote_descr_method(channel_t *chan, int flags); static int channel_tls_has_queued_writes_method(channel_t *chan); static int channel_tls_is_canonical_method(channel_t *chan, int req); static int @@ -412,7 +412,7 @@ channel_tls_get_remote_addr_method(channel_t *chan, tor_addr_t *addr_out) */ static const char * -channel_tls_get_remote_descr_method(channel_t *chan, int req) +channel_tls_get_remote_descr_method(channel_t *chan, int flags) { #define MAX_DESCR_LEN 32 @@ -427,21 +427,34 @@ channel_tls_get_remote_descr_method(channel_t *chan, int req) conn = TO_CONN(tlschan->conn); - switch (req) { + switch (flags) { case 0: - /* Canonical address */ + /* Canonical address with port*/ tor_snprintf(buf, MAX_DESCR_LEN + 1, "%s:%u", conn->address, conn->port); answer = buf; break; - case 1: - /* Actual address */ + 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); diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 53b57eb..4d528a8 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -3105,7 +3105,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) * we might already have corrected base_.addr[ess] for the relay's * canonical IP address. */ if (or_circ && or_circ->p_chan) - address = tor_strdup(channel_get_actual_remote_descr(or_circ->p_chan)); + address = tor_strdup(channel_get_actual_remote_address(or_circ->p_chan)); else address = tor_strdup("127.0.0.1"); port = 1; /* XXXX This value is never actually used anywhere, and there

1 0

[tor/master] Fix a bug in channel_dump_statistics
by nickm＠torproject.org 19 Oct '12

19 Oct '12

commit 850c9901445a1d2a4ce7ecf16e03f939fcd821d6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Oct 17 11:29:59 2012 -0400 Fix a bug in channel_dump_statistics We were calling channel_get_actual_remote_descr() before we used the output of a previous channel_get_canonical_remote_descr(), thus invalidating its output. --- src/or/channel.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index 3acb509..89ec4de 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -3243,6 +3243,7 @@ channel_dump_statistics(channel_t *chan, int severity) /* Handle remote address and descriptions */ have_remote_addr = channel_get_addr_if_possible(chan, &remote_addr); if (have_remote_addr) { + char *actual = tor_strdup(channel_get_actual_remote_descr(chan)); remote_addr_str = tor_dup_addr(&remote_addr); log(severity, LD_GENERAL, " * Channel " U64_FORMAT " says its remote address" @@ -3251,16 +3252,19 @@ channel_dump_statistics(channel_t *chan, int severity) U64_PRINTF_ARG(chan->global_identifier), remote_addr_str, channel_get_canonical_remote_descr(chan), - channel_get_actual_remote_descr(chan)); + actual); tor_free(remote_addr_str); + tor_free(actual); } else { + char *actual = tor_strdup(channel_get_actual_remote_descr(chan)); log(severity, LD_GENERAL, " * Channel " U64_FORMAT " does not know its remote " "address, but gives a canonical description of \"%s\" and an " "actual description of \"%s\"", U64_PRINTF_ARG(chan->global_identifier), channel_get_canonical_remote_descr(chan), - channel_get_actual_remote_descr(chan)); + actual); + tor_free(actual); } /* Handle marks */

1 0