From: Stefan Eissing Date: Wed, 28 Dec 2022 08:58:09 +0000 (+0100) Subject: openssl: remove attached easy handles from SSL instances X-Git-Tag: curl-7_88_0~209 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f39472ea9f4f4e12cfbc0500c4580a8d52ce4a59;p=thirdparty%2Fcurl.git openssl: remove attached easy handles from SSL instances - keeping the "current" easy handle registered at SSL* is no longer necessary, since the "calling" data object is already stored in the cfilter's context (and used by other SSL backends from there). - The "detach" of an easy handle that goes out of scope is then avoided. - using SSL_set0_wbio for clear reference counting where available. Closes #10151 --- diff --git a/configure.ac b/configure.ac index d4fc183ed9..479336f3c9 100644 --- a/configure.ac +++ b/configure.ac @@ -4217,6 +4217,13 @@ if test "x$want_ech" != "xno"; then fi fi +dnl ************************************************************* +dnl check whether OpenSSL (lookalikes) have SSL_set0_wbio +dnl +if test "x$OPENSSL_ENABLED" = "x1"; then + AC_CHECK_FUNCS([SSL_set0_wbio]) +fi + dnl ************************************************************* dnl WebSockets dnl diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index e7a1caabf7..f7ffb1e0ab 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -277,8 +277,6 @@ #endif /* !LIBRESSL_VERSION_NUMBER */ struct ssl_backend_data { - struct Curl_easy *logger; /* transfer handle to pass trace logs to, only - using sockindex 0 */ /* these ones requires specific SSL-types */ SSL_CTX* ctx; SSL* handle; @@ -788,9 +786,6 @@ static void bio_cf_free_methods(void) #endif -static bool ossl_attach_data(struct Curl_cfilter *cf, - struct Curl_easy *data); - /* * 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 @@ -922,18 +917,6 @@ static char *ossl_strerror(unsigned long error, char *buf, size_t size) return buf; } -/* Return an extra data index for the transfer data. - * This index can be used with SSL_get_ex_data() and SSL_set_ex_data(). - */ -static int ossl_get_ssl_data_index(void) -{ - static int ssl_ex_data_data_index = -1; - if(ssl_ex_data_data_index < 0) { - ssl_ex_data_data_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); - } - return ssl_ex_data_data_index; -} - /* Return an extra data index for the associated Curl_cfilter instance. * This index can be used with SSL_get_ex_data() and SSL_set_ex_data(). */ @@ -946,28 +929,20 @@ static int ossl_get_ssl_cf_index(void) return ssl_ex_data_cf_index; } -/* Return an extra data index for the sockindex. - * This index can be used with SSL_get_ex_data() and SSL_set_ex_data(). - */ -static int ossl_get_ssl_sockindex_index(void) +static bool ossl_attach_cf(struct Curl_cfilter *cf) { - static int sockindex_index = -1; - if(sockindex_index < 0) { - sockindex_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); - } - return sockindex_index; -} + struct ssl_connect_data *connssl = cf->ctx; + struct ssl_backend_data *backend = connssl->backend; + int cf_idx = ossl_get_ssl_cf_index(); -/* Return an extra data index for proxy boolean. - * This index can be used with SSL_get_ex_data() and SSL_set_ex_data(). - */ -static int ossl_get_proxy_index(void) -{ - static int proxy_index = -1; - if(proxy_index < 0) { - proxy_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, NULL); + DEBUGASSERT(backend); + + /* If we don't have SSL context, do nothing. */ + if(backend->handle && cf_idx >= 0) { + if(SSL_set_ex_data(backend->handle, cf_idx, cf)) + return TRUE; } - return proxy_index; + return FALSE; } static int passwd_callback(char *buf, int num, int encrypting, @@ -1772,8 +1747,7 @@ static int ossl_init(void) Curl_tls_keylog_open(); /* Initialize the extra data indexes */ - if(ossl_get_ssl_data_index() < 0 || ossl_get_ssl_cf_index() < 0 || - ossl_get_ssl_sockindex_index() < 0 || ossl_get_proxy_index() < 0) + if(ossl_get_ssl_cf_index() < 0) return 0; return 1; @@ -1960,19 +1934,15 @@ static struct curl_slist *ossl_engines_list(struct Curl_easy *data) return list; } -#define set_logger(connssl, data) \ - connssl->backend->logger = data - static void ossl_close(struct Curl_cfilter *cf, struct Curl_easy *data) { struct ssl_connect_data *connssl = cf->ctx; struct ssl_backend_data *backend = connssl->backend; + (void)data; DEBUGASSERT(backend); if(backend->handle) { - set_logger(connssl, data); - if(cf->next && cf->next->connected) { char buf[32]; /* Maybe the server has already sent a close notify alert. @@ -2674,7 +2644,7 @@ static void ossl_trace(int direction, int ssl_ver, int content_type, connssl = cf->ctx; DEBUGASSERT(connssl); DEBUGASSERT(connssl->backend); - data = connssl->backend->logger; + data = connssl->call_data; if(!conn || !data || !data->set.fdebug || (direction != 0 && direction != 1)) @@ -2967,21 +2937,17 @@ static int ossl_new_session_cb(SSL *ssl, SSL_SESSION *ssl_sessionid) struct Curl_easy *data; struct Curl_cfilter *cf; const struct ssl_config_data *config; - curl_socket_t *sockindex_ptr; - int data_idx = ossl_get_ssl_data_index(); int cf_idx = ossl_get_ssl_cf_index(); - int sockindex_idx = ossl_get_ssl_sockindex_index(); - int proxy_idx = ossl_get_proxy_index(); + struct ssl_connect_data *connssl; bool isproxy; - if(data_idx < 0 || cf_idx < 0 || sockindex_idx < 0 || proxy_idx < 0) + if(cf_idx < 0) return 0; - cf = (struct Curl_cfilter*) SSL_get_ex_data(ssl, cf_idx); - data = (struct Curl_easy *) SSL_get_ex_data(ssl, data_idx); + connssl = cf? cf->ctx : NULL; + data = connssl? connssl->call_data : NULL; /* The sockindex has been stored as a pointer to an array element */ - sockindex_ptr = (curl_socket_t*) SSL_get_ex_data(ssl, sockindex_idx); - if(!cf || !data || !sockindex_ptr) + if(!cf || !data) return 0; isproxy = Curl_ssl_cf_is_proxy(cf); @@ -3565,7 +3531,6 @@ static CURLcode ossl_connect_step1(struct Curl_cfilter *cf, /* the SSL trace callback is only used for verbose logging */ SSL_CTX_set_msg_callback(backend->ctx, ossl_trace); SSL_CTX_set_msg_callback_arg(backend->ctx, cf->conn); - set_logger(connssl, data); } #endif @@ -3847,9 +3812,9 @@ static CURLcode ossl_connect_step1(struct Curl_cfilter *cf, } #endif - if(!ossl_attach_data(cf, data)) { + if(!ossl_attach_cf(cf)) { /* Maybe the internal errors of SSL_get_ex_new_index or SSL_set_ex_data */ - failf(data, "SSL: ossl_attach_data failed: %s", + failf(data, "SSL: ossl_attach_cf failed: %s", ossl_strerror(ERR_get_error(), error_buffer, sizeof(error_buffer))); return CURLE_SSL_CONNECT_ERROR; @@ -3877,8 +3842,18 @@ static CURLcode ossl_connect_step1(struct Curl_cfilter *cf, return CURLE_OUT_OF_MEMORY; BIO_set_data(bio, cf); +#ifdef HAVE_SSL_SET0_WBIO + /* with OpenSSL v1.1.1 we get an alternative to SSL_set_bio() that works + * without backward compat quirks. Every call takes one reference, so we + * up it and pass. SSL* then owns it and will free. + * We check on the function in configure, since libressl and friends + * each have their own versions to add support for this. */ + BIO_up_ref(bio); + SSL_set0_rbio(backend->handle, bio); + SSL_set0_wbio(backend->handle, bio); +#else SSL_set_bio(backend->handle, bio, bio); - +#endif connssl->connecting_state = ssl_connect_2; return CURLE_OK; @@ -4523,7 +4498,6 @@ static ssize_t ossl_send(struct Curl_cfilter *cf, ERR_clear_error(); memlen = (len > (size_t)INT_MAX) ? INT_MAX : (int)len; - set_logger(connssl, data); rc = SSL_write(backend->handle, mem, memlen); if(rc <= 0) { @@ -4620,7 +4594,6 @@ static ssize_t ossl_recv(struct Curl_cfilter *cf, ERR_clear_error(); buffsize = (buffersize > (size_t)INT_MAX) ? INT_MAX : (int)buffersize; - set_logger(connssl, data); nread = (ssize_t)SSL_read(backend->handle, buf, buffsize); if(nread <= 0) { @@ -4838,89 +4811,6 @@ static void *ossl_get_internals(struct ssl_connect_data *connssl, (void *)backend->ctx : (void *)backend->handle; } -static bool ossl_attach_data(struct Curl_cfilter *cf, - struct Curl_easy *data) -{ - struct ssl_connect_data *connssl = cf->ctx; - struct ssl_backend_data *backend = connssl->backend; - const struct ssl_config_data *config; - - DEBUGASSERT(backend); - - /* If we don't have SSL context, do nothing. */ - if(!backend->handle) - return FALSE; - - config = Curl_ssl_cf_get_config(cf, data); - if(config->primary.sessionid) { - int data_idx = ossl_get_ssl_data_index(); - int cf_idx = ossl_get_ssl_cf_index(); - int sockindex_idx = ossl_get_ssl_sockindex_index(); - int proxy_idx = ossl_get_proxy_index(); - - if(data_idx >= 0 && cf_idx >= 0 && sockindex_idx >= 0 && - proxy_idx >= 0) { - int data_status, cf_status, sockindex_status, proxy_status; - - /* Store the data needed for the "new session" callback. - * The sockindex is stored as a pointer to an array element. */ - data_status = SSL_set_ex_data(backend->handle, data_idx, data); - cf_status = SSL_set_ex_data(backend->handle, cf_idx, cf); - sockindex_status = SSL_set_ex_data(backend->handle, sockindex_idx, - cf->conn->sock + cf->sockindex); -#ifndef CURL_DISABLE_PROXY - proxy_status = SSL_set_ex_data(backend->handle, proxy_idx, - Curl_ssl_cf_is_proxy(cf)? - (void *) 1 : NULL); -#else - proxy_status = SSL_set_ex_data(backend->handle, proxy_idx, NULL); -#endif - if(data_status && cf_status && sockindex_status && proxy_status) - return TRUE; - } - return FALSE; - } - return TRUE; -} - -/* - * 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_detach_data(struct Curl_cfilter *cf, - struct Curl_easy *data) -{ - struct ssl_config_data *ssl_config = Curl_ssl_cf_get_config(cf, data); - struct ssl_connect_data *connssl = cf->ctx; - struct ssl_backend_data *backend = connssl->backend; - DEBUGASSERT(backend); - - /* If we don't have SSL context, do nothing. */ - if(!backend->handle) - return; - - if(ssl_config->primary.sessionid) { - int data_idx = ossl_get_ssl_data_index(); - int cf_idx = ossl_get_ssl_cf_index(); - int sockindex_idx = ossl_get_ssl_sockindex_index(); - int proxy_idx = ossl_get_proxy_index(); - - if(data_idx >= 0 && cf_idx >= 0 && sockindex_idx >= 0 && - proxy_idx >= 0) { - /* 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, cf_idx, NULL); - SSL_set_ex_data(backend->handle, sockindex_idx, NULL); - SSL_set_ex_data(backend->handle, proxy_idx, NULL); - } - } -} - static void ossl_free_multi_ssl_backend_data( struct multi_ssl_backend_data *mbackend) { @@ -4974,8 +4864,8 @@ const struct Curl_ssl Curl_ssl_openssl = { #else NULL, /* sha256sum */ #endif - ossl_attach_data, /* use of data in this connection */ - ossl_detach_data, /* remote of data from this connection */ + NULL, /* use of data in this connection */ + NULL, /* remote of data from this connection */ ossl_free_multi_ssl_backend_data, /* free_multi_ssl_backend_data */ ossl_recv, /* recv decrypted data */ ossl_send, /* send data to encrypt */