From 469a5cd0fd72e0241979bf5ddb27a7d25c6abfa2 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 23:26:24 +0300 Subject: [PATCH 1/3] test(xmldsig): cover RSAKeyValue resolution Closes #67 --- src/xmldsig/keys.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 1766ea4..9d9d16d 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -274,6 +274,9 @@ mod tests { 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"); + const RSA_KEY_VALUE_SIGNATURE: &str = include_str!( + "../../tests/fixtures/xmldsig/merlin-xmldsig-twenty-three/signature-enveloping-rsa.xml" + ); fn replace_key_info(xml: &str, replacement: &str) -> String { let start = xml.find("").expect("fixture has KeyInfo"); @@ -376,6 +379,18 @@ mod tests { assert_eq!(result.status, super::super::DsigStatus::Valid); } + #[test] + fn resolves_rsa_key_value_end_to_end() { + // Embedded CryptoBinary parameters must become the key used by verification. + let resolver = DefaultKeyResolver::default(); + let result = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(RSA_KEY_VALUE_SIGNATURE) + .expect("RSAKeyValue 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. From a2d958a68e806055002d656ab0218ad62aa3e101 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 28 Jun 2026 23:35:09 +0300 Subject: [PATCH 2/3] feat(xmldsig): resolve RSAKeyValue keys - parse bounded Modulus and Exponent CryptoBinary values - construct validated RSA SPKI keys for verification - cover malformed, weak, and incompatible key parameters Closes #67 --- README.md | 1 + src/xmldsig/keys.rs | 104 +++++++++++- src/xmldsig/parse.rs | 220 ++++++++++++++++++++++++- tests/donor_full_verification_suite.rs | 26 ++- 4 files changed, 339 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 97631e7..ee92fdb 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,7 @@ Currently implemented (core paths): - C14N 1.0, C14N 1.1, and Exclusive C14N - XMLDSig parsing, same-document URI dereference, transform chains, and digest verification - XMLDSig full verify pipeline (`SignedInfo` canonicalization + `SignatureValue` verification) +- Built-in verification-key resolution from X.509, DER, `KeyName`, and RSA `KeyValue` sources - 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 - Opt-in X.509 certificate-chain validation with explicit trust anchors, validity checks, CA constraints, and CRLs diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 9d9d16d..2faf45f 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -2,6 +2,7 @@ use std::{collections::HashMap, time::SystemTime}; +use rsa::pkcs8::EncodePublicKey; use x509_parser::{ prelude::{FromDer, X509Certificate}, public_key::PublicKey, @@ -9,7 +10,7 @@ use x509_parser::{ }; use super::{ - DsigError, KeyInfo, KeyInfoSource, KeyResolver, SignatureAlgorithm, VerifyingKey, + DsigError, KeyInfo, KeyInfoSource, KeyResolver, KeyValueInfo, SignatureAlgorithm, VerifyingKey, X509ChainOptions, X509DataInfo, verify_ecdsa_signature_spki, verify_rsa_signature_spki, verify_x509_certificate_chain, }; @@ -171,6 +172,43 @@ impl DefaultKeyResolver { name: None, })) } + + fn resolve_key_value( + key_value: &KeyValueInfo, + algorithm: SignatureAlgorithm, + ) -> Result, KeyResolutionError> { + let KeyValueInfo::Rsa { modulus, exponent } = key_value else { + return Ok(None); + }; + if !matches!( + algorithm, + SignatureAlgorithm::RsaSha1 + | SignatureAlgorithm::RsaSha256 + | SignatureAlgorithm::RsaSha384 + | SignatureAlgorithm::RsaSha512 + ) { + return Err(KeyResolutionError::AlgorithmMismatch); + } + + let key = rsa::RsaPublicKey::new( + rsa::BigUint::from_bytes_be(modulus), + rsa::BigUint::from_bytes_be(exponent), + ) + .map_err(|_| KeyResolutionError::InvalidPublicKey)?; + let public_key_bytes = key + .to_public_key_der() + .map_err(|_| KeyResolutionError::InvalidPublicKey)? + .as_bytes() + .to_vec(); + validate_spki_algorithm(&public_key_bytes, algorithm)?; + + Ok(Some(VerificationKey { + algorithm, + public_key_bytes, + certificate_der: None, + name: None, + })) + } } impl KeyResolver for DefaultKeyResolver { @@ -206,7 +244,9 @@ impl KeyResolver for DefaultKeyResolver { Ok(key.clone()) }) .transpose()?, - KeyInfoSource::KeyValue(_) => None, + KeyInfoSource::KeyValue(key_value) => { + Self::resolve_key_value(key_value, algorithm)? + } }; if let Some(key) = resolved { return Ok(Some(Box::new(key))); @@ -266,6 +306,7 @@ fn validate_spki_algorithm( #[cfg(test)] mod tests { use base64::{Engine, engine::general_purpose::STANDARD}; + use rsa::{pkcs8::DecodePublicKey, traits::PublicKeyParts}; use super::*; @@ -275,6 +316,9 @@ mod tests { 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"); const RSA_KEY_VALUE_SIGNATURE: &str = include_str!( + "../../tests/fixtures/xmldsig/aleksey-xmldsig-01/enveloping-sha256-rsa-sha256.xml" + ); + const LEGACY_RSA_KEY_VALUE_SIGNATURE: &str = include_str!( "../../tests/fixtures/xmldsig/merlin-xmldsig-twenty-three/signature-enveloping-rsa.xml" ); @@ -287,6 +331,12 @@ mod tests { format!("{}{}{}", &xml[..start], replacement, &xml[end..]) } + fn replace_unprefixed_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(pem_text: &str) -> Vec { let (rest, pem) = x509_parser::pem::parse_x509_pem(pem_text.as_bytes()) .expect("fixture public key is PEM"); @@ -381,16 +431,62 @@ mod tests { #[test] fn resolves_rsa_key_value_end_to_end() { - // Embedded CryptoBinary parameters must become the key used by verification. + // Embedded CryptoBinary parameters must verify the original RSA-2048 donor signature. + let public_key = rsa::RsaPublicKey::from_public_key_pem(RSA_PUBLIC_KEY) + .expect("fixture must contain an RSA public key"); + let key_info = format!( + "{}{}", + STANDARD.encode(public_key.n().to_bytes_be()), + STANDARD.encode(public_key.e().to_bytes_be()), + ); + let xml = replace_unprefixed_key_info(RSA_KEY_VALUE_SIGNATURE, &key_info); let resolver = DefaultKeyResolver::default(); let result = super::super::VerifyContext::new() .key_resolver(&resolver) - .verify(RSA_KEY_VALUE_SIGNATURE) + .verify(&xml) .expect("RSAKeyValue should resolve"); assert_eq!(result.status, super::super::DsigStatus::Valid); } + #[test] + fn rsa_key_value_rejects_legacy_weak_modulus() { + // Embedded keys must obey the same 2048-bit minimum as certificate and DER keys. + let resolver = DefaultKeyResolver::default(); + let error = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(LEGACY_RSA_KEY_VALUE_SIGNATURE) + .expect_err("1024-bit RSAKeyValue must fail closed"); + + assert!(matches!( + error, + DsigError::Crypto(super::super::SignatureVerificationError::InvalidKeyDer) + )); + } + + #[test] + fn rsa_key_value_rejects_ecdsa_signature_method() { + // Embedded RSA parameters must not be relabeled for an ECDSA SignatureMethod. + let public_key = rsa::RsaPublicKey::from_public_key_pem(RSA_PUBLIC_KEY) + .expect("fixture must contain an RSA public key"); + let key_info = format!( + "{}{}", + STANDARD.encode(public_key.n().to_bytes_be()), + STANDARD.encode(public_key.e().to_bytes_be()), + ); + let xml = replace_key_info(SIGNED_SAML, &key_info); + let resolver = DefaultKeyResolver::default(); + let error = super::super::VerifyContext::new() + .key_resolver(&resolver) + .verify(&xml) + .expect_err("RSAKeyValue must not resolve for ECDSA"); + + assert!(matches!( + error, + DsigError::KeyResolution(KeyResolutionError::AlgorithmMismatch) + )); + } + #[test] fn chain_verification_rejects_untrusted_embedded_certificate() { // Enabling chain policy must fail closed when no trust anchor is configured. diff --git a/src/xmldsig/parse.rs b/src/xmldsig/parse.rs index 82f21f4..e60707d 100644 --- a/src/xmldsig/parse.rs +++ b/src/xmldsig/parse.rs @@ -34,6 +34,8 @@ 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; +const MAX_RSA_MODULUS_LEN: usize = 1024; +const MAX_RSA_EXPONENT_LEN: usize = 8; const MAX_X509_BASE64_TEXT_LEN: usize = 262_144; const MAX_X509_BASE64_NORMALIZED_LEN: usize = MAX_X509_BASE64_TEXT_LEN; const MAX_X509_DECODED_BINARY_LEN: usize = MAX_X509_BASE64_NORMALIZED_LEN.div_ceil(4) * 3; @@ -155,8 +157,13 @@ pub enum KeyInfoSource { #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub enum KeyValueInfo { - /// ``. - RsaKeyValue, + /// `` with unsigned big-endian CryptoBinary parameters. + Rsa { + /// RSA modulus. + modulus: Vec, + /// RSA public exponent. + exponent: Vec, + }, /// `dsig11:ECKeyValue` (the XMLDSig 1.1 namespace form). EcKeyValue, /// Any other `` child not yet supported by this phase. @@ -537,7 +544,7 @@ fn parse_key_value_dispatch(node: Node) -> Result { first_child.tag_name().namespace(), first_child.tag_name().name(), ) { - (Some(XMLDSIG_NS), "RSAKeyValue") => Ok(KeyValueInfo::RsaKeyValue), + (Some(XMLDSIG_NS), "RSAKeyValue") => parse_rsa_key_value(first_child), (Some(XMLDSIG11_NS), "ECKeyValue") => Ok(KeyValueInfo::EcKeyValue), (namespace, child_name) => Ok(KeyValueInfo::Unsupported { namespace: namespace.map(str::to_string), @@ -546,6 +553,74 @@ fn parse_key_value_dispatch(node: Node) -> Result { } } +fn parse_rsa_key_value(node: Node<'_, '_>) -> Result { + verify_ds_element(node, "RSAKeyValue")?; + ensure_no_non_whitespace_text(node, "RSAKeyValue")?; + + let mut children = element_children(node); + let modulus_node = children.next().ok_or_else(|| { + ParseError::InvalidStructure("RSAKeyValue requires Modulus and Exponent".into()) + })?; + verify_ds_element(modulus_node, "Modulus")?; + ensure_no_element_children(modulus_node, "Modulus")?; + + let exponent_node = children.next().ok_or_else(|| { + ParseError::InvalidStructure("RSAKeyValue requires Modulus and Exponent".into()) + })?; + verify_ds_element(exponent_node, "Exponent")?; + ensure_no_element_children(exponent_node, "Exponent")?; + if children.next().is_some() { + return Err(ParseError::InvalidStructure( + "RSAKeyValue must contain exactly Modulus followed by Exponent".into(), + )); + } + + Ok(KeyValueInfo::Rsa { + modulus: decode_crypto_binary(modulus_node, "Modulus", MAX_RSA_MODULUS_LEN)?, + exponent: decode_crypto_binary(exponent_node, "Exponent", MAX_RSA_EXPONENT_LEN)?, + }) +} + +fn decode_crypto_binary( + node: Node<'_, '_>, + element_name: &'static str, + max_decoded_len: usize, +) -> Result, ParseError> { + use base64::Engine; + use base64::engine::general_purpose::STANDARD; + + let max_base64_len = max_decoded_len.div_ceil(3) * 4; + let mut cleaned = String::with_capacity(max_base64_len); + for text in node.children().filter_map(|child| child.text()) { + normalize_xml_base64_text(text, &mut cleaned).map_err(|err| { + ParseError::Base64(format!( + "invalid XML whitespace U+{:04X} in {element_name}", + err.invalid_byte + )) + })?; + if cleaned.len() > max_base64_len { + return Err(ParseError::InvalidStructure(format!( + "{element_name} exceeds maximum allowed base64 length" + ))); + } + } + + let value = STANDARD + .decode(&cleaned) + .map_err(|err| ParseError::Base64(format!("{element_name}: {err}")))?; + if value.is_empty() { + return Err(ParseError::InvalidStructure(format!( + "{element_name} must not be empty" + ))); + } + if value.len() > max_decoded_len { + return Err(ParseError::InvalidStructure(format!( + "{element_name} exceeds maximum allowed binary length" + ))); + } + Ok(value) +} + fn parse_x509_data_dispatch(node: Node) -> Result { verify_ds_element(node, "X509Data")?; ensure_no_non_whitespace_text(node, "X509Data")?; @@ -1346,7 +1421,10 @@ mod tests { ); assert_eq!( key_info.sources[1], - KeyInfoSource::KeyValue(KeyValueInfo::RsaKeyValue) + KeyInfoSource::KeyValue(KeyValueInfo::Rsa { + modulus: vec![1, 0, 1], + exponent: vec![1, 0, 1], + }) ); let x509_info = match &key_info.sources[2] { KeyInfoSource::X509Data(x509) => x509, @@ -1406,6 +1484,140 @@ mod tests { ); } + #[test] + fn parse_rsa_key_value_preserves_wrapped_crypto_binary() { + // CryptoBinary is unsigned big-endian data and XML whitespace is insignificant. + let xml = r#" + + AQID +BA== + AQAB + + "#; + let doc = Document::parse(xml).unwrap(); + + assert_eq!( + parse_key_info(doc.root_element()).unwrap().sources, + vec![KeyInfoSource::KeyValue(KeyValueInfo::Rsa { + modulus: vec![1, 2, 3, 4], + exponent: vec![1, 0, 1], + })] + ); + } + + #[test] + fn parse_rsa_key_value_rejects_reordered_parameters() { + // XMLDSig defines Modulus followed by Exponent; accepting reordered input is ambiguous. + let xml = r#" + + AQABAQID + + "#; + let doc = Document::parse(xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::InvalidStructure(_)) + )); + } + + #[test] + fn parse_rsa_key_value_rejects_missing_exponent() { + // Both RSA public parameters are required to construct a usable key. + let xml = r#" + AQID + "#; + let doc = Document::parse(xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::InvalidStructure(_)) + )); + } + + #[test] + fn parse_rsa_key_value_rejects_duplicate_exponent() { + // RSAKeyValue has a closed two-child schema; duplicate parameters are invalid. + let xml = r#" + + AQIDAQABAQAB + + "#; + let doc = Document::parse(xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::InvalidStructure(_)) + )); + } + + #[test] + fn parse_rsa_key_value_rejects_wrong_parameter_namespace() { + // Local names from an extension namespace must not be treated as XMLDSig parameters. + let xml = r#" + + AQIDAQAB + + "#; + let doc = Document::parse(xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::InvalidStructure(_)) + )); + } + + #[test] + fn parse_rsa_key_value_rejects_nested_crypto_binary() { + // CryptoBinary values are text-only and must not hide extension elements. + let xml = r#" + + AQIDAQAB + + "#; + let doc = Document::parse(xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::InvalidStructure(_)) + )); + } + + #[test] + fn parse_rsa_key_value_rejects_malformed_base64() { + // Malformed key parameters must be processing errors, not unresolved keys. + let xml = r#" + + %%%%AQAB + + "#; + let doc = Document::parse(xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::Base64(_)) + )); + } + + #[test] + fn parse_rsa_key_value_rejects_oversized_exponent_before_decode() { + // Bound normalized text before allocation or integer construction. + let exponent = "A".repeat(MAX_RSA_EXPONENT_LEN.div_ceil(3) * 4 + 1); + let xml = format!( + r#" + + AQID{exponent} + + "# + ); + let doc = Document::parse(&xml).unwrap(); + + assert!(matches!( + parse_key_info(doc.root_element()), + Err(ParseError::InvalidStructure(_)) + )); + } + #[test] fn parse_key_info_ignores_unknown_children() { let xml = r#" diff --git a/tests/donor_full_verification_suite.rs b/tests/donor_full_verification_suite.rs index a16bc8e..5f84857 100644 --- a/tests/donor_full_verification_suite.rs +++ b/tests/donor_full_verification_suite.rs @@ -6,12 +6,14 @@ use std::path::{Path, PathBuf}; use xml_sec::xmldsig::{ - DsigError, DsigStatus, FailureReason, ParseError, VerifyContext, verify_signature_with_pem_key, + DefaultKeyResolver, DsigError, DsigStatus, FailureReason, ParseError, VerifyContext, + verify_signature_with_pem_key, }; #[derive(Clone, Copy)] enum SkipProbe { KeyNotFound, + WeakRsaKey, UnsupportedSignatureAlgorithm, } @@ -108,8 +110,8 @@ fn cases() -> Vec { name: "merlin-enveloping-rsa-keyvalue", xml_path: "tests/fixtures/xmldsig/merlin-xmldsig-twenty-three/signature-enveloping-rsa.xml", expectation: Expectation::Skip { - reason: "KeyValue auto-resolution is not implemented yet (planned P2-009)", - probe: SkipProbe::KeyNotFound, + reason: "RSAKeyValue resolves but its legacy 1024-bit modulus is below policy", + probe: SkipProbe::WeakRsaKey, }, }, VectorCase { @@ -202,6 +204,22 @@ fn donor_full_verification_suite_tracks_pass_fail_skip_counts() { case.name )), }, + SkipProbe::WeakRsaKey => match VerifyContext::new() + .key_resolver(&DefaultKeyResolver::default()) + .verify(&xml) + { + Err(DsigError::Crypto( + xml_sec::xmldsig::SignatureVerificationError::InvalidKeyDer, + )) => {} + Ok(result) => failed.push(format!( + "{}: expected weak RSA key error for skipped vector, got {:?}", + case.name, result.status + )), + Err(err) => failed.push(format!( + "{}: expected weak RSA key error for skipped vector, got {err}", + case.name + )), + }, SkipProbe::UnsupportedSignatureAlgorithm => match VerifyContext::new().verify(&xml) { Err(DsigError::ParseSignedInfo(ParseError::UnsupportedAlgorithm { @@ -232,7 +250,7 @@ fn donor_full_verification_suite_tracks_pass_fail_skip_counts() { let expected_skipped = vec![ "aleksey-rsa-sha512-x509-digest: X509Digest key resolution is not implemented yet (planned P2-009)", "merlin-enveloped-dsa: DSA signature method is not implemented yet (planned P4-009)", - "merlin-enveloping-rsa-keyvalue: KeyValue auto-resolution is not implemented yet (planned P2-009)", + "merlin-enveloping-rsa-keyvalue: RSAKeyValue resolves but its legacy 1024-bit modulus is below policy", "merlin-x509-crt: DSA signature method is not implemented yet (planned P4-009); X509 KeyInfo resolution is not implemented yet (planned P2-009)", "merlin-x509-crt-crl: DSA signature method is not implemented yet (planned P4-009); X509/CRL KeyInfo resolution is not implemented yet (planned P2-009/P2-005)", "merlin-x509-is: DSA signature method is not implemented yet (planned P4-009); X509IssuerSerial resolution is not implemented yet (planned P2-009)", From 2f891bccd98fc3b00ba9a28479fe9e07bc200eb7 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 29 Jun 2026 22:52:01 +0300 Subject: [PATCH 3/3] fix(xmldsig): bound crypto binary normalization --- src/xmldsig/parse.rs | 29 +++++++++------ src/xmldsig/whitespace.rs | 78 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 16 deletions(-) diff --git a/src/xmldsig/parse.rs b/src/xmldsig/parse.rs index e60707d..f433d9c 100644 --- a/src/xmldsig/parse.rs +++ b/src/xmldsig/parse.rs @@ -23,7 +23,10 @@ use x509_parser::public_key::PublicKey; use super::digest::DigestAlgorithm; use super::transforms::{self, Transform}; -use super::whitespace::{is_xml_whitespace_only, normalize_xml_base64_text}; +use super::whitespace::{ + XmlBase64NormalizeLimitedError, is_xml_whitespace_only, normalize_xml_base64_text, + normalize_xml_base64_text_with_limit, +}; use crate::c14n::C14nAlgorithm; /// XMLDSig namespace URI. @@ -592,17 +595,19 @@ fn decode_crypto_binary( let max_base64_len = max_decoded_len.div_ceil(3) * 4; let mut cleaned = String::with_capacity(max_base64_len); for text in node.children().filter_map(|child| child.text()) { - normalize_xml_base64_text(text, &mut cleaned).map_err(|err| { - ParseError::Base64(format!( - "invalid XML whitespace U+{:04X} in {element_name}", - err.invalid_byte - )) - })?; - if cleaned.len() > max_base64_len { - return Err(ParseError::InvalidStructure(format!( - "{element_name} exceeds maximum allowed base64 length" - ))); - } + normalize_xml_base64_text_with_limit(text, &mut cleaned, max_base64_len).map_err( + |err| match err { + XmlBase64NormalizeLimitedError::InvalidWhitespace(err) => { + ParseError::Base64(format!( + "invalid XML whitespace U+{:04X} in {element_name}", + err.invalid_byte + )) + } + XmlBase64NormalizeLimitedError::TooLong(_) => ParseError::InvalidStructure( + format!("{element_name} exceeds maximum allowed base64 length"), + ), + }, + )?; } let value = STANDARD diff --git a/src/xmldsig/whitespace.rs b/src/xmldsig/whitespace.rs index 2e634a0..36f8424 100644 --- a/src/xmldsig/whitespace.rs +++ b/src/xmldsig/whitespace.rs @@ -16,11 +16,30 @@ pub(crate) struct XmlBase64NormalizeError { pub normalized_offset: usize, } +/// Error returned when normalized base64 text exceeds its caller-supplied bound. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct XmlBase64LengthError { + /// Maximum allowed normalized byte length. + pub max_len: 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> { + normalize_xml_base64_text_with_limit(text, normalized, usize::MAX).map_err(|err| match err { + XmlBase64NormalizeLimitedError::InvalidWhitespace(err) => err, + XmlBase64NormalizeLimitedError::TooLong(_) => unreachable!("unbounded normalization"), + }) +} + +/// Normalize base64 text while enforcing a maximum normalized byte length. +pub(crate) fn normalize_xml_base64_text_with_limit( + text: &str, + normalized: &mut String, + max_len: usize, +) -> Result<(), XmlBase64NormalizeLimitedError> { for ch in text.chars() { if matches!(ch, ' ' | '\t' | '\r' | '\n') { continue; @@ -29,12 +48,63 @@ pub(crate) fn normalize_xml_base64_text( 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(), - }); + return Err(XmlBase64NormalizeLimitedError::InvalidWhitespace( + XmlBase64NormalizeError { + invalid_byte, + normalized_offset: normalized.len(), + }, + )); + } + if normalized.len() + ch.len_utf8() > max_len { + return Err(XmlBase64NormalizeLimitedError::TooLong( + XmlBase64LengthError { max_len }, + )); } normalized.push(ch); } Ok(()) } + +/// Error returned by bounded XML base64 normalization. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum XmlBase64NormalizeLimitedError { + /// Non-XML ASCII whitespace was found. + InvalidWhitespace(XmlBase64NormalizeError), + /// Normalized output would exceed the supplied bound. + TooLong(XmlBase64LengthError), +} + +impl From for XmlBase64NormalizeLimitedError { + fn from(err: XmlBase64NormalizeError) -> Self { + Self::InvalidWhitespace(err) + } +} + +#[cfg(test)] +mod tests { + use super::{ + XmlBase64NormalizeLimitedError, normalize_xml_base64_text, + normalize_xml_base64_text_with_limit, + }; + + #[test] + fn bounded_base64_normalization_rejects_before_growth() { + let mut normalized = String::from("ABCD"); + + let err = normalize_xml_base64_text_with_limit(" E", &mut normalized, 4) + .expect_err("bounded normalization must reject before appending past the cap"); + + assert!(matches!(err, XmlBase64NormalizeLimitedError::TooLong(_))); + assert_eq!(normalized, "ABCD"); + } + + #[test] + fn unbounded_base64_normalization_preserves_existing_behavior() { + let mut normalized = String::new(); + + normalize_xml_base64_text(" A\tB\r\nC ", &mut normalized) + .expect("XML whitespace must be stripped from base64 text"); + + assert_eq!(normalized, "ABC"); + } +}