Skip to content

Commit

Permalink
Add memory zeroizing when needed
Browse files Browse the repository at this point in the history
Signed-off-by: Hugues de Valon <hugues.devalon@arm.com>
  • Loading branch information
hug-dev committed Sep 1, 2020
1 parent f957949 commit a9ef329
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 12 deletions.
4 changes: 3 additions & 1 deletion src/key_info_managers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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>,
Expand Down
18 changes: 9 additions & 9 deletions src/providers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/providers/pkcs11_provider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use std::io::{Error, ErrorKind};
use std::sync::{Arc, Mutex, RwLock};
use utils::{KeyPairType, ReadWriteSession, Session};
use uuid::Uuid;
use zeroize::Zeroize;

type LocalIdStore = HashSet<[u8; 4]>;

Expand Down Expand Up @@ -275,10 +276,17 @@ 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();
self.user_pin.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 {
Expand Down
6 changes: 5 additions & 1 deletion src/providers/pkcs11_provider/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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];
Expand Down Expand Up @@ -335,7 +336,8 @@ pub struct Session<'a> {
is_logged_in: bool,
}

#[derive(PartialEq)]
#[derive(PartialEq, Zeroize)]
#[zeroize(drop)]
pub enum ReadWriteSession {
ReadOnly,
ReadWrite,
Expand Down Expand Up @@ -517,6 +519,8 @@ impl Drop for Session<'_> {
}
}
}
self.session_handle.zeroize();
self.is_logged_in.zeroize();
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/providers/tpm_provider/key_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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));
Expand All @@ -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)?
Expand Down
7 changes: 7 additions & 0 deletions src/providers/tpm_provider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions src/providers/tpm_provider/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand Down

0 comments on commit a9ef329

Please sign in to comment.