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

Author: toidiu

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()
 }
 

boring/src/ssl/mod.rs

@@ -1240,13 +1240,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<F>(&mut self, callback: F)
+    pub unsafe fn set_ticket_key_callback_unsafe<F>(&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,

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,21 +145,24 @@ 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];
 
     let digest = MessageDigest::sha256();
     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,21 +200,24 @@ 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];
 
     let digest = MessageDigest::sha256();
     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,