feedback from review 2

.github/workflows/build-attest.yaml → .github/workflows/provenance.yaml

@@ -1,6 +1,9 @@
 name: build-attest
 
-on: [ push ]
+on:
+  push:
+    tags:
+      - '*'
 
 jobs:
   build:
@@ -26,13 +29,13 @@ jobs:
       - name: Attest
         uses: actions/attest-build-provenance@v1
         with:
-          subject-path: '${{ github.workspace }}/target/*/release/libfips*'
+          subject-path: '${{ github.workspace }}/target/*/release/libfips204.rlib'
       - name: 'Upload Artifact'
         uses: actions/upload-artifact@v4
         with:
           name: ${{ matrix.target }}--libfips204.rlib
           path: '${{ github.workspace }}/target/*/release/libfips204.rlib'
-          retention-days: 25
+          retention-days: 60
       - name: Checkout actions-oidc-debugger
         uses: actions/checkout@v4
         with:

CHANGELOG.md

@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## 0.2.1 (2024-06-18)
+
+- Internal rework based on review 2 feedback
+- API: try_verify() -> verify() change to prevent usage mistakes
+
 
 ## 0.2.0 (2024-05-25)
 

Cargo.toml

@@ -2,7 +2,7 @@ workspace = { exclude = ["ct_cm4", "dudect", "fuzz", "wasm"] }
 
 [package]
 name = "fips204"
-version = "0.2.0"
+version = "0.2.1"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "FIPS 204 (draft): Module-Lattice-Based Digital Signature"

README.md

@@ -42,7 +42,7 @@ let (pk_recv, msg_recv, sig_recv) = (pk_send, msg_send, sig_send);
 
 // Deserialize the public key and signature, then verify the message
 let pk2 = ml_dsa_44::PublicKey::try_from_bytes(pk_recv)?;
-let v = pk2.try_verify(&msg_recv, &sig_recv)?; // Use the public to verify message signature
+let v = pk2.verify(&msg_recv, &sig_recv); // Use the public to verify message signature
 assert!(v);
 # }
 # Ok(())

benches/benchmark.rs

@@ -59,13 +59,13 @@ pub fn criterion_benchmark(c: &mut Criterion) {
     c.bench_function("ml_dsa_65 esk sign", |b| b.iter(|| esk65.try_sign_with_rng(&mut rng, &msg)));
     c.bench_function("ml_dsa_87 esk sign", |b| b.iter(|| esk87.try_sign_with_rng(&mut rng, &msg)));
 
-    c.bench_function("ml_dsa_44 pk verify", |b| b.iter(|| pk44.try_verify(&msg, &sig44)));
-    c.bench_function("ml_dsa_65 pk verify", |b| b.iter(|| pk65.try_verify(&msg, &sig65)));
-    c.bench_function("ml_dsa_87 pk verify", |b| b.iter(|| pk87.try_verify(&msg, &sig87)));
+    c.bench_function("ml_dsa_44 pk verify", |b| b.iter(|| pk44.verify(&msg, &sig44)));
+    c.bench_function("ml_dsa_65 pk verify", |b| b.iter(|| pk65.verify(&msg, &sig65)));
+    c.bench_function("ml_dsa_87 pk verify", |b| b.iter(|| pk87.verify(&msg, &sig87)));
 
-    c.bench_function("ml_dsa_44 epk verify", |b| b.iter(|| epk44.try_verify(&msg, &sig44)));
-    c.bench_function("ml_dsa_65 epk verify", |b| b.iter(|| epk65.try_verify(&msg, &sig65)));
-    c.bench_function("ml_dsa_87 epk verify", |b| b.iter(|| epk87.try_verify(&msg, &sig87)));
+    c.bench_function("ml_dsa_44 epk verify", |b| b.iter(|| epk44.verify(&msg, &sig44)));
+    c.bench_function("ml_dsa_65 epk verify", |b| b.iter(|| epk65.verify(&msg, &sig65)));
+    c.bench_function("ml_dsa_87 epk verify", |b| b.iter(|| epk87.verify(&msg, &sig87)));
 }
 
 criterion_group!(benches, criterion_benchmark);

ct_cm4/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fips204-ct_cm4"
-version = "0.2.0"
+version = "0.2.1"
 license = "MIT OR Apache-2.0"
 description = "Cortex-M4 testbench for FIPS 204 (draft) ML-DSA"
 authors = ["Eric Schorn <[email protected]>"]

