Replace Zeroize with OPENSSL_cleanse

The Zeroize crate performance is crazy slow. The search for a performance issue with the nssdb storage via valgrind's callgrind revelead that 50% of the time in the HMAC code was spent on zeroization because of the way the Zeroize crate is built. Replace it everywhere with a simpler zeromem function that underneath simply calls OPENSSL_cleanse() on the mutable slice passed in. With this change alone the new test that performs 100 pbkdf2 calls went from ~9s to ~4.5s on my laptop. Signed-off-by: Simo Sorce <[email protected]>
latchset · Dec 19, 2024 · 9af34b5 · 9af34b5
1 parent c46fb72
commit 9af34b5
Show file tree

Hide file tree

Showing 17 changed files with 70 additions and 69 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -44,7 +44,6 @@ serde_json = "1.0.104"
 serial_test = "3.1.1"
 toml = { version = "0.8.19", default-features = false, features = ["display", "parse"] }
 uuid = { version = "1.4.1", features = ["v4"] }
-zeroize = "1.6.0"
 
 [features]
 aes = []

diff --git a/src/aes.rs b/src/aes.rs
@@ -7,14 +7,13 @@ use crate::attribute::Attribute;
 use crate::error::Result;
 use crate::interface::*;
 use crate::mechanism::*;
+use crate::misc::zeromem;
 use crate::object::*;
 use crate::ossl::aes::*;
 use crate::{attr_element, cast_params};
 
 use once_cell::sync::Lazy;
 
-use zeroize::Zeroize;
-
 pub const MIN_AES_SIZE_BYTES: usize = 16; /* 128 bits */
 pub const MID_AES_SIZE_BYTES: usize = 24; /* 192 bits */
 pub const MAX_AES_SIZE_BYTES: usize = 32; /* 256 bits */
