Tackling the 0.1.2alpha TODO list

.github/workflows/publish_doc_benches_to_ghpages.yaml

@@ -37,21 +37,22 @@ jobs:
         with:
           name: code_quality_stats
           path: ./code_stats.txt
-  run_benches:
-    runs-on: ubuntu-latest
-    if: github.ref == 'refs/heads/main'
-    steps:
-      - name: Checkout code
-        uses: actions/checkout@v4
-      - run: cargo bench --all
-      - name: Save artifacts
-        uses: actions/upload-artifact@v4
-        with:
-          name: bc-rust-benches
-          path: ./target/criterion
+  # the benches run crazy slow on the github agent. So there's really no point because it's not useful data.
+  # run_benches:
+  #   runs-on: ubuntu-latest
+  #   if: github.ref == 'refs/heads/main'
+  #   steps:
+  #     - name: Checkout code
+  #       uses: actions/checkout@v4
+  #     - run: cargo bench --all
+  #     - name: Save artifacts
+  #       uses: actions/upload-artifact@v4
+  #       with:
+  #         name: bc-rust-benches
+  #         path: ./target/criterion
   collect_ghpages:
     if: github.ref == 'refs/heads/main'
-    needs: [build_docs, code_stats, run_benches]
+    needs: [build_docs, code_stats]
     runs-on: ubuntu-latest
     steps:
       - run: mkdir ./gh-pages
@@ -65,11 +66,11 @@ jobs:
         with:
           name: code_quality_stats
           path: ./gh-pages/
-      - name: Get benches from previous job
-        uses: actions/download-artifact@v4
-        with:
-          name: bc-rust-benches
-          path: ./gh-pages/benches
+      # - name: Get benches from previous job
+      #   uses: actions/download-artifact@v4
+      #   with:
+      #     name: bc-rust-benches
+      #     path: ./gh-pages/benches
       - name: Archive Compatibility Matrix For Download
         uses:  actions/upload-pages-artifact@v3
         with:

.github/workflows/rust-style.yml

@@ -0,0 +1,28 @@
+name: Rust Style
+
+on:
+  pull_request:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: rust-style-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  rustfmt:
+    if: github.repository == 'bcgit/bc-rust'
+    name: rustfmt
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Install nightly rustfmt
+        run: |
+          rustup toolchain install nightly --profile minimal --component rustfmt
+          rustup override set nightly
+
+      - name: Check formatting
+        run: cargo fmt --all --check

CONTRIBUTING.md

@@ -20,6 +20,23 @@ before posting anything public. See [Security Policy](SECURITY.md).
 
 If a related discussion or issue doesn't exist, and the issue is not security related, you can [open a new issue](https://github.com/bcgit/bc-java/issues/new). An issue can be converted into a discussion if regarded as one.
 
+## Coding philosophy
+
+> Slow is smooth, smooth is fast.
+
+There is a time and a place for "Move fast and break things", but the source code of a crypto library is not one of them.
+
+This project takes the philosophy that taking the time to do things right pays off in the long run, both in terms of 
+the runtime and memory footprint of the code, and it terms of the time required for a future maintainer to get up to speed with the code
+and avoid introducing bugs due to the code being hard to understand.
+
+Some specifics:
+
+* Respect that the innovative process sometimes requires exploring several dead-ends before you find the most elegant solution.
+* Public APIs of a library should be both ergonomic and expressive. When defining a new trait or public function, ask yourself whether a programmer who is new to cryptography is likely to use this in a way that will get them into trouble.
+* Variables should be well-named, well-structured, and well-commented (a comment-to-code ration of 1:1 is a goal to be strived for!). Think about memory footprint and, where possible, use unnamed scopes to allow the compiler to pop intermediate value variables off the stack as soon as they are no longer needed.
+* Always run your code through `cargo mutants` and get the issue count as low as your can. As a first pass, this forces you to write thorough unit tests. As a second pass, this draws your attention to bits of your code that cannot be tested from the outside. Often this means that the code can be simplified without affecting functionality (as defined by your set of unit tests) -- "simpler code" usually means faster runtime and easier future maintenance.
+
 ## Contribute to the code
 
 For substantial, non-trivial contributions, you may be asked to sign a contributor assignment agreement. Optionally, you can also have your name and contact information listed in [Contributors](https://www.bouncycastle.org/contributors.html). 
@@ -56,5 +73,5 @@ Don't forget to self-review. Please follow these simple guidelines:
 
 #### Your pull request is merged
 
-For acceptance, pull requests need to meet specific quality criteria, including tests for anything substantial. Someone on the Bouncy Castle core team will review the pull request when there is time, and let you know if something is missing or suggest improvements. If it is a useful and generic feature it will be integrated in Bouncy Castle to be available in a later release.
+Someone on the Bouncy Castle core team will review the pull request when there is time, and let you know if something is missing or suggest improvements. If it is a useful and generic feature it will be integrated in Bouncy Castle to be available in a later release.
 

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace]
-members = [ "cli", "crypto/*", "mem_usage_benches" ]
+members = ["cli", "crypto/*", "mem_usage_benches"]
 
 [workspace.package]
 edition = "2024"
