From eb2c686d08fe20f751dcdbcb0fcfaba782726b2e Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 10:32:11 +0200 Subject: [PATCH 1/3] perf(xmldsig): execute each signed Reference's transforms once Signing ran every Reference's transform chain twice: dependency analysis executed it to learn which mutable values reach its output and discarded the result, then the digest pass executed it again. - The analysis terminal digests the output when the settled dependencies are empty and no step may read character data outside its input; the digest pass takes that value instead of executing the chain again - The speculative analysis made before committing generated canonicalization keeps its plan after that commit, since the commit only changes nodes it tracks; a KeyInfo commit, which may add an ID, still triggers a full analysis - The aggregate external resource test now admits one dereference at an allowance equal to the resource size and refuses one byte less Closes #367 --- src/xmldsig/sign.rs | 168 +++++++++++++----- src/xmldsig/sign/single_execution_tests.rs | 191 +++++++++++++++++++++ src/xmldsig/transforms.rs | 98 ++++++++--- tests/donor_interop_suite.rs | 8 + 4 files changed, 401 insertions(+), 64 deletions(-) create mode 100644 src/xmldsig/sign/single_execution_tests.rs diff --git a/src/xmldsig/sign.rs b/src/xmldsig/sign.rs index 8bff32f6..5cbe7027 100644 --- a/src/xmldsig/sign.rs +++ b/src/xmldsig/sign.rs @@ -2248,23 +2248,32 @@ impl<'a> SignContext<'a> { // provides, so a failed analysis is decided after the commit. let analysis = (!doctype) .then(|| analyze_signing_references(document, operation, deferred, transform_options)) - .and_then(Result::ok) - .filter(|analyzed| digests_can_defer(document, analyzed) && analyzed.1.splice_unread); + .and_then(Result::ok); + // This analysis succeeds only on the current tree, whose transforms keep + // or drop whole subtrees. Generated canonicalization changes only the + // tracked Reference it joins and adds no ID, so the dependencies and the + // digests this analysis proved final hold after that commit and no chain + // is executed again. The writer's KeyInfo can bring an ID that changes + // what a URI resolves to, so after it everything is analyzed again. + let mut speculative_plan = None; if let Some(analyzed) = analysis { - let budget = operation.budgets_mut().transforms.xml_parse_work(); - let key_info = key_info_content - .as_deref() - .map(|content| { - self.splice_signing_key_info(document, target_signature, content, budget) - }) - .transpose()?; - if key_info.as_ref().is_none_or(|spliced| spliced.inert) { - let splice = DeferredSplice::new(document.as_xml(), key_info, generated); - self.policy - .resources - .validate_xml_document_len(splice.spliced.xml.len())?; - return Ok((analyzed, setup_gate, Some(splice))); + if digests_can_defer(document, &analyzed) && analyzed.1.splice_unread { + let budget = operation.budgets_mut().transforms.xml_parse_work(); + let key_info = key_info_content + .as_deref() + .map(|content| { + self.splice_signing_key_info(document, target_signature, content, budget) + }) + .transpose()?; + if key_info.as_ref().is_none_or(|spliced| spliced.inert) { + let splice = DeferredSplice::new(document.as_xml(), key_info, generated); + self.policy + .resources + .validate_xml_document_len(splice.spliced.xml.len())?; + return Ok((analyzed, setup_gate, Some(splice))); + } } + speculative_plan = key_info_content.is_none().then_some(analyzed.1); } if !generated.is_empty() { setup_gate = self.commit_generated_c14n(document, operation, setup_gate, generated)?; @@ -2278,6 +2287,18 @@ impl<'a> SignContext<'a> { &content, )?; } + if let Some(plan) = speculative_plan { + let policy = operation.policy().clone(); + let references = collect_signing_reference_inputs( + document, + &policy, + operation.budgets_mut(), + request, + )?; + if references.signed_info.len() + references.manifests.len() == plan.digests.len() { + return Ok(((references, plan), setup_gate, None)); + } + } let analyzed = analyze_signing_references(document, operation, request, transform_options)?; Ok((analyzed, setup_gate, None)) } @@ -2527,6 +2548,7 @@ impl<'a> SignContext<'a> { external_resources: &external_resources, generated: &[], speculative: false, + provider: self.provider, }; let (analyzed, setup_gate, splice) = self.plan_signing_splice( document, @@ -2539,7 +2561,6 @@ impl<'a> SignContext<'a> { let (plan_nodes, pending_digests) = fill_analyzed_reference_digest_values( document, transform_options, - self.provider, &mut operation, request, Some(setup_gate), @@ -2917,6 +2938,7 @@ struct SigningReferenceRequest<'a, 'resources> { generated: &'a [NodeId], /// The analysis only asks whether a splice can wait for the final mutation. speculative: bool, + provider: &'a dyn crate::provider::CryptoProvider, } /// The Second Edition transform one `Reference` gains. @@ -3140,7 +3162,10 @@ fn compute_prepared_reference_digest_values_with_options( compute_signing_reference_digests( &doc, signature, - references, + references + .into_iter() + .map(|reference| (reference, None)) + .collect(), transform_options, provider, execution_budget, @@ -3326,7 +3351,10 @@ fn analyze_signing_reference_inputs( request.generated, transform_options, &budgets.transforms, - request.uri_resolution(policy.transforms.same_document_id_semantics), + ( + request.uri_resolution(policy.transforms.same_document_id_semantics), + request.provider, + ), ) .map(Some) })?; @@ -3424,7 +3452,10 @@ fn analyze_signing_placeholder_generation( &[], transform_options, &budgets.transforms, - request.uri_resolution(policy.transforms.same_document_id_semantics), + ( + request.uri_resolution(policy.transforms.same_document_id_semantics), + request.provider, + ), ) .map(|plan| SigningDependencyPlan { splice_unread: false, @@ -3440,12 +3471,24 @@ struct LevelDigestRequest<'a, 'resources> { uri_resolution: SigningUriResolution<'a, 'resources>, } +/// The digest dependency analysis proved final for the `Reference` at `index`. +fn analyzed_digest( + analyzed: &[Option], + index: usize, +) -> Result, SigningDigestError> { + analyzed.get(index).cloned().ok_or_else(|| { + SigningDigestError::InvalidStructure( + "compiled signing Reference index is unavailable".into(), + ) + }) +} + /// Reparse the References of a committed intermediate generation and compute /// the digests of one dependency level against it. fn reparsed_level_digests( view: crate::document::DocumentView<'_>, budgets: &mut SigningOperationBudgets, - level: &[usize], + (level, analyzed): (&[usize], &[Option]), references: &SigningReferenceInputs, request: LevelDigestRequest<'_, '_>, ) -> Result, SigningDigestError> { @@ -3490,7 +3533,7 @@ fn reparsed_level_digests( ) })?; destinations.push(view.node_identity_by_id(reference.digest_value_node_id)); - level_references.push(reference.clone()); + level_references.push((reference.clone(), analyzed_digest(analyzed, *index)?)); } let computed = compute_signing_reference_digests( current_doc, @@ -3519,7 +3562,7 @@ fn compute_deferred_reference_digests( operation: &mut SigningExecution<'_>, node_level: &[OperationNodeId], document: &XmlDocument, - level: &[usize], + (level, analyzed): (&[usize], &[Option]), references: &SigningReferenceInputs, request: LevelDigestRequest<'_, '_>, ) -> Result, SigningDigestError> { @@ -3544,11 +3587,12 @@ fn compute_deferred_reference_digests( "compiled signing Reference index is unavailable".into(), ) }) + .and_then(|reference| Ok((reference, analyzed_digest(analyzed, *index)?))) }) .collect::, _>>()?; let destinations = level_references .iter() - .map(|reference| view.node_identity_by_id(reference.digest_value_node_id)) + .map(|(reference, _)| view.node_identity_by_id(reference.digest_value_node_id)) .collect::>(); let computed = compute_signing_reference_digests( current_doc, @@ -3577,7 +3621,6 @@ fn compute_deferred_reference_digests( fn fill_reference_digest_values_in_dependency_order_with_operation( document: &mut XmlDocument, transform_options: TransformOptions<'_>, - provider: &dyn crate::provider::CryptoProvider, operation: &mut OperationExecutionContext< crate::policy::SigningPolicy, &mut SigningOperationBudgets, @@ -3590,7 +3633,6 @@ fn fill_reference_digest_values_in_dependency_order_with_operation( fill_analyzed_reference_digest_values( document, transform_options, - provider, operation, request, setup_gate, @@ -3646,14 +3688,9 @@ fn digests_can_defer( }) } -#[expect( - clippy::too_many_arguments, - reason = "each argument is one part of the signing operation already in progress" -)] fn fill_analyzed_reference_digest_values( document: &mut XmlDocument, transform_options: TransformOptions<'_>, - provider: &dyn crate::provider::CryptoProvider, operation: &mut OperationExecutionContext< crate::policy::SigningPolicy, &mut SigningOperationBudgets, @@ -3670,7 +3707,7 @@ fn fill_analyzed_reference_digest_values( compile_signing_operation_plan(operation, &dependency_plan, setup_gate)?; let level_request = LevelDigestRequest { transform_options, - provider, + provider: request.provider, target_signature: request.target_signature, uri_resolution: request.uri_resolution(policy.transforms.same_document_id_semantics), }; @@ -3682,19 +3719,26 @@ fn fill_analyzed_reference_digest_values( operation, node_level, document, - level, + (level, &dependency_plan.digests), &references, level_request, )?; return Ok((plan_nodes, pending)); } + let analyzed = dependency_plan.digests; for (level, node_level) in dependency_plan.levels.into_iter().zip(node_levels) { operation.run_document_transition_batch_with_budgets( &node_level, document, |document, budgets| { let replacements = document.with_view(|view| { - reparsed_level_digests(view, budgets, &level, &references, level_request) + reparsed_level_digests( + view, + budgets, + (&level, &analyzed), + &references, + level_request, + ) })?; document .replace_base64_contents_with_budget( @@ -3730,7 +3774,6 @@ fn fill_reference_digest_values_in_dependency_order( fill_reference_digest_values_in_dependency_order_with_operation( document, transform_options, - provider, &mut operation, SigningReferenceRequest { target_signature, @@ -3738,6 +3781,7 @@ fn fill_reference_digest_values_in_dependency_order( external_resources: &external_resources, generated: &[], speculative: false, + provider, }, None, false, @@ -3751,6 +3795,9 @@ struct SigningDependencyPlan { /// No `Reference` can observe the target `KeyInfo` or a `Reference` that /// gains a generated transform. splice_unread: bool, + /// Per `Reference`, the digest its analysis proved final: its output reads + /// no value signing fills, so the digest pass would compute the same bytes. + digests: Vec>, } /// Every node whose content signing changes or adds, with the index of what @@ -3809,7 +3856,10 @@ fn reference_dependency_levels( generated: &[NodeId], transform_options: TransformOptions<'_>, execution_budget: &TransformExecutionBudget, - uri_resolution: SigningUriResolution<'_, '_>, + (uri_resolution, provider): ( + SigningUriResolution<'_, '_>, + &dyn crate::provider::CryptoProvider, + ), ) -> Result { let resolver = uri_resolution.external_resources.bind( doc, @@ -3819,6 +3869,7 @@ fn reference_dependency_levels( let terminal_signature_value_index = references.len(); let key_info_index = terminal_signature_value_index + 1; let tracked_mutable_nodes = tracked_signing_nodes(doc, signature, references, generated)?; + let mut digests = Vec::with_capacity(references.len()); let mut analyses = references .iter() .map(|reference| { @@ -3836,7 +3887,18 @@ fn reference_dependency_levels( transform_options, execution_budget, tracked_mutable_nodes.clone(), - )?; + Some(super::transforms::DigestRequest { + provider, + algorithm: reference.digest_method, + retain_pre_digest: false, + }), + ) + .map_err(map_reference_digest_error)?; + digests.push( + output + .digest + .map(|digest| base64::engine::general_purpose::STANDARD.encode(digest)), + ); Ok(output.dependencies) }) .collect::, SigningDigestError>>()?; @@ -3889,9 +3951,19 @@ fn reference_dependency_levels( dependencies: analyses, levels, splice_unread, + digests, }) } +fn map_reference_digest_error( + error: super::transforms::ReferenceDigestError, +) -> SigningDigestError { + match error { + super::transforms::ReferenceDigestError::Transform(error) => error.into(), + super::transforms::ReferenceDigestError::Provider(error) => error.into(), + } +} + fn signing_reference_origin<'doc, 'input>( doc: &'doc Document<'input>, reference: &SigningReference, @@ -4131,10 +4203,12 @@ struct SigningUriResolution<'a, 'resources> { external_resources: &'a ExternalResourceContext<'resources>, } +/// Digest each `Reference`, taking the value its dependency analysis already +/// proved final where there is one. fn compute_signing_reference_digests( doc: &Document<'_>, signature: Node<'_, '_>, - references: Vec, + references: Vec<(SigningReference, Option)>, transform_options: TransformOptions<'_>, provider: &dyn crate::provider::CryptoProvider, execution_budget: &TransformExecutionBudget, @@ -4148,7 +4222,15 @@ fn compute_signing_reference_digests( references .into_iter() .enumerate() - .map(|(index, reference)| { + .map(|(index, (reference, analyzed))| { + if let Some(digest_value) = analyzed { + return Ok(ComputedReferenceDigest { + index, + uri: reference.uri, + digest_method: reference.digest_method, + digest_value, + }); + } let origin = signing_reference_origin(doc, &reference)?; let initial_data = resolver.dereference_from_with_budget( &reference.uri, @@ -4170,12 +4252,7 @@ fn compute_signing_reference_digests( retain_pre_digest: false, }, ) - .map_err(|error| -> SigningDigestError { - match error { - super::transforms::ReferenceDigestError::Transform(error) => error.into(), - super::transforms::ReferenceDigestError::Provider(error) => error.into(), - } - })? + .map_err(map_reference_digest_error)? .digest; let digest_value = base64::engine::general_purpose::STANDARD.encode(digest); Ok(ComputedReferenceDigest { @@ -4779,6 +4856,9 @@ mod deferred_commit_tests; #[cfg(test)] mod generated_c14n_tests; +#[cfg(test)] +mod single_execution_tests; + fn xml_qualifier(prefix: &str) -> String { if prefix.is_empty() { String::new() diff --git a/src/xmldsig/sign/single_execution_tests.rs b/src/xmldsig/sign/single_execution_tests.rs new file mode 100644 index 00000000..145180bd --- /dev/null +++ b/src/xmldsig/sign/single_execution_tests.rs @@ -0,0 +1,191 @@ +use super::*; +use crate::xmldsig::transforms::take_transform_chains_started; + +const DS: &str = "http://www.w3.org/2000/09/xmldsig#"; +const EXCLUSIVE: &str = "http://www.w3.org/2001/10/xml-exc-c14n#"; +const RSA_SHA256: &str = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; +const SHA256: &str = "http://www.w3.org/2001/04/xmlenc#sha256"; +const XPATH: &str = "http://www.w3.org/TR/1999/REC-xpath-19991116"; + +struct FixedKey; + +impl SigningKey for FixedKey { + fn sign( + &self, + _algorithm: SignatureAlgorithm, + _canonical_signed_info: &[u8], + ) -> Result, SigningKeyError> { + Ok(vec![0x5a; 256]) + } + + fn public_key_info(&self) -> Result { + Ok(SigningPublicKeyInfo::Rsa { + spki_der: Vec::new(), + modulus: vec![0x80; 256], + exponent: vec![1, 0, 1], + }) + } +} + +fn reference(uri: &str, transforms: &str) -> String { + let transforms = if transforms.is_empty() { + String::new() + } else { + format!("{transforms}") + }; + format!( + r#"{transforms}"# + ) +} + +fn template(references: &str, object: &str) -> String { + format!( + r#"onetwox{references}{object}"# + ) +} + +fn transform(algorithm: &str, body: &str) -> String { + format!(r#"{body}"#) +} + +/// Signs `xml`, returning the output and how many transform chains it ran. +fn sign_counting(xml: &str, policy: crate::policy::SigningPolicy) -> (String, usize) { + take_transform_chains_started(); + let signed = SignContext::new(&FixedKey) + .policy(policy) + .sign_template(xml) + .expect("template must sign"); + (signed, take_transform_chains_started()) +} + +/// The `SignedInfo` digests recomputed from the final document, where every +/// mutable value already holds what a verifier will read. +fn assert_digests_match_final_document(signed: &str, policy: &crate::policy::SigningPolicy) { + let expected = compute_prepared_reference_digest_values_with_options( + signed, + TransformOptions::default(), + Some(policy), + crate::provider::default_provider(), + &TransformExecutionBudget::default(), + 0, + &[], + ) + .expect("signed output must digest"); + let actual = signed + .split("") + .skip(1) + .map(|rest| { + rest.split("") + .next() + .expect("closed value") + }) + .take(expected.len()) + .collect::>(); + let expected = expected + .iter() + .map(|digest| digest.digest_value.as_str()) + .collect::>(); + assert_eq!(actual, expected); +} + +#[test] +fn an_independent_reference_executes_its_transforms_once() { + // A Reference whose output reads no value the signature fills is + // digested from its dependency-analysis execution: the current-tree + // analysis (subtree transforms) and the placeholder analysis (a + // structural XPath) must not execute the chain again for the digest. + let policy = crate::policy::SigningPolicy::default(); + let cases = [ + ( + "subtree references", + template( + &[ + reference("#payload", &transform(EXCLUSIVE, "")), + reference("#other", ""), + ] + .concat(), + "", + ), + ), + ( + "structural xpath", + template( + &reference( + "#payload", + &transform(XPATH, "not(self::comment())"), + ), + "", + ), + ), + ]; + for (name, xml) in cases { + let references = xml.matches(""#; + let xml = template( + &reference( + "#payload", + &transform("http://www.w3.org/TR/1999/REC-xslt-19991116", stylesheet), + ), + "", + ); + let policy = crate::policy::SigningPolicy::default(); + let (signed, chains) = sign_counting(&xml, policy.clone()); + assert_eq!(chains, 1); + assert_digests_match_final_document(&signed, &policy); +} + +#[test] +fn a_reference_that_reads_another_digest_executes_again() { + // `#outer` covers a Manifest whose Reference covers another Manifest: + // each depends on the DigestValue below it, so only the innermost one is + // final at analysis. Three analyses plus two later digests. + let manifests = format!( + r#"{}{}"#, + reference("#inner", ""), + reference("#payload", ""), + ); + let xml = template(&reference("#outer", ""), &manifests); + let policy = crate::policy::SigningPolicy { + manifest_processing: crate::policy::ManifestProcessing::Process, + ..crate::policy::SigningPolicy::default() + }; + let (signed, chains) = sign_counting(&xml, policy.clone()); + assert_eq!(chains, 5); + assert_digests_match_final_document(&signed, &policy); +} + +#[test] +fn a_reference_with_a_value_reading_xpath_after_octets_executes_again() { + // After canonicalization the chain's dependencies are settled from the + // octets, but a later XPath that reads character data is not proven + // independent of values outside them, so its digest is recomputed. + let xml = template( + &reference( + "#payload", + &[ + transform(EXCLUSIVE, ""), + transform( + XPATH, + "string-length(string(.)) >= 0", + ), + ] + .concat(), + ), + "", + ); + let policy = crate::policy::SigningPolicy::default(); + let (signed, chains) = sign_counting(&xml, policy.clone()); + assert_eq!(chains, 2); + assert_digests_match_final_document(&signed, &policy); +} diff --git a/src/xmldsig/transforms.rs b/src/xmldsig/transforms.rs index 0e8b43ef..8ed537ae 100644 --- a/src/xmldsig/transforms.rs +++ b/src/xmldsig/transforms.rs @@ -593,6 +593,14 @@ impl XPathDocumentIdentity { #[cfg(test)] thread_local! { static XPATH_DOCUMENT_IDENTITY_COMPUTATIONS: Cell = const { Cell::new(0) }; + static TRANSFORM_CHAINS_STARTED: Cell = const { Cell::new(0) }; +} + +/// Transform chains started on this thread since the last call; tests count +/// how often signing executes Reference transforms. +#[cfg(all(test, feature = "xmldsig-sign"))] +pub(crate) fn take_transform_chains_started() -> usize { + TRANSFORM_CHAINS_STARTED.with(|count| count.replace(0)) } #[derive(Default)] @@ -608,6 +616,8 @@ struct CachedXPathDocumentIdentity { impl TransformChainState { fn begin_chain(&self) { + #[cfg(test)] + TRANSFORM_CHAINS_STARTED.with(|count| count.set(count.get() + 1)); // Document pointer identity is valid only while one transform chain // keeps that parsed document alive. Never carry it into a later chain, // where allocator address reuse could alias unrelated XML. @@ -1516,6 +1526,9 @@ pub(crate) fn enveloped_signature_for_reference<'a, E, I: Instruction>( #[cfg(feature = "xmldsig-sign")] pub(crate) struct TransformDependencyOutput { pub(crate) dependencies: Set, + /// The output's digest, present when the execution proved the output + /// independent of every tracked field, so a later execution would repeat it. + pub(crate) digest: Option, } struct TransformChainOutput { @@ -1781,6 +1794,11 @@ impl<'a, S: TransformScratch, D: DomStorage> ExecutionData<'a, S, D> { pub(crate) trait TransformTerminal { type Output; type Error: From; + + /// Told, before finishing, whether the settled dependencies of a tracked + /// chain are empty. + fn settle(&mut self, _independent: bool) {} + fn finish( self, data: ExecutionData<'_, S, D>, @@ -1842,31 +1860,50 @@ impl TransformTerminal for CollectTerminal { } /// Dependency analysis reads which tracked nodes stay visible, which the chain -/// settles before its terminal; the final output itself is never needed. +/// settles before its terminal. An output independent of them is final, so it +/// is digested here instead of by a second execution; any other output is +/// discarded without decoding or charging it, as the digest pass will. #[cfg(feature = "xmldsig-sign")] -struct DiscardTerminal; +struct AnalysisTerminal<'p>(Option>); #[cfg(feature = "xmldsig-sign")] -impl TransformTerminal for DiscardTerminal { - type Output = (); - type Error = TransformError; +impl TransformTerminal for AnalysisTerminal<'_> { + type Output = Option; + type Error = ReferenceDigestError; + + fn settle(&mut self, independent: bool) { + if !independent { + self.0 = None; + } + } + fn finish( self, - _: ExecutionData<'_, Native, D>, - _: Option<(&C14nAlgorithm, Option<&str>)>, - _: &TransformExecutionBudget, - ) -> Result<(), TransformError> { - Ok(()) + data: ExecutionData<'_, Native, D>, + explicit: Option<(&C14nAlgorithm, Option<&str>)>, + budget: &TransformExecutionBudget, + ) -> Result { + self.0 + .map(|request| { + DigestTerminal(request) + .finish(data, explicit, budget) + .map(|digest| digest.digest) + }) + .transpose() } - // The digest pass decodes and charges the payload once; decoding here - // would spend the operation's allowance on output nobody reads. fn finish_base64( self, - _: ExecutionData<'_, Native, D>, - _: &TransformExecutionBudget, - _: &Native, - ) -> Result<(), TransformError> { - Ok(()) + data: ExecutionData<'_, Native, D>, + budget: &TransformExecutionBudget, + scratch: &Native, + ) -> Result { + self.0 + .map(|request| { + DigestTerminal(request) + .finish_base64(data, budget, scratch) + .map(|digest| digest.digest) + }) + .transpose() } } @@ -2385,6 +2422,11 @@ fn canonicalize_tracked( signature_node: Node<'a, 'a>, @@ -2393,7 +2435,8 @@ pub(crate) fn execute_transforms_with_dependency_nodes<'a>( options: TransformOptions<'_>, budget: &TransformExecutionBudget, tracked_nodes: Vec<(usize, NodeId)>, -) -> Result { + digest: Option>, +) -> Result { ensure_transform_count(transforms.len())?; relationship::validate_edition(transforms, options.opc_relationship_edition)?; budget.state.begin_chain(); @@ -2440,10 +2483,13 @@ pub(crate) fn execute_transforms_with_dependency_nodes<'a>( canonical_positions: None, }), &context, - DiscardTerminal, + AnalysisTerminal( + digest.filter(|_| !transforms.iter().any(transform_may_read_mutable_data)), + ), )?; Ok(TransformDependencyOutput { dependencies: output.dependencies, + digest: output.output, }) } @@ -2580,6 +2626,13 @@ fn apply_chain_step<'e, 'd, S: TransformScratch, D: DomStorage, I: Instruction>( Ok(ChainStep::untracked(enveloped_signature, data)) } +/// Whether a chain's settled dependencies are empty; an untracked chain has none. +fn settled_independent(tracking: Option<&DependencyTracking>) -> bool { + tracking.is_none_or(|tracking| { + tracking.active_nodes.is_empty() && tracking.opaque_dependencies.is_empty() + }) +} + fn chain_output( output: T, #[cfg_attr(not(feature = "xmldsig-sign"), expect(unused_variables))] tracking: &mut Option< @@ -2631,7 +2684,7 @@ fn execute_transform_chain< transforms: &[I], mut dependency_tracking: Option, context: &TransformExecutionContext<'_, S>, - finalize: F, + mut finalize: F, ) -> Result, E> where E: From, @@ -2646,6 +2699,7 @@ where if let (Some(nodes), Some(tracking)) = (data.nodes(), &mut dependency_tracking) { retain_visible_dependencies(nodes, tracking)?; } + finalize.settle(settled_independent(dependency_tracking.as_ref())); let output = finalize.finish(data, None, context.budget)?; return Ok(chain_output(output, &mut dependency_tracking)); }; @@ -2701,6 +2755,7 @@ where tracking.dormant_indexes.clear(); } if STREAM_FINAL_C14N && remaining.is_empty() { + finalize.settle(settled_independent(dependency_tracking.as_ref())); let output = finalize.finish(data, Some((algo, prefixes)), context.budget)?; return Ok(chain_output(output, &mut dependency_tracking)); } @@ -2715,6 +2770,7 @@ where } tracking.discard_provenance(); } + finalize.settle(settled_independent(dependency_tracking.as_ref())); let output = finalize.finish_base64(data, context.budget, context.scratch)?; return Ok(chain_output(output, &mut dependency_tracking)); } @@ -5151,6 +5207,7 @@ mod tests { TransformOptions::default(), &TransformExecutionBudget::default(), vec![(7, digest_text.id())], + None, ) .unwrap(); @@ -5190,6 +5247,7 @@ mod tests { TransformOptions::default(), &TransformExecutionBudget::default(), vec![(11, digest_text.id())], + None, ) .unwrap(); diff --git a/tests/donor_interop_suite.rs b/tests/donor_interop_suite.rs index 3bb8dce0..e251cc7c 100644 --- a/tests/donor_interop_suite.rs +++ b/tests/donor_interop_suite.rs @@ -980,8 +980,16 @@ fn detached_signing_enforces_policy_request_and_shared_budget_atomically() { ))) if uri == "c14n11/xml-base-input.xml" )); + // Signing dereferences the resource once, so an aggregate allowance of + // exactly its size admits it and one byte less refuses it. allowed_policy.resources.max_external_resource_bytes = external_xml.len(); allowed_policy.resources.max_external_resource_total_bytes = external_xml.len(); + SignContext::new(&key) + .policy(allowed_policy.clone()) + .external_resources(&resources) + .sign_template(&template) + .expect("one dereference must fit an allowance of the resource size"); + allowed_policy.resources.max_external_resource_total_bytes = external_xml.len() - 1; let exhausted = SignContext::new(&key) .policy(allowed_policy) .external_resources(&resources) From dce99b4b6854387b45a39e13f80f1b2b78b8cb20 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 10:51:17 +0200 Subject: [PATCH 2/3] perf(xmldsig): analyze XSLT and Base64 references on the current tree XSLT reads only its input octets and its own stylesheet, and Base64 decodes the text of the nodes it is given, so a DigestValue element is in their input exactly when its future text is. References using them no longer need a parse of the document with placeholder values to find their dependencies. With the analysis now producing digests, an analysis made before the writer's KeyInfo could spend the operation's allowances on a result that is discarded: - The KeyInfo is spliced before the analysis; one that is not inert (it may add an ID or a namespace scope) is committed first and the analysis runs once after it, and the splice is reused by that commit - An inert KeyInfo keeps the analysis plan across the commit - A spent resource limit in the speculative analysis ends the operation instead of being retried after the commit --- src/xmldsig/sign.rs | 141 ++++++++++++++------- src/xmldsig/sign/single_execution_tests.rs | 90 ++++++++++++- 2 files changed, 184 insertions(+), 47 deletions(-) diff --git a/src/xmldsig/sign.rs b/src/xmldsig/sign.rs index 5cbe7027..04409af1 100644 --- a/src/xmldsig/sign.rs +++ b/src/xmldsig/sign.rs @@ -2244,39 +2244,49 @@ impl<'a> SignContext<'a> { speculative: true, ..request }; - // A `Reference` may read what these add, or name an ID only the writer - // provides, so a failed analysis is decided after the commit. - let analysis = (!doctype) + // Every path below joins the writer's KeyInfo to this document unless a + // commit comes first, so it is spliced once here and reused. + let mut key_info = match key_info_content.as_deref() { + Some(content) if !doctype => { + let budget = operation.budgets_mut().transforms.xml_parse_work(); + Some(self.splice_signing_key_info(document, target_signature, content, budget)?) + } + _ => None, + }; + // A KeyInfo that is not inert can change what a URI resolves to, so it + // is committed and everything analyzed after it; analyzing before it + // would spend the operation's allowances on a discarded result. A + // `Reference` may also read what these add, so a failed analysis is + // decided after the commit, unless it spent a limit: the analysis after + // the commit would only meet that limit again. + let analysis = match (!doctype && key_info.as_ref().is_none_or(|spliced| spliced.inert)) .then(|| analyze_signing_references(document, operation, deferred, transform_options)) - .and_then(Result::ok); - // This analysis succeeds only on the current tree, whose transforms keep - // or drop whole subtrees. Generated canonicalization changes only the - // tracked Reference it joins and adds no ID, so the dependencies and the - // digests this analysis proved final hold after that commit and no chain - // is executed again. The writer's KeyInfo can bring an ID that changes - // what a URI resolves to, so after it everything is analyzed again. + { + Some(Ok(analyzed)) => Some(analyzed), + Some(Err(error)) if spends_operation_limit(&error) => return Err(error.into()), + Some(Err(_)) | None => None, + }; + // This analysis succeeds only on the current tree, whose transforms + // depend on whole subtrees of their input. Generated canonicalization + // changes only the tracked Reference it joins, and an inert KeyInfo only + // tracked nodes, neither adding an ID or a namespace scope, so the + // dependencies and the digests this analysis proved final hold after + // those commits and no chain is executed again. let mut speculative_plan = None; if let Some(analyzed) = analysis { if digests_can_defer(document, &analyzed) && analyzed.1.splice_unread { - let budget = operation.budgets_mut().transforms.xml_parse_work(); - let key_info = key_info_content - .as_deref() - .map(|content| { - self.splice_signing_key_info(document, target_signature, content, budget) - }) - .transpose()?; - if key_info.as_ref().is_none_or(|spliced| spliced.inert) { - let splice = DeferredSplice::new(document.as_xml(), key_info, generated); - self.policy - .resources - .validate_xml_document_len(splice.spliced.xml.len())?; - return Ok((analyzed, setup_gate, Some(splice))); - } + let splice = DeferredSplice::new(document.as_xml(), key_info, generated); + self.policy + .resources + .validate_xml_document_len(splice.spliced.xml.len())?; + return Ok((analyzed, setup_gate, Some(splice))); } - speculative_plan = key_info_content.is_none().then_some(analyzed.1); + speculative_plan = Some(analyzed.1); } if !generated.is_empty() { setup_gate = self.commit_generated_c14n(document, operation, setup_gate, generated)?; + // The splice was made against the generation this commit replaced. + key_info = None; } if let Some(content) = key_info_content { setup_gate = self.populate_signing_key_info( @@ -2284,7 +2294,7 @@ impl<'a> SignContext<'a> { target_signature, operation, setup_gate, - &content, + (&content, key_info), )?; } if let Some(plan) = speculative_plan { @@ -2303,26 +2313,30 @@ impl<'a> SignContext<'a> { Ok((analyzed, setup_gate, None)) } - /// Commit `KeyInfo` as its own generation, for a `Reference` that reads it. + /// Commit `KeyInfo` as its own generation, for a `Reference` that reads it, + /// from `spliced` when that was made against the current generation. fn populate_signing_key_info( &self, document: &mut XmlDocument, target_signature: usize, operation: &mut SigningExecution<'_>, setup_gate: OperationNodeId, - key_info_content: &str, + (key_info_content, spliced): (&str, Option), ) -> Result { let mutation = operation.add_node(OperationNodeKind::Mutation, OperationStage::Resolve, None); operation.add_dependency(mutation, setup_gate)?; operation.compile()?; operation.run_document_transition(mutation, document, |document, budgets| { - let populated = self.splice_signing_key_info( - document, - target_signature, - key_info_content, - budgets.transforms.xml_parse_work(), - )?; + let populated = match spliced { + Some(spliced) => spliced, + None => self.splice_signing_key_info( + document, + target_signature, + key_info_content, + budgets.transforms.xml_parse_work(), + )?, + }; document .replace_serialized_with_settings( populated.xml, @@ -3307,6 +3321,23 @@ fn append_generated_c14n(references: &mut [SigningReference], generated: &[NodeI } } +/// Whether a transform's input selection and output depend only on whole +/// subtrees of its input. Base64 decodes the text of the nodes it is given +/// (`XMLDSig` 1.1 ยง6.6.2) and XSLT reads only its input octets and its own +/// stylesheet, with no resolver; `XPath` filters can test any node or value. +/// +fn selects_whole_subtrees(transform: &Transform) -> bool { + match transform { + Transform::Enveloped + | Transform::XpathExcludeAllSignatures + | Transform::C14n(_) + | Transform::Base64Decode => true, + #[cfg(feature = "xslt")] + Transform::Xslt(_) => true, + _ => false, + } +} + fn analyze_signing_reference_inputs( document: &XmlDocument, policy: &crate::policy::SigningPolicy, @@ -3316,15 +3347,12 @@ fn analyze_signing_reference_inputs( references: &SigningReferenceInputs, ) -> Result { let all_references = references.signed_info.iter().chain(&references.manifests); - if all_references.clone().all(|reference| { - reference.transforms.iter().all(|transform| { - matches!( - transform, - Transform::Enveloped | Transform::XpathExcludeAllSignatures | Transform::C14n(_) - ) - }) - }) { - // These transforms keep or remove whole subtrees, so a DigestValue or + if all_references + .clone() + .all(|reference| reference.transforms.iter().all(selects_whole_subtrees)) + { + // These transforms keep or remove whole subtrees, or read only the + // octets or text of the nodes they are given, so a DigestValue or // SignatureValue element is covered exactly when its future text is: // the current tree decides every dependency without placeholder text. // That holds only while filling a value removes nothing but text; a @@ -3379,6 +3407,17 @@ fn analyze_signing_reference_inputs( ) } +#[cfg(test)] +thread_local! { + static PLACEHOLDER_ANALYSES: core::cell::Cell = const { core::cell::Cell::new(0) }; +} + +/// Placeholder-generation analyses on this thread since the last call. +#[cfg(test)] +fn take_placeholder_analyses() -> usize { + PLACEHOLDER_ANALYSES.with(|count| count.replace(0)) +} + /// Analyze the `Reference` set on a parse of the document with placeholder text /// in every value the operation fills. fn analyze_signing_placeholder_generation( @@ -3389,6 +3428,8 @@ fn analyze_signing_placeholder_generation( transform_options: TransformOptions<'_>, references: &SigningReferenceInputs, ) -> Result { + #[cfg(test)] + PLACEHOLDER_ANALYSES.with(|count| count.set(count.get() + 1)); let signed_info_references = &references.signed_info; let manifest_references = &references.manifests; let reference_limit = references.maximum; @@ -3955,6 +3996,20 @@ fn reference_dependency_levels( }) } +/// Whether a failure is a refused resource or work limit of the operation. +fn spends_operation_limit(error: &SigningDigestError) -> bool { + use crate::policy::PolicyViolation::{ResourceLimit, ResourceLimitExceeded}; + matches!( + error, + SigningDigestError::Policy(ResourceLimit { .. } | ResourceLimitExceeded { .. }) + | SigningDigestError::Transform( + TransformError::Policy(ResourceLimit { .. } | ResourceLimitExceeded { .. }) + | TransformError::C14n(crate::c14n::C14nError::SharedExecution(_)) + ) + | SigningDigestError::Provider(crate::provider::ProviderError::Resource(_)) + ) +} + fn map_reference_digest_error( error: super::transforms::ReferenceDigestError, ) -> SigningDigestError { diff --git a/src/xmldsig/sign/single_execution_tests.rs b/src/xmldsig/sign/single_execution_tests.rs index 145180bd..8c651c97 100644 --- a/src/xmldsig/sign/single_execution_tests.rs +++ b/src/xmldsig/sign/single_execution_tests.rs @@ -6,6 +6,7 @@ const EXCLUSIVE: &str = "http://www.w3.org/2001/10/xml-exc-c14n#"; const RSA_SHA256: &str = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; const SHA256: &str = "http://www.w3.org/2001/04/xmlenc#sha256"; const XPATH: &str = "http://www.w3.org/TR/1999/REC-xpath-19991116"; +const BASE64: &str = "http://www.w3.org/2000/09/xmldsig#base64"; struct FixedKey; @@ -40,7 +41,7 @@ fn reference(uri: &str, transforms: &str) -> String { fn template(references: &str, object: &str) -> String { format!( - r#"onetwox{references}{object}"# + r#"onetwoxaGVsbG8={references}{object}"# ) } @@ -129,8 +130,9 @@ fn an_independent_reference_executes_its_transforms_once() { #[cfg(feature = "xslt")] #[test] fn an_xslt_reference_executes_its_stylesheet_once() { - // The benchmark shape: one XSLT Reference over a list, analyzed on the - // placeholder generation, is digested from that single execution. + // The benchmark shape: one XSLT Reference over a list. XSLT reads only + // its input octets, so the current tree decides its dependencies without + // parsing a placeholder copy, and the digest comes from that execution. let stylesheet = r#""#; let xml = template( &reference( @@ -140,11 +142,91 @@ fn an_xslt_reference_executes_its_stylesheet_once() { "", ); let policy = crate::policy::SigningPolicy::default(); + take_placeholder_analyses(); let (signed, chains) = sign_counting(&xml, policy.clone()); - assert_eq!(chains, 1); + assert_eq!((chains, take_placeholder_analyses()), (1, 0)); assert_digests_match_final_document(&signed, &policy); } +#[test] +fn only_a_value_testing_transform_needs_the_placeholder_copy() { + // Base64 decodes the text of whole selected subtrees, so the current tree + // decides its dependencies. An XPath filter can test any node, so its + // analysis still runs on a copy with placeholder values. + let policy = crate::policy::SigningPolicy::default(); + let cases = [ + ("base64", reference("#data", &transform(BASE64, "")), 0), + ( + "xpath", + reference( + "#payload", + &transform(XPATH, "not(self::comment())"), + ), + 1, + ), + ]; + for (name, references, placeholder_analyses) in cases { + let xml = template(&references, ""); + take_placeholder_analyses(); + let (signed, chains) = sign_counting(&xml, policy.clone()); + assert_eq!( + (chains, take_placeholder_analyses()), + (1, placeholder_analyses), + "{name}" + ); + assert_digests_match_final_document(&signed, &policy); + } +} + +struct Writer(&'static str); + +impl KeyInfoWriter for Writer { + fn write_key_info(&self, _: &dyn SigningKey) -> Result { + Ok(self.0.to_owned()) + } +} + +#[test] +fn a_writer_key_info_spends_no_canonical_bytes_on_discarded_analysis() { + // The least canonical allowance that signs is the same with or without the + // writer's KeyInfo, which no Reference covers: an analysis made before a + // KeyInfo that can change resolution would be discarded, so its + // canonicalization must not be charged to the operation. + let xml = template(&reference("#payload", &transform(EXCLUSIVE, "")), "") + .replace("", ""); + let least_allowance = |writer: Option<&Writer>| { + let signs = |max_canonicalized_bytes: usize| { + let mut policy = crate::policy::SigningPolicy::default(); + policy.resources.max_canonicalized_bytes = max_canonicalized_bytes; + let context = SignContext::new(&FixedKey).policy(policy); + match writer { + Some(writer) => context.key_info_writer(writer).sign_template(&xml), + None => context.sign_template(&xml), + } + .is_ok() + }; + let (mut low, mut high) = (1, 64 * 1024); + while low < high { + let middle = low + (high - low) / 2; + if signs(middle) { + high = middle; + } else { + low = middle + 1; + } + } + low + }; + let without = least_allowance(None); + for writer in [ + Writer("signer"), + Writer( + "signer", + ), + ] { + assert_eq!(least_allowance(Some(&writer)), without, "{}", writer.0); + } +} + #[test] fn a_reference_that_reads_another_digest_executes_again() { // `#outer` covers a Manifest whose Reference covers another Manifest: From 50f52d648573bd095cce80a4a24d2541ae175b21 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 13:24:49 +0200 Subject: [PATCH 3/3] fix(xmldsig): settle dependent analysis without running its steps - Dependency analysis stops a chain once it is known to depend on a value signing fills: before an octet-consuming step (XSLT, Base64) whose input already holds a tracked node, or after a dependency lost its provenance. The output was discarded anyway; a stylesheet no longer runs on a DigestValue not yet filled, where it could fail although the digest pass after the fill succeeds - A writer KeyInfo that removes an element carrying an ID is not inert: the plan made before the splice is no longer kept across its commit and the splice is not deferred, so a Reference to the removed ID is refused instead of keeping a digest of a target the signed document lacks Regression tests cover a Manifest stylesheet that refuses an empty DigestValue and a KeyName ID dropped by the writer, committed and deferred. --- src/xmldsig/mutation.rs | 91 ++++++++++++++-------- src/xmldsig/sign/single_execution_tests.rs | 55 +++++++++++++ src/xmldsig/transforms.rs | 49 ++++++++++++ 3 files changed, 164 insertions(+), 31 deletions(-) diff --git a/src/xmldsig/mutation.rs b/src/xmldsig/mutation.rs index cda3b436..5914c82b 100644 --- a/src/xmldsig/mutation.rs +++ b/src/xmldsig/mutation.rs @@ -554,6 +554,52 @@ fn key_info_source_extent( ) } +/// The template `KeyInfo` content the writer's output replaces, and whether any +/// of it carries an ID. Writer-provided identity is authoritative within its own +/// group: generated key material replaces stale material, while `KeyName` is +/// replaced only by a generated `KeyName`; extension elements remain. +fn stale_key_info_ranges( + key_info: Node<'_, '_>, + (generated_key_material, generated_key_name, generated_x509_data): (bool, bool, bool), + id_attributes: &[crate::IdAttributeRegistration], +) -> Result<(Vec>, bool), XmlMutationError> { + let mut stale_ranges = Vec::new(); + let mut removes_id = false; + let mut remove = |removed: Node<'_, '_>| { + removes_id |= removed + .descendants() + .filter(Node::is_element) + .any(|element| { + element + .attributes() + .any(|attribute| crate::xml::is_id_attribute(element, attribute, id_attributes)) + }); + actionable_node_range(removed) + }; + for node in key_info.children().filter(Node::is_element) { + let (namespace, name) = (node.tag_name().namespace(), node.tag_name().name()); + let replaces_key_material = + generated_key_material && is_cryptographic_key_info_source(namespace, name); + let replaces_key_name = generated_key_name && is_dsig_key_name(namespace, name); + if !(replaces_key_material || replaces_key_name) + || !has_cryptographic_identity_content(node) + { + continue; + } + if generated_x509_data && is_dsig_x509_data(namespace, name) { + for child in node + .children() + .filter(|child| child.is_element() && is_x509_identity_child(*child)) + { + stale_ranges.push(remove(child)?); + } + } else { + stale_ranges.push(remove(node)?); + } + } + Ok((stale_ranges, removes_id)) +} + /// Merge writer output into the target `/` of `xml`, whose /// current parse is `document`. The result is not reparsed here: the writer's /// content is parsed in its namespace context, and the extent it reports lets @@ -591,7 +637,8 @@ pub(super) fn merge_key_info_source_in_document( // inherited prefixes have exactly the semantics they will have in KeyInfo. let wrapped_source = wrap_key_info_children(key_info_source, key_info, policy)?; let source_document = parse_mutation_xml_with_budget(&wrapped_source, policy, budget)?; - let extent = key_info_source_extent(source_document.root_element(), key_info, id_attributes); + let mut extent = + key_info_source_extent(source_document.root_element(), key_info, id_attributes); let sources = source_document .root_element() .children() @@ -621,36 +668,18 @@ pub(super) fn merge_key_info_source_in_document( .any(|(namespace, name)| is_dsig_x509_data(*namespace, name)); let mut output = xml.to_owned(); if !generated_key_material_sources.is_empty() || generated_key_name { - // Writer-provided identity is authoritative within its own group. - // Generated key material replaces stale material, while KeyName is - // replaced only by a generated KeyName; extension elements remain. - let mut stale_ranges = Vec::new(); - for node in key_info.children().filter(Node::is_element) { - let replaces_key_material = !generated_key_material_sources.is_empty() - && is_cryptographic_key_info_source( - node.tag_name().namespace(), - node.tag_name().name(), - ); - let replaces_key_name = generated_key_name - && is_dsig_key_name(node.tag_name().namespace(), node.tag_name().name()); - if !(replaces_key_material || replaces_key_name) - || !has_cryptographic_identity_content(node) - { - continue; - } - if generated_x509_data - && is_dsig_x509_data(node.tag_name().namespace(), node.tag_name().name()) - { - for child in node - .children() - .filter(|child| child.is_element() && is_x509_identity_child(*child)) - { - stale_ranges.push(actionable_node_range(child)?); - } - } else { - stale_ranges.push(actionable_node_range(node)?); - } - } + let (mut stale_ranges, removes_id) = stale_key_info_ranges( + key_info, + ( + !generated_key_material_sources.is_empty(), + generated_key_name, + generated_x509_data, + ), + id_attributes, + )?; + // Removing an ID changes what a Reference resolves to as much as + // adding one, so a splice that drops one is not inert either. + extent.2 &= !removes_id; order_disjoint_ranges_for_splicing(&mut stale_ranges, output.len())?; for range in stale_ranges { output.replace_range(range, ""); diff --git a/src/xmldsig/sign/single_execution_tests.rs b/src/xmldsig/sign/single_execution_tests.rs index 8c651c97..d0b9efed 100644 --- a/src/xmldsig/sign/single_execution_tests.rs +++ b/src/xmldsig/sign/single_execution_tests.rs @@ -7,6 +7,9 @@ const RSA_SHA256: &str = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; const SHA256: &str = "http://www.w3.org/2001/04/xmlenc#sha256"; const XPATH: &str = "http://www.w3.org/TR/1999/REC-xpath-19991116"; const BASE64: &str = "http://www.w3.org/2000/09/xmldsig#base64"; +const ENVELOPED: &str = "http://www.w3.org/2000/09/xmldsig#enveloped-signature"; +#[cfg(feature = "xslt")] +const XSLT: &str = "http://www.w3.org/TR/1999/REC-xslt-19991116"; struct FixedKey; @@ -271,3 +274,55 @@ fn a_reference_with_a_value_reading_xpath_after_octets_executes_again() { assert_eq!(chains, 2); assert_digests_match_final_document(&signed, &policy); } + +#[cfg(feature = "xslt")] +#[test] +fn a_stylesheet_over_an_unfilled_value_runs_once_the_value_is_filled() { + // A SignedInfo Reference transforms a Manifest whose DigestValue is filled + // first, with a stylesheet that refuses an empty one. Its input already + // holds that value, so analysis knows the dependency without running the + // stylesheet; the digest pass runs it after the value is filled. + let stylesheet = r#"unfilled"#; + let manifest = format!( + r#"{}"#, + reference("#payload", &transform(EXCLUSIVE, "")), + ); + let xml = template( + &reference("#manifest", &transform(XSLT, stylesheet)), + &manifest, + ); + let policy = crate::policy::SigningPolicy { + manifest_processing: crate::policy::ManifestProcessing::Process, + ..crate::policy::SigningPolicy::default() + }; + let signed = SignContext::new(&FixedKey) + .policy(policy.clone()) + .sign_template(&xml) + .expect("the stylesheet runs on the filled Manifest value"); + assert_digests_match_final_document(&signed, &policy); +} + +#[test] +fn a_writer_key_info_that_drops_a_referenced_id_is_refused() { + // The writer's KeyName replaces the template's, which carries the ID a + // Reference names. The signed document no longer holds that target, so + // signing fails instead of keeping a digest taken before the splice, + // whether the splice is committed (another Reference reads KeyInfo) or + // deferred to the final mutation. + let dropped = reference( + "#old", + &[transform(ENVELOPED, ""), transform(EXCLUSIVE, "")].concat(), + ); + let writer = + Writer("signer"); + for references in [[dropped.clone(), reference("#ki", "")].concat(), dropped] { + let xml = template(&references, "").replace( + "", + r#"template"#, + ); + let result = SignContext::new(&FixedKey) + .key_info_writer(&writer) + .sign_template(&xml); + assert!(result.is_err(), "{references}: {result:?}"); + } +} diff --git a/src/xmldsig/transforms.rs b/src/xmldsig/transforms.rs index 8ed537ae..1204540a 100644 --- a/src/xmldsig/transforms.rs +++ b/src/xmldsig/transforms.rs @@ -1799,6 +1799,12 @@ pub(crate) trait TransformTerminal { /// chain are empty. fn settle(&mut self, _independent: bool) {} + /// The output of a tracked chain already known to depend on a tracked + /// node, if this terminal needs nothing more from the chain then. + fn dependent_output(&self) -> Option { + None + } + fn finish( self, data: ExecutionData<'_, S, D>, @@ -1876,6 +1882,11 @@ impl TransformTerminal for AnalysisTerminal<'_> { } } + // A dependent output is discarded, so the chain stops where that is known. + fn dependent_output(&self) -> Option { + Some(None) + } + fn finish( self, data: ExecutionData<'_, Native, D>, @@ -2626,6 +2637,41 @@ fn apply_chain_step<'e, 'd, S: TransformScratch, D: DomStorage, I: Instruction>( Ok(ChainStep::untracked(enveloped_signature, data)) } +/// The terminal's output for a chain that already depends on a tracked node +/// before `transform` runs, when the terminal needs nothing more then. +/// +/// An octet-consuming step reads the content of all of its input, so a tracked +/// node already there makes the chain dependent whatever the step produces, as +/// does a dependency that lost its provenance. A dependent output is discarded, +/// so the step need not run on a value not yet filled, where it could fail +/// although the digest pass after the fill succeeds. +fn known_dependent>( + transform: &I, + data: &ExecutionData<'_, S, D>, + tracking: &mut Option, + finalize: &F, +) -> Result, TransformError> { + let (Some(tracking), Some(output)) = (tracking.as_mut(), finalize.dependent_output()) else { + return Ok(None); + }; + let reads_content = !transform_requires_node_set(transform); + if reads_content && let Some(nodes) = data.nodes() { + retain_visible_dependencies(nodes, tracking)?; + tracking.dormant_indexes.clear(); + } + let reads_tracked = reads_content + && (!tracking.active_nodes.is_empty() + || tracking + .canonical_positions + .as_ref() + .is_some_and(|positions| !positions.is_empty())); + if !reads_tracked && tracking.opaque_dependencies.is_empty() { + return Ok(None); + } + tracking.discard_provenance(); + Ok(Some(output)) +} + /// Whether a chain's settled dependencies are empty; an untracked chain has none. fn settled_independent(tracking: Option<&DependencyTracking>) -> bool { tracking.is_none_or(|tracking| { @@ -2774,6 +2820,9 @@ where let output = finalize.finish_base64(data, context.budget, context.scratch)?; return Ok(chain_output(output, &mut dependency_tracking)); } + if let Some(output) = known_dependent(transform, &data, &mut dependency_tracking, &finalize)? { + return Ok(chain_output(output, &mut dependency_tracking)); + } // The step consumes its input, so a decoded or canonicalized predecessor is // never pinned in this frame while the rest of the chain runs. let next = apply_chain_step(