fixup! Zeroize sensitive memory in barrier_aes_gcm context

.gitignore

@@ -19,3 +19,6 @@ Cargo.lock
 # Added by cargo
 
 /target
+
+# VIM temp file
+*.swp

src/core.rs

@@ -1,6 +1,7 @@
 use std::{
     collections::HashMap,
     sync::{Arc, Mutex, RwLock},
+    ops::{Deref, DerefMut},
 };
 
 use as_any::Downcast;
@@ -26,6 +27,8 @@ use crate::{
     },
 };
 
+use zeroize::Zeroizing;
+
 pub type LogicalBackendNewFunc = dyn Fn(Arc<RwLock<Core>>) -> Result<Arc<dyn Backend>, RvError> + Send + Sync;
 
 pub const SEAL_CONFIG_PATH: &str = "core/seal-config";
@@ -46,9 +49,9 @@ impl SealConfig {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct InitResult {
-    pub secret_shares: Vec<Vec<u8>>,
+    pub secret_shares: Zeroizing<Vec<Vec<u8>>>,
     pub root_token: String,
 }
 
@@ -128,29 +131,30 @@ impl Core {
 
         let barrier = Arc::clone(&self.barrier);
         // Generate a master key
+        // The newly generated master key will be zeroized on drop.
         let master_key = barrier.generate_key()?;
 
         // Initialize the barrier
-        barrier.init(master_key.as_slice())?;
+        barrier.init(master_key.deref().as_slice())?;
 
-        let mut init_result = InitResult { secret_shares: Vec::new(), root_token: String::new() };
+        let mut init_result = InitResult { secret_shares: Zeroizing::new(Vec::new()), root_token: String::new() };
 
         if seal_config.secret_shares == 1 {
-            init_result.secret_shares.push(master_key.clone());
+            init_result.secret_shares.deref_mut().push(master_key.deref().clone());
         } else {
             init_result.secret_shares =
-                ShamirSecret::split(&master_key, seal_config.secret_shares, seal_config.secret_threshold)?;
+                ShamirSecret::split(master_key.deref().as_slice(), seal_config.secret_shares, seal_config.secret_threshold)?;
         }
 
-        log::debug!("master_key: {}", hex::encode(&master_key));
+        log::debug!("master_key: {}", hex::encode(master_key.deref()));
         log::debug!("seal config: {:?}", seal_config);
         log::debug!("secret_shares:");
         for key in init_result.secret_shares.iter() {
             log::debug!("{}", hex::encode(&key));
         }
 
         // Unseal the barrier
-        barrier.unseal(master_key.as_slice())?;
+        barrier.unseal(master_key.deref().as_slice())?;
 
         defer! (
             // Ensure the barrier is re-sealed

src/shamir.rs

@@ -1,6 +1,8 @@
 use rand::{thread_rng, RngCore};
 
 use crate::errors::RvError;
+use zeroize::Zeroizing;
+use std::ops::DerefMut;
 
 static GF256_EXP: [u8; 256] = [
     0x01, 0xe5, 0x4c, 0xb5, 0xfb, 0x9f, 0xfc, 0x12, 0x03, 0x34, 0xd4, 0xc4, 0x16, 0xba, 0x1f, 0x36, 0x05, 0x5c, 0x67,
@@ -128,16 +130,16 @@ impl ShamirSecret {
         Some(mysecretdata)
     }
 
-    pub fn split(secret: &[u8], part: u8, threshold: u8) -> Result<Vec<Vec<u8>>, RvError> {
+    pub fn split(secret: &[u8], part: u8, threshold: u8) -> Result<Zeroizing<Vec<Vec<u8>>>, RvError> {
         if part < threshold || threshold < 2 {
             return Err(RvError::ErrShamirShareCountInvalid);
         }
 
         let secret_data = ShamirSecret::with_secret(secret, threshold);
-        let mut out: Vec<Vec<u8>> = vec![];
+        let mut out: Zeroizing<Vec<Vec<u8>>> = Zeroizing::new(vec![]);
         for i in 1..(part + 1) {
             let shared = secret_data.get_share(i)?;
-            out.push(shared);
+            out.deref_mut().push(shared);
         }
         Ok(out)
     }

src/storage/barrier.rs

@@ -1,12 +1,13 @@
 use super::Storage;
 use crate::errors::RvError;
+use zeroize::Zeroizing;
 
 pub const BARRIER_INIT_PATH: &str = "barrier/init";
 
 pub trait SecurityBarrier: Storage + Send + Sync {
     fn inited(&self) -> Result<bool, RvError>;
     fn init(&self, key: &[u8]) -> Result<(), RvError>;
-    fn generate_key(&self) -> Result<Vec<u8>, RvError>;
+    fn generate_key(&self) -> Result<Zeroizing<Vec<u8>>, RvError>;
     fn key_length_range(&self) -> (usize, usize);
     fn sealed(&self) -> Result<bool, RvError>;
     fn unseal(&self, key: &[u8]) -> Result<(), RvError>;

src/storage/barrier_aes_gcm.rs

@@ -1,4 +1,5 @@
 use std::sync::{Arc, RwLock};
+use std::ops::{Deref, DerefMut};
 
 use openssl::{
     cipher::{Cipher, CipherRef},
@@ -30,9 +31,6 @@ struct BarrierInit {
     key: Vec<u8>,
 }
 
-// TODO: the BarrierInfo structure contains encryption key, so it's zeroized anyway
-// when it's dropped
-// it's not possible to use 'zeroize(drop)' due to trait bounds check
 struct BarrierInfo {
     sealed: bool,
     key: Option<Vec<u8>>,
@@ -105,6 +103,7 @@ impl SecurityBarrier for AESGCMBarrier {
 
     // kek stands for key encryption key, it's used to encrypt the actual
     // encryption key, which is generated during the init() process.
+    // The kek's zerization is handled in the caller.
     fn init(&self, kek: &[u8]) -> Result<(), RvError> {
         let (min, max) = self.key_length_range();
         if kek.len() < min || kek.len() > max {
@@ -118,7 +117,7 @@ impl SecurityBarrier for AESGCMBarrier {
         }
 
         // the encrypt_key variable will be zeroized automatically on drop
-        let encrypt_key = Zeroizing::new(self.generate_key()?);
+        let encrypt_key = self.generate_key()?;
 
         let barrier_init = BarrierInit { version: 1, key: encrypt_key.to_vec() };
 
@@ -137,12 +136,12 @@ impl SecurityBarrier for AESGCMBarrier {
         Ok(())
     }
 
-    fn generate_key(&self) -> Result<Vec<u8>, RvError> {
+    fn generate_key(&self) -> Result<Zeroizing<Vec<u8>>, RvError> {
         let key_size = 2 * AES_BLOCK_SIZE;
-        // TODO: zeroized on drop
-        let mut buf = vec![0u8; key_size];
+        // will be zeroized on drop
+        let mut buf = Zeroizing::new(vec![0u8; key_size]);
 
-        thread_rng().fill(buf.as_mut_slice());
+        thread_rng().fill(buf.deref_mut().as_mut_slice());
         Ok(buf)
     }
 
@@ -175,6 +174,8 @@ impl SecurityBarrier for AESGCMBarrier {
         let barrier_init: BarrierInit = serde_json::from_slice(value.unwrap().as_slice())?;
 
         // the barrier_init.key is the real encryption key generated in init().
+        // the whole barrier_init will be zeroized on drop, so there is no special
+        // zeroizing logic on barrier_init.key.
         self.init_cipher(barrier_init.key.as_slice())?;
 
         let mut barrier_info = self.barrier_info.write()?;
@@ -219,6 +220,8 @@ impl AESGCMBarrier {
 
     fn reset_cipher(&self) -> Result<(), RvError> {
         let mut barrier_info = self.barrier_info.write()?;
+        // Zeroize it explicitly
+        barrier_info.key.zeroize();
         barrier_info.key = None;
         barrier_info.cipher = None;
         barrier_info.cipher_ctx = None;
@@ -238,14 +241,13 @@ impl AESGCMBarrier {
         // Assuming nonce size is the same as IV size
         let nonce_size = cipher.iv_length();
 
-        // TODO: should nonce be zeroized on drop?
         // Generate a random nonce
         let mut nonce = Zeroizing::new(vec![0u8; nonce_size]);
-        thread_rng().fill(nonce.as_mut_slice());
+        thread_rng().fill(nonce.deref_mut().as_mut_slice());
 
         // Encrypt
         let mut ciphertext = vec![0u8; plaintext.len()];
-        cipher_ctx.encrypt_init(Some(cipher), Some(key.to_vec().as_slice()), Some(nonce.as_slice()))?;
+        cipher_ctx.encrypt_init(Some(cipher), Some(key.deref().as_slice()), Some(nonce.deref().as_slice()))?;
         cipher_ctx.set_padding(false);
         let len = cipher_ctx.cipher_update(plaintext, Some(&mut ciphertext))?;
         let _final_len = cipher_ctx.cipher_final(&mut ciphertext[len..])?;
@@ -259,7 +261,7 @@ impl AESGCMBarrier {
 
         out[3] = KEY_EPOCH;
         out[4] = AES_GCM_VERSION;
-        out[5..5 + nonce_size].copy_from_slice(nonce.as_slice());
+        out[5..5 + nonce_size].copy_from_slice(nonce.deref().as_slice());
         out[5 + nonce_size..5 + nonce_size + ciphertext.len()].copy_from_slice(ciphertext.as_slice());
         out[5 + nonce_size + ciphertext.len()..size].copy_from_slice(tag.as_slice());