From 43140e0a18752038433487798555eefe3f73b635 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 17 Nov 2023 10:20:16 +0100 Subject: [PATCH] Use replace_ex_data more Setting callbacks multiple times on a SslContextBuilder causes the previous callback installed to leak, using replace_ex_data internally prevents that. We also start using it in tokio-boring in with_ex_data_future, my understanding is that the futures currently in use are never installed twice by that function but that could change in the future with the addition of more async callbacks. --- boring/src/ssl/mod.rs | 26 +++++++++++++------------- tokio-boring/src/async_callbacks.rs | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 90ad1ed8..c4d5df60 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -837,7 +837,7 @@ impl SslContextBuilder { assert!(!self.is_rpk, "This API is not supported for RPK"); unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), verify); + self.replace_ex_data(SslContext::cached_ex_index::(), verify); ffi::SSL_CTX_set_verify(self.as_ptr(), mode.bits() as c_int, Some(raw_verify::)); } } @@ -866,7 +866,7 @@ impl SslContextBuilder { // still stored in ex data to manage the lifetime. let arg = self .ctx - .set_ex_data(SslContext::cached_ex_index::(), callback); + .replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_tlsext_servername_arg(self.as_ptr(), arg); ffi::SSL_CTX_set_tlsext_servername_callback(self.as_ptr(), Some(raw_sni::)); @@ -1372,7 +1372,7 @@ impl SslContextBuilder { F: for<'a> Fn(&mut SslRef, &'a [u8]) -> Result<&'a [u8], AlpnError> + 'static + Sync + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_alpn_select_cb( self.as_ptr(), Some(callbacks::raw_alpn_select::), @@ -1393,7 +1393,7 @@ impl SslContextBuilder { F: Fn(ClientHello<'_>) -> Result<(), SelectCertError> + Sync + Send + 'static, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_select_certificate_cb( self.as_ptr(), Some(callbacks::raw_select_cert::), @@ -1413,7 +1413,7 @@ impl SslContextBuilder { M: PrivateKeyMethod, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), method); + self.replace_ex_data(SslContext::cached_ex_index::(), method); ffi::SSL_CTX_set_private_key_method( self.as_ptr(), @@ -1480,7 +1480,7 @@ impl SslContextBuilder { F: Fn(&mut SslRef) -> Result + 'static + Sync + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); cvt( ffi::SSL_CTX_set_tlsext_status_cb(self.as_ptr(), Some(raw_tlsext_status::)) as c_int, @@ -1506,7 +1506,7 @@ impl SslContextBuilder { + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_psk_client_callback(self.as_ptr(), Some(raw_client_psk::)); } } @@ -1539,7 +1539,7 @@ impl SslContextBuilder { + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_psk_server_callback(self.as_ptr(), Some(raw_server_psk::)); } } @@ -1565,7 +1565,7 @@ impl SslContextBuilder { F: Fn(&mut SslRef, SslSession) + 'static + Sync + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_sess_set_new_cb(self.as_ptr(), Some(callbacks::raw_new_session::)); } } @@ -1582,7 +1582,7 @@ impl SslContextBuilder { F: Fn(&SslContextRef, &SslSessionRef) + 'static + Sync + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_sess_set_remove_cb( self.as_ptr(), Some(callbacks::raw_remove_session::), @@ -1611,7 +1611,7 @@ impl SslContextBuilder { + Sync + Send, { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_sess_set_get_cb(self.as_ptr(), Some(callbacks::raw_get_session::)); } @@ -1629,7 +1629,7 @@ impl SslContextBuilder { F: Fn(&SslRef, &str) + 'static + Sync + Send, { unsafe { - self.set_ex_data(SslContext::cached_ex_index::(), callback); + self.replace_ex_data(SslContext::cached_ex_index::(), callback); ffi::SSL_CTX_set_keylog_callback(self.as_ptr(), Some(callbacks::raw_keylog::)); } } @@ -2649,7 +2649,7 @@ impl SslRef { unsafe { // this needs to be in an Arc since the callback can register a new callback! - self.set_ex_data(Ssl::cached_ex_index(), Arc::new(verify)); + self.replace_ex_data(Ssl::cached_ex_index(), Arc::new(verify)); ffi::SSL_set_verify( self.as_ptr(), mode.bits() as c_int, diff --git a/tokio-boring/src/async_callbacks.rs b/tokio-boring/src/async_callbacks.rs index 94bd7c62..7ea36444 100644 --- a/tokio-boring/src/async_callbacks.rs +++ b/tokio-boring/src/async_callbacks.rs @@ -298,7 +298,7 @@ fn with_ex_data_future( match fut.as_mut().poll(&mut ctx) { Poll::Ready(fut_result) => Poll::Ready(into_result(fut_result)), Poll::Pending => { - get_ssl_mut(ssl_handle).set_ex_data(index, MutOnly::new(Some(fut))); + get_ssl_mut(ssl_handle).replace_ex_data(index, MutOnly::new(Some(fut))); Poll::Pending }