From 26d2e2d78614d2a9b213e38522f3b08e7e268a15 Mon Sep 17 00:00:00 2001 From: Steffen Eiden Date: Tue, 21 Jan 2025 13:03:18 +0100 Subject: [PATCH] rust/pv: Fix padding of retrievable private keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the padding of keys that are smaller than the architected slot for the retrievable key. Previously the keys where appended with zeros. However, processing software expects a left-padding. Affected key types: * Ed448 * SecP521 While at it, fix documentation in the retrievable key struct. Fixes: fd024387d710 ("rust/pv: Retrievable secrets support") Acked-by: Ingo Franzki Signed-off-by: Steffen Eiden Signed-off-by: Jan Höppner --- rust/pv/src/error.rs | 3 + rust/pv/src/uvsecret/guest_secret.rs | 71 +++++++++++++++++------- rust/pv_core/src/uvdevice/retr_secret.rs | 11 +--- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/rust/pv/src/error.rs b/rust/pv/src/error.rs index 68466448..73bc6200 100644 --- a/rust/pv/src/error.rs +++ b/rust/pv/src/error.rs @@ -117,6 +117,9 @@ pub enum Error { exp: String, }, + #[error("Invalid data from OpenSSL")] + InvalSslData, + // errors from other crates #[error(transparent)] PvCore(#[from] pv_core::Error), diff --git a/rust/pv/src/uvsecret/guest_secret.rs b/rust/pv/src/uvsecret/guest_secret.rs index 87b58bb8..3f3b6d1a 100644 --- a/rust/pv/src/uvsecret/guest_secret.rs +++ b/rust/pv/src/uvsecret/guest_secret.rs @@ -21,7 +21,7 @@ use byteorder::BigEndian; use openssl::{ hash::MessageDigest, nid::Nid, - pkey::{Id, PKey, Private}, + pkey::{Id, PKey, PKeyRef, Private}, }; use pv_core::static_assert; use serde::{Deserialize, Serialize}; @@ -61,7 +61,7 @@ pub enum GuestSecret { name: String, /// SHA256 hash of [`GuestSecret::RetrievableKey::name`] id: SecretId, - /// Confidential actual retrievable secret (32 bytes) + /// Confidential actual retrievable secret #[serde(skip)] secret: Confidential>, }, @@ -293,20 +293,26 @@ fn hmac_sha(key: Confidential>) -> Result { /// Get an EC-private-key fn ec(key: PKey) -> Result { - let (key, nid) = match key.id() { - Id::EC => { - let ec_key = key.ec_key()?; - let key = ec_key.private_key().to_vec(); - let nid = ec_key.group().curve_name().unwrap_or(Nid::UNDEF); - (key, nid) + // reads & left-pads Edward EC keys + fn pad_ed_key(pkey: &PKeyRef, curve: &EcCurves) -> Result> { + let raw_key = pkey.raw_private_key()?; + + match raw_key.len().cmp(&curve.exp_key_size()) { + std::cmp::Ordering::Less => { + let mut key = Vec::with_capacity(curve.exp_key_size()); + key.extend_from_slice(&vec![0u8; curve.exp_key_size() - raw_key.len()]); + key.extend_from_slice(&raw_key); + Ok(key) + } + std::cmp::Ordering::Equal => Ok(raw_key), + std::cmp::Ordering::Greater => Err(Error::InvalSslData), } - // ED keys are not handled via the EC struct in OpenSSL. - id @ (Id::ED25519 | Id::ED448) => { - let key = key.raw_private_key()?; - let nid = Nid::from_raw(id.as_raw()); - (key, nid) - } - _ => (vec![], Nid::UNDEF), + } + + let nid = match key.id() { + Id::EC => key.ec_key()?.group().curve_name().unwrap_or(Nid::UNDEF), + id @ (Id::ED25519 | Id::ED448) => Nid::from_raw(id.as_raw()), + _ => Nid::UNDEF, }; let kind = match nid { @@ -327,7 +333,16 @@ fn ec(key: PKey) -> Result { } }; - let key = kind.resize_raw_key(key); + let key = match key.id() { + Id::EC => key + .ec_key()? + .private_key() + .to_vec_padded(kind.exp_key_size() as i32)?, + // ED keys are not handled via the EC struct in OpenSSL. + Id::ED25519 | Id::ED448 => pad_ed_key(&key, &kind)?, + _ => unreachable!(), + }; + Ok((RetrievableSecret::Ec(kind), key.into())) } @@ -484,16 +499,19 @@ mod test { assert_eq!(secret, exp); } + #[track_caller] + fn gen_ec(nid: Nid) -> PKey { + let group = EcGroup::from_curve_name(nid).unwrap(); + let key = EcKey::generate(&group).unwrap(); + PKey::from_ec_key(key).unwrap() + } + #[track_caller] fn test_ec(grp: Nid, exp_kind: EcCurves, exp_len: usize) { let key = match grp { NID_ED25519 => PKey::generate_ed25519().unwrap(), NID_ED448 => PKey::generate_ed448().unwrap(), - nid => { - let group = EcGroup::from_curve_name(nid).unwrap(); - let key = EcKey::generate(&group).unwrap(); - PKey::from_ec_key(key).unwrap() - } + nid => gen_ec(nid), }; let (kind, key) = ec(key).unwrap(); @@ -510,6 +528,17 @@ mod test { test_ec(NID_ED448, EcCurves::Ed448, 64); } + #[test] + fn retr_ec_pad() { + let pkey = PKey::generate_ed448().unwrap(); + let (_, key) = ec(pkey).unwrap(); + assert_eq!(key.value()[..7], [0; 7]); + + let pkey = gen_ec(Nid::SECP521R1); + let (_, key) = ec(pkey).unwrap(); + assert_eq!(key.value()[..14], [0; 14]); + } + #[test] fn asc_parse() { let id = [ diff --git a/rust/pv_core/src/uvdevice/retr_secret.rs b/rust/pv_core/src/uvdevice/retr_secret.rs index 490152b4..b818d245 100644 --- a/rust/pv_core/src/uvdevice/retr_secret.rs +++ b/rust/pv_core/src/uvdevice/retr_secret.rs @@ -146,7 +146,8 @@ pub enum EcCurves { } impl EcCurves { - const fn exp_size(&self) -> usize { + /// Returns the expected key-byte-size for this curve. + pub const fn exp_key_size(&self) -> usize { match self { Self::Secp256R1 => 32, Self::Secp384R1 => 48, @@ -155,14 +156,6 @@ impl EcCurves { Self::Ed448 => 64, } } - - /// Resizes the raw key to the expected size. - /// - /// See [`Vec::resize`] - pub fn resize_raw_key(&self, mut raw: Vec) -> Vec { - raw.resize(self.exp_size(), 0); - raw - } } // The names have to stay constant, otherwise the PEM contains invalid types