From f357ef9dccccfb5ce2408dbd5f8456de02bac895 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 16 Oct 2012 12:55:50 -0400 Subject: [PATCH 1/3] Discard extraneous renegotiation attempts in the v3 link protocol Failure to do so left us open to a remotely triggerable assertion failure. Fixes CVE-2012-2249; bugfix on 0.2.3.6-alpha. Reported by "some guy from France". --- changes/cve-2012-2249 | 5 +++++ src/or/command.c | 1 + src/or/connection_or.c | 14 ++++++++++++-- src/or/connection_or.h | 1 + 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 changes/cve-2012-2249 diff --git a/changes/cve-2012-2249 b/changes/cve-2012-2249 new file mode 100644 index 0000000000..625bfa2f58 --- /dev/null +++ b/changes/cve-2012-2249 @@ -0,0 +1,5 @@ + o Major bugfixes (security): + - Discard extraneous renegotiation attempts once the V3 link + protocol has been initiated. Failure to do so left us open to + a remotely triggerable assertion failure. Fixes CVE-2012-2249; + bugfix on 0.2.3.6-alpha. Reported by "some guy from France". diff --git a/src/or/command.c b/src/or/command.c index d8a409bc27..975af046cd 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -649,6 +649,7 @@ enter_v3_handshake_with_cell(var_cell_t *cell, or_connection_t *conn) "Received a cell while TLS-handshaking, not in " "OR_HANDSHAKING_V3, on a connection we originated."); } + connection_or_block_renegotiation(conn); conn->_base.state = OR_CONN_STATE_OR_HANDSHAKING_V3; if (connection_init_or_handshake_state(conn, started_here) < 0) { connection_mark_for_close(TO_CONN(conn)); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index d016387935..6293fe881d 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1186,6 +1186,17 @@ connection_tls_start_handshake(or_connection_t *conn, int receiving) return 0; } +/** Block all future attempts to renegotiate on 'conn' */ +void +connection_or_block_renegotiation(or_connection_t *conn) +{ + tor_tls_t *tls = conn->tls; + if (!tls) + return; + tor_tls_set_renegotiate_callback(tls, NULL, NULL); + tor_tls_block_renegotiation(tls); +} + /** Invoked on the server side from inside tor_tls_read() when the server * gets a successful TLS renegotiation from the client. */ static void @@ -1195,8 +1206,7 @@ connection_or_tls_renegotiated_cb(tor_tls_t *tls, void *_conn) (void)tls; /* Don't invoke this again. */ - tor_tls_set_renegotiate_callback(tls, NULL, NULL); - tor_tls_block_renegotiation(tls); + connection_or_block_renegotiation(conn); if (connection_tls_finish_handshake(conn) < 0) { /* XXXX_TLS double-check that it's ok to do this from inside read. */ diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 3e98f5cce1..b78c444921 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -21,6 +21,7 @@ or_connection_t *connection_or_get_for_extend(const char *digest, int *launch_out); void connection_or_set_bad_connections(const char *digest, int force); +void connection_or_block_renegotiation(or_connection_t *conn); int connection_or_reached_eof(or_connection_t *conn); int connection_or_process_inbuf(or_connection_t *conn); int connection_or_flushed_some(or_connection_t *conn); From 8743080a289a20bfaf0a67d6382ba0c2a6d6534d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 17 Oct 2012 19:57:27 -0400 Subject: [PATCH 2/3] 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(+) create mode 100644 changes/bug7139 diff --git a/changes/bug7139 b/changes/bug7139 new file mode 100644 index 0000000000..dfb7d32838 --- /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 c6316120f9..fc0bcb9524 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 From d3bfdd6108d084c77f60b16319f6ef24ac447373 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Tue, 11 Sep 2012 13:00:05 -0700 Subject: [PATCH 3/3] Don't serve or accept v2 HS descs over a DirPort (changes file tweaked by nickm) --- changes/dirserv-BUGGY-a | 7 +++++++ src/or/directory.c | 2 ++ 2 files changed, 9 insertions(+) create mode 100644 changes/dirserv-BUGGY-a diff --git a/changes/dirserv-BUGGY-a b/changes/dirserv-BUGGY-a new file mode 100644 index 0000000000..35b492a2d7 --- /dev/null +++ b/changes/dirserv-BUGGY-a @@ -0,0 +1,7 @@ + o Minor bugfixes: + + - Don't serve or accept v2 hidden service descriptors over a + relay's DirPort. It's never correct to do so, and disabling it + might make it more annoying to exploit any bugs that turn up in the + descriptor-parsing code. Fixes bug 7149. + diff --git a/src/or/directory.c b/src/or/directory.c index f1510b970a..f235bf3b41 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3178,6 +3178,7 @@ directory_handle_command_get(dir_connection_t *conn, const char *headers, } if (options->HidServDirectoryV2 && + connection_dir_is_encrypted(conn) && !strcmpstart(url,"/tor/rendezvous2/")) { /* Handle v2 rendezvous descriptor fetch request. */ const char *descp; @@ -3364,6 +3365,7 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers, /* Handle v2 rendezvous service publish request. */ if (options->HidServDirectoryV2 && + connection_dir_is_encrypted(conn) && !strcmpstart(url,"/tor/rendezvous2/publish")) { switch (rend_cache_store_v2_desc_as_dir(body)) { case -2: