From 13fa5483dad9dc2d9688cea63314ea21f8eea9f8 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 00:08:17 +0200 Subject: [PATCH 01/11] feat(xmldsig): add ecdsa signature verification --- src/xmldsig/mod.rs | 3 +- src/xmldsig/signature.rs | 175 ++++++++++++++++- tests/ecdsa_signature_integration.rs | 271 +++++++++++++++++++++++++++ 3 files changed, 443 insertions(+), 6 deletions(-) create mode 100644 tests/ecdsa_signature_integration.rs diff --git a/src/xmldsig/mod.rs b/src/xmldsig/mod.rs index 1524fe4..024f6b8 100644 --- a/src/xmldsig/mod.rs +++ b/src/xmldsig/mod.rs @@ -21,7 +21,8 @@ pub use parse::{ ParseError, Reference, SignatureAlgorithm, SignedInfo, find_signature_node, parse_signed_info, }; pub use signature::{ - SignatureVerificationError, verify_rsa_signature_pem, verify_rsa_signature_spki, + SignatureVerificationError, verify_ecdsa_signature_pem, verify_ecdsa_signature_spki, + verify_rsa_signature_pem, verify_rsa_signature_spki, }; pub use transforms::{Transform, execute_transforms, parse_transforms}; pub use types::{NodeSet, TransformData, TransformError}; diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 80240e1..7b924e4 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -1,15 +1,17 @@ -//! RSA signature verification helpers for XMLDSig. +//! Signature verification helpers for XMLDSig. //! -//! This module covers roadmap task P1-019: RSA PKCS#1 v1.5 verification for -//! `rsa-sha1`, `rsa-sha256`, `rsa-sha384`, and `rsa-sha512`. +//! This module currently covers roadmap task P1-019 (RSA PKCS#1 v1.5) and +//! P1-020 (ECDSA P-256/P-384) verification. //! //! Input public keys are accepted in SubjectPublicKeyInfo (SPKI) form because //! that is how the vendored PEM fixtures are stored. `ring` expects the inner -//! ASN.1 `RSAPublicKey` bytes, so we validate and unwrap SPKI first. +//! SPKI payload for both algorithm families: +//! - RSA: ASN.1 `RSAPublicKey` +//! - ECDSA: uncompressed SEC1 EC point bytes from the SPKI bit string use ring::signature; use x509_parser::prelude::FromDer; -use x509_parser::public_key::PublicKey; +use x509_parser::public_key::{ECPoint, PublicKey}; use x509_parser::x509::SubjectPublicKeyInfo; use super::parse::SignatureAlgorithm; @@ -39,6 +41,11 @@ pub enum SignatureVerificationError { /// The provided DER bytes were not a valid SPKI-encoded RSA public key. #[error("invalid RSA SubjectPublicKeyInfo DER")] InvalidKeyDer, + + /// The provided ECDSA signature bytes were neither XMLDSig fixed-width + /// nor ASN.1 DER encoded. + #[error("invalid ECDSA signature encoding")] + InvalidSignatureFormat, } /// Verify an RSA XMLDSig signature using a PEM-encoded SPKI public key. @@ -64,6 +71,31 @@ pub fn verify_rsa_signature_pem( verify_rsa_signature_spki(algorithm, &pem.contents, signed_data, signature_value) } +/// Verify an ECDSA XMLDSig signature using a PEM-encoded SPKI public key. +/// +/// The PEM must contain a `PUBLIC KEY` block. The signature value must use the +/// XMLDSig fixed-width `r || s` format required by RFC 6931 / XMLDSig 1.1. +/// Returns `Ok(false)` for signature mismatch and `Err` for +/// algorithm/key/signature-format preparation errors. +#[must_use = "discarding the verification result skips signature validation"] +pub fn verify_ecdsa_signature_pem( + algorithm: SignatureAlgorithm, + public_key_pem: &str, + signed_data: &[u8], + signature_value: &[u8], +) -> Result { + let (rest, pem) = x509_parser::pem::parse_x509_pem(public_key_pem.as_bytes()) + .map_err(|_| SignatureVerificationError::InvalidKeyPem)?; + if !rest.iter().all(|byte| byte.is_ascii_whitespace()) { + return Err(SignatureVerificationError::InvalidKeyPem); + } + if pem.label != "PUBLIC KEY" { + return Err(SignatureVerificationError::InvalidKeyFormat { label: pem.label }); + } + + verify_ecdsa_signature_spki(algorithm, &pem.contents, signed_data, signature_value) +} + /// Verify an RSA XMLDSig signature using DER-encoded SPKI public key bytes. /// /// The input must be an X.509 `SubjectPublicKeyInfo` wrapping an RSA key. @@ -99,6 +131,41 @@ pub fn verify_rsa_signature_spki( } } +/// Verify an ECDSA XMLDSig signature using DER-encoded SPKI public key bytes. +/// +/// The input must be an X.509 `SubjectPublicKeyInfo` wrapping an EC key. +/// Returns `Ok(false)` for signature mismatch and `Err` for algorithm/key +/// preparation errors. +#[must_use = "discarding the verification result skips signature validation"] +pub fn verify_ecdsa_signature_spki( + algorithm: SignatureAlgorithm, + public_key_spki_der: &[u8], + signed_data: &[u8], + signature_value: &[u8], +) -> Result { + let (rest, spki) = SubjectPublicKeyInfo::from_der(public_key_spki_der) + .map_err(|_| SignatureVerificationError::InvalidKeyDer)?; + if !rest.is_empty() { + return Err(SignatureVerificationError::InvalidKeyDer); + } + let public_key = spki + .parsed() + .map_err(|_| SignatureVerificationError::InvalidKeyDer)?; + + match public_key { + PublicKey::EC(ec) => { + let verification_algorithm = + ecdsa_verification_algorithm(&spki, &ec, algorithm, signature_value)?; + let key = signature::UnparsedPublicKey::new( + verification_algorithm, + spki.subject_public_key.data, + ); + Ok(key.verify(signed_data, signature_value).is_ok()) + } + _ => Err(SignatureVerificationError::InvalidKeyDer), + } +} + fn validate_rsa_public_key( rsa: &x509_parser::public_key::RSAPublicKey<'_>, algorithm: SignatureAlgorithm, @@ -162,6 +229,104 @@ fn verification_algorithm( } } +fn ecdsa_verification_algorithm( + spki: &SubjectPublicKeyInfo<'_>, + ec: &ECPoint<'_>, + algorithm: SignatureAlgorithm, + signature_value: &[u8], +) -> Result<&'static dyn signature::VerificationAlgorithm, SignatureVerificationError> { + let curve_oid = spki + .algorithm + .parameters + .as_ref() + .and_then(|params| params.as_oid().ok()) + .ok_or(SignatureVerificationError::InvalidKeyDer)?; + let point_len = ec.key_size(); + + let (fixed_algorithm, asn1_algorithm) = + match (curve_oid.to_id_string().as_str(), point_len, algorithm) { + ("1.2.840.10045.3.1.7", 256, SignatureAlgorithm::EcdsaP256Sha256) => Ok(( + &signature::ECDSA_P256_SHA256_FIXED, + &signature::ECDSA_P256_SHA256_ASN1, + )), + ("1.3.132.0.34", 384, SignatureAlgorithm::EcdsaP384Sha384) => Ok(( + &signature::ECDSA_P384_SHA384_FIXED, + &signature::ECDSA_P384_SHA384_ASN1, + )), + ("1.2.840.10045.3.1.7", _, _) | ("1.3.132.0.34", _, _) | (_, 256, _) | (_, 384, _) => { + Err(SignatureVerificationError::UnsupportedAlgorithm { + uri: algorithm.uri().to_string(), + }) + } + _ => Err(SignatureVerificationError::InvalidKeyDer), + }?; + + if uses_xmlsig_fixed_signature(signature_value, ec)? { + Ok(fixed_algorithm) + } else { + Ok(asn1_algorithm) + } +} + +fn uses_xmlsig_fixed_signature( + signature_value: &[u8], + ec: &ECPoint<'_>, +) -> Result { + let key_size_bytes = ec + .key_size() + .checked_div(8) + .ok_or(SignatureVerificationError::InvalidKeyDer)?; + let expected_len = key_size_bytes + .checked_mul(2) + .ok_or(SignatureVerificationError::InvalidKeyDer)?; + + if signature_value.len() != expected_len { + if looks_like_der_sequence(signature_value) { + return Ok(false); + } + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + + Ok(true) +} + +fn looks_like_der_sequence(signature_value: &[u8]) -> bool { + let Some((&tag, rest)) = signature_value.split_first() else { + return false; + }; + if tag != 0x30 { + return false; + } + + let Some((&len_byte, remainder)) = rest.split_first() else { + return false; + }; + + if len_byte & 0x80 == 0 { + return usize::from(len_byte) == remainder.len(); + } + + let len_len = usize::from(len_byte & 0x7f); + if len_len == 0 || len_len > std::mem::size_of::() || remainder.len() < len_len { + return false; + } + + let (len_bytes, content) = remainder.split_at(len_len); + let mut declared_len = 0_usize; + for &byte in len_bytes { + declared_len = match declared_len.checked_mul(256) { + Some(len) => len, + None => return false, + }; + declared_len = match declared_len.checked_add(usize::from(byte)) { + Some(len) => len, + None => return false, + }; + } + + declared_len == content.len() +} + #[cfg(test)] #[expect(clippy::unwrap_used, reason = "unit tests use fixed fixture data")] mod tests { diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs new file mode 100644 index 0000000..04ef759 --- /dev/null +++ b/tests/ecdsa_signature_integration.rs @@ -0,0 +1,271 @@ +//! Integration tests for XMLDSig ECDSA signature verification. +//! +//! These tests validate roadmap task P1-020: canonicalized `` bytes +//! plus real donor EC public keys must verify against XMLDSig raw `r || s` +//! `SignatureValue` bytes for the declared `SignatureMethod`. + +use std::path::Path; + +use base64::Engine; +use ring::rand::SystemRandom; +use ring::signature::{ECDSA_P384_SHA384_FIXED_SIGNING, EcdsaKeyPair}; +use xml_sec::c14n::canonicalize; +use xml_sec::xmldsig::parse::{SignatureAlgorithm, find_signature_node, parse_signed_info}; +use xml_sec::xmldsig::signature::{ + SignatureVerificationError, verify_ecdsa_signature_pem, verify_ecdsa_signature_spki, +}; + +fn read_fixture(path: &Path) -> String { + std::fs::read_to_string(path) + .unwrap_or_else(|err| panic!("failed to read fixture {}: {err}", path.display())) +} + +fn canonicalized_signed_info_and_signature(xml: &str) -> (SignatureAlgorithm, Vec, Vec) { + let doc = roxmltree::Document::parse(xml).expect("fixture XML should parse"); + let signature_node = find_signature_node(&doc).expect("fixture XML should contain Signature"); + let signed_info_node = signature_node + .children() + .find(|node| node.is_element() && node.tag_name().name() == "SignedInfo") + .expect("fixture XML should contain SignedInfo"); + let signed_info = parse_signed_info(signed_info_node).expect("SignedInfo should parse"); + + let mut canonical = Vec::new(); + canonicalize( + &doc, + Some(&|node| { + node == signed_info_node + || node + .ancestors() + .any(|ancestor| ancestor == signed_info_node) + }), + &signed_info.c14n_method, + &mut canonical, + ) + .expect("SignedInfo canonicalization should succeed"); + + let signature_value_text = signature_node + .children() + .find(|node| node.is_element() && node.tag_name().name() == "SignatureValue") + .and_then(|node| node.text()) + .expect("fixture XML should contain SignatureValue text") + .chars() + .filter(|ch| !ch.is_whitespace()) + .collect::(); + let signature_value = base64::engine::general_purpose::STANDARD + .decode(signature_value_text) + .expect("SignatureValue should be valid base64"); + + (signed_info.signature_method, canonical, signature_value) +} + +fn assert_donor_signature_valid( + xml_path: &Path, + public_key_path: &Path, + expected_algorithm: SignatureAlgorithm, +) { + let xml = read_fixture(xml_path); + let public_key_pem = read_fixture(public_key_path); + let (algorithm, canonical_signed_info, signature_value) = + canonicalized_signed_info_and_signature(&xml); + + assert_eq!(algorithm, expected_algorithm, "unexpected SignatureMethod"); + + let valid = verify_ecdsa_signature_pem( + algorithm, + &public_key_pem, + &canonical_signed_info, + &signature_value, + ) + .expect("ECDSA verification should not error on valid fixtures"); + assert!(valid, "donor ECDSA signature should verify"); +} + +#[test] +fn donor_ecdsa_p256_signature_matches() { + assert_donor_signature_valid( + Path::new("tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha256-ecdsa-sha256.xml"), + Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem"), + SignatureAlgorithm::EcdsaP256Sha256, + ); +} + +#[test] +fn local_p384_signature_matches() { + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha384-ecdsa-sha384.xml", + )); + let private_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-key.pem")); + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); + let (_, canonical_signed_info, _) = canonicalized_signed_info_and_signature(&xml); + + let pkcs8_der = x509_parser::pem::parse_x509_pem(private_key_pem.as_bytes()) + .expect("fixture PEM should parse") + .1 + .contents; + let rng = SystemRandom::new(); + let key_pair = EcdsaKeyPair::from_pkcs8(&ECDSA_P384_SHA384_FIXED_SIGNING, &pkcs8_der, &rng) + .expect("fixture PKCS#8 should parse"); + let signature = key_pair + .sign(&rng, &canonical_signed_info) + .expect("fixture P-384 key should sign"); + + let valid = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP384Sha384, + &public_key_pem, + &canonical_signed_info, + signature.as_ref(), + ) + .expect("P-384 verification should not error on valid fixtures"); + + assert!(valid, "locally signed P-384 signature should verify"); +} + +#[test] +fn tampered_signed_info_fails_verification() { + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha256-ecdsa-sha256.xml", + )); + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem")); + let (algorithm, mut canonical_signed_info, signature_value) = + canonicalized_signed_info_and_signature(&xml); + + let last = canonical_signed_info + .last_mut() + .expect("canonical SignedInfo should not be empty"); + *last ^= 0x01; + + let valid = verify_ecdsa_signature_pem( + algorithm, + &public_key_pem, + &canonical_signed_info, + &signature_value, + ) + .expect("tampered data should still be a valid verification attempt"); + + assert!( + !valid, + "tampering SignedInfo bytes must break ECDSA signature verification" + ); +} + +#[test] +fn curve_mismatched_public_key_returns_unsupported_algorithm() { + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha256-ecdsa-sha256.xml", + )); + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); + let (algorithm, canonical_signed_info, signature_value) = + canonicalized_signed_info_and_signature(&xml); + + let err = verify_ecdsa_signature_pem( + algorithm, + &public_key_pem, + &canonical_signed_info, + &signature_value, + ) + .expect_err("curve-mismatched EC key should be rejected before verification"); + + assert!(matches!( + err, + SignatureVerificationError::UnsupportedAlgorithm { .. } + )); +} + +#[test] +fn non_public_key_pem_returns_invalid_key_format() { + let err = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP256Sha256, + "-----BEGIN CERTIFICATE-----\nZm9v\n-----END CERTIFICATE-----\n", + b"payload", + &[0_u8; 64], + ) + .expect_err("non-public-key PEM should be rejected"); + + assert!(matches!( + err, + SignatureVerificationError::InvalidKeyFormat { .. } + )); +} + +#[test] +fn malformed_pem_returns_typed_error() { + let err = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP256Sha256, + "-----BEGIN PUBLIC KEY-----\n%%%%\n-----END PUBLIC KEY-----\n", + b"payload", + &[0_u8; 64], + ) + .expect_err("corrupt PEM should be rejected"); + + assert!(matches!(err, SignatureVerificationError::InvalidKeyPem)); +} + +#[test] +fn pem_with_trailing_garbage_returns_typed_error() { + let public_key_pem = format!( + "{}TRAILING", + read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem")) + ); + + let err = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP256Sha256, + &public_key_pem, + b"payload", + &[0_u8; 64], + ) + .expect_err("PEM with trailing garbage should be rejected"); + + assert!(matches!(err, SignatureVerificationError::InvalidKeyPem)); +} + +#[test] +fn malformed_spki_der_returns_typed_error() { + let err = verify_ecdsa_signature_spki( + SignatureAlgorithm::EcdsaP256Sha256, + &[0x01, 0x02, 0x03], + b"payload", + &[0_u8; 64], + ) + .expect_err("malformed SPKI DER should be rejected"); + + assert!(matches!(err, SignatureVerificationError::InvalidKeyDer)); +} + +#[test] +fn signature_with_wrong_length_returns_typed_error() { + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem")); + + let err = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP256Sha256, + &public_key_pem, + b"payload", + &[0_u8; 63], + ) + .expect_err("odd-sized XMLDSig ECDSA signature should be rejected"); + + assert!(matches!( + err, + SignatureVerificationError::InvalidSignatureFormat + )); +} + +#[test] +fn spki_der_with_trailing_garbage_returns_typed_error() { + let mut public_key_der = x509_parser::pem::parse_x509_pem( + read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem")).as_bytes(), + ) + .expect("fixture PEM should parse") + .1 + .contents; + public_key_der.extend_from_slice(b"TRAILING"); + + let err = verify_ecdsa_signature_spki( + SignatureAlgorithm::EcdsaP256Sha256, + &public_key_der, + b"payload", + &[0_u8; 64], + ) + .expect_err("SPKI DER with trailing garbage should be rejected"); + + assert!(matches!(err, SignatureVerificationError::InvalidKeyDer)); +} From 9c172148f22449804069071ad509871530df65a5 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 00:29:10 +0200 Subject: [PATCH 02/11] fix(xmldsig): harden ecdsa signature parsing --- src/xmldsig/signature.rs | 220 ++++++++++++++++++++------- tests/ecdsa_signature_integration.rs | 77 +++++++++- 2 files changed, 237 insertions(+), 60 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 7b924e4..868208d 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -16,7 +16,7 @@ use x509_parser::x509::SubjectPublicKeyInfo; use super::parse::SignatureAlgorithm; -/// Errors while preparing or running RSA signature verification. +/// Errors while preparing or running XMLDSig signature verification. #[derive(Debug, thiserror::Error)] #[non_exhaustive] pub enum SignatureVerificationError { @@ -38,10 +38,17 @@ pub enum SignatureVerificationError { label: String, }, - /// The provided DER bytes were not a valid SPKI-encoded RSA public key. - #[error("invalid RSA SubjectPublicKeyInfo DER")] + /// The provided DER bytes were not a valid SPKI-encoded public key. + #[error("invalid SubjectPublicKeyInfo DER")] InvalidKeyDer, + /// The provided public key curve does not match the signature algorithm. + #[error("public key does not match signature algorithm: {uri}")] + KeyAlgorithmMismatch { + /// XMLDSig algorithm URI used for diagnostics. + uri: String, + }, + /// The provided ECDSA signature bytes were neither XMLDSig fixed-width /// nor ASN.1 DER encoded. #[error("invalid ECDSA signature encoding")] @@ -73,10 +80,13 @@ pub fn verify_rsa_signature_pem( /// Verify an ECDSA XMLDSig signature using a PEM-encoded SPKI public key. /// -/// The PEM must contain a `PUBLIC KEY` block. The signature value must use the -/// XMLDSig fixed-width `r || s` format required by RFC 6931 / XMLDSig 1.1. -/// Returns `Ok(false)` for signature mismatch and `Err` for -/// algorithm/key/signature-format preparation errors. +/// The PEM must contain a `PUBLIC KEY` block. The signature value is expected +/// to use the XMLDSig fixed-width `r || s` format required by RFC 6931 / +/// XMLDSig 1.1, but ASN.1 DER-encoded ECDSA signatures are also accepted as an +/// interop fallback. Returns `Ok(false)` for signature mismatch and `Err` for +/// algorithm/key/signature-format preparation errors (including +/// `InvalidSignatureFormat` when the bytes are neither valid fixed-width +/// `r || s` nor valid ASN.1 DER ECDSA). #[must_use = "discarding the verification result skips signature validation"] pub fn verify_ecdsa_signature_pem( algorithm: SignatureAlgorithm, @@ -133,9 +143,11 @@ pub fn verify_rsa_signature_spki( /// Verify an ECDSA XMLDSig signature using DER-encoded SPKI public key bytes. /// -/// The input must be an X.509 `SubjectPublicKeyInfo` wrapping an EC key. -/// Returns `Ok(false)` for signature mismatch and `Err` for algorithm/key -/// preparation errors. +/// The input must be an X.509 `SubjectPublicKeyInfo` wrapping an EC key. The +/// signature value may be either XMLDSig fixed-width `r || s` bytes or ASN.1 +/// DER-encoded ECDSA for interop compatibility. Returns `Ok(false)` for +/// signature mismatch and `Err` for algorithm/key/signature-format preparation +/// errors. #[must_use = "discarding the verification result skips signature validation"] pub fn verify_ecdsa_signature_spki( algorithm: SignatureAlgorithm, @@ -243,88 +255,184 @@ fn ecdsa_verification_algorithm( .ok_or(SignatureVerificationError::InvalidKeyDer)?; let point_len = ec.key_size(); - let (fixed_algorithm, asn1_algorithm) = - match (curve_oid.to_id_string().as_str(), point_len, algorithm) { - ("1.2.840.10045.3.1.7", 256, SignatureAlgorithm::EcdsaP256Sha256) => Ok(( - &signature::ECDSA_P256_SHA256_FIXED, - &signature::ECDSA_P256_SHA256_ASN1, - )), - ("1.3.132.0.34", 384, SignatureAlgorithm::EcdsaP384Sha384) => Ok(( - &signature::ECDSA_P384_SHA384_FIXED, - &signature::ECDSA_P384_SHA384_ASN1, - )), - ("1.2.840.10045.3.1.7", _, _) | ("1.3.132.0.34", _, _) | (_, 256, _) | (_, 384, _) => { - Err(SignatureVerificationError::UnsupportedAlgorithm { + let curve_oid = curve_oid.to_id_string(); + let (fixed_algorithm, asn1_algorithm, component_len) = match algorithm { + SignatureAlgorithm::EcdsaP256Sha256 => { + if curve_oid == "1.2.840.10045.3.1.7" && point_len == 256 { + ( + &signature::ECDSA_P256_SHA256_FIXED, + &signature::ECDSA_P256_SHA256_ASN1, + 32, + ) + } else if curve_oid == "1.2.840.10045.3.1.7" + || curve_oid == "1.3.132.0.34" + || point_len == 256 + || point_len == 384 + { + return Err(SignatureVerificationError::KeyAlgorithmMismatch { + uri: algorithm.uri().to_string(), + }); + } else { + return Err(SignatureVerificationError::InvalidKeyDer); + } + } + SignatureAlgorithm::EcdsaP384Sha384 => { + if curve_oid == "1.3.132.0.34" && point_len == 384 { + ( + &signature::ECDSA_P384_SHA384_FIXED, + &signature::ECDSA_P384_SHA384_ASN1, + 48, + ) + } else if curve_oid == "1.2.840.10045.3.1.7" + || curve_oid == "1.3.132.0.34" + || point_len == 256 + || point_len == 384 + { + return Err(SignatureVerificationError::KeyAlgorithmMismatch { uri: algorithm.uri().to_string(), - }) + }); + } else { + return Err(SignatureVerificationError::InvalidKeyDer); } - _ => Err(SignatureVerificationError::InvalidKeyDer), - }?; + } + _ => { + return Err(SignatureVerificationError::UnsupportedAlgorithm { + uri: algorithm.uri().to_string(), + }); + } + }; - if uses_xmlsig_fixed_signature(signature_value, ec)? { - Ok(fixed_algorithm) - } else { - Ok(asn1_algorithm) + match classify_ecdsa_signature_encoding(signature_value, component_len)? { + EcdsaSignatureEncoding::XmlDsigFixed => Ok(fixed_algorithm), + EcdsaSignatureEncoding::Asn1Der => Ok(asn1_algorithm), } } -fn uses_xmlsig_fixed_signature( +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum EcdsaSignatureEncoding { + XmlDsigFixed, + Asn1Der, +} + +fn classify_ecdsa_signature_encoding( signature_value: &[u8], - ec: &ECPoint<'_>, -) -> Result { - let key_size_bytes = ec - .key_size() - .checked_div(8) - .ok_or(SignatureVerificationError::InvalidKeyDer)?; - let expected_len = key_size_bytes + component_len: usize, +) -> Result { + let expected_len = component_len .checked_mul(2) .ok_or(SignatureVerificationError::InvalidKeyDer)?; - if signature_value.len() != expected_len { - if looks_like_der_sequence(signature_value) { - return Ok(false); - } - return Err(SignatureVerificationError::InvalidSignatureFormat); + match inspect_der_encoded_ecdsa_signature(signature_value, component_len)? { + Some(()) => Ok(EcdsaSignatureEncoding::Asn1Der), + None if signature_value.len() == expected_len => Ok(EcdsaSignatureEncoding::XmlDsigFixed), + None => Err(SignatureVerificationError::InvalidSignatureFormat), } - - Ok(true) } -fn looks_like_der_sequence(signature_value: &[u8]) -> bool { +fn inspect_der_encoded_ecdsa_signature( + signature_value: &[u8], + component_len: usize, +) -> Result, SignatureVerificationError> { let Some((&tag, rest)) = signature_value.split_first() else { - return false; + return Ok(None); }; if tag != 0x30 { - return false; + return Ok(None); + } + + let sequence = parse_der_length(rest) + .ok_or(SignatureVerificationError::InvalidSignatureFormat)? + .map_err(|_| SignatureVerificationError::InvalidSignatureFormat)?; + let (sequence_len, sequence_rest) = sequence; + let (sequence_content, trailing) = sequence_rest + .split_at_checked(sequence_len) + .ok_or(SignatureVerificationError::InvalidSignatureFormat)?; + if !trailing.is_empty() { + return Err(SignatureVerificationError::InvalidSignatureFormat); } - let Some((&len_byte, remainder)) = rest.split_first() else { - return false; + let after_r = parse_der_integer(sequence_content, component_len)?; + let after_s = parse_der_integer(after_r, component_len)?; + if !after_s.is_empty() { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + + Ok(Some(())) +} + +fn parse_der_integer( + input: &[u8], + component_len: usize, +) -> Result<&[u8], SignatureVerificationError> { + let Some((&tag, rest)) = input.split_first() else { + return Err(SignatureVerificationError::InvalidSignatureFormat); + }; + if tag != 0x02 { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + + let (len, rest) = parse_der_length(rest) + .ok_or(SignatureVerificationError::InvalidSignatureFormat)? + .map_err(|_| SignatureVerificationError::InvalidSignatureFormat)?; + let (integer_bytes, remainder) = rest + .split_at_checked(len) + .ok_or(SignatureVerificationError::InvalidSignatureFormat)?; + + if integer_bytes.is_empty() { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + if integer_bytes.len() > component_len + 1 { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + if integer_bytes[0] & 0x80 != 0 { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + if integer_bytes.len() > 1 && integer_bytes[0] == 0 && integer_bytes[1] & 0x80 == 0 { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + + let magnitude = if integer_bytes[0] == 0 { + &integer_bytes[1..] + } else { + integer_bytes }; + if magnitude.is_empty() || magnitude.len() > component_len { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } + + Ok(remainder) +} + +fn parse_der_length(input: &[u8]) -> Option> { + let (&len_byte, rest) = input.split_first()?; if len_byte & 0x80 == 0 { - return usize::from(len_byte) == remainder.len(); + return Some(Ok((usize::from(len_byte), rest))); } let len_len = usize::from(len_byte & 0x7f); - if len_len == 0 || len_len > std::mem::size_of::() || remainder.len() < len_len { - return false; + if len_len == 0 || len_len > std::mem::size_of::() || rest.len() < len_len { + return Some(Err(())); + } + + let (len_bytes, remainder) = rest.split_at(len_len); + if len_bytes[0] == 0 { + return Some(Err(())); } - let (len_bytes, content) = remainder.split_at(len_len); let mut declared_len = 0_usize; for &byte in len_bytes { declared_len = match declared_len.checked_mul(256) { Some(len) => len, - None => return false, + None => return Some(Err(())), }; declared_len = match declared_len.checked_add(usize::from(byte)) { Some(len) => len, - None => return false, + None => return Some(Err(())), }; } - declared_len == content.len() + Some(Ok((declared_len, remainder))) } #[cfg(test)] diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs index 04ef759..f7c9626 100644 --- a/tests/ecdsa_signature_integration.rs +++ b/tests/ecdsa_signature_integration.rs @@ -8,7 +8,9 @@ use std::path::Path; use base64::Engine; use ring::rand::SystemRandom; -use ring::signature::{ECDSA_P384_SHA384_FIXED_SIGNING, EcdsaKeyPair}; +use ring::signature::{ + ECDSA_P384_SHA384_ASN1_SIGNING, ECDSA_P384_SHA384_FIXED_SIGNING, EcdsaKeyPair, +}; use xml_sec::c14n::canonicalize; use xml_sec::xmldsig::parse::{SignatureAlgorithm, find_signature_node, parse_signed_info}; use xml_sec::xmldsig::signature::{ @@ -96,7 +98,13 @@ fn local_p384_signature_matches() { )); let private_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-key.pem")); let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); - let (_, canonical_signed_info, _) = canonicalized_signed_info_and_signature(&xml); + let (signature_algorithm, canonical_signed_info, _) = + canonicalized_signed_info_and_signature(&xml); + assert_eq!( + SignatureAlgorithm::EcdsaP384Sha384, + signature_algorithm, + "fixture SignatureMethod should be EcdsaP384Sha384", + ); let pkcs8_der = x509_parser::pem::parse_x509_pem(private_key_pem.as_bytes()) .expect("fixture PEM should parse") @@ -120,6 +128,43 @@ fn local_p384_signature_matches() { assert!(valid, "locally signed P-384 signature should verify"); } +#[test] +fn local_p384_der_signature_matches() { + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha384-ecdsa-sha384.xml", + )); + let private_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-key.pem")); + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); + let (signature_algorithm, canonical_signed_info, _) = + canonicalized_signed_info_and_signature(&xml); + assert_eq!( + SignatureAlgorithm::EcdsaP384Sha384, + signature_algorithm, + "fixture SignatureMethod should be EcdsaP384Sha384", + ); + + let pkcs8_der = x509_parser::pem::parse_x509_pem(private_key_pem.as_bytes()) + .expect("fixture PEM should parse") + .1 + .contents; + let rng = SystemRandom::new(); + let key_pair = EcdsaKeyPair::from_pkcs8(&ECDSA_P384_SHA384_ASN1_SIGNING, &pkcs8_der, &rng) + .expect("fixture PKCS#8 should parse"); + let signature = key_pair + .sign(&rng, &canonical_signed_info) + .expect("fixture P-384 key should sign"); + + let valid = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP384Sha384, + &public_key_pem, + &canonical_signed_info, + signature.as_ref(), + ) + .expect("P-384 DER verification should not error on valid fixtures"); + + assert!(valid, "locally signed DER P-384 signature should verify"); +} + #[test] fn tampered_signed_info_fails_verification() { let xml = read_fixture(Path::new( @@ -149,7 +194,7 @@ fn tampered_signed_info_fails_verification() { } #[test] -fn curve_mismatched_public_key_returns_unsupported_algorithm() { +fn curve_mismatched_public_key_returns_typed_key_error() { let xml = read_fixture(Path::new( "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha256-ecdsa-sha256.xml", )); @@ -167,7 +212,7 @@ fn curve_mismatched_public_key_returns_unsupported_algorithm() { assert!(matches!( err, - SignatureVerificationError::UnsupportedAlgorithm { .. } + SignatureVerificationError::KeyAlgorithmMismatch { .. } )); } @@ -249,6 +294,30 @@ fn signature_with_wrong_length_returns_typed_error() { )); } +#[test] +fn malformed_der_signature_returns_typed_error() { + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); + let malformed_der_signature = { + let mut signature = vec![0_u8; 96]; + signature[0] = 0x30; + signature[1] = 94; + signature + }; + + let err = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP384Sha384, + &public_key_pem, + b"payload", + &malformed_der_signature, + ) + .expect_err("malformed DER-encoded signature should be rejected"); + + assert!(matches!( + err, + SignatureVerificationError::InvalidSignatureFormat + )); +} + #[test] fn spki_der_with_trailing_garbage_returns_typed_error() { let mut public_key_der = x509_parser::pem::parse_x509_pem( From c2b4dce428d3a05d7404fad93006dfa0a25a664f Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 00:42:53 +0200 Subject: [PATCH 03/11] docs(xmldsig): clarify strict der parsing fallback --- src/xmldsig/signature.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 868208d..19875cb 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -340,6 +340,9 @@ fn inspect_der_encoded_ecdsa_signature( return Ok(None); } + // Do not silently fall back to raw `r || s` once input starts with DER + // SEQUENCE tag (0x30): malformed DER must stay a typed format error rather + // than being treated as an ordinary signature mismatch. let sequence = parse_der_length(rest) .ok_or(SignatureVerificationError::InvalidSignatureFormat)? .map_err(|_| SignatureVerificationError::InvalidSignatureFormat)?; From 9158892cda2a5abf3941977f67938139c7298387 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 01:18:40 +0200 Subject: [PATCH 04/11] fix(xmldsig): handle raw signatures with 0x30 prefix --- src/xmldsig/signature.rs | 13 +++--- tests/ecdsa_signature_integration.rs | 60 ++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 19875cb..2fcef23 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -322,10 +322,12 @@ fn classify_ecdsa_signature_encoding( .checked_mul(2) .ok_or(SignatureVerificationError::InvalidKeyDer)?; - match inspect_der_encoded_ecdsa_signature(signature_value, component_len)? { - Some(()) => Ok(EcdsaSignatureEncoding::Asn1Der), - None if signature_value.len() == expected_len => Ok(EcdsaSignatureEncoding::XmlDsigFixed), - None => Err(SignatureVerificationError::InvalidSignatureFormat), + match inspect_der_encoded_ecdsa_signature(signature_value, component_len) { + Ok(Some(())) => Ok(EcdsaSignatureEncoding::Asn1Der), + Ok(None) | Err(_) if signature_value.len() == expected_len => { + Ok(EcdsaSignatureEncoding::XmlDsigFixed) + } + Ok(None) | Err(_) => Err(SignatureVerificationError::InvalidSignatureFormat), } } @@ -340,9 +342,6 @@ fn inspect_der_encoded_ecdsa_signature( return Ok(None); } - // Do not silently fall back to raw `r || s` once input starts with DER - // SEQUENCE tag (0x30): malformed DER must stay a typed format error rather - // than being treated as an ordinary signature mismatch. let sequence = parse_der_length(rest) .ok_or(SignatureVerificationError::InvalidSignatureFormat)? .map_err(|_| SignatureVerificationError::InvalidSignatureFormat)?; diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs index f7c9626..e1b683d 100644 --- a/tests/ecdsa_signature_integration.rs +++ b/tests/ecdsa_signature_integration.rs @@ -13,7 +13,7 @@ use ring::signature::{ }; use xml_sec::c14n::canonicalize; use xml_sec::xmldsig::parse::{SignatureAlgorithm, find_signature_node, parse_signed_info}; -use xml_sec::xmldsig::signature::{ +use xml_sec::xmldsig::{ SignatureVerificationError, verify_ecdsa_signature_pem, verify_ecdsa_signature_spki, }; @@ -128,6 +128,58 @@ fn local_p384_signature_matches() { assert!(valid, "locally signed P-384 signature should verify"); } +#[test] +fn local_p384_raw_signature_with_der_like_prefix_matches() { + const MAX_ATTEMPTS: usize = 8192; + + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha384-ecdsa-sha384.xml", + )); + let private_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-key.pem")); + let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); + let (signature_algorithm, canonical_signed_info, _) = + canonicalized_signed_info_and_signature(&xml); + assert_eq!( + SignatureAlgorithm::EcdsaP384Sha384, + signature_algorithm, + "fixture SignatureMethod should be EcdsaP384Sha384", + ); + + let pkcs8_der = x509_parser::pem::parse_x509_pem(private_key_pem.as_bytes()) + .expect("fixture PEM should parse") + .1 + .contents; + let rng = SystemRandom::new(); + let key_pair = EcdsaKeyPair::from_pkcs8(&ECDSA_P384_SHA384_FIXED_SIGNING, &pkcs8_der, &rng) + .expect("fixture PKCS#8 should parse"); + + for _ in 0..MAX_ATTEMPTS { + let signature = key_pair + .sign(&rng, &canonical_signed_info) + .expect("fixture P-384 key should sign"); + if signature.as_ref().first().copied() != Some(0x30) { + continue; + } + + let valid = verify_ecdsa_signature_pem( + SignatureAlgorithm::EcdsaP384Sha384, + &public_key_pem, + &canonical_signed_info, + signature.as_ref(), + ); + + assert!( + matches!(valid, Ok(true)), + "fixed-width signature with leading 0x30 must verify as raw r||s, got {valid:?}" + ); + return; + } + + panic!( + "failed to generate a fixed-width P-384 signature starting with 0x30 after {MAX_ATTEMPTS} attempts" + ); +} + #[test] fn local_p384_der_signature_matches() { let xml = read_fixture(Path::new( @@ -295,12 +347,12 @@ fn signature_with_wrong_length_returns_typed_error() { } #[test] -fn malformed_der_signature_returns_typed_error() { +fn malformed_der_signature_with_non_raw_length_returns_typed_error() { let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); let malformed_der_signature = { - let mut signature = vec![0_u8; 96]; + let mut signature = vec![0_u8; 95]; signature[0] = 0x30; - signature[1] = 94; + signature[1] = 93; signature }; From 6eee59f32243b5228f1166b476401207fbd20a5f Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 07:48:23 +0200 Subject: [PATCH 05/11] fix(xmldsig): harden ecdsa key validation - validate EC SPKI point encoding before ring verification - reject DER overlong lengths and keep 0x30 raw signatures deterministic - add regression coverage for invalid EC point prefix and DER classifier edge cases --- src/xmldsig/signature.rs | 64 +++++++++++++++---- tests/ecdsa_signature_integration.rs | 95 ++++++++++++---------------- 2 files changed, 92 insertions(+), 67 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 2fcef23..9d2c30c 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -166,6 +166,7 @@ pub fn verify_ecdsa_signature_spki( match public_key { PublicKey::EC(ec) => { + validate_ec_public_key_encoding(&ec, &spki.subject_public_key.data)?; let verification_algorithm = ecdsa_verification_algorithm(&spki, &ec, algorithm, signature_value)?; let key = signature::UnparsedPublicKey::new( @@ -264,16 +265,10 @@ fn ecdsa_verification_algorithm( &signature::ECDSA_P256_SHA256_ASN1, 32, ) - } else if curve_oid == "1.2.840.10045.3.1.7" - || curve_oid == "1.3.132.0.34" - || point_len == 256 - || point_len == 384 - { + } else { return Err(SignatureVerificationError::KeyAlgorithmMismatch { uri: algorithm.uri().to_string(), }); - } else { - return Err(SignatureVerificationError::InvalidKeyDer); } } SignatureAlgorithm::EcdsaP384Sha384 => { @@ -283,16 +278,10 @@ fn ecdsa_verification_algorithm( &signature::ECDSA_P384_SHA384_ASN1, 48, ) - } else if curve_oid == "1.2.840.10045.3.1.7" - || curve_oid == "1.3.132.0.34" - || point_len == 256 - || point_len == 384 - { + } else { return Err(SignatureVerificationError::KeyAlgorithmMismatch { uri: algorithm.uri().to_string(), }); - } else { - return Err(SignatureVerificationError::InvalidKeyDer); } } _ => { @@ -434,9 +423,35 @@ fn parse_der_length(input: &[u8]) -> Option> { }; } + if declared_len < 128 { + return Some(Err(())); + } + Some(Ok((declared_len, remainder))) } +fn validate_ec_public_key_encoding( + ec: &ECPoint<'_>, + public_key_bytes: &[u8], +) -> Result<(), SignatureVerificationError> { + let coordinate_len = ec + .key_size() + .checked_div(8) + .ok_or(SignatureVerificationError::InvalidKeyDer)?; + let expected_len = coordinate_len + .checked_mul(2) + .and_then(|len| len.checked_add(1)) + .ok_or(SignatureVerificationError::InvalidKeyDer)?; + + let is_uncompressed_sec1 = + public_key_bytes.len() == expected_len && public_key_bytes.first() == Some(&0x04); + if !is_uncompressed_sec1 { + return Err(SignatureVerificationError::InvalidKeyDer); + } + + Ok(()) +} + #[cfg(test)] #[expect(clippy::unwrap_used, reason = "unit tests use fixed fixture data")] mod tests { @@ -455,4 +470,25 @@ mod tests { )); } } + + #[test] + fn der_like_prefix_with_fixed_width_len_is_classified_as_raw() { + let mut signature = vec![0xAA_u8; 96]; + signature[0] = 0x30; + signature[1] = 0x20; + + let encoding = classify_ecdsa_signature_encoding(&signature, 48) + .expect("same-width signature with invalid DER must fall back to raw"); + assert_eq!(encoding, EcdsaSignatureEncoding::XmlDsigFixed); + } + + #[test] + fn overlong_der_length_below_128_is_rejected() { + let bad = [0x81_u8, 0x7f]; + let parsed = parse_der_length(&bad).expect("length bytes should be present"); + assert!( + matches!(parsed, Err(())), + "DER must reject long-form lengths below 128" + ); + } } diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs index e1b683d..577bd37 100644 --- a/tests/ecdsa_signature_integration.rs +++ b/tests/ecdsa_signature_integration.rs @@ -128,58 +128,6 @@ fn local_p384_signature_matches() { assert!(valid, "locally signed P-384 signature should verify"); } -#[test] -fn local_p384_raw_signature_with_der_like_prefix_matches() { - const MAX_ATTEMPTS: usize = 8192; - - let xml = read_fixture(Path::new( - "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha384-ecdsa-sha384.xml", - )); - let private_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-key.pem")); - let public_key_pem = read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime384v1-pubkey.pem")); - let (signature_algorithm, canonical_signed_info, _) = - canonicalized_signed_info_and_signature(&xml); - assert_eq!( - SignatureAlgorithm::EcdsaP384Sha384, - signature_algorithm, - "fixture SignatureMethod should be EcdsaP384Sha384", - ); - - let pkcs8_der = x509_parser::pem::parse_x509_pem(private_key_pem.as_bytes()) - .expect("fixture PEM should parse") - .1 - .contents; - let rng = SystemRandom::new(); - let key_pair = EcdsaKeyPair::from_pkcs8(&ECDSA_P384_SHA384_FIXED_SIGNING, &pkcs8_der, &rng) - .expect("fixture PKCS#8 should parse"); - - for _ in 0..MAX_ATTEMPTS { - let signature = key_pair - .sign(&rng, &canonical_signed_info) - .expect("fixture P-384 key should sign"); - if signature.as_ref().first().copied() != Some(0x30) { - continue; - } - - let valid = verify_ecdsa_signature_pem( - SignatureAlgorithm::EcdsaP384Sha384, - &public_key_pem, - &canonical_signed_info, - signature.as_ref(), - ); - - assert!( - matches!(valid, Ok(true)), - "fixed-width signature with leading 0x30 must verify as raw r||s, got {valid:?}" - ); - return; - } - - panic!( - "failed to generate a fixed-width P-384 signature starting with 0x30 after {MAX_ATTEMPTS} attempts" - ); -} - #[test] fn local_p384_der_signature_matches() { let xml = read_fixture(Path::new( @@ -325,7 +273,48 @@ fn malformed_spki_der_returns_typed_error() { ) .expect_err("malformed SPKI DER should be rejected"); - assert!(matches!(err, SignatureVerificationError::InvalidKeyDer)); + assert!( + matches!(err, SignatureVerificationError::InvalidKeyDer), + "expected InvalidKeyDer, got {err:?}" + ); +} + +#[test] +fn spki_with_invalid_ec_point_prefix_returns_typed_error() { + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha256-ecdsa-sha256.xml", + )); + let (algorithm, canonical_signed_info, signature_value) = + canonicalized_signed_info_and_signature(&xml); + let mut public_key_der = x509_parser::pem::parse_x509_pem( + read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem")).as_bytes(), + ) + .expect("fixture PEM should parse") + .1 + .contents; + + // For P-256 fixtures we expect BIT STRING header 03 42 00 followed by + // uncompressed SEC1 prefix 0x04; mutating it to 0x02 keeps SPKI parseable + // but should be rejected as invalid key encoding for ring. + let marker = [0x03_u8, 0x42, 0x00, 0x04]; + let marker_pos = public_key_der + .windows(marker.len()) + .position(|window| window == marker) + .expect("fixture SPKI should contain uncompressed EC point marker"); + public_key_der[marker_pos + 3] = 0x02; + + let err = verify_ecdsa_signature_spki( + algorithm, + &public_key_der, + &canonical_signed_info, + &signature_value, + ) + .expect_err("invalid EC point encoding should be rejected as key error"); + + assert!( + matches!(err, SignatureVerificationError::InvalidKeyDer), + "expected InvalidKeyDer, got {err:?}" + ); } #[test] From 9213d104478dd40b698537a34bd04f856c913f5e Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 08:29:23 +0200 Subject: [PATCH 06/11] fix(xmldsig): prefer raw width and round ec bytes - prioritize XMLDSig fixed-width encoding when signature length matches - round EC coordinate byte length up for non-byte-aligned curves - add unit coverage for 521-bit coordinate byte rounding --- src/xmldsig/signature.rs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 9d2c30c..c58fc47 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -311,11 +311,14 @@ fn classify_ecdsa_signature_encoding( .checked_mul(2) .ok_or(SignatureVerificationError::InvalidKeyDer)?; + // XMLDSig requires fixed-width raw r||s; prefer that interpretation + // whenever the signature has the expected fixed width. + if signature_value.len() == expected_len { + return Ok(EcdsaSignatureEncoding::XmlDsigFixed); + } + match inspect_der_encoded_ecdsa_signature(signature_value, component_len) { Ok(Some(())) => Ok(EcdsaSignatureEncoding::Asn1Der), - Ok(None) | Err(_) if signature_value.len() == expected_len => { - Ok(EcdsaSignatureEncoding::XmlDsigFixed) - } Ok(None) | Err(_) => Err(SignatureVerificationError::InvalidSignatureFormat), } } @@ -434,10 +437,7 @@ fn validate_ec_public_key_encoding( ec: &ECPoint<'_>, public_key_bytes: &[u8], ) -> Result<(), SignatureVerificationError> { - let coordinate_len = ec - .key_size() - .checked_div(8) - .ok_or(SignatureVerificationError::InvalidKeyDer)?; + let coordinate_len = ec_coordinate_len_bytes(ec.key_size())?; let expected_len = coordinate_len .checked_mul(2) .and_then(|len| len.checked_add(1)) @@ -452,6 +452,13 @@ fn validate_ec_public_key_encoding( Ok(()) } +fn ec_coordinate_len_bytes(key_bits: usize) -> Result { + key_bits + .checked_add(7) + .and_then(|bits| bits.checked_div(8)) + .ok_or(SignatureVerificationError::InvalidKeyDer) +} + #[cfg(test)] #[expect(clippy::unwrap_used, reason = "unit tests use fixed fixture data")] mod tests { @@ -491,4 +498,12 @@ mod tests { "DER must reject long-form lengths below 128" ); } + + #[test] + fn ec_coordinate_length_rounds_up_for_non_byte_aligned_curves() { + assert_eq!( + ec_coordinate_len_bytes(521).expect("521-bit curves require rounded byte length"), + 66 + ); + } } From a06ed8066fcf375b0e4a6cd010cc1003e812cee0 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 08:42:34 +0200 Subject: [PATCH 07/11] fix(xmldsig): retry fixed verifier for ambiguous ecdsa - classify same-width DER-parseable signatures as ambiguous - in ambiguous case verify ASN.1 first, then retry fixed-width - add regression coverage for same-width valid DER ambiguity --- src/xmldsig/signature.rs | 74 +++++++++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 19 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index c58fc47..9bd1297 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -167,13 +167,27 @@ pub fn verify_ecdsa_signature_spki( match public_key { PublicKey::EC(ec) => { validate_ec_public_key_encoding(&ec, &spki.subject_public_key.data)?; - let verification_algorithm = - ecdsa_verification_algorithm(&spki, &ec, algorithm, signature_value)?; - let key = signature::UnparsedPublicKey::new( - verification_algorithm, - spki.subject_public_key.data, - ); - Ok(key.verify(signed_data, signature_value).is_ok()) + let (fixed_algorithm, asn1_algorithm, signature_encoding) = + ecdsa_verification_algorithms(&spki, &ec, algorithm, signature_value)?; + let public_key = &spki.subject_public_key.data; + let fixed_key = signature::UnparsedPublicKey::new(fixed_algorithm, public_key); + let asn1_key = signature::UnparsedPublicKey::new(asn1_algorithm, public_key); + + match signature_encoding { + EcdsaSignatureEncoding::XmlDsigFixed => { + Ok(fixed_key.verify(signed_data, signature_value).is_ok()) + } + EcdsaSignatureEncoding::Asn1Der => { + Ok(asn1_key.verify(signed_data, signature_value).is_ok()) + } + EcdsaSignatureEncoding::Ambiguous => { + if asn1_key.verify(signed_data, signature_value).is_ok() { + return Ok(true); + } + + Ok(fixed_key.verify(signed_data, signature_value).is_ok()) + } + } } _ => Err(SignatureVerificationError::InvalidKeyDer), } @@ -242,12 +256,19 @@ fn verification_algorithm( } } -fn ecdsa_verification_algorithm( +fn ecdsa_verification_algorithms( spki: &SubjectPublicKeyInfo<'_>, ec: &ECPoint<'_>, algorithm: SignatureAlgorithm, signature_value: &[u8], -) -> Result<&'static dyn signature::VerificationAlgorithm, SignatureVerificationError> { +) -> Result< + ( + &'static dyn signature::VerificationAlgorithm, + &'static dyn signature::VerificationAlgorithm, + EcdsaSignatureEncoding, + ), + SignatureVerificationError, +> { let curve_oid = spki .algorithm .parameters @@ -291,16 +312,18 @@ fn ecdsa_verification_algorithm( } }; - match classify_ecdsa_signature_encoding(signature_value, component_len)? { - EcdsaSignatureEncoding::XmlDsigFixed => Ok(fixed_algorithm), - EcdsaSignatureEncoding::Asn1Der => Ok(asn1_algorithm), - } + Ok(( + fixed_algorithm, + asn1_algorithm, + classify_ecdsa_signature_encoding(signature_value, component_len)?, + )) } #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum EcdsaSignatureEncoding { XmlDsigFixed, Asn1Der, + Ambiguous, } fn classify_ecdsa_signature_encoding( @@ -311,14 +334,14 @@ fn classify_ecdsa_signature_encoding( .checked_mul(2) .ok_or(SignatureVerificationError::InvalidKeyDer)?; - // XMLDSig requires fixed-width raw r||s; prefer that interpretation - // whenever the signature has the expected fixed width. - if signature_value.len() == expected_len { - return Ok(EcdsaSignatureEncoding::XmlDsigFixed); - } - match inspect_der_encoded_ecdsa_signature(signature_value, component_len) { + Ok(Some(())) if signature_value.len() == expected_len => { + Ok(EcdsaSignatureEncoding::Ambiguous) + } Ok(Some(())) => Ok(EcdsaSignatureEncoding::Asn1Der), + Ok(None) | Err(_) if signature_value.len() == expected_len => { + Ok(EcdsaSignatureEncoding::XmlDsigFixed) + } Ok(None) | Err(_) => Err(SignatureVerificationError::InvalidSignatureFormat), } } @@ -506,4 +529,17 @@ mod tests { 66 ); } + + #[test] + fn same_width_valid_der_is_marked_ambiguous() { + let mut signature = Vec::with_capacity(64); + signature.extend_from_slice(&[0x30, 0x3e, 0x02, 0x1d]); + signature.extend(std::iter::repeat_n(0x11_u8, 29)); + signature.extend_from_slice(&[0x02, 0x1d]); + signature.extend(std::iter::repeat_n(0x22_u8, 29)); + + let encoding = classify_ecdsa_signature_encoding(&signature, 32) + .expect("same-width structurally valid DER should classify as ambiguous"); + assert_eq!(encoding, EcdsaSignatureEncoding::Ambiguous); + } } From ca79ac6b486ee0c1c9130c9b33031ae4f31a9d67 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 08:44:45 +0200 Subject: [PATCH 08/11] fix(xmldsig): align signature format error mapping - map expected fixed-width length overflow to InvalidSignatureFormat - keep key parsing errors scoped to SPKI/key preparation failures --- src/xmldsig/signature.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 9bd1297..a25169b 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -332,7 +332,7 @@ fn classify_ecdsa_signature_encoding( ) -> Result { let expected_len = component_len .checked_mul(2) - .ok_or(SignatureVerificationError::InvalidKeyDer)?; + .ok_or(SignatureVerificationError::InvalidSignatureFormat)?; match inspect_der_encoded_ecdsa_signature(signature_value, component_len) { Ok(Some(())) if signature_value.len() == expected_len => { From f4b36ac64a0d32c949ac0dc80720d58efec5ed87 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 09:25:13 +0200 Subject: [PATCH 09/11] fix(xmldsig): cover spki happy path and der parsing - deduplicate PUBLIC KEY PEM parsing in a shared helper - keep DER classifier focused on encoding, not scalar validity - add positive verify_ecdsa_signature_spki fixture assertion --- src/xmldsig/signature.rs | 38 +++++++++++++--------------- tests/ecdsa_signature_integration.rs | 25 ++++++++++++++++++ 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index a25169b..8c43fdf 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -66,16 +66,13 @@ pub fn verify_rsa_signature_pem( signed_data: &[u8], signature_value: &[u8], ) -> Result { - let (rest, pem) = x509_parser::pem::parse_x509_pem(public_key_pem.as_bytes()) - .map_err(|_| SignatureVerificationError::InvalidKeyPem)?; - if !rest.iter().all(|byte| byte.is_ascii_whitespace()) { - return Err(SignatureVerificationError::InvalidKeyPem); - } - if pem.label != "PUBLIC KEY" { - return Err(SignatureVerificationError::InvalidKeyFormat { label: pem.label }); - } - - verify_rsa_signature_spki(algorithm, &pem.contents, signed_data, signature_value) + let public_key_spki_der = parse_public_key_pem(public_key_pem)?; + verify_rsa_signature_spki( + algorithm, + &public_key_spki_der, + signed_data, + signature_value, + ) } /// Verify an ECDSA XMLDSig signature using a PEM-encoded SPKI public key. @@ -94,6 +91,16 @@ pub fn verify_ecdsa_signature_pem( signed_data: &[u8], signature_value: &[u8], ) -> Result { + let public_key_spki_der = parse_public_key_pem(public_key_pem)?; + verify_ecdsa_signature_spki( + algorithm, + &public_key_spki_der, + signed_data, + signature_value, + ) +} + +fn parse_public_key_pem(public_key_pem: &str) -> Result, SignatureVerificationError> { let (rest, pem) = x509_parser::pem::parse_x509_pem(public_key_pem.as_bytes()) .map_err(|_| SignatureVerificationError::InvalidKeyPem)?; if !rest.iter().all(|byte| byte.is_ascii_whitespace()) { @@ -103,7 +110,7 @@ pub fn verify_ecdsa_signature_pem( return Err(SignatureVerificationError::InvalidKeyFormat { label: pem.label }); } - verify_ecdsa_signature_spki(algorithm, &pem.contents, signed_data, signature_value) + Ok(pem.contents) } /// Verify an RSA XMLDSig signature using DER-encoded SPKI public key bytes. @@ -408,15 +415,6 @@ fn parse_der_integer( return Err(SignatureVerificationError::InvalidSignatureFormat); } - let magnitude = if integer_bytes[0] == 0 { - &integer_bytes[1..] - } else { - integer_bytes - }; - if magnitude.is_empty() || magnitude.len() > component_len { - return Err(SignatureVerificationError::InvalidSignatureFormat); - } - Ok(remainder) } diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs index 577bd37..c254ca2 100644 --- a/tests/ecdsa_signature_integration.rs +++ b/tests/ecdsa_signature_integration.rs @@ -279,6 +279,31 @@ fn malformed_spki_der_returns_typed_error() { ); } +#[test] +fn spki_der_valid_signature_matches() { + let xml = read_fixture(Path::new( + "tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloped-sha256-ecdsa-sha256.xml", + )); + let (algorithm, canonical_signed_info, signature_value) = + canonicalized_signed_info_and_signature(&xml); + let public_key_der = x509_parser::pem::parse_x509_pem( + read_fixture(Path::new("tests/fixtures/keys/ec/ec-prime256v1-pubkey.pem")).as_bytes(), + ) + .expect("fixture PEM should parse") + .1 + .contents; + + let valid = verify_ecdsa_signature_spki( + algorithm, + &public_key_der, + &canonical_signed_info, + &signature_value, + ) + .expect("SPKI verifier should accept valid fixture key and signature"); + + assert!(valid, "SPKI verifier should validate donor P-256 signature"); +} + #[test] fn spki_with_invalid_ec_point_prefix_returns_typed_error() { let xml = read_fixture(Path::new( From 3b48e580429e905fbce6607370be3268b3ecfed6 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 09:50:38 +0200 Subject: [PATCH 10/11] fix(xmldsig): tighten ecdsa der classification - Reject DER INTEGER values longer than component width unless they use a required leading 0x00 sign byte - Report parsed non-EC SPKI keys in ECDSA verification as KeyAlgorithmMismatch instead of InvalidKeyDer - Add regression coverage for both cases --- src/xmldsig/signature.rs | 24 ++++++++++++++++++++++-- tests/ecdsa_signature_integration.rs | 23 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 8c43fdf..18a4fd3 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -42,7 +42,7 @@ pub enum SignatureVerificationError { #[error("invalid SubjectPublicKeyInfo DER")] InvalidKeyDer, - /// The provided public key curve does not match the signature algorithm. + /// The provided public key does not match the signature algorithm. #[error("public key does not match signature algorithm: {uri}")] KeyAlgorithmMismatch { /// XMLDSig algorithm URI used for diagnostics. @@ -196,7 +196,9 @@ pub fn verify_ecdsa_signature_spki( } } } - _ => Err(SignatureVerificationError::InvalidKeyDer), + _ => Err(SignatureVerificationError::KeyAlgorithmMismatch { + uri: algorithm.uri().to_string(), + }), } } @@ -408,6 +410,9 @@ fn parse_der_integer( if integer_bytes.len() > component_len + 1 { return Err(SignatureVerificationError::InvalidSignatureFormat); } + if integer_bytes.len() == component_len + 1 && integer_bytes[0] != 0 { + return Err(SignatureVerificationError::InvalidSignatureFormat); + } if integer_bytes[0] & 0x80 != 0 { return Err(SignatureVerificationError::InvalidSignatureFormat); } @@ -540,4 +545,19 @@ mod tests { .expect("same-width structurally valid DER should classify as ambiguous"); assert_eq!(encoding, EcdsaSignatureEncoding::Ambiguous); } + + #[test] + fn der_integer_longer_than_component_requires_sign_byte() { + let mut signature = Vec::with_capacity(72); + signature.extend_from_slice(&[0x30, 0x46, 0x02, 0x21, 0x01]); + signature.extend(std::iter::repeat_n(0x11_u8, 32)); + signature.extend_from_slice(&[0x02, 0x21, 0x01]); + signature.extend(std::iter::repeat_n(0x22_u8, 32)); + + let encoding = classify_ecdsa_signature_encoding(&signature, 32); + assert!(matches!( + encoding, + Err(SignatureVerificationError::InvalidSignatureFormat) + )); + } } diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs index c254ca2..3db268b 100644 --- a/tests/ecdsa_signature_integration.rs +++ b/tests/ecdsa_signature_integration.rs @@ -279,6 +279,29 @@ fn malformed_spki_der_returns_typed_error() { ); } +#[test] +fn non_ec_spki_key_returns_algorithm_mismatch_error() { + let public_key_der = x509_parser::pem::parse_x509_pem( + read_fixture(Path::new("tests/fixtures/keys/rsa/rsa-2048-pubkey.pem")).as_bytes(), + ) + .expect("fixture PEM should parse") + .1 + .contents; + + let err = verify_ecdsa_signature_spki( + SignatureAlgorithm::EcdsaP256Sha256, + &public_key_der, + b"payload", + &[0_u8; 64], + ) + .expect_err("non-EC SPKI key should be rejected by ECDSA verifier"); + + assert!(matches!( + err, + SignatureVerificationError::KeyAlgorithmMismatch { .. } + )); +} + #[test] fn spki_der_valid_signature_matches() { let xml = read_fixture(Path::new( From 9a5a9592a52fb7c467be336933cb5da172e7f878 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 27 Mar 2026 11:51:59 +0200 Subject: [PATCH 11/11] fix(xmldsig): reject non-ecdsa algorithms early - Return UnsupportedAlgorithm from verify_ecdsa_signature_spki before SPKI parsing for non-ECDSA signature methods - Add regression coverage to ensure non-ECDSA algorithms are rejected consistently regardless of key bytes --- src/xmldsig/signature.rs | 9 +++++++++ tests/ecdsa_signature_integration.rs | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/xmldsig/signature.rs b/src/xmldsig/signature.rs index 18a4fd3..3e6d13e 100644 --- a/src/xmldsig/signature.rs +++ b/src/xmldsig/signature.rs @@ -162,6 +162,15 @@ pub fn verify_ecdsa_signature_spki( signed_data: &[u8], signature_value: &[u8], ) -> Result { + if !matches!( + algorithm, + SignatureAlgorithm::EcdsaP256Sha256 | SignatureAlgorithm::EcdsaP384Sha384 + ) { + return Err(SignatureVerificationError::UnsupportedAlgorithm { + uri: algorithm.uri().to_string(), + }); + } + let (rest, spki) = SubjectPublicKeyInfo::from_der(public_key_spki_der) .map_err(|_| SignatureVerificationError::InvalidKeyDer)?; if !rest.is_empty() { diff --git a/tests/ecdsa_signature_integration.rs b/tests/ecdsa_signature_integration.rs index 3db268b..58aec8b 100644 --- a/tests/ecdsa_signature_integration.rs +++ b/tests/ecdsa_signature_integration.rs @@ -302,6 +302,31 @@ fn non_ec_spki_key_returns_algorithm_mismatch_error() { )); } +#[test] +fn non_ecdsa_algorithm_is_rejected_before_key_parsing() { + let rsa_public_key_der = x509_parser::pem::parse_x509_pem( + read_fixture(Path::new("tests/fixtures/keys/rsa/rsa-2048-pubkey.pem")).as_bytes(), + ) + .expect("fixture PEM should parse") + .1 + .contents; + + for public_key_der in [&rsa_public_key_der[..], &[0x01_u8, 0x02, 0x03]] { + let err = verify_ecdsa_signature_spki( + SignatureAlgorithm::RsaSha256, + public_key_der, + b"payload", + &[0_u8; 64], + ) + .expect_err("non-ECDSA algorithm must be rejected before key parsing"); + + assert!(matches!( + err, + SignatureVerificationError::UnsupportedAlgorithm { .. } + )); + } +} + #[test] fn spki_der_valid_signature_matches() { let xml = read_fixture(Path::new(