diff --git a/src/pss.rs b/src/pss.rs index 363989a0..1f0103e0 100644 --- a/src/pss.rs +++ b/src/pss.rs @@ -39,7 +39,10 @@ use { crate::encoding::ID_RSASSA_PSS, const_oid::AssociatedOid, pkcs1::RsaPssParams, - spki::{der::Any, AlgorithmIdentifierOwned}, + spki::{ + der::{Any, ErrorKind, Tag}, + AlgorithmIdentifierOwned, + }, }; /// Digital signatures using PSS padding. @@ -272,15 +275,22 @@ pub fn get_default_pss_signature_algo_id() -> spki::Result::output_size() as u8; + let salt_len = ::output_size(); get_pss_signature_algo_id::(salt_len) } #[cfg(feature = "encoding")] -fn get_pss_signature_algo_id(salt_len: u8) -> spki::Result +fn get_pss_signature_algo_id(salt_len: usize) -> spki::Result where D: Digest + AssociatedOid, { + // `RsaPssParams::salt_len` is a `u8`, so a `salt_len` of 256 or more cannot be + // represented in the `AlgorithmIdentifier`. Silently truncating it with `as u8` + // would advertise a wrong salt length to verifiers, causing valid signatures to + // fail verification (see issue #703), so reject it instead. + let salt_len = u8::try_from(salt_len) + .map_err(|_| spki::der::Error::from(ErrorKind::Value { tag: Tag::Integer }))?; + let pss_params = RsaPssParams::new::(salt_len); Ok(AlgorithmIdentifierOwned { @@ -291,6 +301,7 @@ where #[cfg(all(test, feature = "encoding"))] mod test { + use super::get_default_pss_signature_algo_id; use crate::pss::{BlindedSigningKey, Pss, Signature, SigningKey, VerifyingKey}; use crate::{RsaPrivateKey, RsaPublicKey}; @@ -303,6 +314,7 @@ mod test { use sha1::{Digest, Sha1}; use signature::hazmat::{PrehashVerifier, RandomizedPrehashSigner}; use signature::{DigestVerifier, Keypair, RandomizedDigestSigner, RandomizedSigner, Verifier}; + use spki::DynSignatureAlgorithmIdentifier; fn get_private_key() -> RsaPrivateKey { // In order to generate new test vectors you'll need the PEM form of this key: @@ -672,4 +684,77 @@ tAboUGBxTDq3ZroNism3DaMIbKPyYrAqhKov1h5V .expect("verification to succeed"); } } + + /// `signature_algorithm_identifier()` must refuse a `salt_len` that does not fit + /// in the `u8` used by `RsaPssParams`, rather than silently truncating it and + /// advertising a wrong salt length to verifiers (see issue #703). + #[rstest] + #[case(255)] + #[case(128)] + fn test_pss_algo_id_salt_len_representable(#[case] salt_len: usize) { + use spki::der::Encode; + + let priv_key = get_private_key(); + + let signing_key = SigningKey::::new_with_salt_len(priv_key.clone(), salt_len); + let blinded_signing_key = + BlindedSigningKey::::new_with_salt_len(priv_key.clone(), salt_len); + + let algo_ids = [ + signing_key + .signature_algorithm_identifier() + .expect("salt_len fits in a u8, so encoding must succeed"), + blinded_signing_key + .signature_algorithm_identifier() + .expect("salt_len fits in a u8, so encoding must succeed"), + ]; + for algo_id in &algo_ids { + let params_bytes = algo_id + .parameters + .as_ref() + .expect("RSA-PSS AlgorithmIdentifier carries parameters") + .to_der() + .expect("parameters are DER-encodable"); + let pss_params = + ::from_der(¶ms_bytes) + .expect("RSA-PSS parameters are DER-decodable"); + assert_eq!(pss_params.salt_len as usize, salt_len); + } + } + + #[rstest] + #[case(256)] + #[case(300)] + #[case(usize::MAX)] + fn test_pss_algo_id_salt_len_out_of_range(#[case] salt_len: usize) { + let priv_key = get_private_key(); + + let signing_key = SigningKey::::new_with_salt_len(priv_key.clone(), salt_len); + let blinded_signing_key = BlindedSigningKey::::new_with_salt_len(priv_key, salt_len); + + signing_key + .signature_algorithm_identifier() + .expect_err("salt_len overflows the u8 in RsaPssParams, so encoding must fail"); + blinded_signing_key + .signature_algorithm_identifier() + .expect_err("salt_len overflows the u8 in RsaPssParams, so encoding must fail"); + } + + /// The digest-size default salt length is unchanged by the `u8` validation. + #[test] + fn test_pss_algo_id_default_salt_len_unchanged() { + use spki::der::Encode; + + let algo_id = get_default_pss_signature_algo_id::() + .expect("the default salt length must remain encodable"); + let params_bytes = algo_id + .parameters + .as_ref() + .expect("RSA-PSS AlgorithmIdentifier carries parameters") + .to_der() + .expect("parameters are DER-encodable"); + let pss_params = ::from_der(¶ms_bytes) + .expect("RSA-PSS parameters are DER-decodable"); + assert_eq!(pss_params.salt_len as usize, Sha1::output_size()); + } } diff --git a/src/pss/blinded_signing_key.rs b/src/pss/blinded_signing_key.rs index 3063c8a6..b025c154 100644 --- a/src/pss/blinded_signing_key.rs +++ b/src/pss/blinded_signing_key.rs @@ -50,6 +50,11 @@ where /// Create a new RSASSA-PSS signing key which produces "blinded" /// signatures with a salt of the given length. + /// + /// Note: salt lengths of 256 bytes or more are accepted for signing, but + /// [`signature_algorithm_identifier`](spki::DynSignatureAlgorithmIdentifier::signature_algorithm_identifier) + /// will return an error for them, as the resulting `RsaPssParams` cannot + /// represent such a salt length. pub fn new_with_salt_len(key: RsaPrivateKey, salt_len: usize) -> Self { Self { inner: key, @@ -184,7 +189,7 @@ where D: Digest + AssociatedOid, { fn signature_algorithm_identifier(&self) -> spki::Result { - get_pss_signature_algo_id::(self.salt_len as u8) + get_pss_signature_algo_id::(self.salt_len) } } diff --git a/src/pss/signing_key.rs b/src/pss/signing_key.rs index c159f0b4..db837578 100644 --- a/src/pss/signing_key.rs +++ b/src/pss/signing_key.rs @@ -58,6 +58,11 @@ where } /// Create a new RSASSA-PSS signing key with a salt of the given length. + /// + /// Note: salt lengths of 256 bytes or more are accepted for signing, but + /// [`signature_algorithm_identifier`](spki::DynSignatureAlgorithmIdentifier::signature_algorithm_identifier) + /// will return an error for them, as the resulting `RsaPssParams` cannot + /// represent such a salt length. pub fn new_with_salt_len(key: RsaPrivateKey, salt_len: usize) -> Self { Self { inner: key, @@ -221,7 +226,7 @@ where D: Digest + AssociatedOid, { fn signature_algorithm_identifier(&self) -> spki::Result { - get_pss_signature_algo_id::(self.salt_len as u8) + get_pss_signature_algo_id::(self.salt_len) } }