fix: audit and common fixes

.github/workflows/bindings.yaml

@@ -94,25 +94,6 @@ jobs:
           name: mfkdf2-web-mochawesome-report
           path: mfkdf2-web/test-results/mochawesome/
 
-
-      - name: Generate TypeScript bindings for differential tests
-        working-directory: mfkdf2-web
-        run: npm run ubrn:web:differential:release
-
-      - name: Copy index.web.ts implementation
-        run: cp mfkdf2-web/src/index.ts mfkdf2-web/src/index.web.ts
-
-      - name: Run differential tests with reports
-        working-directory: mfkdf2-web
-        run: npm run test:differential:report
-
-      - name: Upload differential HTML test report
-        if: always()
-        uses: actions/upload-artifact@v4
-        with:
-          name: mfkdf2-web-differential-report
-          path: mfkdf2-web/test-results/mochawesome-differential/
-
       - name: Run TypeScript type checking
         working-directory: mfkdf2-web
         run: npm run typecheck

.github/workflows/differential.yaml

@@ -0,0 +1,82 @@
+name: Differential Tests
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+on:
+  workflow_dispatch:
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  differential-tests:
+    name: differential
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout pinned mfkdf2.rs commit
+        uses: actions/checkout@v4
+        with:
+          ref: 7c33c7164d6e40a26c0899f19b8f9ad9b9f0c029
+
+      - name: Install Rust
+        uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: stable
+          targets: wasm32-unknown-unknown
+
+      - name: Rust Cache
+        uses: Swatinem/rust-cache@v2
+        with:
+          key: typescript/differential
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: "20"
+          cache: "npm"
+          cache-dependency-path: mfkdf2-web/package-lock.json
+
+      - name: Install wasm-bindgen-cli
+        uses: taiki-e/install-action@v2
+        with:
+          tool: wasm-bindgen-cli
+
+      - name: Cache node_modules
+        id: cache-node-modules
+        uses: actions/cache@v4
+        with:
+          path: mfkdf2-web/node_modules
+          key: ${{ runner.os }}-node-modules-${{ hashFiles('mfkdf2-web/package-lock.json') }}
+          restore-keys: |
+            ${{ runner.os }}-node-modules-
+
+      - name: Install mfkdf2-web dependencies
+        if: steps.cache-node-modules.outputs.cache-hit != 'true'
+        working-directory: mfkdf2-web
+        run: npm ci
+
+      - name: Generate TypeScript bindings for differential tests
+        working-directory: mfkdf2-web
+        run: npm run ubrn:web:differential:release
+
+      - name: Copy index.web.ts implementation
+        run: cp mfkdf2-web/src/index.ts mfkdf2-web/src/index.web.ts
+
+      - name: Verify bindings were generated
+        run: |
+          if [ ! -d "mfkdf2-web/src/generated" ] || [ -z "$(ls -A mfkdf2-web/src/generated)" ]; then
+            echo "Error: mfkdf2-web/src/generated does not exist or is empty"
+            exit 1
+          fi
+          if [ ! -d "mfkdf2-web/rust_modules" ]; then
+            echo "Error: mfkdf2-web/rust_modules does not exist"
+            exit 1
+          fi
+          echo "✓ TypeScript bindings verified"
+
+      - name: Run differential tests
+        working-directory: mfkdf2-web
+        run: npm run test:differential
+
+

.github/workflows/rust.yaml

@@ -119,6 +119,9 @@ jobs:
 
       - name: Run tests (JUnit)
         run: cargo nextest run --release --profile ci
+
+      - name: Run doctests
+        run: cargo test --release --doc
 
       - name: Publish JUnit test report
         if: always()
@@ -184,7 +187,7 @@ jobs:
         uses: taiki-e/install-action@cargo-llvm-cov
 
       - name: Run cargo-llvm-cov
-        run: cargo llvm-cov --all-features --workspace --html --output-dir target/coverage
+        run: cargo llvm-cov --workspace --html --output-dir target/coverage
 
       - name: Upload coverage to Artifacts
         uses: actions/upload-artifact@v4

SECURITY.md

@@ -12,6 +12,7 @@ Please check here before filing new reports.
 | Affected Version(s) | Description                                                            | Status       | CVE / Advisory |
 | ------------------- | ---------------------------------------------------------------------- | ------------ | -------------- |
 | 0.0.1               | RSA Marvin Attack: potential key recovery through timing sidechannels. | 🔴 Unresolved | 2023-49092     |
+| 0.0.1               | Generic-Array: v0.14.9 is deprecated but used by aes-v0.8.4            | 🔴 Unresolved | -              |
 
 Legend:
 - 🟢 Fixed

docs/src/differential-tests.md

