diff --git a/Cargo.lock b/Cargo.lock index eaf9191f..ca7fb4b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -240,6 +240,7 @@ dependencies = [ "rstest", "rustls-pemfile", "rustversion", + "secrecy", "serde", "serde_json", "serde_test", @@ -1027,6 +1028,15 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "secrecy" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" +dependencies = [ + "zeroize", +] + [[package]] name = "semver" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index 8eff4e59..4c60ad9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ openssl = { version = "0.10.57", optional = true } # 0.10.57 upgrades to proc-macro2 = { version = "1.0.69", optional = true } # Force proc-macro2 to at least 1.0.69 for minimal-version build ring = { version = "0.17", optional = true } rustversion = { version = "1", optional = true } +secrecy = { version = "0.10", optional = true } serde = { version = "1.0.130", optional = true, features = ["derive"] } siphasher = { version = "1", optional = true } smallvec = { version = "1.3", optional = true } @@ -70,7 +71,7 @@ zonefile = ["bytes", "serde", "std"] # Unstable features unstable-client-transport = ["moka", "net", "tracing"] unstable-server-transport = ["arc-swap", "chrono/clock", "libc", "net", "siphasher", "tracing"] -unstable-sign = ["std", "unstable-validate"] +unstable-sign = ["std", "dep:secrecy", "unstable-validate"] unstable-stelline = ["tokio/test-util", "tracing", "tracing-subscriber", "tsig", "unstable-client-transport", "unstable-server-transport", "zonefile"] unstable-validate = ["bytes", "std", "ring"] unstable-validator = ["unstable-validate", "zonefile", "unstable-client-transport"] diff --git a/src/sign/bytes.rs b/src/sign/bytes.rs index 3326ee08..6393a0ac 100644 --- a/src/sign/bytes.rs +++ b/src/sign/bytes.rs @@ -2,6 +2,7 @@ use core::{fmt, str}; +use secrecy::{ExposeSecret, SecretBox}; use std::boxed::Box; use std::vec::Vec; @@ -89,22 +90,22 @@ pub enum SecretKeyBytes { /// An ECDSA P-256/SHA-256 keypair. /// /// The private key is a single 32-byte big-endian integer. - EcdsaP256Sha256(Box<[u8; 32]>), + EcdsaP256Sha256(SecretBox<[u8; 32]>), /// An ECDSA P-384/SHA-384 keypair. /// /// The private key is a single 48-byte big-endian integer. - EcdsaP384Sha384(Box<[u8; 48]>), + EcdsaP384Sha384(SecretBox<[u8; 48]>), /// An Ed25519 keypair. /// /// The private key is a single 32-byte string. - Ed25519(Box<[u8; 32]>), + Ed25519(SecretBox<[u8; 32]>), /// An Ed448 keypair. /// /// The private key is a single 57-byte string. - Ed448(Box<[u8; 57]>), + Ed448(SecretBox<[u8; 57]>), } //--- Inspection @@ -139,23 +140,27 @@ impl SecretKeyBytes { } Self::EcdsaP256Sha256(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 13 (ECDSAP256SHA256)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } Self::EcdsaP384Sha384(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 14 (ECDSAP384SHA384)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } Self::Ed25519(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 15 (ED25519)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } Self::Ed448(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 16 (ED448)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } } } @@ -182,7 +187,7 @@ impl SecretKeyBytes { /// Parse private keys for most algorithms (except RSA). fn parse_pkey( mut data: &str, - ) -> Result, BindFormatError> { + ) -> Result, BindFormatError> { // Look for the 'PrivateKey' field. while let Some((key, val, rest)) = parse_bind_entry(data)? { data = rest; @@ -191,11 +196,15 @@ impl SecretKeyBytes { continue; } - return base64::decode::>(val) - .map_err(|_| BindFormatError::Misformatted)? - .into_boxed_slice() + // TODO: Evaluate security of 'base64::decode()'. + let val: Vec = base64::decode(val) + .map_err(|_| BindFormatError::Misformatted)?; + let val: Box<[u8]> = val.into_boxed_slice(); + let val: Box<[u8; N]> = val .try_into() - .map_err(|_| BindFormatError::Misformatted); + .map_err(|_| BindFormatError::Misformatted)?; + + return Ok(val.into()); } // The 'PrivateKey' field was not found. @@ -245,22 +254,6 @@ impl SecretKeyBytes { } } -//--- Drop - -impl Drop for SecretKeyBytes { - /// Securely clear the secret key bytes from memory. - fn drop(&mut self) { - // Zero the bytes for each field. - match self { - Self::RsaSha256(_) => {} - Self::EcdsaP256Sha256(s) => s.fill(0), - Self::EcdsaP384Sha384(s) => s.fill(0), - Self::Ed25519(s) => s.fill(0), - Self::Ed448(s) => s.fill(0), - } - } -} - //----------- RsaSecretKeyBytes --------------------------------------------------- /// An RSA secret key expressed as raw bytes. @@ -276,22 +269,22 @@ pub struct RsaSecretKeyBytes { pub e: Box<[u8]>, /// The private exponent. - pub d: Box<[u8]>, + pub d: SecretBox<[u8]>, /// The first prime factor of `d`. - pub p: Box<[u8]>, + pub p: SecretBox<[u8]>, /// The second prime factor of `d`. - pub q: Box<[u8]>, + pub q: SecretBox<[u8]>, /// The exponent corresponding to the first prime factor of `d`. - pub d_p: Box<[u8]>, + pub d_p: SecretBox<[u8]>, /// The exponent corresponding to the second prime factor of `d`. - pub d_q: Box<[u8]>, + pub d_q: SecretBox<[u8]>, /// The inverse of the second prime factor modulo the first. - pub q_i: Box<[u8]>, + pub q_i: SecretBox<[u8]>, } //--- Conversion to and from the BIND format @@ -309,17 +302,17 @@ impl RsaSecretKeyBytes { w.write_str("PublicExponent: ")?; writeln!(w, "{}", base64::encode_display(&self.e))?; w.write_str("PrivateExponent: ")?; - writeln!(w, "{}", base64::encode_display(&self.d))?; + writeln!(w, "{}", base64::encode_display(&self.d.expose_secret()))?; w.write_str("Prime1: ")?; - writeln!(w, "{}", base64::encode_display(&self.p))?; + writeln!(w, "{}", base64::encode_display(&self.p.expose_secret()))?; w.write_str("Prime2: ")?; - writeln!(w, "{}", base64::encode_display(&self.q))?; + writeln!(w, "{}", base64::encode_display(&self.q.expose_secret()))?; w.write_str("Exponent1: ")?; - writeln!(w, "{}", base64::encode_display(&self.d_p))?; + writeln!(w, "{}", base64::encode_display(&self.d_p.expose_secret()))?; w.write_str("Exponent2: ")?; - writeln!(w, "{}", base64::encode_display(&self.d_q))?; + writeln!(w, "{}", base64::encode_display(&self.d_q.expose_secret()))?; w.write_str("Coefficient: ")?; - writeln!(w, "{}", base64::encode_display(&self.q_i))?; + writeln!(w, "{}", base64::encode_display(&self.q_i.expose_secret()))?; Ok(()) } @@ -390,12 +383,12 @@ impl RsaSecretKeyBytes { Ok(Self { n: n.unwrap(), e: e.unwrap(), - d: d.unwrap(), - p: p.unwrap(), - q: q.unwrap(), - d_p: d_p.unwrap(), - d_q: d_q.unwrap(), - q_i: q_i.unwrap(), + d: d.unwrap().into(), + p: p.unwrap().into(), + q: q.unwrap().into(), + d_p: d_p.unwrap().into(), + d_q: d_q.unwrap().into(), + q_i: q_i.unwrap().into(), }) } } @@ -411,23 +404,6 @@ impl<'a> From<&'a RsaSecretKeyBytes> for RsaPublicKeyBytes { } } -//--- Drop - -impl Drop for RsaSecretKeyBytes { - /// Securely clear the secret key bytes from memory. - fn drop(&mut self) { - // Zero the bytes for each field. - self.n.fill(0u8); - self.e.fill(0u8); - self.d.fill(0u8); - self.p.fill(0u8); - self.q.fill(0u8); - self.d_p.fill(0u8); - self.d_q.fill(0u8); - self.q_i.fill(0u8); - } -} - //----------- Helpers for parsing the BIND format ---------------------------- /// Extract the next key-value pair in a BIND-format private key file. diff --git a/src/sign/openssl.rs b/src/sign/openssl.rs index 85257137..a7250081 100644 --- a/src/sign/openssl.rs +++ b/src/sign/openssl.rs @@ -12,7 +12,7 @@ #![cfg_attr(docsrs, doc(cfg(feature = "openssl")))] use core::fmt; -use std::vec::Vec; +use std::{boxed::Box, vec::Vec}; use openssl::{ bn::BigNum, @@ -20,6 +20,7 @@ use openssl::{ error::ErrorStack, pkey::{self, PKey, Private}, }; +use secrecy::ExposeSecret; use crate::{ base::iana::SecAlg, @@ -70,12 +71,12 @@ impl KeyPair { let n = num(&s.n)?; let e = num(&s.e)?; - let d = secure_num(&s.d)?; - let p = secure_num(&s.p)?; - let q = secure_num(&s.q)?; - let d_p = secure_num(&s.d_p)?; - let d_q = secure_num(&s.d_q)?; - let q_i = secure_num(&s.q_i)?; + let d = secure_num(s.d.expose_secret())?; + let p = secure_num(s.p.expose_secret())?; + let q = secure_num(s.q.expose_secret())?; + let d_p = secure_num(s.d_p.expose_secret())?; + let d_q = secure_num(s.d_q.expose_secret())?; + let q_i = secure_num(s.q_i.expose_secret())?; // NOTE: The 'openssl' crate doesn't seem to expose // 'EVP_PKEY_fromdata', which could be used to replace the @@ -101,7 +102,7 @@ impl KeyPair { let mut ctx = bn::BigNumContext::new_secure()?; let group = nid::Nid::X9_62_PRIME256V1; let group = ec::EcGroup::from_curve_name(group)?; - let n = secure_num(s.as_slice())?; + let n = secure_num(s.expose_secret().as_slice())?; let p = ec::EcPoint::from_bytes(&group, &**p, &mut ctx)?; let k = ec::EcKey::from_private_components(&group, &n, &p)?; k.check_key().map_err(|_| FromBytesError::InvalidKey)?; @@ -117,7 +118,7 @@ impl KeyPair { let mut ctx = bn::BigNumContext::new_secure()?; let group = nid::Nid::SECP384R1; let group = ec::EcGroup::from_curve_name(group)?; - let n = secure_num(s.as_slice())?; + let n = secure_num(s.expose_secret().as_slice())?; let p = ec::EcPoint::from_bytes(&group, &**p, &mut ctx)?; let k = ec::EcKey::from_private_components(&group, &n, &p)?; k.check_key().map_err(|_| FromBytesError::InvalidKey)?; @@ -128,7 +129,8 @@ impl KeyPair { use openssl::memcmp; let id = pkey::Id::ED25519; - let k = PKey::private_key_from_raw_bytes(&**s, id)?; + let s = s.expose_secret(); + let k = PKey::private_key_from_raw_bytes(s, id)?; if memcmp::eq(&k.raw_public_key().unwrap(), &**p) { k } else { @@ -140,7 +142,8 @@ impl KeyPair { use openssl::memcmp; let id = pkey::Id::ED448; - let k = PKey::private_key_from_raw_bytes(&**s, id)?; + let s = s.expose_secret(); + let k = PKey::private_key_from_raw_bytes(s, id)?; if memcmp::eq(&k.raw_public_key().unwrap(), &**p) { k } else { @@ -182,20 +185,24 @@ impl KeyPair { SecAlg::ECDSAP256SHA256 => { let key = self.pkey.ec_key().unwrap(); let key = key.private_key().to_vec_padded(32).unwrap(); - SecretKeyBytes::EcdsaP256Sha256(key.try_into().unwrap()) + let key: Box<[u8; 32]> = key.try_into().unwrap(); + SecretKeyBytes::EcdsaP256Sha256(key.into()) } SecAlg::ECDSAP384SHA384 => { let key = self.pkey.ec_key().unwrap(); let key = key.private_key().to_vec_padded(48).unwrap(); - SecretKeyBytes::EcdsaP384Sha384(key.try_into().unwrap()) + let key: Box<[u8; 48]> = key.try_into().unwrap(); + SecretKeyBytes::EcdsaP384Sha384(key.into()) } SecAlg::ED25519 => { let key = self.pkey.raw_private_key().unwrap(); - SecretKeyBytes::Ed25519(key.try_into().unwrap()) + let key: Box<[u8; 32]> = key.try_into().unwrap(); + SecretKeyBytes::Ed25519(key.into()) } SecAlg::ED448 => { let key = self.pkey.raw_private_key().unwrap(); - SecretKeyBytes::Ed448(key.try_into().unwrap()) + let key: Box<[u8; 57]> = key.try_into().unwrap(); + SecretKeyBytes::Ed448(key.into()) } _ => unreachable!(), } diff --git a/src/sign/ring.rs b/src/sign/ring.rs index 3b97cf00..d1e29c39 100644 --- a/src/sign/ring.rs +++ b/src/sign/ring.rs @@ -16,6 +16,7 @@ use std::{boxed::Box, sync::Arc, vec::Vec}; use ring::signature::{ EcdsaKeyPair, Ed25519KeyPair, KeyPair as _, RsaKeyPair, }; +use secrecy::ExposeSecret; use crate::{ base::iana::SecAlg, @@ -76,12 +77,12 @@ impl KeyPair { n: s.n.as_ref(), e: s.e.as_ref(), }, - d: s.d.as_ref(), - p: s.p.as_ref(), - q: s.q.as_ref(), - dP: s.d_p.as_ref(), - dQ: s.d_q.as_ref(), - qInv: s.q_i.as_ref(), + d: s.d.expose_secret(), + p: s.p.expose_secret(), + q: s.q.expose_secret(), + dP: s.d_p.expose_secret(), + dQ: s.d_q.expose_secret(), + qInv: s.q_i.expose_secret(), }; ring::signature::RsaKeyPair::from_components(&components) .map_err(|_| FromBytesError::InvalidKey) @@ -95,7 +96,7 @@ impl KeyPair { let alg = &ring::signature::ECDSA_P256_SHA256_FIXED_SIGNING; EcdsaKeyPair::from_private_key_and_public_key( alg, - s.as_slice(), + s.expose_secret(), p.as_slice(), &*rng, ) @@ -110,7 +111,7 @@ impl KeyPair { let alg = &ring::signature::ECDSA_P384_SHA384_FIXED_SIGNING; EcdsaKeyPair::from_private_key_and_public_key( alg, - s.as_slice(), + s.expose_secret(), p.as_slice(), &*rng, ) @@ -120,7 +121,7 @@ impl KeyPair { (SecretKeyBytes::Ed25519(s), PublicKeyBytes::Ed25519(p)) => { Ed25519KeyPair::from_seed_and_public_key( - s.as_slice(), + s.expose_secret(), p.as_slice(), ) .map_err(|_| FromBytesError::InvalidKey) @@ -240,8 +241,8 @@ pub fn generate( // Manually parse the PKCS#8 document for the private key. let sk: Box<[u8]> = Box::from(&doc.as_ref()[36..68]); - let sk = sk.try_into().unwrap(); - let sk = SecretKeyBytes::EcdsaP256Sha256(sk); + let sk: Box<[u8; 32]> = sk.try_into().unwrap(); + let sk = SecretKeyBytes::EcdsaP256Sha256(sk.into()); // Manually parse the PKCS#8 document for the public key. let pk: Box<[u8]> = Box::from(&doc.as_ref()[73..138]); @@ -258,8 +259,8 @@ pub fn generate( // Manually parse the PKCS#8 document for the private key. let sk: Box<[u8]> = Box::from(&doc.as_ref()[35..83]); - let sk = sk.try_into().unwrap(); - let sk = SecretKeyBytes::EcdsaP384Sha384(sk); + let sk: Box<[u8; 48]> = sk.try_into().unwrap(); + let sk = SecretKeyBytes::EcdsaP384Sha384(sk.into()); // Manually parse the PKCS#8 document for the public key. let pk: Box<[u8]> = Box::from(&doc.as_ref()[88..185]); @@ -275,8 +276,8 @@ pub fn generate( // Manually parse the PKCS#8 document for the private key. let sk: Box<[u8]> = Box::from(&doc.as_ref()[16..48]); - let sk = sk.try_into().unwrap(); - let sk = SecretKeyBytes::Ed25519(sk); + let sk: Box<[u8; 32]> = sk.try_into().unwrap(); + let sk = SecretKeyBytes::Ed25519(sk.into()); // Manually parse the PKCS#8 document for the public key. let pk: Box<[u8]> = Box::from(&doc.as_ref()[51..83]);