From 8fa68489d7132881a5197db97b2e075c9788d2f8 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 1 Nov 2026 22:26:12 +0200 Subject: [PATCH 1/4] feat(provider)!: admit digest state and type its refusals - begin_digest receives the operation's Memory; charged_digest_state admits the whole block first, allocates fallibly and keeps the charge for the life of the state - finalize writes to an inline buffer, so no output allocation exists - ProviderError::Resource carries a refusal unchanged to the terminal result; sessions, CMS, XAdES and detached payloads charge the caller's execution - A refusal in a Manifest Reference ends the operation instead of reading as an invalid Reference with more work after it - One DigestOutput replaces the copies in CMS and XAdES BREAKING CHANGE: CryptoProvider::begin_digest and digest take a Memory, DigestState::finalize writes to a caller buffer, and compute_digest_with_provider takes a Memory and returns DigestOutput. Closes #286 --- crates/xml-sec-xades/src/binary.rs | 24 +--- docs/crypto-providers.md | 17 ++- fuzz/Cargo.toml | 1 + fuzz/src/lib.rs | 20 ++- src/key_establishment.rs | 1 + src/provider.rs | 121 ++++++++++++++--- src/provider/aws_lc.rs | 21 ++- src/provider/aws_lc/tests.rs | 2 +- src/provider/pkcs11.rs | 47 ++++++- src/xmldsig/cms.rs | 23 +--- src/xmldsig/digest.rs | 97 +++++++++++--- src/xmldsig/dstu4145.rs | 22 ++- src/xmldsig/keys.rs | 16 ++- src/xmldsig/mod.rs | 6 +- src/xmldsig/parse.rs | 50 +++++-- src/xmldsig/sign.rs | 17 ++- src/xmldsig/transforms.rs | 16 ++- src/xmldsig/verify.rs | 47 ++++--- src/xmldsig/verify/session/detached.rs | 56 ++++++-- src/xmldsig/verify/session/lookup.rs | 169 +++++++++++++++++++++--- src/xmldsig/verify/tests.rs | 29 ++++ src/xmlenc/decrypt.rs | 3 +- src/xmlenc/encrypt.rs | 3 +- tests/aws_lc_provider.rs | 22 ++- tests/cms_timestamp.rs | 33 ++++- tests/detached_payloads.rs | 131 ++++++++++++++++-- tests/key_establishment_policy.rs | 3 +- tests/pkcs11.rs | 48 +++++-- tests/policy_profiles.rs | 1 + tests/provider_contract.rs | 3 +- tests/rust_api.rs | 6 +- tests/session_lookups.rs | 20 ++- tests/streaming_digest.rs | 136 +++++++++++++++++-- tests/support/refusing_state.rs | 154 +++++++++++++++++++++ tools/xmlsec1/tests/process_contract.rs | 5 +- 35 files changed, 1139 insertions(+), 231 deletions(-) create mode 100644 tests/support/refusing_state.rs diff --git a/crates/xml-sec-xades/src/binary.rs b/crates/xml-sec-xades/src/binary.rs index 5cd8084f..5cbec872 100644 --- a/crates/xml-sec-xades/src/binary.rs +++ b/crates/xml-sec-xades/src/binary.rs @@ -12,6 +12,7 @@ use structured_xades::certificate::DigestProvider; use structured_xades::hashes::HashClass; use structured_xades::xml::{Access, Name}; use xml_sec::document::NodeIdentity; +pub use xml_sec::xmldsig::DigestOutput; use xml_sec::xmldsig::extension; use xml_sec::xmldsig::{DigestAlgorithm, DigestMethodResolution, SessionDigest, constant_time_eq}; @@ -104,19 +105,6 @@ pub enum DigestIdentity { Digest(DigestAlgorithm), } -/// A finished certificate digest, held inline. -#[derive(Debug, Clone, Copy)] -pub struct DigestOutput { - bytes: [u8; 64], - len: usize, -} - -impl AsRef<[u8]> for DigestOutput { - fn as_ref(&self) -> &[u8] { - &self.bytes[..self.len] - } -} - impl DigestIdentity { /// Total order key: equal keys name the same digest, so outputs can be shared. fn order_key(self) -> (u8, &'static str) { @@ -159,9 +147,8 @@ impl<'a, W: Work> Digest<'a> for Document<'_, '_, '_, '_, W> { let DigestIdentity::Digest(algorithm) = identity else { return Err(Error::ProhibitedDigest); }; - execution.charge_work(1)?; Ok(Resolution::Supported( - self.session.begin_digest(*algorithm)?, + self.session.begin_digest(*algorithm, execution)?, )) } @@ -179,12 +166,7 @@ impl<'a, W: Work> Digest<'a> for Document<'_, '_, '_, '_, W> { state: SessionDigest<'a>, execution: &Q, ) -> Result { - let mut output = DigestOutput { - bytes: [0; 64], - len: state.output_len(), - }; - state.finish(&mut output.bytes[..output.len], execution)?; - Ok(output) + Ok(state.finish_output(execution)?) } fn matches( diff --git a/docs/crypto-providers.md b/docs/crypto-providers.md index ac6d67a4..653c1e6d 100644 --- a/docs/crypto-providers.md +++ b/docs/crypto-providers.md @@ -129,9 +129,20 @@ Without it, that selection is an explicit unavailable-provider error. Every provider implements `CryptoProvider::begin_digest`, which returns a provider-owned `DigestState`: input is absorbed in order across any number of -`update` calls, and `finalize` consumes the state. The one-shot `digest` is a -provided method built on it, so an engine implements hashing once. Custom -providers implement `begin_digest`; it replaces the former required `digest`. +`update` calls, and `finalize` writes the digest to an inline buffer, once. The +one-shot `digest` is a provided method built on it, so an engine implements hashing +once. Custom providers implement `begin_digest`; it replaces the former required +`digest`. + +`begin_digest` receives the operation's `Memory`. A provider puts its state on the +heap with `charged_digest_state`, which admits the whole block first, keeps the +charge as long as the state and turns an allocator refusal into an error. A refusal +is `ProviderError::Resource`: it ends the operation with that cause and is never +reported as an unsupported algorithm or a digest mismatch. Sessions, CMS, XAdES and +detached payloads charge the caller's execution; plain signing and verification, +which hold one state per Reference, use an account of their own without a ceiling. +A PKCS#11 result passes through a buffer that `cryptoki` allocates itself: its width +is admitted first, but that one allocation cannot report a refusal. XMLDSig Reference processing streams the final transform output into this state for both signing and verification: a final node set is canonicalized straight diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index bff75450..e4786682 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -10,6 +10,7 @@ cargo-fuzz = true [dependencies] libfuzzer-sys = "0.4.13" +structured-asn1 = { version = "0.20", default-features = false, features = ["alloc", "std"] } xml-sec = { path = "..", default-features = false, features = ["xmlenc", "xslt", "xml-backends-all"] } xml-sec-config = { path = "../crates/xml-sec-config" } xml-sec-xslt = { path = "../crates/xml-sec-xslt" } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index de64ab32..08e606ea 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -449,15 +449,27 @@ pub fn provider_operations(data: &[u8]) { if !provider.supports(ProviderCapability::Digest(algorithm)) { continue; } - let mut one = provider.begin_digest(algorithm).expect("advertised digest"); + let memory = structured_asn1::memory::Memory::new(std::sync::Arc::new( + structured_asn1::memory::MemoryAccount::new(1 << 20, 1 << 20), + )); + let finish = |state: &mut dyn xml_sec::provider::DigestState| { + let mut output = [0; xml_sec::xmldsig::MAX_DIGEST_OUTPUT]; + let written = state.finalize(&mut output).expect("finalize"); + output[..written].to_vec() + }; + let mut one = provider + .begin_digest(algorithm, &memory) + .expect("advertised digest"); one.update(data).expect("digest input"); - let one = one.finalize().expect("finalize"); - let mut chunks = provider.begin_digest(algorithm).expect("advertised digest"); + let one = finish(&mut *one); + let mut chunks = provider + .begin_digest(algorithm, &memory) + .expect("advertised digest"); let split = data.len() / 2; chunks.update(&data[..split]).expect("first chunk"); chunks.update(&[]).expect("empty chunk"); chunks.update(&data[split..]).expect("last chunk"); - assert_eq!(one, chunks.finalize().expect("finalize"), "digest chunking"); + assert_eq!(one, finish(&mut *chunks), "digest chunking"); if let Some(expected) = &expected { assert_eq!(&one, expected, "provider digest disagreement"); } diff --git a/src/key_establishment.rs b/src/key_establishment.rs index 98d8c7e3..7f4d0467 100644 --- a/src/key_establishment.rs +++ b/src/key_establishment.rs @@ -331,6 +331,7 @@ mod tests { fn begin_digest( &self, _: crate::xmldsig::DigestAlgorithm, + _: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { unreachable!("no digest after failed entropy") } diff --git a/src/provider.rs b/src/provider.rs index 6583c75e..59e5ced0 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -677,6 +677,10 @@ pub enum ProviderError { /// Operating-system randomness was unavailable. #[error("operating-system random number generation failed: {0}")] Random(String), + /// The operation's account or the allocator refused storage. This is a + /// spent ceiling, never a finding about the document. + #[error("provider resource refusal: {0}")] + Resource(structured_asn1::memory::MemoryError), } /// Operational failure classes for an explicitly selected external provider. @@ -921,11 +925,69 @@ pub trait DigestState { /// Returns the selected provider's digest-update failure. fn update(&mut self, data: &[u8]) -> Result<(), ProviderError>; - /// Return the digest of all absorbed bytes. + /// Write the digest of all absorbed bytes to the start of `output` and + /// return its length. The owner finalizes once, then drops the state. /// /// # Errors /// Returns the selected provider's digest-finalization failure. - fn finalize(self: Box) -> Result, ProviderError>; + fn finalize( + &mut self, + output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result; +} + +/// A digest state with the charge for the block that holds it. +#[cfg(feature = "xmldsig")] +struct Charged { + state: T, + // Inside the block it pays for, so it releases just before the block does: + // the one moment the account is lower than the allocator, and nothing runs in it. + _charge: structured_asn1::memory::MemoryReservation, +} + +// One element, because a fallibly allocated `Box<[T; 1]>` is the only `Box` that +// stable Rust offers without `unsafe`. +#[cfg(feature = "xmldsig")] +impl DigestState for [Charged; 1] { + fn update(&mut self, data: &[u8]) -> Result<(), ProviderError> { + self[0].state.update(data) + } + + fn finalize( + &mut self, + output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result { + self[0].state.finalize(output) + } +} + +/// Move `state` to the heap under `memory`: its complete block is admitted +/// first, the allocation can fail, and the charge lasts as long as the state. +/// +/// # Errors +/// Returns [`ProviderError::Resource`] when the account or the allocator refuses. +#[cfg(feature = "xmldsig")] +pub fn charged_digest_state<'s, T: DigestState + 's>( + state: T, + memory: &structured_asn1::memory::Memory, +) -> Result, ProviderError> { + use structured_asn1::memory::MemoryError; + let charge = memory + .reserve(core::mem::size_of::>()) + .map_err(ProviderError::Resource)?; + let mut block = Vec::new(); + block + .try_reserve_exact(1) + .map_err(|_| ProviderError::Resource(MemoryError::OutOfMemory))?; + block.push(Charged { + state, + _charge: charge, + }); + let block: Box<[Charged; 1]> = match block.into_boxed_slice().try_into() { + Ok(block) => block, + Err(_) => unreachable!("the slice holds the one pushed element"), + }; + Ok(block) } /// Provider operations used by the XML Security pipelines. @@ -1108,25 +1170,34 @@ pub trait CryptoProvider: Send + Sync { } /// Start an incremental message digest; streamed input never needs to be - /// collected in memory first. + /// collected in memory first. Storage for the state is admitted in `memory` + /// before it exists, as [`charged_digest_state`] does. /// /// # Errors - /// Returns an unsupported digest or the selected provider's state-creation failure. + /// Returns an unsupported digest, a resource refusal, or the selected + /// provider's state-creation failure. #[cfg(feature = "xmldsig")] fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError>; - /// Compute the message digest of one contiguous input. + /// Compute the message digest of one contiguous input over the same + /// execution as streamed input. /// /// # Errors /// Propagates state creation, update and finalization failures unchanged. #[cfg(feature = "xmldsig")] - fn digest(&self, algorithm: DigestAlgorithm, data: &[u8]) -> Result, ProviderError> { - let mut state = self.begin_digest(algorithm)?; + fn digest( + &self, + algorithm: DigestAlgorithm, + data: &[u8], + memory: &structured_asn1::memory::Memory, + ) -> Result { + let mut state = self.begin_digest(algorithm, memory)?; state.update(data)?; - state.finalize() + crate::xmldsig::finish_digest(algorithm, &mut *state) } /// Sign bytes with an opaque key handle. @@ -1523,10 +1594,17 @@ macro_rules! rust_crypto_digest { Ok(()) } - fn finalize(self: Box) -> Result, ProviderError> { + fn finalize( + &mut self, + output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result { use sha2::Digest as _; - Ok(match *self { - $($(#[$attr])* Self::$variant(hasher) => hasher.finalize().to_vec(),)+ + Ok(match self { + $($(#[$attr])* Self::$variant(hasher) => { + let digest = hasher.finalize_reset(); + output[..digest.len()].copy_from_slice(&digest); + digest.len() + })+ }) } } @@ -2074,12 +2152,13 @@ impl CryptoProvider for RustCryptoProvider { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { #[cfg(feature = "dstu4145")] if crate::xmldsig::dstu4145::is_ukrainian_digest(algorithm) { - return crate::xmldsig::dstu4145::begin_digest(algorithm); + return crate::xmldsig::dstu4145::begin_digest(algorithm, memory); } - Ok(Box::new(RustCryptoDigest::new(algorithm))) + charged_digest_state(RustCryptoDigest::new(algorithm), memory) } #[cfg(feature = "xmldsig")] @@ -3332,8 +3411,8 @@ mod tests { ), ] { assert_eq!( - RUST_CRYPTO_PROVIDER - .digest(algorithm, b"abc") + &*RUST_CRYPTO_PROVIDER + .digest(algorithm, b"abc", &crate::xmldsig::digest::digest_memory()) .expect("compiled digest must be supported"), expected ); @@ -3352,10 +3431,11 @@ mod tests { fn update(&mut self, data: &[u8]) -> Result<(), ProviderError> { self.0.update(data) } - fn finalize(self: Box) -> Result, ProviderError> { - let mut digest = self.0.finalize()?; - digest.push(0); - Ok(digest) + fn finalize( + &mut self, + output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result { + Ok(self.0.finalize(output)? + 1) } } @@ -3393,6 +3473,7 @@ mod tests { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { if self.reject_digest == Some(algorithm) { return Err(ProviderError::Unsupported { @@ -3400,7 +3481,7 @@ mod tests { algorithm: Some(algorithm.uri().to_owned()), }); } - let state = RUST_CRYPTO_PROVIDER.begin_digest(algorithm)?; + let state = RUST_CRYPTO_PROVIDER.begin_digest(algorithm, memory)?; Ok(if self.extra_digest_byte { Box::new(ExtraByteDigest(state)) } else { diff --git a/src/provider/aws_lc.rs b/src/provider/aws_lc.rs index 63fe8c59..3fe23ad7 100644 --- a/src/provider/aws_lc.rs +++ b/src/provider/aws_lc.rs @@ -57,16 +57,26 @@ fn digest_algorithm(algorithm: DigestAlgorithm) -> Option<&'static digest::Algor }) } -struct AwsLcDigest(digest::Context); +// `finish` consumes the context, which `finalize` receives by reference. +struct AwsLcDigest(Option); impl super::DigestState for AwsLcDigest { fn update(&mut self, data: &[u8]) -> Result<(), ProviderError> { - self.0.update(data); + self.0 + .as_mut() + .expect("the owner finalizes last") + .update(data); Ok(()) } - fn finalize(self: Box) -> Result, ProviderError> { - Ok(self.0.finish().as_ref().to_vec()) + fn finalize( + &mut self, + output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result { + let digest = self.0.take().expect("the owner finalizes once").finish(); + let digest = digest.as_ref(); + output[..digest.len()].copy_from_slice(digest); + Ok(digest.len()) } } @@ -201,10 +211,11 @@ impl CryptoProvider for AwsLcFipsProvider { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { let native = digest_algorithm(algorithm) .ok_or_else(|| unsupported(ProviderCapability::Digest(algorithm)))?; - Ok(Box::new(AwsLcDigest(digest::Context::new(native)))) + super::charged_digest_state(AwsLcDigest(Some(digest::Context::new(native))), memory) } fn sign( diff --git a/src/provider/aws_lc/tests.rs b/src/provider/aws_lc/tests.rs index 76409f2c..b076070d 100644 --- a/src/provider/aws_lc/tests.rs +++ b/src/provider/aws_lc/tests.rs @@ -32,7 +32,7 @@ fn optional_legacy_capabilities_fail_without_another_engine() { for digest in [DigestAlgorithm::Md5, DigestAlgorithm::Ripemd160] { assert!(!AwsLcFipsProvider.supports(ProviderCapability::Digest(digest))); assert!(matches!( - AwsLcFipsProvider.digest(digest, b"test"), + AwsLcFipsProvider.digest(digest, b"test", &crate::xmldsig::digest::digest_memory()), Err(ProviderError::Unsupported { .. }) )); } diff --git a/src/provider/pkcs11.rs b/src/provider/pkcs11.rs index a4e7d876..b0e6d751 100644 --- a/src/provider/pkcs11.rs +++ b/src/provider/pkcs11.rs @@ -277,10 +277,17 @@ fn oaep_error(error: Error, capability: ProviderCapability<'_>) -> ProviderError } } +const fn output_capacity() -> usize { + crate::xmldsig::MAX_DIGEST_OUTPUT +} + /// A multi-part digest holding the serialized session from init to final. struct Pkcs11Digest<'a> { session: MutexGuard<'a, Session>, active: bool, + /// Pays for the result buffer, which only cryptoki can allocate. + memory: structured_asn1::memory::Memory, + width: usize, } impl super::DigestState for Pkcs11Digest<'_> { @@ -294,11 +301,31 @@ impl super::DigestState for Pkcs11Digest<'_> { }) } - fn finalize(mut self: Box) -> Result, ProviderError> { + fn finalize( + &mut self, + output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result { + // The module's reply reaches us through cryptoki's own buffer; it has + // no call that writes to caller storage, so the width is admitted here. + // A refusal leaves the operation active for Drop to end. + let _temporary = self + .memory + .reserve(self.width) + .map_err(ProviderError::Resource)?; self.active = false; - self.session + let digest = self + .session .digest_final() - .map_err(|error| map_error(&error)) + .map_err(|error| map_error(&error))?; + output + .get_mut(..digest.len()) + .ok_or(ProviderError::InvalidOutputSize { + operation: ProviderOperation::Digest, + expected: output_capacity(), + actual: digest.len(), + })? + .copy_from_slice(&digest); + Ok(digest.len()) } } @@ -812,6 +839,7 @@ impl CryptoProvider for Pkcs11Provider { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { self.require_capability(ProviderCapability::Digest(algorithm))?; let mechanism = digest_mechanism(algorithm) @@ -820,10 +848,15 @@ impl CryptoProvider for Pkcs11Provider { session .digest_init(&mechanism) .map_err(|error| map_error(&error))?; - Ok(Box::new(Pkcs11Digest { - session, - active: true, - })) + super::charged_digest_state( + Pkcs11Digest { + session, + active: true, + memory: memory.clone(), + width: algorithm.output_len(), + }, + memory, + ) } fn sign( &self, diff --git a/src/xmldsig/cms.rs b/src/xmldsig/cms.rs index 5b85b782..7ce29794 100644 --- a/src/xmldsig/cms.rs +++ b/src/xmldsig/cms.rs @@ -23,6 +23,7 @@ use structured_pkix::AlgorithmIdentifier; use structured_pkix::algorithm::digest::{Algorithm, AsnDigest}; use structured_pkix::path::WorkingKey; +pub use super::DigestOutput; use super::pkix::CertificateSession; use super::verify::SessionDigest; use super::{DigestAlgorithm, DsigError, X509ChainError}; @@ -110,19 +111,6 @@ impl From for CmsProviderError { } } -/// A finished digest, held inline. -#[derive(Debug, Clone, Copy)] -pub struct DigestOutput { - bytes: [u8; 64], - len: usize, -} - -impl AsRef<[u8]> for DigestOutput { - fn as_ref(&self) -> &[u8] { - &self.bytes[..self.len] - } -} - impl<'a> Digest<'a> for CmsProvider<'_> { type Identity = DigestAlgorithm; type State = SessionDigest<'a>; @@ -148,11 +136,11 @@ impl<'a> Digest<'a> for CmsProvider<'_> { identity: &DigestAlgorithm, execution: &Q, ) -> Result>, CmsProviderError> { - execution.charge_work(1)?; Ok(Resolution::Supported(SessionDigest::begin( self.provider, self.policy, *identity, + execution, )?)) } @@ -170,12 +158,7 @@ impl<'a> Digest<'a> for CmsProvider<'_> { state: SessionDigest<'a>, execution: &Q, ) -> Result { - let mut output = DigestOutput { - bytes: [0; 64], - len: state.output_len(), - }; - state.finish(&mut output.bytes[..output.len], execution)?; - Ok(output) + Ok(state.finish_output(execution)?) } fn matches( diff --git a/src/xmldsig/digest.rs b/src/xmldsig/digest.rs index e56809e5..33567bfa 100644 --- a/src/xmldsig/digest.rs +++ b/src/xmldsig/digest.rs @@ -194,50 +194,96 @@ impl DigestAlgorithm { /// or returns an output width inconsistent with the selected algorithm. #[must_use] pub fn compute_digest(algorithm: DigestAlgorithm, data: &[u8]) -> Vec { - compute_digest_with_provider(crate::provider::default_provider(), algorithm, data) - .expect("default provider advertises every XMLDSig digest") + compute_digest_with_provider( + crate::provider::default_provider(), + algorithm, + data, + &digest_memory(), + ) + .expect("default provider advertises every XMLDSig digest") + .to_vec() } -/// Compute a digest with an explicitly selected provider. +/// An account for digests without an operation account of their own. It has +/// no ceiling because one state lives per call or per Reference, whose number +/// policy bounds; an allocator refusal still comes back typed. +pub(crate) fn digest_memory() -> structured_asn1::memory::Memory { + structured_asn1::memory::Memory::new(alloc::sync::Arc::new( + structured_asn1::memory::MemoryAccount::new(usize::MAX, usize::MAX), + )) +} + +/// Compute a digest with an explicitly selected provider, under `memory`. /// /// # Errors -/// Returns the provider's capability, update, or finalization error, or an error -/// if the finalized digest has the wrong width for the selected algorithm. +/// Returns the provider's capability, update, or finalization error, a resource +/// refusal, or an error if the finalized digest has the wrong width for the +/// selected algorithm. pub fn compute_digest_with_provider( provider: &dyn crate::provider::CryptoProvider, algorithm: DigestAlgorithm, data: &[u8], -) -> Result, crate::provider::ProviderError> { - let mut state = begin_digest(provider, algorithm)?; + memory: &structured_asn1::memory::Memory, +) -> Result { + let mut state = begin_digest(provider, algorithm, memory)?; state.update(data)?; - finish_digest(algorithm, state) + finish_digest(algorithm, &mut *state) } /// Start an incremental digest after checking the advertised capability. pub(crate) fn begin_digest<'p>( provider: &'p dyn crate::provider::CryptoProvider, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, crate::provider::ProviderError> { provider.require_capability(crate::provider::ProviderCapability::Digest(algorithm))?; - provider.begin_digest(algorithm) + provider.begin_digest(algorithm, memory) +} + +/// The longest digest any `XMLDSig` algorithm produces, in bytes. +pub const MAX_DIGEST_OUTPUT: usize = 64; + +/// A finished digest, held inline. +#[derive(Debug, Clone, Copy)] +pub struct DigestOutput { + bytes: [u8; MAX_DIGEST_OUTPUT], + len: usize, +} + +impl core::ops::Deref for DigestOutput { + type Target = [u8]; + + fn deref(&self) -> &[u8] { + &self.bytes[..self.len] + } +} + +impl AsRef<[u8]> for DigestOutput { + fn as_ref(&self) -> &[u8] { + self + } } /// Finish an incremental digest. The facade, not an interchangeable provider, /// owns the output width of each `XMLDSig` algorithm. +/// +/// # Errors +/// Returns the provider's finalization failure or a wrong output width. pub(crate) fn finish_digest( algorithm: DigestAlgorithm, - state: Box, -) -> Result, crate::provider::ProviderError> { - let digest = state.finalize()?; + state: &mut (dyn crate::provider::DigestState + '_), +) -> Result { + let mut bytes = [0; MAX_DIGEST_OUTPUT]; + let len = state.finalize(&mut bytes)?; let expected = algorithm.output_len(); - if digest.len() != expected { + if len != expected { return Err(crate::provider::ProviderError::InvalidOutputSize { operation: crate::provider::ProviderOperation::Digest, expected, - actual: digest.len(), + actual: len, }); } - Ok(digest) + Ok(DigestOutput { bytes, len }) } /// Constant-time comparison of two byte slices. @@ -268,8 +314,12 @@ mod tests { fn update(&mut self, _: &[u8]) -> Result<(), crate::provider::ProviderError> { Ok(()) } - fn finalize(self: Box) -> Result, crate::provider::ProviderError> { - Ok(self.0) + fn finalize( + &mut self, + output: &mut [u8; super::MAX_DIGEST_OUTPUT], + ) -> Result { + output[..self.0.len()].copy_from_slice(&self.0); + Ok(self.0.len()) } } @@ -302,6 +352,7 @@ mod tests { fn begin_digest( &self, _algorithm: DigestAlgorithm, + _memory: &structured_asn1::memory::Memory, ) -> Result, crate::provider::ProviderError> { if let Some(output) = &self.output { @@ -398,7 +449,8 @@ mod tests { compute_digest_with_provider( &RejectingDigestProvider { output: None }, DigestAlgorithm::Sha256, - b"x" + b"x", + &digest_memory(), ), Err(crate::provider::ProviderError::Unsupported { .. }) )); @@ -416,6 +468,7 @@ mod tests { }, DigestAlgorithm::Sha256, b"x", + &digest_memory(), ), Err(crate::provider::ProviderError::InvalidOutputSize { operation: crate::provider::ProviderOperation::Digest, @@ -426,14 +479,16 @@ mod tests { } assert_eq!( - compute_digest_with_provider( + &*compute_digest_with_provider( &RejectingDigestProvider { output: Some(vec![7_u8; 32]), }, DigestAlgorithm::Sha256, b"x", - ), - Ok(vec![7_u8; 32]), + &digest_memory(), + ) + .expect("the width is right"), + &[7_u8; 32], ); } diff --git a/src/xmldsig/dstu4145.rs b/src/xmldsig/dstu4145.rs index 6e55b0c4..86835c91 100644 --- a/src/xmldsig/dstu4145.rs +++ b/src/xmldsig/dstu4145.rs @@ -29,15 +29,26 @@ fn hash(algorithm: DigestAlgorithm) -> Option> { } } -struct UkrainianDigest(State<'static>); +// `State::finalize` consumes the state, which `finalize` receives by reference. +struct UkrainianDigest(Option>); impl DigestState for UkrainianDigest { fn update(&mut self, data: &[u8]) -> Result<(), ProviderError> { - self.0.update(data).map_err(|_| digest_refusal()) + self.0 + .as_mut() + .ok_or_else(digest_refusal)? + .update(data) + .map_err(|_| digest_refusal()) } - fn finalize(self: Box) -> Result, ProviderError> { - Ok(self.0.finalize().as_ref().to_vec()) + fn finalize( + &mut self, + output: &mut [u8; super::MAX_DIGEST_OUTPUT], + ) -> Result { + let digest = self.0.take().ok_or_else(digest_refusal)?.finalize(); + let digest = digest.as_ref(); + output[..digest.len()].copy_from_slice(digest); + Ok(digest.len()) } } @@ -70,10 +81,11 @@ pub(crate) const fn is_ukrainian_digest(algorithm: DigestAlgorithm) -> bool { /// Start the incremental digest of a Ukrainian `algorithm`. pub(crate) fn begin_digest( algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { let identity = hash(algorithm).ok_or_else(digest_refusal)?; let state = State::new(identity).map_err(|_| digest_refusal())?; - Ok(Box::new(UkrainianDigest(state))) + crate::provider::charged_digest_state(UkrainianDigest(Some(state)), memory) } /// The numeric domain written in `curve`, for the crate's own validation. diff --git a/src/xmldsig/keys.rs b/src/xmldsig/keys.rs index 60df234d..3a717fb4 100644 --- a/src/xmldsig/keys.rs +++ b/src/xmldsig/keys.rs @@ -2749,8 +2749,11 @@ mod tests { ); } let signed = der::asn1::Any::new(der::Tag::Sequence, body)?.to_der()?; - let digest = crate::provider::default_provider() - .digest(super::super::DigestAlgorithm::Sha1, &signed)?; + let digest = crate::provider::default_provider().digest( + super::super::DigestAlgorithm::Sha1, + &signed, + &super::super::digest::digest_memory(), + )?; // Test-certificate generation only; production dispatch stays in CryptoProvider. let signature: dsa::Signature = key.sign_prehash_with_rng( &mut ChaCha20Rng::from_seed([u8::from(root); 32]), @@ -3011,6 +3014,7 @@ mod tests { fn begin_digest( &self, algorithm: super::super::DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, crate::provider::ProviderError> { if algorithm == super::super::DigestAlgorithm::Sha512 @@ -3021,7 +3025,7 @@ mod tests { algorithm: Some(algorithm.uri().to_owned()), }); } - crate::provider::default_provider().begin_digest(algorithm) + crate::provider::default_provider().begin_digest(algorithm, memory) } fn sign( @@ -4991,7 +4995,11 @@ mod tests { .der() .to_vec(); let digest = crate::provider::default_provider() - .digest(super::super::DigestAlgorithm::Sha256, &unrelated) + .digest( + super::super::DigestAlgorithm::Sha256, + &unrelated, + &super::super::digest::digest_memory(), + ) .expect("SHA-256 selector digest must be available"); let key_info_xml = format!( "CN=Test Key rsa-2048,O=XML Security Library (http://www.aleksey.com/xmlsec),ST=California,C=US{}", diff --git a/src/xmldsig/mod.rs b/src/xmldsig/mod.rs index 81b62ea8..e52d2954 100644 --- a/src/xmldsig/mod.rs +++ b/src/xmldsig/mod.rs @@ -92,7 +92,11 @@ mod xpath; #[cfg(feature = "xmldsig-sign")] pub use builder::{ReferenceBuilder, SignatureBuilder, SignatureBuilderError}; -pub use digest::{DigestAlgorithm, compute_digest, compute_digest_with_provider, constant_time_eq}; +pub(crate) use digest::finish_digest; +pub use digest::{ + DigestAlgorithm, DigestOutput, MAX_DIGEST_OUTPUT, compute_digest, compute_digest_with_provider, + constant_time_eq, +}; pub use keys::{ DefaultKeyResolver, HmacSha1VerificationKey, HmacVerificationKey, InspectedKeyCandidateBudget, KeyResolutionError, KeyResolverConfig, VerificationKey, diff --git a/src/xmldsig/parse.rs b/src/xmldsig/parse.rs index 077cd99f..f74092bb 100644 --- a/src/xmldsig/parse.rs +++ b/src/xmldsig/parse.rs @@ -3537,16 +3537,27 @@ fn source_digest_matches( .memory() .charge_work(source.len() as u64 + expected.len() as u64) .map_err(resource)?; - let _output = session - .memory() - .reserve(algorithm.output_len()) - .map_err(resource)?; - // Built-in providers return the fixed-width digest; opaque custom-provider - // internals remain that provider's admitted-work responsibility. - let actual = compute_digest_with_provider(provider, algorithm, source)?; + // Opaque custom-provider internals remain that provider's admitted-work + // responsibility; the state and the inline output are admitted here. + let actual = compute_digest_with_provider(provider, algorithm, source, session.memory()) + .map_err(|error| selector_digest_error(error, resource))?; Ok(constant_time_eq(&actual, expected)) } +/// The certificate account's own ceilings read as this session's limits. +fn selector_digest_error( + error: crate::provider::ProviderError, + limit: impl FnOnce(structured_asn1::memory::MemoryError) -> ParseError, +) -> ParseError { + use structured_asn1::memory::MemoryError; + match error { + crate::provider::ProviderError::Resource( + reason @ (MemoryError::LimitExceeded | MemoryError::WorkExceeded), + ) => limit(reason), + error => error.into(), + } +} + fn ensure_x509_data_entry_budget(info: &X509DataInfo) -> Result<(), ParseError> { let total_entries = info.certificates.len() + info.subject_names.len() @@ -5138,6 +5149,27 @@ BA== } } + #[test] + fn a_selector_digest_keeps_an_allocator_refusal_apart_from_a_limit() { + // Only a configured ceiling is a policy limit; running out of memory + // is the provider's cause and must stay recognisable as such. + use crate::provider::ProviderError; + use structured_asn1::memory::MemoryError; + let limit = |_| ParseError::InvalidStructure("limit".into()); + for reason in [MemoryError::LimitExceeded, MemoryError::WorkExceeded] { + assert!(matches!( + selector_digest_error(ProviderError::Resource(reason), limit), + ParseError::InvalidStructure(_) + )); + } + for reason in [MemoryError::OutOfMemory, MemoryError::CapacityOverflow] { + assert!(matches!( + selector_digest_error(ProviderError::Resource(reason), limit), + ParseError::Provider(ProviderError::Resource(actual)) if actual == reason + )); + } + } + #[test] fn certificate_selectors_ignore_rewritten_display_metadata() { // Serial/SKI discovery must consult the source certificate, not a @@ -5509,8 +5541,10 @@ BA== crate::provider::default_provider(), DigestAlgorithm::Sha256, &certificate, + &crate::xmldsig::digest::digest_memory(), ) - .unwrap(); + .unwrap() + .to_vec(); let matching = X509DataInfo { subject_names: vec![parsed.subject_dn.clone()], issuer_serials: vec![( diff --git a/src/xmldsig/sign.rs b/src/xmldsig/sign.rs index 6d7e9531..8bff32f6 100644 --- a/src/xmldsig/sign.rs +++ b/src/xmldsig/sign.rs @@ -807,6 +807,7 @@ impl KeyInfoWriter for X509DigestKeyInfoWriter { provider, self.digest_algorithm, &self.certificate_der, + &super::digest::digest_memory(), )?; let start = ", } @@ -370,6 +372,7 @@ impl TransformExecutionBudget { xml_parse_settings: DocumentParseSettings::default(), state: TransformChainState::default(), opc_workspace: relationship::WorkspaceBudget::default(), + digest_memory: super::digest::digest_memory(), #[cfg(feature = "xslt")] xslt: Rc::default(), } @@ -392,6 +395,7 @@ impl TransformExecutionBudget { xml_parse_settings: DocumentParseSettings::default(), state: TransformChainState::default(), opc_workspace: relationship::WorkspaceBudget::default(), + digest_memory: super::digest::digest_memory(), #[cfg(feature = "xslt")] xslt: Rc::default(), } @@ -411,6 +415,7 @@ impl TransformExecutionBudget { ), xml_parse_settings: DocumentParseSettings::default(), state: TransformChainState::default(), + digest_memory: super::digest::digest_memory(), #[cfg(feature = "xslt")] xslt: Rc::default(), } @@ -450,6 +455,7 @@ impl TransformExecutionBudget { xml_parse_settings: DocumentParseSettings::for_transform_output(resources), state: TransformChainState::default(), opc_workspace: relationship::WorkspaceBudget::new(resources.max_opc_workspace_bytes), + digest_memory: super::digest::digest_memory(), #[cfg(feature = "xslt")] xslt: Rc::new(xslt::Allowance::new(resources)), } @@ -1866,7 +1872,7 @@ impl TransformTerminal for DiscardTerminal { /// Digest of one Reference's final transform output. pub(crate) struct ReferenceDigest { - pub(crate) digest: Vec, + pub(crate) digest: super::DigestOutput, /// The hashed bytes, kept only when the caller asked for them. pub(crate) pre_digest: Option>, } @@ -1949,11 +1955,11 @@ impl<'p> DigestWriter<'p> { fn finish( mut self, algorithm: super::DigestAlgorithm, - ) -> Result<(Vec, Option>), crate::provider::ProviderError> { + ) -> Result<(super::DigestOutput, Option>), crate::provider::ProviderError> { if self.pending > 0 { self.state.update(&self.block[..self.pending])?; } - let digest = super::digest::finish_digest(algorithm, self.state)?; + let digest = super::digest::finish_digest(algorithm, &mut *self.state)?; Ok((digest, self.retained)) } } @@ -1988,7 +1994,7 @@ impl TransformTerminal for DigestTerminal<'_> { retain_pre_digest, } = self.0; let mut sink = DigestWriter::new( - super::digest::begin_digest(provider, algorithm) + super::digest::begin_digest(provider, algorithm, &budget.digest_memory) .map_err(ReferenceDigestError::Provider)?, retain_pre_digest, ); @@ -2042,7 +2048,7 @@ impl TransformTerminal for DigestTerminal<'_> { retain_pre_digest, } = self.0; let mut sink = DigestWriter::new( - super::digest::begin_digest(provider, algorithm) + super::digest::begin_digest(provider, algorithm, &budget.digest_memory) .map_err(ReferenceDigestError::Provider)?, retain_pre_digest, ); diff --git a/src/xmldsig/verify.rs b/src/xmldsig/verify.rs index 113cc7d9..b5ad248a 100644 --- a/src/xmldsig/verify.rs +++ b/src/xmldsig/verify.rs @@ -5440,18 +5440,27 @@ fn add_retrieval_binary_usage( fn manifest_reference_failure_reason( error: ReferenceProcessingError, ref_index: usize, -) -> FailureReason { +) -> Result { + // A spent ceiling says nothing about this Reference and would refuse every + // later one too, so it ends the operation instead of becoming a finding. + if matches!( + error, + ReferenceProcessingError::SharedExecution(_) + | ReferenceProcessingError::Provider(crate::provider::ProviderError::Resource(_)) + ) { + return Err(error); + } // Same Unavailable outcome as a SignedInfo Reference without caller bytes. if matches!( error, ReferenceProcessingError::UriDereference(TransformError::ExternalResourceUnavailable(_)) ) { - return FailureReason::ReferenceUnavailable { ref_index }; + return Ok(FailureReason::ReferenceUnavailable { ref_index }); } - match error.into_policy_violation() { + Ok(match error.into_policy_violation() { Ok(_) => FailureReason::ReferencePolicyViolation { ref_index }, Err(_) => FailureReason::ReferenceProcessingFailure { ref_index }, - } + }) } struct CompiledManifestReference { @@ -5551,7 +5560,7 @@ fn evaluate_manifest_reference( signature_node: Node<'_, '_>, ctx: &VerifyContext<'_>, reference_node: Option>, -) -> ReferenceResult { +) -> Result { if execution.transform_budget.remaining_c14n_output() == 0 || super::transforms::validate_relationship_chain( &reference.transforms, @@ -5564,18 +5573,18 @@ fn evaluate_manifest_reference( .check_digest_algorithm(reference.digest_method) .is_err() { - manifest_reference_invalid_result( + Ok(manifest_reference_invalid_result( reference, index, FailureReason::ReferencePolicyViolation { ref_index: index }, - ) + )) } else { match enforce_reference_policies( core::slice::from_ref(reference), ctx.policy.uris.references, ctx.allowed_transform_uris(), ) { - Ok(()) => process_reference_with_options( + Ok(()) => match process_reference_with_options( reference, resolver, signature_node, @@ -5583,26 +5592,26 @@ fn evaluate_manifest_reference( index, reference_node, execution, - ) - .unwrap_or_else(|error| { - manifest_reference_invalid_result( + ) { + Ok(result) => Ok(result), + Err(error) => Ok(manifest_reference_invalid_result( reference, index, - manifest_reference_failure_reason(error, index), - ) - }), + manifest_reference_failure_reason(error, index)?, + )), + }, Err(SignatureVerificationPipelineError::Policy(_)) => { - manifest_reference_invalid_result( + Ok(manifest_reference_invalid_result( reference, index, FailureReason::ReferencePolicyViolation { ref_index: index }, - ) + )) } - Err(_) => manifest_reference_invalid_result( + Err(_) => Ok(manifest_reference_invalid_result( reference, index, FailureReason::ReferenceProcessingFailure { ref_index: index }, - ), + )), } } } @@ -5657,7 +5666,7 @@ fn execute_manifest_reference( signature_node, ctx, resolver.node_for_node_id(compiled.reference_node_id), - ); + )?; bind_reference_evidence( &mut result, reference, diff --git a/src/xmldsig/verify/session/detached.rs b/src/xmldsig/verify/session/detached.rs index 59bafb61..69244351 100644 --- a/src/xmldsig/verify/session/detached.rs +++ b/src/xmldsig/verify/session/detached.rs @@ -196,8 +196,12 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> states.push(StreamState { algorithm: target.algorithm, chain: target.chain, - state: super::super::super::digest::begin_digest(provider, target.algorithm) - .map_err(|error| DsigError::Reference(error.into()))?, + state: super::super::super::digest::begin_digest( + provider, + target.algorithm, + self.work.memory(), + ) + .map_err(|error| DsigError::Reference(error.into()))?, }); } let base64 = targets @@ -500,7 +504,7 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> self.stop_detached(); } for signature in affected { - self.revalidate(signature); + self.revalidate(signature)?; } super::super::accept_verification_evidence( self.view, @@ -534,10 +538,11 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> /// Recompute one Signature's core status. When its last `SignedInfo` /// Reference passes, core validation succeeds (`XMLDSig` 1.1 §3.2) and the - /// authenticated Manifests are processed exactly as in verification. - fn revalidate(&mut self, signature: usize) { + /// authenticated Manifests are processed exactly as in verification. A + /// failure stays in the evidence; a spent ceiling also reaches the caller. + fn revalidate(&mut self, signature: usize) -> Result<(), DsigError> { let Ok(result) = &self.evidence.signatures[signature].result else { - return; + return Ok(()); }; let was_valid = result.status == DsigStatus::Valid; // NotProcessed is never an overall status: the first actual failure is. @@ -555,10 +560,10 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> if let Ok(result) = &mut self.evidence.signatures[signature].result { result.status = first_failure.unwrap_or(result.status); } - return; + return Ok(()); } if was_valid { - return; + return Ok(()); } let manifests = self.process_manifests(signature, reference_count); let entry = &mut self.evidence.signatures[signature]; @@ -568,8 +573,13 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> result.status = DsigStatus::Valid; result.manifest_references = manifest_references; } + Ok(()) + } + Err(error) => { + let refusal = resource_refusal(&error); + entry.result = Err(error); + refusal.map_or(Ok(()), Err) } - Err(error) => entry.result = Err(error), } } @@ -622,6 +632,23 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> } } +/// A copy of `error` when it is a spent ceiling. Those end the operation; other +/// failures belong to their Signature, whose evidence records them. +fn resource_refusal(error: &DsigError) -> Option { + match error { + DsigError::SharedExecution(cause) => Some(DsigError::SharedExecution(*cause)), + DsigError::Reference(super::ReferenceProcessingError::SharedExecution(cause)) => Some( + DsigError::Reference(super::ReferenceProcessingError::SharedExecution(*cause)), + ), + DsigError::Reference(super::ReferenceProcessingError::Provider( + provider @ crate::provider::ProviderError::Resource(_), + )) => Some(DsigError::Reference( + super::ReferenceProcessingError::Provider(provider.clone()), + )), + _ => None, + } +} + /// Charge one decoded Base64 block against the operation's Base64 output /// allowance, then feed it to every Base64 digest state. fn feed_base64_states( @@ -791,11 +818,14 @@ impl DetachedPayload<'_, '_, '_, '_, W> { None => false, }; let mut digests = Vec::with_capacity(states.len()); - for state in states { + for mut state in states { + session + .work + .charge_work(state.algorithm.output_len() as u64)?; digests.push(( state.algorithm, state.chain, - super::super::super::digest::finish_digest(state.algorithm, state.state) + super::super::super::digest::finish_digest(state.algorithm, &mut *state.state) .map_err(provider_error)?, )); } @@ -807,7 +837,7 @@ impl DetachedPayload<'_, '_, '_, '_, W> { .find(|(algorithm, chain, _)| { *algorithm == DigestAlgorithm::Sha256 && *chain == DetachedChain::Octets }) - .and_then(|(_, _, digest)| <[u8; 32]>::try_from(digest.as_slice()).ok()) + .and_then(|(_, _, digest)| <[u8; 32]>::try_from(&**digest).ok()) .ok_or(DsigError::InvalidRequest { reason: "raw SHA-256 digest is missing for the payload fingerprint", })?, @@ -822,7 +852,7 @@ impl DetachedPayload<'_, '_, '_, '_, W> { .find(|(algorithm, chain, _)| { *algorithm == target.algorithm && *chain == target.chain }) - .map(|(_, _, digest)| digest.as_slice()) + .map(|(_, _, digest)| &**digest) }; let status = digest_status(&target, digest); results.push((target, status)); diff --git a/src/xmldsig/verify/session/lookup.rs b/src/xmldsig/verify/session/lookup.rs index c140ddcc..599e0b23 100644 --- a/src/xmldsig/verify/session/lookup.rs +++ b/src/xmldsig/verify/session/lookup.rs @@ -36,14 +36,17 @@ pub struct SessionDigest<'p> { } impl<'p> SessionDigest<'p> { - /// Start `algorithm` on `provider` once `policy` permits it. - pub(crate) fn begin( + /// Start `algorithm` on `provider` once `policy` permits it, admitting the + /// state's creation and storage in `work`. The state keeps its charge. + pub(crate) fn begin( provider: &'p dyn crate::provider::CryptoProvider, policy: &crate::policy::VerificationPolicy, algorithm: DigestAlgorithm, + work: &Q, ) -> Result { policy.check_digest_algorithm(algorithm)?; - let state = super::super::super::digest::begin_digest(provider, algorithm) + work.charge_work(1)?; + let state = super::super::super::digest::begin_digest(provider, algorithm, work.memory()) .map_err(DsigError::from)?; Ok(Self { algorithm, state }) } @@ -64,14 +67,13 @@ impl<'p> SessionDigest<'p> { self.state.update(data).map_err(DsigError::from) } - /// Write the digest into an exactly sized caller buffer. Admit finalization - /// work and the provider's temporary output before invoking it; the caller - /// can keep an inline output without another heap allocation. + /// Write the digest into an exactly sized caller buffer, admitting + /// finalization work first. The output is inline, so nothing is allocated. /// /// # Errors - /// Returns an error for the wrong output width, refused work or temporary - /// capacity, provider finalization failure, or an invalid provider output size. - pub fn finish(self, output: &mut [u8], work: &Q) -> Result<(), DsigError> { + /// Returns an error for the wrong output width, refused work, provider + /// finalization failure, or an invalid provider output size. + pub fn finish(mut self, output: &mut [u8], work: &Q) -> Result<(), DsigError> { let width = self.algorithm.output_len(); if output.len() != width { return Err(DsigError::InvalidRequest { @@ -79,12 +81,25 @@ impl<'p> SessionDigest<'p> { }); } work.charge_work(width as u64)?; - let _temporary = work.memory().reserve(width)?; - let digest = super::super::super::digest::finish_digest(self.algorithm, self.state) + let digest = super::super::super::digest::finish_digest(self.algorithm, &mut *self.state) .map_err(DsigError::from)?; output.copy_from_slice(&digest); Ok(()) } + + /// Finish into an inline output of this digest's width. + /// + /// # Errors + /// Returns refused work, provider finalization failure, or an invalid + /// provider output size. + pub fn finish_output( + mut self, + work: &Q, + ) -> Result { + work.charge_work(self.algorithm.output_len() as u64)?; + super::super::super::digest::finish_digest(self.algorithm, &mut *self.state) + .map_err(DsigError::from) + } } impl VerifiedSession<'_, '_, '_, W> { @@ -167,13 +182,18 @@ impl VerifiedSession<'_, '_, '_, W> { } /// Start an incremental digest with the selected provider under the - /// compiled digest policy. + /// compiled digest policy, its state charged to `work` for its whole life. /// /// # Errors /// Returns an error for a digest denied by policy, an unavailable provider - /// capability, or failure to create the provider's incremental state. - pub fn begin_digest(&self, algorithm: DigestAlgorithm) -> Result, DsigError> { - SessionDigest::begin(self.context.provider, &self.context.policy, algorithm) + /// capability, a resource refusal, or failure to create the provider's + /// incremental state. + pub fn begin_digest( + &self, + algorithm: DigestAlgorithm, + work: &Q, + ) -> Result, DsigError> { + SessionDigest::begin(self.context.provider, &self.context.policy, algorithm, work) } /// CMS and RFC 3161 signature checks through this operation's provider, @@ -254,19 +274,22 @@ mod digest_admission_tests { fn update(&mut self, _: &[u8]) -> Result<(), crate::provider::ProviderError> { Ok(()) } - fn finalize(self: Box) -> Result, crate::provider::ProviderError> { + fn finalize( + &mut self, + _: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], + ) -> Result { self.0.set(true); - Ok(vec![0; 32]) + Ok(32) } } #[test] fn digest_refusals_never_enter_provider_finalize() { - // Wrong output shape and both admission axes fail before the backend, - // and a temporary output reservation is released on every path. - for (capacity, work, width) in [(31, 100, 32), (32, 31, 32), (32, 100, 31), (32, 100, 32)] { + // Wrong output shape and spent work fail before the backend. The output + // is inline, so no storage is needed and an empty account suffices. + for (work, width) in [(31, 32), (100, 31), (100, 32)] { let memory = structured_asn1::memory::Memory::new(std::sync::Arc::new( - structured_asn1::memory::MemoryAccount::new(capacity, work), + structured_asn1::memory::MemoryAccount::new(0, work), )); let called = Cell::new(false); let state = SessionDigest { @@ -275,10 +298,112 @@ mod digest_admission_tests { }; let mut output = [0; 32]; let result = state.finish(&mut output[..width], &memory); - let admitted = capacity == 32 && work == 100 && width == 32; + let admitted = work == 100 && width == 32; assert_eq!(result.is_ok(), admitted, "{result:?}"); assert_eq!(called.get(), admitted); assert_eq!(memory.used(), 0); } } + + fn account(capacity: usize, work: usize) -> structured_asn1::memory::Memory { + structured_asn1::memory::Memory::new(std::sync::Arc::new( + structured_asn1::memory::MemoryAccount::new(capacity, work), + )) + } + + fn begin( + memory: &structured_asn1::memory::Memory, + ) -> Result, DsigError> { + SessionDigest::begin( + crate::provider::default_provider(), + &crate::policy::VerificationPolicy::default(), + DigestAlgorithm::Sha256, + memory, + ) + } + + #[test] + fn a_state_that_does_not_fit_is_refused_before_it_exists() { + // A resource cause, not an unsupported capability or a mismatch. + for (capacity, work, expected) in [ + (0, 100, MemoryError::LimitExceeded), + (1 << 20, 0, MemoryError::WorkExceeded), + ] { + let memory = account(capacity, work); + let failure = begin(&memory).err().expect("the account refuses"); + let resource = match failure { + DsigError::Provider(crate::provider::ProviderError::Resource(cause)) + | DsigError::SharedExecution(cause) => cause, + other => panic!("{other:?}"), + }; + assert_eq!(resource, expected); + assert_eq!(memory.used(), 0); + } + } + + #[test] + fn a_state_keeps_its_charge_until_it_is_gone() { + // On success and on drop alike, and its whole block, not its output width. + let memory = account(1 << 20, 1 << 20); + let state = begin(&memory).expect("admitted"); + let held = memory.used(); + assert!(held > 32, "{held}"); + let mut output = [0; 32]; + state.finish(&mut output, &memory).expect("finished"); + assert_eq!(memory.used(), 0); + drop(begin(&memory).expect("admitted")); + assert_eq!(memory.used(), 0); + } + + #[test] + fn exactly_the_state_size_is_enough() { + let measured = account(1 << 20, 1 << 20); + let state = begin(&measured).expect("admitted"); + let exact = measured.used(); + drop(state); + begin(&account(exact, 100)).expect("the block fits exactly"); + assert!(begin(&account(exact - 1, 100)).is_err()); + } + + #[test] + fn chunk_boundaries_do_not_change_the_digest() { + let memory = account(1 << 20, 1 << 20); + let message = b"the same bytes in any number of updates"; + let mut whole = begin(&memory).expect("admitted"); + whole.update(message, &memory).expect("absorbed"); + let whole = whole.finish_output(&memory).expect("finished"); + for split in 0..=message.len() { + let mut parts = begin(&memory).expect("admitted"); + let (left, right) = message.split_at(split); + parts.update(left, &memory).expect("absorbed"); + parts.update(right, &memory).expect("absorbed"); + let parts = parts.finish_output(&memory).expect("finished"); + assert_eq!(&*parts, &*whole); + } + let one_shot = crate::xmldsig::compute_digest_with_provider( + crate::provider::default_provider(), + DigestAlgorithm::Sha256, + message, + &memory, + ) + .expect("digested"); + assert_eq!(&*one_shot, &*whole); + } + + #[test] + fn the_one_shot_refuses_like_the_stream() { + let memory = account(0, 100); + assert_eq!( + crate::xmldsig::compute_digest_with_provider( + crate::provider::default_provider(), + DigestAlgorithm::Sha256, + b"", + &memory, + ) + .err(), + Some(crate::provider::ProviderError::Resource( + MemoryError::LimitExceeded + )) + ); + } } diff --git a/src/xmldsig/verify/tests.rs b/src/xmldsig/verify/tests.rs index 0a236302..d135f310 100644 --- a/src/xmldsig/verify/tests.rs +++ b/src/xmldsig/verify/tests.rs @@ -31,6 +31,35 @@ fn make_reference( } } +#[test] +fn a_spent_execution_is_never_a_manifest_finding() { + // A refused admission says nothing about the Reference, whichever account + // it came from; a Reference's own failure stays a per-Reference finding. + use structured_asn1::memory::MemoryError; + assert!(matches!( + manifest_reference_failure_reason( + ReferenceProcessingError::SharedExecution(MemoryError::WorkExceeded), + 0 + ), + Err(ReferenceProcessingError::SharedExecution( + MemoryError::WorkExceeded + )) + )); + assert!(matches!( + manifest_reference_failure_reason( + ReferenceProcessingError::Provider(crate::provider::ProviderError::Resource( + MemoryError::OutOfMemory + )), + 0 + ), + Err(ReferenceProcessingError::Provider(_)) + )); + assert_eq!( + manifest_reference_failure_reason(ReferenceProcessingError::MissingUri, 3).ok(), + Some(FailureReason::ReferenceProcessingFailure { ref_index: 3 }) + ); +} + #[test] fn operation_borrows_caller_utc_facts_without_policy_rebinding() -> Result<(), Box> { diff --git a/src/xmlenc/decrypt.rs b/src/xmlenc/decrypt.rs index 725d2d3b..e8a3df51 100644 --- a/src/xmlenc/decrypt.rs +++ b/src/xmlenc/decrypt.rs @@ -3982,9 +3982,10 @@ mod tests { fn begin_digest( &self, algorithm: crate::xmldsig::DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, crate::provider::ProviderError> { - crate::provider::default_provider().begin_digest(algorithm) + crate::provider::default_provider().begin_digest(algorithm, memory) } #[cfg(feature = "xmldsig")] diff --git a/src/xmlenc/encrypt.rs b/src/xmlenc/encrypt.rs index 413ebc1a..3b49c757 100644 --- a/src/xmlenc/encrypt.rs +++ b/src/xmlenc/encrypt.rs @@ -2163,9 +2163,10 @@ mod tests { fn begin_digest( &self, algorithm: crate::xmldsig::DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, crate::provider::ProviderError> { - crate::provider::default_provider().begin_digest(algorithm) + crate::provider::default_provider().begin_digest(algorithm, memory) } #[cfg(feature = "xmldsig")] diff --git a/tests/aws_lc_provider.rs b/tests/aws_lc_provider.rs index 64df2cb4..96a6823a 100644 --- a/tests/aws_lc_provider.rs +++ b/tests/aws_lc_provider.rs @@ -116,8 +116,8 @@ fn native_primitives_match_published_known_answers() { assert!(AwsLcFipsProvider.fips_module_version().is_some()); // SHA-256("abc") anchors differential testing to a fixed known answer. assert_eq!( - AwsLcFipsProvider - .digest(DigestAlgorithm::Sha256, b"abc") + *AwsLcFipsProvider + .digest(DigestAlgorithm::Sha256, b"abc", &memory()) .unwrap(), hex_literal::hex!("ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad") ); @@ -263,8 +263,10 @@ fn aws_digests_match_rustcrypto() { ] { assert!(aws.supports(ProviderCapability::Digest(algorithm))); assert_eq!( - aws.digest(algorithm, b"abc").unwrap(), - RustCryptoProvider.digest(algorithm, b"abc").unwrap() + &*aws.digest(algorithm, b"abc", &memory()).unwrap(), + &*RustCryptoProvider + .digest(algorithm, b"abc", &memory()) + .unwrap() ); } } @@ -336,7 +338,17 @@ fn unsupported_digest_never_falls_back() { // An absent AWS mechanism cannot silently run in the other engine. let algorithm = DigestAlgorithm::Sha3_224; assert!(!AwsLcFipsProvider.supports(ProviderCapability::Digest(algorithm))); - assert!(AwsLcFipsProvider.digest(algorithm, b"abc").is_err()); + assert!( + AwsLcFipsProvider + .digest(algorithm, b"abc", &memory()) + .is_err() + ); +} + +fn memory() -> structured_asn1::memory::Memory { + structured_asn1::memory::Memory::new(std::sync::Arc::new( + structured_asn1::memory::MemoryAccount::new(1 << 20, 1 << 20), + )) } #[test] diff --git a/tests/cms_timestamp.rs b/tests/cms_timestamp.rs index e403d9be..4c4ee602 100644 --- a/tests/cms_timestamp.rs +++ b/tests/cms_timestamp.rs @@ -5,8 +5,9 @@ use std::sync::Arc; +use structured_asn1::digest::{Digest, Resolution}; use structured_asn1::memory::{Memory, MemoryAccount}; -use structured_asn1::{Encoding, Parser, digest::Resolution}; +use structured_asn1::{Encoding, Parser}; use structured_cms::content_source::ContentError; use structured_cms::digest::DigestError; use structured_cms::ess::BindingError; @@ -194,6 +195,33 @@ fn every_independent_authority_token_verifies_through_the_selected_provider() { } } +#[test] +fn a_digest_the_execution_cannot_hold_is_refused_with_its_cause() { + // The CMS digest callback admits the state in the caller's execution; a + // small ceiling is a resource refusal, never Unsupported or a mismatch. + let policy = VerificationPolicy::default(); + let provider = RustCryptoProvider; + let cms = CmsProvider::new(&provider, &policy); + let empty = Memory::new(Arc::new(MemoryAccount::new(0, 1 << 20))); + let refusal = match Digest::start(&cms, &DigestAlgorithm::Sha256, &empty) { + Err(CmsProviderError::Digest(refusal)) => refusal, + Err(other) => panic!("{other:?}"), + Ok(_) => panic!("an empty account cannot hold a state"), + }; + assert!( + matches!( + refusal, + xml_sec::xmldsig::DsigError::Provider(xml_sec::provider::ProviderError::Resource( + structured_asn1::memory::MemoryError::LimitExceeded + )) + ), + "{refusal:?}" + ); + let spent = Memory::new(Arc::new(MemoryAccount::new(1 << 20, 0))); + assert!(Digest::start(&cms, &DigestAlgorithm::Sha256, &spent).is_err()); + assert_eq!(spent.used(), 0); +} + #[test] fn other_content_is_an_imprint_disagreement_not_an_error() { // The signature stays valid; only the covered content differs, so no @@ -391,8 +419,9 @@ impl CryptoProvider for WithoutEd25519 { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &Memory, ) -> Result, ProviderError> { - RustCryptoProvider.begin_digest(algorithm) + RustCryptoProvider.begin_digest(algorithm, memory) } } diff --git a/tests/detached_payloads.rs b/tests/detached_payloads.rs index 289ed23a..291b5be8 100644 --- a/tests/detached_payloads.rs +++ b/tests/detached_payloads.rs @@ -2,23 +2,28 @@ #![cfg(feature = "xmldsig-sign")] +#[path = "support/refusing_state.rs"] +mod refusing_state; + use std::collections::BTreeMap; use std::sync::Arc; use base64::Engine as _; +use refusing_state::RefusingStateProvider; use sha2::{Digest, Sha256}; -use structured_asn1::memory::{Memory, MemoryAccount}; +use structured_asn1::memory::{Memory, MemoryAccount, MemoryError}; use xml_sec::XmlDocument; use xml_sec::c14n::{C14nAlgorithm, C14nMode}; use xml_sec::policy::{ DetachedDigestAttestation, ManifestProcessing, PolicyViolation, SigningPolicy, VerificationPolicy, }; +use xml_sec::provider::ProviderError; use xml_sec::xmldsig::{ DetachedChain, DetachedCompletion, DigestAlgorithm, DsigError, DsigStatus, ExternalPayload, - FailureReason, HmacSigningKey, HmacVerificationKey, ReferenceBuilder, ReferenceResult, - SignContext, SignatureAlgorithm, SignatureBuilder, Transform, UriTypeSet, VerificationRequest, - VerifiedSession, VerifyContext, with_verified_request, + FailureReason, HmacSigningKey, HmacVerificationKey, ReferenceBuilder, ReferenceProcessingError, + ReferenceResult, SignContext, SignatureAlgorithm, SignatureBuilder, Transform, UriTypeSet, + VerificationRequest, VerifiedSession, VerifyContext, with_verified_request, }; const SECRET: [u8; 32] = [0x5a; 32]; @@ -747,12 +752,9 @@ fn invalid_payload_requests_are_rejected_before_any_byte() { }); } -#[test] -fn completing_core_validation_processes_the_authenticated_manifest() { - // While a SignedInfo Reference is unavailable, core validation has failed - // and Manifests are unparsed. Completing it processes them as verification - // would (XMLDSig 1.1 §3.2), instead of leaving an empty Manifest list. - let payload = b"detached part"; +/// A signature whose `SignedInfo` covers `payload` by a detached Reference and +/// an authenticated Manifest. +fn manifest_document(payload: &[u8]) -> XmlDocument { let digest = base64::engine::general_purpose::STANDARD.encode(Sha256::digest(payload)); let template = format!( r##"manifest target{digest}"## @@ -768,7 +770,114 @@ fn completing_core_validation_processes_the_authenticated_manifest() { .external_resources(&resources) .sign_template(&template) .unwrap(); - let document = XmlDocument::parse(xml).unwrap(); + XmlDocument::parse(xml).unwrap() +} + +#[test] +fn a_manifest_digest_refusal_ends_the_detached_completion() { + // One state serves the #manifest Reference and one the payload, so the + // Manifest's own digest is refused during completion. A spent ceiling must + // reach the caller as well as the evidence, not read as a finished payload. + let payload = b"detached part"; + let document = manifest_document(payload); + let mut policy = verification_policy(); + policy.manifest_processing = ManifestProcessing::Process; + let key = HmacVerificationKey::new(SECRET.to_vec()).unwrap(); + let provider = RefusingStateProvider::after(2); + let context = VerifyContext::new() + .policy(policy) + .key(&key) + .provider(&provider); + let work = work(); + let error = with_verified_request( + &document, + &context, + &VerificationRequest::default(), + &work, + |mut session| { + let reference = pending_references(&session, DetachedCompletion::Complete)[0]; + let outcome = stream( + &mut session, + &[reference], + payload, + 4, + DetachedCompletion::Complete, + ); + assert!(session.evidence().signatures()[0].result().is_err()); + outcome + }, + ) + .expect_err("the Manifest digest cannot start"); + assert!( + matches!( + error, + DsigError::Reference(ReferenceProcessingError::Provider(ProviderError::Resource( + MemoryError::OutOfMemory + ))) + ), + "{error:?}" + ); +} + +#[test] +fn finalization_is_admitted_before_the_provider_runs() { + // Updates can spend the allowance to the last unit; finishing then must + // stop at the admission, with the backend never entered. + let payload = b"detached part"; + let document = signed( + &[Detached { + uri: "part.bin", + base64: false, + digest: DigestAlgorithm::Sha256, + payload, + }], + false, + ); + let run = |allowance: usize| { + let key = HmacVerificationKey::new(SECRET.to_vec()).unwrap(); + let provider = RefusingStateProvider::after(usize::MAX); + let context = VerifyContext::new() + .policy(verification_policy()) + .key(&key) + .provider(&provider); + let work = Memory::new(Arc::new(MemoryAccount::new(1 << 22, allowance))); + let before_finish = std::cell::Cell::new((0, 0)); + let result = with_verified_request( + &document, + &context, + &VerificationRequest::default(), + &work, + |mut session| { + let reference = pending_references(&session, DetachedCompletion::Complete)[0]; + let mut stream = + session.detached_payload(&[reference], DetachedCompletion::Complete)?; + stream.update(payload)?; + before_finish.set((work.work_used(), provider.finalized())); + stream.finish() + }, + ); + (result, before_finish.get(), provider.finalized()) + }; + let (result, (spent, _), _) = run(1 << 28); + result.expect("the account is ample"); + let (result, (_, finalized_before), finalized_after) = run(spent); + assert!( + matches!( + result, + Err(DsigError::SharedExecution(MemoryError::WorkExceeded)) + ), + "{result:?}" + ); + assert_eq!(finalized_after, finalized_before); +} + +#[test] +fn completing_core_validation_processes_the_authenticated_manifest() { + // While a SignedInfo Reference is unavailable, core validation has failed + // and Manifests are unparsed. Completing it processes them as verification + // would (XMLDSig 1.1 §3.2), instead of leaving an empty Manifest list. + let payload = b"detached part"; + let document = manifest_document(payload); let mut policy = verification_policy(); policy.manifest_processing = ManifestProcessing::Process; let key = HmacVerificationKey::new(SECRET.to_vec()).unwrap(); diff --git a/tests/key_establishment_policy.rs b/tests/key_establishment_policy.rs index 79361687..a1896a7e 100644 --- a/tests/key_establishment_policy.rs +++ b/tests/key_establishment_policy.rs @@ -89,8 +89,9 @@ impl CryptoProvider for RecordingProvider { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { - RUST_CRYPTO_PROVIDER.begin_digest(algorithm) + RUST_CRYPTO_PROVIDER.begin_digest(algorithm, memory) } fn sign( &self, diff --git a/tests/pkcs11.rs b/tests/pkcs11.rs index 7e527126..7439d51f 100644 --- a/tests/pkcs11.rs +++ b/tests/pkcs11.rs @@ -10,6 +10,7 @@ use cryptoki::{ types::AuthPin, }; use std::sync::Arc; +use structured_asn1::memory::{Memory, MemoryAccount, MemoryError}; use xml_sec::provider::pkcs11::{Pkcs11AesKey, Pkcs11Provider, Pkcs11RsaKey, cryptoki}; use xml_sec::provider::{ ContentDecryptionKey, CryptoProvider, ExternalProviderError, KeyAgreementParameters, @@ -102,20 +103,46 @@ fn assert_login_and_digests(provider: &Pkcs11Provider, pin: &AuthPin) { continue; } digested += 1; - let expected = RustCryptoProvider.digest(algorithm, &data).unwrap(); - let mut state = provider.begin_digest(algorithm).unwrap(); + let memory = Memory::new(Arc::new(MemoryAccount::new(1 << 20, 1 << 20))); + let expected = RustCryptoProvider + .digest(algorithm, &data, &memory) + .unwrap(); + let mut state = provider.begin_digest(algorithm, &memory).unwrap(); for part in data.chunks(7) { state.update(part).unwrap(); } - assert_eq!(state.finalize().unwrap(), expected, "{algorithm:?}"); - let mut abandoned = provider.begin_digest(algorithm).unwrap(); + let mut output = [0; xml_sec::xmldsig::MAX_DIGEST_OUTPUT]; + let written = state.finalize(&mut output).unwrap(); + assert_eq!(&output[..written], &*expected, "{algorithm:?}"); + drop(state); + let mut abandoned = provider.begin_digest(algorithm, &memory).unwrap(); abandoned.update(&data).unwrap(); drop(abandoned); assert_eq!( - provider.digest(algorithm, &data).unwrap(), - expected, + &*provider.digest(algorithm, &data, &memory).unwrap(), + &*expected, "{algorithm:?}" ); + // The token's state is charged like a software one, and refused alike. + assert_eq!(memory.used(), 0); + // The result crosses a buffer cryptoki allocates, so a ceiling that + // only fits the state refuses finalization, and drop ends the operation. + let state = provider.begin_digest(algorithm, &memory).unwrap(); + let state_size = memory.used(); + drop(state); + let exact = Memory::new(Arc::new(MemoryAccount::new(state_size, 1 << 20))); + let mut state = provider.begin_digest(algorithm, &exact).unwrap(); + assert_eq!( + state.finalize(&mut output).err(), + Some(ProviderError::Resource(MemoryError::LimitExceeded)) + ); + drop(state); + assert_eq!(exact.used(), 0); + let empty = Memory::new(Arc::new(MemoryAccount::new(0, 1 << 20))); + assert_eq!( + provider.begin_digest(algorithm, &empty).err(), + Some(ProviderError::Resource(MemoryError::LimitExceeded)) + ); } assert!(digested > 0, "the test token advertises no digest"); } @@ -332,10 +359,13 @@ fn assert_token_signatures( ) .unwrap() ); + let memory = Memory::new(Arc::new(MemoryAccount::new(1 << 20, 1 << 20))); assert_eq!( - provider.digest(DigestAlgorithm::Sha256, MESSAGE).unwrap(), - RustCryptoProvider - .digest(DigestAlgorithm::Sha256, MESSAGE) + &*provider + .digest(DigestAlgorithm::Sha256, MESSAGE, &memory) + .unwrap(), + &*RustCryptoProvider + .digest(DigestAlgorithm::Sha256, MESSAGE, &memory) .unwrap() ); // A same-name provider on another session must not execute this handle. diff --git a/tests/policy_profiles.rs b/tests/policy_profiles.rs index cce67095..d1c7849a 100644 --- a/tests/policy_profiles.rs +++ b/tests/policy_profiles.rs @@ -86,6 +86,7 @@ impl CryptoProvider for ProfileTestProvider { fn begin_digest( &self, _: xml_sec::xmldsig::DigestAlgorithm, + _: &structured_asn1::memory::Memory, ) -> Result, xml_sec::provider::ProviderError> { panic!("compile must not execute crypto") diff --git a/tests/provider_contract.rs b/tests/provider_contract.rs index c5f17f3f..8d075220 100644 --- a/tests/provider_contract.rs +++ b/tests/provider_contract.rs @@ -108,8 +108,9 @@ impl CryptoProvider for ExternalProvider { fn begin_digest( &self, algorithm: xml_sec::xmldsig::DigestAlgorithm, + memory: &structured_asn1::memory::Memory, ) -> Result, ProviderError> { - xml_sec::provider::default_provider().begin_digest(algorithm) + xml_sec::provider::default_provider().begin_digest(algorithm, memory) } fn sign( diff --git a/tests/rust_api.rs b/tests/rust_api.rs index d085023e..124823ad 100644 --- a/tests/rust_api.rs +++ b/tests/rust_api.rs @@ -49,7 +49,11 @@ impl CryptoProvider for RefusingProvider { fn derive_key(&self, _: &KdfParameters<'_>, _: &[u8]) -> Result, ProviderError> { Err(refusal()) } - fn begin_digest(&self, _: DigestAlgorithm) -> Result, ProviderError> { + fn begin_digest( + &self, + _: DigestAlgorithm, + _: &structured_asn1::memory::Memory, + ) -> Result, ProviderError> { Err(refusal()) } fn sign( diff --git a/tests/session_lookups.rs b/tests/session_lookups.rs index 59bec859..19c32e66 100644 --- a/tests/session_lookups.rs +++ b/tests/session_lookups.rs @@ -6,6 +6,7 @@ use std::collections::BTreeSet; use std::sync::Arc; use structured_asn1::memory::{Memory, MemoryAccount, MemoryError}; use xml_sec::policy::{PolicyViolation, SameDocumentIdSemantics, VerificationPolicy}; +use xml_sec::provider::ProviderError; use xml_sec::{XmlDocument, xmldsig::*}; const SHA1: &str = "http://www.w3.org/2000/09/xmldsig#sha1"; @@ -182,7 +183,7 @@ fn digests_use_the_selected_provider_and_policy() { // FIPS 180-4 Appendix B.1 known answer for SHA-256("abc"), absorbed in two // chunks; each chunk is charged before the provider sees it. with_session(|session, work| { - let mut digest = session.begin_digest(DigestAlgorithm::Sha256).unwrap(); + let mut digest = session.begin_digest(DigestAlgorithm::Sha256, work).unwrap(); digest.update(b"a", work).unwrap(); digest.update(b"bc", work).unwrap(); let before_finish = work.work_used(); @@ -201,15 +202,28 @@ fn digests_use_the_selected_provider_and_policy() { "digest finalization and output must be admitted by the current query" ); assert!(matches!( - session.begin_digest(DigestAlgorithm::Sha1), + session.begin_digest(DigestAlgorithm::Sha1, work), Err(DsigError::Policy(PolicyViolation::Algorithm { .. })) )); let starved = Memory::new(Arc::new(MemoryAccount::new(1 << 20, 2))); - let mut digest = session.begin_digest(DigestAlgorithm::Sha256).unwrap(); + let mut digest = session.begin_digest(DigestAlgorithm::Sha256, work).unwrap(); assert!(matches!( digest.update(b"abc", &starved), Err(DsigError::SharedExecution(MemoryError::WorkExceeded)) )); assert_eq!(starved.used(), 0); + // State creation answers to the same execution, with the same cause. + let empty = Memory::new(Arc::new(MemoryAccount::new(0, 16))); + assert!(matches!( + session.begin_digest(DigestAlgorithm::Sha256, &empty), + Err(DsigError::Provider(ProviderError::Resource( + MemoryError::LimitExceeded + ))) + )); + let spent = Memory::new(Arc::new(MemoryAccount::new(1 << 20, 0))); + assert!(matches!( + session.begin_digest(DigestAlgorithm::Sha256, &spent), + Err(DsigError::SharedExecution(MemoryError::WorkExceeded)) + )); }); } diff --git a/tests/streaming_digest.rs b/tests/streaming_digest.rs index f98b726b..ad3c0178 100644 --- a/tests/streaming_digest.rs +++ b/tests/streaming_digest.rs @@ -1,15 +1,21 @@ #![cfg(feature = "xmldsig-sign")] //! Incremental digests at the provider boundary and in Reference processing. +#[path = "support/refusing_state.rs"] +mod refusing_state; + +use refusing_state::RefusingStateProvider; +use std::sync::Arc; +use structured_asn1::memory::{Memory, MemoryAccount, MemoryError}; use xml_sec::c14n::{C14nAlgorithm, C14nMode}; use xml_sec::provider::{ CryptoProvider, DigestState, ExternalProviderError, KdfParameters, ProviderCapability, ProviderError, RustCryptoProvider, }; use xml_sec::xmldsig::{ - DigestAlgorithm, DsigError, ReferenceBuilder, ReferenceProcessingError, RsaSigningKey, - SignContext, SignatureAlgorithm, SignatureBuilder, SigningDigestError, SigningError, - SigningKey, VerificationKey, VerifyContext, + DigestAlgorithm, DsigError, MAX_DIGEST_OUTPUT, ReferenceBuilder, ReferenceProcessingError, + RsaSigningKey, SignContext, SignatureAlgorithm, SignatureBuilder, SigningDigestError, + SigningError, SigningKey, VerificationKey, VerifyContext, }; fn providers() -> Vec<&'static dyn CryptoProvider> { @@ -27,13 +33,19 @@ fn streamed( chunk: usize, ) -> Vec { let mut state = provider - .begin_digest(algorithm) + .begin_digest(algorithm, &memory()) .expect("advertised digest starts"); for part in data.chunks(chunk) { state.update(part).expect("digest absorbs input"); state.update(&[]).expect("an empty update is a no-op"); } - state.finalize().expect("digest finishes") + let mut output = [0; MAX_DIGEST_OUTPUT]; + let written = state.finalize(&mut output).expect("digest finishes"); + output[..written].to_vec() +} + +fn memory() -> Memory { + Memory::new(Arc::new(MemoryAccount::new(1 << 20, 1 << 20))) } #[test] @@ -47,13 +59,13 @@ fn streamed_digests_equal_one_shot_digests_for_every_algorithm_and_chunking() { continue; } let whole = provider - .digest(algorithm, &data) + .digest(algorithm, &data, &memory()) .expect("advertised digest"); assert_eq!(whole.len(), algorithm.output_len(), "{algorithm:?}"); for chunk in [1, 7, 64, 1_000, data.len()] { assert_eq!( streamed(provider, algorithm, &data, chunk), - whole, + &*whole, "{} {algorithm:?} in {chunk}-byte chunks", provider.name() ); @@ -71,6 +83,109 @@ fn streamed_digests_equal_one_shot_digests_for_every_algorithm_and_chunking() { ); } +#[test] +fn every_provider_charges_its_state_and_releases_it() { + // State creation is refused, not panicked on, and a live state is paid for + // until it is dropped, whatever the backend. + for provider in providers() { + let empty = Memory::new(Arc::new(MemoryAccount::new(0, 1 << 20))); + assert_eq!( + provider.begin_digest(DigestAlgorithm::Sha256, &empty).err(), + Some(ProviderError::Resource(MemoryError::LimitExceeded)), + "{}", + provider.name() + ); + assert_eq!( + provider + .digest(DigestAlgorithm::Sha256, b"abc", &empty) + .err(), + Some(ProviderError::Resource(MemoryError::LimitExceeded)) + ); + let account = memory(); + let state = provider + .begin_digest(DigestAlgorithm::Sha256, &account) + .expect("admitted"); + assert!(account.used() > 0); + drop(state); + assert_eq!(account.used(), 0); + } +} + +#[test] +fn a_storage_refusal_stays_typed_and_terminal() { + // Not unsupported, not a digest mismatch, not a per-Reference finding with + // more work after it: the operation ends with the resource cause. + let key = signing_key(); + let signed = SignContext::new(&key) + .sign_template(&template()) + .expect("signing with the default provider"); + let provider = RefusingStateProvider::after(0); + let error = VerifyContext::new() + .key(&verification_key(&key)) + .provider(&provider) + .verify(&signed) + .expect_err("verification cannot pass without a digest"); + assert!( + matches!( + error, + DsigError::Reference(ReferenceProcessingError::Provider(ProviderError::Resource( + MemoryError::OutOfMemory + ))) + ), + "{error:?}" + ); + let error = SignContext::new(&key) + .provider(&provider) + .sign_template(&template()) + .expect_err("signing cannot pass without a digest"); + assert!( + matches!( + error, + SigningError::Digest(SigningDigestError::Provider(ProviderError::Resource( + MemoryError::OutOfMemory + ))) + ), + "{error:?}" + ); + assert_eq!(provider.finalized(), 0); +} + +#[test] +fn a_storage_refusal_in_a_manifest_reference_ends_the_operation() { + // SignedInfo passes; the Manifest Reference then cannot start its digest. A + // spent ceiling must not read as an invalid Reference with more work after it. + let template = r##"manifest payloadstale"##; + let key = signing_key(); + let signed = SignContext::new(&key) + .policy(xml_sec::policy::SigningPolicy { + manifest_processing: xml_sec::policy::ManifestProcessing::Process, + ..xml_sec::policy::SigningPolicy::default() + }) + .sign_template(template) + .expect("signing with the default provider"); + let public = verification_key(&key); + let verify = |provider: &RefusingStateProvider| { + VerifyContext::new() + .key(&public) + .provider(provider) + .process_manifests(true) + .verify(&signed) + }; + let admitted = verify(&RefusingStateProvider::after(2)).expect("both digests fit"); + assert_eq!(admitted.manifest_references.len(), 1); + let error = + verify(&RefusingStateProvider::after(1)).expect_err("the Manifest digest cannot start"); + assert!( + matches!( + error, + DsigError::Reference(ReferenceProcessingError::Provider(ProviderError::Resource( + MemoryError::OutOfMemory + ))) + ), + "{error:?}" + ); +} + /// Counts updates and refuses after `remaining` of them, as a token or HSM may. struct FailingDigest<'a> { remaining: usize, @@ -88,8 +203,8 @@ impl DigestState for FailingDigest<'_> { self.remaining = remaining; self.inner.update(data) } - fn finalize(self: Box) -> Result, ProviderError> { - self.inner.finalize() + fn finalize(&mut self, output: &mut [u8; MAX_DIGEST_OUTPUT]) -> Result { + self.inner.finalize(output) } } @@ -127,11 +242,12 @@ impl CryptoProvider for FailingDigestProvider { fn begin_digest( &self, algorithm: DigestAlgorithm, + memory: &Memory, ) -> Result, ProviderError> { Ok(Box::new(FailingDigest { remaining: self.updates, calls: &self.calls, - inner: xml_sec::provider::default_provider().begin_digest(algorithm)?, + inner: xml_sec::provider::default_provider().begin_digest(algorithm, memory)?, })) } fn sign( diff --git a/tests/support/refusing_state.rs b/tests/support/refusing_state.rs new file mode 100644 index 00000000..953df6f9 --- /dev/null +++ b/tests/support/refusing_state.rs @@ -0,0 +1,154 @@ +//! A provider whose digest state runs out, and which counts finalizations. + +use std::sync::atomic::{AtomicUsize, Ordering::Relaxed}; +use structured_asn1::memory::{Memory, MemoryError}; +use xml_sec::provider::{ + CryptoProvider, DigestState, KdfParameters, ProviderCapability, ProviderError, + RustCryptoProvider, +}; +use xml_sec::xmldsig::{ + DigestAlgorithm, DsigError, MAX_DIGEST_OUTPUT, SignatureAlgorithm, SigningKey, SigningKeyError, + VerifyingKey, +}; + +/// Refuses to create a state after `allowed` of them, as a spent account or the +/// allocator does. +pub struct RefusingStateProvider { + allowed: AtomicUsize, + finalized: AtomicUsize, +} + +impl RefusingStateProvider { + pub fn after(allowed: usize) -> Self { + Self { + allowed: AtomicUsize::new(allowed), + finalized: AtomicUsize::new(0), + } + } + + /// Provider finalizations so far. + pub fn finalized(&self) -> usize { + self.finalized.load(Relaxed) + } +} + +struct CountedState<'p> { + state: Box, + finalized: &'p AtomicUsize, +} + +impl DigestState for CountedState<'_> { + fn update(&mut self, data: &[u8]) -> Result<(), ProviderError> { + self.state.update(data) + } + fn finalize(&mut self, output: &mut [u8; MAX_DIGEST_OUTPUT]) -> Result { + self.finalized.fetch_add(1, Relaxed); + self.state.finalize(output) + } +} + +impl CryptoProvider for RefusingStateProvider { + fn name(&self) -> &'static str { + "refusing-state" + } + fn supports(&self, capability: ProviderCapability<'_>) -> bool { + RustCryptoProvider.supports(capability) + } + fn fill_random(&self, output: &mut [u8]) -> Result<(), ProviderError> { + RustCryptoProvider.fill_random(output) + } + fn derive_key( + &self, + parameters: &KdfParameters<'_>, + secret: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.derive_key(parameters, secret) + } + fn begin_digest( + &self, + algorithm: DigestAlgorithm, + memory: &Memory, + ) -> Result, ProviderError> { + let remaining = self + .allowed + .load(Relaxed) + .checked_sub(1) + .ok_or(ProviderError::Resource(MemoryError::OutOfMemory))?; + self.allowed.store(remaining, Relaxed); + Ok(Box::new(CountedState { + state: xml_sec::provider::default_provider().begin_digest(algorithm, memory)?, + finalized: &self.finalized, + })) + } + fn sign( + &self, + key: &dyn SigningKey, + algorithm: SignatureAlgorithm, + data: &[u8], + ) -> Result, SigningKeyError> { + RustCryptoProvider.sign(key, algorithm, data) + } + fn verify( + &self, + key: &dyn VerifyingKey, + algorithm: SignatureAlgorithm, + data: &[u8], + signature: &[u8], + ) -> Result { + RustCryptoProvider.verify(key, algorithm, data, signature) + } + #[cfg(feature = "xmlenc")] + fn encrypt_data( + &self, + algorithm: xml_sec::xmlenc::DataEncryptionAlgorithm, + key: &[u8], + plaintext: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.encrypt_data(algorithm, key, plaintext) + } + #[cfg(feature = "xmlenc")] + fn decrypt_data( + &self, + algorithm: xml_sec::xmlenc::DataEncryptionAlgorithm, + key: &[u8], + ciphertext: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.decrypt_data(algorithm, key, ciphertext) + } + #[cfg(feature = "xmlenc")] + fn wrap_key( + &self, + algorithm: xml_sec::xmlenc::KeyWrapAlgorithm, + kek: &[u8], + key: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.wrap_key(algorithm, kek, key) + } + #[cfg(feature = "xmlenc")] + fn unwrap_key( + &self, + algorithm: xml_sec::xmlenc::KeyWrapAlgorithm, + kek: &[u8], + wrapped: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.unwrap_key(algorithm, kek, wrapped) + } + #[cfg(feature = "xmlenc")] + fn transport_key( + &self, + key: &dyn xml_sec::provider::KeyTransportKey, + parameters: &xml_sec::xmlenc::RsaOaepParameters, + plaintext: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.transport_key(key, parameters, plaintext) + } + #[cfg(feature = "xmlenc")] + fn recover_key( + &self, + key: &dyn xml_sec::provider::KeyRecoveryKey, + parameters: &xml_sec::xmlenc::RsaOaepParameters, + ciphertext: &[u8], + ) -> Result, ProviderError> { + RustCryptoProvider.recover_key(key, parameters, ciphertext) + } +} diff --git a/tools/xmlsec1/tests/process_contract.rs b/tools/xmlsec1/tests/process_contract.rs index 12837ca4..f2e6720d 100644 --- a/tools/xmlsec1/tests/process_contract.rs +++ b/tools/xmlsec1/tests/process_contract.rs @@ -7125,9 +7125,8 @@ fn rsa_encryption_matches_selector_only_x509data_to_configured_certificate() { _ => None, }) .unwrap(); - let digest = default_provider() - .digest(DigestAlgorithm::Sha256, &certificate_pem.contents) - .unwrap(); + let digest = + xml_sec::xmldsig::compute_digest(DigestAlgorithm::Sha256, &certificate_pem.contents); fs::write(&plaintext, b"selector recipient identity").unwrap(); let selectors = [ ( From 3f4d2a88c91252a7962129ea78dd9d66474624b7 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 1 Nov 2026 22:50:27 +0200 Subject: [PATCH 2/4] fix(provider): pay for the PKCS#11 result before the operation starts Drop ends an abandoned operation with the same digest_final that allocates the result buffer, so a refusal at finalization caused the very allocation it refused. The width is admitted before digest_init and held for the life of the state. The regression test finishes on an account of exactly that size and leaves the session usable after a refusal. --- docs/crypto-providers.md | 3 ++- src/provider/pkcs11.rs | 20 +++++++++----------- tests/pkcs11.rs | 36 +++++++++++++++++++++++++++++------- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/docs/crypto-providers.md b/docs/crypto-providers.md index 653c1e6d..02a0c1ff 100644 --- a/docs/crypto-providers.md +++ b/docs/crypto-providers.md @@ -142,7 +142,8 @@ reported as an unsupported algorithm or a digest mismatch. Sessions, CMS, XAdES detached payloads charge the caller's execution; plain signing and verification, which hold one state per Reference, use an account of their own without a ceiling. A PKCS#11 result passes through a buffer that `cryptoki` allocates itself: its width -is admitted first, but that one allocation cannot report a refusal. +is admitted with the state, before the token operation starts, but that one +allocation cannot report a refusal. XMLDSig Reference processing streams the final transform output into this state for both signing and verification: a final node set is canonicalized straight diff --git a/src/provider/pkcs11.rs b/src/provider/pkcs11.rs index b0e6d751..b53d43fa 100644 --- a/src/provider/pkcs11.rs +++ b/src/provider/pkcs11.rs @@ -285,9 +285,10 @@ const fn output_capacity() -> usize { struct Pkcs11Digest<'a> { session: MutexGuard<'a, Session>, active: bool, - /// Pays for the result buffer, which only cryptoki can allocate. - memory: structured_asn1::memory::Memory, - width: usize, + /// Pays for the result buffer that only cryptoki can allocate. `digest_final` + /// runs on every way out of an operation, including `Drop`, so it is + /// admitted before the operation starts and held until the state is gone. + _result_buffer: structured_asn1::memory::MemoryReservation, } impl super::DigestState for Pkcs11Digest<'_> { @@ -306,12 +307,7 @@ impl super::DigestState for Pkcs11Digest<'_> { output: &mut [u8; crate::xmldsig::MAX_DIGEST_OUTPUT], ) -> Result { // The module's reply reaches us through cryptoki's own buffer; it has - // no call that writes to caller storage, so the width is admitted here. - // A refusal leaves the operation active for Drop to end. - let _temporary = self - .memory - .reserve(self.width) - .map_err(ProviderError::Resource)?; + // no call that writes to caller storage. self.active = false; let digest = self .session @@ -844,6 +840,9 @@ impl CryptoProvider for Pkcs11Provider { self.require_capability(ProviderCapability::Digest(algorithm))?; let mechanism = digest_mechanism(algorithm) .ok_or_else(|| unsupported(ProviderCapability::Digest(algorithm)))?; + let result_buffer = memory + .reserve(algorithm.output_len()) + .map_err(ProviderError::Resource)?; let session = self.0.session()?; session .digest_init(&mechanism) @@ -852,8 +851,7 @@ impl CryptoProvider for Pkcs11Provider { Pkcs11Digest { session, active: true, - memory: memory.clone(), - width: algorithm.output_len(), + _result_buffer: result_buffer, }, memory, ) diff --git a/tests/pkcs11.rs b/tests/pkcs11.rs index 7439d51f..bd726236 100644 --- a/tests/pkcs11.rs +++ b/tests/pkcs11.rs @@ -125,19 +125,41 @@ fn assert_login_and_digests(provider: &Pkcs11Provider, pin: &AuthPin) { ); // The token's state is charged like a software one, and refused alike. assert_eq!(memory.used(), 0); - // The result crosses a buffer cryptoki allocates, so a ceiling that - // only fits the state refuses finalization, and drop ends the operation. + // The result crosses a buffer cryptoki allocates on every way out of an + // operation, finalization and abandonment alike, so an admitted state + // has already paid for it and finishing needs nothing more. let state = provider.begin_digest(algorithm, &memory).unwrap(); - let state_size = memory.used(); + let admitted = memory.used(); drop(state); - let exact = Memory::new(Arc::new(MemoryAccount::new(state_size, 1 << 20))); + let exact = Memory::new(Arc::new(MemoryAccount::new(admitted, 1 << 20))); let mut state = provider.begin_digest(algorithm, &exact).unwrap(); + assert_eq!(exact.used(), admitted); + state.update(&data).unwrap(); + let written = state.finalize(&mut output).unwrap(); + assert_eq!(&output[..written], &*expected, "{algorithm:?}"); + drop(state); + assert_eq!(exact.used(), 0); + let short = Memory::new(Arc::new(MemoryAccount::new(admitted - 1, 1 << 20))); assert_eq!( - state.finalize(&mut output).err(), + provider.begin_digest(algorithm, &short).err(), Some(ProviderError::Resource(MemoryError::LimitExceeded)) ); - drop(state); - assert_eq!(exact.used(), 0); + // A refusal after the operation started must still leave the session + // ready: the state's block does not fit, the result buffer does. + let width_only = Memory::new(Arc::new(MemoryAccount::new( + algorithm.output_len(), + 1 << 20, + ))); + assert_eq!( + provider.begin_digest(algorithm, &width_only).err(), + Some(ProviderError::Resource(MemoryError::LimitExceeded)) + ); + assert_eq!(width_only.used(), 0); + assert_eq!( + &*provider.digest(algorithm, &data, &memory).unwrap(), + &*expected, + "{algorithm:?}" + ); let empty = Memory::new(Arc::new(MemoryAccount::new(0, 1 << 20))); assert_eq!( provider.begin_digest(algorithm, &empty).err(), From 5da6fa003bf52287ab1a91cb09996f675d4fc17b Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Sun, 1 Nov 2026 23:54:57 +0200 Subject: [PATCH 3/4] fix(xmldsig)!: end verification on a spent Manifest limit An operation limit says nothing about the Reference that reached it, yet a Manifest Reference reported it as a policy violation and the cause was lost. It now ends the operation with the typed limit, as on the SignedInfo path and at Manifest XPath parsing. The separate check for an already spent canonicalization budget is gone: exhaustion surfaces where the bytes are charged, so a Reference needing none is not refused. A sweep over every ceiling through verify() carries the regression, and the existing exhaustion test now expects the typed limit; later References still do not run. BREAKING CHANGE: a spent operation resource limit in a Manifest Reference fails verify() instead of producing ReferencePolicyViolation. --- docs/xmldsig.md | 9 ++-- src/xmldsig/verify.rs | 23 +++++++--- src/xmldsig/verify/tests.rs | 86 +++++++++++++++++++++++++++++++++---- 3 files changed, 98 insertions(+), 20 deletions(-) diff --git a/docs/xmldsig.md b/docs/xmldsig.md index 03173f57..40526e14 100644 --- a/docs/xmldsig.md +++ b/docs/xmldsig.md @@ -558,13 +558,14 @@ Manifest references obey the same per-reference transform-count ceiling and tran `` references; a violation is recorded in that Manifest reference's independent status. They also share `ResourcePolicy::max_references` with ``: every parsed Manifest reference consumes that operation-wide budget, including entries whose transforms are unsupported -and retained only as independent failure results. +and retained only as independent failure results. Exhausting an operation-wide resource limit while +processing a Manifest reference ends `verify()` with an error instead of a per-reference status. Malformed `XMLDSig` structure, unsupported algorithms in core signature processing, disallowed URIs in `` references, and inconsistent `KeyInfo` metadata are processing errors rather -than validity statuses. Manifest policy violations and unsupported transforms remain independent -per-reference statuses as described above. Treat both `Invalid(reason)` and an API error as a -rejected document; never continue an authentication flow after either outcome. +than validity statuses. Other Manifest policy violations and unsupported transforms remain +independent per-reference statuses as described above. Treat both `Invalid(reason)` and an API +error as a rejected document; never continue an authentication flow after either outcome. External references are disabled by default. Verification and signing callers must both allow their URI class with `UriTypeSet` and provide every payload through diff --git a/src/xmldsig/verify.rs b/src/xmldsig/verify.rs index b5ad248a..8890d097 100644 --- a/src/xmldsig/verify.rs +++ b/src/xmldsig/verify.rs @@ -569,7 +569,8 @@ impl<'a> VerifyContext<'a> { /// `VerifyResult::manifest_references` in addition to `VerifyResult::status` /// when interpreting `verify()` results. /// Structural/parse errors in Manifest content abort `verify()` and are - /// returned as `Err(...)`. + /// returned as `Err(...)`, as does a spent operation resource limit, which + /// says nothing about the Reference that reached it. pub fn process_manifests(mut self, enabled: bool) -> Self { self.policy.manifest_processing = if enabled { crate::policy::ManifestProcessing::Process @@ -5447,6 +5448,15 @@ fn manifest_reference_failure_reason( error, ReferenceProcessingError::SharedExecution(_) | ReferenceProcessingError::Provider(crate::provider::ProviderError::Resource(_)) + | ReferenceProcessingError::Policy( + crate::policy::PolicyViolation::ResourceLimit { .. } + ) + | ReferenceProcessingError::UriDereference(TransformError::Policy( + crate::policy::PolicyViolation::ResourceLimit { .. } + )) + | ReferenceProcessingError::Transform(TransformError::Policy( + crate::policy::PolicyViolation::ResourceLimit { .. } + )) ) { return Err(error); } @@ -5561,12 +5571,11 @@ fn evaluate_manifest_reference( ctx: &VerifyContext<'_>, reference_node: Option>, ) -> Result { - if execution.transform_budget.remaining_c14n_output() == 0 - || super::transforms::validate_relationship_chain( - &reference.transforms, - ctx.policy.transforms.opc_relationship_edition, - ) - .is_err() + if super::transforms::validate_relationship_chain( + &reference.transforms, + ctx.policy.transforms.opc_relationship_edition, + ) + .is_err() || reference.transforms.len() > ctx.policy.resources.max_transforms_per_reference || ctx .policy diff --git a/src/xmldsig/verify/tests.rs b/src/xmldsig/verify/tests.rs index d135f310..be208226 100644 --- a/src/xmldsig/verify/tests.rs +++ b/src/xmldsig/verify/tests.rs @@ -509,7 +509,8 @@ fn verification_policy_shares_canonicalization_budget_with_signed_info() { #[test] fn manifest_processing_stops_after_c14n_budget_exhaustion() { // A failed bounded render consumes the remaining operation allowance. - // Later Manifest references, including cheap binary ones, must not run. + // Later Manifest references, including cheap binary ones, must not run: + // the operation ends with the typed limit and returns no results. let digest = base64::engine::general_purpose::STANDARD.encode([0_u8; 32]); let xml = format!( r##"too large{digest}{digest}"## @@ -520,7 +521,7 @@ fn manifest_processing_stops_after_c14n_budget_exhaustion() { .allowed_uri_types(UriTypeSet::ALL) .external_resources(&resources); let document = XmlDocument::parse(xml).expect("test signature must parse"); - let results = document + let error = document .with_view(|view| { let signature = view .document() @@ -564,15 +565,21 @@ fn manifest_processing_stops_after_c14n_budget_exhaustion() { ) .map(|(results, _)| results) }) - .expect("resource exhaustion is reported per Manifest reference"); + .expect_err("resource exhaustion ends the operation"); - assert_eq!(results.len(), 2); - assert!(results.iter().all(|result| { + assert!( matches!( - result.status, - DsigStatus::Invalid(FailureReason::ReferencePolicyViolation { .. }) - ) - })); + &error, + SignatureVerificationPipelineError::Policy( + crate::policy::PolicyViolation::ResourceLimit { + resource: crate::policy::resource_name::CANONICALIZED_BYTES, + maximum: 8, + .. + } + ) + ), + "unexpected error: {error:?}" + ); } #[test] @@ -2433,6 +2440,67 @@ fn verify_context_applies_transform_count_policy_to_manifest_references() { )); } +#[test] +fn a_spent_canonicalization_budget_is_never_a_manifest_finding() { + // The budget is shared by the whole operation, so running out says nothing + // about a Manifest Reference. Sweeping every ceiling reaches both the + // Reference that overruns it and the next one, which finds it spent. + let xml = signature_with_manifest_xml_with_manifest_mutation(true, |xml| { + let start = xml.find("") + .expect("fixture closes the Reference") + + "".len(); + let reference = xml[reference_start..reference_end].to_owned(); + xml.replacen("", &format!("{reference}"), 1) + }); + let mut refused = 0_usize; + let mut completed = 0_usize; + for ceiling in 1..=2048 { + let policy = crate::policy::VerificationPolicy { + manifest_processing: crate::policy::ManifestProcessing::Process, + resources: crate::policy::ResourcePolicy { + max_canonicalized_bytes: ceiling, + ..crate::policy::ResourcePolicy::default() + }, + ..crate::policy::VerificationPolicy::default() + }; + match VerifyContext::new() + .key(&AcceptingKey) + .policy(policy) + .verify(&xml) + { + Ok(result) => { + assert!(matches!(result.status, DsigStatus::Valid), "{ceiling}"); + assert_eq!(result.manifest_references.len(), 2, "{ceiling}"); + assert!( + result + .manifest_references + .iter() + .all(|reference| matches!(reference.status, DsigStatus::Valid)), + "ceiling {ceiling} reported a spent budget as a finding: {:?}", + result + .manifest_references + .iter() + .map(|reference| &reference.status) + .collect::>() + ); + completed += 1; + } + Err(SignatureVerificationPipelineError::Policy( + crate::policy::PolicyViolation::ResourceLimit { .. }, + )) => refused += 1, + Err(error) => panic!("ceiling {ceiling}: {error:?}"), + } + } + assert!(refused > 0 && completed > 0, "{refused} {completed}"); +} + #[test] fn verify_context_skips_manifest_uri_work_when_signature_is_invalid() { // Missing Manifest URIs remain unauthenticated until SignatureValue From 1c72b8d1c0f07b2b93d34365305f21b5b01fe18d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 2 Nov 2026 00:38:22 +0200 Subject: [PATCH 4/4] fix(xmldsig): return a Manifest limit from detached completion Manifest processing now ends with the typed limit, but the detached path did not count it as a spent ceiling: the evidence held the refusal while finish returned Ok. A policy resource limit is returned to the caller as well. A sweep over every ceiling carries the regression. --- src/xmldsig/verify/session/detached.rs | 4 +++ tests/detached_payloads.rs | 45 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/xmldsig/verify/session/detached.rs b/src/xmldsig/verify/session/detached.rs index 69244351..aae45dcf 100644 --- a/src/xmldsig/verify/session/detached.rs +++ b/src/xmldsig/verify/session/detached.rs @@ -637,6 +637,10 @@ impl<'view, 'scope, 'inputs, W: Work> VerifiedSession<'view, 'scope, 'inputs, W> fn resource_refusal(error: &DsigError) -> Option { match error { DsigError::SharedExecution(cause) => Some(DsigError::SharedExecution(*cause)), + DsigError::Policy( + violation @ (crate::policy::PolicyViolation::ResourceLimit { .. } + | crate::policy::PolicyViolation::ResourceLimitExceeded { .. }), + ) => Some(DsigError::Policy(violation.clone())), DsigError::Reference(super::ReferenceProcessingError::SharedExecution(cause)) => Some( DsigError::Reference(super::ReferenceProcessingError::SharedExecution(*cause)), ), diff --git a/tests/detached_payloads.rs b/tests/detached_payloads.rs index 291b5be8..0ddbca8d 100644 --- a/tests/detached_payloads.rs +++ b/tests/detached_payloads.rs @@ -819,6 +819,51 @@ fn a_manifest_digest_refusal_ends_the_detached_completion() { ); } +#[test] +fn a_manifest_policy_limit_ends_the_detached_completion() { + // The Manifest is processed only once the payload completes core validation, + // so a low enough ceiling is spent there. Sweeping every ceiling reaches it: + // the limit must reach the caller as well as the evidence. + let payload = b"detached part"; + let document = manifest_document(payload); + let key = HmacVerificationKey::new(SECRET.to_vec()).unwrap(); + let mut refused = 0_usize; + for ceiling in 1..=4096 { + let mut policy = verification_policy(); + policy.manifest_processing = ManifestProcessing::Process; + policy.resources.max_canonicalized_bytes = ceiling; + let context = VerifyContext::new().policy(policy).key(&key); + let work = work(); + let outcome = with_verified_request( + &document, + &context, + &VerificationRequest::default(), + &work, + |mut session| { + let Some(&reference) = + pending_references(&session, DetachedCompletion::Complete).first() + else { + return Ok(()); + }; + let outcome = stream( + &mut session, + &[reference], + payload, + 4, + DetachedCompletion::Complete, + ); + let in_evidence = session.evidence().signatures()[0].result().is_err(); + assert_eq!(outcome.is_err(), in_evidence, "ceiling {ceiling}"); + outcome + }, + ); + if let Err(DsigError::Policy(PolicyViolation::ResourceLimit { .. })) = outcome { + refused += 1; + } + } + assert!(refused > 0); +} + #[test] fn finalization_is_admitted_before_the_provider_runs() { // Updates can spend the allowance to the last unit; finishing then must