ct_cm4/src/main.rs

@@ -37,7 +37,7 @@ fn main() -> ! {
         let start = DWT::cycle_count();
         asm::isb();
 
-        assert!(pk.try_verify(&MESSAGE, &SIGNATURE).unwrap());
+        assert!(pk.verify(&MESSAGE, &SIGNATURE));
 
         asm::isb();
         let finish = DWT::cycle_count();

dudect/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fips204-dudect"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Eric Schorn <[email protected]>"]
 description = "Dudect testbench for FIPS 204 (draft) ML-DSA"
 publish = false

fuzz/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "fips204-fuzz"
-version = "0.2.0"
+version = "0.2.1"
 publish = false
 edition = "2021"
 license = "MIT OR Apache-2.0"

fuzz/README.md

@@ -12,14 +12,16 @@ $ mkdir -p corpus/fuzz_all
 $ dd if=/dev/zero bs=1 count=6292 > corpus/fuzz_all/seed0
 $ for i in $(seq 1 2); do head -c 6292 </dev/urandom > corpus/fuzz_all/seed$i; done
 $ dd if=/dev/zero bs=1 count=6292 | tr '\0x00' '\377' > corpus/fuzz_all/seed3
-$ cargo fuzz run fuzz_all -j 4 -- -max_total_time=3600  # run twice
+$ cargo fuzz run fuzz_all -j 4 -- -max_total_time=1000  # run twice
 
-#452: cov: 13276 ft: 9644 corp: 84 exec/s 0 oom/timeout/crash: 0/0/0 time: 509s job: 37 dft_time: 0
-#483: cov: 13276 ft: 9651 corp: 86 exec/s 0 oom/timeout/crash: 0/0/0 time: 521s job: 38 dft_time: 0
-#500: cov: 13276 ft: 9652 corp: 87 exec/s 0 oom/timeout/crash: 0/0/0 time: 527s job: 39 dft_time: 0
-#518: cov: 13276 ft: 9652 corp: 87 exec/s 0 oom/timeout/crash: 0/0/0 time: 536s job: 40 dft_time: 0
-#534: cov: 13276 ft: 9681 corp: 89 exec/s 0 oom/timeout/crash: 0/0/0 time: 557s job: 41 dft_time: 0
-#553: cov: 13276 ft: 10255 corp: 92 exec/s 0 oom/timeout/crash: 0/0/0 time: 575s job: 42 dft_time: 0
+#1054: cov: 13559 ft: 10028 corp: 102 exec/s 0 oom/timeout/crash: 0/0/0 time: 899s job: 60 dft_time: 0
+#1084: cov: 13559 ft: 10028 corp: 102 exec/s 0 oom/timeout/crash: 0/0/0 time: 913s job: 61 dft_time: 0
+#1124: cov: 13559 ft: 10028 corp: 102 exec/s 0 oom/timeout/crash: 0/0/0 time: 936s job: 62 dft_time: 0
+#1170: cov: 13559 ft: 10134 corp: 104 exec/s 0 oom/timeout/crash: 0/0/0 time: 954s job: 63 dft_time: 0
+#1207: cov: 13559 ft: 10135 corp: 105 exec/s 0 oom/timeout/crash: 0/0/0 time: 970s job: 64 dft_time: 0
+#1240: cov: 13559 ft: 10135 corp: 105 exec/s 0 oom/timeout/crash: 0/0/0 time: 985s job: 65 dft_time: 0
+INFO: fuzzed for 1001 seconds, wrapping up soon
+INFO: exiting: 0 time: 1004s
 ~~~
 
 Coverage status of ml_dsa_44 is robust, see:

fuzz/fuzz_targets/fuzz_all.rs

