Convert CipherCtx fns into a safe abstraction. Additional testing.

Author: toidiu

boring/src/ssl/test/session_resumption.rs

@@ -153,6 +153,7 @@ fn test_noop_tickey_key_callback(
     encrypt: bool,
 ) -> TicketKeyCallbackResult {
     // These should only be used for testing purposes.
+    const TEST_KEY_NAME: [u8; 16] = [5; 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];
@@ -161,24 +162,29 @@ fn test_noop_tickey_key_callback(
     let cipher = Cipher::aes_128_cbc();
 
     if encrypt {
+        NOOP_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
+
+        // Ensure key_name and iv are initialized and set test values.
         assert_eq!(key_name, &[0; 16]);
         assert_eq!(iv, &[0; 16]);
-
-        NOOP_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
+        key_name.copy_from_slice(&TEST_KEY_NAME);
+        iv.copy_from_slice(&TEST_CBC_IV);
 
         // Set the encryption context.
-        unsafe {
-            evp_ctx
-                .init_encrypt(&cipher, &TEST_AES_128_CBC_KEY, &TEST_CBC_IV)
-                .unwrap()
-        };
+        evp_ctx
+            .init_encrypt(&cipher, &TEST_AES_128_CBC_KEY, &TEST_CBC_IV)
+            .unwrap();
 
         // Set the hmac context.
         unsafe { hmac_ctx.init(&TEST_HMAC_KEY, &digest).unwrap() };
 
         TicketKeyCallbackResult::Success
     } else {
         NOOP_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
+
+        // Check key_name matches.
+        assert_eq!(key_name, &TEST_KEY_NAME);
+
         TicketKeyCallbackResult::Noop
     }
 }
@@ -193,6 +199,7 @@ fn test_success_tickey_key_callback(
     encrypt: bool,
 ) -> TicketKeyCallbackResult {
     // These should only be used for testing purposes.
+    const TEST_KEY_NAME: [u8; 16] = [5; 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];
@@ -201,28 +208,31 @@ fn test_success_tickey_key_callback(
     let cipher = Cipher::aes_128_cbc();
 
     if encrypt {
+        SUCCESS_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
+
+        // Ensure key_name and iv are initialized and set test values.
         assert_eq!(key_name, &[0; 16]);
         assert_eq!(iv, &[0; 16]);
-
-        SUCCESS_ENCRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
+        key_name.copy_from_slice(&TEST_KEY_NAME);
+        iv.copy_from_slice(&TEST_CBC_IV);
 
         // Set the encryption context.
-        unsafe {
-            evp_ctx
-                .init_encrypt(&cipher, &TEST_AES_128_CBC_KEY, &TEST_CBC_IV)
-                .unwrap()
-        };
+        evp_ctx
+            .init_encrypt(&cipher, &TEST_AES_128_CBC_KEY, &TEST_CBC_IV)
+            .unwrap();
 
         // Set the hmac context.
         unsafe { hmac_ctx.init(&TEST_HMAC_KEY, &digest).unwrap() };
     } else {
         SUCCESS_DECRYPTION_CALLED_BACK.fetch_add(1, Ordering::SeqCst);
+
+        // Check key_name matches.
+        assert_eq!(key_name, &TEST_KEY_NAME);
+
         // Set the decryption context.
-        unsafe {
-            evp_ctx
-                .init_decrypt(&cipher, &TEST_AES_128_CBC_KEY, &TEST_CBC_IV)
-                .unwrap()
-        };
+        evp_ctx
+            .init_decrypt(&cipher, &TEST_AES_128_CBC_KEY, iv)
+            .unwrap();
 
         // Set the hmac context.
         unsafe { hmac_ctx.init(&TEST_HMAC_KEY, &digest).unwrap() };

boring/src/symm.rs

@@ -80,21 +80,18 @@ impl CipherCtxRef {
     /// Configures CipherCtx for a fresh encryption operation using `cipher`.
     ///
     /// https://commondatastorage.googleapis.com/chromium-boringssl-docs/cipher.h.html#EVP_EncryptInit_ex
-    ///
-    /// # Safety
-    ///
-    /// The caller must ensure EVP_CIPHER_CTX has been initalized.
-    ///
-    /// The caller is responsible for ensuring the length of `key` and `iv` are appropriate for the
-    /// chosen Cipher.
-    pub unsafe fn init_encrypt(
+    pub fn init_encrypt(
         &mut self,
         cipher: &Cipher,
         key: &[u8],
         iv: &[u8; ffi::EVP_MAX_IV_LENGTH as usize],
     ) -> Result<(), ErrorStack> {
         ffi::init();
 
+        if key.len() != cipher.key_len() {
+            return Err(ErrorStack::get());
+        }
+
         unsafe {
             cvt(ffi::EVP_EncryptInit_ex(
                 self.as_ptr(),
@@ -111,21 +108,18 @@ impl CipherCtxRef {
     /// Configures CipherCtx for a fresh decryption operation using `cipher`.
     ///
     /// https://commondatastorage.googleapis.com/chromium-boringssl-docs/cipher.h.html#EVP_DecryptInit_ex
-    ///
-    /// # Safety
-    ///
-    /// The caller must ensure EVP_CIPHER_CTX has been initalized.
-    ///
-    /// The caller is responsible for ensuring the length of `key` and `iv` are appropriate for the
-    /// chosen Cipher.
-    pub unsafe fn init_decrypt(
+    pub fn init_decrypt(
         &mut self,
         cipher: &Cipher,
         key: &[u8],
         iv: &[u8; ffi::EVP_MAX_IV_LENGTH as usize],
     ) -> Result<(), ErrorStack> {
         ffi::init();
 
+        if key.len() != cipher.key_len() {
+            return Err(ErrorStack::get());
+        }
+
         unsafe {
             cvt(ffi::EVP_DecryptInit_ex(
                 self.as_ptr(),