From db1709dcf4ddad38aae2b1901f87b70f14e21238 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 13 Mar 2025 11:37:15 -0400 Subject: [PATCH] Remove support for "tlssecrets" exporting Before we could rely on RFC5705 key material exporters, we did a fairly hinky thing involving the client random, the server random, and the master secret. These fields are all opaque in sensible TLS libraries, and the master secret is quite sensitive. Therefore, we're removing them. Some code still refers to them, but it does so behind a `define(HAVE_WORKING_TOR_TLS_GET_TLSSECRETS)` check, which macro is now never defined. Part of #41020. --- configure.ac | 3 - src/feature/relay/relay_handshake.c | 10 +-- src/lib/tls/tortls.h | 5 -- src/lib/tls/tortls_internal.h | 6 -- src/lib/tls/tortls_nss.c | 11 --- src/lib/tls/tortls_openssl.c | 131 ---------------------------- src/test/test_link_handshake.c | 10 --- 7 files changed, 4 insertions(+), 172 deletions(-) diff --git a/configure.ac b/configure.ac index 9a8ba52dd1..043692de32 100644 --- a/configure.ac +++ b/configure.ac @@ -1122,10 +1122,7 @@ AC_CHECK_FUNCS([ \ SSL_CIPHER_find \ SSL_CTX_set1_groups_list \ SSL_CTX_set_security_level \ - SSL_SESSION_get_master_key \ SSL_get_client_ciphers \ - SSL_get_client_random \ - SSL_get_server_random \ TLS_method \ ]) diff --git a/src/feature/relay/relay_handshake.c b/src/feature/relay/relay_handshake.c index 75546cdd90..a9f941cf02 100644 --- a/src/feature/relay/relay_handshake.c +++ b/src/feature/relay/relay_handshake.c @@ -410,12 +410,10 @@ connection_or_compute_authenticate_cell_body(or_connection_t *conn, /* HMAC of clientrandom and serverrandom using master key : 32 octets */ if (old_tlssecrets_algorithm) { - if (tor_tls_get_tlssecrets(conn->tls, auth->tlssecrets) < 0) { - log_fn(LOG_PROTOCOL_WARN, LD_OR, "Somebody asked us for an older TLS " - "authentication method (AUTHTYPE_RSA_SHA256_TLSSECRET) " - "which we don't support."); - goto err; - } + log_fn(LOG_PROTOCOL_WARN, LD_OR, "Somebody asked us for an obsolete TLS " + "authentication method (AUTHTYPE_RSA_SHA256_TLSSECRET) " + "which we don't support."); + goto err; } else { char label[128]; tor_snprintf(label, sizeof(label), diff --git a/src/lib/tls/tortls.h b/src/lib/tls/tortls.h index d65882d146..a2a81a65c7 100644 --- a/src/lib/tls/tortls.h +++ b/src/lib/tls/tortls.h @@ -126,11 +126,6 @@ int tor_tls_get_num_server_handshakes(tor_tls_t *tls); int tor_tls_server_got_renegotiate(tor_tls_t *tls); MOCK_DECL(int,tor_tls_cert_matches_key,(const tor_tls_t *tls, const struct tor_x509_cert_t *cert)); -MOCK_DECL(int,tor_tls_get_tlssecrets,(tor_tls_t *tls, uint8_t *secrets_out)); -#ifdef ENABLE_OPENSSL -/* OpenSSL lets us see these master secrets; NSS sensibly does not. */ -#define HAVE_WORKING_TOR_TLS_GET_TLSSECRETS -#endif MOCK_DECL(int,tor_tls_export_key_material,( tor_tls_t *tls, uint8_t *secrets_out, const uint8_t *context, diff --git a/src/lib/tls/tortls_internal.h b/src/lib/tls/tortls_internal.h index a4c6b87cac..b83c03b8e8 100644 --- a/src/lib/tls/tortls_internal.h +++ b/src/lib/tls/tortls_internal.h @@ -51,12 +51,6 @@ void tor_tls_server_info_callback(const struct ssl_st *ssl, int type, int val); void tor_tls_allocate_tor_tls_object_ex_data_index(void); -#if !defined(HAVE_SSL_SESSION_GET_MASTER_KEY) -size_t SSL_SESSION_get_master_key(struct ssl_session_st *s, - uint8_t *out, - size_t len); -#endif - #ifdef TORTLS_OPENSSL_PRIVATE int always_accept_verify_cb(int preverify_ok, X509_STORE_CTX *x509_ctx); int tor_tls_classify_client_ciphers(const struct ssl_st *ssl, diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index 392747e930..7a31679e56 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -788,17 +788,6 @@ tor_tls_cert_matches_key,(const tor_tls_t *tls, return rv; } -MOCK_IMPL(int, -tor_tls_get_tlssecrets,(tor_tls_t *tls, uint8_t *secrets_out)) -{ - tor_assert(tls); - tor_assert(secrets_out); - - /* There's no way to get this information out of NSS. */ - - return -1; -} - MOCK_IMPL(int, tor_tls_export_key_material,(tor_tls_t *tls, uint8_t *secrets_out, const uint8_t *context, diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index ee91715e2d..dc578a35d0 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -1589,137 +1589,6 @@ 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 /* !defined(HAVE_SSL_GET_CLIENT_RANDOM) */ - -#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 /* !defined(HAVE_SSL_GET_SERVER_RANDOM) */ - -#ifndef HAVE_SSL_SESSION_GET_MASTER_KEY -size_t -SSL_SESSION_get_master_key(SSL_SESSION *s, uint8_t *out, size_t len) -{ - tor_assert(s); - if (len == 0) - return s->master_key_length; - tor_assert(len == (size_t)s->master_key_length); - tor_assert(out); - memcpy(out, s->master_key, len); - return len; -} -#endif /* !defined(HAVE_SSL_SESSION_GET_MASTER_KEY) */ - -/** 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. - */ -MOCK_IMPL(int, -tor_tls_get_tlssecrets,(tor_tls_t *tls, uint8_t *secrets_out)) -{ -#define TLSSECRET_MAGIC "Tor V3 handshake TLS cross-certification" - uint8_t buf[128]; - size_t len; - tor_assert(tls); - - 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); - - if (BUG(! server_random_len)) { - log_warn(LD_NET, "Missing server randomness after handshake " - "using %s (cipher: %s, server: %s) from %s", - SSL_get_version(ssl), - SSL_get_cipher_name(ssl), - tls->isServer ? "true" : "false", - ADDR(tls)); - return -1; - } - - if (BUG(! client_random_len)) { - log_warn(LD_NET, "Missing client randomness after handshake " - "using %s (cipher: %s, server: %s) from %s", - SSL_get_version(ssl), - SSL_get_cipher_name(ssl), - tls->isServer ? "true" : "false", - ADDR(tls)); - return -1; - } - - if (BUG(! master_key_len)) { - log_warn(LD_NET, "Missing master key after handshake " - "using %s (cipher: %s, server: %s) from %s", - SSL_get_version(ssl), - SSL_get_cipher_name(ssl), - tls->isServer ? "true" : "false", - ADDR(tls)); - return -1; - } - - 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 - */ - crypto_hmac_sha256((char*)secrets_out, - (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; -} - /** Using the RFC5705 key material exporting construction, and the * provided context (context_len bytes long) and * label (a NUL-terminated string), compute a 32-byte secret in diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index 317f7b0b77..ff7809956d 100644 --- a/src/test/test_link_handshake.c +++ b/src/test/test_link_handshake.c @@ -1154,14 +1154,6 @@ AUTHCHALLENGE_FAIL(nonzero_circid, require_failure_message = "It had a nonzero circuit ID"; d->cell->circ_id = 1337) -static int -mock_get_tlssecrets(tor_tls_t *tls, uint8_t *secrets_out) -{ - (void)tls; - memcpy(secrets_out, "int getRandomNumber(){return 4;}", 32); - return 0; -} - static void mock_set_circid_type(channel_t *chan, crypto_pk_t *identity_rcvd, @@ -1186,7 +1178,6 @@ authenticate_data_cleanup(const struct testcase_t *test, void *arg) UNMOCK(connection_or_write_var_cell_to_buf); UNMOCK(tor_tls_get_peer_cert); UNMOCK(tor_tls_get_own_cert); - UNMOCK(tor_tls_get_tlssecrets); UNMOCK(connection_or_close_for_error); UNMOCK(channel_set_circid_type); UNMOCK(tor_tls_export_key_material); @@ -1223,7 +1214,6 @@ authenticate_data_setup(const struct testcase_t *test) MOCK(connection_or_write_var_cell_to_buf, mock_write_var_cell); MOCK(tor_tls_get_peer_cert, mock_get_peer_cert); MOCK(tor_tls_get_own_cert, mock_get_own_cert); - MOCK(tor_tls_get_tlssecrets, mock_get_tlssecrets); MOCK(connection_or_close_for_error, mock_close_for_err); MOCK(channel_set_circid_type, mock_set_circid_type); MOCK(tor_tls_export_key_material, mock_export_key_material); -- 2.47.2