Skip to content

Commit

Permalink
Use replace_ex_data more
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@nox
nox committed Nov 17, 2023
1 parent 2ab7141 commit 43140e0
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
26 changes: 13 additions & 13 deletions boring/src/ssl/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::<F>(), verify);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), verify);
ffi::SSL_CTX_set_verify(self.as_ptr(), mode.bits() as c_int, Some(raw_verify::<F>));
}
}
Expand Down Expand Up @@ -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::<F>(), callback);
.replace_ex_data(SslContext::cached_ex_index::<F>(), 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::<F>));
Expand Down Expand Up @@ -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::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_set_alpn_select_cb(
self.as_ptr(),
Some(callbacks::raw_alpn_select::<F>),
Expand All @@ -1393,7 +1393,7 @@ impl SslContextBuilder {
F: Fn(ClientHello<'_>) -> Result<(), SelectCertError> + Sync + Send + 'static,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_set_select_certificate_cb(
self.as_ptr(),
Some(callbacks::raw_select_cert::<F>),
Expand All @@ -1413,7 +1413,7 @@ impl SslContextBuilder {
M: PrivateKeyMethod,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<M>(), method);
self.replace_ex_data(SslContext::cached_ex_index::<M>(), method);

ffi::SSL_CTX_set_private_key_method(
self.as_ptr(),
Expand Down Expand Up @@ -1480,7 +1480,7 @@ impl SslContextBuilder {
F: Fn(&mut SslRef) -> Result<bool, ErrorStack> + 'static + Sync + Send,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
cvt(
ffi::SSL_CTX_set_tlsext_status_cb(self.as_ptr(), Some(raw_tlsext_status::<F>))
as c_int,
Expand All @@ -1506,7 +1506,7 @@ impl SslContextBuilder {
+ Send,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_set_psk_client_callback(self.as_ptr(), Some(raw_client_psk::<F>));
}
}
Expand Down Expand Up @@ -1539,7 +1539,7 @@ impl SslContextBuilder {
+ Send,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_set_psk_server_callback(self.as_ptr(), Some(raw_server_psk::<F>));
}
}
Expand All @@ -1565,7 +1565,7 @@ impl SslContextBuilder {
F: Fn(&mut SslRef, SslSession) + 'static + Sync + Send,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_sess_set_new_cb(self.as_ptr(), Some(callbacks::raw_new_session::<F>));
}
}
Expand All @@ -1582,7 +1582,7 @@ impl SslContextBuilder {
F: Fn(&SslContextRef, &SslSessionRef) + 'static + Sync + Send,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_sess_set_remove_cb(
self.as_ptr(),
Some(callbacks::raw_remove_session::<F>),
Expand Down Expand Up @@ -1611,7 +1611,7 @@ impl SslContextBuilder {
+ Sync
+ Send,
{
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_sess_set_get_cb(self.as_ptr(), Some(callbacks::raw_get_session::<F>));
}

Expand All @@ -1629,7 +1629,7 @@ impl SslContextBuilder {
F: Fn(&SslRef, &str) + 'static + Sync + Send,
{
unsafe {
self.set_ex_data(SslContext::cached_ex_index::<F>(), callback);
self.replace_ex_data(SslContext::cached_ex_index::<F>(), callback);
ffi::SSL_CTX_set_keylog_callback(self.as_ptr(), Some(callbacks::raw_keylog::<F>));
}
}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tokio-boring/src/async_callbacks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ fn with_ex_data_future<H, R, T, E>(
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
}
Expand Down

0 comments on commit 43140e0

Please sign in to comment.