diff --git a/src/xmldsig/mod.rs b/src/xmldsig/mod.rs index 781b098..3a913e0 100644 --- a/src/xmldsig/mod.rs +++ b/src/xmldsig/mod.rs @@ -15,10 +15,12 @@ pub mod transforms; pub mod types; pub mod uri; pub mod verify; +pub(crate) mod whitespace; pub use digest::{DigestAlgorithm, compute_digest, constant_time_eq}; pub use parse::{ - ParseError, Reference, SignatureAlgorithm, SignedInfo, find_signature_node, parse_signed_info, + KeyInfo, KeyInfoSource, KeyValueInfo, ParseError, Reference, SignatureAlgorithm, SignedInfo, + X509DataInfo, find_signature_node, parse_key_info, parse_signed_info, }; pub use signature::{ SignatureVerificationError, verify_ecdsa_signature_pem, verify_ecdsa_signature_spki, diff --git a/src/xmldsig/parse.rs b/src/xmldsig/parse.rs index f4ba716..368f9dc 100644 --- a/src/xmldsig/parse.rs +++ b/src/xmldsig/parse.rs @@ -20,10 +20,17 @@ use roxmltree::{Document, Node}; use super::digest::DigestAlgorithm; use super::transforms::{self, Transform}; +use super::whitespace::{is_xml_whitespace_only, normalize_xml_base64_text}; use crate::c14n::C14nAlgorithm; /// XMLDSig namespace URI. pub(crate) const XMLDSIG_NS: &str = "http://www.w3.org/2000/09/xmldsig#"; +/// XMLDSig 1.1 namespace URI. +pub(crate) const XMLDSIG11_NS: &str = "http://www.w3.org/2009/xmldsig11#"; +const MAX_DER_ENCODED_KEY_VALUE_LEN: usize = 8192; +const MAX_DER_ENCODED_KEY_VALUE_TEXT_LEN: usize = 65_536; +const MAX_DER_ENCODED_KEY_VALUE_BASE64_LEN: usize = MAX_DER_ENCODED_KEY_VALUE_LEN.div_ceil(3) * 4; +const MAX_KEY_NAME_TEXT_LEN: usize = 4096; /// Signature algorithms supported for signing and verification. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -110,6 +117,63 @@ pub struct Reference { pub digest_value: Vec, } +/// Parsed `` element. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct KeyInfo { + /// Sources discovered under `` in document order. + pub sources: Vec, +} + +/// Top-level key material source parsed from ``. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum KeyInfoSource { + /// `` source. + KeyName(String), + /// `` source. + KeyValue(KeyValueInfo), + /// `` source. + X509Data(X509DataInfo), + /// `dsig11:DEREncodedKeyValue` source (base64-decoded DER bytes). + DerEncodedKeyValue(Vec), +} + +/// Parsed `` dispatch result. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum KeyValueInfo { + /// ``. + RsaKeyValue, + /// `dsig11:ECKeyValue` (the XMLDSig 1.1 namespace form). + EcKeyValue, + /// Any other `` child not yet supported by this phase. + Unsupported { + /// Namespace URI of the unsupported child, when present. + namespace: Option, + /// Local name of the unsupported child element. + local_name: String, + }, +} + +/// Parsed `` children (dispatch-only in P2-001). +#[derive(Debug, Default, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub struct X509DataInfo { + /// Number of `` children. + pub certificate_count: usize, + /// Number of `` children. + pub subject_name_count: usize, + /// Number of `` children. + pub issuer_serial_count: usize, + /// Number of `` children. + pub ski_count: usize, + /// Number of `` children. + pub crl_count: usize, + /// Number of `` children. + pub digest_count: usize, +} + /// Errors during XMLDSig element parsing. #[derive(Debug, thiserror::Error)] #[non_exhaustive] @@ -259,20 +323,7 @@ pub(crate) fn parse_reference(reference_node: Node) -> Result Result` and dispatch supported child sources. +/// +/// Supported source elements: +/// - `` +/// - `` (dispatch by child QName; only `dsig11:ECKeyValue` is treated as supported EC) +/// - `` +/// - `` +/// +/// Unknown top-level `` children are ignored (lax processing), while +/// unknown XMLDSig-owned (`ds:*` / `dsig11:*`) children inside `` are +/// rejected fail-closed. +/// `` may still be empty or contain only non-XMLDSig extension children. +pub fn parse_key_info(key_info_node: Node) -> Result { + verify_ds_element(key_info_node, "KeyInfo")?; + ensure_no_non_whitespace_text(key_info_node, "KeyInfo")?; + + let mut sources = Vec::new(); + for child in element_children(key_info_node) { + match (child.tag_name().namespace(), child.tag_name().name()) { + (Some(XMLDSIG_NS), "KeyName") => { + ensure_no_element_children(child, "KeyName")?; + let key_name = + collect_text_content_bounded(child, MAX_KEY_NAME_TEXT_LEN, "KeyName")?; + sources.push(KeyInfoSource::KeyName(key_name)); + } + (Some(XMLDSIG_NS), "KeyValue") => { + let key_value = parse_key_value_dispatch(child)?; + sources.push(KeyInfoSource::KeyValue(key_value)); + } + (Some(XMLDSIG_NS), "X509Data") => { + let x509 = parse_x509_data_dispatch(child)?; + sources.push(KeyInfoSource::X509Data(x509)); + } + (Some(XMLDSIG11_NS), "DEREncodedKeyValue") => { + ensure_no_element_children(child, "DEREncodedKeyValue")?; + let der = decode_der_encoded_key_value_base64(child)?; + sources.push(KeyInfoSource::DerEncodedKeyValue(der)); + } + _ => {} + } + } + + Ok(KeyInfo { sources }) +} + // ── Helpers ────────────────────────────────────────────────────────────────── /// Iterate only element children (skip text, comments, PIs). @@ -358,6 +454,72 @@ fn parse_inclusive_prefixes(node: Node) -> Result, ParseError> { Ok(None) } +fn parse_key_value_dispatch(node: Node) -> Result { + verify_ds_element(node, "KeyValue")?; + ensure_no_non_whitespace_text(node, "KeyValue")?; + + let mut children = element_children(node); + let Some(first_child) = children.next() else { + return Err(ParseError::InvalidStructure( + "KeyValue must contain exactly one key-value child".into(), + )); + }; + if children.next().is_some() { + return Err(ParseError::InvalidStructure( + "KeyValue must contain exactly one key-value child".into(), + )); + } + + match ( + first_child.tag_name().namespace(), + first_child.tag_name().name(), + ) { + (Some(XMLDSIG_NS), "RSAKeyValue") => Ok(KeyValueInfo::RsaKeyValue), + (Some(XMLDSIG11_NS), "ECKeyValue") => Ok(KeyValueInfo::EcKeyValue), + (namespace, child_name) => Ok(KeyValueInfo::Unsupported { + namespace: namespace.map(str::to_string), + local_name: child_name.to_string(), + }), + } +} + +fn parse_x509_data_dispatch(node: Node) -> Result { + verify_ds_element(node, "X509Data")?; + ensure_no_non_whitespace_text(node, "X509Data")?; + + let mut info = X509DataInfo::default(); + for child in element_children(node) { + match (child.tag_name().namespace(), child.tag_name().name()) { + (Some(XMLDSIG_NS), "X509Certificate") => { + info.certificate_count += 1; + } + (Some(XMLDSIG_NS), "X509SubjectName") => { + info.subject_name_count += 1; + } + (Some(XMLDSIG_NS), "X509IssuerSerial") => { + info.issuer_serial_count += 1; + } + (Some(XMLDSIG_NS), "X509SKI") => { + info.ski_count += 1; + } + (Some(XMLDSIG_NS), "X509CRL") => { + info.crl_count += 1; + } + (Some(XMLDSIG11_NS), "X509Digest") => { + info.digest_count += 1; + } + (Some(XMLDSIG_NS), child_name) | (Some(XMLDSIG11_NS), child_name) => { + return Err(ParseError::InvalidStructure(format!( + "X509Data contains unsupported XMLDSig child element <{child_name}>" + ))); + } + _ => {} + } + } + + Ok(info) +} + /// Base64-decode a digest value string, stripping whitespace. /// /// XMLDSig allows whitespace within base64 content (line-wrapped encodings). @@ -365,23 +527,23 @@ fn base64_decode_digest(b64: &str, digest_method: DigestAlgorithm) -> Result max_base64_len { + return Err(ParseError::Base64( + "DigestValue exceeds maximum allowed base64 length".into(), + )); } let digest = STANDARD .decode(&cleaned) .map_err(|e| ParseError::Base64(e.to_string()))?; - let expected = digest_method.output_len(); let actual = digest.len(); if actual != expected { return Err(ParseError::DigestLengthMismatch { @@ -393,10 +555,130 @@ fn base64_decode_digest(b64: &str, digest_method: DigestAlgorithm) -> Result, + digest_method: DigestAlgorithm, +) -> Result, ParseError> { + let max_base64_len = digest_method.output_len().div_ceil(3) * 4; + let mut cleaned = String::with_capacity(max_base64_len); + + for child in digest_value_node.children() { + if child.is_element() { + return Err(ParseError::InvalidStructure( + "DigestValue must not contain element children".into(), + )); + } + if let Some(text) = child.text() { + normalize_xml_base64_text(text, &mut cleaned).map_err(|err| { + ParseError::Base64(format!( + "invalid XML whitespace U+{:04X} in DigestValue", + err.invalid_byte + )) + })?; + if cleaned.len() > max_base64_len { + return Err(ParseError::Base64( + "DigestValue exceeds maximum allowed base64 length".into(), + )); + } + } + } + + base64_decode_digest(&cleaned, digest_method) +} + +fn decode_der_encoded_key_value_base64(node: Node<'_, '_>) -> Result, ParseError> { + use base64::Engine; + use base64::engine::general_purpose::STANDARD; + + let mut cleaned = String::new(); + let mut raw_text_len = 0usize; + for text in node + .children() + .filter(|child| child.is_text()) + .filter_map(|child| child.text()) + { + if raw_text_len.saturating_add(text.len()) > MAX_DER_ENCODED_KEY_VALUE_TEXT_LEN { + return Err(ParseError::InvalidStructure( + "DEREncodedKeyValue exceeds maximum allowed text length".into(), + )); + } + raw_text_len = raw_text_len.saturating_add(text.len()); + normalize_xml_base64_text(text, &mut cleaned).map_err(|err| { + ParseError::Base64(format!( + "invalid XML whitespace U+{:04X} in base64 text", + err.invalid_byte + )) + })?; + if cleaned.len() > MAX_DER_ENCODED_KEY_VALUE_BASE64_LEN { + return Err(ParseError::InvalidStructure( + "DEREncodedKeyValue exceeds maximum allowed length".into(), + )); + } + } + + let der = STANDARD + .decode(&cleaned) + .map_err(|e| ParseError::Base64(e.to_string()))?; + if der.is_empty() { + return Err(ParseError::InvalidStructure( + "DEREncodedKeyValue must not be empty".into(), + )); + } + if der.len() > MAX_DER_ENCODED_KEY_VALUE_LEN { + return Err(ParseError::InvalidStructure( + "DEREncodedKeyValue exceeds maximum allowed length".into(), + )); + } + Ok(der) +} + +fn collect_text_content_bounded( + node: Node<'_, '_>, + max_len: usize, + element_name: &'static str, +) -> Result { + let mut text = String::new(); + for chunk in node + .children() + .filter_map(|child| child.is_text().then(|| child.text()).flatten()) + { + if text.len().saturating_add(chunk.len()) > max_len { + return Err(ParseError::InvalidStructure(format!( + "{element_name} exceeds maximum allowed text length" + ))); + } + text.push_str(chunk); + } + Ok(text) +} + +fn ensure_no_element_children(node: Node<'_, '_>, element_name: &str) -> Result<(), ParseError> { + if node.children().any(|child| child.is_element()) { + return Err(ParseError::InvalidStructure(format!( + "{element_name} must not contain child elements" + ))); + } + Ok(()) +} + +fn ensure_no_non_whitespace_text(node: Node<'_, '_>, element_name: &str) -> Result<(), ParseError> { + for child in node.children().filter(|child| child.is_text()) { + if let Some(text) = child.text() + && !is_xml_whitespace_only(text) + { + return Err(ParseError::InvalidStructure(format!( + "{element_name} must not contain non-whitespace mixed content" + ))); + } + } + Ok(()) +} + #[cfg(test)] #[expect(clippy::unwrap_used, reason = "tests use trusted XML fixtures")] mod tests { use super::*; + use base64::Engine; // ── SignatureAlgorithm ─────────────────────────────────────────── @@ -486,6 +768,310 @@ mod tests { assert!(find_signature_node(&doc).is_none()); } + // ── parse_key_info: dispatch parsing ────────────────────────────── + + #[test] + fn parse_key_info_dispatches_supported_children() { + let xml = r#" + idp-signing-key + + + AQAB + AQAB + + + + MIIB + CN=Example + + AQIDBA== + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!(key_info.sources.len(), 4); + + assert_eq!( + key_info.sources[0], + KeyInfoSource::KeyName("idp-signing-key".to_string()) + ); + assert_eq!( + key_info.sources[1], + KeyInfoSource::KeyValue(KeyValueInfo::RsaKeyValue) + ); + assert_eq!( + key_info.sources[2], + KeyInfoSource::X509Data(X509DataInfo { + certificate_count: 1, + subject_name_count: 1, + issuer_serial_count: 0, + ski_count: 0, + crl_count: 0, + digest_count: 0, + }) + ); + assert_eq!( + key_info.sources[3], + KeyInfoSource::DerEncodedKeyValue(vec![1, 2, 3, 4]) + ); + } + + #[test] + fn parse_key_info_ignores_unknown_children() { + let xml = r#" + bar + ok + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!(key_info.sources, vec![KeyInfoSource::KeyName("ok".into())]); + } + + #[test] + fn parse_key_info_keyvalue_requires_single_child() { + let xml = r#" + + "#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_accepts_empty_x509data() { + let xml = r#" + + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::X509Data(X509DataInfo::default())] + ); + } + + #[test] + fn parse_key_info_rejects_unknown_xmlsig_child_in_x509data() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_rejects_unknown_xmlsig11_child_in_x509data() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_accepts_x509data_with_only_foreign_namespace_children() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::X509Data(X509DataInfo::default())] + ); + } + + #[test] + fn parse_key_info_der_encoded_key_value_rejects_invalid_base64() { + let xml = r#" + %%%invalid%%% + "#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::Base64(_))); + } + + #[test] + fn parse_key_info_der_encoded_key_value_accepts_xml_whitespace() { + let xml = r#" + + AQID + BA== + + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::DerEncodedKeyValue(vec![1, 2, 3, 4])] + ); + } + + #[test] + fn parse_key_info_dispatches_dsig11_ec_keyvalue() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::KeyValue(KeyValueInfo::EcKeyValue)] + ); + } + + #[test] + fn parse_key_info_marks_ds_namespace_ec_keyvalue_as_unsupported() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::KeyValue(KeyValueInfo::Unsupported { + namespace: Some(XMLDSIG_NS.to_string()), + local_name: "ECKeyValue".into(), + })] + ); + } + + #[test] + fn parse_key_info_keeps_unsupported_keyvalue_child_as_marker() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::KeyValue(KeyValueInfo::Unsupported { + namespace: Some(XMLDSIG_NS.to_string()), + local_name: "DSAKeyValue".into(), + })] + ); + } + + #[test] + fn parse_key_info_rejects_keyname_with_child_elements() { + let xml = r#" + ok + "#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_preserves_keyname_text_without_trimming() { + let xml = r#" + signing key + "#; + let doc = Document::parse(xml).unwrap(); + + let key_info = parse_key_info(doc.root_element()).unwrap(); + assert_eq!( + key_info.sources, + vec![KeyInfoSource::KeyName(" signing key ".into())] + ); + } + + #[test] + fn parse_key_info_rejects_oversized_keyname_text() { + let oversized = "A".repeat(4097); + let xml = format!( + "{oversized}" + ); + let doc = Document::parse(&xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_rejects_non_whitespace_mixed_content() { + let xml = r#"oopsk"#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_rejects_nbsp_as_non_xml_whitespace_mixed_content() { + let xml = "\u{00A0}k"; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_der_encoded_key_value_rejects_oversized_payload() { + let oversized = + base64::engine::general_purpose::STANDARD + .encode(vec![0u8; MAX_DER_ENCODED_KEY_VALUE_LEN + 1]); + let xml = format!( + "{oversized}" + ); + let doc = Document::parse(&xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_der_encoded_key_value_rejects_empty_payload() { + let xml = r#" + + + + "#; + let doc = Document::parse(xml).unwrap(); + + let err = parse_key_info(doc.root_element()).unwrap_err(); + assert!(matches!(err, ParseError::InvalidStructure(_))); + } + + #[test] + fn parse_key_info_der_encoded_key_value_non_xml_ascii_whitespace_is_not_parseable_xml() { + let xml = "\u{000C}"; + assert!(Document::parse(xml).is_err()); + } + // ── parse_signed_info: happy path ──────────────────────────────── #[test] @@ -909,6 +1495,21 @@ mod tests { assert!(matches!(err, ParseError::Base64(_))); } + #[test] + fn base64_decode_digest_rejects_oversized_base64_before_decode() { + let err = base64_decode_digest("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", DigestAlgorithm::Sha1) + .expect_err("oversized DigestValue base64 must fail before decode"); + match err { + ParseError::Base64(message) => { + assert!( + message.contains("DigestValue exceeds maximum allowed base64 length"), + "unexpected message: {message}" + ); + } + other => panic!("expected ParseError::Base64, got {other:?}"), + } + } + // ── Real-world SAML structure ──────────────────────────────────── #[test] diff --git a/src/xmldsig/verify.rs b/src/xmldsig/verify.rs index cd47efb..2b2d534 100644 --- a/src/xmldsig/verify.rs +++ b/src/xmldsig/verify.rs @@ -18,7 +18,7 @@ use crate::c14n::canonicalize; use super::digest::{DigestAlgorithm, compute_digest, constant_time_eq}; use super::parse::{ParseError, Reference, SignatureAlgorithm, XMLDSIG_NS}; -use super::parse::{parse_reference, parse_signed_info}; +use super::parse::{parse_key_info, parse_reference, parse_signed_info}; use super::signature::{ SignatureVerificationError, verify_ecdsa_signature_pem, verify_rsa_signature_pem, }; @@ -26,6 +26,7 @@ use super::transforms::{ DEFAULT_IMPLICIT_C14N_URI, Transform, XPATH_TRANSFORM_URI, execute_transforms, }; use super::uri::{UriReferenceResolver, parse_xpointer_id_fragment}; +use super::whitespace::{is_xml_whitespace_only, normalize_xml_base64_text}; const MAX_SIGNATURE_VALUE_LEN: usize = 8192; const MAX_SIGNATURE_VALUE_TEXT_LEN: usize = 65_536; @@ -55,6 +56,16 @@ pub trait KeyResolver { /// maps `Ok(None)` to `DsigStatus::Invalid(FailureReason::KeyNotFound)`; /// reserve `Err(...)` for resolver failures. fn resolve<'a>(&'a self, xml: &str) -> Result>, DsigError>; + + /// Return `true` when this resolver consumes document `` material. + /// + /// The verification pipeline uses this to decide whether malformed + /// `` should raise `DsigError::ParseKeyInfo` before resolver + /// execution. Resolvers that ignore document key material can keep the + /// default `false` to avoid fail-closed parsing on advisory ``. + fn consumes_document_key_info(&self) -> bool { + false + } } /// Allowed URI classes for ``. @@ -510,6 +521,10 @@ pub enum DsigError { #[error("failed to parse SignedInfo: {0}")] ParseSignedInfo(#[from] super::parse::ParseError), + /// `` parsing failed. + #[error("failed to parse KeyInfo: {0}")] + ParseKeyInfo(#[source] super::parse::ParseError), + /// `//` parsing failed. #[error("failed to parse Manifest reference: {0}")] ParseManifestReference(#[source] ParseError), @@ -550,19 +565,29 @@ type SignatureVerificationPipelineError = DsigError; /// Verify one XMLDSig `` end-to-end with a PEM public key. /// /// Pipeline: -/// 1. Parse `` -/// 2. Validate all `` digests (fail-fast) -/// 3. Canonicalize `` -/// 4. Base64-decode `` -/// 5. Verify signature bytes against canonicalized `` +/// 1. Parse `` children and enforce structural constraints +/// 2. Parse `` +/// 3. Validate all `` digests (fail-fast) +/// 4. Canonicalize `` +/// 5. Base64-decode `` +/// 6. Verify signature bytes against canonicalized `` using the provided PEM key /// /// If any `` digest mismatches, returns `Ok` with /// `status == Invalid(ReferenceDigestMismatch { .. })`. /// +/// This API uses only the provided PEM key and does not parse embedded +/// `` key material for key selection/validation. Consequently, +/// malformed optional `` does not produce `DsigError::ParseKeyInfo` +/// on this API path. +/// /// Structural constraints enforced by this API: /// - The document must contain exactly one XMLDSig `` element. /// - `` must be the first element child of `` and appear once. /// - `` must be the second element child of `` and appear once. +/// - `` is optional and, when present, must be the third element child. +/// - Only XMLDSig namespace element children are allowed under ``. +/// - Non-whitespace mixed text content under `` is rejected. +/// - After ``, ``, and optional ``, only `` elements are allowed. /// - `` must not contain nested element children. pub fn verify_signature_with_pem_key( xml: &str, @@ -617,6 +642,15 @@ fn verify_signature_with_context( let signature_children = parse_signature_children(signature_node)?; let signed_info_node = signature_children.signed_info_node; + let should_parse_key_info = match (ctx.key, ctx.key_resolver) { + (Some(_), _) => false, + (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 signed_info = parse_signed_info(signed_info_node)?; enforce_reference_policies( @@ -951,6 +985,7 @@ fn transform_uri(transform: &Transform) -> &'static str { struct SignatureChildNodes<'a, 'input> { signed_info_node: Node<'a, 'input>, signature_value_node: Node<'a, 'input>, + key_info_node: Option>, } fn parse_signature_children<'a, 'input>( @@ -958,17 +993,35 @@ fn parse_signature_children<'a, 'input>( ) -> Result, SignatureVerificationPipelineError> { let mut signed_info_node: Option> = None; let mut signature_value_node: Option> = None; + let mut key_info_node: Option> = None; let mut signed_info_index: Option = None; let mut signature_value_index: Option = None; - for (zero_based_index, child) in signature_node - .children() - .filter(|node| node.is_element()) - .enumerate() - { - let element_index = zero_based_index + 1; - if child.tag_name().namespace() != Some(XMLDSIG_NS) { + let mut key_info_index: Option = None; + let mut first_unexpected_dsig_index: Option = None; + + let mut element_index = 0usize; + for child in signature_node.children() { + if child.is_text() { + if child + .text() + .is_some_and(|text| !is_xml_whitespace_only(text)) + { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: "Signature must not contain non-whitespace mixed content", + }); + } + continue; + } + if !child.is_element() { continue; } + + element_index += 1; + if child.tag_name().namespace() != Some(XMLDSIG_NS) { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: "Signature must contain only XMLDSIG element children", + }); + } match child.tag_name().name() { "SignedInfo" => { if signed_info_node.is_some() { @@ -988,7 +1041,24 @@ fn parse_signature_children<'a, 'input>( signature_value_node = Some(child); signature_value_index = Some(element_index); } - _ => {} + "KeyInfo" => { + if key_info_node.is_some() { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: "KeyInfo must appear at most once under Signature", + }); + } + key_info_node = Some(child); + key_info_index = Some(element_index); + } + "Object" => { + // Valid Object elements are allowed only after SignedInfo, SignatureValue, + // and optional KeyInfo; this is enforced via first_unexpected_dsig_index. + } + _ => { + if first_unexpected_dsig_index.is_none() { + first_unexpected_dsig_index = Some(element_index); + } + } } } @@ -1010,9 +1080,29 @@ fn parse_signature_children<'a, 'input>( reason: "SignatureValue must be the second element child of Signature", }); } + if let Some(index) = key_info_index + && index != 3 + { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: "KeyInfo must be the third element child of Signature when present", + }); + } + + let allowed_prefix_end = key_info_index.unwrap_or(2); + if let Some(unexpected_index) = first_unexpected_dsig_index { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: if unexpected_index > allowed_prefix_end { + "After SignedInfo, SignatureValue, and optional KeyInfo, Signature may contain only Object elements" + } else { + "Signature may contain SignedInfo first, SignatureValue second, optional KeyInfo third, and Object elements thereafter" + }, + }); + } + Ok(SignatureChildNodes { signed_info_node, signature_value_node, + key_info_node, }) } @@ -1047,30 +1137,25 @@ fn push_normalized_signature_text( raw_text_len: &mut usize, normalized: &mut String, ) -> Result<(), SignatureVerificationPipelineError> { - for ch in text.chars() { - if raw_text_len.saturating_add(ch.len_utf8()) > MAX_SIGNATURE_VALUE_TEXT_LEN { - return Err(SignatureVerificationPipelineError::InvalidStructure { - reason: "SignatureValue exceeds maximum allowed text length", - }); - } - *raw_text_len = raw_text_len.saturating_add(ch.len_utf8()); - if matches!(ch, ' ' | '\t' | '\r' | '\n') { - continue; - } - if ch.is_ascii_whitespace() { - let invalid_byte = - u8::try_from(u32::from(ch)).expect("ASCII whitespace always fits into u8"); - return Err(SignatureVerificationPipelineError::SignatureValueBase64( - base64::DecodeError::InvalidByte(normalized.len(), invalid_byte), - )); - } - if normalized.len().saturating_add(ch.len_utf8()) > MAX_SIGNATURE_VALUE_LEN { - return Err(SignatureVerificationPipelineError::InvalidStructure { - reason: "SignatureValue exceeds maximum allowed length", - }); - } - normalized.push(ch); + if raw_text_len.saturating_add(text.len()) > MAX_SIGNATURE_VALUE_TEXT_LEN { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: "SignatureValue exceeds maximum allowed text length", + }); } + *raw_text_len = raw_text_len.saturating_add(text.len()); + + normalize_xml_base64_text(text, normalized).map_err(|err| { + SignatureVerificationPipelineError::SignatureValueBase64(base64::DecodeError::InvalidByte( + err.normalized_offset, + err.invalid_byte, + )) + })?; + if normalized.len() > MAX_SIGNATURE_VALUE_LEN { + return Err(SignatureVerificationPipelineError::InvalidStructure { + reason: "SignatureValue exceeds maximum allowed length", + }); + } + Ok(()) } @@ -1188,6 +1273,22 @@ mod tests { } } + struct ConsumingKeyInfoResolver; + + impl KeyResolver for ConsumingKeyInfoResolver { + fn resolve<'a>( + &'a self, + _xml: &str, + ) -> Result>, SignatureVerificationPipelineError> + { + Ok(None) + } + + fn consumes_document_key_info(&self) -> bool { + true + } + } + fn minimal_signature_xml(reference_uri: &str, transforms_xml: &str) -> String { format!( r#" @@ -1863,6 +1964,52 @@ mod tests { )); } + #[test] + fn verify_context_resolver_can_ignore_malformed_keyinfo_by_default() { + let base_xml = signature_with_target_reference("AQ=="); + let xml = base_xml + .replace( + r#""#, + r#""#, + ) + .replace( + "\n ", + "\n %%%invalid%%%\n ", + ); + + let result = VerifyContext::new() + .key_resolver(&MissingKeyResolver) + .verify(&xml) + .expect("resolver path should not hard-fail on advisory malformed KeyInfo by default"); + assert!(matches!( + result.status, + DsigStatus::Invalid(FailureReason::KeyNotFound) + )); + } + + #[test] + fn verify_context_resolver_can_opt_in_to_keyinfo_parse_failures() { + let base_xml = signature_with_target_reference("AQ=="); + let xml = base_xml + .replace( + r#""#, + r#""#, + ) + .replace( + "\n ", + "\n %%%invalid%%%\n ", + ); + + let err = VerifyContext::new() + .key_resolver(&ConsumingKeyInfoResolver) + .verify(&xml) + .expect_err("resolver opted into KeyInfo parsing, malformed KeyInfo must fail"); + assert!(matches!( + err, + SignatureVerificationPipelineError::ParseKeyInfo(_) + )); + } + #[test] fn verify_context_preserves_signaturevalue_decode_errors_when_resolver_misses() { let xml = signature_with_target_reference("@@@"); @@ -2444,4 +2591,153 @@ mod tests { } )); } + + #[test] + fn pipeline_reports_keyinfo_parse_error() { + let xml = r#" + + + + + + + AAAAAAAAAAAAAAAAAAAAAAAAAAA= + + + AA== + + %%%invalid%%% + + +"#; + + let err = VerifyContext::new().verify(xml).expect_err( + "invalid KeyInfo must map to ParseKeyInfo when no explicit key is supplied", + ); + assert!(matches!( + err, + SignatureVerificationPipelineError::ParseKeyInfo(_) + )); + } + + #[test] + fn pipeline_ignores_malformed_keyinfo_when_explicit_key_is_supplied() { + let base_xml = signature_with_target_reference("AQ=="); + let xml = base_xml + .replace( + r#""#, + r#""#, + ) + .replace( + "\n ", + "\n %%%invalid%%%\n ", + ); + + let result = VerifyContext::new() + .key(&RejectingKey) + .verify(&xml) + .expect("explicit key path should not fail on malformed KeyInfo"); + assert!(matches!( + result.status, + DsigStatus::Invalid(FailureReason::SignatureMismatch) + )); + } + + #[test] + fn pipeline_rejects_foreign_element_children_under_signature() { + let base_xml = signature_with_target_reference("AQ=="); + let xml = base_xml + .replace( + r#""#, + r#""#, + ) + .replace( + "\n ", + "\n \n ", + ); + + let err = VerifyContext::new() + .key(&RejectingKey) + .verify(&xml) + .expect_err("foreign element children under Signature must fail closed"); + assert!(matches!( + err, + SignatureVerificationPipelineError::InvalidStructure { + reason: "Signature must contain only XMLDSIG element children", + } + )); + } + + #[test] + fn pipeline_rejects_non_whitespace_mixed_content_under_signature() { + let base_xml = signature_with_target_reference("AQ=="); + let xml = base_xml.replace( + "\n ", + "\n oops\n ", + ); + + let err = VerifyContext::new() + .key(&RejectingKey) + .verify(&xml) + .expect_err("non-whitespace mixed content under Signature must fail closed"); + assert!(matches!( + err, + SignatureVerificationPipelineError::InvalidStructure { + reason: "Signature must not contain non-whitespace mixed content", + } + )); + } + + #[test] + fn pipeline_rejects_keyinfo_out_of_order() { + let base_xml = signature_with_target_reference("AQ=="); + let xml = base_xml.replace( + "\n ", + "\n \n late\n ", + ); + + let err = VerifyContext::new() + .key(&RejectingKey) + .verify(&xml) + .expect_err("KeyInfo after Object must be rejected by Signature child order checks"); + assert!(matches!( + err, + SignatureVerificationPipelineError::InvalidStructure { + reason: "KeyInfo must be the third element child of Signature when present" + } + )); + } + + #[test] + fn pipeline_accepts_comments_and_processing_instructions_under_signature() { + let xml = r#" + + + + + + + + + AAAAAAAAAAAAAAAAAAAAAAAAAAA= + + + + AA== + +"#; + + let doc = Document::parse(xml).expect("test XML must parse"); + let signature_node = doc.root_element(); + let parsed = parse_signature_children(signature_node) + .expect("comment/PI nodes under Signature must be ignored"); + + assert_eq!(parsed.signed_info_node.tag_name().name(), "SignedInfo"); + assert_eq!( + parsed.signature_value_node.tag_name().name(), + "SignatureValue" + ); + assert!(parsed.key_info_node.is_none()); + } } diff --git a/src/xmldsig/whitespace.rs b/src/xmldsig/whitespace.rs new file mode 100644 index 0000000..2e634a0 --- /dev/null +++ b/src/xmldsig/whitespace.rs @@ -0,0 +1,40 @@ +//! Internal XML whitespace helpers shared across XMLDSig parsing and verification. + +/// Return `true` when the text contains only XML 1.0 whitespace chars. +#[inline] +pub(crate) fn is_xml_whitespace_only(text: &str) -> bool { + text.chars() + .all(|ch| matches!(ch, ' ' | '\t' | '\r' | '\n')) +} + +/// Error returned when non-XML ASCII whitespace appears in base64 text. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct XmlBase64NormalizeError { + /// Offending ASCII byte. + pub invalid_byte: u8, + /// Offset in the normalized output where the byte was encountered. + pub normalized_offset: usize, +} + +/// Normalize base64 text by stripping XML whitespace and rejecting other ASCII whitespace. +pub(crate) fn normalize_xml_base64_text( + text: &str, + normalized: &mut String, +) -> Result<(), XmlBase64NormalizeError> { + for ch in text.chars() { + if matches!(ch, ' ' | '\t' | '\r' | '\n') { + continue; + } + if ch.is_ascii_whitespace() { + let mut utf8 = [0_u8; 4]; + let encoded = ch.encode_utf8(&mut utf8); + let invalid_byte = encoded.as_bytes()[0]; + return Err(XmlBase64NormalizeError { + invalid_byte, + normalized_offset: normalized.len(), + }); + } + normalized.push(ch); + } + Ok(()) +}