@@ -31,19 +31,26 @@ See: [multifactor/MFKDF#27](https://github.com/multifactor/MFKDF/pull/27)
 See: [multifactor/MFKDF2.rs#43](https://github.com/multifactor/MFKDF2.rs/pull/43)
 - Add a `differential-test` feature flag providing a global deterministic RNG equivalent to the reference.
 - Provide utility methods in the TypeScript bindings facade for nested parameter parsing and stringification (read/write inner params) to match reference structures.
+- Pin the versions used for differential testing to:
+  - `MFKDF2.rs` commit `7c33c7164d6e40a26c0899f19b8f9ad9b9f0c029`
+  - `MFKDF` commit `3d5bf73b4ce42b23da113b4be6d35e7d941fadf8`
 
 ## How to reproduce
 
-Run the differential tests using the bindings workflow. From the repository root:
+You can run the differential tests **locally** or via the **GitHub Actions workflow**.
+
+### Locally
+
+From the repository root:
 
 ```bash
 # Ensure the WASM target is present (one-time)
 rustup target add wasm32-unknown-unknown
 
-# Generate differential-release bindings (optimized)
+# Generate differential-release bindings (includes the `differential-test` feature)
 just gen-ts-bindings-differential
 
-# Run the TypeScript test suite (includes differential tests)
+# Run only the differential TypeScript test suite
 just test-bindings-differential
 ```
 

mfkdf2-web/package.json

@@ -61,6 +61,6 @@
     "tsconfig-paths": "^4.2.0",
     "tsx": "^4.19.0",
     "typescript": "^5.6.2",
-    "mfkdf": "github:multifactor/MFKDF#test/differential-testing"
+    "mfkdf": "github:multifactor/MFKDF#3d5bf7"
   }
 }

mfkdf2-web/test/factors/hmacsha1.test.ts

@@ -73,7 +73,7 @@ suite('factors/hmacsha1', () => {
     derive.key
       .toString('hex')
       .should.equal(
-        '2747ebf65219aee6630a758e40fd05ccbb39ab465745ea1c9a6c5adb6673d2d3'
+        'e1e67a0a2118867d8baf660d87500e650211855d2eff4c557ef2c8ae26ab5b6f'
       );
   });
 

mfkdf2/Cargo.toml

@@ -14,8 +14,11 @@ required-features = ["bindings"]
 [dependencies]
 # Cryptography
 aes = { version = "0.8", default-features = false }
+cbc = { version = "0.1.2", default-features = false }
 cipher = { version = "0.4", default-features = false, features = [
   "block-padding",
+  "rand_core",
+  "std",
 ] }
 ecb = { version = "0.1", default-features = false }
 hkdf = { version = "0.12", default-features = false }
@@ -84,6 +87,7 @@ data-encoding = { version = "2.9.0", default-features = false, features = [
 ] }
 regex = { version = "1.11.3", default-features = false }
 
+
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 console_log = { version = "1.0", default-features = false }
 getrandom   = { version = "0.2", default-features = false, features = ["js"] }

mfkdf2/README.md

@@ -276,6 +276,8 @@ a Shamir‑style secret sharing scheme, one share per factor. During derive, any
 that supplies at least `threshold` valid shares can reconstruct the same secret and therefore
 the same derived key.
 
+**Note**: MFKDF2 provides no mechanism to invalidate old policies. When threshold is increased via [reconstitution](`crate::definitions::mfkdf_derived_key::reconstitution`), old policies can still be used to derive keys.
+
 ## Setup: configuring a 2‑of‑3 recovery policy
 
 The snippet below constructs a 2‑of‑3 key from a password, an HOTP soft token, and a UUID
