From 9bf21623b040fed1aaec0bd522ba5e7272302083 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Sat, 8 Mar 2025 10:35:46 -0800 Subject: [PATCH 1/6] Add set_ticket_key_callback (SSL_CTX_set_tlsext_ticket_key_cb) Add a wrapper for the `SSL_CTX_set_tlsext_ticket_key_cb`, which allows consumers to configure the EVP_CIPHER_CTX and HMAC_CTX used for encrypting/decrypting session tickets. See https://docs.openssl.org/1.0.2/man3/SSL_CTX_set_tlsext_ticket_key_cb/ for more details. --- boring/src/ssl/callbacks.rs | 41 ++++++ boring/src/ssl/mod.rs | 81 +++++++++++ boring/src/ssl/test/mod.rs | 1 + boring/src/ssl/test/session_resumption.rs | 159 ++++++++++++++++++++++ 4 files changed, 282 insertions(+) create mode 100644 boring/src/ssl/test/session_resumption.rs diff --git a/boring/src/ssl/callbacks.rs b/boring/src/ssl/callbacks.rs index 8ab17b98..a2f83152 100644 --- a/boring/src/ssl/callbacks.rs +++ b/boring/src/ssl/callbacks.rs @@ -8,6 +8,7 @@ use super::{ }; use crate::error::ErrorStack; use crate::ffi; +use crate::ssl::TicketKeyCallbackResult; use crate::x509::{X509StoreContext, X509StoreContextRef}; use foreign_types::ForeignType; use foreign_types::ForeignTypeRef; @@ -269,6 +270,46 @@ where } } +pub(super) unsafe extern "C" fn raw_ticket_key( + ssl: *mut ffi::SSL, + key_name: *mut u8, + iv: *mut u8, + evp_ctx: *mut ffi::EVP_CIPHER_CTX, + hmac_ctx: *mut ffi::HMAC_CTX, + encrypt: c_int, +) -> c_int +where + F: Fn( + &SslRef, + &mut [u8; 16], + *mut u8, + *mut ffi::EVP_CIPHER_CTX, + *mut ffi::HMAC_CTX, + bool, + ) -> TicketKeyCallbackResult + + 'static + + Sync + + Send, +{ + // SAFETY: boring provides valid inputs. + let ssl = unsafe { SslRef::from_ptr_mut(ssl) }; + + let ssl_context = ssl.ssl_context().to_owned(); + let callback = ssl_context + .ex_data::(SslContext::cached_ex_index::()) + .expect("expected session resumption callback"); + + // Safety: the callback guarantees that key_name is 16 bytes + let key_name = + unsafe { slice::from_raw_parts_mut(key_name, ffi::SSL_TICKET_KEY_NAME_LEN as usize) }; + let key_name = <&mut [u8; 16]>::try_from(key_name).expect("boring provides a 16-byte key name"); + + // When encrypting a new ticket, encrypt will be one. + let encrypt = encrypt == 1; + + callback(ssl, key_name, iv, evp_ctx, hmac_ctx, encrypt).into() +} + pub(super) unsafe extern "C" fn raw_alpn_select( ssl: *mut ffi::SSL, out: *mut *const c_uchar, diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 66a11fbc..1fffd6f6 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -903,6 +903,50 @@ pub enum SslInfoCallbackValue { Alert(SslInfoCallbackAlert), } +/// Ticket key callback status. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum TicketKeyCallbackResult { + /// Abort the handshake. + Error, + + /// The peer supplied session ticket was not recognized. Continue with a full handshake. + /// + /// # Note + /// + /// This is a decryption specific status code. + DecryptTicketUnrecognized, + + /// Resumption callback was successful. + /// + /// When in decryption mode, attempt an abbreviated handshake via session resumption. When in + /// encryption mode, provide a new ticket to the client. + Success, + + /// Resumption callback was successful. Attempt an abbreviated handshake, and additionally + /// provide new session tickets to the peer. + /// + /// Session resumption short-circuits some security checks of a full-handshake, in exchange for + /// potential performance gains. For this reason, a session ticket should only be valid for a + /// limited time. Providing the peer with renewed session tickets allows them to continue + /// session resumption with the new tickets. + /// + /// # Note + /// + /// This is a decryption specific status code. + DecryptSuccessRenew, +} + +impl From for c_int { + fn from(value: TicketKeyCallbackResult) -> Self { + match value { + TicketKeyCallbackResult::Error => -1, + TicketKeyCallbackResult::DecryptTicketUnrecognized => 0, + TicketKeyCallbackResult::Success => 1, + TicketKeyCallbackResult::DecryptSuccessRenew => 2, + } + } +} + #[derive(Hash, Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Debug)] pub struct SslInfoCallbackAlert(c_int); @@ -1185,6 +1229,43 @@ impl SslContextBuilder { } } + /// Configures a custom session ticket key callback for session resumption. + /// + /// Session Resumption uses the security context (aka. session tickets) of a previous + /// connection to establish a new connection via an abbreviated handshake. Skipping portions of + /// a handshake can potentially yield performance gains. + /// + /// An attacker that compromises a server's session ticket key can impersonate the server and, + /// prior to TLS 1.3, retroactively decrypt all application traffic from sessions using that + /// ticket key. Thus ticket keys must be regularly rotated for forward secrecy. + /// + /// # Panics + /// + /// This method panics if this `Ssl` is associated with a RPK context. + #[corresponds(SSL_CTX_set_tlsext_ticket_key_cb)] + pub fn set_ticket_key_callback(&mut self, callback: F) + where + F: Fn( + &SslRef, + &mut [u8; 16], + *mut u8, + *mut ffi::EVP_CIPHER_CTX, + *mut ffi::HMAC_CTX, + bool, + ) -> TicketKeyCallbackResult + + 'static + + Sync + + Send, + { + #[cfg(feature = "rpk")] + assert!(!self.is_rpk, "This API is not supported for RPK"); + + unsafe { + self.replace_ex_data(SslContext::cached_ex_index::(), callback); + ffi::SSL_CTX_set_tlsext_ticket_key_cb(self.as_ptr(), Some(raw_ticket_key::)) + }; + } + /// Sets the certificate verification depth. /// /// If the peer's certificate chain is longer than this value, verification will fail. diff --git a/boring/src/ssl/test/mod.rs b/boring/src/ssl/test/mod.rs index 4566b73c..9965bc58 100644 --- a/boring/src/ssl/test/mod.rs +++ b/boring/src/ssl/test/mod.rs @@ -32,6 +32,7 @@ mod ech; mod private_key_method; mod server; mod session; +mod session_resumption; mod verify; static ROOT_CERT: &[u8] = include_bytes!("../../../test/root-ca.pem"); diff --git a/boring/src/ssl/test/session_resumption.rs b/boring/src/ssl/test/session_resumption.rs new file mode 100644 index 00000000..c7556dfe --- /dev/null +++ b/boring/src/ssl/test/session_resumption.rs @@ -0,0 +1,159 @@ +use super::server::Server; +use crate::ssl::test::MessageDigest; +use crate::ssl::SslRef; +use crate::ssl::SslSession; +use crate::ssl::SslSessionCacheMode; +use crate::ssl::TicketKeyCallbackResult; +use crate::symm::Cipher; +use std::ffi::c_void; +use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::OnceLock; + +static CUSTOM_ENCRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); +static CUSTOM_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); + +#[test] +fn resume_session() { + static SESSION_TICKET: OnceLock> = OnceLock::new(); + + let mut server = Server::builder(); + server.expected_connections_count(2); + let server = server.build(); + + let mut client = server.client(); + client + .ctx() + .set_session_cache_mode(SslSessionCacheMode::CLIENT); + client.ctx().set_new_session_callback(|_, session| { + let _can_receive_multiple_tickets = SESSION_TICKET.set(session.to_der().unwrap()); + }); + let ssl_stream = client.connect(); + + assert!(!ssl_stream.ssl().session_reused()); + assert!(SESSION_TICKET.get().is_some()); + + // Retrieve the session ticket + let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); + + // Attempt to resume the connection using the session ticket + let client_2 = server.client(); + let mut ssl_builder = client_2.build().builder(); + unsafe { ssl_builder.ssl().set_session(&session_ticket).unwrap() }; + let ssl_stream_2 = ssl_builder.connect(); + + assert!(ssl_stream_2.ssl().session_reused()); +} + +#[test] +fn custom_callback() { + static SESSION_TICKET: OnceLock> = OnceLock::new(); + + let mut server = Server::builder(); + server.expected_connections_count(2); + server + .ctx() + .set_ticket_key_callback(test_tickey_key_callback); + let server = server.build(); + + let mut client = server.client(); + client + .ctx() + .set_session_cache_mode(SslSessionCacheMode::CLIENT); + client.ctx().set_new_session_callback(|_, session| { + let _can_receive_multiple_tickets = SESSION_TICKET.set(session.to_der().unwrap()); + }); + let ssl_stream = client.connect(); + + assert!(!ssl_stream.ssl().session_reused()); + assert!(SESSION_TICKET.get().is_some()); + assert_eq!(CUSTOM_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 2); + assert_eq!(CUSTOM_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 0); + + // Retrieve the session ticket + let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); + + // Attempt to resume the connection using the session ticket + let client_2 = server.client(); + let mut ssl_builder = client_2.build().builder(); + unsafe { ssl_builder.ssl().set_session(&session_ticket).unwrap() }; + let ssl_stream_2 = ssl_builder.connect(); + + assert!(ssl_stream_2.ssl().session_reused()); + assert_eq!(CUSTOM_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 4); + assert_eq!(CUSTOM_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 1); +} + +// Custom callback to encrypt and decrypt session tickets +fn test_tickey_key_callback( + _ssl: &SslRef, + _key_name: &mut [u8; 16], + _iv: *mut u8, + evp_ctx: *mut ffi::EVP_CIPHER_CTX, + hmac_ctx: *mut ffi::HMAC_CTX, + encrypt: bool, +) -> TicketKeyCallbackResult { + // These should only be used for testing purposes. + const TEST_CBC_IV: [u8; 16] = [1; 16]; + const TEST_AES_128_CBC_KEY: [u8; 16] = [2; 16]; + const TEST_HMAC_KEY: [u8; 32] = [3; 32]; + + let digest = MessageDigest::sha256(); + let cipher = Cipher::aes_128_cbc(); + + if encrypt { + CUSTOM_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + // Set the encryption context. + let ret = unsafe { + ffi::EVP_EncryptInit_ex( + evp_ctx, + cipher.as_ptr(), + // ENGINE api is deprecated + core::ptr::null_mut(), + TEST_AES_128_CBC_KEY.as_ptr(), + TEST_CBC_IV.as_ptr(), + ) + }; + assert!(ret == 1); + + // Set the hmac context. + let ret = unsafe { + ffi::HMAC_Init_ex( + hmac_ctx, + TEST_HMAC_KEY.as_ptr() as *const c_void, + TEST_HMAC_KEY.len(), + digest.as_ptr(), + // ENGINE api is deprecated + core::ptr::null_mut(), + ) + }; + assert!(ret == 1); + } else { + CUSTOM_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + let ret = unsafe { + ffi::EVP_DecryptInit_ex( + evp_ctx, + cipher.as_ptr(), + // ENGINE api is deprecated + core::ptr::null_mut(), + TEST_AES_128_CBC_KEY.as_ptr(), + TEST_CBC_IV.as_ptr(), + ) + }; + assert!(ret == 1); + + // Set the hmac context. + let ret = unsafe { + ffi::HMAC_Init_ex( + hmac_ctx, + TEST_HMAC_KEY.as_ptr() as *const c_void, + TEST_HMAC_KEY.len(), + digest.as_ptr(), + // ENGINE api is deprecated + core::ptr::null_mut(), + ) + }; + assert!(ret == 1); + } + + TicketKeyCallbackResult::Success +} From 818157a146876061d790145d9400b1957ca6d977 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Mon, 10 Mar 2025 22:30:25 -0700 Subject: [PATCH 2/6] pr comments: safety, receive multiple nst, return status refactor --- boring/src/ssl/callbacks.rs | 2 +- boring/src/ssl/mod.rs | 8 +++++--- boring/src/ssl/test/session_resumption.rs | 16 ++++++++++++++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/boring/src/ssl/callbacks.rs b/boring/src/ssl/callbacks.rs index a2f83152..311cf386 100644 --- a/boring/src/ssl/callbacks.rs +++ b/boring/src/ssl/callbacks.rs @@ -299,7 +299,7 @@ where .ex_data::(SslContext::cached_ex_index::()) .expect("expected session resumption callback"); - // Safety: the callback guarantees that key_name is 16 bytes + // SAFETY: the callback guarantees that key_name is 16 bytes let key_name = unsafe { slice::from_raw_parts_mut(key_name, ffi::SSL_TICKET_KEY_NAME_LEN as usize) }; let key_name = <&mut [u8; 16]>::try_from(key_name).expect("boring provides a 16-byte key name"); diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index 1fffd6f6..b9dfc65b 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -909,12 +909,14 @@ pub enum TicketKeyCallbackResult { /// Abort the handshake. Error, - /// The peer supplied session ticket was not recognized. Continue with a full handshake. + /// Continue with a full handshake. + /// + /// The peer supplied session ticket was not recognized. /// /// # Note /// /// This is a decryption specific status code. - DecryptTicketUnrecognized, + Noop, /// Resumption callback was successful. /// @@ -940,7 +942,7 @@ impl From for c_int { fn from(value: TicketKeyCallbackResult) -> Self { match value { TicketKeyCallbackResult::Error => -1, - TicketKeyCallbackResult::DecryptTicketUnrecognized => 0, + TicketKeyCallbackResult::Noop => 0, TicketKeyCallbackResult::Success => 1, TicketKeyCallbackResult::DecryptSuccessRenew => 2, } diff --git a/boring/src/ssl/test/session_resumption.rs b/boring/src/ssl/test/session_resumption.rs index c7556dfe..3492fff8 100644 --- a/boring/src/ssl/test/session_resumption.rs +++ b/boring/src/ssl/test/session_resumption.rs @@ -15,6 +15,7 @@ static CUSTOM_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); #[test] fn resume_session() { static SESSION_TICKET: OnceLock> = OnceLock::new(); + static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); let mut server = Server::builder(); server.expected_connections_count(2); @@ -25,12 +26,17 @@ fn resume_session() { .ctx() .set_session_cache_mode(SslSessionCacheMode::CLIENT); client.ctx().set_new_session_callback(|_, session| { - let _can_receive_multiple_tickets = SESSION_TICKET.set(session.to_der().unwrap()); + NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst); + // The server sends multiple session tickets but we only care to retrieve one. + if SESSION_TICKET.get().is_none() { + SESSION_TICKET.set(session.to_der().unwrap()).unwrap(); + } }); let ssl_stream = client.connect(); assert!(!ssl_stream.ssl().session_reused()); assert!(SESSION_TICKET.get().is_some()); + assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); // Retrieve the session ticket let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); @@ -47,6 +53,7 @@ fn resume_session() { #[test] fn custom_callback() { static SESSION_TICKET: OnceLock> = OnceLock::new(); + static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); let mut server = Server::builder(); server.expected_connections_count(2); @@ -60,7 +67,11 @@ fn custom_callback() { .ctx() .set_session_cache_mode(SslSessionCacheMode::CLIENT); client.ctx().set_new_session_callback(|_, session| { - let _can_receive_multiple_tickets = SESSION_TICKET.set(session.to_der().unwrap()); + NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst); + // The server sends multiple session tickets but we only care to retrieve one. + if SESSION_TICKET.get().is_none() { + SESSION_TICKET.set(session.to_der().unwrap()).unwrap(); + } }); let ssl_stream = client.connect(); @@ -68,6 +79,7 @@ fn custom_callback() { assert!(SESSION_TICKET.get().is_some()); assert_eq!(CUSTOM_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 2); assert_eq!(CUSTOM_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 0); + assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); // Retrieve the session ticket let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); From f6a55efa1993bf6b422bd63f9e56c034132bbf4c Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Tue, 11 Mar 2025 12:10:21 -0700 Subject: [PATCH 3/6] add test case for TicketKeyCallbackResult::Noop --- boring/src/ssl/test/session_resumption.rs | 123 ++++++++++++++++++++-- 1 file changed, 112 insertions(+), 11 deletions(-) diff --git a/boring/src/ssl/test/session_resumption.rs b/boring/src/ssl/test/session_resumption.rs index 3492fff8..638461a7 100644 --- a/boring/src/ssl/test/session_resumption.rs +++ b/boring/src/ssl/test/session_resumption.rs @@ -9,8 +9,10 @@ use std::ffi::c_void; use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::OnceLock; -static CUSTOM_ENCRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); -static CUSTOM_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); +static SUCCESS_ENCRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); +static SUCCESS_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); +static NOOP_ENCRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); +static NOOP_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); #[test] fn resume_session() { @@ -51,7 +53,7 @@ fn resume_session() { } #[test] -fn custom_callback() { +fn custom_callback_success() { static SESSION_TICKET: OnceLock> = OnceLock::new(); static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); @@ -59,7 +61,7 @@ fn custom_callback() { server.expected_connections_count(2); server .ctx() - .set_ticket_key_callback(test_tickey_key_callback); + .set_ticket_key_callback(test_success_tickey_key_callback); let server = server.build(); let mut client = server.client(); @@ -77,8 +79,8 @@ fn custom_callback() { assert!(!ssl_stream.ssl().session_reused()); assert!(SESSION_TICKET.get().is_some()); - assert_eq!(CUSTOM_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 2); - assert_eq!(CUSTOM_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 0); + assert_eq!(SUCCESS_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 2); + assert_eq!(SUCCESS_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 0); assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); // Retrieve the session ticket @@ -91,12 +93,111 @@ fn custom_callback() { let ssl_stream_2 = ssl_builder.connect(); assert!(ssl_stream_2.ssl().session_reused()); - assert_eq!(CUSTOM_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 4); - assert_eq!(CUSTOM_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 1); + assert_eq!(SUCCESS_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 4); + assert_eq!(SUCCESS_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 1); +} + +#[test] +fn custom_callback_unrecognized_decryption_ticket() { + static SESSION_TICKET: OnceLock> = OnceLock::new(); + static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); + + let mut server = Server::builder(); + server.expected_connections_count(2); + server + .ctx() + .set_ticket_key_callback(test_noop_tickey_key_callback); + let server = server.build(); + + let mut client = server.client(); + client + .ctx() + .set_session_cache_mode(SslSessionCacheMode::CLIENT); + client.ctx().set_new_session_callback(|_, session| { + NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst); + // The server sends multiple session tickets but we only care to retrieve one. + if SESSION_TICKET.get().is_none() { + SESSION_TICKET.set(session.to_der().unwrap()).unwrap(); + } + }); + let ssl_stream = client.connect(); + + assert!(!ssl_stream.ssl().session_reused()); + assert!(SESSION_TICKET.get().is_some()); + assert_eq!(NOOP_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 2); + assert_eq!(NOOP_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 0); + assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); + + // Retrieve the session ticket + let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); + + // Attempt to resume the connection using the session ticket + let client_2 = server.client(); + let mut ssl_builder = client_2.build().builder(); + unsafe { ssl_builder.ssl().set_session(&session_ticket).unwrap() }; + let ssl_stream_2 = ssl_builder.connect(); + + // Second connection was NOT resumed due to TicketKeyCallbackResult::Noop on decryption + assert!(!ssl_stream_2.ssl().session_reused()); + assert_eq!(NOOP_ENCRYPTION_CALLED_BACK.load(Ordering::SeqCst), 4); + assert_eq!(NOOP_DECRYPTION_CALLED_BACK.load(Ordering::SeqCst), 1); +} + +// Successfully return a session ticket in encryption mode but return a +// TicketKeyCallbackResult::Noop in decryption mode. +fn test_noop_tickey_key_callback( + _ssl: &SslRef, + _key_name: &mut [u8; 16], + _iv: *mut u8, + evp_ctx: *mut ffi::EVP_CIPHER_CTX, + hmac_ctx: *mut ffi::HMAC_CTX, + encrypt: bool, +) -> TicketKeyCallbackResult { + // These should only be used for testing purposes. + const TEST_CBC_IV: [u8; 16] = [1; 16]; + const TEST_AES_128_CBC_KEY: [u8; 16] = [2; 16]; + const TEST_HMAC_KEY: [u8; 32] = [3; 32]; + + let digest = MessageDigest::sha256(); + let cipher = Cipher::aes_128_cbc(); + + if encrypt { + NOOP_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + // Set the encryption context. + let ret = unsafe { + ffi::EVP_EncryptInit_ex( + evp_ctx, + cipher.as_ptr(), + // ENGINE api is deprecated + core::ptr::null_mut(), + TEST_AES_128_CBC_KEY.as_ptr(), + TEST_CBC_IV.as_ptr(), + ) + }; + assert!(ret == 1); + + // Set the hmac context. + let ret = unsafe { + ffi::HMAC_Init_ex( + hmac_ctx, + TEST_HMAC_KEY.as_ptr() as *const c_void, + TEST_HMAC_KEY.len(), + digest.as_ptr(), + // ENGINE api is deprecated + core::ptr::null_mut(), + ) + }; + assert!(ret == 1); + + TicketKeyCallbackResult::Success + } else { + NOOP_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + TicketKeyCallbackResult::Noop + } } // Custom callback to encrypt and decrypt session tickets -fn test_tickey_key_callback( +fn test_success_tickey_key_callback( _ssl: &SslRef, _key_name: &mut [u8; 16], _iv: *mut u8, @@ -113,7 +214,7 @@ fn test_tickey_key_callback( let cipher = Cipher::aes_128_cbc(); if encrypt { - CUSTOM_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + SUCCESS_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); // Set the encryption context. let ret = unsafe { ffi::EVP_EncryptInit_ex( @@ -140,7 +241,7 @@ fn test_tickey_key_callback( }; assert!(ret == 1); } else { - CUSTOM_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + SUCCESS_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); let ret = unsafe { ffi::EVP_DecryptInit_ex( evp_ctx, From 9a6e16649e8097d6db0540ea097a6498710ff017 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Tue, 1 Apr 2025 12:19:23 -0700 Subject: [PATCH 4/6] update documentation --- boring/src/ssl/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index b9dfc65b..a2e66ef8 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -911,11 +911,12 @@ pub enum TicketKeyCallbackResult { /// Continue with a full handshake. /// - /// The peer supplied session ticket was not recognized. + /// When in decryption mode, this indicates that the peer supplied session ticket was not + /// recognized. When in encryption mode, this instructs boring to not send a session ticket. /// /// # Note /// - /// This is a decryption specific status code. + /// This is a decryption specific status code when using the submoduled BoringSSL. Noop, /// Resumption callback was successful. From cb41dbc1841b801a8fdf3628f4b9a0bb66b861a8 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Wed, 2 Apr 2025 09:46:50 -0700 Subject: [PATCH 5/6] simplify tests --- boring/src/ssl/test/session_resumption.rs | 24 +++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/boring/src/ssl/test/session_resumption.rs b/boring/src/ssl/test/session_resumption.rs index 638461a7..df9c62b7 100644 --- a/boring/src/ssl/test/session_resumption.rs +++ b/boring/src/ssl/test/session_resumption.rs @@ -16,7 +16,7 @@ static NOOP_DECRYPTION_CALLED_BACK: AtomicU8 = AtomicU8::new(0); #[test] fn resume_session() { - static SESSION_TICKET: OnceLock> = OnceLock::new(); + static SESSION_TICKET: OnceLock = OnceLock::new(); static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); let mut server = Server::builder(); @@ -30,9 +30,7 @@ fn resume_session() { client.ctx().set_new_session_callback(|_, session| { NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst); // The server sends multiple session tickets but we only care to retrieve one. - if SESSION_TICKET.get().is_none() { - SESSION_TICKET.set(session.to_der().unwrap()).unwrap(); - } + let _ = SESSION_TICKET.set(session); }); let ssl_stream = client.connect(); @@ -41,7 +39,7 @@ fn resume_session() { assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); // Retrieve the session ticket - let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); + let session_ticket = SESSION_TICKET.get().unwrap(); // Attempt to resume the connection using the session ticket let client_2 = server.client(); @@ -54,7 +52,7 @@ fn resume_session() { #[test] fn custom_callback_success() { - static SESSION_TICKET: OnceLock> = OnceLock::new(); + static SESSION_TICKET: OnceLock = OnceLock::new(); static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); let mut server = Server::builder(); @@ -71,9 +69,7 @@ fn custom_callback_success() { client.ctx().set_new_session_callback(|_, session| { NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst); // The server sends multiple session tickets but we only care to retrieve one. - if SESSION_TICKET.get().is_none() { - SESSION_TICKET.set(session.to_der().unwrap()).unwrap(); - } + let _ = SESSION_TICKET.set(session); }); let ssl_stream = client.connect(); @@ -84,7 +80,7 @@ fn custom_callback_success() { assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); // Retrieve the session ticket - let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); + let session_ticket = SESSION_TICKET.get().unwrap(); // Attempt to resume the connection using the session ticket let client_2 = server.client(); @@ -99,7 +95,7 @@ fn custom_callback_success() { #[test] fn custom_callback_unrecognized_decryption_ticket() { - static SESSION_TICKET: OnceLock> = OnceLock::new(); + static SESSION_TICKET: OnceLock = OnceLock::new(); static NST_RECIEVED_COUNT: AtomicU8 = AtomicU8::new(0); let mut server = Server::builder(); @@ -116,9 +112,7 @@ fn custom_callback_unrecognized_decryption_ticket() { client.ctx().set_new_session_callback(|_, session| { NST_RECIEVED_COUNT.fetch_add(1, Ordering::SeqCst); // The server sends multiple session tickets but we only care to retrieve one. - if SESSION_TICKET.get().is_none() { - SESSION_TICKET.set(session.to_der().unwrap()).unwrap(); - } + let _ = SESSION_TICKET.set(session); }); let ssl_stream = client.connect(); @@ -129,7 +123,7 @@ fn custom_callback_unrecognized_decryption_ticket() { assert_eq!(NST_RECIEVED_COUNT.load(Ordering::SeqCst), 2); // Retrieve the session ticket - let session_ticket = SslSession::from_der(SESSION_TICKET.get().unwrap()).unwrap(); + let session_ticket = SESSION_TICKET.get().unwrap(); // Attempt to resume the connection using the session ticket let client_2 = server.client(); From d5ecffde0f92b9d0943a96529121ebd55ea2ce45 Mon Sep 17 00:00:00 2001 From: Apoorv Kothari Date: Wed, 2 Apr 2025 10:40:52 -0700 Subject: [PATCH 6/6] clippy --- boring/src/ssl/test/session_resumption.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/boring/src/ssl/test/session_resumption.rs b/boring/src/ssl/test/session_resumption.rs index df9c62b7..e604d410 100644 --- a/boring/src/ssl/test/session_resumption.rs +++ b/boring/src/ssl/test/session_resumption.rs @@ -44,7 +44,7 @@ fn resume_session() { // Attempt to resume the connection using the session ticket let client_2 = server.client(); let mut ssl_builder = client_2.build().builder(); - unsafe { ssl_builder.ssl().set_session(&session_ticket).unwrap() }; + unsafe { ssl_builder.ssl().set_session(session_ticket).unwrap() }; let ssl_stream_2 = ssl_builder.connect(); assert!(ssl_stream_2.ssl().session_reused()); @@ -85,7 +85,7 @@ fn custom_callback_success() { // Attempt to resume the connection using the session ticket let client_2 = server.client(); let mut ssl_builder = client_2.build().builder(); - unsafe { ssl_builder.ssl().set_session(&session_ticket).unwrap() }; + unsafe { ssl_builder.ssl().set_session(session_ticket).unwrap() }; let ssl_stream_2 = ssl_builder.connect(); assert!(ssl_stream_2.ssl().session_reused()); @@ -128,7 +128,7 @@ fn custom_callback_unrecognized_decryption_ticket() { // Attempt to resume the connection using the session ticket let client_2 = server.client(); let mut ssl_builder = client_2.build().builder(); - unsafe { ssl_builder.ssl().set_session(&session_ticket).unwrap() }; + unsafe { ssl_builder.ssl().set_session(session_ticket).unwrap() }; let ssl_stream_2 = ssl_builder.connect(); // Second connection was NOT resumed due to TicketKeyCallbackResult::Noop on decryption