From 84f024d3dcbf2b8188c447acaa6a889405eccd39 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 11:16:11 +0200 Subject: [PATCH 1/4] perf: hash arena identities without SipHash in content tables Content-keyed tables hashed every key with the randomly keyed SipHash, including the arena identities (document identity and slot) that XPath node-set deduplication stores for every selected node. - The table alias now uses a seeded hasher: integer words are mixed by a folded multiply under a random odd seed per table, byte strings and every word after them go through the randomly keyed SipHash in order, so neither form lets a document predict bucket positions - Tables are built with Default instead of the RandomState-only new - Without std the tables remain ordered trees Closes #368 --- scripts/patches/xmloxide-0.5.0.patch | 9 ++ src/collections.rs | 181 +++++++++++++++++++++++++- src/collections/tests.rs | 51 ++++++++ src/key_manager.rs | 4 +- src/operation.rs | 4 +- src/parser_xmloxide/validation/dtd.rs | 2 +- src/sxd_document/lazy_hash_map.rs | 2 +- src/sxd_xpath/nodeset/storage.rs | 2 +- src/sxd_xpath/runtime.rs | 2 +- src/xml.rs | 4 +- src/xml/dom/storage.rs | 2 +- src/xml/dom/storage/names.rs | 2 +- src/xml/dom/tree/comparison.rs | 2 +- src/xmldsig/sign.rs | 4 +- src/xmldsig/transforms.rs | 8 +- src/xmldsig/verify.rs | 8 +- src/xmldsig/verify/tests.rs | 6 +- src/xmldsig/xpath/storage.rs | 2 +- 18 files changed, 264 insertions(+), 31 deletions(-) create mode 100644 src/collections/tests.rs diff --git a/scripts/patches/xmloxide-0.5.0.patch b/scripts/patches/xmloxide-0.5.0.patch index b2323a56..1d158dce 100644 --- a/scripts/patches/xmloxide-0.5.0.patch +++ b/scripts/patches/xmloxide-0.5.0.patch @@ -10587,6 +10587,15 @@ /// use xmloxide::Document; /// use xmloxide::validation::dtd::{parse_dtd, validate}; /// +@@ -2262,7 +2553,7 @@ + pub fn validate(doc: &mut Document, dtd: &Dtd) -> ValidationResult { + let mut errors = Vec::new(); + let mut warnings = Vec::new(); +- let mut id_values: HashSet = HashSet::new(); ++ let mut id_values: HashSet = HashSet::default(); + let mut idref_values: Vec = Vec::new(); + + // Check root element name against DOCTYPE @@ -2311,21 +2602,19 @@ } }); diff --git a/src/collections.rs b/src/collections.rs index 98e22ef4..b6d0b154 100644 --- a/src/collections.rs +++ b/src/collections.rs @@ -11,13 +11,183 @@ pub(crate) use alloc::collections::btree_map::Entry; pub(crate) use alloc::collections::{BTreeMap as Map, BTreeSet as Set}; #[cfg(all(not(feature = "std"), feature = "xmldsig"))] pub(crate) use alloc::collections::{btree_map as map, btree_set as set}; +#[cfg(feature = "std")] +use core::hash::{BuildHasher, Hasher}; #[cfg(all(feature = "std", any(feature = "xmldsig", test)))] pub(crate) use std::collections::hash_map::Entry; -#[cfg(feature = "std")] -pub(crate) use std::collections::{HashMap as Map, HashSet as Set}; #[cfg(all(feature = "std", feature = "xmldsig"))] pub(crate) use std::collections::{hash_map as map, hash_set as set}; +/// A hash table keyed by document content. +#[cfg(feature = "std")] +pub(crate) type Map = std::collections::HashMap; +/// A hash set keyed by document content. +#[cfg(feature = "std")] +pub(crate) type Set = std::collections::HashSet; + +/// Builds the hasher of one content-keyed table. +/// +/// A document controls the strings these tables hash and chooses which arena +/// identities they hold. Strings go through the standard randomly keyed +/// `SipHash`; integer words, which most keys are (a document identity and a +/// slot), are mixed by a folded multiply under a random odd seed. Neither lets +/// a document predict where its keys land, and an identity costs a few +/// multiplications instead of a `SipHash`. +/// +/// `pub` only because public storage traits name these tables in associated +/// types; this module is private, so nothing outside the crate can name it. +#[cfg(feature = "std")] +#[derive(Clone)] +pub struct SeededState { + strings: std::hash::RandomState, + seed: u64, +} + +#[cfg(feature = "std")] +impl Default for SeededState { + fn default() -> Self { + let strings = std::hash::RandomState::new(); + // The keyed SipHash of a constant is as unpredictable as its keys. + let seed = strings.hash_one(0x5eed_u64) | 1; + Self { strings, seed } + } +} + +#[cfg(feature = "std")] +impl BuildHasher for SeededState { + type Hasher = SeededHasher; + + #[inline] + fn build_hasher(&self) -> SeededHasher { + SeededHasher { + state: self.seed, + seed: self.seed, + keys: self.strings.clone(), + strings: None, + } + } +} + +/// The hasher of [`SeededState`]. +#[cfg(feature = "std")] +pub struct SeededHasher { + state: u64, + seed: u64, + keys: std::hash::RandomState, + /// Started at the first byte string written, with the words before it. + /// Every later word goes into it in order: a string's length or end mark + /// mixed apart from its bytes would let `("a", "bc")` equal `("ab", "c")`. + strings: Option, +} + +/// The 128-bit product folded to 64 bits: every input bit reaches the low +/// bits that select a bucket. +#[cfg(feature = "std")] +#[inline] +fn folded_multiply(x: u64, y: u64) -> u64 { + let (low, high) = halves(u128::from(x) * u128::from(y)); + low ^ high +} + +/// The low and high 64 bits of `word`. +#[cfg(feature = "std")] +#[inline] +fn halves(word: u128) -> (u64, u64) { + // Both halves fit by construction; the checks fold away. + let low = u64::try_from(word & u128::from(u64::MAX)).expect("masked to 64 bits"); + let high = u64::try_from(word >> 64).expect("shifted to 64 bits"); + (low, high) +} + +#[cfg(feature = "std")] +impl Hasher for SeededHasher { + #[inline] + fn write(&mut self, bytes: &[u8]) { + let (keys, state) = (&self.keys, self.state); + self.strings + .get_or_insert_with(|| { + let mut strings = keys.build_hasher(); + strings.write_u64(state); + strings + }) + .write(bytes); + } + + #[inline] + fn write_u8(&mut self, word: u8) { + self.write_u64(u64::from(word)); + } + + #[inline] + fn write_u16(&mut self, word: u16) { + self.write_u64(u64::from(word)); + } + + #[inline] + fn write_u32(&mut self, word: u32) { + self.write_u64(u64::from(word)); + } + + #[inline] + fn write_u64(&mut self, word: u64) { + match &mut self.strings { + None => self.state = folded_multiply(self.state ^ word, self.seed), + Some(strings) => strings.write_u64(word), + } + } + + #[inline] + fn write_u128(&mut self, word: u128) { + let (low, high) = halves(word); + self.write_u64(low); + self.write_u64(high); + } + + #[inline] + fn write_usize(&mut self, word: usize) { + self.write_u64(word as u64); + } + + #[inline] + fn write_i8(&mut self, word: i8) { + self.write_u8(word.cast_unsigned()); + } + + #[inline] + fn write_i16(&mut self, word: i16) { + self.write_u16(word.cast_unsigned()); + } + + #[inline] + fn write_i32(&mut self, word: i32) { + self.write_u32(word.cast_unsigned()); + } + + #[inline] + fn write_i64(&mut self, word: i64) { + self.write_u64(word.cast_unsigned()); + } + + #[inline] + fn write_i128(&mut self, word: i128) { + self.write_u128(word.cast_unsigned()); + } + + #[inline] + fn write_isize(&mut self, word: isize) { + self.write_usize(word.cast_unsigned()); + } + + #[inline] + fn finish(&self) -> u64 { + let state = match &self.strings { + Some(strings) => strings.finish(), + None => self.state, + }; + folded_multiply(state, self.seed.rotate_left(32) | 1) + } +} + /// What [`Map`] and [`Set`] require of their keys in this build. #[cfg(feature = "std")] pub(crate) trait Key: core::hash::Hash + Eq {} @@ -47,7 +217,7 @@ pub(crate) trait Table: Sized { #[cfg(feature = "std")] impl Table for Map { fn with_entry_capacity(capacity: usize) -> Self { - Self::with_capacity(capacity) + Self::with_capacity_and_hasher(capacity, SeededState::default()) } fn try_reserve_entries(&mut self, additional: usize) -> Result<(), TryReserveError> { @@ -63,7 +233,7 @@ impl Table for Map { #[cfg(feature = "std")] impl Table for Set { fn with_entry_capacity(capacity: usize) -> Self { - Self::with_capacity(capacity) + Self::with_capacity_and_hasher(capacity, SeededState::default()) } fn try_reserve_entries(&mut self, additional: usize) -> Result<(), TryReserveError> { @@ -107,3 +277,6 @@ impl Table for Set { self.len() } } + +#[cfg(all(test, feature = "std"))] +mod tests; diff --git a/src/collections/tests.rs b/src/collections/tests.rs new file mode 100644 index 00000000..f62c0bbd --- /dev/null +++ b/src/collections/tests.rs @@ -0,0 +1,51 @@ +use super::SeededState; +use core::hash::BuildHasher; + +/// Distinct values among `bits` low bits of the hashes of `keys`. +fn distinct_low_bits(state: &SeededState, keys: impl Iterator, bits: u32) -> usize { + let mask = (1_u64 << bits) - 1; + keys.map(|key| state.hash_one(key) & mask) + .collect::>() + .len() +} + +#[test] +fn equal_keys_hash_equally_within_a_table() { + // A table finds a key only if every hash of it agrees, for integer words, + // strings and the node-shaped tuples the XPath engine stores. + let state = SeededState::default(); + assert_eq!(state.hash_one(42_u64), state.hash_one(42_u64)); + assert_eq!(state.hash_one("prefix"), state.hash_one("prefix")); + assert_eq!( + state.hash_one((7_usize, 3_u8, 1024_usize)), + state.hash_one((7_usize, 3_u8, 1024_usize)) + ); + assert_ne!(state.hash_one("ab"), state.hash_one("ba")); + assert_ne!(state.hash_one(("a", "bc")), state.hash_one(("ab", "c"))); +} + +#[test] +fn strided_identities_spread_across_buckets() { + // A stylesheet can select every 2^k-th node, so slot indices sharing low + // zero bits must still reach distinct buckets. 4096 keys over 4096 + // buckets fill about 63% when hashing is uniform. + let state = SeededState::default(); + for stride in [1_u64, 1 << 8, 1 << 16, 1 << 32] { + let distinct = distinct_low_bits(&state, (0..4096).map(|key| key * stride), 12); + assert!(distinct > 2400, "stride {stride}: {distinct} buckets"); + } + // hashbrown reads the top seven bits as the control byte. + let control = (0..4096_u64) + .map(|key| state.hash_one(key << 20) >> 57) + .collect::>(); + assert_eq!(control.len(), 128); +} + +#[test] +fn tables_are_seeded_independently() { + // A document cannot carry collisions computed for one table to another. + let first = SeededState::default(); + let second = SeededState::default(); + assert_ne!(first.hash_one(1_u64), second.hash_one(1_u64)); + assert_ne!(first.hash_one("namespace"), second.hash_one("namespace")); +} diff --git a/src/key_manager.rs b/src/key_manager.rs index 6c536e63..dcf81bfe 100644 --- a/src/key_manager.rs +++ b/src/key_manager.rs @@ -1172,7 +1172,7 @@ impl KeyInventory { resources.max_external_resource_total_bytes, )); } - let mut names = Set::new(); + let mut names = Set::default(); for name in other .symmetric_keys .iter() @@ -2668,7 +2668,7 @@ impl KeyInventory { used: live_material + bytes.len(), maximum: resources.max_external_resource_total_bytes, }; - let mut names = Set::new(); + let mut names = Set::default(); let mut store = Self::default(); let mut entry_count = 0_usize; // One account for every public key read while loading this store. diff --git a/src/operation.rs b/src/operation.rs index 80bc7ce5..9ad7832e 100644 --- a/src/operation.rs +++ b/src/operation.rs @@ -347,9 +347,9 @@ impl OperationExecutionContext { builder: Some(OperationPlanBuilder::default()), plan: RefCell::new(None), document: document.map(|(identity, generation)| (identity, Cell::new(generation))), - executed: RefCell::new(Set::new()), + executed: RefCell::new(Set::default()), #[cfg(feature = "xmldsig")] - authenticated_nodes: RefCell::new(Set::new()), + authenticated_nodes: RefCell::new(Set::default()), decisions: RefCell::new(Vec::new()), first_failure: RefCell::new(None), } diff --git a/src/parser_xmloxide/validation/dtd.rs b/src/parser_xmloxide/validation/dtd.rs index 0faea645..fce870fa 100644 --- a/src/parser_xmloxide/validation/dtd.rs +++ b/src/parser_xmloxide/validation/dtd.rs @@ -2553,7 +2553,7 @@ fn is_name_char(c: char) -> bool { pub fn validate(doc: &mut Document, dtd: &Dtd) -> ValidationResult { let mut errors = Vec::new(); let mut warnings = Vec::new(); - let mut id_values: HashSet = HashSet::new(); + let mut id_values: HashSet = HashSet::default(); let mut idref_values: Vec = Vec::new(); // Check root element name against DOCTYPE diff --git a/src/sxd_document/lazy_hash_map.rs b/src/sxd_document/lazy_hash_map.rs index 6b2304b2..dc5d8a8f 100644 --- a/src/sxd_document/lazy_hash_map.rs +++ b/src/sxd_document/lazy_hash_map.rs @@ -33,7 +33,7 @@ where pub fn insert(&mut self, key: K, val: V) -> Option { self.map = match self.map.take() { Some(m) => Some(m), - None => Some(Map::new()), + None => Some(Map::default()), }; self.map.as_mut().and_then(|m| m.insert(key, val)) diff --git a/src/sxd_xpath/nodeset/storage.rs b/src/sxd_xpath/nodeset/storage.rs index da4bd7a0..23064d7e 100644 --- a/src/sxd_xpath/nodeset/storage.rs +++ b/src/sxd_xpath/nodeset/storage.rs @@ -13,7 +13,7 @@ pub(crate) enum Storage<'d> { impl Default for Storage<'_> { fn default() -> Self { - Self::Native(Set::new()) + Self::Native(Set::default()) } } diff --git a/src/sxd_xpath/runtime.rs b/src/sxd_xpath/runtime.rs index 06fdcb09..457524cb 100644 --- a/src/sxd_xpath/runtime.rs +++ b/src/sxd_xpath/runtime.rs @@ -394,7 +394,7 @@ impl Runtime for Native { node.visit_string_value_with_context(context, visit) } fn namespaces(self) -> Self::Namespaces { - Self::Namespaces::new() + Self::Namespaces::default() } fn list(self) -> Vec { Vec::new() diff --git a/src/xml.rs b/src/xml.rs index c17d5375..ae4ca8d5 100644 --- a/src/xml.rs +++ b/src/xml.rs @@ -164,8 +164,8 @@ impl<'a> XmlIdIndex<'a> { document: &'a Document<'a>, registrations: &[IdAttributeRegistration], ) -> Self { - let mut nodes = Map::new(); - let mut duplicates = Set::new(); + let mut nodes = Map::default(); + let mut duplicates = Set::default(); for node in document.descendants().filter(Node::is_element) { // ID registration is local-name based: qualified profile attributes // such as wsu:Id and xml:id participate alongside unqualified Id. diff --git a/src/xml/dom/storage.rs b/src/xml/dom/storage.rs index 8391e672..d397e764 100644 --- a/src/xml/dom/storage.rs +++ b/src/xml/dom/storage.rs @@ -189,7 +189,7 @@ impl Storage for Native { } fn declarations<'input>(&self) -> Self::Declarations<'input> { - crate::collections::Map::new() + crate::collections::Map::default() } fn freeze(value: String) -> String { value diff --git a/src/xml/dom/storage/names.rs b/src/xml/dom/storage/names.rs index a8df2a4f..50107967 100644 --- a/src/xml/dom/storage/names.rs +++ b/src/xml/dom/storage/names.rs @@ -75,7 +75,7 @@ impl> structured_asn1::memory::HashWork for Name { impl NameStorage for Native { type Names, V> = Map, V>; fn names, V>(&self) -> Self::Names { - Map::new() + Map::default() } } diff --git a/src/xml/dom/tree/comparison.rs b/src/xml/dom/tree/comparison.rs index bbebe3e6..870a31eb 100644 --- a/src/xml/dom/tree/comparison.rs +++ b/src/xml/dom/tree/comparison.rs @@ -30,7 +30,7 @@ pub(crate) trait Counts<'a, S: Storage> { impl ComparisonStorage for Native { type Counts<'a> = Map, usize>; fn counts<'a>(&self, capacity: usize) -> Result, ParseError> { - let mut counts = Map::new(); + let mut counts = Map::default(); counts .try_reserve_entries(capacity) .map_err(|_| ParseError::AllocationFailed)?; diff --git a/src/xmldsig/sign.rs b/src/xmldsig/sign.rs index 04409af1..c6989554 100644 --- a/src/xmldsig/sign.rs +++ b/src/xmldsig/sign.rs @@ -29,7 +29,7 @@ use sha2::{Sha224, Sha256, Sha384, Sha512}; use signature::hazmat::{PrehashSigner, RandomizedPrehashSigner}; use std::{ borrow::Cow, - collections::{BTreeMap, HashMap, HashSet}, + collections::{BTreeMap, HashMap}, ops::Range, }; use zeroize::Zeroizing; @@ -3831,7 +3831,7 @@ fn fill_reference_digest_values_in_dependency_order( } struct SigningDependencyPlan { - dependencies: Vec>, + dependencies: Vec>, levels: Vec>, /// No `Reference` can observe the target `KeyInfo` or a `Reference` that /// gains a generated transform. diff --git a/src/xmldsig/transforms.rs b/src/xmldsig/transforms.rs index 1204540a..9a771332 100644 --- a/src/xmldsig/transforms.rs +++ b/src/xmldsig/transforms.rs @@ -2452,8 +2452,8 @@ pub(crate) fn execute_transforms_with_dependency_nodes<'a>( relationship::validate_edition(transforms, options.opc_relationship_edition)?; budget.state.begin_chain(); let mut active_nodes = Vec::with_capacity(tracked_nodes.len()); - let mut opaque_dependencies = Set::new(); - let mut dormant_indexes = Set::new(); + let mut opaque_dependencies = Set::default(); + let mut dormant_indexes = Set::default(); for (index, node_id) in tracked_nodes { if let Some(node) = signature_node.document().get_node(node_id) { let belongs_to_input = match &initial_data { @@ -2891,7 +2891,7 @@ fn map_transform_xml_decode_error(error: crate::encoding::XmlEncodingError) -> T #[cfg(feature = "xmldsig-sign")] fn dependency_indexes(tracking: Option) -> Set { let Some(tracking) = tracking else { - return Set::new(); + return Set::default(); }; tracking .active_nodes @@ -5260,7 +5260,7 @@ mod tests { ) .unwrap(); - assert_eq!(output.dependencies, Set::from([7])); + assert_eq!(output.dependencies, Set::from_iter([7])); } #[cfg(feature = "xmldsig-sign")] diff --git a/src/xmldsig/verify.rs b/src/xmldsig/verify.rs index 8890d097..5022a8cd 100644 --- a/src/xmldsig/verify.rs +++ b/src/xmldsig/verify.rs @@ -1327,8 +1327,8 @@ impl<'a> VerificationOperationBudgets<'a> { ), xpath_parse: RefCell::new(xpath_parse), key_info_materialization: RefCell::new(KeyInfoMaterializationState::default()), - external_resource_fingerprints: RefCell::new(Map::new()), - resolved_reference_resources: RefCell::new(Map::new()), + external_resource_fingerprints: RefCell::new(Map::default()), + resolved_reference_resources: RefCell::new(Map::default()), } } @@ -5240,7 +5240,7 @@ fn materialize_retrieval_methods_with_budgets( } let mut total_binary_len = existing_x509_binary_len(key_info)?; - let mut seen = Set::new(); + let mut seen = Set::default(); let mut materialized = Vec::with_capacity(key_info.sources.len()); // Origins, when tracked, stay index for index with the sources. let mut pending = origins @@ -5716,7 +5716,7 @@ fn process_authenticated_manifest_references( core_validation: &[OperationNodeId], ) -> Result<(Vec, Vec), SignatureVerificationPipelineError> { let ctx = verification.context; - let mut processed_manifests = Set::new(); + let mut processed_manifests = Set::default(); let mut next_reference_index = 0usize; let mut frontier_index = 0usize; let mut completed_digests = Vec::new(); diff --git a/src/xmldsig/verify/tests.rs b/src/xmldsig/verify/tests.rs index be208226..6adfbc26 100644 --- a/src/xmldsig/verify/tests.rs +++ b/src/xmldsig/verify/tests.rs @@ -2684,7 +2684,7 @@ fn manifest_reference_limit_counts_unsupported_entries() { Some((view.identity(), view.generation())), ); operation.authenticate(view.node_identity(object)); - let mut processed = Set::new(); + let mut processed = Set::default(); let mut remaining = MAX_REFERENCES_PER_SIGNATURE; let mut next_index = 0; let mut xpath_parse = XPathSignatureParseBudget::default(); @@ -2727,7 +2727,7 @@ fn unsigned_manifest_discovery_cannot_bypass_the_filter_work_budget() { &budgets, Some((view.identity(), view.generation())), ); - let mut processed = Set::new(); + let mut processed = Set::default(); let mut remaining = 1; let mut index = 0; let mut xpath = XPathSignatureParseBudget::default(); @@ -2774,7 +2774,7 @@ fn unsigned_manifest_remains_eligible_after_trust_expands() { Some((view.identity(), view.generation())), ); operation.authenticate(view.node_identity(outer)); - let mut processed = Set::new(); + let mut processed = Set::default(); let mut remaining = 2; let mut next_index = 0; let mut xpath_budget = XPathSignatureParseBudget::default(); diff --git a/src/xmldsig/xpath/storage.rs b/src/xmldsig/xpath/storage.rs index d39db2f9..5eefe80e 100644 --- a/src/xmldsig/xpath/storage.rs +++ b/src/xmldsig/xpath/storage.rs @@ -52,7 +52,7 @@ impl<'source, 'w, W: Work> Bindings<'source, &'w W> for OwnedMap impl NativeIndex { pub(crate) fn new() -> Self { - Self(Map::new()) + Self(Map::default()) } } From 437f49339d872293979a91dc30d77e69ca5cd2b9 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 14:48:19 +0200 Subject: [PATCH 2/4] perf: take the seeded table hasher from structured-asn1 - Content-keyed tables use `structured_asn1::hash::RandomState`, the shared seeded hasher (integer mix under a per-table seed, keyed SipHash for strings and every word after one); this crate keeps only the table aliases. structured-asn1 0.20.2 becomes a required dependency, since tables exist without signature support too - Dependency analysis stops a chain early only before a step that reads content: a node-set step such as Enveloped still runs, so it can drop the Signature's own values instead of a known dependency turning them into a false cycle (regression test with a value-reading XPath, then Enveloped and canonicalization) --- Cargo.toml | 7 +- src/collections.rs | 173 ++------------------- src/xmldsig/sign/single_execution_tests.rs | 29 ++++ src/xmldsig/transforms.rs | 4 +- 4 files changed, 44 insertions(+), 169 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0b282774..1a485e16 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -364,8 +364,8 @@ ripemd = { version = "0.2.0", default-features = false, features = ["oid", "zero pkcs12 = { version = "=0.2.0-pre.0", default-features = false, features = ["kdf"], optional = true } pbkdf2 = { version = "0.13", default-features = false, features = ["hmac"], optional = true } -# X.509 certificates -structured-asn1 = { version = "0.20.0", default-features = false, features = ["alloc"], optional = true } +# X.509 certificates, and the seeded hashing of every table keyed by document content +structured-asn1 = { version = "0.20.2", default-features = false, features = ["alloc"] } structured-pkix = { version = "0.20.0", default-features = false, features = ["alloc"], optional = true } # PKCS#12 container framing; this crate keeps the password, KDF and cipher work. structured-cms = { version = "0.20.0", default-features = false, features = ["alloc"], optional = true } @@ -423,7 +423,7 @@ default = ["std", "xmldsig", "xmldsig-sign", "xmlenc", "c14n", "xml-backend-xmlo std = [ "thiserror/std", "xmlparser/std", - "structured-asn1?/std", + "structured-asn1/std", "structured-pkix?/std", "structured-cms?/std", "base64/std", @@ -471,7 +471,6 @@ xmldsig = [ # XML Digital Signature verification and public-key inven "dep:signature", "dep:snafu", "dep:subtle", - "dep:structured-asn1", "dep:structured-pkix", "dep:tinyvec", "dep:zeroize", diff --git a/src/collections.rs b/src/collections.rs index b6d0b154..a28ec558 100644 --- a/src/collections.rs +++ b/src/collections.rs @@ -11,182 +11,27 @@ pub(crate) use alloc::collections::btree_map::Entry; pub(crate) use alloc::collections::{BTreeMap as Map, BTreeSet as Set}; #[cfg(all(not(feature = "std"), feature = "xmldsig"))] pub(crate) use alloc::collections::{btree_map as map, btree_set as set}; -#[cfg(feature = "std")] -use core::hash::{BuildHasher, Hasher}; #[cfg(all(feature = "std", any(feature = "xmldsig", test)))] pub(crate) use std::collections::hash_map::Entry; #[cfg(all(feature = "std", feature = "xmldsig"))] pub(crate) use std::collections::{hash_map as map, hash_set as set}; -/// A hash table keyed by document content. -#[cfg(feature = "std")] -pub(crate) type Map = std::collections::HashMap; -/// A hash set keyed by document content. -#[cfg(feature = "std")] -pub(crate) type Set = std::collections::HashSet; - /// Builds the hasher of one content-keyed table. /// /// A document controls the strings these tables hash and chooses which arena -/// identities they hold. Strings go through the standard randomly keyed -/// `SipHash`; integer words, which most keys are (a document identity and a -/// slot), are mixed by a folded multiply under a random odd seed. Neither lets -/// a document predict where its keys land, and an identity costs a few -/// multiplications instead of a `SipHash`. -/// -/// `pub` only because public storage traits name these tables in associated -/// types; this module is private, so nothing outside the crate can name it. -#[cfg(feature = "std")] -#[derive(Clone)] -pub struct SeededState { - strings: std::hash::RandomState, - seed: u64, -} - -#[cfg(feature = "std")] -impl Default for SeededState { - fn default() -> Self { - let strings = std::hash::RandomState::new(); - // The keyed SipHash of a constant is as unpredictable as its keys. - let seed = strings.hash_one(0x5eed_u64) | 1; - Self { strings, seed } - } -} - -#[cfg(feature = "std")] -impl BuildHasher for SeededState { - type Hasher = SeededHasher; - - #[inline] - fn build_hasher(&self) -> SeededHasher { - SeededHasher { - state: self.seed, - seed: self.seed, - keys: self.strings.clone(), - strings: None, - } - } -} - -/// The hasher of [`SeededState`]. +/// identities they hold. Each table gets its own random seed: integer words, +/// which most keys are (a document identity and a slot), take a seeded integer +/// mix, and strings and every word after one take the keyed `SipHash`, so a +/// document can predict where neither lands. #[cfg(feature = "std")] -pub struct SeededHasher { - state: u64, - seed: u64, - keys: std::hash::RandomState, - /// Started at the first byte string written, with the words before it. - /// Every later word goes into it in order: a string's length or end mark - /// mixed apart from its bytes would let `("a", "bc")` equal `("ab", "c")`. - strings: Option, -} +pub(crate) type SeededState = structured_asn1::hash::RandomState; -/// The 128-bit product folded to 64 bits: every input bit reaches the low -/// bits that select a bucket. -#[cfg(feature = "std")] -#[inline] -fn folded_multiply(x: u64, y: u64) -> u64 { - let (low, high) = halves(u128::from(x) * u128::from(y)); - low ^ high -} - -/// The low and high 64 bits of `word`. +/// A hash table keyed by document content. #[cfg(feature = "std")] -#[inline] -fn halves(word: u128) -> (u64, u64) { - // Both halves fit by construction; the checks fold away. - let low = u64::try_from(word & u128::from(u64::MAX)).expect("masked to 64 bits"); - let high = u64::try_from(word >> 64).expect("shifted to 64 bits"); - (low, high) -} - +pub(crate) type Map = std::collections::HashMap; +/// A hash set keyed by document content. #[cfg(feature = "std")] -impl Hasher for SeededHasher { - #[inline] - fn write(&mut self, bytes: &[u8]) { - let (keys, state) = (&self.keys, self.state); - self.strings - .get_or_insert_with(|| { - let mut strings = keys.build_hasher(); - strings.write_u64(state); - strings - }) - .write(bytes); - } - - #[inline] - fn write_u8(&mut self, word: u8) { - self.write_u64(u64::from(word)); - } - - #[inline] - fn write_u16(&mut self, word: u16) { - self.write_u64(u64::from(word)); - } - - #[inline] - fn write_u32(&mut self, word: u32) { - self.write_u64(u64::from(word)); - } - - #[inline] - fn write_u64(&mut self, word: u64) { - match &mut self.strings { - None => self.state = folded_multiply(self.state ^ word, self.seed), - Some(strings) => strings.write_u64(word), - } - } - - #[inline] - fn write_u128(&mut self, word: u128) { - let (low, high) = halves(word); - self.write_u64(low); - self.write_u64(high); - } - - #[inline] - fn write_usize(&mut self, word: usize) { - self.write_u64(word as u64); - } - - #[inline] - fn write_i8(&mut self, word: i8) { - self.write_u8(word.cast_unsigned()); - } - - #[inline] - fn write_i16(&mut self, word: i16) { - self.write_u16(word.cast_unsigned()); - } - - #[inline] - fn write_i32(&mut self, word: i32) { - self.write_u32(word.cast_unsigned()); - } - - #[inline] - fn write_i64(&mut self, word: i64) { - self.write_u64(word.cast_unsigned()); - } - - #[inline] - fn write_i128(&mut self, word: i128) { - self.write_u128(word.cast_unsigned()); - } - - #[inline] - fn write_isize(&mut self, word: isize) { - self.write_usize(word.cast_unsigned()); - } - - #[inline] - fn finish(&self) -> u64 { - let state = match &self.strings { - Some(strings) => strings.finish(), - None => self.state, - }; - folded_multiply(state, self.seed.rotate_left(32) | 1) - } -} +pub(crate) type Set = std::collections::HashSet; /// What [`Map`] and [`Set`] require of their keys in this build. #[cfg(feature = "std")] diff --git a/src/xmldsig/sign/single_execution_tests.rs b/src/xmldsig/sign/single_execution_tests.rs index d0b9efed..68efe5e2 100644 --- a/src/xmldsig/sign/single_execution_tests.rs +++ b/src/xmldsig/sign/single_execution_tests.rs @@ -326,3 +326,32 @@ fn a_writer_key_info_that_drops_a_referenced_id_is_refused() { assert!(result.is_err(), "{references}: {result:?}"); } } + +#[test] +fn a_known_dependency_still_lets_later_node_set_steps_drop_the_signature() { + // A value-reading XPath excludes KeyInfo, leaving a dependency that lost + // its provenance; the Enveloped step after it still removes the + // Reference's own DigestValue and SignatureValue. Stopping the chain at the + // first known dependency would keep them and report a false cycle. + let xpath = transform( + XPATH, + "not(ancestor-or-self::ds:KeyInfo) and string-length(string(.)) >= 0", + ); + let xml = template( + &reference( + "", + &[xpath, transform(ENVELOPED, ""), transform(EXCLUSIVE, "")].concat(), + ), + "", + ) + .replace( + "", + "key", + ); + let policy = crate::policy::SigningPolicy::default(); + let signed = SignContext::new(&FixedKey) + .policy(policy.clone()) + .sign_template(&xml) + .expect("Enveloped removes the signature's own values from the dependencies"); + assert_digests_match_final_document(&signed, &policy); +} diff --git a/src/xmldsig/transforms.rs b/src/xmldsig/transforms.rs index 9a771332..4180add7 100644 --- a/src/xmldsig/transforms.rs +++ b/src/xmldsig/transforms.rs @@ -2665,7 +2665,9 @@ fn known_dependent Date: Mon, 2 Nov 2026 15:47:38 +0200 Subject: [PATCH 3/4] build(bench): lock competitor harnesses to structured-asn1 0.20.2 The product now requires structured-asn1 0.20.2, so the competitor harness lockfile moves with it. --- benchmarks/competitors/Cargo.lock | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/benchmarks/competitors/Cargo.lock b/benchmarks/competitors/Cargo.lock index 8237309f..dfe071e1 100644 --- a/benchmarks/competitors/Cargo.lock +++ b/benchmarks/competitors/Cargo.lock @@ -2453,12 +2453,13 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "structured-asn1" -version = "0.20.0" +version = "0.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2327671666df0e22a40a20971a6eba300aeae791e363d71b074444e4c7f796d2" +checksum = "23852bb67534243c15bc4f14aa5b859c0fa75d5d25806df4d908380445b62a24" dependencies = [ "allocator-api2 0.2.21", "allocator-api2 0.4.0", + "foldhash", "hashbrown", "miniz_oxide", ] @@ -2923,7 +2924,7 @@ dependencies = [ [[package]] name = "xml-sec" -version = "0.3.0" +version = "0.4.1" dependencies = [ "aes 0.9.3", "aes-gcm", From 3c8b4f721cd52869d5386c6f1cf78afac4704b38 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 16:29:35 +0200 Subject: [PATCH 4/4] fix(xmldsig): keep values an XPath reads as signing dependencies A value-reading XPath recorded as dependencies only the tracked values it excluded; a value it read and kept stayed a plain visible node, so a later Enveloped step removed it and the dependency with it. The XPath had selected nodes by the empty placeholder, a verifier selects them by the filled value, and the signed digest did not verify. Every tracked value such an XPath can read now stays a dependency, visible or not, so a Reference whose selection reads its own DigestValue is refused as a cycle. The regression test covers a predicate on the DigestValue length and a string(.) test, which reads it through every ancestor's string value; it replaces a test that expected the latter to sign. --- src/xmldsig/sign/single_execution_tests.rs | 55 ++++++++++++---------- src/xmldsig/transforms.rs | 14 +++--- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/xmldsig/sign/single_execution_tests.rs b/src/xmldsig/sign/single_execution_tests.rs index 68efe5e2..a2500738 100644 --- a/src/xmldsig/sign/single_execution_tests.rs +++ b/src/xmldsig/sign/single_execution_tests.rs @@ -328,30 +328,35 @@ fn a_writer_key_info_that_drops_a_referenced_id_is_refused() { } #[test] -fn a_known_dependency_still_lets_later_node_set_steps_drop_the_signature() { - // A value-reading XPath excludes KeyInfo, leaving a dependency that lost - // its provenance; the Enveloped step after it still removes the - // Reference's own DigestValue and SignatureValue. Stopping the chain at the - // first known dependency would keep them and report a false cycle. - let xpath = transform( - XPATH, - "not(ancestor-or-self::ds:KeyInfo) and string-length(string(.)) >= 0", - ); - let xml = template( - &reference( +fn a_value_read_by_xpath_stays_a_dependency_after_enveloped_removes_it() { + // The XPath selects nodes by the length of the Reference's own + // DigestValue, which it can still see; Enveloped removes that value from + // the output afterwards. The selection was made on the empty placeholder + // and a verifier makes it on the filled value, so the Reference depends on + // its own DigestValue: signing must refuse, not digest the placeholder + // selection. A `string(.)` test reads it too, through the string value of + // every ancestor. + for predicate in [ + "string-length(string(//ds:DigestValue)) < 10", + "not(ancestor-or-self::ds:KeyInfo) and string-length(string(.)) >= 0", + ] { + let xpath = transform(XPATH, &format!("{predicate}")); + let xml = template( + &reference( + "", + &[xpath, transform(ENVELOPED, ""), transform(EXCLUSIVE, "")].concat(), + ), "", - &[xpath, transform(ENVELOPED, ""), transform(EXCLUSIVE, "")].concat(), - ), - "", - ) - .replace( - "", - "key", - ); - let policy = crate::policy::SigningPolicy::default(); - let signed = SignContext::new(&FixedKey) - .policy(policy.clone()) - .sign_template(&xml) - .expect("Enveloped removes the signature's own values from the dependencies"); - assert_digests_match_final_document(&signed, &policy); + ) + .replace( + "", + "key", + ); + let result = SignContext::new(&FixedKey).sign_template(&xml); + let refusal = format!("{result:?}"); + assert!( + result.is_err() && refusal.contains("dependency cycle"), + "{predicate}: {refusal}" + ); + } } diff --git a/src/xmldsig/transforms.rs b/src/xmldsig/transforms.rs index 4180add7..66cc6f02 100644 --- a/src/xmldsig/transforms.rs +++ b/src/xmldsig/transforms.rs @@ -2288,16 +2288,18 @@ impl DependencyTracking { Some(node) => nodes.contains_tree(node).map_err(TransformError::from)?, None => false, }; - if remains_visible { - active_nodes.push(tracked); - } else if preserve_excluded_as_opaque { + if preserve_excluded_as_opaque { // SXD exposes output membership but not which source // values a predicate coerced. Structural selection does // not depend on mutable DigestValue text, but a value - // scan may control another node's inclusion, so absence - // is not proof of independence in that case. + // scan may control another node's inclusion, so neither + // absence nor a later Enveloped removal proves + // independence: the read stays a dependency either way. self.opaque_dependencies.insert(tracked.index); - } else { + } + if remains_visible { + active_nodes.push(tracked); + } else if !preserve_excluded_as_opaque { self.dormant_indexes.insert(tracked.index); } }