diff --git a/Cargo.toml b/Cargo.toml index 1e9a629..648895e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ sha1 = { version = "0.11", features = ["oid"], optional = true } sha2 = { version = "0.11", features = ["oid"], optional = true } p256 = { version = "0.14", features = ["ecdsa"], optional = true } p384 = { version = "0.14", features = ["ecdsa"], optional = true } -p521 = { version = "0.14.0-rc.15", features = ["ecdsa"], optional = true } +p521 = { version = "0.14", features = ["ecdsa"], optional = true } signature = { version = "3", optional = true } subtle = { version = "2", optional = true } getrandom = { version = "0.4", features = ["sys_rng"], optional = true } diff --git a/README.md b/README.md index 767ca41..f93d771 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Pure Rust XML Security library. Drop-in replacement for libxmlsec1. ## Features - **C14N** — XML Canonicalization (inclusive + exclusive, W3C compliant) -- **XMLDSig** — XML Digital Signatures (verify pipeline + template signing implemented; broader signing interop in progress) +- **XMLDSig** — XML Digital Signatures (verify pipeline + template signing with X.509 KeyInfo implemented; broader signing interop in progress) - **XMLEnc** — XML Encryption (symmetric + asymmetric) - **X.509** — Certificate-based key extraction and validation @@ -40,6 +40,7 @@ Currently implemented (core paths): - XMLDSig parsing, same-document URI dereference, transform chains, and digest verification - XMLDSig full verify pipeline (`SignedInfo` canonicalization + `SignatureValue` verification) - XMLDSig template signing pipeline (`DigestValue` fill + `SignedInfo` canonicalization + `SignatureValue` fill) +- XMLDSig signing KeyInfo writer for embedded X.509 certificates - Built-in verification-key resolution from embedded X.509/DER/`KeyValue` sources and configured `KeyName`, X.509 subject, issuer/serial, SKI, or digest selectors - RSA PKCS#1 v1.5 verification helpers for SHA-1 / SHA-256 / SHA-384 / SHA-512 - ECDSA verification helpers for P-256/SHA-256 and P-384/SHA-384 @@ -47,7 +48,7 @@ Currently implemented (core paths): - Opt-in X.509 certificate-chain validation with explicit trust anchors, validity checks, CA constraints, and CRLs Still in progress: -- XMLDSig signing KeyInfo writer, examples, and broader donor/CLI interop coverage +- XMLDSig signing examples and broader donor/CLI interop coverage - XMLEnc encryption/decryption pipeline Current toolchain target: latest stable Rust. diff --git a/src/xmldsig/mod.rs b/src/xmldsig/mod.rs index 174a157..47a634f 100644 --- a/src/xmldsig/mod.rs +++ b/src/xmldsig/mod.rs @@ -30,8 +30,9 @@ pub use parse::{ X509DataInfo, find_signature_node, parse_key_info, parse_signed_info, }; pub use sign::{ - ComputedReferenceDigest, EcdsaP256SigningKey, EcdsaP384SigningKey, RsaSigningKey, SignContext, - SigningDigestError, SigningError, SigningKey, SigningKeyError, compute_reference_digest_values, + ComputedReferenceDigest, EcdsaP256SigningKey, EcdsaP384SigningKey, KeyInfoWriteError, + KeyInfoWriter, RsaSigningKey, SignContext, SigningDigestError, SigningError, SigningKey, + SigningKeyError, X509CertificateKeyInfoWriter, compute_reference_digest_values, fill_reference_digest_values, }; pub use signature::{ diff --git a/src/xmldsig/mutation.rs b/src/xmldsig/mutation.rs index 77bfe9c..82a2df3 100644 --- a/src/xmldsig/mutation.rs +++ b/src/xmldsig/mutation.rs @@ -126,6 +126,7 @@ where .into_iter() .map(|value| value.as_ref().to_owned()) .collect(); + let target_signature = last_signature_index(xml)?; let expected = count_signed_info_digest_values(xml)?; if expected != values.len() { return Err(XmlMutationError::ValueCountMismatch { @@ -135,7 +136,9 @@ where }); } - fill_dsig_values_matching(xml, "DigestValue", values, is_signed_info_reference_context) + fill_dsig_values_matching(xml, "DigestValue", values, |stack, namespace| { + is_signed_info_reference_context(stack, namespace, target_signature) + }) } /// Fill XMLDSig `` elements in document order. @@ -149,6 +152,7 @@ where /// Fill the direct `/` child for a signing template. pub fn fill_signature_value(xml: &str, value: &str) -> Result { + let target_signature = last_signature_index(xml)?; let expected = count_direct_signature_values(xml)?; if expected != 1 { return Err(XmlMutationError::ValueCountMismatch { @@ -162,10 +166,27 @@ pub fn fill_signature_value(xml: &str, value: &str) -> Result/` child with XML child content. +pub fn fill_key_info(xml: &str, key_info_content: &str) -> Result { + let target_signature = last_signature_index(xml)?; + let expected = count_direct_key_infos(xml)?; + if expected != 1 { + return Err(XmlMutationError::ValueCountMismatch { + element: "KeyInfo", + expected, + actual: 1, + }); + } + + fill_dsig_element_raw_matching(xml, "KeyInfo", key_info_content, |stack, namespace| { + is_direct_signature_context(stack, namespace, target_signature) + }) +} + fn fill_dsig_values( xml: &str, local_name: &'static str, @@ -195,14 +216,15 @@ fn fill_dsig_values_matching( xml: &str, local_name: &'static str, values: Vec, - mut should_replace: impl FnMut(&[(bool, Vec)], &ResolveResult<'_>) -> bool, + mut should_replace: impl FnMut(&[(bool, Vec, Option)], &ResolveResult<'_>) -> bool, ) -> Result { let mut reader = NsReader::from_str(xml); let mut writer = Writer::new(Vec::new()); let mut buf = Vec::new(); let mut value_index = 0usize; let mut replacing_depth: Option = None; - let mut element_stack: Vec<(bool, Vec)> = Vec::new(); + let mut element_stack: Vec<(bool, Vec, Option)> = Vec::new(); + let mut signature_index = 0usize; loop { let (namespace, event) = reader.read_resolved_event_into(&mut buf)?; @@ -227,9 +249,15 @@ fn fill_dsig_values_matching( if is_dsig_element(&namespace, element.local_name().as_ref(), local_name) && should_replace(&element_stack, &namespace) => { + let signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); element_stack.push(( is_dsig_namespace(&namespace), element.local_name().as_ref().to_vec(), + signature, )); writer.write_event(Event::Start(element))?; writer.write_event(Event::Text(BytesText::new(&values[value_index])))?; @@ -240,19 +268,37 @@ fn fill_dsig_values_matching( if is_dsig_element(&namespace, element.local_name().as_ref(), local_name) && should_replace(&element_stack, &namespace) => { + let _signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); writer.write_event(Event::Start(element.borrow()))?; writer.write_event(Event::Text(BytesText::new(&values[value_index])))?; value_index += 1; writer.write_event(Event::End(element.to_end()))?; } Event::Start(element) => { + let signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); element_stack.push(( is_dsig_namespace(&namespace), element.local_name().as_ref().to_vec(), + signature, )); writer.write_event(Event::Start(element))?; } - Event::Empty(element) => writer.write_event(Event::Empty(element))?, + Event::Empty(element) => { + let _signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); + writer.write_event(Event::Empty(element))? + } Event::End(element) => { element_stack.pop(); writer.write_event(Event::End(element))?; @@ -276,6 +322,105 @@ fn fill_dsig_values_matching( Ok(output) } +fn fill_dsig_element_raw_matching( + xml: &str, + local_name: &'static str, + content: &str, + mut should_replace: impl FnMut(&[(bool, Vec, Option)], &ResolveResult<'_>) -> bool, +) -> Result { + let mut reader = NsReader::from_str(xml); + let mut writer = Writer::new(Vec::new()); + let mut buf = Vec::new(); + let mut replacing_depth: Option = None; + let mut element_stack: Vec<(bool, Vec, Option)> = Vec::new(); + let mut signature_index = 0usize; + + loop { + let (namespace, event) = reader.read_resolved_event_into(&mut buf)?; + if let Some(depth) = replacing_depth.as_mut() { + match event { + Event::Start(_) => *depth += 1, + Event::End(end) if *depth == 0 => { + writer.write_event(Event::End(end))?; + replacing_depth = None; + element_stack.pop(); + } + Event::End(_) => *depth -= 1, + Event::Eof => break, + _ => {} + } + buf.clear(); + continue; + } + + match event { + Event::Start(element) + if is_dsig_element(&namespace, element.local_name().as_ref(), local_name) + && should_replace(&element_stack, &namespace) => + { + let signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); + element_stack.push(( + is_dsig_namespace(&namespace), + element.local_name().as_ref().to_vec(), + signature, + )); + writer.write_event(Event::Start(element))?; + writer.get_mut().write_all(content.as_bytes())?; + replacing_depth = Some(0); + } + Event::Empty(element) + if is_dsig_element(&namespace, element.local_name().as_ref(), local_name) + && should_replace(&element_stack, &namespace) => + { + let _signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); + writer.write_event(Event::Start(element.borrow()))?; + writer.get_mut().write_all(content.as_bytes())?; + writer.write_event(Event::End(element.to_end()))?; + } + Event::Start(element) => { + let signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); + element_stack.push(( + is_dsig_namespace(&namespace), + element.local_name().as_ref().to_vec(), + signature, + )); + writer.write_event(Event::Start(element))?; + } + Event::Empty(element) => { + let _signature = signature_stack_index( + &namespace, + element.local_name().as_ref(), + &mut signature_index, + ); + writer.write_event(Event::Empty(element))? + } + Event::End(element) => { + element_stack.pop(); + writer.write_event(Event::End(element))?; + } + Event::Eof => break, + event => writer.write_event(event)?, + } + buf.clear(); + } + + let output = String::from_utf8(writer.into_inner())?; + roxmltree::Document::parse(&output)?; + Ok(output) +} + fn validate_signature_template(signature_template: &str) -> Result<(), XmlMutationError> { let document = roxmltree::Document::parse(signature_template)?; let root = document.root_element(); @@ -300,28 +445,74 @@ fn count_dsig_elements(xml: &str, local_name: &str) -> Result Result { let document = roxmltree::Document::parse(xml)?; + let Some(signature) = last_signature_node(&document) else { + return Ok(0); + }; Ok(document .descendants() - .filter(|node| is_direct_signed_info_reference_digest(*node)) + .filter(|node| is_direct_signed_info_reference_digest(*node, signature)) .count()) } fn count_direct_signature_values(xml: &str) -> Result { let document = roxmltree::Document::parse(xml)?; + let Some(signature) = last_signature_node(&document) else { + return Ok(0); + }; Ok(document .descendants() .filter(|node| { node.is_element() && node.tag_name().namespace() == Some(XMLDSIG_NS) && node.tag_name().name() == "SignatureValue" - && node - .parent() - .is_some_and(|parent| is_dsig_node(parent, "Signature")) + && node.parent().is_some_and(|parent| parent == signature) + }) + .count()) +} + +fn count_direct_key_infos(xml: &str) -> Result { + let document = roxmltree::Document::parse(xml)?; + let Some(signature) = last_signature_node(&document) else { + return Ok(0); + }; + Ok(document + .descendants() + .filter(|node| { + node.is_element() + && node.tag_name().namespace() == Some(XMLDSIG_NS) + && node.tag_name().name() == "KeyInfo" + && node.parent().is_some_and(|parent| parent == signature) }) .count()) } -fn is_direct_signed_info_reference_digest(node: roxmltree::Node<'_, '_>) -> bool { +fn last_signature_node<'a>( + document: &'a roxmltree::Document<'a>, +) -> Option> { + document + .descendants() + .rfind(|node| is_dsig_node(*node, "Signature")) +} + +fn last_signature_index(xml: &str) -> Result { + let document = roxmltree::Document::parse(xml)?; + document + .descendants() + .filter(|node| is_dsig_node(*node, "Signature")) + .enumerate() + .last() + .map(|(index, _)| index) + .ok_or(XmlMutationError::ValueCountMismatch { + element: "Signature", + expected: 1, + actual: 0, + }) +} + +fn is_direct_signed_info_reference_digest( + node: roxmltree::Node<'_, '_>, + signature: roxmltree::Node<'_, '_>, +) -> bool { node.is_element() && node.tag_name().namespace() == Some(XMLDSIG_NS) && node.tag_name().name() == "DigestValue" @@ -332,6 +523,11 @@ fn is_direct_signed_info_reference_digest(node: roxmltree::Node<'_, '_>) -> bool .parent() .and_then(|parent| parent.parent()) .is_some_and(|grandparent| is_dsig_node(grandparent, "SignedInfo")) + && node + .parent() + .and_then(|parent| parent.parent()) + .and_then(|grandparent| grandparent.parent()) + .is_some_and(|parent| parent == signature) } fn is_dsig_node(node: roxmltree::Node<'_, '_>, expected_local: &str) -> bool { @@ -341,29 +537,48 @@ fn is_dsig_node(node: roxmltree::Node<'_, '_>, expected_local: &str) -> bool { } fn is_signed_info_reference_context( - element_stack: &[(bool, Vec)], + element_stack: &[(bool, Vec, Option)], namespace: &ResolveResult<'_>, + target_signature: usize, ) -> bool { is_dsig_namespace(namespace) + && is_in_target_signature(element_stack, target_signature) && matches!( element_stack, - [.., (true, signed_info), (true, reference)] + [.., (true, signed_info, _), (true, reference, _)] if signed_info.as_slice() == b"SignedInfo" && reference.as_slice() == b"Reference" ) } fn is_direct_signature_context( - element_stack: &[(bool, Vec)], + element_stack: &[(bool, Vec, Option)], namespace: &ResolveResult<'_>, + target_signature: usize, ) -> bool { is_dsig_namespace(namespace) + && is_in_target_signature(element_stack, target_signature) && matches!( element_stack, - [.., (true, signature)] if signature.as_slice() == b"Signature" + [.., (true, signature, Some(index))] + if signature.as_slice() == b"Signature" && *index == target_signature ) } +fn is_in_target_signature( + element_stack: &[(bool, Vec, Option)], + target_signature: usize, +) -> bool { + element_stack + .iter() + .rev() + .any(|(is_dsig, local_name, signature)| { + *is_dsig + && local_name.as_slice() == b"Signature" + && *signature == Some(target_signature) + }) +} + fn is_dsig_element(namespace: &ResolveResult<'_>, local: &[u8], expected_local: &str) -> bool { is_dsig_namespace(namespace) && local == expected_local.as_bytes() } @@ -372,6 +587,20 @@ fn is_dsig_namespace(namespace: &ResolveResult<'_>) -> bool { matches!(namespace, ResolveResult::Bound(Namespace(ns)) if *ns == XMLDSIG_NS.as_bytes()) } +fn signature_stack_index( + namespace: &ResolveResult<'_>, + local_name: &[u8], + next_signature_index: &mut usize, +) -> Option { + if is_dsig_namespace(namespace) && local_name == b"Signature" { + let index = *next_signature_index; + *next_signature_index += 1; + Some(index) + } else { + None + } +} + #[cfg(test)] mod tests { use crate::c14n::{C14nAlgorithm, C14nMode}; diff --git a/src/xmldsig/sign.rs b/src/xmldsig/sign.rs index b7d75f0..c3553a0 100644 --- a/src/xmldsig/sign.rs +++ b/src/xmldsig/sign.rs @@ -8,7 +8,7 @@ use base64::Engine; use getrandom::SysRng; use p256::ecdsa::{Signature as P256Signature, SigningKey as P256SigningKey}; -use p256::pkcs8::DecodePrivateKey; +use p256::pkcs8::{DecodePrivateKey, EncodePublicKey}; use p384::ecdsa::{Signature as P384Signature, SigningKey as P384SigningKey}; use roxmltree::{Document, Node}; use rsa::RsaPrivateKey; @@ -17,13 +17,14 @@ use rsa::pkcs1v15::SigningKey as RsaPkcs1v15SigningKey; use rsa::signature::{RandomizedSigner, SignatureEncoding, Signer}; use sha2::{Sha256, Sha384, Sha512}; use std::collections::HashSet; +use x509_parser::prelude::FromDer; use crate::c14n::canonicalize; use super::builder::{SignatureBuilder, SignatureBuilderError}; use super::digest::{DigestAlgorithm, compute_digest}; use super::mutation::{ - XmlMutationError, append_signature_to_root, fill_signature_value, + XmlMutationError, append_signature_to_root, fill_key_info, fill_signature_value, fill_signed_info_digest_values, }; use super::parse::{SignatureAlgorithm, XMLDSIG_NS, parse_signed_info}; @@ -109,6 +110,10 @@ pub enum SigningError { #[error("XML mutation error: {0}")] XmlMutation(#[from] XmlMutationError), + /// Writing `` failed. + #[error("KeyInfo writer error: {0}")] + KeyInfo(#[from] KeyInfoWriteError), + /// Signature template generation failed. #[error("signature template error: {0}")] Template(#[from] SignatureBuilderError), @@ -143,6 +148,10 @@ pub enum SigningKeyError { /// The private-key signing operation failed. #[error("private-key signing operation failed")] SigningFailed, + + /// Public-key encoding failed for a supported signing key. + #[error("failed to encode signing public key as SPKI DER")] + PublicKeyEncodingFailed, } /// Private key abstraction used by [`SignContext`]. @@ -153,6 +162,95 @@ pub trait SigningKey { algorithm: SignatureAlgorithm, canonical_signed_info: &[u8], ) -> Result, SigningKeyError>; + + /// Return the public key corresponding to this signing key as SPKI DER. + fn public_key_spki_der(&self) -> Result, SigningKeyError>; +} + +/// Writes signing key metadata into a template `` element. +pub trait KeyInfoWriter { + /// Return XML child content for the direct `/` element. + fn write_key_info(&self, signing_key: &dyn SigningKey) -> Result; +} + +/// Errors while preparing XMLDSig signing `` output. +#[derive(Debug, thiserror::Error)] +#[non_exhaustive] +pub enum KeyInfoWriteError { + /// PEM input could not be parsed. + #[error("invalid PEM certificate")] + InvalidCertificatePem, + + /// PEM block was not an X.509 certificate. + #[error("invalid certificate format: expected CERTIFICATE PEM, got {label}")] + InvalidCertificateFormat { + /// Actual PEM label. + label: String, + }, + + /// DER bytes could not be decoded as one complete X.509 certificate. + #[error("invalid X.509 certificate DER")] + InvalidCertificateDer, + + /// The signing key could not expose public-key material for validation. + #[error("signing key public-key extraction failed: {0}")] + SigningKey(#[from] SigningKeyError), + + /// The configured certificate does not contain the signing key's public key. + #[error("X.509 certificate public key does not match signing key")] + CertificateKeyMismatch, +} + +/// `` writer that embeds one DER X.509 certificate. +pub struct X509CertificateKeyInfoWriter { + certificate_der: Vec, +} + +impl X509CertificateKeyInfoWriter { + /// Parse a PEM `CERTIFICATE` block for XMLDSig `` output. + pub fn from_pem(certificate_pem: &str) -> Result { + let (rest, pem) = x509_parser::pem::parse_x509_pem(certificate_pem.as_bytes()) + .map_err(|_| KeyInfoWriteError::InvalidCertificatePem)?; + if !rest.iter().all(|byte| byte.is_ascii_whitespace()) { + return Err(KeyInfoWriteError::InvalidCertificatePem); + } + if pem.label != "CERTIFICATE" { + return Err(KeyInfoWriteError::InvalidCertificateFormat { label: pem.label }); + } + Self::from_der(&pem.contents) + } + + /// Validate and store DER certificate bytes for XMLDSig `` output. + pub fn from_der(certificate_der: &[u8]) -> Result { + let (rest, _) = x509_parser::certificate::X509Certificate::from_der(certificate_der) + .map_err(|_| KeyInfoWriteError::InvalidCertificateDer)?; + if !rest.is_empty() { + return Err(KeyInfoWriteError::InvalidCertificateDer); + } + Ok(Self { + certificate_der: certificate_der.to_vec(), + }) + } +} + +impl KeyInfoWriter for X509CertificateKeyInfoWriter { + fn write_key_info(&self, signing_key: &dyn SigningKey) -> Result { + let (rest, certificate) = + x509_parser::certificate::X509Certificate::from_der(&self.certificate_der) + .map_err(|_| KeyInfoWriteError::InvalidCertificateDer)?; + if !rest.is_empty() { + return Err(KeyInfoWriteError::InvalidCertificateDer); + } + if certificate.public_key().raw != signing_key.public_key_spki_der()? { + return Err(KeyInfoWriteError::CertificateKeyMismatch); + } + + let certificate_b64 = + base64::engine::general_purpose::STANDARD.encode(&self.certificate_der); + Ok(format!( + "{certificate_b64}" + )) + } } /// RSA PKCS#1 v1.5 private key for XMLDSig signing. @@ -199,6 +297,14 @@ impl SigningKey for RsaSigningKey { }), } } + + fn public_key_spki_der(&self) -> Result, SigningKeyError> { + self.key + .to_public_key() + .to_public_key_der() + .map(|doc| doc.as_bytes().to_vec()) + .map_err(|_| SigningKeyError::PublicKeyEncodingFailed) + } } fn sign_rsa_pkcs1v15_with_rng( @@ -248,6 +354,14 @@ impl SigningKey for EcdsaP256SigningKey { .map_err(|_| SigningKeyError::SigningFailed)?; Ok(signature.to_bytes().to_vec()) } + + fn public_key_spki_der(&self) -> Result, SigningKeyError> { + self.key + .verifying_key() + .to_public_key_der() + .map(|doc| doc.as_bytes().to_vec()) + .map_err(|_| SigningKeyError::PublicKeyEncodingFailed) + } } /// ECDSA P-384 private key for XMLDSig signing. @@ -287,17 +401,36 @@ impl SigningKey for EcdsaP384SigningKey { .map_err(|_| SigningKeyError::SigningFailed)?; Ok(signature.to_bytes().to_vec()) } + + fn public_key_spki_der(&self) -> Result, SigningKeyError> { + self.key + .verifying_key() + .to_public_key_der() + .map(|doc| doc.as_bytes().to_vec()) + .map_err(|_| SigningKeyError::PublicKeyEncodingFailed) + } } /// XMLDSig signing context. pub struct SignContext<'a> { signing_key: &'a dyn SigningKey, + key_info_writer: Option<&'a dyn KeyInfoWriter>, } impl<'a> SignContext<'a> { /// Create a signing context using the supplied private key. pub fn new(signing_key: &'a dyn SigningKey) -> Self { - Self { signing_key } + Self { + signing_key, + key_info_writer: None, + } + } + + /// Configure signing to populate the direct `/` placeholder. + #[must_use] + pub fn key_info_writer(mut self, writer: &'a dyn KeyInfoWriter) -> Self { + self.key_info_writer = Some(writer); + self } /// Sign XML that already contains a `` template. @@ -311,7 +444,13 @@ impl<'a> SignContext<'a> { let (algorithm, canonical_signed_info) = canonicalize_signed_info(&with_digests)?; let signature_value = self.signing_key.sign(algorithm, &canonical_signed_info)?; let signature_b64 = base64::engine::general_purpose::STANDARD.encode(signature_value); - Ok(fill_signature_value(&with_digests, &signature_b64)?) + let signed = fill_signature_value(&with_digests, &signature_b64)?; + if let Some(writer) = self.key_info_writer { + let key_info_content = writer.write_key_info(self.signing_key)?; + Ok(fill_key_info(&signed, &key_info_content)?) + } else { + Ok(signed) + } } /// Build a signature template, append it to the source root, then sign it. @@ -335,14 +474,15 @@ struct SigningReference { /// Compute base64 digest values for every `` in the signing template. /// -/// References are processed in `` document order. The input must -/// contain exactly one XMLDSig `` element so an enveloped-signature -/// transform cannot accidentally target the wrong signature subtree. +/// References are processed in `` document order under the last +/// XMLDSig `` element. `sign_with_builder()` appends a new template +/// at the end of the source root, so older signatures in an already-signed +/// document must not become the signing target. pub fn compute_reference_digest_values( xml: &str, ) -> Result, SigningDigestError> { let doc = Document::parse(xml)?; - let signature = find_single_signature_node(&doc)?; + let signature = find_signing_signature_node(&doc)?; let signed_info = find_required_child(signature, "SignedInfo")?; let references = parse_signing_references(signed_info)?; let resolver = UriReferenceResolver::new(&doc); @@ -380,7 +520,7 @@ pub fn fill_reference_digest_values(xml: &str) -> Result Result<(SignatureAlgorithm, Vec), SigningError> { let doc = Document::parse(xml).map_err(SigningDigestError::XmlParse)?; - let signature = find_single_signature_node(&doc).map_err(SigningError::Digest)?; + let signature = find_signing_signature_node(&doc).map_err(SigningError::Digest)?; let signed_info_node = find_required_child(signature, "SignedInfo").map_err(SigningError::Digest)?; let signed_info = parse_signed_info(signed_info_node)?; @@ -410,25 +550,18 @@ fn parse_private_key_pem(private_key_pem: &str) -> Result, SigningKeyErr Ok(pem.contents) } -fn find_single_signature_node<'a>( +fn find_signing_signature_node<'a>( doc: &'a Document<'a>, ) -> Result, SigningDigestError> { - let mut signatures = doc.descendants().filter(|node| { - node.is_element() - && node.tag_name().name() == "Signature" - && node.tag_name().namespace() == Some(XMLDSIG_NS) - }); - let signature = signatures - .next() + doc.descendants() + .rfind(|node| { + node.is_element() + && node.tag_name().name() == "Signature" + && node.tag_name().namespace() == Some(XMLDSIG_NS) + }) .ok_or(SigningDigestError::MissingElement { element: "Signature", - })?; - if signatures.next().is_some() { - return Err(SigningDigestError::InvalidStructure( - "expected exactly one element".into(), - )); - } - Ok(signature) + }) } fn parse_signing_references( diff --git a/tests/signing_digest.rs b/tests/signing_digest.rs index a0949e4..1cabd7d 100644 --- a/tests/signing_digest.rs +++ b/tests/signing_digest.rs @@ -4,9 +4,10 @@ use xml_sec::xmldsig::parse::{find_signature_node, parse_signed_info}; use xml_sec::xmldsig::uri::UriReferenceResolver; use xml_sec::xmldsig::verify::process_all_references; use xml_sec::xmldsig::{ - DigestAlgorithm, DsigStatus, EcdsaP256SigningKey, EcdsaP384SigningKey, ReferenceBuilder, - RsaSigningKey, SignContext, SignatureAlgorithm, SignatureBuilder, SigningDigestError, - Transform, compute_reference_digest_values, fill_reference_digest_values, + DefaultKeyResolver, DigestAlgorithm, DsigStatus, EcdsaP256SigningKey, EcdsaP384SigningKey, + KeyInfoWriter, ReferenceBuilder, RsaSigningKey, SignContext, SignatureAlgorithm, + SignatureBuilder, SigningDigestError, SigningError, Transform, X509CertificateKeyInfoWriter, + compute_reference_digest_values, fill_reference_digest_values, parse_key_info, verify_signature_with_pem_key, }; @@ -199,6 +200,237 @@ fn signs_rsa_sha256_template_and_verifies_round_trip() { assert!(!signed.contains("")); } +#[test] +fn x509_key_info_writer_serializes_certificate_data() { + // The writer emits XMLDSig X509Data child content, not escaped text, so the + // existing KeyInfo parser must be able to consume it directly. + let certificate = X509CertificateKeyInfoWriter::from_pem(&read_fixture( + "tests/fixtures/keys/rsa/rsa-2048-cert.pem", + )) + .expect("RSA certificate fixture must parse"); + let key_info_xml = format!( + "{}", + certificate + .write_key_info( + &RsaSigningKey::from_pkcs8_pem(&read_fixture( + "tests/fixtures/keys/rsa/rsa-2048-key.pem", + )) + .expect("RSA private key fixture must parse"), + ) + .expect("write KeyInfo") + ); + let doc = roxmltree::Document::parse(&key_info_xml).expect("writer output must parse"); + let key_info = parse_key_info(doc.root_element()).expect("writer output must parse as KeyInfo"); + + assert_eq!(key_info.sources.len(), 1); +} + +#[test] +fn signs_rsa_template_with_embedded_x509_key_info() { + // KeyInfo is outside SignedInfo, but SAML verifiers commonly need the + // embedded signing certificate to resolve the public key. This verifies the + // writer path through the existing DefaultKeyResolver. + let private_key = + RsaSigningKey::from_pkcs8_pem(&read_fixture("tests/fixtures/keys/rsa/rsa-2048-key.pem")) + .expect("RSA private key fixture must parse"); + let key_info_writer = X509CertificateKeyInfoWriter::from_pem(&read_fixture( + "tests/fixtures/keys/rsa/rsa-2048-cert.pem", + )) + .expect("RSA certificate fixture must parse"); + let builder = SignatureBuilder::new(exclusive_c14n(), SignatureAlgorithm::RsaSha256) + .key_info(true) + .add_reference( + ReferenceBuilder::new(DigestAlgorithm::Sha256) + .uri("#payload") + .transform(Transform::C14n(exclusive_c14n())), + ); + + let signed = SignContext::new(&private_key) + .key_info_writer(&key_info_writer) + .sign_with_builder( + "hello", + &builder, + ) + .expect("RSA signing with KeyInfo must succeed"); + let resolver = DefaultKeyResolver::default(); + let verify_result = xml_sec::xmldsig::VerifyContext::new() + .key_resolver(&resolver) + .verify(&signed) + .expect("embedded certificate KeyInfo must resolve"); + + assert_eq!(verify_result.status, DsigStatus::Valid); + assert!(signed.contains("")); + assert!(signed.contains("")); +} + +#[test] +fn key_info_writer_requires_direct_template_placeholder() { + // The writer is intentionally opt-in and template-scoped. Without a direct + // KeyInfo slot, signing fails instead of inventing insertion policy. + let private_key = + RsaSigningKey::from_pkcs8_pem(&read_fixture("tests/fixtures/keys/rsa/rsa-2048-key.pem")) + .expect("RSA private key fixture must parse"); + let key_info_writer = X509CertificateKeyInfoWriter::from_pem(&read_fixture( + "tests/fixtures/keys/rsa/rsa-2048-cert.pem", + )) + .expect("RSA certificate fixture must parse"); + let builder = SignatureBuilder::new(exclusive_c14n(), SignatureAlgorithm::RsaSha256) + .add_reference( + ReferenceBuilder::new(DigestAlgorithm::Sha256) + .uri("#payload") + .transform(Transform::C14n(exclusive_c14n())), + ); + + let err = SignContext::new(&private_key) + .key_info_writer(&key_info_writer) + .sign_with_builder( + "hello", + &builder, + ) + .expect_err("writer without KeyInfo placeholder must fail"); + + assert!(matches!( + err, + SigningError::XmlMutation( + xml_sec::xmldsig::mutation::XmlMutationError::ValueCountMismatch { + element: "KeyInfo", + expected: 0, + actual: 1, + } + ) + )); +} + +#[test] +fn key_info_writer_rejects_duplicate_direct_template_placeholders() { + // Duplicate direct KeyInfo slots are ambiguous: signing must fail instead of + // choosing one and silently leaving another template placeholder behind. + let private_key = + RsaSigningKey::from_pkcs8_pem(&read_fixture("tests/fixtures/keys/rsa/rsa-2048-key.pem")) + .expect("RSA private key fixture must parse"); + let key_info_writer = X509CertificateKeyInfoWriter::from_pem(&read_fixture( + "tests/fixtures/keys/rsa/rsa-2048-cert.pem", + )) + .expect("RSA certificate fixture must parse"); + let template = SignatureBuilder::new(exclusive_c14n(), SignatureAlgorithm::RsaSha256) + .key_info(true) + .add_reference( + ReferenceBuilder::new(DigestAlgorithm::Sha256) + .uri("#payload") + .transform(Transform::C14n(exclusive_c14n())), + ) + .build_template() + .expect("valid signature template") + .replace("", ""); + let xml = append_signature_to_root( + "hello", + &template, + ) + .expect("append signature"); + + let err = SignContext::new(&private_key) + .key_info_writer(&key_info_writer) + .sign_template(&xml) + .expect_err("duplicate direct KeyInfo placeholders must fail"); + + assert!(matches!( + err, + SigningError::XmlMutation( + xml_sec::xmldsig::mutation::XmlMutationError::ValueCountMismatch { + element: "KeyInfo", + expected: 2, + actual: 1, + } + ) + )); +} + +#[test] +fn x509_key_info_writer_rejects_certificate_for_different_key() { + // A successful signing call must not produce a document that embeds an + // unrelated certificate which the default resolver will later reject. + let private_key = + RsaSigningKey::from_pkcs8_pem(&read_fixture("tests/fixtures/keys/rsa/rsa-2048-key.pem")) + .expect("RSA private key fixture must parse"); + let key_info_writer = X509CertificateKeyInfoWriter::from_pem(&read_fixture( + "tests/fixtures/keys/ec/ec-prime256v1-cert.pem", + )) + .expect("EC certificate fixture must parse"); + let builder = SignatureBuilder::new(exclusive_c14n(), SignatureAlgorithm::RsaSha256) + .key_info(true) + .add_reference( + ReferenceBuilder::new(DigestAlgorithm::Sha256) + .uri("#payload") + .transform(Transform::C14n(exclusive_c14n())), + ); + + let err = SignContext::new(&private_key) + .key_info_writer(&key_info_writer) + .sign_with_builder( + "hello", + &builder, + ) + .expect_err("mismatched certificate must fail before output"); + + assert!(matches!( + err, + SigningError::KeyInfo(xml_sec::xmldsig::KeyInfoWriteError::CertificateKeyMismatch) + )); +} + +#[test] +fn sign_with_builder_targets_appended_signature_when_existing_key_info_is_present() { + // Signing an already-signed document should fill only the newly appended + // template. Existing Signature/KeyInfo blocks are immutable historical data. + let private_key = + RsaSigningKey::from_pkcs8_pem(&read_fixture("tests/fixtures/keys/rsa/rsa-2048-key.pem")) + .expect("RSA private key fixture must parse"); + let key_info_writer = X509CertificateKeyInfoWriter::from_pem(&read_fixture( + "tests/fixtures/keys/rsa/rsa-2048-cert.pem", + )) + .expect("RSA certificate fixture must parse"); + let first_builder = SignatureBuilder::new(exclusive_c14n(), SignatureAlgorithm::RsaSha256) + .key_info(true) + .add_reference( + ReferenceBuilder::new(DigestAlgorithm::Sha256) + .uri("#first") + .transform(Transform::C14n(exclusive_c14n())), + ); + let first_signed = SignContext::new(&private_key) + .key_info_writer(&key_info_writer) + .sign_with_builder( + "onetwo", + &first_builder, + ) + .expect("initial signing with KeyInfo must succeed"); + let second_builder = SignatureBuilder::new(exclusive_c14n(), SignatureAlgorithm::RsaSha256) + .key_info(true) + .add_reference( + ReferenceBuilder::new(DigestAlgorithm::Sha256) + .uri("#second") + .transform(Transform::C14n(exclusive_c14n())), + ); + + let second_signed = SignContext::new(&private_key) + .key_info_writer(&key_info_writer) + .sign_with_builder(&first_signed, &second_builder) + .expect("existing Signature/KeyInfo must not block appended template signing"); + let document = roxmltree::Document::parse(&second_signed).expect("signed XML must parse"); + let signature_count = document + .descendants() + .filter(|node| { + node.is_element() + && node.tag_name().namespace() == Some("http://www.w3.org/2000/09/xmldsig#") + && node.tag_name().name() == "Signature" + }) + .count(); + + assert_eq!(signature_count, 2); + assert_eq!(second_signed.matches("").count(), 2); + assert!(!second_signed.contains("")); + assert!(!second_signed.contains("")); +} + #[test] fn signing_fills_only_top_level_signature_value() { // Object payloads may contain SignatureValue-named XMLDSig elements. The