Skip to content

Commit 4d1c464

Browse files
def-claude
andauthored
tls-util: zeroize sensitive data in consumers, not just Pkcs12Archive (#36133)
The std::mem::take pattern moved der/pass out of Pkcs12Archive into types that didn't zeroize on drop, so the archive's Drop only zeroed empty fields. Fix by adding Pkcs12Archive::into_parts() to cleanly consume the archive, and implementing Zeroize + Drop on ccsr::Identity so the actual sensitive data is zeroized when dropped. Follow-up to 16c15ae Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6f79fa3 commit 4d1c464

5 files changed

Lines changed: 29 additions & 8 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/ccsr/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ openssl.workspace = true
1616
reqwest.workspace = true
1717
mz-tls-util = { path = "../tls-util" }
1818
proptest.workspace = true
19+
zeroize.workspace = true
1920
proptest-derive.workspace = true
2021
serde.workspace = true
2122
serde_json.workspace = true

src/ccsr/src/tls.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use serde::{Deserialize, Serialize};
1313

1414
use mz_tls_util::pkcs12der_from_pem;
15+
use zeroize::Zeroize;
1516

1617
/// A [Serde][serde]-enabled wrapper around [`reqwest::Identity`].
1718
///
@@ -22,14 +23,24 @@ pub struct Identity {
2223
pass: String,
2324
}
2425

26+
impl Zeroize for Identity {
27+
fn zeroize(&mut self) {
28+
self.der.zeroize();
29+
self.pass.zeroize();
30+
}
31+
}
32+
33+
impl Drop for Identity {
34+
fn drop(&mut self) {
35+
self.zeroize();
36+
}
37+
}
38+
2539
impl Identity {
2640
/// Constructs an identity from a PEM-formatted key and certificate using OpenSSL.
2741
pub fn from_pem(key: &[u8], cert: &[u8]) -> Result<Self, openssl::error::ErrorStack> {
28-
let mut archive = pkcs12der_from_pem(key, cert)?;
29-
Ok(Identity {
30-
der: std::mem::take(&mut archive.der),
31-
pass: std::mem::take(&mut archive.pass),
32-
})
42+
let (der, pass) = pkcs12der_from_pem(key, cert)?.into_parts();
43+
Ok(Identity { der, pass })
3344
}
3445

3546
/// Wraps [`reqwest::Identity::from_pkcs12_der`].

src/storage-types/src/connections.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2006,9 +2006,8 @@ impl MySqlConnection<InlinedConnection> {
20062006
.read_string_in_task_if(in_task, identity.key)
20072007
.await?;
20082008
let cert = identity.cert.get_string(in_task, secrets_reader).await?;
2009-
let mut archive = mz_tls_util::pkcs12der_from_pem(key.as_bytes(), cert.as_bytes())?;
2010-
let der = std::mem::take(&mut archive.der);
2011-
let pass = std::mem::take(&mut archive.pass);
2009+
let (der, pass) =
2010+
mz_tls_util::pkcs12der_from_pem(key.as_bytes(), cert.as_bytes())?.into_parts();
20122011

20132012
// Add client identity to SSLOpts
20142013
ssl_opts = ssl_opts.map(|opts| {

src/tls-util/src/lib.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,15 @@ impl Drop for Pkcs12Archive {
114114
}
115115
}
116116

117+
impl Pkcs12Archive {
118+
pub fn into_parts(self) -> (Vec<u8>, String) {
119+
let mut md = std::mem::ManuallyDrop::new(self);
120+
let der = std::mem::take(&mut md.der);
121+
let pass = std::mem::take(&mut md.pass);
122+
(der, pass)
123+
}
124+
}
125+
117126
/// Constructs an identity from a PEM-formatted key and certificate using OpenSSL.
118127
pub fn pkcs12der_from_pem(
119128
key: &[u8],

0 commit comments

Comments
 (0)