diff --git a/Cargo.toml b/Cargo.toml index 81b0022f..4141d9d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -113,3 +113,6 @@ codegen-units = 1 name = "benchmarks" path = "benches/benchmark.rs" harness = false + +[package.metadata.cargo-semver-checks.lints] +enum_variant_marked_deprecated = "warn" diff --git a/src/alg_tests.rs b/src/alg_tests.rs index 20bab199..e8dca86a 100644 --- a/src/alg_tests.rs +++ b/src/alg_tests.rs @@ -17,15 +17,15 @@ use std::prelude::v1::*; use base64::{Engine as _, engine::general_purpose}; +use pki_types::alg_id; -use crate::error::{DerTypeId, Error}; +use crate::error::{DerTypeId, Error, UnsupportedSignatureAlgorithmForPublicKeyContext}; use crate::verify_cert::Budget; use crate::{der, signed_data}; use super::{ - INVALID_SIGNATURE_FOR_RSA_KEY, OK_IF_POINT_COMPRESSION_SUPPORTED, OK_IF_RSA_AVAILABLE, - SUPPORTED_ALGORITHMS_IN_TESTS, UNSUPPORTED_ECDSA_SHA512_SIGNATURE, - UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY, + OK_IF_POINT_COMPRESSION_SUPPORTED, SUPPORTED_ALGORITHMS_IN_TESTS, invalid_rsa_signature, + maybe_rsa, unsupported, unsupported_for_ecdsa, unsupported_for_rsa, }; macro_rules! test_file_bytes { @@ -113,7 +113,10 @@ fn test_ecdsa_prime256v1_sha512_spki_params_null() { test_verify_signed_data(test_file_bytes!( "ecdsa-prime256v1-sha512-spki-params-null.pem" )), - Err(UNSUPPORTED_ECDSA_SHA512_SIGNATURE) + Err(unsupported_for_ecdsa( + &alg_id::ECDSA_SHA512, + include_bytes!("../tests/signatures/alg-id-ecpublickey-params-null.der") + )) ); } @@ -135,7 +138,10 @@ fn test_ecdsa_prime256v1_sha512_using_ecdh_key() { test_verify_signed_data(test_file_bytes!( "ecdsa-prime256v1-sha512-using-ecdh-key.pem" )), - Err(UNSUPPORTED_ECDSA_SHA512_SIGNATURE) + Err(unsupported_for_ecdsa( + &alg_id::ECDSA_SHA512, + include_bytes!("../tests/signatures/alg-ecdh-secp256r1.der") + )) ); } @@ -147,7 +153,10 @@ fn test_ecdsa_prime256v1_sha512_using_ecmqv_key() { test_verify_signed_data(test_file_bytes!( "ecdsa-prime256v1-sha512-using-ecmqv-key.pem" )), - Err(UNSUPPORTED_ECDSA_SHA512_SIGNATURE) + Err(unsupported_for_ecdsa( + &alg_id::ECDSA_SHA512, + include_bytes!("../tests/signatures/alg-ecmqv-secp256r1.der") + )) ); } @@ -156,8 +165,11 @@ fn test_ecdsa_prime256v1_sha512_using_rsa_algorithm() { assert_eq!( test_verify_signed_data(test_file_bytes!( "ecdsa-prime256v1-sha512-using-rsa-algorithm.pem" - )), - Err(UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY) + ),), + Err(unsupported_for_rsa( + &alg_id::RSA_PKCS1_SHA512, + &alg_id::ECDSA_P256, + )) ); } @@ -169,7 +181,10 @@ fn test_ecdsa_prime256v1_sha512_wrong_signature_format() { test_verify_signed_data(test_file_bytes!( "ecdsa-prime256v1-sha512-wrong-signature-format.pem" )), - Err(UNSUPPORTED_ECDSA_SHA512_SIGNATURE) + Err(unsupported_for_ecdsa( + &alg_id::ECDSA_SHA512, + &alg_id::ECDSA_P256, + )) ); } @@ -178,7 +193,10 @@ fn test_ecdsa_prime256v1_sha512_wrong_signature_format() { fn test_ecdsa_prime256v1_sha512() { assert_eq!( test_verify_signed_data(test_file_bytes!("ecdsa-prime256v1-sha512.pem")), - Err(UNSUPPORTED_ECDSA_SHA512_SIGNATURE) + Err(unsupported_for_ecdsa( + &alg_id::ECDSA_SHA512, + &alg_id::ECDSA_P256, + )) ); } @@ -204,7 +222,14 @@ fn test_ecdsa_secp384r1_sha256() { fn test_ecdsa_using_rsa_key() { assert_eq!( test_verify_signed_data(test_file_bytes!("ecdsa-using-rsa-key.pem")), - Err(Error::UnsupportedSignatureAlgorithmForPublicKey) + Err(Error::UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: alg_id::ECDSA_SHA256.as_ref().to_vec(), + #[cfg(feature = "alloc")] + public_key_algorithm_id: alg_id::RSA_ENCRYPTION.as_ref().to_vec(), + } + )) ); } @@ -228,7 +253,9 @@ fn test_rsa_pkcs1_sha1_bad_key_der_null() { fn test_rsa_pkcs1_sha1_key_params_absent() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pkcs1-sha1-key-params-absent.pem")), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsae-sha1.der" + ))) ); } @@ -238,7 +265,9 @@ fn test_rsa_pkcs1_sha1_using_pss_key_no_params() { test_verify_signed_data(test_file_bytes!( "rsa-pkcs1-sha1-using-pss-key-no-params.pem" )), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsae-sha1.der" + ))) ); } @@ -246,7 +275,7 @@ fn test_rsa_pkcs1_sha1_using_pss_key_no_params() { fn test_rsa_pkcs1_sha1_wrong_algorithm() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pkcs1-sha1-wrong-algorithm.pem")), - Err(INVALID_SIGNATURE_FOR_RSA_KEY) + Err(invalid_rsa_signature()) ); } @@ -254,7 +283,9 @@ fn test_rsa_pkcs1_sha1_wrong_algorithm() { fn test_rsa_pkcs1_sha1() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pkcs1-sha1.pem")), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsae-sha1.der" + ))) ); } @@ -267,7 +298,7 @@ fn test_rsa_pkcs1_sha1() { fn test_rsa_pkcs1_sha256() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pkcs1-sha256.pem")), - Err(INVALID_SIGNATURE_FOR_RSA_KEY) + Err(invalid_rsa_signature()) ); } @@ -285,7 +316,10 @@ fn test_rsa_pkcs1_sha256_spki_non_null_params() { test_verify_signed_data(test_file_bytes!( "rsa-pkcs1-sha256-spki-non-null-params.pem" )), - Err(UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY) + Err(unsupported_for_rsa( + &alg_id::RSA_PKCS1_SHA256, + include_bytes!("../tests/signatures/alg-rsae-bad-params.der") + )) ); } @@ -295,7 +329,14 @@ fn test_rsa_pkcs1_sha256_using_ecdsa_algorithm() { test_verify_signed_data(test_file_bytes!( "rsa-pkcs1-sha256-using-ecdsa-algorithm.pem" )), - Err(Error::UnsupportedSignatureAlgorithmForPublicKey) + Err(Error::UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: alg_id::ECDSA_SHA256.as_ref().to_vec(), + #[cfg(feature = "alloc")] + public_key_algorithm_id: alg_id::RSA_ENCRYPTION.as_ref().to_vec(), + } + )) ); } @@ -303,7 +344,10 @@ fn test_rsa_pkcs1_sha256_using_ecdsa_algorithm() { fn test_rsa_pkcs1_sha256_using_id_ea_rsa() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pkcs1-sha256-using-id-ea-rsa.pem")), - Err(UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY) + Err(unsupported_for_rsa( + &alg_id::RSA_PKCS1_SHA256, + include_bytes!("../tests/signatures/alg-rsa-null-params.der") + )) ); } @@ -315,7 +359,9 @@ fn test_rsa_pss_sha1_salt20_using_pss_key_no_params() { test_verify_signed_data(test_file_bytes!( "rsa-pss-sha1-salt20-using-pss-key-no-params.pem" )), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-defaults.der" + ))) ); } @@ -325,14 +371,18 @@ fn test_rsa_pss_sha1_salt20_using_pss_key_with_null_params() { test_verify_signed_data(test_file_bytes!( "rsa-pss-sha1-salt20-using-pss-key-with-null-params.pem" )), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-defaults.der" + ))) ); } #[test] fn test_rsa_pss_sha1_salt20() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pss-sha1-salt20.pem")), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-defaults.der" + ))) ); } @@ -340,7 +390,9 @@ fn test_rsa_pss_sha1_salt20() { fn test_rsa_pss_sha1_wrong_salt() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pss-sha1-wrong-salt.pem")), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-salt23.der" + ))) ); } @@ -348,7 +400,9 @@ fn test_rsa_pss_sha1_wrong_salt() { fn test_rsa_pss_sha256_mgf1_sha512_salt33() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pss-sha256-mgf1-sha512-salt33.pem")), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-sha256-mgf1-sha512-salt33.der" + ))) ); } @@ -358,7 +412,9 @@ fn test_rsa_pss_sha256_salt10_using_pss_key_with_params() { test_verify_signed_data(test_file_bytes!( "rsa-pss-sha256-salt10-using-pss-key-with-params.pem" )), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-sha256-mgf1-sha256-salt10.der" + ))) ); } #[test] @@ -367,7 +423,9 @@ fn test_rsa_pss_sha256_salt10_using_pss_key_with_wrong_params() { test_verify_signed_data(test_file_bytes!( "rsa-pss-sha256-salt10-using-pss-key-with-wrong-params.pem" )), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-sha256-mgf1-sha256-salt10.der" + ))) ); } @@ -375,7 +433,9 @@ fn test_rsa_pss_sha256_salt10_using_pss_key_with_wrong_params() { fn test_rsa_pss_sha256_salt10() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-pss-sha256-salt10.pem")), - Err(Error::UnsupportedSignatureAlgorithm) + Err(unsupported(include_bytes!( + "../tests/signatures/alg-rsapss-sha256-mgf1-sha256-salt10.der" + ))) ); } @@ -385,7 +445,7 @@ fn test_rsa_pss_sha256_salt10() { fn test_rsa_pss_sha256_salt32() { assert_eq!( test_verify_signed_data(test_file_bytes!("ours/rsa-pss-sha256-salt32.pem")), - OK_IF_RSA_AVAILABLE + maybe_rsa() ); } @@ -393,7 +453,7 @@ fn test_rsa_pss_sha256_salt32() { fn test_rsa_pss_sha384_salt48() { assert_eq!( test_verify_signed_data(test_file_bytes!("ours/rsa-pss-sha384-salt48.pem")), - OK_IF_RSA_AVAILABLE + maybe_rsa() ); } @@ -401,7 +461,7 @@ fn test_rsa_pss_sha384_salt48() { fn test_rsa_pss_sha512_salt64() { assert_eq!( test_verify_signed_data(test_file_bytes!("ours/rsa-pss-sha512-salt64.pem")), - OK_IF_RSA_AVAILABLE + maybe_rsa() ); } @@ -411,7 +471,7 @@ fn test_rsa_pss_sha256_salt32_corrupted_data() { test_verify_signed_data(test_file_bytes!( "ours/rsa-pss-sha256-salt32-corrupted-data.pem" )), - Err(INVALID_SIGNATURE_FOR_RSA_KEY) + Err(invalid_rsa_signature()) ); } @@ -421,7 +481,7 @@ fn test_rsa_pss_sha384_salt48_corrupted_data() { test_verify_signed_data(test_file_bytes!( "ours/rsa-pss-sha384-salt48-corrupted-data.pem" )), - Err(INVALID_SIGNATURE_FOR_RSA_KEY) + Err(invalid_rsa_signature()) ); } @@ -431,7 +491,7 @@ fn test_rsa_pss_sha512_salt64_corrupted_data() { test_verify_signed_data(test_file_bytes!( "ours/rsa-pss-sha512-salt64-corrupted-data.pem" )), - Err(INVALID_SIGNATURE_FOR_RSA_KEY) + Err(invalid_rsa_signature()) ); } @@ -439,7 +499,10 @@ fn test_rsa_pss_sha512_salt64_corrupted_data() { fn test_rsa_using_ec_key() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa-using-ec-key.pem")), - Err(UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY) + Err(unsupported_for_rsa( + &alg_id::RSA_PKCS1_SHA256, + &alg_id::ECDSA_P256 + )) ); } @@ -447,7 +510,7 @@ fn test_rsa_using_ec_key() { fn test_rsa2048_pkcs1_sha512() { assert_eq!( test_verify_signed_data(test_file_bytes!("rsa2048-pkcs1-sha512.pem")), - OK_IF_RSA_AVAILABLE + maybe_rsa() ); } diff --git a/src/aws_lc_rs_algs.rs b/src/aws_lc_rs_algs.rs index 30b368f1..6e321bc4 100644 --- a/src/aws_lc_rs_algs.rs +++ b/src/aws_lc_rs_algs.rs @@ -260,7 +260,10 @@ pub static ED25519: &dyn SignatureVerificationAlgorithm = &AwsLcRsAlgorithm { #[cfg(test)] #[path = "."] mod tests { - use crate::Error; + use crate::error::{ + Error, UnsupportedSignatureAlgorithmContext, + UnsupportedSignatureAlgorithmForPublicKeyContext, + }; static SUPPORTED_ALGORITHMS_IN_TESTS: &[&dyn super::SignatureVerificationAlgorithm] = &[ // Reasonable algorithms. @@ -288,17 +291,50 @@ mod tests { super::ML_DSA_87, ]; - const UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY: Error = - Error::UnsupportedSignatureAlgorithmForPublicKey; - - const UNSUPPORTED_ECDSA_SHA512_SIGNATURE: Error = - Error::UnsupportedSignatureAlgorithmForPublicKey; - - const INVALID_SIGNATURE_FOR_RSA_KEY: Error = Error::InvalidSignatureForPublicKey; - - const OK_IF_RSA_AVAILABLE: Result<(), Error> = Ok(()); const OK_IF_POINT_COMPRESSION_SUPPORTED: Result<(), Error> = Ok(()); #[path = "alg_tests.rs"] mod alg_tests; + + fn maybe_rsa() -> Result<(), Error> { + Ok(()) + } + + fn unsupported_for_rsa(_sig_alg_id: &[u8], _public_key_alg_id: &[u8]) -> Error { + Error::UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: _sig_alg_id.to_vec(), + #[cfg(feature = "alloc")] + public_key_algorithm_id: _public_key_alg_id.to_vec(), + }, + ) + } + + fn invalid_rsa_signature() -> Error { + Error::InvalidSignatureForPublicKey + } + + fn unsupported_for_ecdsa(_sig_alg_id: &[u8], _public_key_alg_id: &[u8]) -> Error { + Error::UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: _sig_alg_id.to_vec(), + #[cfg(feature = "alloc")] + public_key_algorithm_id: _public_key_alg_id.to_vec(), + }, + ) + } + + fn unsupported(_sig_alg_id: &[u8]) -> Error { + Error::UnsupportedSignatureAlgorithmContext(UnsupportedSignatureAlgorithmContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: _sig_alg_id.to_vec(), + #[cfg(feature = "alloc")] + supported_algorithms: SUPPORTED_ALGORITHMS_IN_TESTS + .iter() + .map(|&alg| alg.signature_alg_id()) + .collect(), + }) + } } diff --git a/src/crl/mod.rs b/src/crl/mod.rs index 921abf26..41f0e3f4 100644 --- a/src/crl/mod.rs +++ b/src/crl/mod.rs @@ -210,10 +210,18 @@ impl KeyUsageMode { // signature, not a certificate. fn crl_signature_err(err: Error) -> Error { match err { + #[allow(deprecated)] Error::UnsupportedSignatureAlgorithm => Error::UnsupportedCrlSignatureAlgorithm, + Error::UnsupportedSignatureAlgorithmContext(cx) => { + Error::UnsupportedCrlSignatureAlgorithmContext(cx) + } + #[allow(deprecated)] Error::UnsupportedSignatureAlgorithmForPublicKey => { Error::UnsupportedCrlSignatureAlgorithmForPublicKey } + Error::UnsupportedSignatureAlgorithmForPublicKeyContext(cx) => { + Error::UnsupportedCrlSignatureAlgorithmForPublicKeyContext(cx) + } Error::InvalidSignatureForPublicKey => Error::InvalidCrlSignatureForPublicKey, _ => err, } diff --git a/src/error.rs b/src/error.rs index a066514d..028763f0 100644 --- a/src/error.rs +++ b/src/error.rs @@ -19,9 +19,9 @@ use alloc::vec::Vec; use core::fmt; use core::ops::ControlFlow; -#[cfg(feature = "alloc")] -use pki_types::ServerName; use pki_types::UnixTime; +#[cfg(feature = "alloc")] +use pki_types::{AlgorithmIdentifier, ServerName}; use crate::verify_cert::RequiredEkuNotFoundContext; @@ -201,12 +201,28 @@ pub enum Error { /// The signature algorithm for a signature over a CRL is not in the set of supported /// signature algorithms given. + #[deprecated( + since = "0.103.4", + note = "use UnsupportedCrlSignatureAlgorithmContext instead" + )] UnsupportedCrlSignatureAlgorithm, /// The signature algorithm for a signature is not in the set of supported /// signature algorithms given. + UnsupportedCrlSignatureAlgorithmContext(UnsupportedSignatureAlgorithmContext), + + /// The signature algorithm for a signature is not in the set of supported + /// signature algorithms given. + #[deprecated( + since = "0.103.4", + note = "use UnsupportedSignatureAlgorithmContext instead" + )] UnsupportedSignatureAlgorithm, + /// The signature algorithm for a signature is not in the set of supported + /// signature algorithms given. + UnsupportedSignatureAlgorithmContext(UnsupportedSignatureAlgorithmContext), + /// The CRL signature's algorithm does not match the algorithm of the issuer /// public key it is being validated for. This may be because the public key /// algorithm's OID isn't recognized (e.g. DSA), or the public key @@ -214,6 +230,10 @@ pub enum Error { /// algorithm (e.g. ECC keys for unsupported curves), or the public key /// algorithm and the signature algorithm simply don't match (e.g. /// verifying an RSA signature with an ECC public key). + #[deprecated( + since = "0.103.4", + note = "use UnsupportedCrlSignatureAlgorithmForPublicKeyContext instead" + )] UnsupportedCrlSignatureAlgorithmForPublicKey, /// The signature's algorithm does not match the algorithm of the public @@ -223,7 +243,33 @@ pub enum Error { /// algorithm (e.g. ECC keys for unsupported curves), or the public key /// algorithm and the signature algorithm simply don't match (e.g. /// verifying an RSA signature with an ECC public key). + UnsupportedCrlSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext, + ), + + /// The signature's algorithm does not match the algorithm of the public + /// key it is being validated for. This may be because the public key + /// algorithm's OID isn't recognized (e.g. DSA), or the public key + /// algorithm's parameters don't match the supported parameters for that + /// algorithm (e.g. ECC keys for unsupported curves), or the public key + /// algorithm and the signature algorithm simply don't match (e.g. + /// verifying an RSA signature with an ECC public key). + #[deprecated( + since = "0.103.4", + note = "use UnsupportedSignatureAlgorithmForPublicKeyContext instead" + )] UnsupportedSignatureAlgorithmForPublicKey, + + /// The signature's algorithm does not match the algorithm of the public + /// key it is being validated for. This may be because the public key + /// algorithm's OID isn't recognized (e.g. DSA), or the public key + /// algorithm's parameters don't match the supported parameters for that + /// algorithm (e.g. ECC keys for unsupported curves), or the public key + /// algorithm and the signature algorithm simply don't match (e.g. + /// verifying an RSA signature with an ECC public key). + UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext, + ), } impl Error { @@ -260,9 +306,16 @@ impl Error { Self::InvalidCrlNumber => 160, // Errors related to unsupported features. + #[allow(deprecated)] Self::UnsupportedCrlSignatureAlgorithmForPublicKey - | Self::UnsupportedSignatureAlgorithmForPublicKey => 150, - Self::UnsupportedCrlSignatureAlgorithm | Self::UnsupportedSignatureAlgorithm => 140, + | Self::UnsupportedCrlSignatureAlgorithmForPublicKeyContext(_) + | Self::UnsupportedSignatureAlgorithmForPublicKey + | Self::UnsupportedSignatureAlgorithmForPublicKeyContext(_) => 150, + #[allow(deprecated)] + Self::UnsupportedCrlSignatureAlgorithm + | Self::UnsupportedCrlSignatureAlgorithmContext(_) + | Self::UnsupportedSignatureAlgorithm + | Self::UnsupportedSignatureAlgorithmContext(_) => 140, Self::UnsupportedCriticalExtension => 130, Self::UnsupportedCertVersion => 130, Self::UnsupportedCrlVersion => 120, @@ -343,6 +396,32 @@ pub struct InvalidNameContext { pub presented: Vec, } +/// Additional context for the `UnsupportedSignatureAlgorithmForPublicKey` error variant. +/// +/// The contents of this type depend on whether the `alloc` feature is enabled. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UnsupportedSignatureAlgorithmForPublicKeyContext { + /// The signature algorithm OID. + #[cfg(feature = "alloc")] + pub signature_algorithm_id: Vec, + /// The public key algorithm OID. + #[cfg(feature = "alloc")] + pub public_key_algorithm_id: Vec, +} + +/// Additional context for the `UnsupportedSignatureAlgorithm` error variant. +/// +/// The contents of this type depend on whether the `alloc` feature is enabled. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct UnsupportedSignatureAlgorithmContext { + /// The signature algorithm OID that was unsupported. + #[cfg(feature = "alloc")] + pub signature_algorithm_id: Vec, + /// Supported algorithms that were available for signature verification. + #[cfg(feature = "alloc")] + pub supported_algorithms: Vec, +} + /// Trailing data was found while parsing DER-encoded input for the named type. #[allow(missing_docs)] #[non_exhaustive] diff --git a/src/ring_algs.rs b/src/ring_algs.rs index 0c038c33..7b7ae3fe 100644 --- a/src/ring_algs.rs +++ b/src/ring_algs.rs @@ -213,7 +213,9 @@ pub static ED25519: &dyn SignatureVerificationAlgorithm = &RingAlgorithm { #[cfg(test)] #[path = "."] mod tests { - use crate::Error; + #[cfg(feature = "alloc")] + use crate::error::UnsupportedSignatureAlgorithmForPublicKeyContext; + use crate::error::{Error, UnsupportedSignatureAlgorithmContext}; static SUPPORTED_ALGORITHMS_IN_TESTS: &[&dyn super::SignatureVerificationAlgorithm] = &[ // Reasonable algorithms. @@ -239,29 +241,63 @@ mod tests { super::ECDSA_P384_SHA256, // Digest is unnecessarily short. ]; - const UNSUPPORTED_SIGNATURE_ALGORITHM_FOR_RSA_KEY: Error = if cfg!(feature = "alloc") { - Error::UnsupportedSignatureAlgorithmForPublicKey - } else { - Error::UnsupportedSignatureAlgorithm - }; - - const UNSUPPORTED_ECDSA_SHA512_SIGNATURE: Error = Error::UnsupportedSignatureAlgorithm; - - const INVALID_SIGNATURE_FOR_RSA_KEY: Error = if cfg!(feature = "alloc") { - Error::InvalidSignatureForPublicKey - } else { - Error::UnsupportedSignatureAlgorithm - }; - - const OK_IF_RSA_AVAILABLE: Result<(), Error> = if cfg!(feature = "alloc") { - Ok(()) - } else { - Err(Error::UnsupportedSignatureAlgorithm) - }; - const OK_IF_POINT_COMPRESSION_SUPPORTED: Result<(), Error> = Err(Error::InvalidSignatureForPublicKey); #[path = "alg_tests.rs"] mod alg_tests; + + fn maybe_rsa() -> Result<(), Error> { + #[cfg(feature = "alloc")] + { + Ok(()) + } + #[cfg(not(feature = "alloc"))] + { + Err(unsupported(&[])) + } + } + + fn unsupported_for_rsa(sig_alg_id: &[u8], _public_key_alg_id: &[u8]) -> Error { + #[cfg(feature = "alloc")] + { + Error::UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext { + signature_algorithm_id: sig_alg_id.to_vec(), + public_key_algorithm_id: _public_key_alg_id.to_vec(), + }, + ) + } + #[cfg(not(feature = "alloc"))] + { + unsupported(sig_alg_id) + } + } + + fn invalid_rsa_signature() -> Error { + #[cfg(feature = "alloc")] + { + Error::InvalidSignatureForPublicKey + } + #[cfg(not(feature = "alloc"))] + { + unsupported(&[]) + } + } + + fn unsupported_for_ecdsa(sig_alg_id: &[u8], _public_key_alg_id: &[u8]) -> Error { + unsupported(sig_alg_id) + } + + fn unsupported(_sig_alg_id: &[u8]) -> Error { + Error::UnsupportedSignatureAlgorithmContext(UnsupportedSignatureAlgorithmContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: _sig_alg_id.to_vec(), + #[cfg(feature = "alloc")] + supported_algorithms: SUPPORTED_ALGORITHMS_IN_TESTS + .iter() + .map(|&alg| alg.signature_alg_id()) + .collect(), + }) + } } diff --git a/src/signed_data.rs b/src/signed_data.rs index 99f65d47..1367d23f 100644 --- a/src/signed_data.rs +++ b/src/signed_data.rs @@ -13,7 +13,10 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. use crate::der::{self, FromDer}; -use crate::error::{DerTypeId, Error}; +use crate::error::{ + DerTypeId, Error, UnsupportedSignatureAlgorithmContext, + UnsupportedSignatureAlgorithmForPublicKeyContext, +}; use crate::verify_cert::Budget; use pki_types::SignatureVerificationAlgorithm; @@ -180,7 +183,7 @@ pub(crate) fn verify_signed_data( // Parse the signature. // - let mut found_signature_alg_match = false; + let mut invalid_for_public_key = None; for supported_alg in supported_algorithms .iter() .filter(|alg| alg.signature_alg_id().as_ref() == signed_data.algorithm.as_slice_less_safe()) @@ -191,21 +194,29 @@ pub(crate) fn verify_signed_data( signed_data.data, signed_data.signature, ) { - Err(Error::UnsupportedSignatureAlgorithmForPublicKey) => { - found_signature_alg_match = true; + Err(Error::UnsupportedSignatureAlgorithmForPublicKeyContext(cx)) => { + invalid_for_public_key = Some(cx); continue; } - result => { - return result; - } + result => return result, } } - if found_signature_alg_match { - Err(Error::UnsupportedSignatureAlgorithmForPublicKey) - } else { - Err(Error::UnsupportedSignatureAlgorithm) + if let Some(cx) = invalid_for_public_key { + return Err(Error::UnsupportedSignatureAlgorithmForPublicKeyContext(cx)); } + + Err(Error::UnsupportedSignatureAlgorithmContext( + UnsupportedSignatureAlgorithmContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: signed_data.algorithm.as_slice_less_safe().to_vec(), + #[cfg(feature = "alloc")] + supported_algorithms: supported_algorithms + .iter() + .map(|&alg| alg.signature_alg_id()) + .collect(), + }, + )) } pub(crate) fn verify_signature( @@ -216,7 +227,14 @@ pub(crate) fn verify_signature( ) -> Result<(), Error> { let spki = der::read_all::>(spki_value)?; if signature_alg.public_key_alg_id().as_ref() != spki.algorithm_id_value.as_slice_less_safe() { - return Err(Error::UnsupportedSignatureAlgorithmForPublicKey); + return Err(Error::UnsupportedSignatureAlgorithmForPublicKeyContext( + UnsupportedSignatureAlgorithmForPublicKeyContext { + #[cfg(feature = "alloc")] + signature_algorithm_id: signature_alg.signature_alg_id().as_ref().to_vec(), + #[cfg(feature = "alloc")] + public_key_algorithm_id: spki.algorithm_id_value.as_slice_less_safe().to_vec(), + }, + )); } signature_alg diff --git a/tests/generate.py b/tests/generate.py index 159fb005..d61b82f0 100755 --- a/tests/generate.py +++ b/tests/generate.py @@ -772,10 +772,10 @@ def bad_algorithms_for_key( fn %(test_name_lower)s() { let ee = include_bytes!("%(cert_path)s"); for algorithm in &[ %(unusable_algs_str)s ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } }""" % locals(), diff --git a/tests/signatures.rs b/tests/signatures.rs index 02ff886b..6f1f81af 100644 --- a/tests/signatures.rs +++ b/tests/signatures.rs @@ -123,10 +123,10 @@ fn ed25519_key_rejected_by_other_algorithms() { RSA_PSS_2048_8192_SHA384_LEGACY_KEY, RSA_PSS_2048_8192_SHA512_LEGACY_KEY, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } @@ -245,10 +245,10 @@ fn ecdsa_p256_key_rejected_by_other_algorithms() { RSA_PSS_2048_8192_SHA384_LEGACY_KEY, RSA_PSS_2048_8192_SHA512_LEGACY_KEY, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } @@ -367,10 +367,10 @@ fn ecdsa_p384_key_rejected_by_other_algorithms() { RSA_PSS_2048_8192_SHA384_LEGACY_KEY, RSA_PSS_2048_8192_SHA512_LEGACY_KEY, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } @@ -544,10 +544,10 @@ fn ecdsa_p521_key_rejected_by_other_algorithms() { RSA_PSS_2048_8192_SHA384_LEGACY_KEY, RSA_PSS_2048_8192_SHA512_LEGACY_KEY, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } @@ -879,10 +879,10 @@ fn rsa_2048_key_rejected_by_other_algorithms() { ECDSA_P384_SHA384, ED25519, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } @@ -1266,10 +1266,10 @@ fn rsa_3072_key_rejected_by_other_algorithms() { ECDSA_P384_SHA384, ED25519, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } @@ -1653,10 +1653,10 @@ fn rsa_4096_key_rejected_by_other_algorithms() { ECDSA_P384_SHA384, ED25519, ] { - assert_eq!( + assert!(matches!( check_sig(ee, *algorithm, b"", b""), - Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKey) - ); + Err(webpki::Error::UnsupportedSignatureAlgorithmForPublicKeyContext(_)) + )); } } diff --git a/tests/signatures/alg-ecdh-secp256r1.der b/tests/signatures/alg-ecdh-secp256r1.der new file mode 100644 index 00000000..d32a3fa3 --- /dev/null +++ b/tests/signatures/alg-ecdh-secp256r1.der @@ -0,0 +1 @@ ++ *H= \ No newline at end of file diff --git a/tests/signatures/alg-ecmqv-secp256r1.der b/tests/signatures/alg-ecmqv-secp256r1.der new file mode 100644 index 00000000..30b0ef89 --- /dev/null +++ b/tests/signatures/alg-ecmqv-secp256r1.der @@ -0,0 +1 @@ ++ *H= \ No newline at end of file diff --git a/tests/signatures/alg-id-ecpublickey-params-null.der b/tests/signatures/alg-id-ecpublickey-params-null.der new file mode 100644 index 00000000..730b883d Binary files /dev/null and b/tests/signatures/alg-id-ecpublickey-params-null.der differ diff --git a/tests/signatures/alg-rsa-null-params.der b/tests/signatures/alg-rsa-null-params.der new file mode 100644 index 00000000..abc9bd64 Binary files /dev/null and b/tests/signatures/alg-rsa-null-params.der differ diff --git a/tests/signatures/alg-rsae-bad-params.der b/tests/signatures/alg-rsae-bad-params.der new file mode 100644 index 00000000..7a747d4c Binary files /dev/null and b/tests/signatures/alg-rsae-bad-params.der differ diff --git a/tests/signatures/alg-rsae-sha1.der b/tests/signatures/alg-rsae-sha1.der new file mode 100644 index 00000000..56eb06c6 Binary files /dev/null and b/tests/signatures/alg-rsae-sha1.der differ diff --git a/tests/signatures/alg-rsapss-defaults.der b/tests/signatures/alg-rsapss-defaults.der new file mode 100644 index 00000000..f6018808 Binary files /dev/null and b/tests/signatures/alg-rsapss-defaults.der differ diff --git a/tests/signatures/alg-rsapss-salt23.der b/tests/signatures/alg-rsapss-salt23.der new file mode 100644 index 00000000..2843a12c --- /dev/null +++ b/tests/signatures/alg-rsapss-salt23.der @@ -0,0 +1,2 @@ + *H  +0 \ No newline at end of file diff --git a/tests/signatures/alg-rsapss-sha256-mgf1-sha256-salt10.der b/tests/signatures/alg-rsapss-sha256-mgf1-sha256-salt10.der new file mode 100644 index 00000000..f0b1b7db Binary files /dev/null and b/tests/signatures/alg-rsapss-sha256-mgf1-sha256-salt10.der differ diff --git a/tests/signatures/alg-rsapss-sha256-mgf1-sha512-salt33.der b/tests/signatures/alg-rsapss-sha256-mgf1-sha512-salt33.der new file mode 100644 index 00000000..f1c75115 Binary files /dev/null and b/tests/signatures/alg-rsapss-sha256-mgf1-sha512-salt33.der differ