Skip to content

Commit

Permalink
[sign] Use 'secrecy' to protect private keys
Browse files Browse the repository at this point in the history
  • Loading branch information
arya dradjica committed Nov 4, 2024
1 parent 354bf0a commit ca10361
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 95 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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"]
Expand Down
104 changes: 40 additions & 64 deletions src/sign/bytes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use core::{fmt, str};

use secrecy::{ExposeSecret, SecretBox};
use std::boxed::Box;
use std::vec::Vec;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
}
}
Expand All @@ -182,7 +187,7 @@ impl SecretKeyBytes {
/// Parse private keys for most algorithms (except RSA).
fn parse_pkey<const N: usize>(
mut data: &str,
) -> Result<Box<[u8; N]>, BindFormatError> {
) -> Result<SecretBox<[u8; N]>, BindFormatError> {
// Look for the 'PrivateKey' field.
while let Some((key, val, rest)) = parse_bind_entry(data)? {
data = rest;
Expand All @@ -191,11 +196,15 @@ impl SecretKeyBytes {
continue;
}

return base64::decode::<Vec<u8>>(val)
.map_err(|_| BindFormatError::Misformatted)?
.into_boxed_slice()
// TODO: Evaluate security of 'base64::decode()'.
let val: Vec<u8> = 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.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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(())
}

Expand Down Expand Up @@ -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(),
})
}
}
Expand All @@ -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.
Expand Down
37 changes: 22 additions & 15 deletions src/sign/openssl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@
#![cfg_attr(docsrs, doc(cfg(feature = "openssl")))]

use core::fmt;
use std::vec::Vec;
use std::{boxed::Box, vec::Vec};

use openssl::{
bn::BigNum,
ecdsa::EcdsaSig,
error::ErrorStack,
pkey::{self, PKey, Private},
};
use secrecy::ExposeSecret;

use crate::{
base::iana::SecAlg,
Expand Down Expand Up @@ -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
Expand All @@ -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)?;
Expand All @@ -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)?;
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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!(),
}
Expand Down
Loading

0 comments on commit ca10361

Please sign in to comment.