initialize key_name and iv. mark fn as _unsafe to allow for future changes to the api

This commit is contained in:
Apoorv Kothari 2025-09-29 14:15:41 -07:00 committed by Kornel
parent b9af0ef176
commit 5cb35db989
3 changed files with 47 additions and 15 deletions

View File

@ -282,7 +282,7 @@ where
F: Fn( F: Fn(
&SslRef, &SslRef,
&mut [u8; 16], &mut [u8; 16],
*mut u8, &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize],
*mut ffi::EVP_CIPHER_CTX, *mut ffi::EVP_CIPHER_CTX,
*mut ffi::HMAC_CTX, *mut ffi::HMAC_CTX,
bool, bool,
@ -304,9 +304,23 @@ where
unsafe { slice::from_raw_parts_mut(key_name, ffi::SSL_TICKET_KEY_NAME_LEN as usize) }; 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"); let key_name = <&mut [u8; 16]>::try_from(key_name).expect("boring provides a 16-byte key name");
// SAFETY: the callback provides 16 bytes iv
//
// https://github.com/google/boringssl/blob/main/ssl/ssl_session.cc#L331
let iv = unsafe { core::slice::from_raw_parts_mut(iv, ffi::EVP_MAX_IV_LENGTH as usize) };
let iv = <&mut [u8; ffi::EVP_MAX_IV_LENGTH as usize]>::try_from(iv)
.expect("boring provides a 16-byte iv");
// When encrypting a new ticket, encrypt will be one. // When encrypting a new ticket, encrypt will be one.
let encrypt = encrypt == 1; let encrypt = encrypt == 1;
// Zero-initialize the key_name and iv, since the application is expected to populate these
// fields in the encrypt mode.
if encrypt {
unsafe { ptr::write(key_name, [0; 16]) };
unsafe { ptr::write(iv, [0; ffi::EVP_MAX_IV_LENGTH as usize]) };
}
callback(ssl, key_name, iv, evp_ctx, hmac_ctx, encrypt).into() callback(ssl, key_name, iv, evp_ctx, hmac_ctx, encrypt).into()
} }

View File

