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.rs b/src/xmldsig/sign.rs index 8bff32f6..04409af1 100644 --- a/src/xmldsig/sign.rs +++ b/src/xmldsig/sign.rs @@ -2244,30 +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) - .filter(|analyzed| digests_can_defer(document, analyzed) && analyzed.1.splice_unread); + { + 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 { - 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) { + if digests_can_defer(document, &analyzed) && analyzed.1.splice_unread { 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 = 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( @@ -2275,33 +2294,49 @@ impl<'a> SignContext<'a> { target_signature, operation, setup_gate, - &content, + (&content, key_info), )?; } + 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)) } - /// 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, @@ -2527,6 +2562,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 +2575,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 +2952,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 +3176,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, @@ -3282,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, @@ -3291,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 @@ -3326,7 +3379,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) })?; @@ -3351,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( @@ -3361,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; @@ -3424,7 +3493,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 +3512,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 +3574,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 +3603,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 +3628,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 +3662,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 +3674,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 +3729,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 +3748,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 +3760,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 +3815,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 +3822,7 @@ fn fill_reference_digest_values_in_dependency_order( external_resources: &external_resources, generated: &[], speculative: false, + provider, }, None, false, @@ -3751,6 +3836,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 +3897,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 +3910,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 +3928,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 +3992,33 @@ fn reference_dependency_levels( dependencies: analyses, levels, splice_unread, + digests, }) } +/// 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 { + 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 +4258,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 +4277,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 +4307,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 +4911,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..d0b9efed --- /dev/null +++ b/src/xmldsig/sign/single_execution_tests.rs @@ -0,0 +1,328 @@ +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"; +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; + +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#"onetwoxaGVsbG8={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(); + take_placeholder_analyses(); + let (signed, chains) = sign_counting(&xml, policy.clone()); + 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: + // 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); +} + +#[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 0e8b43ef..1204540a 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,17 @@ 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) {} + + /// 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>, @@ -1842,31 +1866,55 @@ 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; + } + } + + // A dependent output is discarded, so the chain stops where that is known. + fn dependent_output(&self) -> Option { + Some(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 +2433,11 @@ fn canonicalize_tracked( signature_node: Node<'a, 'a>, @@ -2393,7 +2446,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 +2494,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 +2637,48 @@ 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| { + 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 +2730,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 +2745,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 +2801,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,9 +2816,13 @@ 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)); } + 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( @@ -5151,6 +5256,7 @@ mod tests { TransformOptions::default(), &TransformExecutionBudget::default(), vec![(7, digest_text.id())], + None, ) .unwrap(); @@ -5190,6 +5296,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)