@@ -463,6 +465,13 @@ let derived = derive::key(
 The same outer key can also be derived with only `password3` by supplying a single password
 factor keyed by `"password3"` to [setup key](`crate::derive::key`).
 
+# Integrity Protetion
+
+
+MFKDF2 allows policy integrity to be enforced between each subsequent derives, and is enabled by default. An honest client will only accept a state if the key it derives from that state correctly validates the state’s integrity. Before deriving the final key, current policy's self-referential tag is checked. This is enabled using `verify` flag in [setup](`crate::setup::key`) and [derive](`crate::derive::key`). If any mismatch is detected, the [PolicyIntegrityCheckFailed](`crate::error::MFKDF2Error::PolicyIntegrityCheckFailed`) error is returned.
+
+When integrity is disabled, adversary can modify factor public state like threshold, factor parameters, encrypted shares. This may expose underlying keys and factor secrets, reducing the overall entropy of the key.
+
 # Feature Flags
 
 - `bindings`: Generate FFI bindings of the library to other languages.

mfkdf2/src/crypto.rs

@@ -1,12 +1,16 @@
 //! Cryptographic functions for the MFKDF2 library.
 use aes::Aes256;
-use cipher::{BlockDecryptMut, BlockEncryptMut, KeyInit, block_padding::NoPadding};
+use cipher::{
+  BlockDecryptMut, BlockEncryptMut, Iv, Key, KeyInit, KeyIvInit, block_padding::NoPadding,
+};
 use ecb::{Decryptor, Encryptor};
 use hkdf::Hkdf;
 use hmac::{Hmac, Mac};
 use sha1::Sha1;
 use sha2::Sha256;
 
+use crate::error::MFKDF2Error;
+
 /// Derives a 32-byte key using HKDF-SHA256 with the given salt and info.
 pub(crate) fn hkdf_sha256_with_info(input: &[u8], salt: &[u8], info: &[u8]) -> [u8; 32] {
   let hk = Hkdf::<Sha256>::new(Some(salt), input);
@@ -33,6 +37,30 @@ pub(crate) fn encrypt(data: &[u8], key: &[u8; 32]) -> Vec<u8> {
   buf
 }
 
+pub(crate) fn encrypt_cbc<C>(
+  data: &[u8],
+  key: &Key<cbc::Encryptor<C>>,
+  iv: &Iv<cbc::Encryptor<C>>,
+) -> Result<Vec<u8>, MFKDF2Error>
+where
+  C: cipher::BlockCipher + cipher::BlockEncrypt,
+  cbc::Encryptor<C>: KeyIvInit + cipher::IvSizeUser + BlockEncryptMut,
+{
+  let mut buf = {
+    let mut v = data.to_vec();
+    let rem = v.len() % 16;
+    if rem != 0 {
+      v.extend(vec![0u8; 16 - rem]);
+    }
+    v
+  };
+  let padded_len = buf.len();
+
+  let ct =
+    cbc::Encryptor::<C>::new(key, iv).encrypt_padded_mut::<NoPadding>(&mut buf, padded_len)?;
+  Ok(ct.to_vec())
+}
+
 /// Decrypts a buffer using AES256-ECB with the given 32-byte key.
 // TODO (@lonerapier): check every use of decrypt and unpad properly or use assert.
 pub(crate) fn decrypt(mut data: Vec<u8>, key: &[u8; 32]) -> Vec<u8> {
@@ -41,6 +69,20 @@ pub(crate) fn decrypt(mut data: Vec<u8>, key: &[u8; 32]) -> Vec<u8> {
   data
 }
 
+pub(crate) fn decrypt_cbc<C>(
+  data: &[u8],
+  key: &Key<cbc::Decryptor<C>>,
+  iv: &Iv<cbc::Decryptor<C>>,
+) -> Result<Vec<u8>, MFKDF2Error>
+where
+  C: cipher::BlockCipher + cipher::BlockDecrypt,
+  cbc::Decryptor<C>: KeyIvInit + cipher::IvSizeUser + BlockDecryptMut,
+{
+  let mut buf = data.to_vec();
+  let ct = cbc::Decryptor::<C>::new(key, iv).decrypt_padded_mut::<NoPadding>(&mut buf)?;
+  Ok(ct.to_vec())
+}
+
 /// Computes an HMAC-SHA1 over the given challenge using the provided secret.
 pub(crate) fn hmacsha1(secret: &[u8], challenge: &[u8]) -> [u8; 20] {
   let mut mac = <Hmac<Sha1> as Mac>::new_from_slice(secret).unwrap();

mfkdf2/src/definitions/factor.rs

@@ -35,7 +35,7 @@ pub(crate) trait FactorMetadata: Send + Sync + std::fmt::Debug {
 ///
 /// ```rust
 /// use mfkdf2::{
-///   definitions::{FactorMetadata, FactorType},
+///   definitions::FactorType,
 ///   derive::factors::password as derive_password,
 ///   setup::factors::password::{PasswordOptions, password},
 /// };
@@ -48,7 +48,6 @@ pub(crate) trait FactorMetadata: Send + Sync + std::fmt::Debug {
 ///   _ => panic!("Wrong factor type"),
 /// };
 /// assert_eq!(p.password, "password123");
-/// assert_eq!(p.bytes(), "password123".as_bytes());
 ///
 /// // derive a key using the password factor
 /// let derive = derive_password("password123")?;

mfkdf2/src/definitions/mfkdf_derived_key/crypto.rs

@@ -1,9 +1,15 @@
+use crate::error::MFKDF2Result;
+
 impl crate::definitions::MFKDF2DerivedKey {
   /// Returns an HKDF-SHA256 derived key for the given purpose and salt.
-  pub fn get_subkey(&self, purpose: Option<&str>, salt: Option<&[u8]>) -> [u8; 32] {
+  pub fn get_subkey(&self, purpose: Option<&str>, salt: Option<&[u8]>) -> MFKDF2Result<[u8; 32]> {
     let salt = salt.unwrap_or(&[]);
     let purpose = purpose.unwrap_or("");
-    crate::crypto::hkdf_sha256_with_info(&self.key, salt, purpose.as_bytes())
+
+    // derive internal key
+    let internal_key = self.derive_internal_key()?;
+    // derive subkey
+    Ok(crate::crypto::hkdf_sha256_with_info(&internal_key, salt, purpose.as_bytes()))
   }
 }
 
@@ -13,8 +19,8 @@ fn derived_key_get_subkey(
   derived_key: &crate::definitions::MFKDF2DerivedKey,
   purpose: Option<String>,
   salt: Option<Vec<u8>>,
-) -> Vec<u8> {
+) -> MFKDF2Result<Vec<u8>> {
   let purpose = purpose.as_deref();
   let salt = salt.as_deref();
-  derived_key.get_subkey(purpose, salt).to_vec()
+  Ok(derived_key.get_subkey(purpose, salt)?.to_vec())
 }