diff --git a/boring/src/ssl/callbacks.rs b/boring/src/ssl/callbacks.rs index a8e3d5ca..d1f42f77 100644 --- a/boring/src/ssl/callbacks.rs +++ b/boring/src/ssl/callbacks.rs @@ -282,7 +282,7 @@ where F: Fn( &SslRef, &mut [u8; 16], - *mut u8, + &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize], *mut ffi::EVP_CIPHER_CTX, *mut ffi::HMAC_CTX, bool, @@ -304,9 +304,23 @@ where 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"); + // 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. 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() } diff --git a/boring/src/ssl/mod.rs b/boring/src/ssl/mod.rs index c19c7381..147ce469 100644 --- a/boring/src/ssl/mod.rs +++ b/boring/src/ssl/mod.rs @@ -1140,13 +1140,20 @@ impl SslContextBuilder { /// # Panics /// /// 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)] - pub fn set_ticket_key_callback(&mut self, callback: F) + pub unsafe fn set_ticket_key_callback_unsafe(&mut self, callback: F) where F: Fn( &SslRef, &mut [u8; 16], - *mut u8, + &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize], *mut ffi::EVP_CIPHER_CTX, *mut ffi::HMAC_CTX, bool, diff --git a/boring/src/ssl/test/session_resumption.rs b/boring/src/ssl/test/session_resumption.rs index e604d410..c4b6c2d4 100644 --- a/boring/src/ssl/test/session_resumption.rs +++ b/boring/src/ssl/test/session_resumption.rs @@ -57,9 +57,11 @@ fn custom_callback_success() { let mut server = Server::builder(); server.expected_connections_count(2); - server - .ctx() - .set_ticket_key_callback(test_success_tickey_key_callback); + unsafe { + server + .ctx() + .set_ticket_key_callback_unsafe(test_success_tickey_key_callback) + }; let server = server.build(); let mut client = server.client(); @@ -100,9 +102,11 @@ fn custom_callback_unrecognized_decryption_ticket() { let mut server = Server::builder(); server.expected_connections_count(2); - server - .ctx() - .set_ticket_key_callback(test_noop_tickey_key_callback); + unsafe { + server + .ctx() + .set_ticket_key_callback_unsafe(test_noop_tickey_key_callback) + }; let server = server.build(); let mut client = server.client(); @@ -141,14 +145,14 @@ fn custom_callback_unrecognized_decryption_ticket() { // TicketKeyCallbackResult::Noop in decryption mode. fn test_noop_tickey_key_callback( _ssl: &SslRef, - _key_name: &mut [u8; 16], - _iv: *mut u8, + key_name: &mut [u8; 16], + iv: &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize], 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_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_HMAC_KEY: [u8; 32] = [3; 32]; @@ -156,6 +160,9 @@ fn test_noop_tickey_key_callback( let cipher = Cipher::aes_128_cbc(); if encrypt { + assert_eq!(key_name, &[0; 16]); + assert_eq!(iv, &[0; 16]); + NOOP_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); // Set the encryption context. let ret = unsafe { @@ -193,14 +200,14 @@ fn test_noop_tickey_key_callback( // Custom callback to encrypt and decrypt session tickets fn test_success_tickey_key_callback( _ssl: &SslRef, - _key_name: &mut [u8; 16], - _iv: *mut u8, + key_name: &mut [u8; 16], + iv: &mut [u8; ffi::EVP_MAX_IV_LENGTH as usize], 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_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_HMAC_KEY: [u8; 32] = [3; 32]; @@ -208,6 +215,9 @@ fn test_success_tickey_key_callback( let cipher = Cipher::aes_128_cbc(); if encrypt { + assert_eq!(key_name, &[0; 16]); + assert_eq!(iv, &[0; 16]); + SUCCESS_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); // Set the encryption context. let ret = unsafe { @@ -236,6 +246,7 @@ fn test_success_tickey_key_callback( assert!(ret == 1); } else { SUCCESS_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst); + // Set the decryption context. let ret = unsafe { ffi::EVP_DecryptInit_ex( evp_ctx,