Zeroize sensitive memory in barrier_aes_gcm context

.gitignore

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

Cargo.toml

@@ -38,6 +38,7 @@ delay_timer = "0.11"
 as-any = "0.3.1"
 pem = "3.0"
 chrono = "0.4"
+zeroize = { version = "1.4.0", features= ["derive"] }
 
 [target.'cfg(unix)'.dependencies]
 daemonize = "0.5"

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},
@@ -13,14 +14,18 @@ use super::{
     Storage, StorageEntry,
 };
 use crate::errors::RvError;
+use zeroize::{Zeroize, Zeroizing};
 
 const EPOCH_SIZE: usize = 4;
 const KEY_EPOCH: u8 = 1;
 const AES_GCM_VERSION: u8 = 0x1;
 const AES_BLOCK_SIZE: usize = 16;
 
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+// the BarrierInit structure contains the encryption key, so it's zeroized anyway
+// when it's dropped
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Zeroize)]
 #[serde(deny_unknown_fields)]
+#[zeroize(drop)]
 struct BarrierInit {
     version: u32,
     key: Vec<u8>,
@@ -96,9 +101,12 @@ impl SecurityBarrier for AESGCMBarrier {
         Ok(res.is_some())
     }
 
-    fn init(&self, key: &[u8]) -> Result<(), RvError> {
+    // 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 key.len() < min || key.len() > max {
+        if kek.len() < min || kek.len() > max {
             return Err(RvError::ErrBarrierKeyInvalid);
         }
 
@@ -108,13 +116,14 @@ impl SecurityBarrier for AESGCMBarrier {
             return Err(RvError::ErrBarrierAlreadyInit);
         }
 
+        // the encrypt_key variable will be zeroized automatically on drop
         let encrypt_key = self.generate_key()?;
 
-        let barrier_init = BarrierInit { version: 1, key: encrypt_key };
+        let barrier_init = BarrierInit { version: 1, key: encrypt_key.to_vec() };
 
         let serialized_barrier_init = serde_json::to_string(&barrier_init)?;
 
-        self.init_cipher(key)?;
+        self.init_cipher(kek)?;
 
         let value = self.encrypt(serialized_barrier_init.as_bytes())?;
 
@@ -127,11 +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;
-        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)
     }
 
@@ -144,7 +154,7 @@ impl SecurityBarrier for AESGCMBarrier {
         Ok(barrier_info.sealed)
     }
 
-    fn unseal(&self, key: &[u8]) -> Result<(), RvError> {
+    fn unseal(&self, kek: &[u8]) -> Result<(), RvError> {
         let sealed = self.sealed()?;
         if !sealed {
             return Ok(());
@@ -155,14 +165,17 @@ impl SecurityBarrier for AESGCMBarrier {
             return Err(RvError::ErrBarrierNotInit);
         }
 
-        self.init_cipher(key)?;
+        self.init_cipher(kek)?;
 
         let value = self.decrypt(entry.unwrap().value.as_slice());
         if value.is_err() {
             return Err(RvError::ErrBarrierUnsealFailed);
         }
         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()?;
@@ -207,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;
@@ -220,18 +235,19 @@ impl AESGCMBarrier {
 
         let cipher = barrier_info.cipher.unwrap();
         let mut cipher_ctx = barrier_info.cipher_ctx.as_ref().unwrap().write()?;
-        let key = barrier_info.key.clone().unwrap();
+        // XXX: the cloned variable 'key' will be zeroized automatically on drop
+        let key = Zeroizing::new(barrier_info.key.clone().unwrap());
 
         // Assuming nonce size is the same as IV size
         let nonce_size = cipher.iv_length();
 
         // Generate a random nonce
-        let mut nonce = vec![0u8; nonce_size];
-        thread_rng().fill(nonce.as_mut_slice());
+        let mut nonce = Zeroizing::new(vec![0u8; nonce_size]);
+        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.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..])?;
@@ -245,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());
 
@@ -264,7 +280,8 @@ impl AESGCMBarrier {
 
         let cipher = barrier_info.cipher.unwrap();
         let mut cipher_ctx = barrier_info.cipher_ctx.as_ref().unwrap().write()?;
-        let key = barrier_info.key.clone().unwrap();
+        // XXX: the cloned variable 'key' will be zeroized automatically on drop
+        let key = Zeroizing::new(barrier_info.key.clone().unwrap());
 
         let nonce_size = cipher.iv_length();
 
@@ -274,7 +291,7 @@ impl AESGCMBarrier {
 
         let nonce = &ciphertext[5..5 + nonce_size];
 
-        cipher_ctx.decrypt_init(Some(cipher), Some(key.as_slice()), Some(nonce))?;
+        cipher_ctx.decrypt_init(Some(cipher), Some(key.deref().as_slice()), Some(nonce))?;
         cipher_ctx.set_padding(false);
 
         let tag_size = cipher_ctx.tag_length();