From 2fb08dbb57bf69f054ddc797390c9884d6d7cee0 Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 10 Feb 2026 21:54:24 +0000 Subject: [PATCH 1/2] Add missing error handling in ML-KEM generate --- boring/src/mlkem.rs | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/boring/src/mlkem.rs b/boring/src/mlkem.rs index f5b84147e..e99613928 100644 --- a/boring/src/mlkem.rs +++ b/boring/src/mlkem.rs @@ -91,14 +91,14 @@ impl MlKemPrivateKey { pub fn generate(algorithm: Algorithm) -> Result<(MlKemPublicKey, MlKemPrivateKey), ErrorStack> { match algorithm { Algorithm::MlKem768 => { - let (pk, sk) = MlKem768PrivateKey::generate(); + let (pk, sk) = MlKem768PrivateKey::generate()?; Ok(( MlKemPublicKey(Either::MlKem768(pk)), MlKemPrivateKey(Either::MlKem768(sk)), )) } Algorithm::MlKem1024 => { - let (pk, sk) = MlKem1024PrivateKey::generate(); + let (pk, sk) = MlKem1024PrivateKey::generate()?; Ok(( MlKemPublicKey(Either::MlKem1024(pk)), MlKemPrivateKey(Either::MlKem1024(sk)), @@ -222,8 +222,7 @@ impl MlKem768PrivateKey { pub const CIPHERTEXT_BYTES: usize = ffi::MLKEM768_CIPHERTEXT_BYTES as usize; /// Generate a new key pair. - #[must_use] - fn generate() -> (Box, Box) { + fn generate() -> Result<(Box, Box), ErrorStack> { // SAFETY: all buffers are out parameters, correctly sized unsafe { ffi::init(); @@ -243,9 +242,12 @@ impl MlKem768PrivateKey { // Parse the public key bytes to get the parsed struct let mut cbs = cbs_init(&bytes); let mut parsed: MaybeUninit = MaybeUninit::uninit(); - ffi::MLKEM768_parse_public_key(parsed.as_mut_ptr(), &mut cbs); + cvt(ffi::MLKEM768_parse_public_key( + parsed.as_mut_ptr(), + &mut cbs, + ))?; - ( + Ok(( Box::new(MlKem768PublicKey { bytes, parsed: parsed.assume_init(), @@ -254,7 +256,7 @@ impl MlKem768PrivateKey { seed: seed.assume_init(), expanded: expanded.assume_init(), }), - ) + )) } } @@ -439,8 +441,7 @@ impl MlKem1024PrivateKey { pub const CIPHERTEXT_BYTES: usize = ffi::MLKEM1024_CIPHERTEXT_BYTES as usize; /// Generate a new key pair. - #[must_use] - fn generate() -> (Box, Box) { + fn generate() -> Result<(Box, Box), ErrorStack> { // SAFETY: all buffers are out parameters, correctly sized unsafe { ffi::init(); @@ -460,9 +461,12 @@ impl MlKem1024PrivateKey { // Parse the public key bytes to get the parsed struct let mut cbs = cbs_init(&bytes); let mut parsed: MaybeUninit = MaybeUninit::uninit(); - ffi::MLKEM1024_parse_public_key(parsed.as_mut_ptr(), &mut cbs); + cvt(ffi::MLKEM1024_parse_public_key( + parsed.as_mut_ptr(), + &mut cbs, + ))?; - ( + Ok(( Box::new(MlKem1024PublicKey { bytes, parsed: parsed.assume_init(), @@ -471,7 +475,7 @@ impl MlKem1024PrivateKey { seed: seed.assume_init(), expanded: expanded.assume_init(), }), - ) + )) } } @@ -650,14 +654,14 @@ mod tests { #[test] fn roundtrip() { let (pk, sk) = <$priv>::generate(); - let (ct, ss1) = pk.encapsulate(); + let (ct, ss1) = pk.encapsulate().unwrap(); let ss2 = sk.decapsulate(&ct); assert_eq!(ss1, ss2); } #[test] fn seed_roundtrip() { - let (pk, sk) = <$priv>::generate(); + let (pk, sk) = <$priv>::generate().unwrap(); let sk2 = <$priv>::from_seed(&sk.seed).unwrap(); let (ct, ss1) = pk.encapsulate(); let ss2 = sk2.decapsulate(&ct); @@ -666,7 +670,7 @@ mod tests { #[test] fn derive_pubkey() { - let (pk, sk) = <$priv>::generate(); + let (pk, sk) = <$priv>::generate().unwrap(); assert_eq!(pk.bytes, sk.public_key().unwrap().bytes); } @@ -678,14 +682,14 @@ mod tests { #[test] fn from_slice_roundtrip() { - let (pk, _) = <$priv>::generate(); + let (pk, _) = <$priv>::generate().unwrap(); let pk2 = <$pub>::from_slice(&pk.bytes).unwrap(); assert_eq!(pk.bytes, pk2.bytes); } #[test] fn implicit_rejection() { - let (_, sk) = <$priv>::generate(); + let (_, sk) = <$priv>::generate().unwrap(); let bad_ct = [0x42u8; $ct_len]; // bad ciphertext still "works", just returns deterministic garbage let ss1 = sk.decapsulate(&bad_ct); @@ -695,7 +699,7 @@ mod tests { #[test] fn debug_redacts_seed() { - let (_, sk) = <$priv>::generate(); + let (_, sk) = <$priv>::generate().unwrap(); let dbg = format!("{:?}", sk); assert!(dbg.contains("redacted")); } From 3d2cafee59fa057ddc098f33247f44622fc3e07b Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 10 Feb 2026 22:54:37 +0000 Subject: [PATCH 2/2] Ensure we don't leave unit memory if generate_key fails --- boring/src/mlkem.rs | 56 ++++++++++++--------------------------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/boring/src/mlkem.rs b/boring/src/mlkem.rs index e99613928..ead96b86e 100644 --- a/boring/src/mlkem.rs +++ b/boring/src/mlkem.rs @@ -226,34 +226,20 @@ impl MlKem768PrivateKey { // SAFETY: all buffers are out parameters, correctly sized unsafe { ffi::init(); - let mut public_key_bytes: MaybeUninit<[u8; MlKem768PublicKey::PUBLIC_KEY_BYTES]> = - MaybeUninit::uninit(); - let mut seed: MaybeUninit = MaybeUninit::uninit(); + let mut bytes = [0; MlKem768PublicKey::PUBLIC_KEY_BYTES]; + let mut seed = [0; PRIVATE_KEY_SEED_BYTES]; let mut expanded: MaybeUninit = MaybeUninit::uninit(); ffi::MLKEM768_generate_key( - public_key_bytes.as_mut_ptr().cast(), - seed.as_mut_ptr().cast(), + bytes.as_mut_ptr().cast(), + seed.as_mut_ptr(), expanded.as_mut_ptr(), ); - let bytes = public_key_bytes.assume_init(); - - // Parse the public key bytes to get the parsed struct - let mut cbs = cbs_init(&bytes); - let mut parsed: MaybeUninit = MaybeUninit::uninit(); - cvt(ffi::MLKEM768_parse_public_key( - parsed.as_mut_ptr(), - &mut cbs, - ))?; - Ok(( - Box::new(MlKem768PublicKey { - bytes, - parsed: parsed.assume_init(), - }), + Box::new(MlKem768PublicKey::from_slice(&bytes)?), Box::new(MlKem768PrivateKey { - seed: seed.assume_init(), + seed, expanded: expanded.assume_init(), }), )) @@ -445,34 +431,20 @@ impl MlKem1024PrivateKey { // SAFETY: all buffers are out parameters, correctly sized unsafe { ffi::init(); - let mut public_key_bytes: MaybeUninit<[u8; MlKem1024PublicKey::PUBLIC_KEY_BYTES]> = - MaybeUninit::uninit(); - let mut seed: MaybeUninit = MaybeUninit::uninit(); + let mut bytes = [0; MlKem1024PublicKey::PUBLIC_KEY_BYTES]; + let mut seed = [0; PRIVATE_KEY_SEED_BYTES]; let mut expanded: MaybeUninit = MaybeUninit::uninit(); ffi::MLKEM1024_generate_key( - public_key_bytes.as_mut_ptr().cast(), - seed.as_mut_ptr().cast(), + bytes.as_mut_ptr().cast(), + seed.as_mut_ptr(), expanded.as_mut_ptr(), ); - let bytes = public_key_bytes.assume_init(); - - // Parse the public key bytes to get the parsed struct - let mut cbs = cbs_init(&bytes); - let mut parsed: MaybeUninit = MaybeUninit::uninit(); - cvt(ffi::MLKEM1024_parse_public_key( - parsed.as_mut_ptr(), - &mut cbs, - ))?; - Ok(( - Box::new(MlKem1024PublicKey { - bytes, - parsed: parsed.assume_init(), - }), + Box::new(MlKem1024PublicKey::from_slice(&bytes)?), Box::new(MlKem1024PrivateKey { - seed: seed.assume_init(), + seed, expanded: expanded.assume_init(), }), )) @@ -653,8 +625,8 @@ mod tests { #[test] fn roundtrip() { - let (pk, sk) = <$priv>::generate(); - let (ct, ss1) = pk.encapsulate().unwrap(); + let (pk, sk) = <$priv>::generate().unwrap(); + let (ct, ss1) = pk.encapsulate(); let ss2 = sk.decapsulate(&ct); assert_eq!(ss1, ss2); }