@@ -24,31 +24,25 @@ fuzz_target!(|data: [u8; 2560+2420+1312]| {  // sk_len + sig_len + pk_len = 6292
     // Try to use 'fuzzy' sk
     if let Ok(ref sk) = sk_fuzz {
         let sig1 = sk.try_sign(&[0u8, 1, 2, 3]).unwrap();
-        let sk2 = KG::gen_expanded_private(&sk).unwrap();
+        let sk2 = KG::gen_expanded_private(sk).unwrap();
         let sig2 = sk2.try_sign(&[4u8, 5, 6, 7]).unwrap();
         // ...with good pk
-        let res = pk_good.try_verify(&[0u8, 1, 2, 3], &sig1);
-        assert!(res.is_err() || !res.unwrap(), "err 1");
-        let res = pk_good.try_verify(&[0u8, 1, 2, 3], &sig2);
-        assert!(res.is_err() || !res.unwrap(), "err 2");
+        let _res = pk_good.verify(&[0u8, 1, 2, 3], &sig1);
+        let _res = pk_good.verify(&[0u8, 1, 2, 3], &sig2);
     }
 
     // Try to use 'fuzzy' pk
     if let Ok(ref pk) = pk_fuzz {
-        let res = pk.try_verify(&[0u8, 1, 2, 3], &sig_fuzz);
-        assert!(res.is_err() || !res.unwrap(), "err 3");
-        let pk2 = KG::gen_expanded_public(&pk).unwrap();
-        let res = pk2.try_verify(&[0u8, 1, 2, 3], &sig_fuzz);
-        assert!(res.is_err() || !res.unwrap(), "err 4");
+        let _res = pk.verify(&[0u8, 1, 2, 3], &sig_fuzz);
+        let pk2 = KG::gen_expanded_public(pk).unwrap();
+        let _res = pk2.verify(&[0u8, 1, 2, 3], &sig_fuzz);
         // .. with good sig
-        let res = pk2.try_verify(&[0u8, 1, 2, 3], &sig_good);
-        assert!(res.is_err() || !res.unwrap(), "err 5");
+        let _res = pk2.verify(&[0u8, 1, 2, 3], &sig_good);
     }
 
     // Try to use 'fuzzy' sk and 'fuzzy' pk
     if let (Ok(sk), Ok(pk)) = (sk_fuzz, pk_fuzz) {
         let _sig = sk.try_sign(&[0u8, 1, 2, 3]).unwrap();
-        let res = pk.try_verify(&[0u8, 1, 2, 3], &sig_fuzz);
-        assert!(res.is_err() || !res.unwrap(), "err 6");  // hmm, odds of getting good sig on fuzzy signature
+        let _res = pk.verify(&[0u8, 1, 2, 3], &sig_fuzz);
     }
 });

src/conversion.rs

@@ -18,7 +18,7 @@ use crate::Q;
 // algorithms have been reimplemented at a higher level.
 
 
-/// # Algorithm 8: `CoefFromThreeBytes(b0,b1,b2)` on page 21.
+/// # Algorithm 8: `CoeffFromThreeBytes(b0,b1,b2)` on page 21.
 /// Generates an element of `{0, 1, 2, ... , q − 1} ∪ {⊥}` used in rejection sampling.
 /// This function is used during keygen and signing, but only operates on the non-secret
 /// `rho` value stored in the public key, so need not be constant-time in normal
@@ -33,7 +33,7 @@ use crate::Q;
 /// Returns an error `⊥` on input 3 bytes forming values between `Q=0x7F_E0_01`--`0x7F_FF_FF`,
 /// and between `0xFF_E0_01`--`0xFF_FF_FF` (latter range due to masking of bit 7 of byte 2)
 /// per spec; for rejection sampling.
-pub(crate) fn coef_from_three_bytes<const CTEST: bool>(bbb: [u8; 3]) -> Result<i32, &'static str> {
+pub(crate) fn coeff_from_three_bytes<const CTEST: bool>(bbb: [u8; 3]) -> Result<i32, &'static str> {
     // 1: if b2 > 127 then
     // 2: b2 ← b2 − 128     ▷ Set the top bit of b2 to zero
     // 3: end if
@@ -56,7 +56,7 @@ pub(crate) fn coef_from_three_bytes<const CTEST: bool>(bbb: [u8; 3]) -> Result<i
 }
 
 
-/// # Algorithm 9: `CoefFromHalfByte(b)` on page 22.
+/// # Algorithm 9: `CoeffFromHalfByte(b)` on page 22.
 /// Generates an element of `{−η, −η + 1, ... , η} ∪ {⊥}` used in rejection sampling.
 /// This function is used during keygen, but only operates on the hash-derived
 /// `rho_prime` value that is rejection-sampled/expanded into the internal `s_1` and
@@ -72,7 +72,9 @@ pub(crate) fn coef_from_three_bytes<const CTEST: bool>(bbb: [u8; 3]) -> Result<i
 /// # Errors
 /// Returns an error `⊥` on when eta = 4 and b > 8 for rejection sampling. (panics on b > 15)
 #[allow(clippy::cast_possible_truncation)] // rem as u8
