diff --git a/CHANGELOG.md b/CHANGELOG.md index d372bd5..3c45a6f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Cargo.lock b/Cargo.lock index 53f3559..c3a9c00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -602,6 +602,7 @@ dependencies = [ "time", "tokio", "x509-certificate", + "zeroize", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index b0b957b..fa126d0 100644 --- a/Cargo.toml +++ b/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"] diff --git a/src/ssl.rs b/src/ssl.rs index 16b36ea..baec2a2 100644 --- a/src/ssl.rs +++ b/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); impl PrivateKey { pub(crate) fn from_byte_string(byte_string: ua::ByteString) -> Option { - (!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 } } diff --git a/src/ua/data_types/byte_string.rs b/src/ua/data_types/byte_string.rs index af9ab0a..87aee9a 100644 --- a/src/ua/data_types/byte_string.rs +++ b/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); + } +}