perf: hash arena identities without SipHash in content tables

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",

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<String> = HashSet::new();
++    let mut id_values: HashSet<String> = HashSet::default();
+     let mut idref_values: Vec<String> = Vec::new();
+
+     // Check root element name against DOCTYPE
 @@ -2311,21 +2602,19 @@
          }
      });

src/collections.rs

@@ -13,11 +13,26 @@ pub(crate) use alloc::collections::{BTreeMap as Map, BTreeSet as Set};
 pub(crate) use alloc::collections::{btree_map as map, btree_set as set};
 #[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};
 
+/// Builds the hasher of one content-keyed table.
+///
+/// A document controls the strings these tables hash and chooses which arena
+/// 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(crate) type SeededState = structured_asn1::hash::RandomState;
+
+/// A hash table keyed by document content.
+#[cfg(feature = "std")]
+pub(crate) type Map<K, V> = std::collections::HashMap<K, V, SeededState>;
+/// A hash set keyed by document content.
+#[cfg(feature = "std")]
+pub(crate) type Set<T> = std::collections::HashSet<T, SeededState>;
+
 /// What [`Map`] and [`Set`] require of their keys in this build.
 #[cfg(feature = "std")]
 pub(crate) trait Key: core::hash::Hash + Eq {}
@@ -47,7 +62,7 @@ pub(crate) trait Table: Sized {
 #[cfg(feature = "std")]
 impl<K: Key, V> Table for Map<K, V> {
     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 +78,7 @@ impl<K: Key, V> Table for Map<K, V> {
 #[cfg(feature = "std")]
 impl<T: Key> Table for Set<T> {
     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 +122,6 @@ impl<T: Key> Table for Set<T> {
         self.len()
     }
 }
+
+#[cfg(all(test, feature = "std"))]
+mod tests;

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<Item = u64>, bits: u32) -> usize {
+    let mask = (1_u64 << bits) - 1;
+    keys.map(|key| state.hash_one(key) & mask)
+        .collect::<std::collections::BTreeSet<_>>()
+        .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::<std::collections::BTreeSet<_>>();
+    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"));
+}

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.

src/operation.rs

@@ -347,9 +347,9 @@ impl<P, B> OperationExecutionContext<P, B> {
             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),
         }

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<String> = HashSet::new();
+    let mut id_values: HashSet<String> = HashSet::default();
     let mut idref_values: Vec<String> = Vec::new();
 
     // Check root element name against DOCTYPE

src/sxd_document/lazy_hash_map.rs

@@ -33,7 +33,7 @@ where
     pub fn insert(&mut self, key: K, val: V) -> Option<V> {
         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))

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())
     }
 }
 

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<T>(self) -> Vec<T> {
         Vec::new()

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.

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

src/xml/dom/storage/names.rs

@@ -75,7 +75,7 @@ impl<T: AsRef<str>> structured_asn1::memory::HashWork for Name<T> {
 impl NameStorage for Native {
     type Names<K: AsRef<str>, V> = Map<Name<K>, V>;
     fn names<K: AsRef<str>, V>(&self) -> Self::Names<K, V> {
-        Map::new()
+        Map::default()
     }
 }
 

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<NamespaceKey<'a>, usize>;
     fn counts<'a>(&self, capacity: usize) -> Result<Self::Counts<'a>, ParseError> {
-        let mut counts = Map::new();
+        let mut counts = Map::default();
         counts
             .try_reserve_entries(capacity)
             .map_err(|_| ParseError::AllocationFailed)?;

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<HashSet<usize>>,
+    dependencies: Vec<crate::collections::Set<usize>>,
     levels: Vec<Vec<usize>>,
     /// No `Reference` can observe the target `KeyInfo` or a `Reference` that
     /// gains a generated transform.

src/xmldsig/sign/single_execution_tests.rs

@@ -326,3 +326,37 @@ fn a_writer_key_info_that_drops_a_referenced_id_is_refused() {
         assert!(result.is_err(), "{references}: {result:?}");
     }
 }
+
+#[test]
+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!("<ds:XPath>{predicate}</ds:XPath>"));
+        let xml = template(
+            &reference(
+                "",
+                &[xpath, transform(ENVELOPED, ""), transform(EXCLUSIVE, "")].concat(),
+            ),
+            "",
+        )
+        .replace(
+            "<ds:SignatureValue/>",
+            "<ds:SignatureValue/><ds:KeyInfo><ds:KeyName>key</ds:KeyName></ds:KeyInfo>",
+        );
+        let result = SignContext::new(&FixedKey).sign_template(&xml);
+        let refusal = format!("{result:?}");
+        assert!(
+            result.is_err() && refusal.contains("dependency cycle"),
+            "{predicate}: {refusal}"
+        );
+    }
+}

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);
             }
         }
@@ -2452,8 +2454,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 {
@@ -2665,7 +2667,9 @@ fn known_dependent<S: TransformScratch, D: DomStorage, I: Instruction, F: Transf
                 .canonical_positions
                 .as_ref()
                 .is_some_and(|positions| !positions.is_empty()));
-    if !reads_tracked && tracking.opaque_dependencies.is_empty() {
+    // A node-set step still runs: Enveloped or a filter can drop tracked nodes
+    // the dependencies would otherwise keep, such as the Signature's own values.
+    if !reads_content || (!reads_tracked && tracking.opaque_dependencies.is_empty()) {
         return Ok(None);
     }
     tracking.discard_provenance();
@@ -2891,7 +2895,7 @@ fn map_transform_xml_decode_error(error: crate::encoding::XmlEncodingError) -> T
 #[cfg(feature = "xmldsig-sign")]
 fn dependency_indexes(tracking: Option<DependencyTracking>) -> Set<usize> {
     let Some(tracking) = tracking else {
-        return Set::new();
+        return Set::default();
     };
     tracking
         .active_nodes
@@ -5260,7 +5264,7 @@ mod tests {
         )
         .unwrap();
 
-        assert_eq!(output.dependencies, Set::from([7]));
+        assert_eq!(output.dependencies, Set::from_iter([7]));
     }
 
     #[cfg(feature = "xmldsig-sign")]