@@ -8,21 +8,21 @@ edition = "2024"
 
 # *** Internal Dependencies ***
 bouncycastle = { path = "./", version = "0.1.1" }
-bouncycastle-base64 = { path = "./crypto/base64", version = "0.1.1"}
+bouncycastle-base64 = { path = "./crypto/base64", version = "0.1.1" }
 bouncycastle-core = { path = "crypto/core", version = "0.1.1" }
-bouncycastle-core-test-framework = { path = "./crypto/core-test-framework", version = "0.1.1"}
-bouncycastle-factory = { path = "./crypto/factory", version = "0.1.1"}
+bouncycastle-core-test-framework = { path = "./crypto/core-test-framework", version = "0.1.1" }
+bouncycastle-factory = { path = "./crypto/factory", version = "0.1.1" }
 bouncycastle-hex = { path = "./crypto/hex", version = "0.1.1" }
-bouncycastle-hkdf = { path = "./crypto/hkdf", version = "0.1.1"}
-bouncycastle-hmac = { path = "./crypto/hmac", version = "0.1.1"}
+bouncycastle-hkdf = { path = "./crypto/hkdf", version = "0.1.1" }
+bouncycastle-hmac = { path = "./crypto/hmac", version = "0.1.1" }
 bouncycastle-mlkem = { path = "./crypto/mlkem", version = "0.1.2" }
-bouncycastle-mlkem-lowmemory = { path = "./crypto/mlkem_lowmemory", version = "0.1.2" }
+bouncycastle-mlkem-lowmemory = { path = "./crypto/mlkem-lowmemory", version = "0.1.2" }
 bouncycastle-mldsa = { path = "./crypto/mldsa", version = "0.1.2" }
-bouncycastle-mldsa-lowmemory = { path = "./crypto/mldsa_lowmemory", version = "0.1.2" }
+bouncycastle-mldsa-lowmemory = { path = "./crypto/mldsa-lowmemory", version = "0.1.2" }
 bouncycastle-rng = { path = "./crypto/rng", version = "0.1.1" }
-bouncycastle-sha2 = { path = "./crypto/sha2", version = "0.1.1"}
-bouncycastle-sha3 = { path = "./crypto/sha3", version = "0.1.1"}
-bouncycastle-utils = { path = "./crypto/utils", version = "0.1.1"}
+bouncycastle-sha2 = { path = "./crypto/sha2", version = "0.1.1" }
+bouncycastle-sha3 = { path = "./crypto/sha3", version = "0.1.1" }
+bouncycastle-utils = { path = "./crypto/utils", version = "0.1.1" }
 
 
 # *** External Dependencies ***

alpha_0.1.2_release_notes.md

@@ -12,8 +12,6 @@
 * 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."
