Implement memory zeroization before dropping private key

CHANGELOG.md

@@ -7,6 +7,10 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Added
+
+- Zeroize memory held by `PrivateKey` when dropped.
+
 ## [0.7.0] - 2024-12-13
 
 ### Added

Cargo.toml

@@ -33,6 +33,7 @@ tokio = { version = "1.35.1", optional = true, features = [
   "time",
 ] }
 x509-certificate = { version = "0.24.0", optional = true }
+zeroize = { version = "1.8.1", optional = true }
 
 [dev-dependencies]
 anyhow = "1.0.79"
@@ -47,7 +48,7 @@ tokio = { version = "1.35.1", features = ["macros", "rt-multi-thread"] }
 
 [features]
 default = ["serde", "time", "tokio"]
-mbedtls = ["open62541-sys/mbedtls"]
+mbedtls = ["dep:zeroize", "open62541-sys/mbedtls"]
 serde = ["dep:serde", "dep:serde_json", "time?/formatting", "time?/serde"]
 time = ["dep:time"]
 tokio = ["dep:tokio"]

src/ssl.rs

@@ -1,6 +1,7 @@
 use std::{fmt, ptr};
 
 use open62541_sys::UA_CreateCertificate;
+use zeroize::Zeroizing;
 
 use crate::{ua, DataType, Error};
 
@@ -63,14 +64,17 @@ impl Certificate {
 
 /// Private key in [DER] or [PEM] format.
 ///
+/// The wrapped memory is [zeroized] when dropped.
+///
 /// [DER]: https://en.wikipedia.org/wiki/X.690#DER_encoding
 /// [PEM]: https://en.wikipedia.org/wiki/Privacy-Enhanced_Mail
+/// [zeroized]: https://crates.io/crates/zeroize
 #[derive(Clone)]
-pub struct PrivateKey(ua::ByteString);
+pub struct PrivateKey(Zeroizing<ua::ByteString>);
 
 impl PrivateKey {
     pub(crate) fn from_byte_string(byte_string: ua::ByteString) -> Option<Self> {
-        (!byte_string.is_invalid()).then(|| Self(byte_string))
+        (!byte_string.is_invalid()).then(|| Self(Zeroizing::new(byte_string)))
     }
 
     pub(crate) unsafe fn from_string_unchecked(string: ua::String) -> Self {
@@ -81,9 +85,12 @@ impl PrivateKey {
     ///
     /// This does not validate the data. When passing the instance to another method, that method
     /// may still fail if the private key is not valid.
+    ///
+    /// As this takes only a slice and returns an owned copy, the origial data should be zeroized
+    /// independently as soon as it is no longer needed.
     #[must_use]
     pub fn from_bytes(bytes: &[u8]) -> Self {
-        Self(ua::ByteString::new(bytes))
+        Self(Zeroizing::new(ua::ByteString::new(bytes)))
     }
 
     /// Gets certificate data.
@@ -93,7 +100,7 @@ impl PrivateKey {
         unsafe { self.0.as_bytes_unchecked() }
     }
 
-    pub(crate) const fn as_byte_string(&self) -> &ua::ByteString {
+    pub(crate) fn as_byte_string(&self) -> &ua::ByteString {
         &self.0
     }
 }

src/ua/data_types/byte_string.rs

@@ -1,6 +1,6 @@
 use std::slice;
 
-use open62541_sys::{UA_ByteString_copy, UA_String};
+use open62541_sys::{UA_ByteString_clear, UA_ByteString_copy, UA_ByteString_memZero, UA_String};
 
 use crate::{ua, ArrayValue, DataType};
 
@@ -38,6 +38,22 @@ impl ByteString {
         dst
     }
 
+    fn clear(&mut self) {
+        unsafe {
+            // Clearing frees the referenced heap memory and resets length and data pointer to all
+            // zeroes, i.e. the string becomes an "invalid" string (as defined by OPC UA).
+            UA_ByteString_clear(self.as_mut_ptr());
+        }
+    }
+
+    fn mem_zero(&mut self) {
+        unsafe {
+            // This zeroizes the string contents, i.e. characters, leaving the string object itself
+            // intact. The string has the same length as before but is all `\0`.
+            UA_ByteString_memZero(self.as_mut_ptr());
+        }
+    }
+
     /// Checks if byte string is invalid.
     ///
     /// The invalid state is defined by OPC UA. It is a third state which is distinct from empty and
@@ -98,3 +114,44 @@ impl serde::Serialize for ByteString {
             .and_then(|bytes| serializer.serialize_bytes(bytes))
     }
 }
+
+#[cfg(feature = "mbedtls")]
+impl zeroize::Zeroize for ua::ByteString {
+    fn zeroize(&mut self) {
+        // Clear the heap memory of the string characters.
+        self.mem_zero();
+        // Clear the string data structure, i.e. length.
+        self.clear();
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn zeroizing() {
+        let mut string = ua::ByteString::new("SECRET".as_bytes());
+        let data_ptr = string.as_bytes().unwrap().as_ptr();
+        let object_ptr = unsafe { string.as_ptr() };
+
+        let data = unsafe { slice::from_raw_parts(data_ptr, 6) };
+        assert_eq!(data, "SECRET".as_bytes());
+        let object = unsafe { std::ptr::read(object_ptr) };
+        assert!(!object.data.is_null());
+        assert_eq!(object.length, 6);
+
+        string.mem_zero();
+
+        // SAFETY: Memory has been zeroized but is still allocated.
+        let data = unsafe { slice::from_raw_parts(data_ptr, 6) };
+        assert_eq!(data, "\0\0\0\0\0\0".as_bytes());
+
+        string.clear();
+
+        // SAFETY: Object has been reset but is still allocated.
+        let object = unsafe { std::ptr::read(object_ptr) };
+        assert!(object.data.is_null());
+        assert_eq!(object.length, 0);
+    }
+}