From 7f4a9a9b2a49547eae24d2e19bc5c346e9026479 Mon Sep 17 00:00:00 2001 From: Harry Sintonen Date: Wed, 5 May 2021 13:42:26 +0200 Subject: [PATCH] openssl: associate/detach the transfer from connection CVE-2021-22901 Bug: https://curl.se/docs/CVE-2021-22901.html --- lib/multi.c | 5 +- lib/vtls/gskit.c | 4 +- lib/vtls/gtls.c | 4 +- lib/vtls/mbedtls.c | 4 +- lib/vtls/mesalink.c | 4 +- lib/vtls/nss.c | 4 +- lib/vtls/openssl.c | 146 +++++++++++++++++++++++++++++++------------ lib/vtls/rustls.c | 4 +- lib/vtls/schannel.c | 6 +- lib/vtls/sectransp.c | 2 + lib/vtls/vtls.c | 23 ++++++- lib/vtls/vtls.h | 12 ++++ lib/vtls/wolfssl.c | 4 +- 13 files changed, 172 insertions(+), 50 deletions(-) diff --git a/lib/multi.c b/lib/multi.c index 54365f399..1b3e261c6 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -878,8 +878,10 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi) void Curl_detach_connnection(struct Curl_easy *data) { struct connectdata *conn = data->conn; - if(conn) + if(conn) { Curl_llist_remove(&conn->easyq, &data->conn_queue, NULL); + Curl_ssl_detach_conn(data, conn); + } data->conn = NULL; } @@ -898,6 +900,7 @@ void Curl_attach_connnection(struct Curl_easy *data, &data->conn_queue); if(conn->handler->attach) conn->handler->attach(data, conn); + Curl_ssl_associate_conn(data, conn); } static int waitconnect_getsock(struct connectdata *conn, diff --git a/lib/vtls/gskit.c b/lib/vtls/gskit.c index c648f6245..ca953769d 100644 --- a/lib/vtls/gskit.c +++ b/lib/vtls/gskit.c @@ -1304,7 +1304,9 @@ const struct Curl_ssl Curl_ssl_gskit = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - NULL /* sha256sum */ + NULL, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif /* USE_GSKIT */ diff --git a/lib/vtls/gtls.c b/lib/vtls/gtls.c index a10c0dbcc..ecde5c44d 100644 --- a/lib/vtls/gtls.c +++ b/lib/vtls/gtls.c @@ -1656,7 +1656,9 @@ const struct Curl_ssl Curl_ssl_gnutls = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - gtls_sha256sum /* sha256sum */ + gtls_sha256sum, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif /* USE_GNUTLS */ diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c index ca77de586..3a0be0f04 100644 --- a/lib/vtls/mbedtls.c +++ b/lib/vtls/mbedtls.c @@ -1093,7 +1093,9 @@ const struct Curl_ssl Curl_ssl_mbedtls = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - mbedtls_sha256sum /* sha256sum */ + mbedtls_sha256sum, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif /* USE_MBEDTLS */ diff --git a/lib/vtls/mesalink.c b/lib/vtls/mesalink.c index f16c77c27..bf8600d32 100644 --- a/lib/vtls/mesalink.c +++ b/lib/vtls/mesalink.c @@ -666,7 +666,9 @@ const struct Curl_ssl Curl_ssl_mesalink = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - NULL /* sha256sum */ + NULL, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif diff --git a/lib/vtls/nss.c b/lib/vtls/nss.c index 2aa4bdaa1..1582b1e58 100644 --- a/lib/vtls/nss.c +++ b/lib/vtls/nss.c @@ -2465,7 +2465,9 @@ const struct Curl_ssl Curl_ssl_nss = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ nss_false_start, /* false_start */ - nss_sha256sum /* sha256sum */ + nss_sha256sum, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif /* USE_NSS */ diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index 1521600dd..ebd7abc3b 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -240,6 +240,10 @@ struct ssl_backend_data { #endif }; +static void ossl_associate_connection(struct Curl_easy *data, + struct connectdata *conn, + int sockindex); + /* * Number of bytes to read from the random number seed file. This must be * a finite value (because some entropy "files" like /dev/urandom have @@ -2581,6 +2585,7 @@ static CURLcode ossl_connect_step1(struct Curl_easy *data, curl_socket_t sockfd = conn->sock[sockindex]; struct ssl_connect_data *connssl = &conn->ssl[sockindex]; ctx_option_t ctx_options = 0; + void *ssl_sessionid = NULL; #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME bool sni; @@ -3225,46 +3230,23 @@ static CURLcode ossl_connect_step1(struct Curl_easy *data, } #endif - /* Check if there's a cached ID we can/should use here! */ - if(SSL_SET_OPTION(primary.sessionid)) { - void *ssl_sessionid = NULL; - int data_idx = ossl_get_ssl_data_index(); - int connectdata_idx = ossl_get_ssl_conn_index(); - int sockindex_idx = ossl_get_ssl_sockindex_index(); - int proxy_idx = ossl_get_proxy_index(); - - if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 && - proxy_idx >= 0) { - /* Store the data needed for the "new session" callback. - * The sockindex is stored as a pointer to an array element. */ - SSL_set_ex_data(backend->handle, data_idx, data); - SSL_set_ex_data(backend->handle, connectdata_idx, conn); - SSL_set_ex_data(backend->handle, sockindex_idx, conn->sock + sockindex); -#ifndef CURL_DISABLE_PROXY - SSL_set_ex_data(backend->handle, proxy_idx, SSL_IS_PROXY() ? (void *) 1: - NULL); -#else - SSL_set_ex_data(backend->handle, proxy_idx, NULL); -#endif + ossl_associate_connection(data, conn, sockindex); + Curl_ssl_sessionid_lock(data); + if(!Curl_ssl_getsessionid(data, conn, SSL_IS_PROXY() ? TRUE : FALSE, + &ssl_sessionid, NULL, sockindex)) { + /* we got a session id, use it! */ + if(!SSL_set_session(backend->handle, ssl_sessionid)) { + Curl_ssl_sessionid_unlock(data); + failf(data, "SSL: SSL_set_session failed: %s", + ossl_strerror(ERR_get_error(), error_buffer, + sizeof(error_buffer))); + return CURLE_SSL_CONNECT_ERROR; } - - Curl_ssl_sessionid_lock(data); - if(!Curl_ssl_getsessionid(data, conn, SSL_IS_PROXY() ? TRUE : FALSE, - &ssl_sessionid, NULL, sockindex)) { - /* we got a session id, use it! */ - if(!SSL_set_session(backend->handle, ssl_sessionid)) { - Curl_ssl_sessionid_unlock(data); - failf(data, "SSL: SSL_set_session failed: %s", - ossl_strerror(ERR_get_error(), error_buffer, - sizeof(error_buffer))); - return CURLE_SSL_CONNECT_ERROR; - } - /* Informational message */ - infof(data, "SSL re-using session ID\n"); - } - Curl_ssl_sessionid_unlock(data); + /* Informational message */ + infof(data, "SSL re-using session ID\n"); } + Curl_ssl_sessionid_unlock(data); #ifndef CURL_DISABLE_PROXY if(conn->proxy_ssl[sockindex].use) { @@ -4498,6 +4480,90 @@ static void *ossl_get_internals(struct ssl_connect_data *connssl, (void *)backend->ctx : (void *)backend->handle; } +static void ossl_associate_connection(struct Curl_easy *data, + struct connectdata *conn, + int sockindex) +{ + struct ssl_connect_data *connssl = &conn->ssl[sockindex]; + struct ssl_backend_data *backend = connssl->backend; + + /* If we don't have SSL context, do nothing. */ + if(!backend->handle) + return; + + if(SSL_SET_OPTION(primary.sessionid)) { + int data_idx = ossl_get_ssl_data_index(); + int connectdata_idx = ossl_get_ssl_conn_index(); + int sockindex_idx = ossl_get_ssl_sockindex_index(); + int proxy_idx = ossl_get_proxy_index(); + + if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 && + proxy_idx >= 0) { + /* Store the data needed for the "new session" callback. + * The sockindex is stored as a pointer to an array element. */ + SSL_set_ex_data(backend->handle, data_idx, data); + SSL_set_ex_data(backend->handle, connectdata_idx, conn); + SSL_set_ex_data(backend->handle, sockindex_idx, conn->sock + sockindex); +#ifndef CURL_DISABLE_PROXY + SSL_set_ex_data(backend->handle, proxy_idx, SSL_IS_PROXY() ? (void *) 1: + NULL); +#else + SSL_set_ex_data(backend->handle, proxy_idx, NULL); +#endif + } + } +} + +/* + * Starting with TLS 1.3, the ossl_new_session_cb callback gets called after + * the handshake. If the transfer that sets up the callback gets killed before + * this callback arrives, we must make sure to properly clear the data to + * avoid UAF problems. A future optimization could be to instead store another + * transfer that might still be using the same connection. + */ + +static void ossl_disassociate_connection(struct Curl_easy *data, + int sockindex) +{ + struct connectdata *conn = data->conn; + struct ssl_connect_data *connssl = &conn->ssl[sockindex]; + struct ssl_backend_data *backend = connssl->backend; + + /* If we don't have SSL context, do nothing. */ + if(!backend->handle) + return; + + if(SSL_SET_OPTION(primary.sessionid)) { + bool isproxy = FALSE; + bool incache; + void *old_ssl_sessionid = NULL; + int data_idx = ossl_get_ssl_data_index(); + int connectdata_idx = ossl_get_ssl_conn_index(); + int sockindex_idx = ossl_get_ssl_sockindex_index(); + int proxy_idx = ossl_get_proxy_index(); + + if(data_idx >= 0 && connectdata_idx >= 0 && sockindex_idx >= 0 && + proxy_idx >= 0) { + /* Invalidate the session cache entry, if any */ + isproxy = SSL_get_ex_data(backend->handle, proxy_idx) ? TRUE : FALSE; + + /* Disable references to data in "new session" callback to avoid + * accessing a stale pointer. */ + SSL_set_ex_data(backend->handle, data_idx, NULL); + SSL_set_ex_data(backend->handle, connectdata_idx, NULL); + SSL_set_ex_data(backend->handle, sockindex_idx, NULL); + SSL_set_ex_data(backend->handle, proxy_idx, NULL); + } + + Curl_ssl_sessionid_lock(data); + incache = !(Curl_ssl_getsessionid(data, conn, isproxy, + &old_ssl_sessionid, NULL, sockindex)); + if(incache) + Curl_ssl_delsessionid(data, old_ssl_sessionid); + Curl_ssl_sessionid_unlock(data); + } +} + const struct Curl_ssl Curl_ssl_openssl = { { CURLSSLBACKEND_OPENSSL, "openssl" }, /* info */ @@ -4533,10 +4599,12 @@ const struct Curl_ssl Curl_ssl_openssl = { ossl_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ #if (OPENSSL_VERSION_NUMBER >= 0x0090800fL) && !defined(OPENSSL_NO_SHA256) - ossl_sha256sum /* sha256sum */ + ossl_sha256sum, /* sha256sum */ #else - NULL /* sha256sum */ + NULL, /* sha256sum */ #endif + ossl_associate_connection, /* associate_connection */ + ossl_disassociate_connection /* disassociate_connection */ }; #endif /* USE_OPENSSL */ diff --git a/lib/vtls/rustls.c b/lib/vtls/rustls.c index 9dfbd2c3c..161f3bf51 100644 --- a/lib/vtls/rustls.c +++ b/lib/vtls/rustls.c @@ -604,7 +604,9 @@ const struct Curl_ssl Curl_ssl_rustls = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - NULL /* sha256sum */ + NULL, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif /* USE_RUSTLS */ diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c index dba707227..2bcf11db2 100644 --- a/lib/vtls/schannel.c +++ b/lib/vtls/schannel.c @@ -329,7 +329,7 @@ get_alg_id_by_name(char *name) static CURLcode set_ssl_ciphers(SCHANNEL_CRED *schannel_cred, char *ciphers, - int *algIds) + ALG_ID *algIds) { char *startCur = ciphers; int algCount = 0; @@ -2433,7 +2433,9 @@ const struct Curl_ssl Curl_ssl_schannel = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - schannel_sha256sum /* sha256sum */ + schannel_sha256sum, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif /* USE_SCHANNEL */ diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c index 4276b89cf..8b1e84ed7 100644 --- a/lib/vtls/sectransp.c +++ b/lib/vtls/sectransp.c @@ -3453,6 +3453,8 @@ const struct Curl_ssl Curl_ssl_sectransp = { Curl_none_engines_list, /* engines_list */ sectransp_false_start, /* false_start */ sectransp_sha256sum /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #ifdef __clang__ diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c index d63fd5c76..65f4f773d 100644 --- a/lib/vtls/vtls.c +++ b/lib/vtls/vtls.c @@ -586,6 +586,25 @@ CURLcode Curl_ssl_addsessionid(struct Curl_easy *data, return CURLE_OK; } +void Curl_ssl_associate_conn(struct Curl_easy *data, + struct connectdata *conn) +{ + if(Curl_ssl->associate_connection) { + Curl_ssl->associate_connection(data, conn, FIRSTSOCKET); + if(conn->sock[SECONDARYSOCKET] && conn->bits.sock_accepted) + Curl_ssl->associate_connection(data, conn, SECONDARYSOCKET); + } +} + +void Curl_ssl_detach_conn(struct Curl_easy *data, + struct connectdata *conn) +{ + if(Curl_ssl->disassociate_connection) { + Curl_ssl->disassociate_connection(data, FIRSTSOCKET); + if(conn->sock[SECONDARYSOCKET] && conn->bits.sock_accepted) + Curl_ssl->disassociate_connection(data, SECONDARYSOCKET); + } +} void Curl_ssl_close_all(struct Curl_easy *data) { @@ -1214,7 +1233,9 @@ static const struct Curl_ssl Curl_ssl_multi = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - NULL /* sha256sum */ + NULL, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; const struct Curl_ssl *Curl_ssl = diff --git a/lib/vtls/vtls.h b/lib/vtls/vtls.h index a22d526ca..7f93e7aed 100644 --- a/lib/vtls/vtls.h +++ b/lib/vtls/vtls.h @@ -84,6 +84,11 @@ struct Curl_ssl { bool (*false_start)(void); CURLcode (*sha256sum)(const unsigned char *input, size_t inputlen, unsigned char *sha256sum, size_t sha256sumlen); + + void (*associate_connection)(struct Curl_easy *data, + struct connectdata *conn, + int sockindex); + void (*disassociate_connection)(struct Curl_easy *data, int sockindex); }; #ifdef USE_SSL @@ -283,6 +288,11 @@ bool Curl_ssl_cert_status_request(void); bool Curl_ssl_false_start(void); +void Curl_ssl_associate_conn(struct Curl_easy *data, + struct connectdata *conn); +void Curl_ssl_detach_conn(struct Curl_easy *data, + struct connectdata *conn); + #define SSL_SHUTDOWN_TIMEOUT 10000 /* ms */ #else /* if not USE_SSL */ @@ -309,6 +319,8 @@ bool Curl_ssl_false_start(void); #define Curl_ssl_cert_status_request() FALSE #define Curl_ssl_false_start() FALSE #define Curl_ssl_tls13_ciphersuites() FALSE +#define Curl_ssl_associate_conn(a,b) Curl_nop_stmt +#define Curl_ssl_detach_conn(a,b) Curl_nop_stmt #endif #endif /* HEADER_CURL_VTLS_H */ diff --git a/lib/vtls/wolfssl.c b/lib/vtls/wolfssl.c index 02fcd2366..60e27e366 100644 --- a/lib/vtls/wolfssl.c +++ b/lib/vtls/wolfssl.c @@ -1125,7 +1125,9 @@ const struct Curl_ssl Curl_ssl_wolfssl = { Curl_none_set_engine_default, /* set_engine_default */ Curl_none_engines_list, /* engines_list */ Curl_none_false_start, /* false_start */ - wolfssl_sha256sum /* sha256sum */ + wolfssl_sha256sum, /* sha256sum */ + NULL, /* associate_connection */ + NULL /* disassociate_connection */ }; #endif