-* Anywhere that you have an `_out(.. out: &mut [u8])`, start by zeroizing it with .fill(0); .. a good task for Claude?
-  And should be documented in the style guide?
 * 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)]`
@@ -23,6 +21,8 @@
   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
@@ -49,10 +49,18 @@
   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/encoders_cmd.rs

@@ -1,17 +1,17 @@
 use std::io;
 use std::io::{Read, Write};
 
-use bouncycastle::hex;
 use bouncycastle::base64;
+use bouncycastle::hex;
 
 pub(crate) fn hex_encode_cmd() {
     // Stream from stdin to stdout in chunks of 1 kb
     let mut buf: [u8; 1024] = [0u8; 1024];
     let mut bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     while bytes_read != 0 {
-        io::stdout().write_all(
-            hex::encode(&buf[..bytes_read]).as_bytes()
-        ).expect("Failed to write to stdout");
+        io::stdout()
+            .write_all(hex::encode(&buf[..bytes_read]).as_bytes())
+            .expect("Failed to write to stdout");
 
         bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     }
@@ -22,13 +22,12 @@ pub(crate) fn hex_decode_cmd() {
     let mut buf: [u8; 1024] = [0u8; 1024];
     let mut bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     while bytes_read != 0 {
-        let chunk_str: String = String::from_utf8(
-            Vec::from(&buf[..bytes_read])
-        ).expect("Input was not valid utf8.");
+        let chunk_str: String =
+            String::from_utf8(Vec::from(&buf[..bytes_read])).expect("Input was not valid utf8.");
 
-        io::stdout().write_all(
-            &*hex::decode(chunk_str.as_str()).expect("Input was not valid hex.")
-        ).expect("Failed to write to stdout");
+        io::stdout()
+            .write_all(&*hex::decode(chunk_str.as_str()).expect("Input was not valid hex."))
+            .expect("Failed to write to stdout");
 
         bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     }
@@ -40,9 +39,9 @@ pub(crate) fn base64_encode_cmd() {
     let mut buf: [u8; 1024] = [0u8; 1024];
     let mut bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     while bytes_read != 0 {
-        io::stdout().write_all(
-            encoder.do_update(&buf[..bytes_read]).as_bytes()
-        ).expect("Failed to write to stdout");
+        io::stdout()
+            .write_all(encoder.do_update(&buf[..bytes_read]).as_bytes())
+            .expect("Failed to write to stdout");
 
         bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     }
@@ -54,14 +53,18 @@ pub(crate) fn base64_decode_cmd() {
     let mut decoder = base64::Base64Decoder::new(true);
     let mut bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     while bytes_read != 0 {
-        let chunk_str: String = String::from_utf8(
-            Vec::from(&buf[..bytes_read])
-        ).expect("Input was not valid utf8.");
+        let chunk_str: String =
+            String::from_utf8(Vec::from(&buf[..bytes_read])).expect("Input was not valid utf8.");
 
-        io::stdout().write_all(
-            decoder.do_update(chunk_str.as_str()).expect("Input was not valid base64.").as_slice()
-        ).expect("Failed to write to stdout");
+        io::stdout()
+            .write_all(
+                decoder
+                    .do_update(chunk_str.as_str())
+                    .expect("Input was not valid base64.")
+                    .as_slice(),
+            )
+            .expect("Failed to write to stdout");
 
         bytes_read = io::stdin().read(&mut buf).expect("Failed to read from stdin");
     }
-}
+}

cli/src/helpers.rs

@@ -1,31 +1,31 @@
-use std::{io};
+use bouncycastle::core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType};
+use bouncycastle::core::traits::SecurityStrength;
+use bouncycastle::hex;
 use std::fs::File;
+use std::io;
 use std::io::{Read, Write};
 use std::process::exit;
-use bouncycastle::core::key_material::{KeyMaterial, KeyMaterialTrait, KeyType};
-use bouncycastle::core::traits::{SecurityStrength};
-use bouncycastle::hex;
 
 /// Reads either bin or hex
 pub(crate) fn read_from_file(filename: &str) -> Vec<u8> {
     let file = File::open(&filename);
     if file.is_ok() {
         let mut buf = Vec::<u8>::new();
         match file.unwrap().read_to_end(&mut buf) {
-            Ok(_bytes_read) => { 
+            Ok(_bytes_read) => {
                 // try hex decoding it
                 match hex::decode(&buf) {
-                    Ok(decoded) => { decoded },
+                    Ok(decoded) => decoded,
                     Err(_) => {
                         // well, it's not hex, so return it raw
                         buf
-                    },
+                    }
                 }
-            },
+            }
             Err(_) => {
                 eprintln!("Error: couldn't open file '{}'", &filename);
                 exit(-1);
-            },
+            }
         }
     } else {
         eprintln!("Error: couldn't open file '{}'", &filename);
@@ -35,22 +35,21 @@ pub(crate) fn read_from_file(filename: &str) -> Vec<u8> {
 
 /// Reads either bin or hex
 pub(crate) fn read_from_file_or_stdin(filename: &Option<String>) -> Vec<u8> {
-
     if filename.is_some() {
         // This already reads either bin or hex
         return read_from_file(filename.as_ref().unwrap());
     }
-    
+
     let mut buf = Vec::<u8>::new();
     io::stdin().read_to_end(&mut buf).expect("Failed to read from stdin");
 
     // try hex decoding it
     match hex::decode(&buf) {
-        Ok(decoded) => { decoded },
+        Ok(decoded) => decoded,
         Err(_) => {
             // well, it's not hex, so return it raw
             buf
-        },
+        }
     }
 }
 
@@ -84,15 +83,15 @@ pub(crate) fn parse_seed<const SEED_LEN: usize>(bytes: &[u8]) -> Result<KeyMater
     // try decoding it as hex first
     let seed_bytes: [u8; SEED_LEN] = match &hex::decode(&bytes) {
         Ok(decoded_bytes) => {
-            if decoded_bytes.len() < SEED_LEN || decoded_bytes.len() > SEED_LEN +1  {
+            if decoded_bytes.len() < SEED_LEN || decoded_bytes.len() > SEED_LEN + 1 {
                 // it was valid hex, but the wrong length
                 return Err(());
             }
             decoded_bytes[..SEED_LEN].try_into().unwrap()
         }
         Err(_) => {
             // it's not hex, so take the fist SEED_LEN bytes of the raw binary
-            if bytes.len() < SEED_LEN || bytes.len() > SEED_LEN +1 {
+            if bytes.len() < SEED_LEN || bytes.len() > SEED_LEN + 1 {
                 return Err(());
             }
             bytes[..SEED_LEN].try_into().unwrap()
@@ -113,4 +112,4 @@ pub(crate) fn parse_seed<const SEED_LEN: usize>(bytes: &[u8]) -> Result<KeyMater
         seed.drop_hazardous_operations();
     }
     Ok(seed)
-}
+}