From 0c1cfc5c9526466d4e043ee68b00666a9ccdcb9f Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Mon, 2 Nov 2026 12:13:11 -0700 Subject: [PATCH] Minimum modulus size checks These were originally added in #526 and then subsequently reverted due to flaky tests. The checks were partially restored in #576 which applied to keygen, however this applies the checks generally. Closes #445 --- checkct/driver/src/raw_rsa.rs | 6 +- src/key.rs | 67 +++++++++---- src/oaep/encrypting_key.rs | 17 ++-- src/pkcs1v15/encrypting_key.rs | 17 ++-- src/pkcs1v15/verifying_key.rs | 16 ++- src/pss/verifying_key.rs | 16 ++- tests/examples/pkcs8/rsa2048-pub.hex | 1 + tests/key_size.rs | 140 +++++++++++++++++++++++++++ 8 files changed, 220 insertions(+), 60 deletions(-) create mode 100644 tests/examples/pkcs8/rsa2048-pub.hex create mode 100644 tests/key_size.rs diff --git a/checkct/driver/src/raw_rsa.rs b/checkct/driver/src/raw_rsa.rs index 8b2bb6f4..7d85636e 100644 --- a/checkct/driver/src/raw_rsa.rs +++ b/checkct/driver/src/raw_rsa.rs @@ -21,7 +21,8 @@ pub fn rsa_decrypt_unblinded_non_crt_128() { // Public modulus (2^64 - 59) * (2^64 - 83), with fixed 128-bit precision. let n = BoxedUint::from_be_hex("FFFFFFFFFFFFFF720000000000001321", 128).unwrap(); - let public = RsaPublicKey::new(n, BoxedUint::from(65537u64)).unwrap(); + let public = + RsaPublicKey::new_with_size_limits(n, BoxedUint::from(65537u64), 128..=128).unwrap(); let mut bytes = [0u8; 16]; PrivateRng.fill_bytes(&mut bytes); @@ -107,7 +108,8 @@ pub fn rsa_decrypt_unblinded_crt_128() { unsafe { crate::allocator::reset() }; let n = BoxedUint::from_be_hex("FFFFFFFFFFFFFF720000000000001321", 128).unwrap(); - let public = RsaPublicKey::new(n, BoxedUint::from(65537u64)).unwrap(); + let public = + RsaPublicKey::new_with_size_limits(n, BoxedUint::from(65537u64), 128..=128).unwrap(); let p = secret_modulus_64(); let q = secret_modulus_64(); let primes = vec![p.modulus().clone(), q.modulus().clone()]; diff --git a/src/key.rs b/src/key.rs index 9f0e6881..1b2cccac 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1,6 +1,7 @@ use alloc::vec::Vec; use core::fmt; use core::hash::{Hash, Hasher}; +use core::ops::RangeInclusive; use crypto_bigint::{ BoxedUint, ConcatenatingMul, Integer, NonZero, Odd, Resize, @@ -232,26 +233,51 @@ impl RsaPublicKey { /// If for some reason you have a legitimate reason to use keys with public exponents larger /// than this value, use the special APIs: /// - /// - [`RsaPublicKey::new_with_max_size`] + /// - [`RsaPublicKey::new_unchecked`] /// - [`RsaPrivateKey::from_components_with_large_exponent`] pub const MAX_PUB_EXPONENT: u64 = (1 << 33) - 1; - /// Maximum size of the modulus `n` in bits. + /// Default minimum size of the modulus `n` in bits. + pub const MIN_SIZE: usize = 1024; + + /// Default maximum size of the modulus `n` in bits. pub const MAX_SIZE: usize = 8192; /// Create a new public key from its components. /// - /// This function accepts public keys with a modulus size up to 8192-bits, - /// i.e. [`RsaPublicKey::MAX_SIZE`]. + /// Accepts modulus sizes from [`Self::MIN_SIZE`] to [`Self::MAX_SIZE`], inclusive. + /// Use [`Self::new_with_size_limits`] to explicitly allow other sizes. #[allow(clippy::missing_errors_doc, reason = "TODO")] pub fn new(n: BoxedUint, e: BoxedUint) -> Result { - Self::new_with_max_size(n, e, Self::MAX_SIZE) + Self::new_with_size_limits(n, e, Self::MIN_SIZE..=Self::MAX_SIZE) } - /// Create a new public key from its components. - #[allow(clippy::missing_errors_doc, reason = "TODO")] - pub fn new_with_max_size(n: BoxedUint, e: BoxedUint, max_size: usize) -> Result { - check_public_with_max_size(&n, &e, Some(max_size))?; + /// Create a new public key with explicit, inclusive modulus size limits in bits. + /// + /// All other public-key checks still apply. Allowing a smaller minimum can admit insecure + /// keys and is intended for testing or compatibility with legacy systems. + /// + /// For encoded keys outside the default limits, decode the components with the `pkcs1` + /// crate and pass them to this constructor explicitly. + /// + /// # Errors + /// Returns [`Error::InvalidArguments`] for an empty range, [`Error::ModulusTooSmall`] or + /// [`Error::ModulusTooLarge`] for a modulus outside the range, or an error if the modulus + /// or public exponent is invalid. + pub fn new_with_size_limits( + n: BoxedUint, + e: BoxedUint, + size_range_bits: RangeInclusive, + ) -> Result { + if size_range_bits.is_empty() { + return Err(Error::InvalidArguments); + } + + if (n.bits_vartime() as usize) < *size_range_bits.start() { + return Err(Error::ModulusTooSmall); + } + + check_public_with_max_size(&n, &e, Some(*size_range_bits.end()))?; let n_odd = Odd::new(n.clone()) .into_option() @@ -262,12 +288,19 @@ impl RsaPublicKey { Ok(Self { n, e, n_params }) } + /// Create a new public key with the default minimum modulus size and a custom maximum. + #[deprecated(since = "0.10.0", note = "please use `new_with_size_limits` instead")] + #[allow(clippy::missing_errors_doc, reason = "TODO")] + pub fn new_with_max_size(n: BoxedUint, e: BoxedUint, max_size: usize) -> Result { + Self::new_with_size_limits(n, e, Self::MIN_SIZE..=max_size) + } + /// Create a new public key, bypassing checks around the modulus and public /// exponent size. /// /// This method is not recommended, and only intended for unusual use cases. /// Most applications should use [`RsaPublicKey::new`] or - /// [`RsaPublicKey::new_with_max_size`] instead. + /// [`RsaPublicKey::new_with_size_limits`] instead. #[must_use] pub fn new_unchecked(n: BoxedUint, e: BoxedUint) -> Self { let n_odd = Odd::new(n.clone()).expect("n must be odd"); @@ -296,9 +329,6 @@ impl RsaPrivateKey { /// Default exponent for RSA keys. const EXP: u64 = 65537; - /// Minimum size of the modulus `n` in bits. Currently only applies to keygen. - const MIN_SIZE: u32 = 1024; - /// Generate a new RSA key pair with a modulus of the given bit size using the passed in `rng`. /// /// # Errors @@ -330,7 +360,7 @@ impl RsaPrivateKey { bit_size: usize, exp: BoxedUint, ) -> Result { - if bit_size < Self::MIN_SIZE as usize { + if bit_size < RsaPublicKey::MIN_SIZE { return Err(Error::ModulusTooSmall); } @@ -1178,7 +1208,7 @@ mod tests { fn test_serde() { use rand::rngs::ChaCha8Rng; use rand_core::SeedableRng; - use serde_test::{Configure, Token, assert_tokens}; + use serde_test::{Configure, Token, assert_ser_tokens, assert_tokens}; let mut rng = ChaCha8Rng::from_seed([42; 32]); let priv_key = RsaPrivateKey::new_unchecked(&mut rng, 64).expect("failed to generate key"); @@ -1190,12 +1220,13 @@ mod tests { ))]; assert_tokens(&priv_key.clone().readable(), &priv_tokens); - let priv_tokens = [Token::Str( + let pub_tokens = [Token::Str( "3024300d06092a864886f70d01010105000313003010020900ab240c3361d02e370203010001", )]; - assert_tokens( + // Small public keys can still be serialized, but default decoding rejects them. + assert_ser_tokens( &RsaPublicKey::from(priv_key.clone()).readable(), - &priv_tokens, + &pub_tokens, ); } diff --git a/src/oaep/encrypting_key.rs b/src/oaep/encrypting_key.rs index 7ea6e562..cf7ba365 100644 --- a/src/oaep/encrypting_key.rs +++ b/src/oaep/encrypting_key.rs @@ -74,17 +74,16 @@ where mod tests { #[test] - #[cfg(all(feature = "hazmat", feature = "serde"))] + #[cfg(feature = "serde")] fn test_serde() { use super::*; - use rand::rngs::ChaCha8Rng; - use rand_core::SeedableRng; + use pkcs8::DecodePublicKey; use serde_test::{Configure, Token, assert_tokens}; - let mut rng = ChaCha8Rng::from_seed([42; 32]); - let priv_key = - crate::RsaPrivateKey::new_unchecked(&mut rng, 64).expect("failed to generate key"); - let encrypting_key = EncryptingKey::::new(priv_key.to_public_key()); + let der = include_bytes!("../../tests/examples/pkcs8/rsa2048-pub.der"); + let pub_key = RsaPublicKey::from_public_key_der(der).unwrap(); + let encoded = include_str!("../../tests/examples/pkcs8/rsa2048-pub.hex").trim(); + let encrypting_key = EncryptingKey::::new(pub_key); let tokens = [ Token::Struct { @@ -92,9 +91,7 @@ mod tests { len: 4, }, Token::Str("inner"), - Token::Str( - "3024300d06092a864886f70d01010105000313003010020900ab240c3361d02e370203010001", - ), + Token::Str(encoded), Token::Str("label"), Token::None, Token::Str("phantom"), diff --git a/src/pkcs1v15/encrypting_key.rs b/src/pkcs1v15/encrypting_key.rs index 23a48c19..bb82f4ae 100644 --- a/src/pkcs1v15/encrypting_key.rs +++ b/src/pkcs1v15/encrypting_key.rs @@ -31,17 +31,16 @@ impl RandomizedEncryptor for EncryptingKey { #[cfg(test)] mod tests { #[test] - #[cfg(all(feature = "hazmat", feature = "serde"))] + #[cfg(feature = "serde")] fn test_serde() { use super::*; - use crate::RsaPrivateKey; - use rand::rngs::ChaCha8Rng; - use rand_core::SeedableRng; + use pkcs8::DecodePublicKey; use serde_test::{Configure, Token, assert_tokens}; - let mut rng = ChaCha8Rng::from_seed([42; 32]); - let priv_key = RsaPrivateKey::new_unchecked(&mut rng, 64).expect("failed to generate key"); - let encrypting_key = EncryptingKey::new(priv_key.to_public_key()); + let der = include_bytes!("../../tests/examples/pkcs8/rsa2048-pub.der"); + let pub_key = RsaPublicKey::from_public_key_der(der).unwrap(); + let encoded = include_str!("../../tests/examples/pkcs8/rsa2048-pub.hex").trim(); + let encrypting_key = EncryptingKey::new(pub_key); let tokens = [ Token::Struct { @@ -49,9 +48,7 @@ mod tests { len: 1, }, Token::Str("inner"), - Token::Str( - "3024300d06092a864886f70d01010105000313003010020900ab240c3361d02e370203010001", - ), + Token::Str(encoded), Token::StructEnd, ]; assert_tokens(&encrypting_key.clone().readable(), &tokens); diff --git a/src/pkcs1v15/verifying_key.rs b/src/pkcs1v15/verifying_key.rs index 6c4937dd..b4e34078 100644 --- a/src/pkcs1v15/verifying_key.rs +++ b/src/pkcs1v15/verifying_key.rs @@ -249,23 +249,19 @@ where #[cfg(test)] mod tests { #[test] - #[cfg(all(feature = "hazmat", feature = "serde"))] + #[cfg(feature = "serde")] fn test_serde() { use super::*; - use crate::RsaPrivateKey; - use rand::rngs::ChaCha8Rng; - use rand_core::SeedableRng; + use pkcs8::DecodePublicKey; use serde_test::{Configure, Token, assert_tokens}; use sha2::Sha256; - let mut rng = ChaCha8Rng::from_seed([42; 32]); - let priv_key = RsaPrivateKey::new_unchecked(&mut rng, 64).expect("failed to generate key"); - let pub_key = priv_key.to_public_key(); + let der = include_bytes!("../../tests/examples/pkcs8/rsa2048-pub.der"); + let pub_key = RsaPublicKey::from_public_key_der(der).unwrap(); + let encoded = include_str!("../../tests/examples/pkcs8/rsa2048-pub.hex").trim(); let verifying_key = VerifyingKey::::new(pub_key); - let tokens = [Token::Str( - "3024300d06092a864886f70d01010105000313003010020900ab240c3361d02e370203010001", - )]; + let tokens = [Token::Str(encoded)]; assert_tokens(&verifying_key.readable(), &tokens); } diff --git a/src/pss/verifying_key.rs b/src/pss/verifying_key.rs index 099b4732..2075f913 100644 --- a/src/pss/verifying_key.rs +++ b/src/pss/verifying_key.rs @@ -246,23 +246,19 @@ where #[cfg(test)] mod tests { #[test] - #[cfg(all(feature = "hazmat", feature = "serde"))] + #[cfg(feature = "serde")] fn test_serde() { use super::*; - use crate::RsaPrivateKey; - use rand::rngs::ChaCha8Rng; - use rand_core::SeedableRng; + use pkcs8::DecodePublicKey; use serde_test::{Configure, Token, assert_tokens}; use sha2::Sha256; - let mut rng = ChaCha8Rng::from_seed([42; 32]); - let priv_key = RsaPrivateKey::new_unchecked(&mut rng, 64).expect("failed to generate key"); - let pub_key = priv_key.to_public_key(); + let der = include_bytes!("../../tests/examples/pkcs8/rsa2048-pub.der"); + let pub_key = RsaPublicKey::from_public_key_der(der).unwrap(); + let encoded = include_str!("../../tests/examples/pkcs8/rsa2048-pub.hex").trim(); let verifying_key = VerifyingKey::::new(pub_key); - let tokens = [Token::Str( - "3024300d06092a864886f70d01010105000313003010020900ab240c3361d02e370203010001", - )]; + let tokens = [Token::Str(encoded)]; assert_tokens(&verifying_key.readable(), &tokens); } diff --git a/tests/examples/pkcs8/rsa2048-pub.hex b/tests/examples/pkcs8/rsa2048-pub.hex new file mode 100644 index 00000000..13a7bed0 --- /dev/null +++ b/tests/examples/pkcs8/rsa2048-pub.hex @@ -0,0 +1 @@ +30820122300d06092a864886f70d01010105000382010f003082010a0282010100b6c42c515f10a6aaf282c63edbe24243a170f3fa2633bd4833637f47ca4f6f36e03a5d29efc3191ac80f390d874b39e30f414fcec1fca0ed81e547edc2cd382c76f61c9018973db9fa537972a7c701f6b77e0982dfc15fc01927ee5e7cd94b4f599ff07013a7c8281bdf22dcbc9ad7cabb7c4311c982f58edb7213ad4558b332266d743aed8192d1884cadb8b14739a8dada66dc970806d9c7ac450cb13d0d7c575fb198534fc61bc41bc0f0574e0e0130c7bbbfbdfdc9f6a6e2e3e2aff1cbeac89ba57884528d55cfb08327a1e8c89f4e003cf2888e933241d9d695bcbbacdc90b44e3e095fa37058ea25b13f5e295cbeac6de838ab8c50af61e298975b872f0203010001 diff --git a/tests/key_size.rs b/tests/key_size.rs new file mode 100644 index 00000000..403c13a5 --- /dev/null +++ b/tests/key_size.rs @@ -0,0 +1,140 @@ +//! Public-key modulus size policy and explicit overrides. + +use crypto_bigint::Resize; +use rand::rngs::ChaCha8Rng; +use rand_core::SeedableRng; +use rsa::{BoxedUint, Error, Oaep, Pkcs1v15Encrypt, RsaPublicKey}; + +// Public-key construction checks the modulus size and parity, not its factorization. +fn modulus(bits: u32) -> BoxedUint { + let one = BoxedUint::one_with_precision(bits); + (&one << (bits - 1)) | one +} + +#[test] +#[allow(deprecated)] +fn default_public_key_size_limits() { + for (bits, error) in [ + (1023, Some(Error::ModulusTooSmall)), + (1024, None), + (8192, None), + (8193, Some(Error::ModulusTooLarge)), + ] { + let n = modulus(bits); + let e = BoxedUint::from(65537u64); + assert_eq!(RsaPublicKey::new(n.clone(), e.clone()).err(), error); + assert_eq!( + RsaPublicKey::new_with_max_size(n, e, RsaPublicKey::MAX_SIZE).err(), + error + ); + } + + // Storage precision must not allow a short modulus to evade the minimum. + assert_eq!( + RsaPublicKey::new( + BoxedUint::from(3233u64).resize_unchecked(1024), + 17u64.into() + ), + Err(Error::ModulusTooSmall) + ); +} + +#[test] +fn explicit_public_key_size_limits() { + for bits in [512, 1024, 8193] { + let key = RsaPublicKey::new_with_size_limits( + modulus(bits), + 65537u64.into(), + bits as usize..=bits as usize, + ) + .unwrap(); + + // Operation-time checks must respect the constructor's explicit size override. + let mut rng = ChaCha8Rng::from_seed([42; 32]); + key.encrypt(&mut rng, Pkcs1v15Encrypt, b"test").unwrap(); + key.encrypt(&mut rng, Oaep::::new(), b"test") + .unwrap(); + } + + for (limits, error) in [ + (1025..=8192, Error::ModulusTooSmall), + (0..=1023, Error::ModulusTooLarge), + (1025..=1024, Error::InvalidArguments), + ] { + assert_eq!( + RsaPublicKey::new_with_size_limits(modulus(1024), 65537u64.into(), limits), + Err(error) + ); + } +} + +#[test] +fn size_override_preserves_public_key_validation() { + for (n, e, error) in [ + (0u64, 3u64, Error::InvalidModulus), + (3234, 17, Error::InvalidModulus), + (3233, 3233, Error::InvalidModulus), + (3233, 2, Error::InvalidExponent), + (3233, 1, Error::PublicExponentTooSmall), + (1 << 40 | 1, 1 << 34 | 1, Error::PublicExponentTooLarge), + ] { + assert_eq!( + RsaPublicKey::new_with_size_limits(n.into(), e.into(), 0..=8192), + Err(error) + ); + } +} + +#[test] +#[cfg(feature = "encoding")] +fn public_key_decoding_enforces_default_size_limits() { + use rsa::{ + pkcs1::{DecodeRsaPublicKey, EncodeRsaPublicKey}, + pkcs8::{DecodePublicKey, EncodePublicKey}, + }; + + for bits in [512, 1023, 1024] { + let key = + RsaPublicKey::new_with_size_limits(modulus(bits), 65537u64.into(), 0..=8192).unwrap(); + let pkcs1 = key.to_pkcs1_der().unwrap(); + let spki = key.to_public_key_der().unwrap(); + if bits < 1024 { + assert!(matches!( + RsaPublicKey::from_pkcs1_der(pkcs1.as_bytes()), + Err(pkcs1::Error::KeyMalformed) + )); + assert!(matches!( + RsaPublicKey::from_public_key_der(spki.as_bytes()), + Err(spki::Error::KeyMalformed) + )); + } else { + assert_eq!(RsaPublicKey::from_pkcs1_der(pkcs1.as_bytes()).unwrap(), key); + assert_eq!( + RsaPublicKey::from_public_key_der(spki.as_bytes()).unwrap(), + key + ); + } + + // A legacy key can be decoded into components and explicitly allowed. + use rsa::pkcs8::der::Decode; + let components = pkcs1::RsaPublicKeyRef::from_der(pkcs1.as_bytes()).unwrap(); + let imported = RsaPublicKey::new_with_size_limits( + BoxedUint::from_be_slice_vartime(components.modulus.as_bytes()), + BoxedUint::from_be_slice_vartime(components.public_exponent.as_bytes()), + 0..=8192, + ) + .unwrap(); + assert_eq!(imported, key); + + #[cfg(feature = "serde")] + { + let json = serde_json::to_string(&key).unwrap(); + let decoded = serde_json::from_str::(&json); + if bits < 1024 { + assert!(decoded.is_err()); + } else { + assert_eq!(decoded.unwrap(), key); + } + } + } +}