Merge pull request parallaxsecond#239 from hug-dev/memory

Add memory zeroizing when needed

Author: hug-dev

src/key_info_managers/mod.rs

@@ -12,6 +12,7 @@ use parsec_interface::operations::psa_key_attributes::Attributes;
 use parsec_interface::requests::{ProviderID, ResponseStatus};
 use serde::{Deserialize, Serialize};
 use std::fmt;
+use zeroize::Zeroize;
 
 pub mod on_disk_manager;
 
@@ -53,7 +54,8 @@ impl fmt::Display for KeyTriple {
 }
 
 /// Information stored about a key
-#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Clone, Zeroize)]
+#[zeroize(drop)]
 pub struct KeyInfo {
     /// Reference to a key in the Provider
     pub id: Vec<u8>,

src/providers/mod.rs

@@ -10,6 +10,7 @@ use log::trace;
 use parsec_interface::requests::{Opcode, ProviderID};
 use serde::Deserialize;
 use std::collections::HashSet;
+use zeroize::Zeroize;
 
 pub mod core_provider;
 
@@ -27,7 +28,8 @@ pub mod tpm_provider;
 /// to the one described in the Internally Tagged Enum representation
 /// where "provider_type" is the tag field. For details see:
 /// https://serde.rs/enum-representations.html
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize, Debug, Zeroize)]
+#[zeroize(drop)]
 #[serde(tag = "provider_type")]
 pub enum ProviderConfig {
     /// Mbed Crypto provider configuration
@@ -57,21 +59,19 @@ pub enum ProviderConfig {
     },
 }
 