@ -1140,13 +1140,20 @@ impl SslContextBuilder {
/// # Panics /// # Panics
/// ///
/// This method panics if this `Ssl` is associated with a RPK context. /// This method panics if this `Ssl` is associated with a RPK context.
///
/// # Safety
///
/// The application is responsible for correctly setting the key_name, iv, encryption context
/// and hmac context. See the [`SSL_CTX_set_tlsext_ticket_key_cb`] docs for additional info.
///
/// [`SSL_CTX_set_tlsext_ticket_key_cb`]: https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_CTX_set_tlsext_ticket_key_cb
#[corresponds(SSL_CTX_set_tlsext_ticket_key_cb)] #[corresponds(SSL_CTX_set_tlsext_ticket_key_cb)]
pub fn set_ticket_key_callback<F>(&mut self, callback: F) pub unsafe fn set_ticket_key_callback_unsafe<F>(&mut self, callback: F)
where where
F: Fn( F: Fn(
&SslRef, &SslRef,
&mut [u8; 16], &mut [u8; 16],
*mut u8, &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize],
*mut ffi::EVP_CIPHER_CTX, *mut ffi::EVP_CIPHER_CTX,
*mut ffi::HMAC_CTX, *mut ffi::HMAC_CTX,
bool, bool,

View File

@ -57,9 +57,11 @@ fn custom_callback_success() {
let mut server = Server::builder(); let mut server = Server::builder();
server.expected_connections_count(2); server.expected_connections_count(2);
unsafe {
server server
.ctx() .ctx()
.set_ticket_key_callback(test_success_tickey_key_callback); .set_ticket_key_callback_unsafe(test_success_tickey_key_callback)
};
let server = server.build(); let server = server.build();
let mut client = server.client(); let mut client = server.client();
@ -100,9 +102,11 @@ fn custom_callback_unrecognized_decryption_ticket() {
let mut server = Server::builder(); let mut server = Server::builder();
server.expected_connections_count(2); server.expected_connections_count(2);
unsafe {
server server
.ctx() .ctx()
.set_ticket_key_callback(test_noop_tickey_key_callback); .set_ticket_key_callback_unsafe(test_noop_tickey_key_callback)
};
let server = server.build(); let server = server.build();
let mut client = server.client(); let mut client = server.client();
@ -141,14 +145,14 @@ fn custom_callback_unrecognized_decryption_ticket() {
// TicketKeyCallbackResult::Noop in decryption mode. // TicketKeyCallbackResult::Noop in decryption mode.
fn test_noop_tickey_key_callback( fn test_noop_tickey_key_callback(
_ssl: &SslRef, _ssl: &SslRef,
_key_name: &mut [u8; 16], key_name: &mut [u8; 16],
_iv: *mut u8, iv: &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize],
evp_ctx: *mut ffi::EVP_CIPHER_CTX, evp_ctx: *mut ffi::EVP_CIPHER_CTX,
hmac_ctx: *mut ffi::HMAC_CTX, hmac_ctx: *mut ffi::HMAC_CTX,
encrypt: bool, encrypt: bool,
) -> TicketKeyCallbackResult { ) -> TicketKeyCallbackResult {
// These should only be used for testing purposes. // These should only be used for testing purposes.
const TEST_CBC_IV: [u8; 16] = [1; 16]; const TEST_CBC_IV: [u8; ffi::EVP_MAX_IV_LENGTH as usize] = [1; ffi::EVP_MAX_IV_LENGTH as usize];
const TEST_AES_128_CBC_KEY: [u8; 16] = [2; 16]; const TEST_AES_128_CBC_KEY: [u8; 16] = [2; 16];
const TEST_HMAC_KEY: [u8; 32] = [3; 32]; const TEST_HMAC_KEY: [u8; 32] = [3; 32];
@ -156,6 +160,9 @@ fn test_noop_tickey_key_callback(
let cipher = Cipher::aes_128_cbc(); let cipher = Cipher::aes_128_cbc();
if encrypt { if encrypt {
assert_eq!(key_name, &[0; 16]);
assert_eq!(iv, &[0; 16]);
NOOP_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); NOOP_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
// Set the encryption context. // Set the encryption context.
let ret = unsafe { let ret = unsafe {
@ -193,14 +200,14 @@ fn test_noop_tickey_key_callback(
// Custom callback to encrypt and decrypt session tickets // Custom callback to encrypt and decrypt session tickets
fn test_success_tickey_key_callback( fn test_success_tickey_key_callback(
_ssl: &SslRef, _ssl: &SslRef,
_key_name: &mut [u8; 16], key_name: &mut [u8; 16],
_iv: *mut u8, iv: &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize],
evp_ctx: *mut ffi::EVP_CIPHER_CTX, evp_ctx: *mut ffi::EVP_CIPHER_CTX,
hmac_ctx: *mut ffi::HMAC_CTX, hmac_ctx: *mut ffi::HMAC_CTX,
encrypt: bool, encrypt: bool,
) -> TicketKeyCallbackResult { ) -> TicketKeyCallbackResult {
// These should only be used for testing purposes. // These should only be used for testing purposes.
const TEST_CBC_IV: [u8; 16] = [1; 16]; const TEST_CBC_IV: [u8; ffi::EVP_MAX_IV_LENGTH as usize] = [1; ffi::EVP_MAX_IV_LENGTH as usize];
const TEST_AES_128_CBC_KEY: [u8; 16] = [2; 16]; const TEST_AES_128_CBC_KEY: [u8; 16] = [2; 16];
const TEST_HMAC_KEY: [u8; 32] = [3; 32]; const TEST_HMAC_KEY: [u8; 32] = [3; 32];
@ -208,6 +215,9 @@ fn test_success_tickey_key_callback(
let cipher = Cipher::aes_128_cbc(); let cipher = Cipher::aes_128_cbc();
if encrypt { if encrypt {
assert_eq!(key_name, &[0; 16]);
assert_eq!(iv, &[0; 16]);
SUCCESS_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); SUCCESS_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
// Set the encryption context. // Set the encryption context.
let ret = unsafe { let ret = unsafe {
@ -236,6 +246,7 @@ fn test_success_tickey_key_callback(
assert!(ret == 1); assert!(ret == 1);
} else { } else {
SUCCESS_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); SUCCESS_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
// Set the decryption context.
let ret = unsafe { let ret = unsafe {
ffi::EVP_DecryptInit_ex( ffi::EVP_DecryptInit_ex(
evp_ctx, evp_ctx,