From 44259b89423fd6b26b451eacfec85a2463a7f99d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 22 May 2015 10:22:11 -0400 Subject: [PATCH 1/5] Revert "Try using SSL_get_ciphers in place of session->ciphers" This reverts commit 67964cfa787461bc56380fe46439fd5c9863bb4f. It was the cause of #16153, and was not in any released Tor. We need a better solution for getting session->ciphers. --- src/common/tortls.c | 36 +++++------------------------------- 1 file changed, 5 insertions(+), 31 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index e0265b4939..654efb50e4 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1683,39 +1683,13 @@ tor_tls_classify_client_ciphers(const SSL *ssl, static int tor_tls_client_is_using_v2_ciphers(const SSL *ssl) { - STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl); - -#if OPENSSL_VERSION_NUMBER < OPENSSL_V_SERIES(1,1,0) - { - SSL_SESSION *session; - STACK_OF(SSL_CIPHER) *c1; - int i; - if (!(session = SSL_get_session((SSL *)ssl))) { - log_info(LD_NET, "No session on TLS?"); - return CIPHERS_ERR; - } - c1 = session->ciphers; - - if (sk_SSL_CIPHER_num(c1) != sk_SSL_CIPHER_num(ciphers)) { - log_warn(LD_BUG, "Whoops. session->ciphers doesn't " - "match SSL_get_ciphers()"); - return 0; - } - for (i = 0; i < sk_SSL_CIPHER_num(c1); ++i) { - SSL_CIPHER *a = sk_SSL_CIPHER_value(ciphers, i); - SSL_CIPHER *b = sk_SSL_CIPHER_value(c1, i); - unsigned long a_id = SSL_CIPHER_get_id(a); - unsigned long b_id = SSL_CIPHER_get_id(b); - if (a_id != b_id) { - log_warn(LD_BUG, "Cipher mismatch between session->ciphers and " - "SSL_get_ciphers() at %d: %lx vs %lx", i, - a_id, b_id); - } - } + SSL_SESSION *session; + if (!(session = SSL_get_session((SSL *)ssl))) { + log_info(LD_NET, "No session on TLS?"); + return CIPHERS_ERR; } -#endif - return tor_tls_classify_client_ciphers(ssl, ciphers) >= CIPHERS_V2; + return tor_tls_classify_client_ciphers(ssl, session->ciphers) >= CIPHERS_V2; } /** Invoked when we're accepting a connection on ssl, and the connection From 80082b7185feb77f83ff484e1779438aa0396634 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 26 May 2015 10:56:54 -0400 Subject: [PATCH 2/5] Remove rectify_client_ciphers as needless. We previously used this function instead of SSL_set_cipher_list() to set up a stack of client SSL_CIPHERs for these reasons: A) In order to force a particular order of the results. B) In order to be able to include dummy entries for ciphers that this build of openssl did not support, so we could impersonate Firefox harder. But we no longer do B, since we merged proposal 198 and stopped lying about what ciphers we know. And A was actually pointless, since I had misread the implementation of SSL_set_cipher_list(). It _does_ do some internal sorting, but that is pre-sorting on the master list of ciphers, not sorting on the user's preferred order. --- src/common/tortls.c | 184 -------------------------------------------- 1 file changed, 184 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index 654efb50e4..01bccd7a53 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -217,16 +217,6 @@ struct tor_tls_t { void *callback_arg; }; -#ifdef V2_HANDSHAKE_CLIENT -/** An array of fake SSL_CIPHER objects that we use in order to trick OpenSSL - * in client mode into advertising the ciphers we want. See - * rectify_client_ciphers() for details. */ -static SSL_CIPHER *CLIENT_CIPHER_DUMMIES = NULL; -/** A stack of SSL_CIPHER objects, some real, some fake. - * See rectify_client_ciphers() for details. */ -static STACK_OF(SSL_CIPHER) *CLIENT_CIPHER_STACK = NULL; -#endif - /** The ex_data index in which we store a pointer to an SSL object's * corresponding tor_tls_t object. */ static int tor_tls_object_ex_data_index = -1; @@ -595,12 +585,6 @@ tor_tls_free_all(void) client_tls_context = NULL; tor_tls_context_decref(ctx); } -#ifdef V2_HANDSHAKE_CLIENT - if (CLIENT_CIPHER_DUMMIES) - tor_free(CLIENT_CIPHER_DUMMIES); - if (CLIENT_CIPHER_STACK) - sk_SSL_CIPHER_free(CLIENT_CIPHER_STACK); -#endif } /** We need to give OpenSSL a callback to verify certificates. This is @@ -790,7 +774,6 @@ const char UNRESTRICTED_SERVER_CIPHER_LIST[] = * (SSL3_TXT_RSA_NULL_SHA). If you do this, you won't be able to communicate * with any of the "real" Tors, though. */ -#ifdef V2_HANDSHAKE_CLIENT #define CIPHER(id, name) name ":" #define XCIPHER(id, name) /** List of ciphers that clients should advertise, omitting items that @@ -804,28 +787,6 @@ static const char CLIENT_CIPHER_LIST[] = #undef CIPHER #undef XCIPHER -/** Holds a cipher that we want to advertise, and its 2-byte ID. */ -typedef struct cipher_info_t { unsigned id; const char *name; } cipher_info_t; -/** A list of all the ciphers that clients should advertise, including items - * that OpenSSL might not know about. */ -static const cipher_info_t CLIENT_CIPHER_INFO_LIST[] = { -#define CIPHER(id, name) { id, name }, -#define XCIPHER(id, name) { id, #name }, -#include "./ciphers.inc" -#undef CIPHER -#undef XCIPHER -}; - -/** The length of CLIENT_CIPHER_INFO_LIST and CLIENT_CIPHER_DUMMIES. */ -static const int N_CLIENT_CIPHERS = ARRAY_LENGTH(CLIENT_CIPHER_INFO_LIST); -#endif - -#ifndef V2_HANDSHAKE_CLIENT -#undef CLIENT_CIPHER_LIST -#define CLIENT_CIPHER_LIST (TLS1_TXT_DHE_RSA_WITH_AES_128_SHA ":" \ - SSL3_TXT_EDH_RSA_DES_192_CBC3_SHA) -#endif - /** Free all storage held in cert */ void tor_cert_free(tor_cert_t *cert) @@ -1796,149 +1757,6 @@ tor_tls_setup_session_secret_cb(tor_tls_t *tls) #define tor_tls_setup_session_secret_cb(tls) STMT_NIL #endif -/** Explain which ciphers we're missing. */ -static void -log_unsupported_ciphers(smartlist_t *unsupported) -{ - char *joined; - - log_notice(LD_NET, "We weren't able to find support for all of the " - "TLS ciphersuites that we wanted to advertise. This won't " - "hurt security, but it might make your Tor (if run as a client) " - "more easy for censors to block."); - - if (SSLeay() < 0x10000000L) { - log_notice(LD_NET, "To correct this, use a more recent OpenSSL, " - "built without disabling any secure ciphers or features."); - } else { - log_notice(LD_NET, "To correct this, use a version of OpenSSL " - "built with none of its ciphers disabled."); - } - - joined = smartlist_join_strings(unsupported, ":", 0, NULL); - log_info(LD_NET, "The unsupported ciphers were: %s", joined); - tor_free(joined); -} - -static void -set_ssl_ciphers_to_list(SSL *ssl, STACK_OF(SSL_CIPHER) *stack) -{ - STACK_OF(SSL_CIPHER) *ciphers; - - int r, i; - /* #1: ensure that the ssl object has its own list of ciphers. Otherwise we - * might be about to stomp the SSL_CTX ciphers list. */ - r = SSL_set_cipher_list(ssl, "HIGH"); - tor_assert(r); - - /* #2: Grab ssl_ciphers and clear it. */ - ciphers = SSL_get_ciphers(ssl); - tor_assert(ciphers); - sk_SSL_CIPHER_zero(ciphers); - - /* #3: Copy the elements from stack. */ - for (i = 0; i < sk_SSL_CIPHER_num(stack); ++i) { - SSL_CIPHER *c = sk_SSL_CIPHER_value(stack, i); - sk_SSL_CIPHER_push(ciphers, c); - } -} - -/** Replace the ciphers on ssl with a new list of SSL ciphersuites: - * specifically, a list designed to mimic a common web browser. We might not - * be able to do that if OpenSSL doesn't support all the ciphers we want. - * Some of the ciphers in the list won't actually be implemented by OpenSSL: - * that's okay so long as the server doesn't select them. - * - * [If the server does select a bogus cipher, we won't crash or - * anything; we'll just fail later when we try to look up the cipher in - * ssl->cipher_list_by_id.] - */ -static void -rectify_client_ciphers(SSL *ssl) -{ -#ifdef V2_HANDSHAKE_CLIENT - if (PREDICT_UNLIKELY(!CLIENT_CIPHER_STACK)) { - STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(ssl); - - /* We need to set CLIENT_CIPHER_STACK to an array of the ciphers - * we want to use/advertise. */ - int i = 0, j = 0; - smartlist_t *unsupported = smartlist_new(); - - /* First, create a dummy SSL_CIPHER for every cipher. */ - CLIENT_CIPHER_DUMMIES = - tor_malloc_zero(sizeof(SSL_CIPHER)*N_CLIENT_CIPHERS); - for (i=0; i < N_CLIENT_CIPHERS; ++i) { - CLIENT_CIPHER_DUMMIES[i].valid = 1; - /* The "3<<24" here signifies that the cipher is supposed to work with - * SSL3 and TLS1. */ - CLIENT_CIPHER_DUMMIES[i].id = CLIENT_CIPHER_INFO_LIST[i].id | (3<<24); - CLIENT_CIPHER_DUMMIES[i].name = CLIENT_CIPHER_INFO_LIST[i].name; - } - - CLIENT_CIPHER_STACK = sk_SSL_CIPHER_new_null(); - tor_assert(CLIENT_CIPHER_STACK); - - log_debug(LD_NET, "List was: %s", CLIENT_CIPHER_LIST); - for (j = 0; j < sk_SSL_CIPHER_num(ciphers); ++j) { - SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, j); - log_debug(LD_NET, "Cipher %d: %lx %s", j, - SSL_CIPHER_get_id(cipher), SSL_CIPHER_get_name(cipher)); - } - - /* Then copy as many ciphers as we can from the good list, inserting - * dummies as needed. Let j be an index into list of ciphers we have - * (ciphers) and let i be an index into the ciphers we want - * (CLIENT_INFO_CIPHER_LIST). We are building a list of ciphers in - * CLIENT_CIPHER_STACK. - */ - for (i = j = 0; i < N_CLIENT_CIPHERS; ) { - SSL_CIPHER *cipher = NULL; - if (j < sk_SSL_CIPHER_num(ciphers)) - cipher = sk_SSL_CIPHER_value(ciphers, j); - if (cipher && ((SSL_CIPHER_get_id(cipher) >> 24) & 0xff) != 3) { - /* Skip over non-v3 ciphers entirely. (This should no longer be - * needed, thanks to saying !SSLv2 above.) */ - log_debug(LD_NET, "Skipping v%d cipher %s", - (int)((SSL_CIPHER_get_id(cipher)>>24) & 0xff), - SSL_CIPHER_get_name(cipher)); - ++j; - } else if (cipher && - (SSL_CIPHER_get_id(cipher) & 0xffff) == CLIENT_CIPHER_INFO_LIST[i].id) { - /* "cipher" is the cipher we expect. Put it on the list. */ - log_debug(LD_NET, "Found cipher %s", SSL_CIPHER_get_name(cipher)); - sk_SSL_CIPHER_push(CLIENT_CIPHER_STACK, cipher); - ++j; - ++i; - } else if (!strcmp(CLIENT_CIPHER_DUMMIES[i].name, - "SSL_RSA_FIPS_WITH_3DES_EDE_CBC_SHA")) { - /* We found bogus cipher 0xfeff, which OpenSSL doesn't support and - * never has. For this one, we need a dummy. */ - log_debug(LD_NET, "Inserting fake %s", CLIENT_CIPHER_DUMMIES[i].name); - sk_SSL_CIPHER_push(CLIENT_CIPHER_STACK, &CLIENT_CIPHER_DUMMIES[i]); - ++i; - } else { - /* OpenSSL doesn't have this one. */ - log_debug(LD_NET, "Completely omitting unsupported cipher %s", - CLIENT_CIPHER_INFO_LIST[i].name); - smartlist_add(unsupported, (char*) CLIENT_CIPHER_INFO_LIST[i].name); - ++i; - } - } - - if (smartlist_len(unsupported)) - log_unsupported_ciphers(unsupported); - - smartlist_free(unsupported); - } - - set_ssl_ciphers_to_list(ssl, CLIENT_CIPHER_STACK); - -#else - (void)ciphers; -#endif -} - /** Create a new TLS object from a file descriptor, and a flag to * determine whether it is functioning as a server. */ @@ -1978,8 +1796,6 @@ tor_tls_new(int sock, int isServer) tor_free(result); goto err; } - if (!isServer) - rectify_client_ciphers(result->ssl); result->socket = sock; bio = BIO_new_socket(sock, BIO_NOCLOSE); if (! bio) { From 95375963981bb2346429de86b0cbb558d6b399d5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 26 May 2015 11:05:36 -0400 Subject: [PATCH 3/5] Stop looking at session->ciphers when possible If the OpenSSL team accepts my patch to add an SSL_get_client_ciphers function, this patch will make Tor use it when available, thereby working better with openssl 1.1. --- configure.ac | 17 +++++++++++++++++ src/common/tortls.c | 8 +++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index cc271c83c3..ede8f8438c 100644 --- a/configure.ac +++ b/configure.ac @@ -623,10 +623,27 @@ else fi AC_SUBST(TOR_OPENSSL_LIBS) +dnl Now check for particular openssl functions. +save_LIBS="$LIBS" +save_LDFLAGS="$LDFLAGS" +save_CPPFLAGS="$CPPFLAGS" +LIBS="$TOR_OPENSSL_LIBS $LIBS" +LDFLAGS="$TOR_LDFLAGS_openssl $LDFLAGS" +CPPFLAGS="$TOR_CPPFLAGS_openssl $CPPFLAGS" AC_CHECK_MEMBERS([struct ssl_method_st.get_cipher_by_char], , , [#include ]) +AC_CHECK_FUNCS([ \ + SSL_SESSION_get_master_key \ + SSL_get_server_random \ + SSL_get_client_ciphers \ + SSL_get_client_random \ + ]) +LIBS="$save_LIBS" +LDFLAGS="$save_LDFLAGS" +CPPFLAGS="$save_CPPFLAGS" + dnl ------------------------------------------------------ dnl Where do you live, zlib? And how do we call you? diff --git a/src/common/tortls.c b/src/common/tortls.c index 01bccd7a53..d4a565c017 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1644,13 +1644,19 @@ tor_tls_classify_client_ciphers(const SSL *ssl, static int tor_tls_client_is_using_v2_ciphers(const SSL *ssl) { + STACK_OF(SSL_CIPHER) *ciphers; +#ifdef HAVE_SSL_GET_CLIENT_CIPHERS + ciphers = SSL_get_client_ciphers(ssl); +#else SSL_SESSION *session; if (!(session = SSL_get_session((SSL *)ssl))) { log_info(LD_NET, "No session on TLS?"); return CIPHERS_ERR; } + ciphers = session->ciphers; +#endif - return tor_tls_classify_client_ciphers(ssl, session->ciphers) >= CIPHERS_V2; + return tor_tls_classify_client_ciphers(ssl, ciphers) >= CIPHERS_V2; } /** Invoked when we're accepting a connection on ssl, and the connection From f90a704f12587ba7585a8a4d31cb6de756bb3868 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 26 May 2015 12:09:53 -0400 Subject: [PATCH 4/5] Use accessor functions for client_random/server_random/master_key If OpenSSL accepts my patch to introduce these functions, they'll be a way to help Tor work with OpenSSL 1.1. --- src/common/tortls.c | 94 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 83 insertions(+), 11 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index d4a565c017..d80cf42606 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -2707,6 +2707,46 @@ tor_tls_server_got_renegotiate(tor_tls_t *tls) return tls->got_renegotiate; } +#ifndef HAVE_SSL_GET_CLIENT_RANDOM +static size_t +SSL_get_client_random(SSL *s, uint8_t *out, size_t len) +{ + if (len == 0) + return SSL3_RANDOM_SIZE; + tor_assert(len == SSL3_RANDOM_SIZE); + tor_assert(s->s3); + memcpy(out, s->s3->client_random, len); + return len; +} +#endif + +#ifndef HAVE_SSL_GET_SERVER_RANDOM +static size_t +SSL_get_server_random(SSL *s, uint8_t *out, size_t len) +{ + if (len == 0) + return SSL3_RANDOM_SIZE; + tor_assert(len == SSL3_RANDOM_SIZE); + tor_assert(s->s3); + memcpy(out, s->s3->server_random, len); + return len; +} +#endif + +#ifndef HAVE_SSL_SESSION_GET_MASTER_KEY +static size_t +SSL_SESSION_get_master_key(SSL_SESSION *s, uint8_t *out, size_t len) +{ + if (len == 0) + return s->master_key_length; + tor_assert(len == (size_t)s->master_key_length); + tor_assert(s->master_key); + memcpy(out, s->master_key, len); + return len; +} +#endif + + /** Set the DIGEST256_LEN buffer at secrets_out to the value used in * the v3 handshake to prove that the client knows the TLS secrets for the * connection tls. Return 0 on success, -1 on failure. @@ -2715,25 +2755,57 @@ int tor_tls_get_tlssecrets(tor_tls_t *tls, uint8_t *secrets_out) { #define TLSSECRET_MAGIC "Tor V3 handshake TLS cross-certification" - char buf[128]; + uint8_t buf[128]; size_t len; + tor_assert(tls); - tor_assert(tls->ssl); - tor_assert(tls->ssl->s3); - tor_assert(tls->ssl->session); + + SSL *const ssl = tls->ssl; + SSL_SESSION *const session = SSL_get_session(ssl); + + tor_assert(ssl); + tor_assert(session); + + const size_t server_random_len = SSL_get_server_random(ssl, NULL, 0); + const size_t client_random_len = SSL_get_client_random(ssl, NULL, 0); + const size_t master_key_len = SSL_SESSION_get_master_key(session, NULL, 0); + + tor_assert(server_random_len); + tor_assert(client_random_len); + tor_assert(master_key_len); + + len = client_random_len + server_random_len + strlen(TLSSECRET_MAGIC) + 1; + tor_assert(len <= sizeof(buf)); + + { + size_t r = SSL_get_client_random(ssl, buf, client_random_len); + tor_assert(r == client_random_len); + } + { + size_t r = SSL_get_server_random(ssl, buf+client_random_len, server_random_len); + tor_assert(r == server_random_len); + } + uint8_t *master_key = tor_malloc_zero(master_key_len); + { + size_t r = SSL_SESSION_get_master_key(session, master_key, master_key_len); + tor_assert(r == master_key_len); + } + + uint8_t *nextbuf = buf + client_random_len + server_random_len; + memcpy(nextbuf, TLSSECRET_MAGIC, strlen(TLSSECRET_MAGIC) + 1); + /* The value is an HMAC, using the TLS master key as the HMAC key, of client_random | server_random | TLSSECRET_MAGIC */ - memcpy(buf + 0, tls->ssl->s3->client_random, 32); - memcpy(buf + 32, tls->ssl->s3->server_random, 32); - memcpy(buf + 64, TLSSECRET_MAGIC, strlen(TLSSECRET_MAGIC) + 1); - len = 64 + strlen(TLSSECRET_MAGIC) + 1; crypto_hmac_sha256((char*)secrets_out, - (char*)tls->ssl->session->master_key, - tls->ssl->session->master_key_length, - buf, len); + (char*)master_key, + master_key_len, + (char*)buf, len); memwipe(buf, 0, sizeof(buf)); + memwipe(master_key, 0, master_key_len); + tor_free(master_key); + return 0; } From ff835e23280c0f29ca9ab25bfc3069276c0b8ab6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 2 Jun 2015 13:27:55 -0400 Subject: [PATCH 5/5] Use autoconf, not OPENSSL_VERSION_NUMBER, to detect SSL_CIPHER_find Repairs build with libressl --- configure.ac | 1 + src/common/tortls.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ede8f8438c..ab96c20f3f 100644 --- a/configure.ac +++ b/configure.ac @@ -639,6 +639,7 @@ AC_CHECK_FUNCS([ \ SSL_get_server_random \ SSL_get_client_ciphers \ SSL_get_client_random \ + SSL_CIPHER_find \ ]) LIBS="$save_LIBS" LDFLAGS="$save_LDFLAGS" diff --git a/src/common/tortls.c b/src/common/tortls.c index d80cf42606..deeee5f052 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -1487,7 +1487,7 @@ static int find_cipher_by_id(const SSL *ssl, const SSL_METHOD *m, uint16_t cipher) { const SSL_CIPHER *c; -#if OPENSSL_VERSION_NUMBER >= OPENSSL_V_SERIES(1,0,2) +#ifdef HAVE_SSL_CIPHER_FIND { unsigned char cipherid[3]; tor_assert(ssl);