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/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", 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..a28ec558 100644 --- a/src/collections.rs +++ b/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 = std::collections::HashMap; +/// A hash set keyed by document content. +#[cfg(feature = "std")] +pub(crate) type Set = std::collections::HashSet; + /// 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 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 +78,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 +122,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/sign/single_execution_tests.rs b/src/xmldsig/sign/single_execution_tests.rs index d0b9efed..a2500738 100644 --- a/src/xmldsig/sign/single_execution_tests.rs +++ b/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!("{predicate}")); + let xml = template( + &reference( + "", + &[xpath, transform(ENVELOPED, ""), transform(EXCLUSIVE, "")].concat(), + ), + "", + ) + .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 1204540a..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); } } @@ -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 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 +5264,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()) } }