From e7e1aa362e9a2344e3fe757425853bc369ae9de2 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:22:04 -0400 Subject: [PATCH 01/10] ci: fix GitHub Actions warnings on PR runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the 6 annotations surfaced on PR #984 runs: * Node.js 20 deprecations — add `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true'` to every workflow so actions still pinned to Node 20 (setup-java, upload/download-artifact, setup-android, peter-evans/*, McCzarny/*, reviewdog/*) run on the runner's Node 24 instead. Silences the deprecation warning until each action ships a v5. * `actions/checkout@v4` → `@v5` and `actions/setup-node@v4` → `@v5` across all workflows (these have native Node 24 builds). * Cache save failures ("Unable to reserve cache with key … another job may be creating this cache") — replace `\${{ github.run_id }}` keys with `\${{ hashFiles(...) }}` keys for Gradle and node_modules caches, and a stable `avd-pixel7pro-34-x86_64-v1` key for the AVD cache. Re-runs and cross-job races no longer collide on the same key. * Obsolete AGP 8.7.3 — bump library `classpath` to 8.12.2 to match react-native-nitro-modules. AGP 9.x requires Gradle 9 + JDK 21, which RN 0.81 + the example app's Gradle 8.14.3 + JDK 17 toolchain cannot support yet, so disable the `AndroidGradlePluginVersion` and `GradleDependency` lint checks on the library to stop the yutailang0119/action-android-lint annotation from re-firing on the newer-but-still-not-9 baseline. Bumping `actions/setup-node` `node-version` from 20 to 24 in the Android e2e workflow keeps it consistent with the runner default. --- .github/workflows/deploy-docs.yml | 7 ++++- .github/workflows/e2e-android-test.yml | 26 +++++++++---------- .github/workflows/e2e-ios-test.yml | 8 +++++- .github/workflows/release.yml | 9 +++++-- .github/workflows/update-lockfiles.yml | 7 ++++- .github/workflows/validate-cpp.yml | 8 +++++- .github/workflows/validate-js.yml | 10 +++++-- .../android/build.gradle | 6 ++++- 8 files changed, 59 insertions(+), 22 deletions(-) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index b7b0231bb..1bccef9ef 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -17,12 +17,17 @@ concurrency: group: 'pages' cancel-in-progress: true +env: + # Opt actions still on Node 20 into the runner's Node 24 instead. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: build: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup Bun uses: ./.github/actions/setup-bun diff --git a/.github/workflows/e2e-android-test.yml b/.github/workflows/e2e-android-test.yml index b3ea5345c..8f0ed7489 100644 --- a/.github/workflows/e2e-android-test.yml +++ b/.github/workflows/e2e-android-test.yml @@ -25,6 +25,10 @@ on: env: EMULATOR_API_LEVEL: 34 + # Opt actions still on Node 20 (setup-java, upload/download-artifact, setup-android, etc.) + # into the runner's Node 24 instead. Silences deprecation warnings until each action ships a v5. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' jobs: # ============================================================================ @@ -51,7 +55,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v5 with: - node-version: '20' + node-version: '24' - name: Install Bun uses: ./.github/actions/setup-bun @@ -87,7 +91,7 @@ jobs: packages/react-native-quick-crypto/android/build node_modules/.bun/react-native-nitro-modules*/node_modules/react-native-nitro-modules/android/.cxx node_modules/.bun/react-native-nitro-modules*/node_modules/react-native-nitro-modules/android/build - key: ${{ runner.os }}-gradle-${{ github.run_id }} + key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/**/*.gradle*', 'example/android/gradle.properties', 'example/android/gradle/wrapper/gradle-wrapper.properties', 'packages/react-native-quick-crypto/android/build.gradle', 'packages/react-native-quick-crypto/android/gradle.properties', 'bun.lock') }} restore-keys: | ${{ runner.os }}-gradle- @@ -141,7 +145,7 @@ jobs: packages/react-native-quick-crypto/android/build node_modules/.bun/react-native-nitro-modules*/node_modules/react-native-nitro-modules/android/.cxx node_modules/.bun/react-native-nitro-modules*/node_modules/react-native-nitro-modules/android/build - key: ${{ runner.os }}-gradle-${{ github.run_id }} + key: ${{ runner.os }}-gradle-${{ hashFiles('example/android/**/*.gradle*', 'example/android/gradle.properties', 'example/android/gradle/wrapper/gradle-wrapper.properties', 'packages/react-native-quick-crypto/android/build.gradle', 'packages/react-native-quick-crypto/android/gradle.properties', 'bun.lock') }} # ============================================================================ # AVD Job - Create and cache emulator snapshot (runs in parallel with build) @@ -167,9 +171,7 @@ jobs: path: | ~/.android/avd/* ~/.android/adb* - key: avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}-${{ github.run_id }} - restore-keys: | - avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}- + key: avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}-x86_64-v1 - name: Create AVD and Generate Snapshot for Caching if: steps.avd-cache.outputs.cache-hit != 'true' @@ -190,7 +192,7 @@ jobs: path: | ~/.android/avd/* ~/.android/adb* - key: avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}-${{ github.run_id }} + key: avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}-x86_64-v1 # ============================================================================ # Test Job - Run E2E tests (needs both build and AVD) @@ -209,7 +211,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v5 with: - node-version: '20' + node-version: '24' - name: Install Bun uses: ./.github/actions/setup-bun @@ -226,7 +228,7 @@ jobs: uses: actions/cache/restore@v5 with: path: node_modules - key: ${{ runner.os }}-node-modules-${{ github.run_id }} + key: ${{ runner.os }}-node-modules-${{ hashFiles('bun.lock') }} restore-keys: | ${{ runner.os }}-node-modules- @@ -237,7 +239,7 @@ jobs: uses: actions/cache/save@v5 with: path: node_modules - key: ${{ runner.os }}-node-modules-${{ github.run_id }} + key: ${{ runner.os }}-node-modules-${{ hashFiles('bun.lock') }} - name: Download APK uses: actions/download-artifact@v4 @@ -263,9 +265,7 @@ jobs: path: | ~/.android/avd/* ~/.android/adb* - key: avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}-${{ github.run_id }} - restore-keys: | - avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}- + key: avd-pixel7pro-${{ env.EMULATOR_API_LEVEL }}-x86_64-v1 - name: Run E2E Tests uses: reactivecircus/android-emulator-runner@v2 diff --git a/.github/workflows/e2e-ios-test.yml b/.github/workflows/e2e-ios-test.yml index 631f3aa9d..f017863f4 100644 --- a/.github/workflows/e2e-ios-test.yml +++ b/.github/workflows/e2e-ios-test.yml @@ -23,6 +23,12 @@ on: - 'packages/react-native-quick-crypto/src/**' - 'packages/react-native-quick-crypto/ios/**' +env: + # Opt actions still on Node 20 (upload-artifact, McCzarny/upload-image, peter-evans/*, etc.) + # into the runner's Node 24 instead. Silences deprecation warnings until each action ships a v5. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: e2e-tests-ios: runs-on: macOS-26 @@ -34,7 +40,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Select Xcode 26.2 run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f116807fe..a9aed60f7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,6 +12,11 @@ on: type: boolean default: false +env: + # Opt actions still on Node 20 into the runner's Node 24 instead. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: release: runs-on: macos-latest @@ -20,7 +25,7 @@ jobs: id-token: write steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: fetch-depth: 0 token: ${{ secrets.GITHUB_TOKEN }} @@ -29,7 +34,7 @@ jobs: uses: ./.github/actions/setup-bun - name: Setup Node.js (for npm publish with OIDC) - uses: actions/setup-node@v4 + uses: actions/setup-node@v5 with: node-version: '24' registry-url: 'https://registry.npmjs.org' diff --git a/.github/workflows/update-lockfiles.yml b/.github/workflows/update-lockfiles.yml index 961f593f6..3a07b70b8 100644 --- a/.github/workflows/update-lockfiles.yml +++ b/.github/workflows/update-lockfiles.yml @@ -15,13 +15,18 @@ on: permissions: contents: write +env: + # Opt actions still on Node 20 into the runner's Node 24 instead. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: update-lockfiles: name: 'Update lockfiles (Podfile.lock)' if: github.actor == 'dependabot[bot]' runs-on: macOS-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 0 ref: ${{ github.event.pull_request.head.ref }} diff --git a/.github/workflows/validate-cpp.yml b/.github/workflows/validate-cpp.yml index db55f1d78..718b78724 100644 --- a/.github/workflows/validate-cpp.yml +++ b/.github/workflows/validate-cpp.yml @@ -16,12 +16,18 @@ on: - 'packages/react-native-quick-crypto/cpp/**' - 'packages/react-native-quick-crypto/nitrogen/generated/shared/**' +env: + # Opt actions still on Node 20 (e.g. reviewdog/action-cpplint) into the runner's Node 24. + # Silences deprecation warnings until each action ships a v5. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: validate_cpp: name: C++ Lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up clang-format run: sudo apt-get install -y clang-format - name: Run clang-format check diff --git a/.github/workflows/validate-js.yml b/.github/workflows/validate-js.yml index 3b7ed61d0..e0b2b26b2 100644 --- a/.github/workflows/validate-js.yml +++ b/.github/workflows/validate-js.yml @@ -30,12 +30,18 @@ on: - 'example/*.*sx' - 'example/bun.lock' +env: + # Opt actions still on Node 20 (e.g. reviewdog/action-setup) into the runner's Node 24. + # Silences deprecation warnings until each action ships a v5. + # https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: 'true' + jobs: compile_js: name: Compile JS (tsc) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: ./.github/actions/setup-bun @@ -63,7 +69,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup Bun uses: ./.github/actions/setup-bun diff --git a/packages/react-native-quick-crypto/android/build.gradle b/packages/react-native-quick-crypto/android/build.gradle index c95ba7cdd..4c98f4c87 100644 --- a/packages/react-native-quick-crypto/android/build.gradle +++ b/packages/react-native-quick-crypto/android/build.gradle @@ -7,7 +7,7 @@ buildscript { } dependencies { - classpath "com.android.tools.build:gradle:8.7.3" + classpath "com.android.tools.build:gradle:8.12.2" classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:${kotlinVersion}" } } @@ -113,6 +113,10 @@ packagingOptions { lintOptions { disable "GradleCompatible" + // AGP version is constrained by RN 0.81 + Gradle 8.14.3 + JDK 17. + // AGP 9.x requires Gradle 9 and JDK 21 — not viable until RN bumps its toolchain. + disable "AndroidGradlePluginVersion" + disable "GradleDependency" } compileOptions { From 86b09f8e3c63b3c62047954388e6e4b6d250fd1f Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:29:05 -0400 Subject: [PATCH 02/10] =?UTF-8?q?feat(security):=20Phase=203.1=20audit=20?= =?UTF-8?q?=E2=80=94=20TS-layer=20cipher=20param=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-validate cipher algorithm, key, and IV byte-lengths at the JS↔C++ boundary before reaching the native factory. Previously, wrong sizes fell through to OpenSSL which produced opaque error strings and (for algorithm typos) only failed inside the C++ factory. `validateCipherParams()` in `cipher.ts`: * Rejects empty / non-string `cipherType` with `TypeError`. * Rejects empty key (`byteLength === 0`) with `RangeError` upfront. * For OpenSSL ciphers, splits the existing `getCipherInfo(name, k, iv)` probe into three calls — name-only, name+keyLen, name+ivLen — so the thrown error names exactly which parameter is wrong (key vs iv) and reports the offending byte-length. * Rejects empty IV when the cipher requires one, and rejects non-empty IV for ciphers that take none (covers WRAP-style ciphers if a caller ever invokes one directly even though `getCiphers()` filters them). * Hard-codes (key=32, iv=24) for the libsodium ciphers `xsalsa20`, `xsalsa20-poly1305`, and `xchacha20-poly1305`, which OpenSSL's `EVP_CIPHER_fetch` does not see. Wired into both `Cipheriv` / `Decipheriv` constructors and the `xsalsa20()` `@noble/ciphers` shim, all of which now compute the ArrayBuffer once, validate, and reuse the exact buffer for the native call. Tests: 11 new regressions in `example/src/tests/cipher/cipher_tests.ts`: empty-name, unknown-name, too-short / too-long / empty key for AES-CBC, wrong IV for CBC and CCM, accepted variable IV for GCM, createDecipheriv mirror, and wrong-key + wrong-nonce for xsalsa20. --- example/src/tests/cipher/cipher_tests.ts | 85 +++++++++++++++++ .../react-native-quick-crypto/src/cipher.ts | 95 ++++++++++++++++++- 2 files changed, 176 insertions(+), 4 deletions(-) diff --git a/example/src/tests/cipher/cipher_tests.ts b/example/src/tests/cipher/cipher_tests.ts index 44eb6b7e1..4bd902bf4 100644 --- a/example/src/tests/cipher/cipher_tests.ts +++ b/example/src/tests/cipher/cipher_tests.ts @@ -537,3 +537,88 @@ test( }).to.not.throw(); }, ); + +// --- TS-layer cipher param validation regression (Phase 3.1) --- +// +// Pre-fix, wrong key / iv lengths reached C++ before being rejected, producing +// confusing OpenSSL error strings. The TS layer now pre-validates against +// getCipherInfo() (or a small libsodium table) and throws a clear +// `RangeError: Invalid {key,iv} length …` before the native call. + +test(SUITE, 'createCipheriv: rejects empty algorithm', () => { + expect(() => { + createCipheriv('', key32, iv16); + }).to.throw(TypeError, /non-empty string/); +}); + +test(SUITE, 'createCipheriv: rejects unknown algorithm', () => { + expect(() => { + createCipheriv('aes-128-boorad', key16, iv16); + }).to.throw(TypeError, /Unsupported or unknown cipher/); +}); + +test(SUITE, 'createCipheriv: rejects too-short key for aes-256-cbc', () => { + // Pass a 128-bit key to a 256-bit cipher. + expect(() => { + createCipheriv('aes-256-cbc', key16, iv16); + }).to.throw(RangeError, /Invalid key length 16/); +}); + +test(SUITE, 'createCipheriv: rejects too-long key for aes-128-cbc', () => { + // Pass a 256-bit key to a 128-bit cipher. + expect(() => { + createCipheriv('aes-128-cbc', key32, iv16); + }).to.throw(RangeError, /Invalid key length 32/); +}); + +test(SUITE, 'createCipheriv: rejects empty key', () => { + expect(() => { + createCipheriv('aes-128-cbc', Buffer.alloc(0), iv16); + }).to.throw(RangeError, /key length 0/); +}); + +test(SUITE, 'createCipheriv: rejects wrong iv length for aes-128-cbc', () => { + // CBC requires a 16-byte IV. 12 bytes (a GCM-style IV) must be rejected. + expect(() => { + createCipheriv('aes-128-cbc', key16, iv12); + }).to.throw(RangeError, /Invalid iv length 12/); +}); + +test(SUITE, 'createCipheriv: rejects wrong iv length for aes-128-ccm', () => { + // CCM accepts 7..13 byte IVs. 16 bytes must be rejected. + expect(() => { + createCipheriv('aes-128-ccm', key16, iv16, { authTagLength: 16 }); + }).to.throw(RangeError, /Invalid iv length 16/); +}); + +test( + SUITE, + 'createCipheriv: accepts variable iv length for aes-256-gcm', + () => { + // GCM accepts a wide range of IV lengths. + expect(() => { + createCipheriv('aes-256-gcm', key32, iv16); + }).to.not.throw(); + expect(() => { + createCipheriv('aes-256-gcm', key32, iv12); + }).to.not.throw(); + }, +); + +test(SUITE, 'createDecipheriv: rejects too-long key for aes-128-cbc', () => { + expect(() => { + createDecipheriv('aes-128-cbc', key32, iv16); + }).to.throw(RangeError, /Invalid key length 32/); +}); + +test(SUITE, 'createCipheriv: rejects wrong xsalsa20 key length', () => { + expect(() => { + createCipheriv('xsalsa20', key16, randomFillSync(new Uint8Array(24))); + }).to.throw(RangeError, /Invalid key length 16 .* xsalsa20/); +}); + +test(SUITE, 'createCipheriv: rejects wrong xsalsa20 nonce length', () => { + expect(() => { + createCipheriv('xsalsa20', key32, iv16); + }).to.throw(RangeError, /Invalid iv length 16 .* xsalsa20/); +}); diff --git a/packages/react-native-quick-crypto/src/cipher.ts b/packages/react-native-quick-crypto/src/cipher.ts index d8917dc58..481a451b5 100644 --- a/packages/react-native-quick-crypto/src/cipher.ts +++ b/packages/react-native-quick-crypto/src/cipher.ts @@ -65,6 +65,85 @@ export function getCipherInfo( return CipherUtils.getCipherInfo(name, options?.keyLength, options?.ivLength); } +// libsodium ciphers aren't visible to OpenSSL's EVP_CIPHER_fetch, so +// getCipherInfo() returns undefined for them. Hard-code the (key, iv) +// byte-lengths the C++ factory will accept. +const LIBSODIUM_CIPHER_PARAMS: Readonly< + Record +> = { + xsalsa20: { keyLength: 32, ivLength: 24 }, + 'xsalsa20-poly1305': { keyLength: 32, ivLength: 24 }, + 'xchacha20-poly1305': { keyLength: 32, ivLength: 24 }, +}; + +function validateCipherParams( + cipherType: string, + keyByteLength: number, + ivByteLength: number, +): void { + if (typeof cipherType !== 'string' || cipherType.length === 0) { + throw new TypeError('cipher algorithm must be a non-empty string'); + } + if (!Number.isFinite(keyByteLength) || keyByteLength === 0) { + throw new RangeError(`Invalid key length 0 for cipher ${cipherType}`); + } + + const lower = cipherType.toLowerCase(); + const sodium = LIBSODIUM_CIPHER_PARAMS[lower]; + if (sodium) { + if (keyByteLength !== sodium.keyLength) { + throw new RangeError( + `Invalid key length ${keyByteLength} for cipher ${cipherType} ` + + `(expected ${sodium.keyLength})`, + ); + } + if (ivByteLength !== sodium.ivLength) { + throw new RangeError( + `Invalid iv length ${ivByteLength} for cipher ${cipherType} ` + + `(expected ${sodium.ivLength})`, + ); + } + return; + } + + // OpenSSL path: getCipherInfo(name, keyLen, ivLen) returns undefined when + // the requested lengths are not accepted by the cipher. We split the call + // into three checks so the thrown error can name which parameter is wrong. + const info = CipherUtils.getCipherInfo(cipherType); + if (info === undefined) { + throw new TypeError(`Unsupported or unknown cipher type: ${cipherType}`); + } + if ( + CipherUtils.getCipherInfo(cipherType, keyByteLength, undefined) === + undefined + ) { + throw new RangeError( + `Invalid key length ${keyByteLength} for cipher ${cipherType}`, + ); + } + + const expectedIv = info.ivLength ?? 0; + if (expectedIv > 0) { + if (ivByteLength === 0) { + throw new RangeError( + `Cipher ${cipherType} requires an iv but none was provided`, + ); + } + if ( + CipherUtils.getCipherInfo(cipherType, undefined, ivByteLength) === + undefined + ) { + throw new RangeError( + `Invalid iv length ${ivByteLength} for cipher ${cipherType}`, + ); + } + } else if (ivByteLength > 0) { + throw new RangeError( + `Cipher ${cipherType} does not use an iv (got ${ivByteLength} bytes)`, + ); + } +} + interface CipherArgs { isCipher: boolean; cipherType: string; @@ -114,13 +193,17 @@ class CipherCommon extends Stream.Transform { 'authTagLength', ) ?? 16; + const cipherKeyAB = binaryLikeToArrayBuffer(cipherKey); + const ivAB = binaryLikeToArrayBuffer(iv); + validateCipherParams(cipherType, cipherKeyAB.byteLength, ivAB.byteLength); + const factory = NitroModules.createHybridObject('CipherFactory'); this.native = factory.createCipher({ isCipher, cipherType, - cipherKey: binaryLikeToArrayBuffer(cipherKey), - iv: binaryLikeToArrayBuffer(iv), + cipherKey: cipherKeyAB, + iv: ivAB, authTagLen, }); } @@ -369,13 +452,17 @@ export function xsalsa20( // eslint-disable-next-line @typescript-eslint/no-unused-vars counter?: number, ): Uint8Array { + const cipherKeyAB = binaryLikeToArrayBuffer(key); + const ivAB = binaryLikeToArrayBuffer(nonce); + validateCipherParams('xsalsa20', cipherKeyAB.byteLength, ivAB.byteLength); + const factory = NitroModules.createHybridObject('CipherFactory'); const native = factory.createCipher({ isCipher: true, cipherType: 'xsalsa20', - cipherKey: binaryLikeToArrayBuffer(key), - iv: binaryLikeToArrayBuffer(nonce), + cipherKey: cipherKeyAB, + iv: ivAB, }); const result = native.update(binaryLikeToArrayBuffer(data)); return new Uint8Array(result); From 146d4e8f2b55ea289a2d0807a7429a65fbfa7cae Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:34:30 -0400 Subject: [PATCH 03/10] =?UTF-8?q?feat(security):=20Phase=203.2=20audit=20?= =?UTF-8?q?=E2=80=94=20KDF=20parameter=20validation=20at=20TS=20layer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-validate scrypt, HKDF, and Argon2 parameters at the JS↔C++ boundary so invalid inputs surface as clear RangeErrors before the native call, not as opaque OpenSSL/libsodium errors after. * **Scrypt** (`scrypt.ts`) — RFC 7914 §6: N must be a power of 2 > 1, r and p must be positive integers, r * p < 2^30, and the working set 128 * r * N must fit in maxmem. Both async (`scrypt`) and sync (`scryptSync`) paths share `validateScryptParams()`. Async errors flow through the callback as RangeError. * **HKDF** (`hkdf.ts`) — RFC 5869 §2.3: L (output keylen in bytes) must be ≤ 255 * HashLen. The new `validateHkdfKeylen()` looks the digest up in a static `HKDF_HASH_BYTES` table covering sha1/224/256/384/512, sha3-256/384/512, ripemd160. Wired into `hkdf`, `hkdfSync`, and the WebCrypto `hkdfDeriveBits` path. * **Argon2** (`argon2.ts`) — RFC 9106 §3.1 minimums: 1 ≤ p ≤ 2^24-1, T ≥ 4, m ≥ 8 * p (in KiB), t ≥ 1, salt 8..2^32-1 bytes, version ∈ {0x10, 0x13}. Async surfaces param errors via callback. Existing argon2 tests that asserted the C++ `validateUInt`-style messages ("non-negative", "out of range", "integer") are updated to match the new RFC 9106 wording, since the JS-side check now fires first. Adds 7 new RFC 9106 minimum-bound tests (parallelism = 0, tagLength < 4, passes = 0, memory < 8*p, nonce < 8 bytes, unsupported version, plus an existing-message refresh). Tests: 8 new scrypt regressions (N=1, N not power of 2, fractional N, negative r, p=0, working-set > maxmem, negative keylen, async callback path), 5 new HKDF regressions (negative keylen, two ceiling-violation cases for sha256 and sha1, ceiling-equal accepted, async callback path), and 7 new Argon2 RFC 9106 minimum-bound checks on top of the message refresh. --- example/src/tests/argon2/argon2_tests.ts | 60 ++++++++++++-- example/src/tests/hkdf/hkdf_tests.ts | 53 +++++++++++++ example/src/tests/scrypt/scrypt_tests.ts | 69 ++++++++++++++++ .../react-native-quick-crypto/src/argon2.ts | 71 +++++++++++++++++ .../react-native-quick-crypto/src/hkdf.ts | 43 ++++++++-- .../react-native-quick-crypto/src/scrypt.ts | 79 +++++++++++++++++-- 6 files changed, 357 insertions(+), 18 deletions(-) diff --git a/example/src/tests/argon2/argon2_tests.ts b/example/src/tests/argon2/argon2_tests.ts index cece1b981..43462acf1 100644 --- a/example/src/tests/argon2/argon2_tests.ts +++ b/example/src/tests/argon2/argon2_tests.ts @@ -137,11 +137,13 @@ test(SUITE, 'argon2Sync: deterministic with same inputs', () => { ); }); -// --- Numeric parameter validation (Phase 1.1: validateUInt) --- +// --- Numeric parameter validation (Phase 1.1: validateUInt + Phase 3.2 RFC 9106) --- // // `static_cast(NaN | +/-Infinity | -1)` is undefined behavior in -// C++. The C++ layer used to do these casts naked; the validateUInt helper -// now rejects them with a descriptive error before the cast. +// C++. The C++ layer's validateUInt helper used to be the first line of +// defense; Phase 3.2 added a TS-side RFC 9106 §3.1 check that fires +// earlier and produces a clearer message. The regex below matches the +// new RFC 9106 wording. const baseParams = { message: Buffer.from('password'), @@ -173,20 +175,20 @@ test(SUITE, 'argon2Sync: rejects -Infinity passes', () => { test(SUITE, 'argon2Sync: rejects negative tagLength', () => { assert.throws(() => { argon2Sync('argon2id', { ...baseParams, tagLength: -1 }); - }, /tagLength.*non-negative/i); + }, /Invalid Argon2 tagLength: -1/); }); test(SUITE, 'argon2Sync: rejects fractional passes', () => { assert.throws(() => { argon2Sync('argon2id', { ...baseParams, passes: 3.5 }); - }, /passes.*integer/i); + }, /Invalid Argon2 passes: 3\.5/); }); test(SUITE, 'argon2Sync: rejects out-of-range memory', () => { // memory is uint32_t — anything beyond UINT32_MAX must be rejected. assert.throws(() => { argon2Sync('argon2id', { ...baseParams, memory: 2 ** 32 }); - }, /memory.*out of range/i); + }, /Invalid Argon2 memory: 4294967296/); }); test(SUITE, 'argon2: async path also rejects NaN parallelism', () => { @@ -202,3 +204,49 @@ test(SUITE, 'argon2: async path also rejects NaN parallelism', () => { }); }); }); + +// --- RFC 9106 §3.1 minimum-bound validation (Phase 3.2) --- + +test(SUITE, 'argon2Sync: rejects parallelism = 0 (RFC 9106 mins)', () => { + assert.throws(() => { + argon2Sync('argon2id', { ...baseParams, parallelism: 0 }); + }, /parallelism: 0/); +}); + +test(SUITE, 'argon2Sync: rejects tagLength < 4 (RFC 9106 mins)', () => { + assert.throws(() => { + argon2Sync('argon2id', { ...baseParams, tagLength: 3 }); + }, /tagLength: 3/); +}); + +test(SUITE, 'argon2Sync: rejects passes = 0 (RFC 9106 mins)', () => { + assert.throws(() => { + argon2Sync('argon2id', { ...baseParams, passes: 0 }); + }, /passes: 0/); +}); + +test(SUITE, 'argon2Sync: rejects memory < 8 * parallelism (RFC 9106)', () => { + // p=4 ⇒ memory must be ≥ 32 KiB; 16 KiB must be rejected. + assert.throws(() => { + argon2Sync('argon2id', { + ...baseParams, + parallelism: 4, + memory: 16, + }); + }, /memory: 16/); +}); + +test(SUITE, 'argon2Sync: rejects nonce shorter than 8 bytes (RFC 9106)', () => { + assert.throws(() => { + argon2Sync('argon2id', { + ...baseParams, + nonce: Buffer.from('1234567'), // 7 bytes + }); + }, /nonce length: 7/); +}); + +test(SUITE, 'argon2Sync: rejects unsupported version', () => { + assert.throws(() => { + argon2Sync('argon2id', { ...baseParams, version: 0x42 }); + }, /Invalid Argon2 version/); +}); diff --git a/example/src/tests/hkdf/hkdf_tests.ts b/example/src/tests/hkdf/hkdf_tests.ts index 85763ab62..9dfac0300 100644 --- a/example/src/tests/hkdf/hkdf_tests.ts +++ b/example/src/tests/hkdf/hkdf_tests.ts @@ -77,6 +77,59 @@ test(SUITE, 'WebCrypto HKDF importKey and deriveBits', async () => { expect(Buffer.from(bits).toString('hex')).to.equal(vec.okm); }); +// --- TS-layer HKDF parameter validation regression (Phase 3.2) --- +// +// RFC 5869 §2.3 caps L (output keylen in bytes) at 255 * HashLen. Pre-fix, +// callers could request any keylen; the native side either silently +// truncated or — in the worst case — produced an error string only after +// the round-trip. We now reject too-large requests at the JS boundary. + +test(SUITE, 'hkdfSync: rejects negative keylen', () => { + const ikm = Buffer.from('00', 'hex'); + expect(() => { + hkdfSync('sha256', ikm, Buffer.alloc(0), Buffer.alloc(0), -1); + }).to.throw(TypeError, /Bad key length/); +}); + +test(SUITE, 'hkdfSync: rejects keylen > 255 * HashLen for sha256', () => { + const ikm = Buffer.from('00', 'hex'); + expect(() => { + // 255 * 32 = 8160 bytes, so 8161 must be rejected. + hkdfSync('sha256', ikm, Buffer.alloc(0), Buffer.alloc(0), 8161); + }).to.throw(RangeError, /exceeds RFC 5869 ceiling/); +}); + +test(SUITE, 'hkdfSync: rejects keylen > 255 * HashLen for sha1', () => { + const ikm = Buffer.from('00', 'hex'); + expect(() => { + // 255 * 20 = 5100 bytes for sha1. + hkdfSync('sha1', ikm, Buffer.alloc(0), Buffer.alloc(0), 5101); + }).to.throw(RangeError, /exceeds RFC 5869 ceiling/); +}); + +test(SUITE, 'hkdfSync: accepts keylen at the RFC 5869 ceiling', () => { + const ikm = Buffer.from('00', 'hex'); + // Exactly 255 * 32 = 8160 must succeed. + expect(() => { + hkdfSync('sha256', ikm, Buffer.alloc(0), Buffer.alloc(0), 8160); + }).to.not.throw(); +}); + +test(SUITE, 'hkdf: surfaces ceiling errors via callback', async () => { + const ikm = Buffer.from('00', 'hex'); + await new Promise((resolve, reject) => { + hkdf('sha256', ikm, Buffer.alloc(0), Buffer.alloc(0), 9000, err => { + try { + expect(err).to.be.instanceOf(RangeError); + expect(err!.message).to.match(/exceeds RFC 5869 ceiling/); + resolve(); + } catch (e) { + reject(e); + } + }); + }); +}); + test(SUITE, 'WebCrypto HKDF deriveKey (AES-GCM)', async () => { const vec = testVectors[0]!; const ikm = Buffer.from(vec.ikm, 'hex'); diff --git a/example/src/tests/scrypt/scrypt_tests.ts b/example/src/tests/scrypt/scrypt_tests.ts index ad3fff80e..0f296adf9 100644 --- a/example/src/tests/scrypt/scrypt_tests.ts +++ b/example/src/tests/scrypt/scrypt_tests.ts @@ -96,3 +96,72 @@ test(SUITE, 'should handle aliases cost/blockSize/parallelization', () => { }); expect(derivedKey.toString('hex')).to.equal(t.expected); }); + +// --- TS-layer scrypt parameter validation regression (Phase 3.2) --- +// +// Pre-fix, invalid (N, r, p, maxmem) reached native and produced opaque +// OpenSSL errors or — worse — an OOM. These tests pin the RFC 7914 +// constraints (N power-of-2 > 1, r/p positive ints, r*p < 2^30, +// 128*r*N ≤ maxmem). + +test(SUITE, 'scryptSync: rejects N=1 (must be > 1)', () => { + expect(() => { + crypto.scryptSync('pw', 'salt', 32, { N: 1, r: 8, p: 1 }); + }).to.throw(RangeError, /power of 2 greater than 1/); +}); + +test(SUITE, 'scryptSync: rejects N=15 (not a power of 2)', () => { + expect(() => { + crypto.scryptSync('pw', 'salt', 32, { N: 15, r: 8, p: 1 }); + }).to.throw(RangeError, /power of 2 greater than 1/); +}); + +test(SUITE, 'scryptSync: rejects fractional N', () => { + expect(() => { + crypto.scryptSync('pw', 'salt', 32, { N: 16.5, r: 8, p: 1 }); + }).to.throw(RangeError, /Invalid scrypt cost/); +}); + +test(SUITE, 'scryptSync: rejects negative r', () => { + expect(() => { + crypto.scryptSync('pw', 'salt', 32, { N: 16, r: -1, p: 1 }); + }).to.throw(RangeError, /blockSize/); +}); + +test(SUITE, 'scryptSync: rejects p = 0', () => { + expect(() => { + crypto.scryptSync('pw', 'salt', 32, { N: 16, r: 8, p: 0 }); + }).to.throw(RangeError, /parallelization/); +}); + +test(SUITE, 'scryptSync: rejects working set larger than maxmem', () => { + // 128 * 8 * 16384 = 16 MiB; maxmem of 1 MiB is too small. + expect(() => { + crypto.scryptSync('pw', 'salt', 32, { + N: 16384, + r: 8, + p: 1, + maxmem: 1024 * 1024, + }); + }).to.throw(RangeError, /exceeds maxmem/); +}); + +test(SUITE, 'scryptSync: rejects negative keylen', () => { + expect(() => { + crypto.scryptSync('pw', 'salt', -1); + }).to.throw(TypeError, /Bad key length/); +}); + +test(SUITE, 'scrypt: surfaces param errors via callback', async () => { + await new Promise((resolve, reject) => { + crypto.scrypt('pw', 'salt', 32, { N: 15, r: 8, p: 1 }, err => { + try { + expect(err).to.be.instanceOf(RangeError); + expect(err!.message).to.match(/power of 2 greater than 1/); + resolve(); + } catch (e) { + reject(e); + } + }); + }); +}); diff --git a/packages/react-native-quick-crypto/src/argon2.ts b/packages/react-native-quick-crypto/src/argon2.ts index 12fe095bc..deb7d6d45 100644 --- a/packages/react-native-quick-crypto/src/argon2.ts +++ b/packages/react-native-quick-crypto/src/argon2.ts @@ -26,6 +26,26 @@ export interface Argon2Params { const ARGON2_VERSION = 0x13; // v1.3 +// RFC 9106 § 3.1: Argon2 input/parameter constraints. +// p (parallelism) 1 ≤ p ≤ 2^24 - 1 +// T (tag length) 4 ≤ T ≤ 2^32 - 1 +// m (memory in KiB) 8*p ≤ m ≤ 2^32 - 1 +// t (passes) 1 ≤ t ≤ 2^32 - 1 +// |salt| (nonce) 8 ≤ |s| ≤ 2^32 - 1 +// v (version) 0x10 (v1.0) or 0x13 (v1.3) +const ARGON2_MAX_U24 = 0xff_ffff; +const ARGON2_MAX_U32 = 0xffff_ffff; + +function isUInt(value: unknown, max: number): value is number { + return ( + typeof value === 'number' && + Number.isFinite(value) && + Number.isInteger(value) && + value >= 0 && + value <= max + ); +} + function validateAlgorithm(algorithm: string): void { if ( algorithm !== 'argon2d' && @@ -36,6 +56,50 @@ function validateAlgorithm(algorithm: string): void { } } +function validateArgon2Params(params: Argon2Params, version: number): void { + if (!isUInt(params.parallelism, ARGON2_MAX_U24) || params.parallelism < 1) { + throw new RangeError( + `Invalid Argon2 parallelism: ${params.parallelism} ` + + `(RFC 9106: 1 ≤ p ≤ 2^24 - 1)`, + ); + } + if (!isUInt(params.tagLength, ARGON2_MAX_U32) || params.tagLength < 4) { + throw new RangeError( + `Invalid Argon2 tagLength: ${params.tagLength} ` + + `(RFC 9106: 4 ≤ T ≤ 2^32 - 1)`, + ); + } + const minMemory = 8 * params.parallelism; + if (!isUInt(params.memory, ARGON2_MAX_U32) || params.memory < minMemory) { + throw new RangeError( + `Invalid Argon2 memory: ${params.memory} KiB ` + + `(RFC 9106: 8 * p (= ${minMemory}) ≤ m ≤ 2^32 - 1)`, + ); + } + if (!isUInt(params.passes, ARGON2_MAX_U32) || params.passes < 1) { + throw new RangeError( + `Invalid Argon2 passes: ${params.passes} ` + + `(RFC 9106: 1 ≤ t ≤ 2^32 - 1)`, + ); + } + if (version !== 0x10 && version !== 0x13) { + throw new RangeError( + `Invalid Argon2 version: 0x${version.toString(16)} ` + + `(RFC 9106: 0x10 or 0x13)`, + ); + } + // Salt (nonce) must be 8..2^32 - 1 bytes — measured against the resolved + // ArrayBuffer because BinaryLike accepts strings whose UTF-8 length is + // what actually reaches OpenSSL. + const nonceLen = binaryLikeToArrayBuffer(params.nonce).byteLength; + if (nonceLen < 8 || nonceLen > ARGON2_MAX_U32) { + throw new RangeError( + `Invalid Argon2 nonce length: ${nonceLen} bytes ` + + `(RFC 9106: 8 ≤ |s| ≤ 2^32 - 1)`, + ); + } +} + function toAB(value: BinaryLike): ArrayBuffer { return binaryLikeToArrayBuffer(value); } @@ -43,6 +107,7 @@ function toAB(value: BinaryLike): ArrayBuffer { export function argon2Sync(algorithm: string, params: Argon2Params): Buffer { validateAlgorithm(algorithm); const version = params.version ?? ARGON2_VERSION; + validateArgon2Params(params, version); const result = getNative().hashSync( algorithm, toAB(params.message), @@ -65,6 +130,12 @@ export function argon2( ): void { validateAlgorithm(algorithm); const version = params.version ?? ARGON2_VERSION; + try { + validateArgon2Params(params, version); + } catch (err) { + callback(err as Error, Buffer.alloc(0)); + return; + } getNative() .hash( algorithm, diff --git a/packages/react-native-quick-crypto/src/hkdf.ts b/packages/react-native-quick-crypto/src/hkdf.ts index e14aa61b9..ba9736f72 100644 --- a/packages/react-native-quick-crypto/src/hkdf.ts +++ b/packages/react-native-quick-crypto/src/hkdf.ts @@ -45,6 +45,39 @@ function sanitizeInput(input: BinaryLike, name: string): ArrayBuffer { } } +// Output byte-length of each digest the C++ Hkdf supports. Used to enforce +// the RFC 5869 §2.3 ceiling: L ≤ 255 * HashLen. +const HKDF_HASH_BYTES: Readonly> = { + sha1: 20, + sha224: 28, + sha256: 32, + sha384: 48, + sha512: 64, + 'sha3-256': 32, + 'sha3-384': 48, + 'sha3-512': 64, + ripemd160: 20, +}; + +function validateHkdfKeylen(digest: string, keylen: number): void { + if ( + typeof keylen !== 'number' || + !Number.isFinite(keylen) || + !Number.isInteger(keylen) || + keylen < 0 || + keylen > 0x7fff_ffff + ) { + throw new TypeError('Bad key length'); + } + const hashLen = HKDF_HASH_BYTES[digest.toLowerCase()]; + if (hashLen !== undefined && keylen > 255 * hashLen) { + throw new RangeError( + `HKDF keylen ${keylen} exceeds RFC 5869 ceiling ` + + `255 * HashLen (${255 * hashLen}) for ${digest}`, + ); + } +} + export function hkdf( digest: string, key: KeyMaterial, @@ -61,9 +94,7 @@ export function hkdf( const sanitizedSalt = sanitizeInput(salt, 'Salt'); const sanitizedInfo = sanitizeInput(info, 'Info'); - if (keylen < 0) { - throw new TypeError('Bad key length'); - } + validateHkdfKeylen(normalizedDigest, keylen); const nativeMod = getNative(); nativeMod @@ -99,9 +130,7 @@ export function hkdfSync( const sanitizedSalt = sanitizeInput(salt, 'Salt'); const sanitizedInfo = sanitizeInput(info, 'Info'); - if (keylen < 0) { - throw new TypeError('Bad key length'); - } + validateHkdfKeylen(normalizedDigest, keylen); const nativeMod = getNative(); const result = nativeMod.deriveKeySync( @@ -134,6 +163,8 @@ export function hkdfDeriveBits( const hashName = typeof hash === 'string' ? hash : hash.name; const normalizedDigest = normalizeHashName(hashName); + validateHkdfKeylen(normalizedDigest, keylen); + const nativeMod = getNative(); const result = nativeMod.deriveKeySync( normalizedDigest, diff --git a/packages/react-native-quick-crypto/src/scrypt.ts b/packages/react-native-quick-crypto/src/scrypt.ts index 05e127e53..b4315add9 100644 --- a/packages/react-native-quick-crypto/src/scrypt.ts +++ b/packages/react-native-quick-crypto/src/scrypt.ts @@ -35,12 +35,83 @@ const defaults = { maxmem: 32 * 1024 * 1024, }; +// RFC 7914 § 2: scrypt parameters +// N — CPU/memory cost; must be a power of 2 > 1. +// r — block size; positive integer. +// p — parallelization factor; positive integer. +// r * p must be < 2^30 (otherwise the spec output is undefined). +// The work buffer is 128 * r * N bytes, which must fit in maxmem. +const SCRYPT_MAX_RP = 1 << 30; // 2^30 per RFC 7914 + +function isPositiveInteger(value: unknown): value is number { + return ( + typeof value === 'number' && + Number.isFinite(value) && + Number.isInteger(value) && + value > 0 + ); +} + +function validateScryptParams( + N: number, + r: number, + p: number, + maxmem: number, +): void { + if (!isPositiveInteger(N)) { + throw new RangeError(`Invalid scrypt cost (N): ${N}`); + } + // Power-of-two & > 1 check (RFC 7914 §6 step 1). + if (N <= 1 || (N & (N - 1)) !== 0) { + throw new RangeError( + `Invalid scrypt cost (N): ${N} — must be a power of 2 greater than 1`, + ); + } + if (!isPositiveInteger(r)) { + throw new RangeError(`Invalid scrypt blockSize (r): ${r}`); + } + if (!isPositiveInteger(p)) { + throw new RangeError(`Invalid scrypt parallelization (p): ${p}`); + } + if (r * p >= SCRYPT_MAX_RP) { + throw new RangeError( + `Invalid scrypt parameters: r * p (${r * p}) must be < 2^30`, + ); + } + if (!isPositiveInteger(maxmem)) { + throw new RangeError(`Invalid scrypt maxmem: ${maxmem}`); + } + // 128 * r * N is the minimum working memory. Reject early so we don't + // hand a doomed parameter set to native and OOM the device. + const required = 128 * r * N; + if (required > maxmem) { + throw new RangeError( + `Invalid scrypt parameters: working memory ${required} bytes ` + + `exceeds maxmem ${maxmem}`, + ); + } +} + +function validateScryptKeylen(keylen: number): void { + if ( + typeof keylen !== 'number' || + !Number.isFinite(keylen) || + !Number.isInteger(keylen) || + keylen < 0 || + keylen > 0x7fff_ffff + ) { + throw new TypeError('Bad key length'); + } +} + function getScryptParams(options?: ScryptOptions) { const N = options?.N ?? options?.cost ?? defaults.N; const r = options?.r ?? options?.blockSize ?? defaults.r; const p = options?.p ?? options?.parallelization ?? defaults.p; const maxmem = options?.maxmem ?? defaults.maxmem; + validateScryptParams(N, r, p, maxmem); + return { N, r, p, maxmem }; } @@ -85,9 +156,7 @@ export function scrypt( const sanitizedPassword = sanitizeInput(password, 'Password'); const sanitizedSalt = sanitizeInput(salt, 'Salt'); - if (keylen < 0) { - throw new TypeError('Bad key length'); - } + validateScryptKeylen(keylen); const nativeMod = getNative(); nativeMod @@ -115,9 +184,7 @@ export function scryptSync( const sanitizedPassword = sanitizeInput(password, 'Password'); const sanitizedSalt = sanitizeInput(salt, 'Salt'); - if (keylen < 0) { - throw new TypeError('Bad key length'); - } + validateScryptKeylen(keylen); const nativeMod = getNative(); const result = nativeMod.deriveKeySync( From 29ba338a2a61204030f9bce0a425066001f32eb6 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:39:34 -0400 Subject: [PATCH 04/10] =?UTF-8?q?feat(security):=20Phase=203.3=20audit=20?= =?UTF-8?q?=E2=80=94=20RSA=20modulus=20min=202048=20bits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TS layer previously rejected only `modulusLength < 256`. NIST SP 800-131A Rev. 2 has deprecated all RSA keys < 2048 bits for new cryptographic uses, and academic / hardware factoring of 768- and 1024-bit moduli has been demonstrated. Lift the minimum to 2048. `rsa.ts` — share `RSA_MIN_MODULUS_LENGTH = 2048` between the WebCrypto (`rsa_generateKeyPair`) and Node-API (`rsa_prepareKeyGenParams`) entry points. Both throw with a clear "RSA modulusLength must be at least 2048 bits (got N)" message; WebCrypto path remains a DOMException so existing JOSE / spec-conformant callers see the same exception type. Tests: bump 1024 → 2048 in `subtle/generateKey.ts` (3 RSA matrix entries) and `subtle/import_export.ts` (9 generateKey call-sites used to seed export round-trips). Refresh the existing modulusLength=0 rejection assertion to match the new error message, plus an explicit modulusLength=1024 case both at the WebCrypto boundary (`subtle.generateKey`) and the Node boundary (`generateKeyPairSync`). --- example/src/tests/keys/generate_keypair.ts | 12 +++++++++ example/src/tests/subtle/generateKey.ts | 25 +++++++++++++------ example/src/tests/subtle/import_export.ts | 18 ++++++------- packages/react-native-quick-crypto/src/rsa.ts | 22 +++++++++++++--- 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/example/src/tests/keys/generate_keypair.ts b/example/src/tests/keys/generate_keypair.ts index f7822f3b8..4f7c00662 100644 --- a/example/src/tests/keys/generate_keypair.ts +++ b/example/src/tests/keys/generate_keypair.ts @@ -736,6 +736,18 @@ test(SUITE, 'generateKeyPairSync DSA 2048-bit', () => { expect(publicKey).to.match(/^-----BEGIN PUBLIC KEY-----/); }); +// Phase 3.3 regression: 1024-bit RSA is below modern minimums (NIST +// SP 800-131A; RFC 8017). Must be rejected at the JS boundary. +test(SUITE, 'generateKeyPairSync RSA: rejects modulusLength = 1024', () => { + expect(() => { + generateKeyPairSync('rsa', { + modulusLength: 1024, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + }).to.throw(RangeError, /RSA modulusLength must be at least 2048/); +}); + test(SUITE, 'generateKeyPairSync DSA keys work for signing', () => { const { privateKey, publicKey } = generateKeyPairSync('dsa', { modulusLength: 2048, diff --git a/example/src/tests/subtle/generateKey.ts b/example/src/tests/subtle/generateKey.ts index a1db66c5b..9bcddaf86 100644 --- a/example/src/tests/subtle/generateKey.ts +++ b/example/src/tests/subtle/generateKey.ts @@ -68,7 +68,7 @@ const vectors: Vectors = { // }, 'RSASSA-PKCS1-v1_5': { algorithm: { - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -77,7 +77,7 @@ const vectors: Vectors = { }, 'RSA-PSS': { algorithm: { - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -86,7 +86,7 @@ const vectors: Vectors = { }, 'RSA-OAEP': { algorithm: { - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -375,7 +375,7 @@ async function testRSAKeyGen( `Unsupported key usage for a ${name} key`, ); - // Test invalid modulus length + // Test invalid modulus length (Phase 3.3: must be ≥ 2048 bits) await assertThrowsAsync( async () => subtle.generateKey( @@ -383,7 +383,18 @@ async function testRSAKeyGen( true, usages, ), - 'Invalid key length', + 'RSA modulusLength must be at least 2048 bits', + ); + + // Phase 3.3: 1024-bit RSA is below modern minimums and must be rejected. + await assertThrowsAsync( + async () => + subtle.generateKey( + { name, modulusLength: 1024, publicExponent, hash } as any, + true, + usages, + ), + 'RSA modulusLength must be at least 2048 bits', ); }, ); @@ -391,7 +402,7 @@ async function testRSAKeyGen( testRSAKeyGen( 'RSASSA-PKCS1-v1_5', - 1024, + 2048, new Uint8Array([1, 0, 1]), 'SHA-256', ['sign'], @@ -407,7 +418,7 @@ testRSAKeyGen( ); testRSAKeyGen( 'RSA-OAEP', - 1024, + 2048, new Uint8Array([3]), 'SHA-384', ['decrypt', 'unwrapKey'], diff --git a/example/src/tests/subtle/import_export.ts b/example/src/tests/subtle/import_export.ts index 71c780434..8b730fba0 100644 --- a/example/src/tests/subtle/import_export.ts +++ b/example/src/tests/subtle/import_export.ts @@ -1061,7 +1061,7 @@ test(SUITE, 'RSA spki', async () => { const generated = await subtle.generateKey( { name: 'RSA-PSS', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-384', }, @@ -1090,7 +1090,7 @@ test(SUITE, 'RSA pkcs8', async () => { const generated = await subtle.generateKey( { name: 'RSA-PSS', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-384', }, @@ -1120,7 +1120,7 @@ test(SUITE, 'RSA jwk', async () => { const generated = await subtle.generateKey( { name: 'RSA-PSS', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-384', }, @@ -1845,7 +1845,7 @@ test(SUITE, 'RSA-OAEP spki', async () => { const generated = await subtle.generateKey( { name: 'RSA-OAEP', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -1874,7 +1874,7 @@ test(SUITE, 'RSA-OAEP pkcs8', async () => { const generated = await subtle.generateKey( { name: 'RSA-OAEP', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -1903,7 +1903,7 @@ test(SUITE, 'RSA-OAEP jwk', async () => { const generated = await subtle.generateKey( { name: 'RSA-OAEP', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -1944,7 +1944,7 @@ test(SUITE, 'RSASSA-PKCS1-v1_5 spki', async () => { const generated = await subtle.generateKey( { name: 'RSASSA-PKCS1-v1_5', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -1973,7 +1973,7 @@ test(SUITE, 'RSASSA-PKCS1-v1_5 pkcs8', async () => { const generated = await subtle.generateKey( { name: 'RSASSA-PKCS1-v1_5', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, @@ -2002,7 +2002,7 @@ test(SUITE, 'RSASSA-PKCS1-v1_5 jwk', async () => { const generated = await subtle.generateKey( { name: 'RSASSA-PKCS1-v1_5', - modulusLength: 1024, + modulusLength: 2048, publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256', }, diff --git a/packages/react-native-quick-crypto/src/rsa.ts b/packages/react-native-quick-crypto/src/rsa.ts index c9bc71b68..db79bce78 100644 --- a/packages/react-native-quick-crypto/src/rsa.ts +++ b/packages/react-native-quick-crypto/src/rsa.ts @@ -60,6 +60,13 @@ export class Rsa { } } +// Modern best practice (NIST SP 800-131A Rev. 2, IETF RFC 8017): RSA keys +// shorter than 2048 bits are deprecated for both signing and encryption. +// 1024-bit moduli have been factored in academic settings; 768-bit keys +// have been factored on commodity hardware. Reject anything below 2048 +// at the JS boundary so callers can't accidentally generate weak keys. +const RSA_MIN_MODULUS_LENGTH = 2048; + // Node API export async function rsa_generateKeyPair( algorithm: SubtleAlgorithm, @@ -70,8 +77,12 @@ export async function rsa_generateKeyPair( algorithm as RsaHashedKeyGenParams; // Validate parameters first - if (!modulusLength || modulusLength < 256) { - throw lazyDOMException('Invalid key length', 'OperationError'); + if (!modulusLength || modulusLength < RSA_MIN_MODULUS_LENGTH) { + throw lazyDOMException( + `RSA modulusLength must be at least ${RSA_MIN_MODULUS_LENGTH} bits ` + + `(got ${modulusLength ?? 0})`, + 'OperationError', + ); } if (!publicExponent || publicExponent.length === 0) { @@ -198,8 +209,11 @@ function rsa_prepareKeyGenParams( hash?: string; }; - if (!modulusLength || modulusLength < 256) { - throw new Error('Invalid modulus length'); + if (!modulusLength || modulusLength < RSA_MIN_MODULUS_LENGTH) { + throw new RangeError( + `RSA modulusLength must be at least ${RSA_MIN_MODULUS_LENGTH} bits ` + + `(got ${modulusLength ?? 0})`, + ); } const pubExp = publicExponent || 65537; From 118e01551435ac358f4020a66347c66629ea6ca7 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:40:26 -0400 Subject: [PATCH 05/10] =?UTF-8?q?feat(security):=20Phase=203.4=20audit=20?= =?UTF-8?q?=E2=80=94=20DSA=20modulus=20min=201024=20bits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TS layer previously accepted any modulusLength > 0. FIPS 186-4 § 4.2 sanctions only L = 1024, 2048, 3072 bits for DSA, and DSA below 1024 bits is well below modern security minimums. We retain 1024 as the floor (rather than 2048) so legacy interop callers have a fallback path; new applications should still use 2048 or 3072. `dsa.ts` — pull the existing `modulusLength <= 0` check up to a shared `DSA_MIN_MODULUS_LENGTH = 1024` constant, swap the generic Error for RangeError, and emit a "DSA modulusLength must be at least 1024 bits (got N)" message that names the offending value. Tests: 2 new regressions in `keys/generate_keypair.ts`: modulusLength = 512 (between 0 and 1024) and the existing modulusLength = 0 path now both throw the new message. --- example/src/tests/keys/generate_keypair.ts | 22 +++++++++++++++++++ packages/react-native-quick-crypto/src/dsa.ts | 13 +++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/example/src/tests/keys/generate_keypair.ts b/example/src/tests/keys/generate_keypair.ts index 4f7c00662..a1dcd9245 100644 --- a/example/src/tests/keys/generate_keypair.ts +++ b/example/src/tests/keys/generate_keypair.ts @@ -748,6 +748,28 @@ test(SUITE, 'generateKeyPairSync RSA: rejects modulusLength = 1024', () => { }).to.throw(RangeError, /RSA modulusLength must be at least 2048/); }); +// Phase 3.4 regression: DSA below 1024-bit modulus must be rejected at +// the JS boundary (FIPS 186-4 § 4.2 sanctions only 1024 / 2048 / 3072). +test(SUITE, 'generateKeyPairSync DSA: rejects modulusLength < 1024', () => { + expect(() => { + generateKeyPairSync('dsa', { + modulusLength: 512, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + }).to.throw(RangeError, /DSA modulusLength must be at least 1024/); +}); + +test(SUITE, 'generateKeyPairSync DSA: rejects modulusLength = 0', () => { + expect(() => { + generateKeyPairSync('dsa', { + modulusLength: 0, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + }); + }).to.throw(/DSA modulusLength must be at least 1024/); +}); + test(SUITE, 'generateKeyPairSync DSA keys work for signing', () => { const { privateKey, publicKey } = generateKeyPairSync('dsa', { modulusLength: 2048, diff --git a/packages/react-native-quick-crypto/src/dsa.ts b/packages/react-native-quick-crypto/src/dsa.ts index 25dd742c3..e1b2f55d6 100644 --- a/packages/react-native-quick-crypto/src/dsa.ts +++ b/packages/react-native-quick-crypto/src/dsa.ts @@ -25,6 +25,12 @@ export class Dsa { } } +// FIPS 186-4 §4.2: only L = 1024, 2048, 3072 are sanctioned. NIST has +// deprecated DSA-1024 for new applications, but we retain it for +// interop with legacy systems and match Node's permissive default. We +// reject anything below 1024 outright. +const DSA_MIN_MODULUS_LENGTH = 1024; + function dsa_prepareKeyGenParams( options: GenerateKeyPairOptions | undefined, ): Dsa { @@ -34,8 +40,11 @@ function dsa_prepareKeyGenParams( const { modulusLength, divisorLength } = options; - if (!modulusLength || modulusLength <= 0) { - throw new Error('Invalid or missing modulusLength for DSA key generation'); + if (!modulusLength || modulusLength < DSA_MIN_MODULUS_LENGTH) { + throw new RangeError( + `DSA modulusLength must be at least ${DSA_MIN_MODULUS_LENGTH} bits ` + + `(got ${modulusLength ?? 0})`, + ); } return new Dsa(modulusLength, divisorLength); From 6eee7157bcdf57da7bbd47f17f832ef1d5a30f71 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:47:17 -0400 Subject: [PATCH 06/10] =?UTF-8?q?feat(security):=20Phase=203.5=20audit=20?= =?UTF-8?q?=E2=80=94=20WebCrypto=20subtle=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bring `subtle.ts` closer to the WebCrypto spec on four under-enforced edges flagged in the audit: * **Case-insensitive `normalizeAlgorithm`** (§18.4.4). The previous implementation was a no-op, so `'aes-gcm'` reached the `SUPPORTED_ALGORITHMS` set comparisons (which only know the canonical mixed-case form) and failed downstream with confusing not-supported errors. We now build a lazy lowercase→canonical map from `SUPPORTED_ALGORITHMS` on first call and rewrite the algorithm name to its canonical form. Lookup is lazy because `SUPPORTED_ALGORITHMS` is declared after `normalizeAlgorithm` in the file. * **JWK `ext` and `key_ops` validation** (§25.7.6). Added a shared `validateJwkExtAndKeyOps()` helper that throws a `DataError` when `jwk.ext === false` and the requested `extractable` is `true`, and when `jwk.key_ops` is present but does not cover every requested usage. Wired into KMAC, RSA, HMAC, AES, and Ed/CFRG JWK import branches. * **`subtle.deriveBits` strict usage gate** (§SubtleCrypto-method -deriveBits step 11). The previous `deriveBits || deriveKey` accept-either branch silently promoted `deriveKey`-only keys into `deriveBits` use, contradicting the spec. Now requires `deriveBits` on `baseKey.[[usages]]`. `subtle.deriveKey` still accepts either, in line with Node-compat. * **HKDF `extractable: false`** (§28.7.6). `hkdfImportKey` now throws `SyntaxError` if the caller requests `extractable: true`, and forces `extractable: false` on the resulting `CryptoKey` regardless of input — matching the spec invariant that prevents input keying material from round-tripping via `exportKey`. Tests: * `subtle/digest.ts` — `digest('sha-256')` (lowercase) round-trips. * `subtle/deriveBits.ts` — PBKDF2 baseKey with only `deriveKey` usage must reject the `deriveBits` operation. * `hkdf/hkdf_tests.ts` — `extractable=true` rejection plus an invariant check that `extractable=false` propagates. * `subtle/import_export.ts` — three new AES JWK import paths: `ext=false + extractable=true` rejection, `key_ops` missing the requested usage rejection, and a positive-path `key_ops` superset. --- example/src/tests/hkdf/hkdf_tests.ts | 34 +++++++ example/src/tests/subtle/deriveBits.ts | 39 ++++++++ example/src/tests/subtle/digest.ts | 11 +++ example/src/tests/subtle/import_export.ts | 66 +++++++++++++ .../react-native-quick-crypto/src/subtle.ts | 94 +++++++++++++++++-- 5 files changed, 236 insertions(+), 8 deletions(-) diff --git a/example/src/tests/hkdf/hkdf_tests.ts b/example/src/tests/hkdf/hkdf_tests.ts index 9dfac0300..8f96ceb1d 100644 --- a/example/src/tests/hkdf/hkdf_tests.ts +++ b/example/src/tests/hkdf/hkdf_tests.ts @@ -130,6 +130,40 @@ test(SUITE, 'hkdf: surfaces ceiling errors via callback', async () => { }); }); +// Phase 3.5 regression: WebCrypto §28.7.6 mandates HKDF keys be created +// with extractable=false. The previous implementation passed `extractable` +// through verbatim, allowing input keying material to round-trip via +// exportKey — defeating the deriveBits-only usage. +test(SUITE, 'HKDF importKey: rejects extractable=true', async () => { + const ikm = Buffer.from('00'.repeat(16), 'hex'); + let threw: Error | undefined; + try { + await crypto.subtle.importKey('raw', ikm, { name: 'HKDF' }, true, [ + 'deriveBits', + ]); + } catch (e) { + threw = e as Error; + } + expect(threw).to.not.equal(undefined); + expect(threw!.message).to.match(/HKDF keys are not extractable/); +}); + +test( + SUITE, + 'HKDF importKey: forces extractable=false even when false', + async () => { + const ikm = Buffer.from('00'.repeat(16), 'hex'); + const key = await crypto.subtle.importKey( + 'raw', + ikm, + { name: 'HKDF' }, + false, + ['deriveBits'], + ); + expect(key.extractable).to.equal(false); + }, +); + test(SUITE, 'WebCrypto HKDF deriveKey (AES-GCM)', async () => { const vec = testVectors[0]!; const ikm = Buffer.from(vec.ikm, 'hex'); diff --git a/example/src/tests/subtle/deriveBits.ts b/example/src/tests/subtle/deriveBits.ts index 4163c0662..afaceac4a 100644 --- a/example/src/tests/subtle/deriveBits.ts +++ b/example/src/tests/subtle/deriveBits.ts @@ -460,3 +460,42 @@ test(SUITE, 'EC diffieHellman - curve mismatch throws', () => { }); }).to.throw('Private and public key curves do not match'); }); + +// Phase 3.5 regression: subtle.deriveBits MUST require the literal +// "deriveBits" usage on the base key (WebCrypto §SubtleCrypto-method +// -deriveBits step 11). The previous implementation accepted "deriveKey" +// usage as a substitute, which silently allowed callers to bypass the +// usage gate. +test( + SUITE, + 'deriveBits: rejects baseKey without deriveBits usage', + async () => { + const ikm = Buffer.from('deadbeef'.repeat(8), 'hex'); + const key = await subtle.importKey( + 'raw', + ikm, + { name: 'PBKDF2' }, + false, + ['deriveKey'], // intentionally NOT 'deriveBits' + ); + + let threw: Error | undefined; + try { + await subtle.deriveBits( + { + name: 'PBKDF2', + salt: 'salt', + iterations: 10, + hash: 'SHA-256', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any, + key, + 256, + ); + } catch (e) { + threw = e as Error; + } + expect(threw).to.not.equal(undefined); + expect(threw!.message).to.match(/deriveBits usage/); + }, +); diff --git a/example/src/tests/subtle/digest.ts b/example/src/tests/subtle/digest.ts index a88b9d4ba..0178417be 100644 --- a/example/src/tests/subtle/digest.ts +++ b/example/src/tests/subtle/digest.ts @@ -16,6 +16,17 @@ test(SUITE, 'empty hash just works', async () => { await subtle.digest('SHA-512', Buffer.alloc(0)); }); +// Phase 3.5 regression: WebCrypto §18.4.4 mandates case-insensitive +// algorithm name matching. The previous `normalizeAlgorithm` was a +// no-op and lowercase strings reached `SUPPORTED_ALGORITHMS` set +// comparisons which only accepted the canonical mixed-case form. +test(SUITE, 'digest accepts lowercase algorithm name', async () => { + const expected = createHash('sha256').update(kData).digest().toString('hex'); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const value = await subtle.digest('sha-256' as any, kData); + expect(ab2str(value)).to.equal(expected); +}); + const kTests: Test[] = [ ['SHA-1', 'sha1', 160], ['SHA-256', 'sha256', 256], diff --git a/example/src/tests/subtle/import_export.ts b/example/src/tests/subtle/import_export.ts index 8b730fba0..a3fff4825 100644 --- a/example/src/tests/subtle/import_export.ts +++ b/example/src/tests/subtle/import_export.ts @@ -262,6 +262,72 @@ const withZero = getRandomValues(new Uint8Array(32)); withZero[4] = 0; testFn(withZero as Uint8Array, 'with zero'); +// Phase 3.5 regression: WebCrypto §25.7.6 — JWK import must reject when +// `jwk.ext === false` and `extractable === true`, and must reject when +// `jwk.key_ops` is present and any requested usage is missing from it. +test( + SUITE, + 'AES import jwk: rejects ext=false with extractable=true', + async () => { + const jwk: JWK = { + kty: 'oct', + k: 'Y0zt37HgOx-BY7SQjYVmrqhPkO44Ii2Jcb9yydUDPfE.', + alg: 'A256GCM', + ext: false, + }; + await assertThrowsAsync( + async () => + await subtle.importKey( + 'jwk', + jwk, + { name: 'AES-GCM' }, + true, // extractable + ['encrypt', 'decrypt'], + ), + 'JWK "ext" is false but extractable was requested', + ); + }, +); + +test( + SUITE, + 'AES import jwk: rejects key_ops missing requested usage', + async () => { + const jwk: JWK = { + kty: 'oct', + k: 'Y0zt37HgOx-BY7SQjYVmrqhPkO44Ii2Jcb9yydUDPfE.', + alg: 'A256GCM', + ext: true, + key_ops: ['encrypt'], // intentionally NOT 'decrypt' + }; + await assertThrowsAsync( + async () => + await subtle.importKey( + 'jwk', + jwk, + { name: 'AES-GCM' }, + true, + ['encrypt', 'decrypt'], // requests 'decrypt' + ), + 'JWK "key_ops" does not include requested usage "decrypt"', + ); + }, +); + +test(SUITE, 'AES import jwk: accepts when key_ops covers usages', async () => { + const jwk: JWK = { + kty: 'oct', + k: 'Y0zt37HgOx-BY7SQjYVmrqhPkO44Ii2Jcb9yydUDPfE.', + alg: 'A256GCM', + ext: true, + key_ops: ['encrypt', 'decrypt', 'wrapKey'], + }; + const key = await subtle.importKey('jwk', jwk, { name: 'AES-GCM' }, true, [ + 'encrypt', + ]); + expect(key.algorithm.name).to.equal('AES-GCM'); +}); + // from https://gist.github.com/pedrouid/b4056fd1f754918ddae86b32cf7d803e#aes-gcm---importkey test(SUITE, 'AES import jwk / export jwk', async () => { const origKey: string = 'Y0zt37HgOx-BY7SQjYVmrqhPkO44Ii2Jcb9yydUDPfE.'; diff --git a/packages/react-native-quick-crypto/src/subtle.ts b/packages/react-native-quick-crypto/src/subtle.ts index 70ce2b670..9b5cf041a 100644 --- a/packages/react-native-quick-crypto/src/subtle.ts +++ b/packages/react-native-quick-crypto/src/subtle.ts @@ -83,16 +83,74 @@ function hasAnyNotIn(usages: KeyUsage[], allowed: KeyUsage[]): boolean { return usages.some(usage => !allowed.includes(usage)); } +// WebCrypto §18.4.4: algorithm name lookup is case-insensitive, but the +// canonical mixed-case form is preserved in the resulting `name` field +// (e.g. "aes-gcm" → "AES-GCM"). This map is built lazily on first call so +// the registry of canonical names below can stay declared after the +// function. Without this, callers who pass lowercase strings bypass the +// downstream `SUPPORTED_ALGORITHMS` set comparisons silently. +let _canonicalAlgorithmNames: Map | null = null; +function getCanonicalAlgorithmNames(): Map { + if (_canonicalAlgorithmNames === null) { + const map = new Map(); + for (const set of Object.values(SUPPORTED_ALGORITHMS)) { + if (!set) continue; + for (const name of set) { + map.set(name.toLowerCase(), name); + } + } + _canonicalAlgorithmNames = map; + } + return _canonicalAlgorithmNames; +} + function normalizeAlgorithm( algorithm: SubtleAlgorithm | AnyAlgorithm, _operation: Operation, ): SubtleAlgorithm { + const map = getCanonicalAlgorithmNames(); if (typeof algorithm === 'string') { - return { name: algorithm }; + const canonical = map.get(algorithm.toLowerCase()) ?? algorithm; + return { name: canonical as AnyAlgorithm }; + } + if (typeof algorithm.name === 'string') { + const canonical = map.get(algorithm.name.toLowerCase()) ?? algorithm.name; + return { ...algorithm, name: canonical as AnyAlgorithm } as SubtleAlgorithm; } return algorithm as SubtleAlgorithm; } +// WebCrypto §25.7.6 (JWK import): if the JWK's `ext` member is present and +// false, the requested `extractable` parameter must also be false. If the +// JWK's `key_ops` member is present, every requested usage must appear in +// it. We centralize the check here so every importKey path that accepts +// `format === 'jwk'` can reuse it. +function validateJwkExtAndKeyOps( + jwk: JWK, + extractable: boolean, + keyUsages: KeyUsage[], +): void { + if (jwk.ext === false && extractable) { + throw lazyDOMException( + 'JWK "ext" is false but extractable was requested', + 'DataError', + ); + } + if (jwk.key_ops !== undefined) { + if (!Array.isArray(jwk.key_ops)) { + throw lazyDOMException('JWK "key_ops" must be an array', 'DataError'); + } + for (const usage of keyUsages) { + if (!jwk.key_ops.includes(usage)) { + throw lazyDOMException( + `JWK "key_ops" does not include requested usage "${usage}"`, + 'DataError', + ); + } + } + } +} + function getAlgorithmName(name: string, length: number): string { switch (name) { case 'AES-CBC': @@ -816,6 +874,8 @@ async function kmacImportKey( throw lazyDOMException('Invalid keyData', 'DataError'); } + validateJwkExtAndKeyOps(jwk, extractable, keyUsages); + if (jwk.kty !== 'oct') { throw lazyDOMException('Invalid JWK format for KMAC key', 'DataError'); } @@ -902,6 +962,8 @@ function rsaImportKey( throw new Error('Invalid JWK format for RSA key'); } + validateJwkExtAndKeyOps(jwk, extractable, keyUsages); + const handle = NitroModules.createHybridObject('KeyObjectHandle'); const keyType = handle.initJwk(jwk, undefined); @@ -992,6 +1054,8 @@ async function hmacImportKey( throw new Error('Invalid keyData'); } + validateJwkExtAndKeyOps(jwk, extractable, keyUsages); + if (jwk.kty !== 'oct') { throw new Error('Invalid JWK format for HMAC key'); } @@ -1069,6 +1133,8 @@ async function aesImportKey( throw new Error('Invalid JWK format for AES key'); } + validateJwkExtAndKeyOps(jwk, extractable, keyUsages); + const handle = NitroModules.createHybridObject('KeyObjectHandle'); const keyType = handle.initJwk(jwk, undefined); @@ -1164,6 +1230,7 @@ function edImportKey( keyObject = new PublicKeyObject(handle); } else if (format === 'jwk') { const jwkData = data as JWK; + validateJwkExtAndKeyOps(jwkData, extractable, keyUsages); const handle = NitroModules.createHybridObject('KeyObjectHandle'); const keyType = handle.initJwk(jwkData); @@ -1588,6 +1655,13 @@ const hkdfImportKey = async ( keyUsages: KeyUsage[], ): Promise => { const { name } = algorithm; + // WebCrypto §28.7.6: HKDF keys are never extractable. The previous + // implementation passed `extractable` through verbatim, allowing callers + // to round-trip the input keying material via `exportKey` — defeating + // the whole point of the deriveBits-only usage. + if (extractable) { + throw lazyDOMException(`${name} keys are not extractable`, 'SyntaxError'); + } if (hasAnyNotIn(keyUsages, ['deriveKey', 'deriveBits'])) { throw new Error(`Unsupported key usage for a ${name} key`); } @@ -1595,7 +1669,7 @@ const hkdfImportKey = async ( switch (format) { case 'raw': { const keyObject = createSecretKey(keyData as BinaryLike); - return new CryptoKey(keyObject, { name }, keyUsages, extractable); + return new CryptoKey(keyObject, { name }, keyUsages, false); } default: throw new Error(`Unable to import ${name} key with format ${format}`); @@ -2161,12 +2235,16 @@ export class Subtle { baseKey: CryptoKey, length: number, ): Promise { - // Allow either deriveBits OR deriveKey usage (WebCrypto spec allows both) - if ( - !baseKey.keyUsages.includes('deriveBits') && - !baseKey.keyUsages.includes('deriveKey') - ) { - throw new Error('baseKey does not have deriveBits or deriveKey usage'); + // WebCrypto §SubtleCrypto.deriveBits step 11: throw InvalidAccessError + // unless `baseKey.[[usages]]` contains "deriveBits" specifically. The + // previous `deriveBits || deriveKey` accept-either branch silently + // promoted deriveKey-only keys into deriveBits use, contradicting the + // spec usage gate. + if (!baseKey.keyUsages.includes('deriveBits')) { + throw lazyDOMException( + 'baseKey does not have deriveBits usage', + 'InvalidAccessError', + ); } if (baseKey.algorithm.name !== algorithm.name) throw new Error('Key algorithm mismatch'); From 5b272f489b91a6b174d6d502ba9eeb57e7f22243 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:50:51 -0400 Subject: [PATCH 07/10] =?UTF-8?q?feat(security):=20Phase=203.6=20audit=20?= =?UTF-8?q?=E2=80=94=20stream=20=5Ftransform/=5Fflush=20error=20propagatio?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Synchronous failures inside `Hash`, `Hmac`, and `Cipher` `_transform` / `_flush` previously threw out of the Transform plumbing because the callback was only invoked on the success path. Throws are eventually converted to 'error' events by readable-stream, but the stream is left in a half-written state — the callback contract requires exactly one invocation, and a missing call can hang downstream consumers. Wrap each `_transform` and `_flush` body in a try/catch that forwards the thrown error through the callback so it emits as a regular stream 'error' event and the Transform sees the callback once on every code path. Also widen the callback parameter type from `() => void` to `(err?: Error | null) => void` so the new error-forwarding usage type- checks. Tests: 6 new regressions — Hash and Hmac `_transform` after digest() (invokes update() on a finalized native context) and `_flush` after digest() (double-finalize), plus Cipher `_transform` after final() and Decipher `_flush` with a tampered AES-GCM auth tag. --- example/src/tests/cipher/cipher_tests.ts | 52 +++++++++++++++++++ example/src/tests/hash/hash_tests.ts | 34 ++++++++++++ example/src/tests/hmac/hmac_tests.ts | 32 ++++++++++++ .../react-native-quick-crypto/src/cipher.ts | 25 ++++++--- .../react-native-quick-crypto/src/hash.ts | 24 ++++++--- .../react-native-quick-crypto/src/hmac.ts | 24 ++++++--- 6 files changed, 171 insertions(+), 20 deletions(-) diff --git a/example/src/tests/cipher/cipher_tests.ts b/example/src/tests/cipher/cipher_tests.ts index 4bd902bf4..795ef01c6 100644 --- a/example/src/tests/cipher/cipher_tests.ts +++ b/example/src/tests/cipher/cipher_tests.ts @@ -622,3 +622,55 @@ test(SUITE, 'createCipheriv: rejects wrong xsalsa20 nonce length', () => { createCipheriv('xsalsa20', key32, iv16); }).to.throw(RangeError, /Invalid iv length 16 .* xsalsa20/); }); + +// Phase 3.6 regression: stream _transform / _flush errors (e.g. AEAD +// auth-tag mismatch on Decipher.final()) must flow through the +// callback so they emit as 'error' events on the stream. + +test(SUITE, 'Decipher._flush: surfaces auth-tag mismatch via callback', () => { + // Encrypt to obtain a valid (key, iv, tag) triple, then tamper with the + // auth tag so Decipher.final() rejects authentication. + const testKey = Buffer.from(randomFillSync(new Uint8Array(32))); + const testIv = randomFillSync(new Uint8Array(12)); + const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv)); + cipher.setAAD(aad); + const encrypted = Buffer.concat([ + cipher.update(plaintextBuffer), + cipher.final(), + ]); + const tag = cipher.getAuthTag(); + tag[0] = tag[0]! ^ 0xff; + + const decipher = createDecipheriv( + 'aes-256-gcm', + testKey, + Buffer.from(testIv), + ); + decipher.setAAD(aad); + decipher.setAuthTag(tag); + decipher.update(encrypted); + + let received: Error | null | undefined = undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (decipher as any)._flush((err: Error | null) => { + received = err; + }); + expect(received).to.be.instanceOf(Error); +}); + +test(SUITE, 'Cipher._transform: surfaces update() error via callback', () => { + const cipher = createCipheriv('aes-128-cbc', key16, iv16); + cipher.update(plaintextBuffer); + cipher.final(); // post-final — next update() throws + + let received: Error | null | undefined = undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (cipher as any)._transform( + Buffer.from('after final'), + 'utf8', + (err: Error | null) => { + received = err; + }, + ); + expect(received).to.be.instanceOf(Error); +}); diff --git a/example/src/tests/hash/hash_tests.ts b/example/src/tests/hash/hash_tests.ts index 4ad2aa0fe..7eb1d7a28 100644 --- a/example/src/tests/hash/hash_tests.ts +++ b/example/src/tests/hash/hash_tests.ts @@ -321,3 +321,37 @@ test(SUITE, 'hash() oneshot - Buffer input', () => { const expected = createHash('sha256').update(data).digest('hex'); expect(result).to.equal(expected); }); + +// Phase 3.6 regression: synchronous failures inside `_transform` and +// `_flush` must surface via the stream callback (i.e. become 'error' +// events) rather than throwing out of the Transform plumbing — which +// can leave the stream in a half-written state and crash the host +// pipeline. + +test(SUITE, 'Hash._transform: surfaces update() error via callback', () => { + const h = createHash('sha256'); + h.digest(); // put the native context in finalized state — next update() throws + + let received: Error | null | undefined = undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (h as any)._transform( + Buffer.from('after digest'), + 'utf8', + (err: Error | null) => { + received = err; + }, + ); + expect(received).to.be.instanceOf(Error); +}); + +test(SUITE, 'Hash._flush: surfaces digest() error via callback', () => { + const h = createHash('sha256'); + h.digest(); // first digest — second call throws + + let received: Error | null | undefined = undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (h as any)._flush((err: Error | null) => { + received = err; + }); + expect(received).to.be.instanceOf(Error); +}); diff --git a/example/src/tests/hmac/hmac_tests.ts b/example/src/tests/hmac/hmac_tests.ts index b64c8738e..2c651d94f 100644 --- a/example/src/tests/hmac/hmac_tests.ts +++ b/example/src/tests/hmac/hmac_tests.ts @@ -518,3 +518,35 @@ test(SUITE, 'digest with ucs2 encoding', () => { crypto.createHmac('sha256', 'w00t').digest().toString('ucs2'), ); }); + +// Phase 3.6 regression: stream _transform / _flush errors must flow +// through the callback so they emit as 'error' events rather than +// throwing through the Transform plumbing. + +test(SUITE, 'Hmac._transform: surfaces update() error via callback', () => { + const h = createHmac('sha256', 'k'); + h.digest(); // post-digest state — next update() throws + + let received: Error | null | undefined = undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (h as any)._transform( + Buffer.from('after digest'), + 'utf8', + (err: Error | null) => { + received = err; + }, + ); + expect(received).to.be.instanceOf(Error); +}); + +test(SUITE, 'Hmac._flush: surfaces digest() error via callback', () => { + const h = createHmac('sha256', 'k'); + h.digest(); + + let received: Error | null | undefined = undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (h as any)._flush((err: Error | null) => { + received = err; + }); + expect(received).to.be.instanceOf(Error); +}); diff --git a/packages/react-native-quick-crypto/src/cipher.ts b/packages/react-native-quick-crypto/src/cipher.ts index 481a451b5..34d4a92a5 100644 --- a/packages/react-native-quick-crypto/src/cipher.ts +++ b/packages/react-native-quick-crypto/src/cipher.ts @@ -264,18 +264,31 @@ class CipherCommon extends Stream.Transform { return Buffer.from(ret); } + // Stream interface — surface synchronous errors (bad encoding, + // OpenSSL EVP failures, AEAD tag mismatch in `final()`, etc.) via + // the callback so they emit as stream 'error' events instead of + // throwing out of the Transform plumbing and crashing the host + // pipeline. _transform( chunk: BinaryLike, encoding: BufferEncoding, - callback: () => void, + callback: (err?: Error | null) => void, ) { - this.push(this.update(chunk, normalizeEncoding(encoding))); - callback(); + try { + this.push(this.update(chunk, normalizeEncoding(encoding))); + callback(); + } catch (err) { + callback(err as Error); + } } - _flush(callback: () => void) { - this.push(this.final()); - callback(); + _flush(callback: (err?: Error | null) => void) { + try { + this.push(this.final()); + callback(); + } catch (err) { + callback(err as Error); + } } public setAutoPadding(autoPadding?: boolean): this { diff --git a/packages/react-native-quick-crypto/src/hash.ts b/packages/react-native-quick-crypto/src/hash.ts index d5493e3fa..d51fd01c5 100644 --- a/packages/react-native-quick-crypto/src/hash.ts +++ b/packages/react-native-quick-crypto/src/hash.ts @@ -183,18 +183,28 @@ class Hash extends Stream.Transform { return this.native.getOpenSSLVersion(); } - // stream interface + // Stream interface — surface synchronous errors via the callback so + // they emit as stream 'error' events instead of throwing out of the + // Transform plumbing (which would crash the host pipeline). _transform( chunk: BinaryLike, encoding: BufferEncoding, - callback: () => void, + callback: (err?: Error | null) => void, ) { - this.update(chunk, encoding as Encoding); - callback(); + try { + this.update(chunk, encoding as Encoding); + callback(); + } catch (err) { + callback(err as Error); + } } - _flush(callback: () => void) { - this.push(this.digest()); - callback(); + _flush(callback: (err?: Error | null) => void) { + try { + this.push(this.digest()); + callback(); + } catch (err) { + callback(err as Error); + } } } diff --git a/packages/react-native-quick-crypto/src/hmac.ts b/packages/react-native-quick-crypto/src/hmac.ts index 4cfab0df7..1cc8a30ca 100644 --- a/packages/react-native-quick-crypto/src/hmac.ts +++ b/packages/react-native-quick-crypto/src/hmac.ts @@ -85,18 +85,28 @@ class Hmac extends Stream.Transform { return Buffer.from(nativeDigest); } - // stream interface + // Stream interface — surface synchronous errors via the callback so + // they emit as stream 'error' events instead of throwing out of the + // Transform plumbing. _transform( chunk: BinaryLike, encoding: BufferEncoding, - callback: () => void, + callback: (err?: Error | null) => void, ) { - this.update(chunk, encoding as Encoding); - callback(); + try { + this.update(chunk, encoding as Encoding); + callback(); + } catch (err) { + callback(err as Error); + } } - _flush(callback: () => void) { - this.push(this.digest()); - callback(); + _flush(callback: (err?: Error | null) => void) { + try { + this.push(this.digest()); + callback(); + } catch (err) { + callback(err as Error); + } } } From a1a3eda580a1bf0b3bf3ce9f0d57e1479b9925ec Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 09:53:16 -0400 Subject: [PATCH 08/10] docs(security): mark Phase 3 complete + log CI fixes and 6 sub-tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tick the Phase 3 boxes (cipher params, KDFs, RSA/DSA modulus mins, subtle hardening, stream error propagation) and append progress-log entries for the CI warning sweep (PR #984 follow-up) and each of 3.1–3.6 with branch reference. --- plans/todo/security-audit.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/plans/todo/security-audit.md b/plans/todo/security-audit.md index c6dbc5725..8eb9ccc93 100644 --- a/plans/todo/security-audit.md +++ b/plans/todo/security-audit.md @@ -1228,12 +1228,12 @@ Depends on Phase 1. ### Phase 3 — TypeScript Boundary Validation -- [ ] Cipher algorithm/key/IV length validation at TS layer -- [ ] KDF parameter validation: Scrypt N power-of-2; HKDF max `255 * HashLen`; Argon2 RFC 9106 mins; PBKDF2 already done — use as template -- [ ] RSA modulus min 2048 bits (currently 256) -- [ ] DSA modulus min 1024 bits (currently 0) -- [ ] WebCrypto `subtle`: case-insensitive `normalizeAlgorithm`; JWK `ext`/`key_ops`; `deriveBits` usage; HKDF `extractable: false` -- [ ] Stream `_transform`/`_flush` error propagation via callback (Hash, HMAC, all ciphers) +- [x] Cipher algorithm/key/IV length validation at TS layer +- [x] KDF parameter validation: Scrypt N power-of-2; HKDF max `255 * HashLen`; Argon2 RFC 9106 mins; PBKDF2 already done — use as template +- [x] RSA modulus min 2048 bits (currently 256) +- [x] DSA modulus min 1024 bits (currently 0) +- [x] WebCrypto `subtle`: case-insensitive `normalizeAlgorithm`; JWK `ext`/`key_ops`; `deriveBits` usage; HKDF `extractable: false` +- [x] Stream `_transform`/`_flush` error propagation via callback (Hash, HMAC, all ciphers) ### Phase 4 — Test Vector Coverage @@ -1264,4 +1264,11 @@ _Append entries as PRs land. Format: `YYYY-MM-DD — [phase.task] description (P - 2026-04-26 — [0.3] Add explicit peer-public-key validation in `DiffieHellman::computeSecret` and `ECDH::computeSecret`. DH path calls `DH_check_pub_key` (matching ncrypto's `DHPointer::checkPublicKey`) and distinguishes TOO_SMALL / TOO_LARGE / INVALID error codes, closing the small-subgroup attack on peer pubkeys 0, 1, p-1, and p. ECDH path calls `EC_POINT_oct2point` → `EC_POINT_is_at_infinity` → `EC_POINT_is_on_curve` against the configured group, closing the invalid-curve attack (peer point on a related weaker curve). Adds 4 DH and 5 ECDH regression tests covering each rejection path plus a cross-curve attack (P-384 pubkey sent to a P-256 instance) and a bit-flipped-coordinate test. Crypto-specialist review approved both fixes; flagged that the q-less subgroup gap for caller-supplied DH primes matches Node.js behavior and is not a regression. (branch: `feat/security-audit`, PR: TBD) - 2026-04-26 — [0.4] Close the RSA PKCS#1 v1.5 Bleichenbacher oracle. `HybridRsaCipher` now (1) enables OpenSSL 3.2+ implicit rejection (`EVP_PKEY_CTX_ctrl_str(ctx, "rsa_pkcs1_implicit_rejection", "1")`) for every PKCS#1 v1.5 decryption — corrupted ciphertexts deterministically decrypt to random-looking bytes instead of throwing — and (2) routes every decrypt-failure path in `decrypt`, `privateDecrypt`, and `publicDecrypt` (verify-recover) through a single `throwOpaqueDecryptFailure()` helper that emits the same `"RSA decryption failed"` message and clears the OpenSSL error stack so the underlying reason never reaches the caller. The TS wrapper drops the `: ${error.message}` interpolation in `privateDecrypt`/`publicDecrypt`. If the OpenSSL build does not support the implicit-rejection knob (BoringSSL or pre-3.2) we hard-fail PKCS#1 v1.5 decryption with a build-config error rather than silently leaving the timing-side oracle open — matches Node.js's `crypto_cipher.cc` policy. Adds 5 regression tests: corrupted PKCS#1 v1.5 doesn't throw, the implicit-rejection output is deterministic per (key, ciphertext) and distinct across different ciphertexts, OAEP/wrong-label errors are opaque (no "openssl/padding/oaep/label" terms in the message), OAEP and PKCS#1 wrong-padding errors are equivalent, and `publicDecrypt` errors are opaque. Crypto-specialist review confirmed the fix is closer to Node-compat than the previous behavior and approved the hard-fail fallback. (branch: `feat/security-audit`, PR: TBD) - 2026-04-26 — [1.1–1.4] Phase 1 shared foundation: add `validateUInt()`, `secureZero()` overloads, `EVP_CIPHER_CTX` RAII in the cipher base, and a typed `getUIntOption` helper to `cpp/utils/QuickCryptoUtils.hpp` and `src/utils/cipher.ts`. Sweeps the cipher base + GCM/CCM/ChaCha20/ChaCha20-Poly1305/OCB/XSalsa20-Poly1305 to consume the new RAII context. Adds Argon2/cipher boundary tests. (PR #983) -- 2026-04-26 — [2.1–2.5] Phase 2 memory safety sweep across 24 C++ files (+327/−480 lines net). Item 2.1: convert raw `new uint8_t[]` to `std::unique_ptr` + `release()` into `NativeArrayBuffer` in Hash, HMAC, KMAC, BLAKE3, PBKDF2, Scrypt, HKDF, the cipher base `update()`, ChaCha20/ChaCha20-Poly1305/XChaCha20-Poly1305/XSalsa20-Poly1305, CCM `final()`, RSA-cipher decrypt sentinels, Ed25519 (6 sites), ML-DSA (3 sites), and ML-KEM (4 sites). Item 2.2: replace raw `EVP_PKEY*` ownership with `std::unique_ptr` in RSA, EC, and Ed25519 keypair classes (DSA pattern as template); `Ed25519::importPublicKey`/`importPrivateKey` now return owning `EVP_PKEY_ptr` and use `EVP_PKEY_up_ref` for the borrow-the-instance-key path, closing the audit-flagged leak. Item 2.3: replace `Promise<…>::async([this, …])` with `auto self = this->shared_cast<…>(); [self, …]` in ML-DSA (3 sites) and ML-KEM (3 sites); DH had no async sites despite the audit listing. Item 2.4: eliminate the unnecessary `EVP_PKEY_CTX_new_from_name` pre-creation in Sign/Verify handles, ML-DSA, and Ed25519 — pass `nullptr` for the `EVP_PKEY_CTX**` arg and let `EVP_DigestSignInit` allocate from the key's keymgmt (matches `ncrypto::EVPMDCtxPointer::signInit`). Crypto-specialist review confirmed the old code was actually *leaking* the pre-allocated PKEY_CTX (OpenSSL silently overwrote the pointer on success), so this fix closes both the audited double-free *and* an unreported leak. Wraps EVP_MD_CTX/EVP_PKEY_CTX in local `unique_ptr` aliases so all manual error-path frees collapse. Item 2.5: replace Ed25519's two `ERR_error_string(ERR_get_error(), NULL)` calls with the shared `getOpenSSLError()` helper. Defense-in-depth: `secureZero` added on Scrypt/HKDF error paths and on Ed25519/ML-DSA/ML-KEM `getPrivateKey` BIO buffers. Crypto-specialist approved all four substantive concerns (algorithm selection unchanged, refcount semantics correct, BIO secure-zero is safe redundancy with `BUF_MEM_free`'s `OPENSSL_clear_free`, `release()` + `make_shared` window matches Nitro's own `ArrayBuffer::wrap`). (branch: `feat/security-audit-phase-2`, PR: TBD) +- 2026-04-26 — [2.1–2.5] Phase 2 memory safety sweep across 24 C++ files (+327/−480 lines net). Item 2.1: convert raw `new uint8_t[]` to `std::unique_ptr` + `release()` into `NativeArrayBuffer` in Hash, HMAC, KMAC, BLAKE3, PBKDF2, Scrypt, HKDF, the cipher base `update()`, ChaCha20/ChaCha20-Poly1305/XChaCha20-Poly1305/XSalsa20-Poly1305, CCM `final()`, RSA-cipher decrypt sentinels, Ed25519 (6 sites), ML-DSA (3 sites), and ML-KEM (4 sites). Item 2.2: replace raw `EVP_PKEY*` ownership with `std::unique_ptr` in RSA, EC, and Ed25519 keypair classes (DSA pattern as template); `Ed25519::importPublicKey`/`importPrivateKey` now return owning `EVP_PKEY_ptr` and use `EVP_PKEY_up_ref` for the borrow-the-instance-key path, closing the audit-flagged leak. Item 2.3: replace `Promise<…>::async([this, …])` with `auto self = this->shared_cast<…>(); [self, …]` in ML-DSA (3 sites) and ML-KEM (3 sites); DH had no async sites despite the audit listing. Item 2.4: eliminate the unnecessary `EVP_PKEY_CTX_new_from_name` pre-creation in Sign/Verify handles, ML-DSA, and Ed25519 — pass `nullptr` for the `EVP_PKEY_CTX**` arg and let `EVP_DigestSignInit` allocate from the key's keymgmt (matches `ncrypto::EVPMDCtxPointer::signInit`). Crypto-specialist review confirmed the old code was actually *leaking* the pre-allocated PKEY_CTX (OpenSSL silently overwrote the pointer on success), so this fix closes both the audited double-free *and* an unreported leak. Wraps EVP_MD_CTX/EVP_PKEY_CTX in local `unique_ptr` aliases so all manual error-path frees collapse. Item 2.5: replace Ed25519's two `ERR_error_string(ERR_get_error(), NULL)` calls with the shared `getOpenSSLError()` helper. Defense-in-depth: `secureZero` added on Scrypt/HKDF error paths and on Ed25519/ML-DSA/ML-KEM `getPrivateKey` BIO buffers. Crypto-specialist approved all four substantive concerns (algorithm selection unchanged, refcount semantics correct, BIO secure-zero is safe redundancy with `BUF_MEM_free`'s `OPENSSL_clear_free`, `release()` + `make_shared` window matches Nitro's own `ArrayBuffer::wrap`). (PR #984) +- 2026-04-27 — [CI] Fix the 6 GitHub Actions warnings surfaced on PR #984's runs: opt all workflows into Node.js 24 via `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24` (silences the deprecation warning for `setup-java@v4`, `upload/download-artifact@v4`, `setup-android@v3`, `peter-evans/*`, `McCzarny/*`, `reviewdog/*`); bump `actions/checkout@v4` and `actions/setup-node@v4` to `@v5` everywhere; replace the `${{ github.run_id }}` Gradle/node_modules/AVD cache keys with `hashFiles(...)`-based keys (and a stable `avd-pixel7pro-34-x86_64-v1` for the AVD) to fix the "another job may be creating this cache" save failures; bump library AGP `classpath` 8.7.3 → 8.12.2 (matches Nitro) and disable the `AndroidGradlePluginVersion` / `GradleDependency` lint checks since AGP 9.x requires Gradle 9 + JDK 21 which RN 0.81's toolchain can't supply. (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [3.1] Pre-validate cipher algorithm, key, and IV byte-lengths at the JS↔C++ boundary. `validateCipherParams()` in `src/cipher.ts` rejects empty / non-string `cipherType` with `TypeError`, splits the existing `getCipherInfo()` probe into name-only / name+keyLen / name+ivLen calls so the thrown error names exactly which parameter is wrong, hard-codes (key=32, iv=24) for libsodium ciphers OpenSSL doesn't see (xsalsa20, xsalsa20-poly1305, xchacha20-poly1305), and rejects empty IV when the cipher requires one and non-empty IV when it doesn't. Wired into `Cipheriv` / `Decipheriv` constructors and the `xsalsa20()` shim. 11 regression tests covering empty/unknown name, too-short / too-long / empty key for AES-CBC, wrong IV for CBC and CCM, accepted variable IV for GCM, decipher mirror, and wrong-key + wrong-nonce for xsalsa20. (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [3.2] KDF parameter validation at the TS layer. **Scrypt**: `validateScryptParams()` enforces RFC 7914 §6 — N power-of-2 > 1, r/p positive integers, r * p < 2^30, and 128 * r * N ≤ maxmem. **HKDF**: `validateHkdfKeylen()` enforces RFC 5869 §2.3 (L ≤ 255 * HashLen) using a static `HKDF_HASH_BYTES` table covering sha1/224/256/384/512, sha3-256/384/512, ripemd160. Wired into hkdf, hkdfSync, and the WebCrypto `hkdfDeriveBits`. **Argon2**: `validateArgon2Params()` enforces RFC 9106 §3.1 minimums — 1 ≤ p ≤ 2^24-1, T ≥ 4, m ≥ 8 * p (KiB), t ≥ 1, salt 8..2^32-1 bytes, version ∈ {0x10, 0x13}. Async paths surface the new errors via callback. Existing argon2 tests that asserted the C++ `validateUInt`-style messages are refreshed to match the new RFC 9106 wording (the JS-side check now fires first). 8 scrypt + 5 HKDF + 7 Argon2 RFC 9106 minimum-bound regressions. (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [3.3] RSA modulus minimum lifted from 256 → 2048 bits (NIST SP 800-131A Rev. 2; RFC 8017). `RSA_MIN_MODULUS_LENGTH` is shared between the WebCrypto (`rsa_generateKeyPair`) and Node-API (`rsa_prepareKeyGenParams`) entry points. WebCrypto path stays a `DOMException` so JOSE callers see the same exception type. Bumped 12 in-repo test fixtures from `modulusLength: 1024` → `2048` across `subtle/generateKey.ts` and `subtle/import_export.ts`, and added explicit modulusLength=1024 rejection coverage at both boundaries. (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [3.4] DSA modulus minimum lifted from `> 0` → 1024 bits (FIPS 186-4 §4.2 sanctions only L ∈ {1024, 2048, 3072}). 1024 retained as the floor (rather than 2048) so legacy interop callers have a fallback. The `Invalid or missing modulusLength` generic Error becomes a `RangeError: DSA modulusLength must be at least 1024 bits (got N)`. 2 regression tests (modulusLength 512 and 0). (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [3.5] WebCrypto `subtle` hardening on four under-enforced edges. (a) `normalizeAlgorithm` performs case-insensitive lookup against a lazy `SUPPORTED_ALGORITHMS` lower→canonical map, so `'aes-gcm'` → `'AES-GCM'` instead of bypassing the supported-set comparisons. (b) New `validateJwkExtAndKeyOps()` helper rejects `jwk.ext === false` with `extractable === true` and rejects when `jwk.key_ops` is present but does not cover every requested usage; wired into KMAC, RSA, HMAC, AES, and Ed/CFRG JWK import branches. (c) `subtle.deriveBits` now strictly requires the literal `deriveBits` usage (was `deriveBits || deriveKey`), per spec step 11. (d) `hkdfImportKey` throws `SyntaxError` when `extractable: true` is requested and forces `extractable: false` on the resulting `CryptoKey`, matching §28.7.6. 6 regression tests (lowercase digest, deriveBits-without-deriveBits, HKDF non-extractable enforcement + force-false invariant, AES JWK ext/key_ops triad). (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [3.6] Stream `_transform` / `_flush` error propagation in Hash, Hmac, and Cipher. Each wrapped body is now a try/catch that forwards the thrown `Error` through the stream callback so it emits as a regular `'error'` event and the Transform always sees the callback exactly once on every code path. Callback parameter type widened from `() => void` to `(err?: Error | null) => void`. 6 regression tests covering Hash/Hmac update/digest after digest() and Cipher update after final() / Decipher final with a tampered AES-GCM tag. (branch: `feat/security-audit-phase-3`, PR: TBD) From e980749771255f73a0db5c6e4c325540551f02f4 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 10:50:35 -0400 Subject: [PATCH 09/10] fix(cipher): align libsodium error wording, fix DES-EDE order, query getCipherInfo for IV length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-on tweaks to Phase 3.1 cipher param validation that landed together but post-dated the audit commit: * libsodium error messages — change "Invalid iv length" → "Invalid nonce length" and "(expected N)" → "(nonce must be N bytes)" / "(key must be N bytes)" so the wording matches the libsodium parlance that callers expect for xsalsa20 / xsalsa20-poly1305 / xchacha20-poly1305. Test regex is updated in lockstep. * allCiphers loop key-length detection — add an explicit `DES-EDE3` branch ahead of `DES-EDE`, since `'DES-EDE3-CBC'.includes('DES-EDE')` is also true and the loop was silently picking the 16-byte 2-key 3DES path for 24-byte 3-key 3DES ciphers. * allCiphers loop IV-length detection — replace the hard-coded iv12 / iv16 split (only correct for AEAD vs everything else) with a per-cipher `getCipherInfo(name)?.ivLength` query, so the loop covers ECB (0), DES-family (8), AES (16), etc. without further hard-coding. Also fix the `'strings'` test, which was passing `key16.toString('hex')` (32 UTF-8 bytes) into `aes-128-cbc` — fine before validation, but now rejected by the new key-length check. Use 16-char ASCII so the byte length matches the cipher's (16, 16) sizes. --- example/src/tests/cipher/cipher_tests.ts | 43 ++++++++++++++----- .../react-native-quick-crypto/src/cipher.ts | 9 ++-- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/example/src/tests/cipher/cipher_tests.ts b/example/src/tests/cipher/cipher_tests.ts index 795ef01c6..cbb0a02ad 100644 --- a/example/src/tests/cipher/cipher_tests.ts +++ b/example/src/tests/cipher/cipher_tests.ts @@ -1,5 +1,6 @@ import { Buffer, + getCipherInfo, getCiphers, createCipheriv, createDecipheriv, @@ -38,11 +39,12 @@ test(SUITE, 'invalid algorithm', () => { }); test(SUITE, 'strings', () => { - // roundtrip expects Buffers, convert strings first + // Strings are interpreted as UTF-8 bytes by createCipheriv, so use + // 16-char ASCII so the byte-length matches aes-128-cbc's (16, 16) sizes. roundTrip( 'aes-128-cbc', - key16.toString('hex'), - iv.toString('hex'), + 'YELLOW SUBMARINE', + '0123456789ABCDEF', plaintextBuffer, ); }); @@ -69,12 +71,20 @@ const allCiphers = getCiphers().filter( allCiphers.forEach(cipherName => { test(SUITE, cipherName, () => { try { - // Determine correct key length - let keyLen = 32; // Default to 256-bit - if (cipherName.includes('128')) { + // Determine correct key length. Order matters: DES-EDE3 must be + // checked before DES-EDE because `'DES-EDE3-CBC'.includes('DES-EDE')` + // is also true. AES / ARIA / CAMELLIA carry their key size in the name. + let keyLen: number; + if (cipherName.includes('DES-EDE3')) { + keyLen = 24; // 3-key 3DES + } else if (cipherName.includes('DES-EDE')) { + keyLen = 16; // 2-key 3DES + } else if (cipherName.includes('128')) { keyLen = 16; } else if (cipherName.includes('192')) { keyLen = 24; + } else { + keyLen = 32; // Default to 256-bit } let testKey: Uint8Array; if (cipherName.includes('XTS')) { @@ -93,14 +103,25 @@ allCiphers.forEach(cipherName => { testKey = randomFillSync(new Uint8Array(keyLen)); } - // Select IV size based on mode - const testIv: Uint8Array = + // Select IV size. AEAD modes get the canonical 12-byte nonce; for + // every other cipher we ask the runtime what IV length the cipher + // expects (0 for ECB, 8 for DES-family 64-bit blocks, 16 for AES, + // …). This keeps the loop generic instead of hard-coding sizes. + let testIv: Uint8Array; + if ( cipherName.includes('GCM') || cipherName.includes('OCB') || cipherName.includes('CCM') || cipherName.includes('Poly1305') - ? iv12 - : iv16; + ) { + testIv = iv12; + } else { + const ivLen = getCipherInfo(cipherName)?.ivLength ?? 0; + testIv = + ivLen === 0 + ? new Uint8Array(0) + : randomFillSync(new Uint8Array(ivLen)); + } // Create key and iv as Buffers for the roundtrip functions const key = Buffer.from(testKey); @@ -620,7 +641,7 @@ test(SUITE, 'createCipheriv: rejects wrong xsalsa20 key length', () => { test(SUITE, 'createCipheriv: rejects wrong xsalsa20 nonce length', () => { expect(() => { createCipheriv('xsalsa20', key32, iv16); - }).to.throw(RangeError, /Invalid iv length 16 .* xsalsa20/); + }).to.throw(RangeError, /Invalid nonce length 16 .* xsalsa20/); }); // Phase 3.6 regression: stream _transform / _flush errors (e.g. AEAD diff --git a/packages/react-native-quick-crypto/src/cipher.ts b/packages/react-native-quick-crypto/src/cipher.ts index 34d4a92a5..a3eed9689 100644 --- a/packages/react-native-quick-crypto/src/cipher.ts +++ b/packages/react-native-quick-crypto/src/cipher.ts @@ -91,16 +91,19 @@ function validateCipherParams( const lower = cipherType.toLowerCase(); const sodium = LIBSODIUM_CIPHER_PARAMS[lower]; if (sodium) { + // libsodium parlance: "nonce" rather than "iv". Phrase the expected + // size as a natural-language clause so callers asserting on either + // `key must be N bytes` or `Invalid key length N` both match. if (keyByteLength !== sodium.keyLength) { throw new RangeError( `Invalid key length ${keyByteLength} for cipher ${cipherType} ` + - `(expected ${sodium.keyLength})`, + `(key must be ${sodium.keyLength} bytes)`, ); } if (ivByteLength !== sodium.ivLength) { throw new RangeError( - `Invalid iv length ${ivByteLength} for cipher ${cipherType} ` + - `(expected ${sodium.ivLength})`, + `Invalid nonce length ${ivByteLength} for cipher ${cipherType} ` + + `(nonce must be ${sodium.ivLength} bytes)`, ); } return; From 5c54492e3c59ae47818d8c0138f3a1aac9d44b55 Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Mon, 27 Apr 2026 10:57:31 -0400 Subject: [PATCH 10/10] =?UTF-8?q?refactor(security-audit):=20Phase=203=20r?= =?UTF-8?q?eview=20polish=20=E2=80=94=20cipher=20fast-path,=20HKDF=20unkno?= =?UTF-8?q?wn-digest=20throw,=20type=20tightening,=20idiomatic=20stream=20?= =?UTF-8?q?tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six follow-ups from the Phase 3 self-review: * **(2) cipher fast-path.** `validateCipherParams` now caches the `getCipherInfo(name)` result and short-circuits the common case where `(key, iv)` match the cipher's default lengths exactly (AES-CBC, ECB, AES-GCM with the canonical 12-byte IV, etc.). Native round-trip count drops from 3 → 1 on the happy path while still producing the per-parameter error message on failure. * **(3) drop unreachable Number.isFinite check.** `ArrayBuffer.byteLength` is always a non-negative integer; the only meaningful guard is `keyByteLength === 0`. * **(4) HKDF unsupported-digest throw.** `validateHkdfKeylen` previously silently skipped the RFC 5869 ceiling check for digests not in the `HKDF_HASH_BYTES` table — including XOFs like SHAKE128/256, which are invalid HKDF inputs (HKDF builds on HMAC, which requires a fixed-length hash). Throw `TypeError: Unsupported HKDF digest: …` instead, with a regression test for `shake128`. * **(5) Argon2 nonce single-resolution.** `validateArgon2Params` now returns the resolved `nonceAB: ArrayBuffer` and the caller passes it straight to native, dropping the second `binaryLikeToArrayBuffer` round-trip on `params.nonce`. * **(6) canonical-name map type.** Type the lazy `_canonicalAlgorithmNames` map as `Map` and do the cast once at insertion (where `SUPPORTED_ALGORITHMS` is the contract source) rather than on every `normalizeAlgorithm` lookup. All `as AnyAlgorithm` casts in `normalizeAlgorithm` go away. * **(7) idiomatic stream tests.** Re-express the Phase 3.6 Hash / Hmac / Cipher / Decipher stream-error regressions through the public stream API: drive `_transform` via `stream.write()` and `_flush` via `stream.end()`, then `await` an `'error'` event. Removes every `(stream as any)._transform / _flush(...)` cast and the matching `eslint-disable @typescript-eslint/no-explicit-any` annotations. --- example/src/tests/cipher/cipher_tests.ts | 85 +++++++++---------- example/src/tests/hash/hash_tests.ts | 42 ++++----- example/src/tests/hkdf/hkdf_tests.ts | 11 +++ example/src/tests/hmac/hmac_tests.ts | 38 ++++----- .../react-native-quick-crypto/src/argon2.ts | 23 +++-- .../react-native-quick-crypto/src/cipher.ts | 56 +++++++----- .../react-native-quick-crypto/src/hkdf.ts | 13 ++- .../react-native-quick-crypto/src/subtle.ts | 18 ++-- plans/todo/security-audit.md | 1 + 9 files changed, 158 insertions(+), 129 deletions(-) diff --git a/example/src/tests/cipher/cipher_tests.ts b/example/src/tests/cipher/cipher_tests.ts index cbb0a02ad..8941ab0f8 100644 --- a/example/src/tests/cipher/cipher_tests.ts +++ b/example/src/tests/cipher/cipher_tests.ts @@ -645,53 +645,52 @@ test(SUITE, 'createCipheriv: rejects wrong xsalsa20 nonce length', () => { }); // Phase 3.6 regression: stream _transform / _flush errors (e.g. AEAD -// auth-tag mismatch on Decipher.final()) must flow through the -// callback so they emit as 'error' events on the stream. +// auth-tag mismatch on Decipher.final()) must surface as 'error' events +// rather than throwing through the Transform plumbing. Drive each path +// through the public stream API. -test(SUITE, 'Decipher._flush: surfaces auth-tag mismatch via callback', () => { - // Encrypt to obtain a valid (key, iv, tag) triple, then tamper with the - // auth tag so Decipher.final() rejects authentication. - const testKey = Buffer.from(randomFillSync(new Uint8Array(32))); - const testIv = randomFillSync(new Uint8Array(12)); - const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv)); - cipher.setAAD(aad); - const encrypted = Buffer.concat([ - cipher.update(plaintextBuffer), - cipher.final(), - ]); - const tag = cipher.getAuthTag(); - tag[0] = tag[0]! ^ 0xff; +test( + SUITE, + 'Decipher: auth-tag mismatch surfaces as "error" event', + async () => { + // Encrypt to obtain a valid (key, iv, tag) triple, then tamper with the + // auth tag so Decipher.final() rejects authentication. + const testKey = Buffer.from(randomFillSync(new Uint8Array(32))); + const testIv = randomFillSync(new Uint8Array(12)); + const cipher = createCipheriv('aes-256-gcm', testKey, Buffer.from(testIv)); + cipher.setAAD(aad); + const encrypted = Buffer.concat([ + cipher.update(plaintextBuffer), + cipher.final(), + ]); + const tag = cipher.getAuthTag(); + tag[0] = tag[0]! ^ 0xff; - const decipher = createDecipheriv( - 'aes-256-gcm', - testKey, - Buffer.from(testIv), - ); - decipher.setAAD(aad); - decipher.setAuthTag(tag); - decipher.update(encrypted); + const decipher = createDecipheriv( + 'aes-256-gcm', + testKey, + Buffer.from(testIv), + ); + decipher.setAAD(aad); + decipher.setAuthTag(tag); + decipher.update(encrypted); - let received: Error | null | undefined = undefined; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (decipher as any)._flush((err: Error | null) => { - received = err; - }); - expect(received).to.be.instanceOf(Error); -}); + const error = await new Promise(resolve => { + decipher.once('error', resolve); + decipher.end(); // triggers _flush → final() → tag mismatch + }); + expect(error).to.be.instanceOf(Error); + }, +); -test(SUITE, 'Cipher._transform: surfaces update() error via callback', () => { +test(SUITE, 'Cipher: _transform error surfaces as "error" event', async () => { const cipher = createCipheriv('aes-128-cbc', key16, iv16); cipher.update(plaintextBuffer); - cipher.final(); // post-final — next update() throws - - let received: Error | null | undefined = undefined; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (cipher as any)._transform( - Buffer.from('after final'), - 'utf8', - (err: Error | null) => { - received = err; - }, - ); - expect(received).to.be.instanceOf(Error); + cipher.final(); // finalize — next update() throws + + const error = await new Promise(resolve => { + cipher.once('error', resolve); + cipher.write('after final'); + }); + expect(error).to.be.instanceOf(Error); }); diff --git a/example/src/tests/hash/hash_tests.ts b/example/src/tests/hash/hash_tests.ts index 7eb1d7a28..357c38775 100644 --- a/example/src/tests/hash/hash_tests.ts +++ b/example/src/tests/hash/hash_tests.ts @@ -323,35 +323,29 @@ test(SUITE, 'hash() oneshot - Buffer input', () => { }); // Phase 3.6 regression: synchronous failures inside `_transform` and -// `_flush` must surface via the stream callback (i.e. become 'error' -// events) rather than throwing out of the Transform plumbing — which -// can leave the stream in a half-written state and crash the host -// pipeline. +// `_flush` must surface as stream 'error' events rather than throwing +// out of the Transform plumbing — which can leave the stream in a +// half-written state and crash the host pipeline. Drive each path +// through the public stream API (write/end) and assert on 'error'. -test(SUITE, 'Hash._transform: surfaces update() error via callback', () => { +test(SUITE, 'Hash: _transform error surfaces as "error" event', async () => { const h = createHash('sha256'); - h.digest(); // put the native context in finalized state — next update() throws - - let received: Error | null | undefined = undefined; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (h as any)._transform( - Buffer.from('after digest'), - 'utf8', - (err: Error | null) => { - received = err; - }, - ); - expect(received).to.be.instanceOf(Error); + h.digest(); // finalize the native context — next update() throws + + const error = await new Promise(resolve => { + h.once('error', resolve); + h.write('after digest'); + }); + expect(error).to.be.instanceOf(Error); }); -test(SUITE, 'Hash._flush: surfaces digest() error via callback', () => { +test(SUITE, 'Hash: _flush error surfaces as "error" event', async () => { const h = createHash('sha256'); - h.digest(); // first digest — second call throws + h.digest(); // first digest — second call (from _flush) throws - let received: Error | null | undefined = undefined; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (h as any)._flush((err: Error | null) => { - received = err; + const error = await new Promise(resolve => { + h.once('error', resolve); + h.end(); }); - expect(received).to.be.instanceOf(Error); + expect(error).to.be.instanceOf(Error); }); diff --git a/example/src/tests/hkdf/hkdf_tests.ts b/example/src/tests/hkdf/hkdf_tests.ts index 8f96ceb1d..b4c3ee59b 100644 --- a/example/src/tests/hkdf/hkdf_tests.ts +++ b/example/src/tests/hkdf/hkdf_tests.ts @@ -115,6 +115,17 @@ test(SUITE, 'hkdfSync: accepts keylen at the RFC 5869 ceiling', () => { }).to.not.throw(); }); +test(SUITE, 'hkdfSync: rejects unsupported digest (shake128)', () => { + const ikm = Buffer.from('00', 'hex'); + // SHAKE is an extendable-output function, not a fixed-length hash, so it + // is not a valid HKDF digest (HKDF builds on HMAC, which requires a + // fixed-length hash). The validator surfaces this as a TypeError before + // the call reaches OpenSSL. + expect(() => { + hkdfSync('shake128', ikm, Buffer.alloc(0), Buffer.alloc(0), 32); + }).to.throw(TypeError, /Unsupported HKDF digest/); +}); + test(SUITE, 'hkdf: surfaces ceiling errors via callback', async () => { const ikm = Buffer.from('00', 'hex'); await new Promise((resolve, reject) => { diff --git a/example/src/tests/hmac/hmac_tests.ts b/example/src/tests/hmac/hmac_tests.ts index 2c651d94f..65df870d9 100644 --- a/example/src/tests/hmac/hmac_tests.ts +++ b/example/src/tests/hmac/hmac_tests.ts @@ -519,34 +519,28 @@ test(SUITE, 'digest with ucs2 encoding', () => { ); }); -// Phase 3.6 regression: stream _transform / _flush errors must flow -// through the callback so they emit as 'error' events rather than -// throwing through the Transform plumbing. +// Phase 3.6 regression: stream _transform / _flush errors must surface +// as 'error' events rather than throwing through the Transform +// plumbing. Drive each path through the public stream API. -test(SUITE, 'Hmac._transform: surfaces update() error via callback', () => { +test(SUITE, 'Hmac: _transform error surfaces as "error" event', async () => { const h = createHmac('sha256', 'k'); - h.digest(); // post-digest state — next update() throws + h.digest(); // finalize — next update() throws - let received: Error | null | undefined = undefined; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (h as any)._transform( - Buffer.from('after digest'), - 'utf8', - (err: Error | null) => { - received = err; - }, - ); - expect(received).to.be.instanceOf(Error); + const error = await new Promise(resolve => { + h.once('error', resolve); + h.write('after digest'); + }); + expect(error).to.be.instanceOf(Error); }); -test(SUITE, 'Hmac._flush: surfaces digest() error via callback', () => { +test(SUITE, 'Hmac: _flush error surfaces as "error" event', async () => { const h = createHmac('sha256', 'k'); - h.digest(); + h.digest(); // first digest — second call (from _flush) throws - let received: Error | null | undefined = undefined; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (h as any)._flush((err: Error | null) => { - received = err; + const error = await new Promise(resolve => { + h.once('error', resolve); + h.end(); }); - expect(received).to.be.instanceOf(Error); + expect(error).to.be.instanceOf(Error); }); diff --git a/packages/react-native-quick-crypto/src/argon2.ts b/packages/react-native-quick-crypto/src/argon2.ts index deb7d6d45..66c64fa2e 100644 --- a/packages/react-native-quick-crypto/src/argon2.ts +++ b/packages/react-native-quick-crypto/src/argon2.ts @@ -56,7 +56,12 @@ function validateAlgorithm(algorithm: string): void { } } -function validateArgon2Params(params: Argon2Params, version: number): void { +// Returns the resolved nonce ArrayBuffer so the caller can pass it +// straight to native without re-resolving `params.nonce`. +function validateArgon2Params( + params: Argon2Params, + version: number, +): ArrayBuffer { if (!isUInt(params.parallelism, ARGON2_MAX_U24) || params.parallelism < 1) { throw new RangeError( `Invalid Argon2 parallelism: ${params.parallelism} ` + @@ -91,13 +96,14 @@ function validateArgon2Params(params: Argon2Params, version: number): void { // Salt (nonce) must be 8..2^32 - 1 bytes — measured against the resolved // ArrayBuffer because BinaryLike accepts strings whose UTF-8 length is // what actually reaches OpenSSL. - const nonceLen = binaryLikeToArrayBuffer(params.nonce).byteLength; - if (nonceLen < 8 || nonceLen > ARGON2_MAX_U32) { + const nonceAB = binaryLikeToArrayBuffer(params.nonce); + if (nonceAB.byteLength < 8 || nonceAB.byteLength > ARGON2_MAX_U32) { throw new RangeError( - `Invalid Argon2 nonce length: ${nonceLen} bytes ` + + `Invalid Argon2 nonce length: ${nonceAB.byteLength} bytes ` + `(RFC 9106: 8 ≤ |s| ≤ 2^32 - 1)`, ); } + return nonceAB; } function toAB(value: BinaryLike): ArrayBuffer { @@ -107,11 +113,11 @@ function toAB(value: BinaryLike): ArrayBuffer { export function argon2Sync(algorithm: string, params: Argon2Params): Buffer { validateAlgorithm(algorithm); const version = params.version ?? ARGON2_VERSION; - validateArgon2Params(params, version); + const nonceAB = validateArgon2Params(params, version); const result = getNative().hashSync( algorithm, toAB(params.message), - toAB(params.nonce), + nonceAB, params.parallelism, params.tagLength, params.memory, @@ -130,8 +136,9 @@ export function argon2( ): void { validateAlgorithm(algorithm); const version = params.version ?? ARGON2_VERSION; + let nonceAB: ArrayBuffer; try { - validateArgon2Params(params, version); + nonceAB = validateArgon2Params(params, version); } catch (err) { callback(err as Error, Buffer.alloc(0)); return; @@ -140,7 +147,7 @@ export function argon2( .hash( algorithm, toAB(params.message), - toAB(params.nonce), + nonceAB, params.parallelism, params.tagLength, params.memory, diff --git a/packages/react-native-quick-crypto/src/cipher.ts b/packages/react-native-quick-crypto/src/cipher.ts index a3eed9689..8d7a8469d 100644 --- a/packages/react-native-quick-crypto/src/cipher.ts +++ b/packages/react-native-quick-crypto/src/cipher.ts @@ -84,7 +84,10 @@ function validateCipherParams( if (typeof cipherType !== 'string' || cipherType.length === 0) { throw new TypeError('cipher algorithm must be a non-empty string'); } - if (!Number.isFinite(keyByteLength) || keyByteLength === 0) { + // ArrayBuffer.byteLength is always a non-negative integer, so the only + // out-of-range value we need to guard is 0 — empty key buffers must not + // reach OpenSSL's EVP_CipherInit_ex. + if (keyByteLength === 0) { throw new RangeError(`Invalid key length 0 for cipher ${cipherType}`); } @@ -109,13 +112,35 @@ function validateCipherParams( return; } - // OpenSSL path: getCipherInfo(name, keyLen, ivLen) returns undefined when - // the requested lengths are not accepted by the cipher. We split the call - // into three checks so the thrown error can name which parameter is wrong. + // OpenSSL path. Look up the cipher's defaults once. Most callers pass + // exactly the cipher's default key/iv lengths (e.g. AES-128-CBC always + // wants 16/16) — short-circuit those to a single native round-trip. + // Variable-length ciphers (GCM, CCM, OCB, ChaCha20-Poly1305) fall through + // to per-parameter validation calls so the error message can name which + // of {key, iv} is wrong. const info = CipherUtils.getCipherInfo(cipherType); if (info === undefined) { throw new TypeError(`Unsupported or unknown cipher type: ${cipherType}`); } + + const expectedIv = info.ivLength ?? 0; + if (expectedIv === 0 && ivByteLength > 0) { + throw new RangeError( + `Cipher ${cipherType} does not use an iv (got ${ivByteLength} bytes)`, + ); + } + if (expectedIv > 0 && ivByteLength === 0) { + throw new RangeError( + `Cipher ${cipherType} requires an iv but none was provided`, + ); + } + + // Fast path: lengths match the cipher's defaults exactly. + if (info.keyLength === keyByteLength && expectedIv === ivByteLength) { + return; + } + + // Variable-length: verify against native one parameter at a time. if ( CipherUtils.getCipherInfo(cipherType, keyByteLength, undefined) === undefined @@ -124,25 +149,12 @@ function validateCipherParams( `Invalid key length ${keyByteLength} for cipher ${cipherType}`, ); } - - const expectedIv = info.ivLength ?? 0; - if (expectedIv > 0) { - if (ivByteLength === 0) { - throw new RangeError( - `Cipher ${cipherType} requires an iv but none was provided`, - ); - } - if ( - CipherUtils.getCipherInfo(cipherType, undefined, ivByteLength) === - undefined - ) { - throw new RangeError( - `Invalid iv length ${ivByteLength} for cipher ${cipherType}`, - ); - } - } else if (ivByteLength > 0) { + if ( + expectedIv > 0 && + CipherUtils.getCipherInfo(cipherType, undefined, ivByteLength) === undefined + ) { throw new RangeError( - `Cipher ${cipherType} does not use an iv (got ${ivByteLength} bytes)`, + `Invalid iv length ${ivByteLength} for cipher ${cipherType}`, ); } } diff --git a/packages/react-native-quick-crypto/src/hkdf.ts b/packages/react-native-quick-crypto/src/hkdf.ts index ba9736f72..5f12ae9bb 100644 --- a/packages/react-native-quick-crypto/src/hkdf.ts +++ b/packages/react-native-quick-crypto/src/hkdf.ts @@ -45,8 +45,11 @@ function sanitizeInput(input: BinaryLike, name: string): ArrayBuffer { } } -// Output byte-length of each digest the C++ Hkdf supports. Used to enforce -// the RFC 5869 §2.3 ceiling: L ≤ 255 * HashLen. +// Output byte-length of each fixed-length digest. HKDF requires a fixed- +// output hash (it builds on HMAC), so XOFs like SHAKE128/256 are not +// included even though `normalizeHashName` will accept them — passing +// SHAKE here is a caller bug we surface as `Unsupported HKDF digest` +// instead of letting the native side return an opaque error. const HKDF_HASH_BYTES: Readonly> = { sha1: 20, sha224: 28, @@ -70,7 +73,11 @@ function validateHkdfKeylen(digest: string, keylen: number): void { throw new TypeError('Bad key length'); } const hashLen = HKDF_HASH_BYTES[digest.toLowerCase()]; - if (hashLen !== undefined && keylen > 255 * hashLen) { + if (hashLen === undefined) { + throw new TypeError(`Unsupported HKDF digest: ${digest}`); + } + // RFC 5869 §2.3: L ≤ 255 * HashLen. + if (keylen > 255 * hashLen) { throw new RangeError( `HKDF keylen ${keylen} exceeds RFC 5869 ceiling ` + `255 * HashLen (${255 * hashLen}) for ${digest}`, diff --git a/packages/react-native-quick-crypto/src/subtle.ts b/packages/react-native-quick-crypto/src/subtle.ts index 9b5cf041a..5aee89b5a 100644 --- a/packages/react-native-quick-crypto/src/subtle.ts +++ b/packages/react-native-quick-crypto/src/subtle.ts @@ -89,14 +89,19 @@ function hasAnyNotIn(usages: KeyUsage[], allowed: KeyUsage[]): boolean { // the registry of canonical names below can stay declared after the // function. Without this, callers who pass lowercase strings bypass the // downstream `SUPPORTED_ALGORITHMS` set comparisons silently. -let _canonicalAlgorithmNames: Map | null = null; -function getCanonicalAlgorithmNames(): Map { +// +// The map's value type is `AnyAlgorithm` so callers can use the lookup +// result directly without re-asserting. The `as AnyAlgorithm` at insertion +// is the single contract boundary: every name in `SUPPORTED_ALGORITHMS` is +// already a member of `AnyAlgorithm` by construction. +let _canonicalAlgorithmNames: Map | null = null; +function getCanonicalAlgorithmNames(): Map { if (_canonicalAlgorithmNames === null) { - const map = new Map(); + const map = new Map(); for (const set of Object.values(SUPPORTED_ALGORITHMS)) { if (!set) continue; for (const name of set) { - map.set(name.toLowerCase(), name); + map.set(name.toLowerCase(), name as AnyAlgorithm); } } _canonicalAlgorithmNames = map; @@ -110,12 +115,11 @@ function normalizeAlgorithm( ): SubtleAlgorithm { const map = getCanonicalAlgorithmNames(); if (typeof algorithm === 'string') { - const canonical = map.get(algorithm.toLowerCase()) ?? algorithm; - return { name: canonical as AnyAlgorithm }; + return { name: map.get(algorithm.toLowerCase()) ?? algorithm }; } if (typeof algorithm.name === 'string') { const canonical = map.get(algorithm.name.toLowerCase()) ?? algorithm.name; - return { ...algorithm, name: canonical as AnyAlgorithm } as SubtleAlgorithm; + return { ...algorithm, name: canonical }; } return algorithm as SubtleAlgorithm; } diff --git a/plans/todo/security-audit.md b/plans/todo/security-audit.md index 8eb9ccc93..a743700b9 100644 --- a/plans/todo/security-audit.md +++ b/plans/todo/security-audit.md @@ -1272,3 +1272,4 @@ _Append entries as PRs land. Format: `YYYY-MM-DD — [phase.task] description (P - 2026-04-27 — [3.4] DSA modulus minimum lifted from `> 0` → 1024 bits (FIPS 186-4 §4.2 sanctions only L ∈ {1024, 2048, 3072}). 1024 retained as the floor (rather than 2048) so legacy interop callers have a fallback. The `Invalid or missing modulusLength` generic Error becomes a `RangeError: DSA modulusLength must be at least 1024 bits (got N)`. 2 regression tests (modulusLength 512 and 0). (branch: `feat/security-audit-phase-3`, PR: TBD) - 2026-04-27 — [3.5] WebCrypto `subtle` hardening on four under-enforced edges. (a) `normalizeAlgorithm` performs case-insensitive lookup against a lazy `SUPPORTED_ALGORITHMS` lower→canonical map, so `'aes-gcm'` → `'AES-GCM'` instead of bypassing the supported-set comparisons. (b) New `validateJwkExtAndKeyOps()` helper rejects `jwk.ext === false` with `extractable === true` and rejects when `jwk.key_ops` is present but does not cover every requested usage; wired into KMAC, RSA, HMAC, AES, and Ed/CFRG JWK import branches. (c) `subtle.deriveBits` now strictly requires the literal `deriveBits` usage (was `deriveBits || deriveKey`), per spec step 11. (d) `hkdfImportKey` throws `SyntaxError` when `extractable: true` is requested and forces `extractable: false` on the resulting `CryptoKey`, matching §28.7.6. 6 regression tests (lowercase digest, deriveBits-without-deriveBits, HKDF non-extractable enforcement + force-false invariant, AES JWK ext/key_ops triad). (branch: `feat/security-audit-phase-3`, PR: TBD) - 2026-04-27 — [3.6] Stream `_transform` / `_flush` error propagation in Hash, Hmac, and Cipher. Each wrapped body is now a try/catch that forwards the thrown `Error` through the stream callback so it emits as a regular `'error'` event and the Transform always sees the callback exactly once on every code path. Callback parameter type widened from `() => void` to `(err?: Error | null) => void`. 6 regression tests covering Hash/Hmac update/digest after digest() and Cipher update after final() / Decipher final with a tampered AES-GCM tag. (branch: `feat/security-audit-phase-3`, PR: TBD) +- 2026-04-27 — [Phase 3 review polish] Address review follow-ups across the Phase 3 work. **(2)** `validateCipherParams` fast-path: cache the `getCipherInfo(name)` result and short-circuit when `(key, iv)` match the cipher's defaults, dropping the round-trip count from 3→1 for the common case (AES-CBC, AES-GCM with default 12-byte IV, ECB, etc.) while preserving the per-parameter error messages on the failure path. **(3)** Drop the `Number.isFinite(keyByteLength)` clause — `ArrayBuffer.byteLength` is always a non-negative integer, so the only meaningful guard is `=== 0`. **(4)** `validateHkdfKeylen` now throws `TypeError: Unsupported HKDF digest: ` for digests not in `HKDF_HASH_BYTES` (e.g. SHAKE128 — XOFs aren't valid HKDF inputs since HKDF builds on HMAC), instead of silently skipping the ceiling check. **(5)** `validateArgon2Params` returns the resolved `nonceAB` so both the validator and the native call share a single `binaryLikeToArrayBuffer(params.nonce)` round-trip. **(6)** Type the lazy canonical-name map as `Map` so the cast lives at insertion (where the contract is enforced by `SUPPORTED_ALGORITHMS`) rather than at every `normalizeAlgorithm` lookup. **(7)** Re-express the Phase 3.6 stream tests through the public stream API (`h.write()` / `h.end()`) and assert on `'error'` events, removing the `(stream as any)._transform/_flush(...)` casts. Adds 1 new HKDF regression for the unknown-digest throw. (branch: `feat/security-audit-phase-3`, PR: TBD)