-use self::ProviderConfig::{MbedCrypto, Pkcs11, Tpm};
-
 impl ProviderConfig {
     /// Get the name of the Key Info Manager in the provider configuration
     pub fn key_info_manager(&self) -> &String {
         match *self {
-            MbedCrypto {
+            ProviderConfig::MbedCrypto {
                 ref key_info_manager,
                 ..
             } => key_info_manager,
-            Pkcs11 {
+            ProviderConfig::Pkcs11 {
                 ref key_info_manager,
                 ..
             } => key_info_manager,
-            Tpm {
+            ProviderConfig::Tpm {
                 ref key_info_manager,
                 ..
             } => key_info_manager,
@@ -80,9 +80,9 @@ impl ProviderConfig {
     /// Get the Provider ID of the provider
     pub fn provider_id(&self) -> ProviderID {
         match *self {
-            MbedCrypto { .. } => ProviderID::MbedCrypto,
-            Pkcs11 { .. } => ProviderID::Pkcs11,
-            Tpm { .. } => ProviderID::Tpm,
+            ProviderConfig::MbedCrypto { .. } => ProviderID::MbedCrypto,
+            ProviderConfig::Pkcs11 { .. } => ProviderID::Pkcs11,
+            ProviderConfig::Tpm { .. } => ProviderID::Tpm,
         }
     }
 }

src/providers/pkcs11_provider/mod.rs

@@ -15,13 +15,16 @@ use parsec_interface::operations::{
     psa_generate_key, psa_import_key, psa_sign_hash, psa_verify_hash,
 };
 use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
+use parsec_interface::secrecy::SecretString;
 use pkcs11::types::{CKF_OS_LOCKING_OK, CK_C_INITIALIZE_ARGS, CK_SLOT_ID};
 use pkcs11::Ctx;
 use std::collections::HashSet;
 use std::io::{Error, ErrorKind};
+use std::str::FromStr;
 use std::sync::{Arc, Mutex, RwLock};
 use utils::{KeyPairType, ReadWriteSession, Session};
 use uuid::Uuid;
+use zeroize::Zeroize;
 
 type LocalIdStore = HashSet<[u8; 4]>;
 
@@ -66,7 +69,7 @@ pub struct Pkcs11Provider {
     backend: Ctx,
     slot_number: CK_SLOT_ID,
     // Some PKCS 11 devices do not need a pin, the None variant means that.
-    user_pin: Option<String>,
+    user_pin: Option<SecretString>,
 }
 
 impl Pkcs11Provider {
@@ -78,7 +81,7 @@ impl Pkcs11Provider {
         key_info_store: Arc<RwLock<dyn ManageKeyInfo + Send + Sync>>,
         backend: Ctx,
         slot_number: usize,
-        user_pin: Option<String>,
+        user_pin: Option<SecretString>,
     ) -> Option<Pkcs11Provider> {
         #[allow(clippy::mutex_atomic)]
         let pkcs11_provider = Pkcs11Provider {
@@ -275,18 +278,24 @@ impl Drop for Pkcs11Provider {
         if let Err(e) = self.backend.finalize() {
             format_error!("Error when dropping the PKCS 11 provider", e);
         }
+        // The other fields containing confidential information should implement zeroizing on drop.
+        self.slot_number.zeroize();
     }
 }
 
 /// Builder for Pkcs11Provider
+///
+/// This builder contains some confidential information that is passed to the Pkcs11Provider. The
+/// Pkcs11Provider will zeroize this data when dropping. This data will not be cloned when
+/// building.
 #[derive(Default, Derivative)]
 #[derivative(Debug)]
 pub struct Pkcs11ProviderBuilder {
     #[derivative(Debug = "ignore")]
     key_info_store: Option<Arc<RwLock<dyn ManageKeyInfo + Send + Sync>>>,
     pkcs11_library_path: Option<String>,
     slot_number: Option<usize>,
-    user_pin: Option<String>,
+    user_pin: Option<SecretString>,
 }
 
 impl Pkcs11ProviderBuilder {
@@ -328,8 +337,13 @@ impl Pkcs11ProviderBuilder {
     }
 
     /// Specify the user pin
-    pub fn with_user_pin(mut self, user_pin: Option<String>) -> Pkcs11ProviderBuilder {
-        self.user_pin = user_pin;
+    pub fn with_user_pin(mut self, mut user_pin: Option<String>) -> Pkcs11ProviderBuilder {
+        self.user_pin = match user_pin {
+            // The conversion form a String is infallible.
+            Some(ref pin) => Some(SecretString::from_str(&pin).unwrap()),
+            None => None,
+        };
+        user_pin.zeroize();
 
         self
     }

src/providers/pkcs11_provider/utils.rs

@@ -7,12 +7,14 @@ use parsec_interface::operations::psa_algorithm::*;
 use parsec_interface::operations::psa_key_attributes::*;
 use parsec_interface::requests::ResponseStatus;
 use parsec_interface::requests::Result;
+use parsec_interface::secrecy::ExposeSecret;
 use picky_asn1_x509::{AlgorithmIdentifier, DigestInfo, SHAVariant};
 use pkcs11::errors::Error;
 use pkcs11::types::*;
 use pkcs11::types::{CKF_RW_SESSION, CKF_SERIAL_SESSION, CKU_USER};
 use std::convert::{TryFrom, TryInto};
 use std::pin::Pin;
+use zeroize::Zeroize;
 
 // Public exponent value for all RSA keys.
 const PUBLIC_EXPONENT: [u8; 3] = [0x01, 0x00, 0x01];
@@ -335,7 +337,8 @@ pub struct Session<'a> {
     is_logged_in: bool,
 }
 
-#[derive(PartialEq)]
+#[derive(PartialEq, Zeroize)]
+#[zeroize(drop)]
 pub enum ReadWriteSession {
     ReadOnly,
     ReadWrite,
@@ -417,11 +420,11 @@ impl Session<'_> {
             Ok(())
         } else if let Some(user_pin) = self.provider.user_pin.as_ref() {
             trace!("Login command");
-            match self
-                .provider
-                .backend
-                .login(self.session_handle, CKU_USER, Some(user_pin))
-            {
+            match self.provider.backend.login(
+                self.session_handle,
+                CKU_USER,
+                Some(user_pin.expose_secret()),
+            ) {
                 Ok(_) => {
                     if crate::utils::GlobalConfig::log_error_details() {
                         info!("Logging in session {}.", self.session_handle);
@@ -517,6 +520,8 @@ impl Drop for Session<'_> {
                 }
             }
         }
+        self.session_handle.zeroize();
+        self.is_logged_in.zeroize();
     }
 }
 

src/providers/tpm_provider/key_management.rs

@@ -15,6 +15,7 @@ use parsec_interface::operations::{
 use parsec_interface::requests::{ProviderID, ResponseStatus, Result};
 use parsec_interface::secrecy::ExposeSecret;
 use picky_asn1_x509::RSAPublicKey;
+use zeroize::Zeroize;
 
 // Public exponent value for all RSA keys.
 const PUBLIC_EXPONENT: [u8; 3] = [0x01, 0x00, 0x01];
@@ -24,7 +25,7 @@ const AUTH_VAL_LEN: usize = 32;
 fn insert_password_context(
     store_handle: &mut dyn ManageKeyInfo,
     key_triple: KeyTriple,
-    password_context: PasswordContext,
+    mut password_context: PasswordContext,
     key_attributes: Attributes,
 ) -> Result<()> {
     let error_storing = |e| Err(key_info_managers::to_response_status(e));
@@ -34,6 +35,8 @@ fn insert_password_context(
         attributes: key_attributes,
     };
 
+    password_context.auth_value.zeroize();
+
     if store_handle
         .insert(key_triple, key_info)
         .or_else(error_storing)?

src/providers/tpm_provider/mod.rs

@@ -22,6 +22,7 @@ use std::sync::{Arc, Mutex, RwLock};
 use tss_esapi::constants::algorithm::{Cipher, HashingAlgorithm};
 use tss_esapi::Tcti;
 use uuid::Uuid;
+use zeroize::Zeroize;
 
 mod asym_encryption;
 mod asym_sign;
@@ -171,6 +172,10 @@ impl Drop for TpmProvider {
 }
 
 /// Builder for TpmProvider
+///
+/// This builder contains some confidential information that is passed to the TpmProvider. The
+/// TpmProvider will zeroize this data when dropping. This data will not be cloned when
+/// building.
 #[derive(Default, Derivative)]
 #[derivative(Debug)]
 pub struct TpmProviderBuilder {
@@ -284,6 +289,8 @@ impl TpmProviderBuilder {
         .map_err(|_| {
             std::io::Error::new(ErrorKind::InvalidData, "Invalid TCTI configuration string")
         })?;
+        self.tcti.zeroize();
+        self.owner_hierarchy_auth.zeroize();
         TpmProvider::new(
             self.key_info_store.ok_or_else(|| {
                 std::io::Error::new(ErrorKind::InvalidData, "missing key info store")

src/providers/tpm_provider/utils.rs

@@ -90,6 +90,7 @@ pub fn to_response_status(error: Error) -> ResponseStatus {
 #[derive(Serialize, Deserialize)]
 pub struct PasswordContext {
     pub context: TpmsContext,
+    /// This value is confidential and needs to be zeroized by its new owner.
     pub auth_value: Vec<u8>,
 }