From 3e5d5eb83271918c7a4abb005df1c243b2713b7a Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sat, 27 Jun 2026 21:52:28 +0300 Subject: [PATCH 01/10] feat(xmldsig): add key resolver configuration - Add caller-owned trust anchors and named verification keys - Preserve documented chain policy, verification time, and depth defaults Closes #65 --- src/xmldsig/keys.rs | 81 +++++++++++++++++++++++++++++++++++++++++++++ src/xmldsig/mod.rs | 2 ++ 2 files changed, 83 insertions(+) create mode 100644 src/xmldsig/keys.rs diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs new file mode 100644 index 0000000..8032d22 --- /dev/null +++ b/src/xmldsig/keys.rs @@ -0,0 +1,81 @@ +//! Configuration and key material for XMLDSig key resolution. + +use std::{collections::HashMap, time::SystemTime}; + +use super::SignatureAlgorithm; + +/// A public verification key available to key resolvers. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VerificationKey { + /// Signature algorithm this key is configured to verify. + pub algorithm: SignatureAlgorithm, + /// DER-encoded SubjectPublicKeyInfo bytes. + pub public_key_bytes: Vec, + /// DER certificate from which the key was extracted, when applicable. + pub certificate_der: Option>, + /// Name used to register this key for `` resolution. + pub name: Option, +} + +/// Configuration for the default XMLDSig key resolver. +/// +/// The configuration owns all key material and has no global registry. Chain +/// verification is opt-in so callers that pin an embedded certificate can use +/// the documented TOFU model without constructing a certificate path. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct KeyResolverConfig { + /// DER-encoded certificates accepted as trust anchors. + pub trusted_certs: Vec>, + /// Verification keys addressable by `` content. + pub named_keys: HashMap, + /// Whether embedded X.509 certificate chains must terminate at a trust anchor. + pub verify_chains: bool, + /// Certificate verification time override; `None` selects the system clock. + pub verification_time: Option, + /// Maximum certificates in a validated path, including the trust anchor. + pub max_chain_depth: usize, +} + +impl Default for KeyResolverConfig { + fn default() -> Self { + Self { + trusted_certs: Vec::new(), + named_keys: HashMap::new(), + verify_chains: false, + verification_time: None, + max_chain_depth: 9, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn defaults_match_key_resolution_policy() { + // Defaults must remain compatible with xmlsec1's depth and opt-in trust policy. + let config = KeyResolverConfig::default(); + + assert!(config.trusted_certs.is_empty()); + assert!(config.named_keys.is_empty()); + assert!(!config.verify_chains); + assert_eq!(config.verification_time, None); + assert_eq!(config.max_chain_depth, 9); + } + + #[test] + fn stores_named_verification_key_metadata() { + // Named resolution must retain every field needed by the later resolver wiring. + let key = VerificationKey { + algorithm: SignatureAlgorithm::RsaSha256, + public_key_bytes: vec![1, 2, 3], + certificate_der: Some(vec![4, 5, 6]), + name: Some("idp-signing".into()), + }; + let mut config = KeyResolverConfig::default(); + config.named_keys.insert("idp-signing".into(), key.clone()); + + assert_eq!(config.named_keys.get("idp-signing"), Some(&key)); + } +} diff --git a/src/xmldsig/mod.rs b/src/xmldsig/mod.rs index 08a9e80..47e952b 100644 --- a/src/xmldsig/mod.rs +++ b/src/xmldsig/mod.rs @@ -9,6 +9,7 @@ //! - Node set types for the transform pipeline pub mod digest; +pub mod keys; pub mod parse; pub mod signature; pub mod transforms; @@ -19,6 +20,7 @@ pub(crate) mod whitespace; pub mod x509; pub use digest::{DigestAlgorithm, compute_digest, constant_time_eq}; +pub use keys::{KeyResolverConfig, VerificationKey}; pub use parse::{ KeyInfo, KeyInfoSource, KeyValueInfo, ParseError, Reference, SignatureAlgorithm, SignedInfo, X509DataInfo, find_signature_node, parse_key_info, parse_signed_info, From 7366b11e29bd6c14a05df5c347fe28c9e9abb20b Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sat, 27 Jun 2026 21:52:51 +0300 Subject: [PATCH 02/10] ci(release): validate changelog inputs - Require Conventional Commit pull request titles - Test title validation in CI - Exclude test-only commits from release changelogs Refs #65 --- .github/scripts/test-validate-pr-title.sh | 28 +++++++++++++++++++++++ .github/scripts/validate-pr-title.sh | 15 ++++++++++++ .github/workflows/ci.yml | 12 ++++++++++ .release-plz.toml | 2 +- 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100755 .github/scripts/test-validate-pr-title.sh create mode 100755 .github/scripts/validate-pr-title.sh diff --git a/.github/scripts/test-validate-pr-title.sh b/.github/scripts/test-validate-pr-title.sh new file mode 100755 index 0000000..5540adc --- /dev/null +++ b/.github/scripts/test-validate-pr-title.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +set -euo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +validator="$script_dir/validate-pr-title.sh" + +valid_titles=( + "feat(xmldsig): add key resolver" + "fix!: reject unsafe default" + "chore: release v0.1.8" +) + +invalid_titles=( + "Add key resolver" + "feat: Add key resolver" + "feature(xmldsig): add key resolver" +) + +for title in "${valid_titles[@]}"; do + "$validator" "$title" +done + +for title in "${invalid_titles[@]}"; do + if "$validator" "$title" >/dev/null 2>&1; then + echo "expected invalid title to fail: $title" >&2 + exit 1 + fi +done diff --git a/.github/scripts/validate-pr-title.sh b/.github/scripts/validate-pr-title.sh new file mode 100755 index 0000000..8b70836 --- /dev/null +++ b/.github/scripts/validate-pr-title.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -euo pipefail + +title="${1:-}" +pattern='^(feat|fix|perf|refactor|docs|test|build|ci|chore|style)(\([a-z0-9][a-z0-9._/-]*\))?(!)?: [a-z0-9].*$' + +if [[ ${#title} -gt 72 ]]; then + echo "PR title exceeds 72 characters: ${#title}" >&2 + exit 1 +fi + +if [[ ! $title =~ $pattern ]]; then + echo "PR title must use Conventional Commits, for example: feat(xmldsig): add key resolver" >&2 + exit 1 +fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 996057a..7b5c37d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,18 @@ env: RUSTFLAGS: -Dwarnings jobs: + pr-title: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Test title validator + run: .github/scripts/test-validate-pr-title.sh + - name: Validate pull request title + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: .github/scripts/validate-pr-title.sh "$PR_TITLE" + build-matrix: name: build-matrix (${{ matrix.rust }}) runs-on: ubuntu-latest diff --git a/.release-plz.toml b/.release-plz.toml index f8482b7..babbea4 100644 --- a/.release-plz.toml +++ b/.release-plz.toml @@ -18,5 +18,5 @@ commit_parsers = [ { message = "^fix", group = "Fixed" }, { message = "^perf", group = "Performance" }, { message = "^refactor", group = "Refactored" }, - { message = "^test", group = "Testing" }, + { message = "^test", skip = true }, ] From b0c91fb63b04f49c848e6c84045899f405d168e7 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 18:23:28 +0300 Subject: [PATCH 03/10] ci: remove pull request title validation --- .github/scripts/test-validate-pr-title.sh | 28 ----------------------- .github/scripts/validate-pr-title.sh | 15 ------------ .github/workflows/ci.yml | 12 ---------- 3 files changed, 55 deletions(-) delete mode 100755 .github/scripts/test-validate-pr-title.sh delete mode 100755 .github/scripts/validate-pr-title.sh diff --git a/.github/scripts/test-validate-pr-title.sh b/.github/scripts/test-validate-pr-title.sh deleted file mode 100755 index 5540adc..0000000 --- a/.github/scripts/test-validate-pr-title.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -validator="$script_dir/validate-pr-title.sh" - -valid_titles=( - "feat(xmldsig): add key resolver" - "fix!: reject unsafe default" - "chore: release v0.1.8" -) - -invalid_titles=( - "Add key resolver" - "feat: Add key resolver" - "feature(xmldsig): add key resolver" -) - -for title in "${valid_titles[@]}"; do - "$validator" "$title" -done - -for title in "${invalid_titles[@]}"; do - if "$validator" "$title" >/dev/null 2>&1; then - echo "expected invalid title to fail: $title" >&2 - exit 1 - fi -done diff --git a/.github/scripts/validate-pr-title.sh b/.github/scripts/validate-pr-title.sh deleted file mode 100755 index 8b70836..0000000 --- a/.github/scripts/validate-pr-title.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -title="${1:-}" -pattern='^(feat|fix|perf|refactor|docs|test|build|ci|chore|style)(\([a-z0-9][a-z0-9._/-]*\))?(!)?: [a-z0-9].*$' - -if [[ ${#title} -gt 72 ]]; then - echo "PR title exceeds 72 characters: ${#title}" >&2 - exit 1 -fi - -if [[ ! $title =~ $pattern ]]; then - echo "PR title must use Conventional Commits, for example: feat(xmldsig): add key resolver" >&2 - exit 1 -fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b5c37d..996057a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,18 +10,6 @@ env: RUSTFLAGS: -Dwarnings jobs: - pr-title: - if: github.event_name == 'pull_request' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v7 - - name: Test title validator - run: .github/scripts/test-validate-pr-title.sh - - name: Validate pull request title - env: - PR_TITLE: ${{ github.event.pull_request.title }} - run: .github/scripts/validate-pr-title.sh "$PR_TITLE" - build-matrix: name: build-matrix (${{ matrix.rust }}) runs-on: ubuntu-latest From b92f618a4df45eed78bde6547bc75dc6f773d81f Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 21:37:12 +0300 Subject: [PATCH 04/10] feat(xmldsig): wire default key resolver --- src/xmldsig/keys.rs | 340 +++++++++++++++++++++++++++++++++++++++++- src/xmldsig/mod.rs | 2 +- src/xmldsig/verify.rs | 45 ++++-- 3 files changed, 372 insertions(+), 15 deletions(-) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 8032d22..940c82b 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -2,7 +2,17 @@ use std::{collections::HashMap, time::SystemTime}; -use super::SignatureAlgorithm; +use x509_parser::{ + prelude::{FromDer, X509Certificate}, + public_key::PublicKey, + x509::SubjectPublicKeyInfo, +}; + +use super::{ + DsigError, KeyInfo, KeyInfoSource, KeyResolver, SignatureAlgorithm, VerifyingKey, + X509ChainOptions, X509DataInfo, verify_ecdsa_signature_spki, verify_rsa_signature_spki, + verify_x509_certificate_chain, +}; /// A public verification key available to key resolvers. #[derive(Debug, Clone, PartialEq, Eq)] @@ -17,6 +27,57 @@ pub struct VerificationKey { pub name: Option, } +impl VerifyingKey for VerificationKey { + fn verify( + &self, + algorithm: SignatureAlgorithm, + signed_data: &[u8], + signature_value: &[u8], + ) -> Result { + if algorithm != self.algorithm { + return Err(KeyResolutionError::AlgorithmMismatch.into()); + } + let result = match algorithm { + SignatureAlgorithm::RsaSha1 + | SignatureAlgorithm::RsaSha256 + | SignatureAlgorithm::RsaSha384 + | SignatureAlgorithm::RsaSha512 => verify_rsa_signature_spki( + algorithm, + &self.public_key_bytes, + signed_data, + signature_value, + ), + SignatureAlgorithm::EcdsaP256Sha256 | SignatureAlgorithm::EcdsaP384Sha384 => { + verify_ecdsa_signature_spki( + algorithm, + &self.public_key_bytes, + signed_data, + signature_value, + ) + } + }; + result.map_err(DsigError::Crypto) + } +} + +/// Failures while applying [`KeyResolverConfig`] to parsed key material. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum KeyResolutionError { + /// A configured or embedded key does not match the signature method. + #[error("verification key does not match the signature algorithm")] + AlgorithmMismatch, + /// An embedded certificate could not be parsed completely. + #[error("invalid embedded certificate DER")] + InvalidCertificate, + /// Embedded certificate path validation failed. + #[error("certificate chain validation failed: {0}")] + Chain(#[from] super::X509ChainError), + /// System time was unavailable for certificate validation. + #[error("system time is unavailable")] + SystemTime, +} + /// Configuration for the default XMLDSig key resolver. /// /// The configuration owns all key material and has no global registry. Chain @@ -48,10 +109,181 @@ impl Default for KeyResolverConfig { } } +/// Configuration-driven resolver for embedded certificates, DER keys, and key names. +#[derive(Debug, Clone, Default)] +pub struct DefaultKeyResolver { + config: KeyResolverConfig, +} + +impl DefaultKeyResolver { + /// Construct a resolver from explicit caller-owned key policy. + #[must_use] + pub fn new(config: KeyResolverConfig) -> Self { + Self { config } + } + + /// Borrow the active resolver configuration. + #[must_use] + pub fn config(&self) -> &KeyResolverConfig { + &self.config + } + + fn resolve_x509( + &self, + info: &X509DataInfo, + algorithm: SignatureAlgorithm, + ) -> Result, KeyResolutionError> { + let Some(&signing_index) = info.certificate_chain.first() else { + return Ok(None); + }; + let certificate_der = info + .certificates + .get(signing_index) + .ok_or(KeyResolutionError::InvalidCertificate)?; + + if self.config.verify_chains { + let options = X509ChainOptions { + trusted_certs: &self.config.trusted_certs, + verification_time: self + .config + .verification_time + .unwrap_or_else(SystemTime::now), + max_chain_depth: self.config.max_chain_depth, + check_crls: false, + }; + verify_x509_certificate_chain(info, &options)?; + } + + let (rest, certificate) = X509Certificate::from_der(certificate_der) + .map_err(|_| KeyResolutionError::InvalidCertificate)?; + if !rest.is_empty() { + return Err(KeyResolutionError::InvalidCertificate); + } + let public_key_bytes = certificate.public_key().raw.to_vec(); + validate_spki_algorithm(&public_key_bytes, algorithm)?; + Ok(Some(VerificationKey { + algorithm, + public_key_bytes, + certificate_der: Some(certificate_der.clone()), + name: None, + })) + } +} + +impl KeyResolver for DefaultKeyResolver { + fn resolve<'a>( + &'a self, + key_info: Option<&KeyInfo>, + algorithm: SignatureAlgorithm, + ) -> Result>, DsigError> { + let Some(key_info) = key_info else { + return Ok(None); + }; + for source in &key_info.sources { + let resolved = match source { + KeyInfoSource::X509Data(info) => self.resolve_x509(info, algorithm)?, + KeyInfoSource::DerEncodedKeyValue(public_key_bytes) => { + validate_spki_algorithm(public_key_bytes, algorithm)?; + Some(VerificationKey { + algorithm, + public_key_bytes: public_key_bytes.clone(), + certificate_der: None, + name: None, + }) + } + KeyInfoSource::KeyName(name) => self + .config + .named_keys + .get(name) + .map(|key| { + if key.algorithm != algorithm { + return Err(KeyResolutionError::AlgorithmMismatch); + } + Ok(key.clone()) + }) + .transpose()?, + KeyInfoSource::KeyValue(_) => None, + }; + if let Some(key) = resolved { + return Ok(Some(Box::new(key))); + } + } + Ok(None) + } + + fn consumes_document_key_info(&self) -> bool { + true + } +} + +fn validate_spki_algorithm( + public_key_bytes: &[u8], + algorithm: SignatureAlgorithm, +) -> Result<(), KeyResolutionError> { + let (rest, spki) = SubjectPublicKeyInfo::from_der(public_key_bytes) + .map_err(|_| KeyResolutionError::InvalidCertificate)?; + if !rest.is_empty() { + return Err(KeyResolutionError::InvalidCertificate); + } + let parsed = spki + .parsed() + .map_err(|_| KeyResolutionError::InvalidCertificate)?; + let curve_oid = spki + .algorithm + .parameters + .as_ref() + .and_then(|value| value.as_oid().ok()) + .map(|oid| oid.to_id_string()); + match (algorithm, parsed) { + ( + SignatureAlgorithm::RsaSha1 + | SignatureAlgorithm::RsaSha256 + | SignatureAlgorithm::RsaSha384 + | SignatureAlgorithm::RsaSha512, + PublicKey::RSA(_), + ) => Ok(()), + (SignatureAlgorithm::EcdsaP256Sha256, PublicKey::EC(_)) + if curve_oid.as_deref() == Some("1.2.840.10045.3.1.7") => + { + Ok(()) + } + (SignatureAlgorithm::EcdsaP384Sha384, PublicKey::EC(_)) + if matches!(curve_oid.as_deref(), Some("1.3.132.0.34" | "1.3.132.0.35")) => + { + Ok(()) + } + _ => Err(KeyResolutionError::AlgorithmMismatch), + } +} + #[cfg(test)] mod tests { + use base64::{Engine, engine::general_purpose::STANDARD}; + use super::*; + const SIGNED_SAML: &str = + include_str!("../../tests/fixtures/saml/response_signed_by_idp_ecdsa.xml"); + const SAML_PUBLIC_KEY: &str = + include_str!("../../tests/fixtures/keys/ec/saml-idp-ecdsa-pubkey.pem"); + + fn replace_key_info(xml: &str, replacement: &str) -> String { + let start = xml.find("").expect("fixture has KeyInfo"); + let end = xml + .find("") + .expect("fixture has closing KeyInfo") + + "".len(); + format!("{}{}{}", &xml[..start], replacement, &xml[end..]) + } + + fn public_key_der() -> Vec { + let (rest, pem) = x509_parser::pem::parse_x509_pem(SAML_PUBLIC_KEY.as_bytes()) + .expect("fixture public key is PEM"); + assert!(rest.iter().all(|byte| byte.is_ascii_whitespace())); + assert_eq!(pem.label, "PUBLIC KEY"); + pem.contents + } + #[test] fn defaults_match_key_resolution_policy() { // Defaults must remain compatible with xmlsec1's depth and opt-in trust policy. @@ -78,4 +310,110 @@ mod tests { assert_eq!(config.named_keys.get("idp-signing"), Some(&key)); } + + #[test] + fn resolves_embedded_certificate_end_to_end() { + // The default resolver must make parsed X509Data usable by VerifyContext. + let resolver = DefaultKeyResolver::default(); + let result = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(SIGNED_SAML) + .expect("embedded certificate should resolve"); + + assert_eq!(result.status, super::super::DsigStatus::Valid); + } + + #[test] + fn resolves_named_key_end_to_end() { + // KeyName lookup must preserve the same cryptographic result as embedded X509Data. + let xml = replace_key_info( + SIGNED_SAML, + "idp-signing", + ); + let mut config = KeyResolverConfig::default(); + config.named_keys.insert( + "idp-signing".into(), + VerificationKey { + algorithm: SignatureAlgorithm::EcdsaP256Sha256, + public_key_bytes: public_key_der(), + certificate_der: None, + name: Some("idp-signing".into()), + }, + ); + let resolver = DefaultKeyResolver::new(config); + let result = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(&xml) + .expect("named key should resolve"); + + assert_eq!(result.status, super::super::DsigStatus::Valid); + } + + #[test] + fn resolves_der_encoded_key_end_to_end() { + // DSig 1.1 DEREncodedKeyValue must feed the same SPKI verifier path. + let encoded = STANDARD.encode(public_key_der()); + let xml = replace_key_info( + SIGNED_SAML, + &format!( + "{encoded}" + ), + ); + let resolver = DefaultKeyResolver::default(); + let result = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(&xml) + .expect("DER key should resolve"); + + assert_eq!(result.status, super::super::DsigStatus::Valid); + } + + #[test] + fn chain_verification_rejects_untrusted_embedded_certificate() { + // Enabling chain policy must fail closed when no trust anchor is configured. + let resolver = DefaultKeyResolver::new(KeyResolverConfig { + verify_chains: true, + ..KeyResolverConfig::default() + }); + let error = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(SIGNED_SAML) + .expect_err("untrusted certificate must fail chain validation"); + + assert!(matches!( + error, + DsigError::KeyResolution(KeyResolutionError::Chain( + super::super::X509ChainError::UntrustedRoot + )) + )); + } + + #[test] + fn named_key_algorithm_mismatch_fails_closed() { + // A key registered for RSA must never be attempted for an ECDSA signature. + let xml = replace_key_info( + SIGNED_SAML, + "wrong-algorithm", + ); + let mut config = KeyResolverConfig::default(); + config.named_keys.insert( + "wrong-algorithm".into(), + VerificationKey { + algorithm: SignatureAlgorithm::RsaSha256, + public_key_bytes: public_key_der(), + certificate_der: None, + name: Some("wrong-algorithm".into()), + }, + ); + let resolver = DefaultKeyResolver::new(config); + let error = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(&xml) + .expect_err("algorithm mismatch must fail closed"); + + assert!(matches!( + error, + DsigError::KeyResolution(KeyResolutionError::AlgorithmMismatch) + )); + } } diff --git a/src/xmldsig/mod.rs b/src/xmldsig/mod.rs index 47e952b..b12ca9c 100644 --- a/src/xmldsig/mod.rs +++ b/src/xmldsig/mod.rs @@ -20,7 +20,7 @@ pub(crate) mod whitespace; pub mod x509; pub use digest::{DigestAlgorithm, compute_digest, constant_time_eq}; -pub use keys::{KeyResolverConfig, VerificationKey}; +pub use keys::{DefaultKeyResolver, KeyResolutionError, KeyResolverConfig, VerificationKey}; pub use parse::{ KeyInfo, KeyInfoSource, KeyValueInfo, ParseError, Reference, SignatureAlgorithm, SignedInfo, X509DataInfo, find_signature_node, parse_key_info, parse_signed_info, diff --git a/src/xmldsig/verify.rs b/src/xmldsig/verify.rs index 2b2d534..60f249c 100644 --- a/src/xmldsig/verify.rs +++ b/src/xmldsig/verify.rs @@ -17,7 +17,7 @@ use std::collections::HashSet; use crate::c14n::canonicalize; use super::digest::{DigestAlgorithm, compute_digest, constant_time_eq}; -use super::parse::{ParseError, Reference, SignatureAlgorithm, XMLDSIG_NS}; +use super::parse::{KeyInfo, ParseError, Reference, SignatureAlgorithm, XMLDSIG_NS}; use super::parse::{parse_key_info, parse_reference, parse_signed_info}; use super::signature::{ SignatureVerificationError, verify_ecdsa_signature_pem, verify_rsa_signature_pem, @@ -49,13 +49,17 @@ pub trait VerifyingKey { /// This trait intentionally has no `Send + Sync` supertraits; callers that need /// cross-thread sharing can wrap resolvers/keys in their own thread-safe types. pub trait KeyResolver { - /// Resolve a verification key for the provided XML document. + /// Resolve a verification key from parsed `` sources. /// /// Return `Ok(None)` when no suitable key could be resolved from available /// key material (for example, missing `` candidates). `VerifyContext` /// maps `Ok(None)` to `DsigStatus::Invalid(FailureReason::KeyNotFound)`; /// reserve `Err(...)` for resolver failures. - fn resolve<'a>(&'a self, xml: &str) -> Result>, DsigError>; + fn resolve<'a>( + &'a self, + key_info: Option<&KeyInfo>, + algorithm: SignatureAlgorithm, + ) -> Result>, DsigError>; /// Return `true` when this resolver consumes document `` material. /// @@ -525,6 +529,10 @@ pub enum DsigError { #[error("failed to parse KeyInfo: {0}")] ParseKeyInfo(#[source] super::parse::ParseError), + /// Configuration-driven key resolution failed. + #[error("key resolution failed: {0}")] + KeyResolution(#[from] super::keys::KeyResolutionError), + /// `//` parsing failed. #[error("failed to parse Manifest reference: {0}")] ParseManifestReference(#[source] ParseError), @@ -647,10 +655,15 @@ fn verify_signature_with_context( (None, Some(resolver)) => resolver.consumes_document_key_info(), (None, None) => true, }; - if should_parse_key_info && let Some(key_info_node) = signature_children.key_info_node { - // P2-001: validate KeyInfo structure now; key material consumption is deferred. - parse_key_info(key_info_node).map_err(SignatureVerificationPipelineError::ParseKeyInfo)?; - } + let key_info = if should_parse_key_info { + signature_children + .key_info_node + .map(parse_key_info) + .transpose() + .map_err(SignatureVerificationPipelineError::ParseKeyInfo)? + } else { + None + }; let signed_info = parse_signed_info(signed_info_node)?; enforce_reference_policies( @@ -698,7 +711,9 @@ fn verify_signature_with_context( )?; let signature_value = decode_signature_value(signature_children.signature_value_node)?; - let Some(resolved_key) = resolve_verifying_key(ctx, xml)? else { + let Some(resolved_key) = + resolve_verifying_key(ctx, key_info.as_ref(), signed_info.signature_method)? + else { return Ok(VerifyResult { status: DsigStatus::Invalid(FailureReason::KeyNotFound), signed_info_references: references.results, @@ -919,13 +934,14 @@ impl ResolvedVerifyingKey<'_> { fn resolve_verifying_key<'k>( ctx: &VerifyContext<'k>, - xml: &str, + key_info: Option<&KeyInfo>, + algorithm: SignatureAlgorithm, ) -> Result>, SignatureVerificationPipelineError> { if let Some(key) = ctx.key { return Ok(Some(ResolvedVerifyingKey::Borrowed(key))); } if let Some(resolver) = ctx.key_resolver { - let resolved = resolver.resolve(xml)?; + let resolved = resolver.resolve(key_info, algorithm)?; return Ok(resolved.map(ResolvedVerifyingKey::Owned)); } Ok(None) @@ -1254,7 +1270,8 @@ mod tests { impl KeyResolver for PanicResolver { fn resolve<'a>( &'a self, - _xml: &str, + _key_info: Option<&KeyInfo>, + _algorithm: SignatureAlgorithm, ) -> Result>, SignatureVerificationPipelineError> { panic!("resolver should not be called when references already fail"); @@ -1266,7 +1283,8 @@ mod tests { impl KeyResolver for MissingKeyResolver { fn resolve<'a>( &'a self, - _xml: &str, + _key_info: Option<&KeyInfo>, + _algorithm: SignatureAlgorithm, ) -> Result>, SignatureVerificationPipelineError> { Ok(None) @@ -1278,7 +1296,8 @@ mod tests { impl KeyResolver for ConsumingKeyInfoResolver { fn resolve<'a>( &'a self, - _xml: &str, + _key_info: Option<&KeyInfo>, + _algorithm: SignatureAlgorithm, ) -> Result>, SignatureVerificationPipelineError> { Ok(None) From 753f148ddf63e635fc0b008590b5c0ce6d9e3639 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 21:48:31 +0300 Subject: [PATCH 05/10] test(xmldsig): cover mislabeled named key --- src/xmldsig/keys.rs | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 940c82b..c35e8d1 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -266,6 +266,7 @@ mod tests { include_str!("../../tests/fixtures/saml/response_signed_by_idp_ecdsa.xml"); const SAML_PUBLIC_KEY: &str = include_str!("../../tests/fixtures/keys/ec/saml-idp-ecdsa-pubkey.pem"); + const RSA_PUBLIC_KEY: &str = include_str!("../../tests/fixtures/keys/rsa/rsa-2048-pubkey.pem"); fn replace_key_info(xml: &str, replacement: &str) -> String { let start = xml.find("").expect("fixture has KeyInfo"); @@ -276,8 +277,8 @@ mod tests { format!("{}{}{}", &xml[..start], replacement, &xml[end..]) } - fn public_key_der() -> Vec { - let (rest, pem) = x509_parser::pem::parse_x509_pem(SAML_PUBLIC_KEY.as_bytes()) + fn public_key_der(pem_text: &str) -> Vec { + let (rest, pem) = x509_parser::pem::parse_x509_pem(pem_text.as_bytes()) .expect("fixture public key is PEM"); assert!(rest.iter().all(|byte| byte.is_ascii_whitespace())); assert_eq!(pem.label, "PUBLIC KEY"); @@ -335,7 +336,7 @@ mod tests { "idp-signing".into(), VerificationKey { algorithm: SignatureAlgorithm::EcdsaP256Sha256, - public_key_bytes: public_key_der(), + public_key_bytes: public_key_der(SAML_PUBLIC_KEY), certificate_der: None, name: Some("idp-signing".into()), }, @@ -352,7 +353,7 @@ mod tests { #[test] fn resolves_der_encoded_key_end_to_end() { // DSig 1.1 DEREncodedKeyValue must feed the same SPKI verifier path. - let encoded = STANDARD.encode(public_key_der()); + let encoded = STANDARD.encode(public_key_der(SAML_PUBLIC_KEY)); let xml = replace_key_info( SIGNED_SAML, &format!( @@ -400,7 +401,7 @@ mod tests { "wrong-algorithm".into(), VerificationKey { algorithm: SignatureAlgorithm::RsaSha256, - public_key_bytes: public_key_der(), + public_key_bytes: public_key_der(SAML_PUBLIC_KEY), certificate_der: None, name: Some("wrong-algorithm".into()), }, @@ -416,4 +417,33 @@ mod tests { DsigError::KeyResolution(KeyResolutionError::AlgorithmMismatch) )); } + + #[test] + fn named_key_spki_type_mismatch_fails_during_resolution() { + // The configured algorithm label cannot override the actual SPKI key type. + let xml = replace_key_info( + SIGNED_SAML, + "mislabeled", + ); + let mut config = KeyResolverConfig::default(); + config.named_keys.insert( + "mislabeled".into(), + VerificationKey { + algorithm: SignatureAlgorithm::EcdsaP256Sha256, + public_key_bytes: public_key_der(RSA_PUBLIC_KEY), + certificate_der: None, + name: Some("mislabeled".into()), + }, + ); + let resolver = DefaultKeyResolver::new(config); + let error = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(&xml) + .expect_err("mislabeled named key must fail during resolution"); + + assert!(matches!( + error, + DsigError::KeyResolution(KeyResolutionError::AlgorithmMismatch) + )); + } } From 48a0a139703f1ef647a93bca76333d81eec17f32 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 21:50:00 +0300 Subject: [PATCH 06/10] fix(xmldsig): validate named key material --- src/xmldsig/keys.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index c35e8d1..79b65ea 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -199,6 +199,7 @@ impl KeyResolver for DefaultKeyResolver { if key.algorithm != algorithm { return Err(KeyResolutionError::AlgorithmMismatch); } + validate_spki_algorithm(&key.public_key_bytes, algorithm)?; Ok(key.clone()) }) .transpose()?, @@ -247,6 +248,8 @@ fn validate_spki_algorithm( { Ok(()) } + // The XMLDSig ecdsa-sha384 URI identifies the digest, not a curve. The + // verifier intentionally supports the donor P-521/SHA-384 interop case. (SignatureAlgorithm::EcdsaP384Sha384, PublicKey::EC(_)) if matches!(curve_oid.as_deref(), Some("1.3.132.0.34" | "1.3.132.0.35")) => { From 0087de153034036ce6b383b798d4b34475de04f8 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 21:53:48 +0300 Subject: [PATCH 07/10] docs(xmldsig): clarify ECDSA curve compatibility --- src/xmldsig/keys.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 79b65ea..495a9cc 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -248,8 +248,9 @@ fn validate_spki_algorithm( { Ok(()) } - // The XMLDSig ecdsa-sha384 URI identifies the digest, not a curve. The - // verifier intentionally supports the donor P-521/SHA-384 interop case. + // xmlsec's OpenSSL backend maps ecdsa-sha384 to EVP_sha384() plus the + // generic EC key class, without restricting the curve to P-384. Keep + // P-521/SHA-384 compatible with that donor contract. (SignatureAlgorithm::EcdsaP384Sha384, PublicKey::EC(_)) if matches!(curve_oid.as_deref(), Some("1.3.132.0.34" | "1.3.132.0.35")) => { From 0c2b7b3765f1611e91e509bb54a1e0a4a4d3e60b Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 22:09:35 +0300 Subject: [PATCH 08/10] test(xmldsig): cover malformed named key error --- src/xmldsig/keys.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 495a9cc..9a66128 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -450,4 +450,33 @@ mod tests { DsigError::KeyResolution(KeyResolutionError::AlgorithmMismatch) )); } + + #[test] + fn malformed_named_key_reports_public_key_error() { + // Non-certificate SPKI failures must not be mislabeled as certificate errors. + let xml = replace_key_info( + SIGNED_SAML, + "malformed", + ); + let mut config = KeyResolverConfig::default(); + config.named_keys.insert( + "malformed".into(), + VerificationKey { + algorithm: SignatureAlgorithm::EcdsaP256Sha256, + public_key_bytes: vec![1, 2, 3], + certificate_der: None, + name: Some("malformed".into()), + }, + ); + let resolver = DefaultKeyResolver::new(config); + let error = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(&xml) + .expect_err("malformed named key must fail during resolution"); + + assert_eq!( + error.to_string(), + "key resolution failed: invalid public key DER" + ); + } } From a1f6520c7dec2d4136343b7bbfb765581da933f7 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 22:11:16 +0300 Subject: [PATCH 09/10] fix(xmldsig): distinguish invalid public keys --- src/xmldsig/keys.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 9a66128..01181a6 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -70,6 +70,9 @@ pub enum KeyResolutionError { /// An embedded certificate could not be parsed completely. #[error("invalid embedded certificate DER")] InvalidCertificate, + /// Configured or embedded public key DER could not be parsed completely. + #[error("invalid public key DER")] + InvalidPublicKey, /// Embedded certificate path validation failed. #[error("certificate chain validation failed: {0}")] Chain(#[from] super::X509ChainError), @@ -222,13 +225,13 @@ fn validate_spki_algorithm( algorithm: SignatureAlgorithm, ) -> Result<(), KeyResolutionError> { let (rest, spki) = SubjectPublicKeyInfo::from_der(public_key_bytes) - .map_err(|_| KeyResolutionError::InvalidCertificate)?; + .map_err(|_| KeyResolutionError::InvalidPublicKey)?; if !rest.is_empty() { - return Err(KeyResolutionError::InvalidCertificate); + return Err(KeyResolutionError::InvalidPublicKey); } let parsed = spki .parsed() - .map_err(|_| KeyResolutionError::InvalidCertificate)?; + .map_err(|_| KeyResolutionError::InvalidPublicKey)?; let curve_oid = spki .algorithm .parameters From cff8cd2b5517277184fa734fe6a4ba98bd993e77 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 22:27:50 +0300 Subject: [PATCH 10/10] test(xmldsig): assert malformed key variant --- src/xmldsig/keys.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 01181a6..1766ea4 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -477,9 +477,9 @@ mod tests { .verify(&xml) .expect_err("malformed named key must fail during resolution"); - assert_eq!( - error.to_string(), - "key resolution failed: invalid public key DER" - ); + assert!(matches!( + error, + DsigError::KeyResolution(KeyResolutionError::InvalidPublicKey) + )); } }