@@ -100,7 +99,7 @@ impl ObjectFactory for AesKeyFactory {
             Some(idx) => {
                 let len = usize::try_from(template[idx].to_ulong()?)?;
                 if len > data.len() {
-                    data.zeroize();
+                    zeromem(data.as_mut_slice());
                     return Err(CKR_KEY_SIZE_RANGE)?;
                 }
                 if len < data.len() {
@@ -112,7 +111,7 @@ impl ObjectFactory for AesKeyFactory {
         match check_key_len(data.len()) {
             Ok(_) => (),
             Err(e) => {
-                data.zeroize();
+                zeromem(data.as_mut_slice());
                 return Err(e);
             }
         }

diff --git a/src/attribute.rs b/src/attribute.rs
@@ -5,10 +5,9 @@ use std::borrow::Cow;
 
 use crate::error::{Error, Result};
 use crate::interface::*;
+use crate::misc::zeromem;
 use crate::{bytes_to_vec, sizeof, void_ptr};
 
-use zeroize::Zeroize;
-
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum AttrType {
     BoolType,
@@ -271,7 +270,7 @@ impl Attribute {
     }
 
     pub fn zeroize(&mut self) {
-        self.value.zeroize();
+        zeromem(self.value.as_mut_slice());
     }
 
     pub fn from_date_bytes(t: CK_ULONG, val: Vec<u8>) -> Attribute {
@@ -559,7 +558,7 @@ impl Drop for CkAttrs<'_> {
     fn drop(&mut self) {
         if self.zeroize {
             while let Some(mut elem) = self.v.pop() {
-                elem.zeroize();
+                zeromem(elem.as_mut_slice());
             }
         }
     }

diff --git a/src/hmac.rs b/src/hmac.rs
@@ -7,11 +7,11 @@ use crate::error::{Error, Result};
 use crate::hash;
 use crate::interface::*;
 use crate::mechanism::*;
+use crate::misc::zeromem;
 use crate::object::*;
 use crate::sizeof;
 
 use once_cell::sync::Lazy;
-use zeroize::Zeroize;
 
 #[cfg(not(feature = "fips"))]
 use crate::native::hmac::HMACOperation;
@@ -26,7 +26,7 @@ pub struct HmacKey {
 
 impl Drop for HmacKey {
     fn drop(&mut self) {
-        self.raw.zeroize()
+        zeromem(self.raw.as_mut_slice())
     }
 }
 

diff --git a/src/kasn1/mod.rs b/src/kasn1/mod.rs
@@ -5,9 +5,9 @@ use std::borrow::Cow;
 
 use crate::error::Result;
 use crate::interface::*;
+use crate::misc::zeromem;
 
 use asn1;
-use zeroize::Zeroize;
 
 /* Helper routines to use with rust/asn1 */
 
@@ -68,7 +68,7 @@ impl<'a> DerEncBigUint<'a> {
 impl Drop for DerEncBigUint<'_> {
     fn drop(&mut self) {
         match &self.data {
-            Cow::Owned(_) => self.data.to_mut().zeroize(),
+            Cow::Owned(_) => zeromem(self.data.to_mut()),
             _ => (),
         }
     }
@@ -111,7 +111,7 @@ impl<'a> DerEncOctetString<'a> {
 impl Drop for DerEncOctetString<'_> {
     fn drop(&mut self) {
         match &self.data {
-            Cow::Owned(_) => self.data.to_mut().zeroize(),
+            Cow::Owned(_) => zeromem(self.data.to_mut()),
             _ => (),
         }
     }

diff --git a/src/misc.rs b/src/misc.rs
@@ -6,7 +6,7 @@ use crate::attribute::{Attribute, CkAttrs};
 use crate::error::Result;
 use crate::interface::*;
 use crate::object::{Object, ObjectFactories, ObjectType};
-
+use crate::ossl::common::zeromem as ossl_zeromem;
 pub const CK_ULONG_SIZE: usize = std::mem::size_of::<CK_ULONG>();
 
 #[macro_export]
@@ -191,3 +191,7 @@ pub fn copy_sized_string(s: &[u8], d: &mut [u8]) {
         d[slen..].fill(0x20); /* space in ASCII/UTF8 */
     }
 }
+
+pub fn zeromem(mem: &mut [u8]) {
+    ossl_zeromem(mem);
+}
diff --git a/src/native/hmac.rs b/src/native/hmac.rs
@@ -8,9 +8,9 @@ use crate::hash;
 use crate::hmac::*;
 use crate::interface::*;
 use crate::mechanism::*;
+use crate::misc::zeromem;
 
 use constant_time_eq::constant_time_eq;
-use zeroize::Zeroize;
 
 /* HMAC spec From FIPS 198-1 */
 
@@ -32,9 +32,9 @@ pub struct HMACOperation {
 
 impl Drop for HMACOperation {
     fn drop(&mut self) {
-        self.state.zeroize();
-        self.ipad.zeroize();
-        self.opad.zeroize();
+        zeromem(self.state.as_mut_slice());
+        zeromem(self.ipad.as_mut_slice());
+        zeromem(self.opad.as_mut_slice());
     }
 }
 

diff --git a/src/object.rs b/src/object.rs
@@ -8,12 +8,12 @@ use crate::attribute::{AttrType, Attribute};
 use crate::error::{Error, Result};
 use crate::interface::*;
 use crate::mechanism::{Mechanism, Mechanisms};
+use crate::misc::zeromem;
 use crate::CSPRNG;
 
 use bitflags::bitflags;
 use once_cell::sync::Lazy;
 use uuid::Uuid;
-use zeroize::Zeroize;
 
 macro_rules! create_bool_checker {
     (make $name:ident; from $id:expr; def $def:expr) => {
@@ -1032,7 +1032,7 @@ macro_rules! ok_or_clear {
         match $exp {
             Ok(x) => x,
             Err(e) => {
-                $clear.zeroize();
+                zeromem($clear.as_mut_slice());
                 return Err(e);
             }
         }

diff --git a/src/ossl/aes.rs b/src/ossl/aes.rs
@@ -8,6 +8,7 @@ use crate::error;
 use crate::error::Result;
 use crate::interface::*;
 use crate::mechanism::*;
+use crate::misc::zeromem;
 use crate::object::Object;
 use crate::ossl::bindings::*;
 use crate::ossl::common::*;
@@ -19,7 +20,6 @@ use crate::get_random_data;
 
 use constant_time_eq::constant_time_eq;
 use once_cell::sync::Lazy;
-use zeroize::Zeroize;
 
 const MAX_CCM_BUF: usize = 1 << 20; /* 1MiB */
 const MIN_RANDOM_IV_BITS: usize = 64;
@@ -126,7 +126,7 @@ struct AesKey {
 
 impl Drop for AesKey {
     fn drop(&mut self) {
-        self.raw.zeroize()
+        zeromem(self.raw.as_mut_slice());
     }
 }
 
@@ -176,7 +176,7 @@ impl AesIvData {
 }
 impl Drop for AesIvData {
     fn drop(&mut self) {
-        self.buf.zeroize();
+        zeromem(self.buf.as_mut_slice());
     }
 }
 
@@ -193,8 +193,8 @@ struct AesParams {
 #[cfg(feature = "fips")]
 impl AesParams {
     fn zeroize(&mut self) {
-        self.iv.buf.zeroize();
-        self.aad.zeroize();
+        zeromem(self.iv.buf.as_mut_slice());
+        zeromem(self.aad.as_mut_slice());
     }
 }
 
@@ -215,7 +215,7 @@ pub struct AesOperation {
 
 impl Drop for AesOperation {
     fn drop(&mut self) {
-        self.finalbuf.zeroize()
+        zeromem(self.finalbuf.as_mut_slice());
     }
 }
 
@@ -944,7 +944,7 @@ impl AesOperation {
         let mut op = match Self::encrypt_new(mech, wrapping_key) {
             Ok(o) => o,
             Err(e) => {
-                keydata.zeroize();
+                zeromem(keydata.as_mut_slice());
                 return Err(e);
             }
         };
@@ -967,7 +967,7 @@ impl AesOperation {
             _ => (),
         }
         let result = op.encrypt(&keydata, output);
-        keydata.zeroize();
+        zeromem(keydata.as_mut_slice());
         result
     }
 
@@ -997,8 +997,8 @@ impl AesOperation {
     ) -> Result<CK_BYTE_PTR> {
         #[cfg(feature = "fips")]
         {
-            self.params.iv.buf.zeroize();
-            self.params.aad.zeroize();
+            zeromem(self.params.iv.buf.as_mut_slice());
+            zeromem(self.params.aad.as_mut_slice());
         }
         match self.mech {
             CKM_AES_CCM => {
@@ -1277,7 +1277,7 @@ impl AesOperation {
         #[cfg(feature = "fips")]
         {
             self.params.zeroize();
-            self.finalbuf.zeroize();
+            zeromem(self.finalbuf.as_mut_slice());
         }
         self.finalized = false;
         self.in_use = true;
@@ -1343,7 +1343,7 @@ impl AesOperation {
         #[cfg(feature = "fips")]
         {
             self.params.zeroize();
-            self.finalbuf.zeroize();
+            zeromem(self.finalbuf.as_mut_slice());
         }
         self.finalized = false;
         self.in_use = true;
@@ -1523,7 +1523,7 @@ impl Encryption for AesOperation {
         }
         if self.mech == CKM_AES_CCM {
             if plain_len > 0 && plain_buf == self.finalbuf.as_ptr() {
-                self.finalbuf.zeroize();
+                zeromem(self.finalbuf.as_mut_slice());
                 self.finalbuf.clear();
             }
         }
@@ -2274,11 +2274,11 @@ impl MsgEncryption for AesOperation {
             )
         };
         if res != 1 {
-            cipher.zeroize();
+            zeromem(cipher);
             return Err(self.op_err(CKR_DEVICE_ERROR));
         }
         if outl != 0 {
-            cipher.zeroize();
+            zeromem(cipher);
             return Err(self.op_err(CKR_DEVICE_ERROR));
         }
         let res = unsafe {
@@ -2290,7 +2290,7 @@ impl MsgEncryption for AesOperation {
             )
         };
         if res != 1 {
-            cipher.zeroize();
+            zeromem(cipher);
             return Err(self.op_err(CKR_DEVICE_ERROR));
         }
 
@@ -2469,11 +2469,11 @@ impl MsgDecryption for AesOperation {
                     )
                 };
                 if res != 1 {
-                    plain.zeroize();
+                    zeromem(plain);
                     return Err(self.op_err(CKR_ENCRYPTED_DATA_INVALID));
                 }
                 if outl != 0 {
-                    plain.zeroize();
+                    zeromem(plain);
                     return Err(self.op_err(CKR_DEVICE_ERROR));
                 }
 
@@ -2629,7 +2629,7 @@ impl AesCmacOperation {
         }
 
         output.copy_from_slice(&buf[..output.len()]);
-        buf.zeroize();
+        zeromem(&mut buf);
 
         #[cfg(feature = "fips")]
         {
@@ -2740,8 +2740,8 @@ pub struct AesMacOperation {
 
 impl Drop for AesMacOperation {
     fn drop(&mut self) {
-        self.padbuf.zeroize();
-        self.macbuf.zeroize();
+        zeromem(&mut self.padbuf);
+        zeromem(&mut self.macbuf);
     }
 }
 

diff --git a/src/ossl/common.rs b/src/ossl/common.rs
@@ -24,7 +24,6 @@ use crate::ossl::montgomery as ecm;
 use crate::ossl::rsa;
 
 use asn1;
-use zeroize::Zeroize;
 
 macro_rules! ptr_wrapper_struct {
     ($name:ident; $ctx:ident) => {
@@ -367,7 +366,7 @@ impl Drop for OsslParam<'_> {
     fn drop(&mut self) {
         if self.zeroize {
             while let Some(mut elem) = self.v.pop() {
-                elem.zeroize();
+                zeromem(elem.as_mut_slice());
             }
         }
     }
@@ -892,3 +891,9 @@ fn oid_to_ossl_name(oid: &asn1::ObjectIdentifier) -> Result<&'static [u8]> {
 pub fn get_ossl_name_from_obj(key: &Object) -> Result<&'static [u8]> {
     oid_to_ossl_name(&get_oid_from_obj(key)?)
 }
+
+pub fn zeromem(mem: &mut [u8]) {
+    unsafe {
+        OPENSSL_cleanse(void_ptr!(mem.as_mut_ptr()), mem.len());
+    }
+}