fix(pq): clear SHAKE state on drop

.github/workflows/ci.yml

@@ -287,6 +287,10 @@ jobs:
           cargo clippy -p xml-sec --lib --no-default-features
           --features cms,xml-backend-xmloxide
           --target thumbv7em-none-eabihf -- -D warnings
+      - run: >-
+          cargo clippy -p xml-sec --lib --no-default-features
+          --features experimental-pq,xml-backend-roxmltree
+          --target thumbv7em-none-eabihf -- -D warnings
       - run: >-
           cargo clippy -p xml-sec --lib --no-default-features
           --features dstu4145,cms,xml-backend-xmloxide

Cargo.toml

@@ -328,7 +328,7 @@ common = { package = "crypto-common", version = "0.2", default-features = false,
 ctutils = { version = "0.4", default-features = false, optional = true }
 hybrid-array = { version = "0.4", default-features = false, features = ["extra-sizes", "alloc", "zeroize"], optional = true }
 module-lattice = { version = "0.2.3", default-features = false, features = ["ctutils", "alloc", "zeroize"], optional = true }
-shake = { version = "0.1", default-features = false, optional = true }
+shake = { version = "0.1", default-features = false, features = ["zeroize"], optional = true }
 const-oid = { version = "0.10", default-features = false, features = ["db"], optional = true }
 slh-dsa = { version = "=0.2.0-rc.5", default-features = false, features = ["alloc", "zeroize"], optional = true }
 ml-kem = { version = "0.3.2", default-features = false, features = ["alloc", "zeroize"], optional = true }

docs/ml-dsa-patch.md

@@ -28,6 +28,11 @@ Expanded-only keys are exported as expanded keys, never a fabricated seed.
 Secret staging buffers are zeroized. All public signing and verification still
 pass through `CryptoProvider`.
 
+`experimental-pq` enables `shake/zeroize`, so the `Shake` and `ShakeReader` values
+clear their own state and cursor when dropped. This is dependency-owned clearing of
+those values only: it does not reach compiler-generated copies or other temporaries,
+and it is no claim of complete secret erasure.
+
 Namespace, feature and Rust 1.92 lint adaptations let the same algorithm live as a private
 module. They do not change cryptographic behavior. Inspect the generated delta
 in `scripts/patches/ml-dsa-0.1.1.patch` when updating the donor.

src/key_establishment.rs

@@ -313,7 +313,11 @@ mod tests {
             "failed-random"
         }
         fn supports(&self, capability: ProviderCapability<'_>) -> bool {
-            RustCryptoProvider.supports(capability)
+            // Its own source, which fails, exists in every build.
+            matches!(
+                capability,
+                ProviderCapability::Random | ProviderCapability::Encapsulate(_)
+            ) || RustCryptoProvider.supports(capability)
         }
         fn fill_random(&self, _: &mut [u8]) -> Result<(), ProviderError> {
             Err(ProviderError::Random("test entropy failure".into()))
@@ -431,7 +435,7 @@ mod tests {
         ));
         assert_eq!(usage.encapsulation_operations, 1);
         assert!(matches!(
-            usage.encapsulate(&policy, &RustCryptoProvider, &key),
+            usage.encapsulate(&policy, &FailedRandom, &key),
             Err(KeyEstablishmentError::Policy(
                 PolicyViolation::ResourceLimit { .. }
             ))

src/provider/key_encapsulation/rustcrypto.rs

@@ -5,6 +5,7 @@ use super::{
     KeyEncapsulationKey, ProviderError, Zeroizing,
 };
 use crate::provider::{ProviderCapability, ProviderInputError};
+use alloc::vec::Vec;
 use der::{
     AnyRef, Decode, Encode, Reader, Tag, TagMode, TagNumber, Tagged,
     asn1::{BitStringRef, ContextSpecific, OctetStringRef},
@@ -546,6 +547,22 @@ mod tests {
         KeyEncapsulationAlgorithm::MlKem1024,
     ];
 
+    #[cfg(not(feature = "xmldsig-sign"))]
+    #[test]
+    fn encapsulation_without_a_randomness_source_is_unsupported() {
+        // FIPS 203 section 7.2 needs fresh m, and a build without the bundled
+        // source has none: refusing is the only answer that is not a weak key.
+        // https://doi.org/10.6028/NIST.FIPS.203
+        let key = RustCryptoMlKemPrivateKey::from_seed(ALGORITHMS[0], &[42; 64])
+            .expect("seed")
+            .public_key();
+        assert!(matches!(
+            RustCryptoProvider.encapsulate_key(&key),
+            Err(ProviderError::Unsupported { .. })
+        ));
+    }
+
+    #[cfg(feature = "xmldsig-sign")]
     #[test]
     fn all_parameter_sets_roundtrip_every_private_encoding() {
         // RFC 9935 section 6 permits every CHOICE branch, not just the preferred seed.
@@ -616,6 +633,7 @@ mod tests {
         }
     }
 
+    #[cfg(feature = "xmldsig-sign")]
     #[test]
     fn valid_size_invalid_ciphertext_is_implicitly_rejected() {
         // FIPS 203 section 7.3 returns a deterministic fallback secret, never an oracle bit.

tests/shake_state_clearing.rs

@@ -0,0 +1,24 @@
+//! The SHAKE state of the embedded ML-DSA code is cleared when it is dropped.
+//!
+//! A workspace build cannot detect a missing `shake/zeroize`, because
+//! dev-dependencies enable it too; `check-features experimental` proves it on an
+//! isolated consumer. This target pins the marker the feature must produce.
+#![cfg(feature = "experimental-pq")]
+
+use shake::{Shake128, Shake128Reader, Shake256, Shake256Reader};
+use zeroize::ZeroizeOnDrop;
+
+const fn clears_on_drop<T: ZeroizeOnDrop>() {}
+
+const _: () = {
+    clears_on_drop::<Shake128>();
+    clears_on_drop::<Shake256>();
+};
+
+#[test]
+fn readers_clear_through_their_own_drop() {
+    // A reader has no marker: its Drop always exists and clears under the same
+    // feature as the hasher's, so the assertion above stands for both.
+    assert!(core::mem::needs_drop::<Shake128Reader>());
+    assert!(core::mem::needs_drop::<Shake256Reader>());
+}

tools/xml-sec-tools/src/check_features.rs

@@ -2,7 +2,9 @@
 //! feature profile of the core package alone. Workspace feature unification
 //! could otherwise hide a broken gate.
 
-use crate::repo::{Failure, Run, Step, System, TOOL, require};
+use crate::repo::{Failure, Run, Step, System, TOOL, require, root};
+use serde_json::Value;
+use std::path::Path;
 
 #[cfg(test)]
 mod tests;
@@ -30,6 +32,9 @@ const CMS_FORBIDDEN: &[&str] = VERIFY_ONLY_FORBIDDEN
 
 const BACKEND_DIAGNOSTIC: &str = "compile at least one XML backend";
 
+/// What the embedded post-quantum code relies on from its dependencies.
+const EXPERIMENTAL_CONSUMER: [(&str, &str); 1] = [("shake", "zeroize")];
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) enum Phase {
     All,
@@ -63,6 +68,9 @@ pub(crate) enum Action {
     VerifyOnlyGraph(Step, &'static [&'static str]),
     /// A build that must fail with the missing-backend diagnostic.
     MissingBackend(Step),
+    /// `cargo metadata` of a throwaway consumer, whose graph must enable each
+    /// `(package, feature)` pair without asking for it.
+    ConsumerFeatures(Step, &'static [(&'static str, &'static str)]),
 }
 
 struct Profile {
@@ -136,7 +144,7 @@ const PROFILES: [Profile; 17] = [
     profile(
         "experimental",
         Some("experimental-pq,xmldsig-sign,xml-backend-xmloxide"),
-        &[],
+        &["shake_state_clearing"],
         true,
     ),
     profile(
@@ -227,6 +235,16 @@ pub(crate) fn plan(
             forbidden,
         ));
     }
+    if phase.includes(Phase::Lint) && profile == "experimental" {
+        actions.push(Action::ConsumerFeatures(
+            Step::new(
+                "consumer graph",
+                "cargo",
+                &["metadata", "--format-version", "1"],
+            ),
+            &EXPERIMENTAL_CONSUMER,
+        ));
+    }
     if phase.includes(Phase::Lint) {
         let clippy = [
             &["clippy"][..],
@@ -263,6 +281,50 @@ pub(crate) fn forbidden_crate(graph: &str, forbidden: &[&'static str]) -> Option
     })
 }
 
+/// A manifest outside the workspace that depends on `root` like a downstream
+/// crate: no dev-dependency or workspace member can unify features into it.
+fn consumer_manifest(root: &Path) -> String {
+    format!(
+        "[package]\nname = \"feature-consumer\"\nversion = \"0.0.0\"\nedition = \"2024\"\n\n\
+         [lib]\npath = \"lib.rs\"\n\n[workspace]\n\n[dependencies]\n\
+         xml-sec = {{ path = '{}', default-features = false, \
+         features = [\"experimental-pq\", \"xml-backend-roxmltree\"] }}\n",
+        root.display()
+    )
+}
+
+/// The first `(package, feature)` of `required` that `metadata`, the output of
+/// `cargo metadata`, leaves off. A package missing from the graph counts: an
+/// absent crate must not pass as a clean one.
+fn missing_feature(
+    metadata: &str,
+    required: &[(&'static str, &'static str)],
+) -> Result<Option<(&'static str, &'static str)>, String> {
+    let metadata: Value =
+        serde_json::from_str(metadata).map_err(|error| format!("parse cargo metadata: {error}"))?;
+    let list = |value: Option<&Value>| value.and_then(Value::as_array).cloned().unwrap_or_default();
+    let packages = list(metadata.get("packages"));
+    let nodes = list(metadata.pointer("/resolve/nodes"));
+    Ok(required.iter().copied().find(|(package, feature)| {
+        let mut identities = packages
+            .iter()
+            .filter(|candidate| candidate.get("name").and_then(Value::as_str) == Some(package))
+            .filter_map(|candidate| candidate.get("id"))
+            .peekable();
+        identities.peek().is_none()
+            || identities.any(|identity| {
+                !nodes
+                    .iter()
+                    .filter(|node| node.get("id") == Some(identity))
+                    .any(|node| {
+                        list(node.get("features"))
+                            .iter()
+                            .any(|name| name.as_str() == Some(feature))
+                    })
+            })
+    }))
+}
+
 /// Executes `actions` in order; the first failure stops qualification.
 pub(crate) fn execute(actions: &[Action], runner: &mut impl Run) -> Result<(), Failure> {
     for action in actions {
@@ -275,6 +337,24 @@ pub(crate) fn execute(actions: &[Action], runner: &mut impl Run) -> Result<(), F
                     return Err(format!("verification-only graph contains {name}").into());
                 }
             }
+            Action::ConsumerFeatures(step, required) => {
+                let consumer = tempfile::tempdir().map_err(|error| error.to_string())?;
+                let write = |name: &str, text: String| {
+                    std::fs::write(consumer.path().join(name), text)
+                        .map_err(|error| error.to_string())
+                };
+                write("Cargo.toml", consumer_manifest(&root()))?;
+                write("lib.rs", String::new())?;
+                let (status, metadata) =
+                    runner.stdout(&step.clone().in_directory(consumer.path()))?;
+                require(status)?;
+                if let Some((package, feature)) = missing_feature(&metadata, required)? {
+                    return Err(format!(
+                        "a downstream consumer does not enable {package}/{feature}"
+                    )
+                    .into());
+                }
+            }
             Action::MissingBackend(step) => {
                 let (status, diagnostic) = runner.combined(step)?;
                 if status.success() {

tools/xml-sec-tools/src/check_features/tests.rs

@@ -1,6 +1,10 @@
-use super::{Action, VERIFY_ONLY_FORBIDDEN, execute, forbidden_crate, plan};
+use super::{
+    Action, VERIFY_ONLY_FORBIDDEN, consumer_manifest, execute, forbidden_crate, missing_feature,
+    plan,
+};
 use crate::repo::{Failure, Run, Step};
 use std::os::unix::process::ExitStatusExt as _;
+use std::path::Path;
 use std::process::ExitStatus;
 
 /// The planned command lines, as the original runner's Cargo log recorded them.
@@ -10,7 +14,8 @@ fn lines(actions: &[Action]) -> Vec<String> {
         .map(|action| {
             let (Action::Run(step)
             | Action::VerifyOnlyGraph(step, _)
-            | Action::MissingBackend(step)) = action;
+            | Action::MissingBackend(step)
+            | Action::ConsumerFeatures(step, _)) = action;
             step.args.join(" ")
         })
         .collect()
@@ -228,13 +233,76 @@ fn each_minimal_profile_selects_only_its_declared_feature_graph() {
                 "tree -p xml-sec --no-default-features --features {features} -e normal --prefix none --format {{p}}"
             ));
         }
+        if profile == "experimental" {
+            expected.push("metadata --format-version 1".to_owned());
+        }
         expected.push(format!(
             "clippy -p xml-sec --no-default-features --features {features} --all-targets -- -D warnings"
         ));
         assert_eq!(planned(profile, "lint"), expected, "{profile}");
     }
 }
 
+/// `cargo metadata` output naming `shake` with `features`.
+fn shake_metadata(features: &str) -> String {
+    format!(
+        r#"{{"packages":[{{"name":"shake","id":"id#shake"}},{{"name":"other","id":"id#other"}}],
+        "resolve":{{"nodes":[{{"id":"id#shake","features":[{features}]}},
+        {{"id":"id#other","features":["zeroize"]}}]}}}}"#
+    )
+}
+
+#[test]
+fn a_consumer_without_shake_state_clearing_fails_qualification() {
+    // Another crate's feature of the same name must not pass for shake's; this
+    // is the graph a downstream crate resolved before the manifest asked for it.
+    let actions = plan("experimental", "lint", None).unwrap();
+    assert!(matches!(actions[0], Action::ConsumerFeatures(..)));
+    let mut runner = Recorded {
+        output: shake_metadata(""),
+        ..Recorded::default()
+    };
+    assert_eq!(
+        execute(&actions, &mut runner).unwrap_err(),
+        Failure {
+            code: 1,
+            message: "a downstream consumer does not enable shake/zeroize".to_owned(),
+        }
+    );
+    assert_eq!(runner.calls, 1);
+}
+
+#[test]
+fn a_consumer_graph_without_the_package_is_no_proof() {
+    // A renamed or dropped dependency must fail the check, not satisfy it.
+    assert_eq!(
+        missing_feature(
+            r#"{"packages":[],"resolve":{"nodes":[]}}"#,
+            &[("shake", "zeroize")]
+        ),
+        Ok(Some(("shake", "zeroize")))
+    );
+    assert!(missing_feature("not json", &[("shake", "zeroize")]).is_err());
+}
+
+#[test]
+fn a_consumer_with_shake_state_clearing_passes() {
+    assert_eq!(
+        missing_feature(&shake_metadata(r#""zeroize""#), &[("shake", "zeroize")]),
+        Ok(None)
+    );
+}
+
+#[test]
+fn the_consumer_stands_outside_the_workspace() {
+    // Its own empty workspace table is what stops feature unification, and
+    // default features stay off as they do for a minimal downstream crate.
+    let manifest = consumer_manifest(Path::new("/repository"));
+    assert!(manifest.contains("\n[workspace]\n"));
+    assert!(manifest.contains("path = '/repository', default-features = false"));
+    assert!(!manifest.contains("dev-dependencies"));
+}
+
 #[test]
 fn the_dstu_profile_runs_its_interoperability_tests_and_no_signing_facade() {
     // The feature only verifies, so the signing facade has no place here, and