Tackling the 0.1.2alpha TODO list (#16)

Various small refactoring tasks:

* removed nightly feature from mldsa
* Split the Signature trait into Signer and SignatureVerifier
* Split the KEM trait into KEMEncapsulator and KEMDecapsulator
* Touched up some documentation

Author: ounsworth

alpha_0.1.2_release_notes.md

@@ -6,12 +6,8 @@
     * Check the crate release checklist and run claude against the style guide (maybe Francis could cross-check me)
     * Run Crucible testing
     * Add factories for ML-DSA and ML-KEM (if we are keeping factories, see below)
-* Split the Signature trait into a Signer and a Verifier so that, for example, we can implement the verifier for MTC in
-  a different struct from the signer; or so that you can get FIPS compliance on old algorithms that are currently only
-  FIPS-allowed for verification of existing signatures but not for creation of new ones.
 * Check out Megan's email May 13 about KeyMaterial: "I was wondering if there might be scope for a closure based
-  approach that could
-  guarantee encapsulation of the state change from safe to hazardous back to safe again."
+  approach that could guarantee encapsulation of the state change from safe to hazardous back to safe again."
 * Go back to previous algs and apply memory optimization tricks like internal functions. And add a docs section "Memory
   Usage" that measures with valgrind.
 * Ensure that all crates have `#![forbid(missing_docs)]`
@@ -21,40 +17,27 @@
   appropriate.
 * Probably it makes sense to leave Hex and Base64 as requiring std; ... or maybe add a no_std version that uses
   fixed-sized blocks?
+* Make this build on the stable compiler. IE Remove the rust-toolchain.toml file that builds with nightly. Will require
+  some refactoring.
 * Create a cargo feature #[cfg(feature='rng')] and put it around things like keygen that takes an rng so that the build
   dependency on bouncycastle_rng is optional.
-* Enhance the default HashDRBG instantiation to take in NIST-compatible CPU jitter entropy? Or not? Maybe this is the
-  problem of the caller to properly seed the RNG?
 * Factories ... Are they worth it? Michael Richardson says Very Yes. If we are keeping them, then we need a serious
   re-engineering of them because I really dislike that currently they make it hard for the underlying primitive to have
   static one-shot APIs.
-* Add back the Memoable trait from nursery (maybe under a different name) that lets you serialize out the
-  intermediate state, especially important for SHA2, SHA3, and HMAC because TLS needs to be able to fork a state,
-  finalize() a copy and then keep feeding the other copy.
-* Do some science about perf impacts of acting on a local hard-copy vs acting in-place on some specific bit of
-  memory
-* Change the tone of the documentation (both the crate docs and the inline comments) to be less individual ("I"
-  statements) and be more factual ("it is", or "the project", or "the bc-rust library" as appropriate).
-* Relax the requirement on XOF that once you start squeezing, you can't absorb anymore. This will likely need to be an
-  exposed "bell & whistle" because it is an obvious way to do something like the TLS handshake transcript where you need
-  to periodically spit out hashes and then continue absorbing more input. We'll need to study the SHA3 / SHAKE FIPS
-  documents because it might be that this is forbidden as part of the definition of SHAKE, but is allowed if you use the
-  KECCAK primitive raw. We need to make a decision on how to handle this, and provide some sample code in crate docs.
-* Need a rust expert: I use a bunch of #![feature(_)]'s that are only available in nightly. ... what should I do
-  about that?
-* Research task: no_std means that everything is on the stack, which can cause you to blow your stack limit. Research
-  how an application that itself is not no_std can put our large structs (like key objects) on the heap. Is this what
-  Box is for?
 * Deal with as many of the inline TODOs as possible
 * Close all open github issues and document them in this file.
 * After everything is merged, circle back to crucible, and make sure that the harness still works (and maybe remove the
   nightly build toolchain)
 
 # 0.1.2 Features / Changelog
 
-* ML-DSA
-* Low-Memory ML-DSA -- runs in about 1/10th of the usual memory (~ 30 kb of stack) with only minor performance impact.
+* New algorithms added to crypto/ :
+    * mldsa (FIPS 204)
+    * mldsa-lowmemory -- runs in about 1/10th of the usual memory (~ 30 kb of stack) with comparable performance impact.
+    * mlkem (FIPS 203)
+    * mlkem-lowmemory -- runs in about 1/4th of the usual memory (~ 12 kb of stack) with comparable performance impact.
 * All public `*_out(.., out: &mut [u8])` functions now begin by zeroizing the entire output buffer with `.fill(0)`,
   preventing exposure of stale data in oversized output buffers or on early error returns.
 * Github issues resolved:
-    * #2, or whatever
+    * #6: https://github.com/bcgit/bc-rust/issues/6, thanks to Q. T. Felix (github: @Quant-TheodoreFelix)
+    * #10: https://github.com/bcgit/bc-rust/issues/10, thanks to Nicola Tuveri (github: @romen)

cli/src/mldsa_cmd.rs

@@ -2,7 +2,9 @@
 //! by using generics or macros. I just, haven't ... yet.
 
 use crate::helpers::{parse_seed, read_from_file, read_from_file_or_stdin, write_bytes_or_hex};
-use bouncycastle::core::traits::{Signature, SignaturePrivateKey, SignaturePublicKey};
+use bouncycastle::core::traits::{
+    SignaturePrivateKey, SignaturePublicKey, SignatureVerifier, Signer,
+};
 use bouncycastle::hex;
 use bouncycastle::mldsa::{
     HashMLDSA44_with_SHA512, HashMLDSA65_with_SHA512, HashMLDSA87_with_SHA512, MLDSA_SEED_LEN,

cli/src/mlkem_cmd.rs

@@ -6,7 +6,7 @@ use crate::helpers::{
     write_bytes_or_hex_to_file,
 };
 use bouncycastle::core::key_material::KeyMaterialTrait;
-use bouncycastle::core::traits::{KEM, KEMPrivateKey, KEMPublicKey};
+use bouncycastle::core::traits::{KEMDecapsulator, KEMEncapsulator, KEMPrivateKey, KEMPublicKey};
 use bouncycastle::hex;
 use bouncycastle::mlkem::{
     MLKEM512, MLKEM512_CT_LEN, MLKEM512_PK_LEN, MLKEM512_SK_LEN, MLKEM512PrivateKey,

crypto/core-test-framework/src/kem.rs

@@ -1,5 +1,5 @@
 use bouncycastle_core::errors::KEMError;
-use bouncycastle_core::traits::{KEM, KEMPrivateKey, KEMPublicKey};
+use bouncycastle_core::traits::{KEMDecapsulator, KEMEncapsulator, KEMPrivateKey, KEMPublicKey};
 
 pub struct TestFrameworkKEM {
     // Put any config options here
@@ -16,43 +16,48 @@ impl TestFrameworkKEM {
         Self { alg_is_deterministic, is_implicitly_rejecting }
     }
 
-    /// Test all the members of trait Hash against the given input-output pair.
+    /// Test all the members of traits [KEMEncapsulator] and [KEMDecapsulator] against the given input-output pair.
     /// This gives good baseline test coverage, but is not exhaustive.
+    ///
+    /// Since key generation is not part of either KEM trait, the caller supplies a
+    /// `keygen` function pointer (the inherent `keygen` associated function on the algorithm struct).
     pub fn test_kem<
         PK: KEMPublicKey<PK_LEN>,
         SK: KEMPrivateKey<SK_LEN>,
-        KEMAlg: KEM<PK, SK, PK_LEN, SK_LEN, CT_LEN, SS_LEN>,
+        ENCAPSULATOR: KEMEncapsulator<PK, PK_LEN, CT_LEN, SS_LEN>,
+        DECAPSULATOR: KEMDecapsulator<SK, SK_LEN, CT_LEN, SS_LEN>,
         const PK_LEN: usize,
         const SK_LEN: usize,
         const CT_LEN: usize,
         const SS_LEN: usize,
     >(
         &self,
+        keygen: fn() -> Result<(PK, SK), KEMError>,
         run_full_bitflipping_tests: bool,
     ) {
         // Basic test
-        let (pk, sk) = KEMAlg::keygen().unwrap();
-        let (ss, ct) = KEMAlg::encaps(&pk).unwrap();
-        let ss1 = KEMAlg::decaps(&sk, &ct).unwrap();
+        let (pk, sk) = keygen().unwrap();
+        let (ss, ct) = ENCAPSULATOR::encaps(&pk).unwrap();
+        let ss1 = DECAPSULATOR::decaps(&sk, &ct).unwrap();
         assert_eq!(ss, ss1);
 
         // Test non-determinism
         if !self.alg_is_deterministic {
-            let (ss1, ct1) = KEMAlg::encaps(&pk).unwrap();
-            let (ss2, ct2) = KEMAlg::encaps(&pk).unwrap();
+            let (ss1, ct1) = ENCAPSULATOR::encaps(&pk).unwrap();
+            let (ss2, ct2) = ENCAPSULATOR::encaps(&pk).unwrap();
             assert_ne!(ss1, ss2);
             assert_ne!(ct1, ct2);
         }
 
         // Test that decaps fails for broken ct value
-        let (pk, sk) = KEMAlg::keygen().unwrap();
-        let (ss, mut ct) = KEMAlg::encaps(&pk).unwrap();
+        let (pk, sk) = keygen().unwrap();
+        let (ss, mut ct) = ENCAPSULATOR::encaps(&pk).unwrap();
         ct[17] ^= 0xFF;
         if self.is_implicitly_rejecting {
-            let ss2 = KEMAlg::decaps(&sk, &ct).unwrap();
+            let ss2 = DECAPSULATOR::decaps(&sk, &ct).unwrap();
             assert_ne!(ss, ss2);
         } else {
-            match KEMAlg::decaps(&sk, &ct) {
+            match DECAPSULATOR::decaps(&sk, &ct) {
                 Err(KEMError::DecapsulationFailed) =>
                 /* good */
                 {
@@ -71,10 +76,10 @@ impl TestFrameworkKEM {
 
                     // should throw an Err
                     if self.is_implicitly_rejecting {
-                        let ss2 = KEMAlg::decaps(&sk, &ct_copy).unwrap();
+                        let ss2 = DECAPSULATOR::decaps(&sk, &ct_copy).unwrap();
                         assert_ne!(ss, ss2);
                     } else {
-                        match KEMAlg::decaps(&sk, &ct) {
+                        match DECAPSULATOR::decaps(&sk, &ct) {
                             Err(KEMError::DecapsulationFailed) =>
                             /* good */
                             {
@@ -88,19 +93,18 @@ impl TestFrameworkKEM {
         }
 
         // test ct the wrong length
-        let (pk, sk) = KEMAlg::keygen().unwrap();
-        let (_ss, ct) = KEMAlg::encaps(&pk).unwrap();
-
+        let (pk, sk) = keygen().unwrap();
+        let (_ss, ct) = ENCAPSULATOR::encaps(&pk).unwrap();
         // too short
-        match KEMAlg::decaps(&sk, &ct[..CT_LEN - 1]) {
+        match DECAPSULATOR::decaps(&sk, &ct[..CT_LEN - 1]) {
             Err(KEMError::LengthError(_)) => { /* good */ }
             _ => panic!("This should have thrown an error but it didn't."),
         };
 
         // too long
         let mut long_ct = vec![1u8; CT_LEN + 2];
         long_ct.as_mut_slice()[..CT_LEN].copy_from_slice(&ct);
-        match KEMAlg::decaps(&sk, &long_ct) {
+        match DECAPSULATOR::decaps(&sk, &long_ct) {
             Err(KEMError::LengthError(_)) => { /* good */ }
             _ => panic!("This should have thrown an error but it didn't."),
         };
@@ -114,33 +118,31 @@ impl TestFrameworkKEMKeys {
         Self {}
     }
 
+    /// Since key generation is not part of either KEM trait, the caller supplies a
+    /// `keygen` function pointer (the inherent `keygen` associated function on the algorithm struct).
     pub fn test_keys<
         PK: KEMPublicKey<PK_LEN>,
         SK: KEMPrivateKey<SK_LEN>,
-        KEMAlg: KEM<PK, SK, PK_LEN, SK_LEN, CT_LEN, SS_LEN>,
         const PK_LEN: usize,
         const SK_LEN: usize,
-        const CT_LEN: usize,
-        const SS_LEN: usize,
     >(
         &self,
+        keygen: fn() -> Result<(PK, SK), KEMError>,
     ) {
-        self.test_boundary_conditions::<PK, SK, KEMAlg, PK_LEN, SK_LEN, CT_LEN, SS_LEN>();
+        self.test_boundary_conditions::<PK, SK, PK_LEN, SK_LEN>(keygen);
     }
 
     /// Tests the correct behaviour on buffers too large / too small.
     fn test_boundary_conditions<
         PK: KEMPublicKey<PK_LEN>,
         SK: KEMPrivateKey<SK_LEN>,
-        KEMAlg: KEM<PK, SK, PK_LEN, SK_LEN, CT_LEN, SS_LEN>,
         const PK_LEN: usize,
         const SK_LEN: usize,
-        const CT_LEN: usize,
-        const SS_LEN: usize,
     >(
         &self,
+        keygen: fn() -> Result<(PK, SK), KEMError>,
     ) {
-        let (pk, sk) = KEMAlg::keygen().unwrap();
+        let (pk, sk) = keygen().unwrap();
 
         let pk_bytes = pk.encode();
         assert_eq!(pk_bytes.len(), PK_LEN);