-pub(crate) fn coef_from_half_byte<const CTEST: bool>(eta: i32, b: u8) -> Result<i32, &'static str> {
+pub(crate) fn coeff_from_half_byte<const CTEST: bool>(
+    eta: i32, b: u8,
+) -> Result<i32, &'static str> {
     const M5: u32 = ((1u32 << 24) / 5) + 1;
     debug_assert!((eta == 2) || (eta == 4), "Alg 9: incorrect eta");
     debug_assert!(b < 16, "Alg 9: b out of range"); // Note other cases involving b/eta will fall through to Err()
@@ -269,9 +271,9 @@ pub(crate) fn hint_bit_pack<const CTEST: bool, const K: usize>(
             if CTEST || (h[i].0[j] != 0) {
                 //
                 // 6: y[Index] ← j      ▷ Store the locations of the nonzero coefficients in h[i]
-                y_bytes[index] = j.to_le_bytes()[0];
-                //
-                // 7: Index ← Index + 1
+                y_bytes[index] = j.to_le_bytes()[0]; // ...bytes()[0] for clippy benefit ; )
+                                                     //
+                                                     // 7: Index ← Index + 1
                 index += 1;
 
                 // 8: end if
@@ -378,43 +380,43 @@ mod tests {
     #[test]
     fn test_coef_from_three_bytes1() {
         let bytes = [0x12u8, 0x34, 0x56];
-        let res = coef_from_three_bytes::<false>(bytes).unwrap();
+        let res = coeff_from_three_bytes::<false>(bytes).unwrap();
         assert_eq!(res, 0x0056_3412);
     }
 
     #[test]
     fn test_coef_from_three_bytes2() {
         let bytes = [0x12u8, 0x34, 0x80];
-        let res = coef_from_three_bytes::<false>(bytes).unwrap();
+        let res = coeff_from_three_bytes::<false>(bytes).unwrap();
         assert_eq!(res, 0x0000_3412);
     }
 
     #[test]
     fn test_coef_from_three_bytes3() {
         let bytes = [0x01u8, 0xe0, 0x80];
-        let res = coef_from_three_bytes::<false>(bytes).unwrap();
+        let res = coeff_from_three_bytes::<false>(bytes).unwrap();
         assert_eq!(res, 0x0000_e001);
     }
 
     #[test]
     #[should_panic(expected = "panic: out of range")]
     fn test_coef_from_three_bytes4() {
         let bytes = [0x01u8, 0xe0, 0x7f];
-        let res = coef_from_three_bytes::<false>(bytes).expect("panic: out of range");
+        let res = coeff_from_three_bytes::<false>(bytes).expect("panic: out of range");
         assert_eq!(res, 0x0056_3412);
     }
 
     #[test]
     fn test_coef_from_half_byte1() {
         let inp = 3;
-        let res = coef_from_half_byte::<false>(2, inp).unwrap();
+        let res = coeff_from_half_byte::<false>(2, inp).unwrap();
         assert_eq!(-1, res);
     }
 
     #[test]
     fn test_coef_from_half_byte2() {
         let inp = 8;
-        let res = coef_from_half_byte::<false>(4, inp).unwrap();
+        let res = coeff_from_half_byte::<false>(4, inp).unwrap();
         assert_eq!(-4, res);
     }
 
@@ -423,7 +425,7 @@ mod tests {
     #[test]
     fn test_coef_from_half_byte_validation1() {
         let inp = 22;
-        let res = coef_from_half_byte::<false>(2, inp);
+        let res = coeff_from_half_byte::<false>(2, inp);
         assert!(res.is_err());
     }
 
@@ -432,14 +434,14 @@ mod tests {
     #[test]
     fn test_coef_from_half_byte_validation2() {
         let inp = 5;
-        let res = coef_from_half_byte::<false>(1, inp);
+        let res = coeff_from_half_byte::<false>(1, inp);
         assert!(res.is_err());
     }
 
     #[test]
     fn test_coef_from_half_byte_validation3() {
         let inp = 10;
-        let res = coef_from_half_byte::<false>(4, inp);
+        let res = coeff_from_half_byte::<false>(4, inp);
         assert!(res.is_err());
     }
 

src/hashing.rs

@@ -1,6 +1,6 @@
 // This file implements functionality from FIPS 204 section 8.3 Hashing and Pseudorandom Sampling
 
-use crate::conversion::{bit_unpack, coef_from_half_byte, coef_from_three_bytes};
+use crate::conversion::{bit_unpack, coeff_from_half_byte, coeff_from_three_bytes};
 use crate::helpers::{bit_length, is_in_range};
 use crate::types::{R, R0, T, T0};
 use sha3::digest::{ExtendableOutput, Update, XofReader};
@@ -84,8 +84,17 @@ pub(crate) fn sample_in_ball<const CTEST: bool>(tau: i32, rho: &[u8; 32]) -> R {
         // 11: end for
     }
 
+    // slightly redundant...
+    debug_assert!(
+        c.0.iter().map(|&e| usize::from(e != 0)).sum::<usize>() == tau,
+        "Alg 23: bad hamming weight (a)"
+    );
+    debug_assert!(
+        c.0.iter().map(|&e| e & 1).sum::<i32>() == tau.try_into().expect("cannot fail"),
+        "Alg 23: bad hamming weight (b)"
+    );
+
     // 12: return c
-    debug_assert!(c.0.iter().filter(|e| e.abs() != 0).count() == tau, "Alg 23: bad hamming weight");
     c
 }
 
@@ -112,7 +121,7 @@ pub(crate) fn rej_ntt_poly<const CTEST: bool>(rhos: &[&[u8]]) -> T {
         // 4: a_hat[j] ← CoefFromThreeBytes(H128(ρ)[[c]], H128(ρ)[[c + 1]], H128(ρ)[[c + 2]])
         let mut h128pc = [0u8; 3];
         xof.read(&mut h128pc); // implicit c += 3
-        let a_hat_j = coef_from_three_bytes::<CTEST>(h128pc); // gets a result
+        let a_hat_j = coeff_from_three_bytes::<CTEST>(h128pc); // gets a result
 
         // 5: c ← c + 3  (implicit)
 
@@ -162,10 +171,10 @@ pub(crate) fn rej_bounded_poly<const CTEST: bool>(eta: i32, rhos: &[&[u8]]) -> R
         xof.read(&mut z);
 
         // 5: z0 ← CoefFromHalfByte(z mod 16, η)
-        let z0 = coef_from_half_byte::<CTEST>(eta, z[0] & 0x0f);
+        let z0 = coeff_from_half_byte::<CTEST>(eta, z[0] & 0x0f);
 
         // 6: z1 ← CoefFromHalfByte(⌊z/16⌋, η)
-        let z1 = coef_from_half_byte::<CTEST>(eta, z[0] >> 4);
+        let z1 = coeff_from_half_byte::<CTEST>(eta, z[0] >> 4);
 
         // 7: if z0 != ⊥ then
         if let Ok(z0) = z0 {
@@ -280,8 +289,8 @@ pub(crate) fn expand_mask<const L: usize>(gamma1: i32, rho: &[u8; 64], mu: u16)
     for r in 0..u16::try_from(L).expect("cannot fail") {
         //
         // 3: n ← IntegerToBits(µ + r, 16)
-        debug_assert!((mu + r < 1024), "Alg 28: mu + r out of range"); // TODO consider revising
-        let n = mu + r;
+        // debug_assert!((mu + r < 1024), "Alg 28: mu + r out of range");   // arbitrary limit not needed
+        let n = mu + r; // This will perform overflow check in debug, which removes need for above assert
 
         // 4: v ← (H(ρ || n)[[32rc]], H(ρ || n)[[32rc+1]], ..., H(ρ || n)[[32rc+32c − 1]])
         let mut xof = h_xof(&[rho, &n.to_le_bytes()]);
@@ -290,7 +299,7 @@ pub(crate) fn expand_mask<const L: usize>(gamma1: i32, rho: &[u8; 64], mu: u16)
         // 5: s[r] ← BitUnpack(v, γ_1 − 1, γ_1)
         s[r as usize] = bit_unpack(&v[0..32 * c], gamma1 - 1, gamma1).expect("cannot fail");
         debug_assert!(
-            s.iter().all(|r| is_in_range(r, gamma1, gamma1)),
+            s.iter().all(|r| is_in_range(r, gamma1 - 1, gamma1)),
             "Alg 28: s coeff out of range"
         );