diff --git a/CHANGELOG.md b/CHANGELOG.md index de269b0..7b7671a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +- Security pass over the legacy Kyber code (issue #18): the random bytes for `Kyber*` key generation and encapsulation now come from the operating system generator (`OsRng`) instead of `thread_rng()`, and a failure of that generator is raised as `RandomnessError` (a `KyberError`) instead of panicking or being retried with a weaker source. Seeded paths and all Kyber output are unchanged. +- Security pass over the legacy Kyber code (issue #18), continued: optimization barriers in `poly_frommsg`, `verify` and `cmov`; decapsulation and the PRF noise buffers are wiped on every exit; secrets returned to Python are wiped on the Rust side; the Rust keypair and encapsulation functions check buffer lengths; `rand` is 0.8.6 in `Cargo.lock`. The findings, the CVE and paper table and the two decisions not to fix (Compress_11, Keccak stack spills) are in `SECURITY.md`. Kyber output is unchanged. +- Behavior change, `Kyber512/768/1024.from_keys` (issue #18): it now raises `KyberError` when the public key is not the copy stored inside the secret key (a round-3 Kyber secret key is `sk_pke || pk || H(pk) || z`). Before, any public key and secret key of the right sizes were accepted, so `Kyber768.from_keys(a.public_key, b.secret_key)` built a keypair whose `decapsulate` silently returned a different secret than `encapsulate` had produced. A key pair that matches is accepted as before, and a wrong-size key keeps its existing size error. `ML_KEM_*.from_keys` is not changed. + ## v2.0.0 Planned release; not tagged or published yet. Adds ML-KEM (FIPS 203) beside the legacy Kyber classes, which are deprecated. Kyber output is unchanged from 1.0.5. Kyber (round 3) and ML-KEM are not interoperable, so both sides of a connection must switch; see `docs/migration.md`. diff --git a/Cargo.lock b/Cargo.lock index efdc975..721c1b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -359,9 +359,9 @@ dependencies = [ [[package]] name = "rand" -version = "0.8.5" +version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +checksum = "5ca0ecfa931c29007047d1bc58e623ab12e5590e8c7cc53200d5202b69266d8a" dependencies = [ "libc", "rand_chacha", diff --git a/SECURITY.md b/SECURITY.md index 5b82b44..e694f23 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -109,3 +109,205 @@ The `keccak_squeeze` function used `store_u64` blindly, which could write past t The 7-layer Number Theoretic Transform (NTT) used `i16` intermediate values. While mathematically bounded, it was close to the limit. - **Resolution:** Hardened NTT butterfly logic using explicit `i32` arithmetic and updated `barrett_reduce` to handle a wider input range. - **Status:** FIXED. + +# Legacy Kyber security pass (issue #18, WP-9) + +Audit and fixes for the round-3 `Kyber*` code, done on 2026-10-30 on branch +`wp9-legacy-security`. The result is evidence, not a proof, and this was done +by the project's own tooling, not by a third party. + +Method and limits. Assembly was read from release builds (`opt-level = 3`, +one codegen unit) of rustc 1.95.0 with `llvm-objdump`, for x86_64 and aarch64 +(and i686 and armv7 for the division scan). One compiler version, one flag set +and a disassembly show what that compiler did; they do not show that another +compiler, flag set or CPU will do the same, and nothing in this repo guarantees +constant time. The barriers below (`core::hint::black_box`) are best effort. +There is no dynamic timing test for the legacy code (that is WP-8, not merged). +Memory hygiene is limited to what safe Rust can do: moves and register spills +leave copies that cannot be wiped without `unsafe`, and every secret that +crosses into Python becomes a Python `bytes` object that cannot be wiped. + +No finding is High or above. + +## 7. `poly_frommsg` mask (SEC-001, Low, hardened) + +`poly_frommsg` builds `-(bit)` from a message bit, the pattern behind +CVE-2024-37880 ("Clangover"). In the release builds checked here (rustc 1.95, +x86_64 and aarch64) it was vectorized with no branch on message bits, so the +leak was not observed. The bit and the mask now go through `black_box`. +Disassembly after the change: the only branches are the bounds check and the +loop counter. Output is unchanged. Status: HARDENED. + +## 8. `verify` and `cmov` (SEC-002, Info, hardened) + +Before the change the compare ended in `setg` and the select was a vector +`and/andn/or`, with no branch on the result. The accumulated difference, the +flag and the mask now go through `black_box`, as `mlkem::ct` does. Disassembly +after the change shows `setne`/`cset` and a vector select; in `crypto_kem_dec` +the flag is stored, reloaded and passed to the select with no branch on it. +Status: HARDENED. + +## 9. Randomness source (SEC-003, Low, fixed) + +Legacy key generation and encapsulation used `rand::thread_rng()`, and the KEM +functions called `fill_bytes`, which panics when the generator fails. They now +use `OsRng` and `try_fill_bytes`, return `KyberError::RandomBytesGeneration`, +and Python raises `RandomnessError`. No fallback source exists. The half-built +secret key is wiped if the second draw fails. Checked by Rust tests with a +generator that fails, and end to end with `strace -e inject=getrandom:error=EIO` +(all six Python entry points raised `RandomnessError`, no panic). Status: FIXED. + +## 10. Decapsulation intermediates (SEC-004, Low, fixed) + +`m'`, the KDF input and the re-encrypted ciphertext were wiped only on the +normal exit path, the reject flag was never wiped, and the PRF output buffers in +`poly_getnoise_*` (the secret noise: the key in key generation, `r` in +encryption) were never wiped. They are now `Zeroizing`, so they are wiped on +unwinding too. The release disassembly shows the flag and buffer wipes on both +the normal and the cleanup path. There is no runtime test of the wipe (it needs +`unsafe` or an allocator hook); the implicit-rejection output is covered by a +new test. Status: FIXED, with the limits stated above. + +## 11. `from_keys` binding (SEC-005, Low, fixed, behavior change) + +`Kyber512/768/1024.from_keys` accepted any public key and secret key of the +right sizes, so a pair from two key pairs decapsulated to a different secret +with no error (the class of bug in CVE-2026-115408 for another library). It now +raises `KyberError` when the public key is not the copy in the secret key +(`sk = sk_pke || pk || H(pk) || z`). `H(pk)` and `z` are not checked. +`ML_KEM_*.from_keys` has the same gap and is not changed here. Status: FIXED +for `Kyber*`. + +## 12. Secrets returned to Python (SEC-006, Low, fixed) + +The bindings returned secret keys and shared secrets as `Vec`, freed with +the secret still in the allocation after PyO3 copied it into `bytes`. They are +now wiped after the copy. Status: FIXED. The Python `bytes` objects remain. + +## 13. Length checks in the Rust API (SEC-007) and `DummyRng` (SEC-008), Info, fixed + +`crypto_kem_keypair*` and `crypto_kem_enc*` indexed caller buffers and seeds +with no length check and panicked on a short one. The Python bindings size +every buffer, so no Python path reached this. They now return `InvalidInput`. +`DummyRng` (the stand-in on the seeded path, never asked for bytes) returned a +panic from `try_fill_bytes`; it now returns an error. `api::public` still +slices its argument and would panic on a short `sk`; no Python path calls it. +Status: FIXED, one Rust-only note left. + +## 14. Compress_11 differs from FIPS 203 (SEC-009, Info, not fixed) + +`polyvec_compress_1024` (`du = 11`) gives a result one too high at x = 1875 and +3125 compared with exact rounding (found in #28, pinned by +`legacy_compress_11_deviates_from_spec_at_two_residues`). It uses the d = 4/5/10 +multiplier 10321345 >> 35, which is not exact for d = 11. + +A fix was written and tested on 2026-10-31 and then reverted by the maintainer's +decision, to keep Kyber-1024 output compatible with v1.0.5. The replacement was +`(((x << 11) + 1664) * 645084) >> 31`, which matches exact rounding at every +residue (checked over all of Z_q). Its cost was: golden vector #6 and #7 +(Kyber-1024) in `tests/vectors/legacy-kyber-v1.0.5.json` changed one or more +ciphertext bytes, and by estimate about 46% of Kyber-1024 ciphertexts +(1 - (1 - 2/3329)^1024, assuming uniform coefficients; not measured) contain a +coefficient at one of the two residues, so a 1.x ciphertext would decapsulate to +the implicit-rejection secret across the change and the reverse. + +What stays correct: ML-KEM-1024 does not use this routine. `kpke` calls +`compress::poly_compress` with d = 11 and the exact multiplier 645084 >> 31, +which is tested against FIPS 203 (4.7) for every residue and negative +representative, by the ACVP and Wycheproof vectors, and by the differential test +against the `ml-kem` crate. Both ends of one legacy version use the same routine, +so Kyber-1024 round trips work; only interoperability with other round-3 +implementations (which use exact rounding) is affected at those two residues. +Status: NOT FIXED, by decision. Anyone who needs exact Compress_11 should use +`ML_KEM_1024`. + +## 15. Keccak stack spills (SEC-010, Low, not fixed) + +What the issue is. `keccakf1600_statepermute` runs the 24 rounds with the state +in registers and stack temporaries. Zeroizing the 25-word state array after the +hash (`s.zeroize()` in `fips202.rs`) clears the array, not the temporaries. The +temporaries depend on the hashed input: `m`, `z`, the PRF seed and the +decapsulation intermediates all pass through SHA3 or SHAKE. A reader of freed +stack memory (a core dump, a swapped page, a memory-disclosure bug, cold boot) +could find parts of them after the call returns. + +What was measured. Release build of this branch, rustc 1.95.0, `llvm-objdump` +on x86_64 (2026-10-31): one copy of the permutation, six callee-saved pushes, a +`subq $0x78, %rsp` frame and 22 `mov` stores to `(%rsp)`. The #28 figures +(0xf8-byte frame, about 70 spill stores) did not reproduce on this build, so the +size of the leak is not stable; its existence is. aarch64, i686 and armv7 were +not looked at. + +Why it is not fixed. +- Safe Rust cannot reach it. The temporaries exist only inside a compiler-chosen + frame that is dead after `ret`; clearing them needs a volatile write to + addresses the compiler picked (`unsafe`, forbidden here) or hand-written + assembly (also `unsafe`). +- The safe workaround, a stack-scrub call that writes a large zeroed local + through `black_box` after each hash, is a guess about where the old frame was. + Frame size and layout change with compiler version, target and inlining (the + 0x78 against 0xf8 above is an example), so the scrub would be too small on + some builds and hide the problem with no way to test it. It also costs a + memset per hash. I judged a mitigation that cannot be verified worse than a + documented limit. +- It is not the only copy. Registers spilled by callers, moves of secret arrays, + and the Python `bytes` objects holding keys and shared secrets all leave + copies that cannot be wiped either (see "Method and limits" above), so fixing this one would not + make the process memory clean. +- The permutation is shared with ML-KEM (`src/mlkem/mod.rs`); any change to it + belongs in its own reviewed change with new assembly evidence, not in a + security log commit. + +What would close it. A permutation written to keep the state in a fixed +`Zeroizing` buffer end to end (no temporaries the compiler can spill, which +cannot be guaranteed in safe Rust), or an `unsafe` stack scrub gated on a +maintainer decision to allow `unsafe` in one audited function. +Status: NOT FIXED, documented limit. + +## 16. Dependency advisories (SEC-011, SEC-012, Info) + +Queried OSV (which carries RustSec) on 2026-10-30 for the locked runtime +dependencies; `cargo audit` is not installed here, so this is not its output. +`rand` 0.8.5 has RUSTSEC-2026-0097 (needs the `log` feature, which is off): +`Cargo.lock` moved to 0.8.6. `pyo3` 0.27.2 has RUSTSEC-2026-0176 (`nth` on list +and tuple iterators) and RUSTSEC-2026-0177 (`PyCFunction::new_closure`), both +fixed only in 0.29.0; this code uses neither API, and the 0.29 upgrade is left +to the maintainer. `zeroize`, `rand_core`, `getrandom`, `rand_chacha`: none. + +## Checked, no findings + +- Panics reachable from Python: none. About 1000 inputs (random, all-0xFF and + coefficient-edge public keys, secret keys and ciphertexts, and wrong sizes for + every argument and seed) went through the `_pykyber` functions in a build with + overflow checks on; every result was a success or a plain exception. +- Integer overflow: sizes are compile-time constants or `len()` of a checked + slice; the fuzz above had overflow checks on. Not a proof for adversarial + coefficients beyond those tried. +- Length checks before secret work: `crypto_kem_dec*` and every binding check + `ct`, `sk`, `pk` and the seed before touching secrets. +- Ciphertext compare covers the full ciphertext (see CVE-2026-6330 below). +- KyberSlash scan: see the table. + +## CVE and paper review (PLAN-2.0.md section 9) + +Dates are 2026-10-30. "Primary" means the issuing page or the paper itself. + +| Item | Status | Evidence | +|---|---|---| +| KyberSlash 1 (division in `poly_tomsg`) and 2 (`poly_compress`, `polyvec_compress`), (primary, read) | Not affected | The kyberslash.cr.yp.to page could not be fetched (transport error); the paper and search results agree the bug is a division by q on secret data. The code uses multiply-and-shift (`* 10321345 >> 35`). Whole-library release disassembly has no `div`/`idiv`/`udiv`/`sdiv` and no `__aeabi_*div`/`__udiv*` relocation on x86_64, aarch64, i686 and armv7; a control function with a runtime divisor does show them. This covers all three levels, compress, decompress, `poly_tomsg` and the ML-KEM code. The constant divisions became multiplies; their timing on cores with a data-dependent multiplier was not assessed. | +| CVE-2024-14041 (KyberSlash in Bouncy Castle Java) | Not affected | Other library; found in search (mondoo), not on the primary page. | +| CVE-2024-37880 (Clangover, `poly_frommsg`) | Mitigated | Not observed with rustc 1.95 on x86_64 and aarch64; barrier added (item 7). Record read via mirrors (Debian tracker, OSV, INCIBE); the NVD page itself did not render. The CVE names LLVM Clang through 18; Rust also uses LLVM, so the class applies. | +| CVE-2026-115408 (crypto11 ML-KEM key pair lookup) | Not affected, class addressed | Another library (Go). Read on OpenCVE (NVD status "Received"); not yet on NVD proper. Same class as `from_keys`: fixed for `Kyber*` (item 11), still open for `ML_KEM_*.from_keys`. | +| CVE-2026-6330 (wolfSSL ML-KEM ARM64 NEON compares half the ciphertext) | Not affected | CVE record read via the MITRE API listing and Debian tracker, not NVD. Our compare covers the whole ciphertext; the rejection test damages the first, middle and last bytes at all three levels. An AVX2 variant that compares 1536 of 1568 bytes appears in one vendor-blog result, unconfirmed. | +| 2026 SEALSQ and LightSec work on ML-KEM key generation and encoding | Unconfirmed, out of scope | Press release only (thequantuminsider, 2026-10-14); the paper was not read. It concerns hardware leakage through profiling plus lattice reduction; power and EM attacks are out of scope for this project. | +| Airtight side-channel testing framework (AFIT, ) | Not assessed | The AFIT page was not found by search; a paper with that title exists at ECCWS (academic-conferences.org). The framework was not run; dynamic timing tests are WP-8, which is not merged. | +| Reference implementations (Go, BoringSSL, RustCrypto, libcrux) | Not assessed | Comparison only; no code was copied. | + +## Benchmark (release wheels, base `8e56f40` vs this branch) + +`python/tests/test_kyber.py -k performance`, 100 iterations per op, median of +three alternating runs, one machine, noisy: decapsulation about the same at all +levels (+0.9%, 0.0%, -4.2%); key generation and encapsulation slower by up to +about 0.02 ms (Kyber768 keygen 0.051 to 0.074 ms, Kyber1024 keygen 0.087 to +0.113 ms, encapsulation +7% to +13%), most likely the `getrandom` call that +replaced `thread_rng`. The full key exchange moved -14% to +2%, inside the noise. diff --git a/python/pykyber/_kyber.py b/python/pykyber/_kyber.py index 74d4678..ea9225e 100644 --- a/python/pykyber/_kyber.py +++ b/python/pykyber/_kyber.py @@ -4,7 +4,18 @@ from ._deprecation import _deprecated_kyber from ._error import KyberError from ._types import EncapsulationResult, Keypair -from ._validation import _validate_encapsulate, _validate_decapsulate, _validate_keypair, _validate_pk +from ._validation import ( + _validate_decapsulate, + _validate_encapsulate, + _validate_key_binding, + _validate_keypair, + _validate_pk, +) + +# Bytes of one serialized polynomial (ByteEncode12 of 256 coefficients). +_POLY_BYTES = 384 +# Module rank k: the IND-CPA secret key s of a round-3 Kyber secret key is k polynomials. +_K_512, _K_768, _K_1024 = 2, 3, 4 class Kyber512: @@ -41,9 +52,13 @@ def decapsulate(ciphertext: bytes, secret_key: bytes) -> bytes: @classmethod @_deprecated_kyber def from_keys(cls, public_key: bytes, secret_key: bytes) -> Keypair: - """Create a Keypair object from existing public and secret keys.""" + """Create a Keypair object from existing public and secret keys. + + Raises KyberError if the public key is not the one stored inside the secret key. + """ from . import _pykyber _validate_pk(public_key, cls.PUBLIC_KEY_SIZE, f"{cls.__name__}.from_keys") + _validate_key_binding(public_key, secret_key, cls.SECRET_KEY_SIZE, _K_512 * _POLY_BYTES, f"{cls.__name__}.from_keys") return Keypair(secret_key, public_key, _validate_encapsulate(_pykyber._encapsulate_512, cls.PUBLIC_KEY_SIZE, cls.__name__), _validate_decapsulate(_pykyber._decapsulate_512, cls.CIPHERTEXT_SIZE, cls.__name__)) @@ -83,9 +98,13 @@ def decapsulate(ciphertext: bytes, secret_key: bytes) -> bytes: @classmethod @_deprecated_kyber def from_keys(cls, public_key: bytes, secret_key: bytes) -> Keypair: - """Create a Keypair object from existing public and secret keys.""" + """Create a Keypair object from existing public and secret keys. + + Raises KyberError if the public key is not the one stored inside the secret key. + """ from . import _pykyber _validate_pk(public_key, cls.PUBLIC_KEY_SIZE, f"{cls.__name__}.from_keys") + _validate_key_binding(public_key, secret_key, cls.SECRET_KEY_SIZE, _K_768 * _POLY_BYTES, f"{cls.__name__}.from_keys") return Keypair(secret_key, public_key, _validate_encapsulate(_pykyber._encapsulate_768, cls.PUBLIC_KEY_SIZE, cls.__name__), _validate_decapsulate(_pykyber._decapsulate_768, cls.CIPHERTEXT_SIZE, cls.__name__)) @@ -125,9 +144,13 @@ def decapsulate(ciphertext: bytes, secret_key: bytes) -> bytes: @classmethod @_deprecated_kyber def from_keys(cls, public_key: bytes, secret_key: bytes) -> Keypair: - """Create a Keypair object from existing public and secret keys.""" + """Create a Keypair object from existing public and secret keys. + + Raises KyberError if the public key is not the one stored inside the secret key. + """ from . import _pykyber _validate_pk(public_key, cls.PUBLIC_KEY_SIZE, f"{cls.__name__}.from_keys") + _validate_key_binding(public_key, secret_key, cls.SECRET_KEY_SIZE, _K_1024 * _POLY_BYTES, f"{cls.__name__}.from_keys") return Keypair(secret_key, public_key, _validate_encapsulate(_pykyber._encapsulate_1024, cls.PUBLIC_KEY_SIZE, cls.__name__), _validate_decapsulate(_pykyber._decapsulate_1024, cls.CIPHERTEXT_SIZE, cls.__name__)) diff --git a/python/pykyber/_validation.py b/python/pykyber/_validation.py index 36df8a2..6777348 100644 --- a/python/pykyber/_validation.py +++ b/python/pykyber/_validation.py @@ -16,6 +16,31 @@ def _validate_pk(pk: bytes, expected_size: int, method_name: str) -> None: ) +def _validate_key_binding( + public_key: bytes, + secret_key: bytes, + secret_key_size: int, + pke_secret_key_size: int, + method_name: str, +) -> None: + """Reject a public key that is not the copy embedded in the secret key. + + A round-3 Kyber secret key is ``sk_pke || pk || H(pk) || z`` (``pk`` starts at + ``pke_secret_key_size``), so a pair of keys from different key pairs can be + detected. Both sides are public data, so a plain comparison is fine. A secret + key of the wrong size is left alone: it fails its size check when it is used. + """ + if len(secret_key) != secret_key_size: + return + embedded = bytes(secret_key[pke_secret_key_size:pke_secret_key_size + len(public_key)]) + if bytes(public_key) != embedded: + raise KyberError( + f"Invalid key pair for {method_name}: the public key does not match the " + f"public key stored inside the secret key. Use the public key and the " + f"secret key of the same key pair." + ) + + def _validate_seed(seed: Optional[bytes], expected_size: int = 64) -> None: """Validate seed length.""" if seed is not None: @@ -32,6 +57,9 @@ def wrapper(pk: bytes) -> Tuple[bytes, bytes]: _validate_pk(pk, expected_pk_size, method_name) try: return fn(pk) + except KyberError: + # Keep the subclass, e.g. RandomnessError when the OS generator fails. + raise except Exception as e: raise KyberError(str(e)) from None return wrapper @@ -63,6 +91,8 @@ def wrapper(seed: Optional[bytes] = None) -> Tuple[bytes, bytes]: _validate_seed(seed) try: return fn(seed) + except KyberError: + raise except Exception as e: raise KyberError(str(e)) from None return wrapper diff --git a/python/tests/test_kyber.py b/python/tests/test_kyber.py index 1b787d3..1247f22 100644 --- a/python/tests/test_kyber.py +++ b/python/tests/test_kyber.py @@ -1570,3 +1570,75 @@ def test_keypair_as_tuple_in_function(): decrypted = pykyber.Kyber768.decapsulate(ciphertext, sk) assert shared_secret == decrypted + + +class TestRandomnessFailure: + """A failing OS generator reaches the caller as RandomnessError, a KyberError.""" + + @staticmethod + def _failing(*_args): + raise pykyber.RandomnessError("Random bytes generation failed") + + def test_keypair_wrapper_keeps_randomness_error(self): + from pykyber._validation import _validate_keypair + + with pytest.raises(pykyber.RandomnessError) as excinfo: + _validate_keypair(self._failing)() + assert isinstance(excinfo.value, pykyber.KyberError) + + def test_encapsulate_wrapper_keeps_randomness_error(self): + from pykyber._validation import _validate_encapsulate + + with pytest.raises(pykyber.RandomnessError): + _validate_encapsulate(self._failing, 1184, "Kyber768")(bytes(1184)) + + def test_other_errors_are_still_wrapped(self): + from pykyber._validation import _validate_keypair + + def boom(_seed): + raise ValueError("boom") + + with pytest.raises(pykyber.KyberError) as excinfo: + _validate_keypair(boom)() + assert not isinstance(excinfo.value, pykyber.RandomnessError) + + +# from_keys must bind the public key to the secret key it is paired with. + +_LEVELS = [pykyber.Kyber512, pykyber.Kyber768, pykyber.Kyber1024] + + +@pytest.mark.parametrize("cls", _LEVELS) +class TestFromKeysBinding: + def test_matching_pair_is_accepted(self, cls): + original = cls() + rebuilt = cls.from_keys(original.public_key, original.secret_key) + result = rebuilt.encapsulate() + assert original.decapsulate(result.ciphertext) == result.shared_secret + assert rebuilt.decapsulate(result.ciphertext) == result.shared_secret + + def test_mismatched_pair_is_rejected(self, cls): + a, b = cls(), cls() + with pytest.raises(pykyber.KyberError, match="does not match"): + cls.from_keys(a.public_key, b.secret_key) + with pytest.raises(pykyber.KyberError, match="does not match"): + cls.from_keys(b.public_key, a.secret_key) + + @pytest.mark.parametrize("position", [0, -1]) + def test_one_changed_byte_of_the_public_key_is_rejected(self, cls, position): + pair = cls() + pk = bytearray(pair.public_key) + pk[position] ^= 1 + with pytest.raises(pykyber.KyberError, match="does not match"): + cls.from_keys(bytes(pk), pair.secret_key) + + def test_short_public_key_keeps_its_size_error(self, cls): + pair = cls() + with pytest.raises(pykyber.KyberError, match="Invalid public key"): + cls.from_keys(pair.public_key[:-1], pair.secret_key) + + def test_short_secret_key_keeps_its_size_error(self, cls): + pair = cls() + short = cls.from_keys(pair.public_key, pair.secret_key[:-1]) + with pytest.raises(pykyber.KyberError, match="Invalid input for 'sk'"): + short.decapsulate(pair.encapsulate().ciphertext) diff --git a/src/kyber/api.rs b/src/kyber/api.rs index 2cc0997..ad8b5c9 100644 --- a/src/kyber/api.rs +++ b/src/kyber/api.rs @@ -65,6 +65,9 @@ impl Keypair { } } +/// Stands in for a generator on the seeded path, which draws no randomness. The +/// KEM code asks only through `try_fill_bytes`, so if that ever changed it would +/// get a `RandomBytesGeneration` error and not a panic. struct DummyRng {} impl CryptoRng for DummyRng {} impl RngCore for DummyRng { @@ -75,7 +78,7 @@ impl RngCore for DummyRng { panic!() } fn try_fill_bytes(&mut self, _dest: &mut [u8]) -> Result<(), rand_core::Error> { - panic!() + Err(rand_core::Error::new("the seeded path draws no randomness")) } fn fill_bytes(&mut self, _dest: &mut [u8]) { panic!() diff --git a/src/kyber/indcpa.rs b/src/kyber/indcpa.rs index 325ced7..3de0494 100644 --- a/src/kyber/indcpa.rs +++ b/src/kyber/indcpa.rs @@ -8,7 +8,7 @@ use super::symmetric::{ hash_sha3_512, xof_absorb_bytes, xof_squeeze_blocks, XofState, XOF_BLOCKBYTES, }; use rand_core::{CryptoRng, RngCore}; -use zeroize::Zeroize; +use zeroize::{Zeroize, Zeroizing}; fn pack_pk(r: &mut [u8], pk: &mut Polyvec, seed: &[u8]) { const END: usize = KYBER_SYMBYTES + KYBER_POLYVECBYTES; @@ -115,15 +115,16 @@ where let (mut e, mut pkpv, mut skpv) = (Polyvec::new(), Polyvec::new(), Polyvec::new()); let mut nonce = 0u8; let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut randbuf = [0u8; 2 * KYBER_SYMBYTES]; + let mut randbuf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); if let Some(s) = _seed { randbuf[..KYBER_SYMBYTES].copy_from_slice(s.0); } else { - _rng.fill_bytes(&mut randbuf[..KYBER_SYMBYTES]); + _rng.try_fill_bytes(&mut randbuf[..KYBER_SYMBYTES]) + .map_err(|_| KyberError::RandomBytesGeneration)?; } - hash_sha3_512(&mut buf, &randbuf, KYBER_SYMBYTES); + hash_sha3_512(&mut buf, &randbuf[..], KYBER_SYMBYTES); let (publicseed, noiseseed) = buf.split_at(KYBER_SYMBYTES); gen_a(&mut a, publicseed); @@ -297,15 +298,16 @@ where let (mut e, mut pkpv, mut skpv) = (Polyvec512::new(), Polyvec512::new(), Polyvec512::new()); let mut nonce = 0u8; let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut randbuf = [0u8; 2 * KYBER_SYMBYTES]; + let mut randbuf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); if let Some(s) = _seed { randbuf[..KYBER_SYMBYTES].copy_from_slice(s.0); } else { - _rng.fill_bytes(&mut randbuf[..KYBER_SYMBYTES]); + _rng.try_fill_bytes(&mut randbuf[..KYBER_SYMBYTES]) + .map_err(|_| KyberError::RandomBytesGeneration)?; } - hash_sha3_512(&mut buf, &randbuf, KYBER_SYMBYTES); + hash_sha3_512(&mut buf, &randbuf[..], KYBER_SYMBYTES); let (publicseed, noiseseed) = buf.split_at(KYBER_SYMBYTES); gen_a_512(&mut a, publicseed); @@ -481,15 +483,16 @@ where let (mut e, mut pkpv, mut skpv) = (Polyvec1024::new(), Polyvec1024::new(), Polyvec1024::new()); let mut nonce = 0u8; let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut randbuf = [0u8; 2 * KYBER_SYMBYTES]; + let mut randbuf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); if let Some(s) = _seed { randbuf[..KYBER_SYMBYTES].copy_from_slice(s.0); } else { - _rng.fill_bytes(&mut randbuf[..KYBER_SYMBYTES]); + _rng.try_fill_bytes(&mut randbuf[..KYBER_SYMBYTES]) + .map_err(|_| KyberError::RandomBytesGeneration)?; } - hash_sha3_512(&mut buf, &randbuf, KYBER_SYMBYTES); + hash_sha3_512(&mut buf, &randbuf[..], KYBER_SYMBYTES); let (publicseed, noiseseed) = buf.split_at(KYBER_SYMBYTES); gen_a_1024(&mut a, publicseed); diff --git a/src/kyber/kem.rs b/src/kyber/kem.rs index 1819f51..fe8a3d5 100644 --- a/src/kyber/kem.rs +++ b/src/kyber/kem.rs @@ -7,7 +7,50 @@ use super::params::*; use super::symmetric::{hash_sha3_256, hash_sha3_512, shake256_kdf}; use super::verify::{cmov, verify}; use rand_core::{CryptoRng, RngCore}; -use zeroize::Zeroize; +use zeroize::{Zeroize, Zeroizing}; + +/// Fails with `InvalidInput` unless `actual == expected`; lengths are public. +fn check_len(param: &'static str, expected: usize, actual: usize) -> Result<(), KyberError> { + if actual != expected { + return Err(KyberError::invalid_input(param, expected, actual)); + } + Ok(()) +} + +/// Length checks shared by the three key generation functions. +fn check_keypair_args( + pk: &[u8], + sk: &[u8], + seed: Option<(&[u8], &[u8])>, + pk_len: usize, + sk_len: usize, +) -> Result<(), KyberError> { + check_len("pk", pk_len, pk.len())?; + check_len("sk", sk_len, sk.len())?; + if let Some((d, z)) = seed { + check_len("seed d", KYBER_SYMBYTES, d.len())?; + check_len("seed z", KYBER_SYMBYTES, z.len())?; + } + Ok(()) +} + +/// Length checks shared by the three encapsulation functions. +fn check_enc_args( + ct: &[u8], + ss: &[u8], + pk: &[u8], + seed: Option<&[u8]>, + ct_len: usize, + pk_len: usize, +) -> Result<(), KyberError> { + check_len("ct", ct_len, ct.len())?; + check_len("ss", KYBER_SSBYTES, ss.len())?; + check_len("pk", pk_len, pk.len())?; + if let Some(m) = seed { + check_len("seed", KYBER_SYMBYTES, m.len())?; + } + Ok(()) +} pub fn crypto_kem_keypair( pk: &mut [u8], @@ -18,6 +61,8 @@ pub fn crypto_kem_keypair( where R: RngCore + CryptoRng, { + check_keypair_args(pk, sk, _seed, KYBER_PUBLICKEYBYTES, KYBER_SECRETKEYBYTES)?; + const PK_START: usize = KYBER_SECRETKEYBYTES - (2 * KYBER_SYMBYTES); const SK_START: usize = KYBER_SECRETKEYBYTES - KYBER_SYMBYTES; const END: usize = KYBER_INDCPA_PUBLICKEYBYTES + KYBER_INDCPA_SECRETKEYBYTES; @@ -30,7 +75,14 @@ where if let Some(s) = _seed { sk[SK_START..].copy_from_slice(s.1) } else { - _rng.fill_bytes(&mut sk[SK_START..SK_START + KYBER_SYMBYTES]); + if _rng + .try_fill_bytes(&mut sk[SK_START..SK_START + KYBER_SYMBYTES]) + .is_err() + { + // sk already holds the IND-CPA secret key; do not hand it back. + sk.zeroize(); + return Err(KyberError::RandomBytesGeneration); + } } Ok(()) } @@ -45,17 +97,27 @@ pub fn crypto_kem_enc( where R: RngCore + CryptoRng, { + check_enc_args( + ct, + ss, + pk, + _seed, + KYBER_CIPHERTEXTBYTES, + KYBER_PUBLICKEYBYTES, + )?; + let mut kr = [0u8; 2 * KYBER_SYMBYTES]; let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut randbuf = [0u8; 2 * KYBER_SYMBYTES]; + let mut randbuf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); if let Some(s) = _seed { randbuf[..KYBER_SYMBYTES].copy_from_slice(s); } else { - _rng.fill_bytes(&mut randbuf); + _rng.try_fill_bytes(&mut randbuf[..]) + .map_err(|_| KyberError::RandomBytesGeneration)?; } - hash_sha3_256(&mut buf, &randbuf, KYBER_SYMBYTES); + hash_sha3_256(&mut buf, &randbuf[..], KYBER_SYMBYTES); hash_sha3_256(&mut buf[KYBER_SYMBYTES..], pk, KYBER_PUBLICKEYBYTES); hash_sha3_512(&mut kr, &buf, 2 * KYBER_SYMBYTES); @@ -89,29 +151,28 @@ pub fn crypto_kem_dec(ss: &mut [u8], ct: &[u8], sk: &[u8]) -> Result<(), KyberEr return Err(KyberError::invalid_input("ss", KYBER_SSBYTES, ss.len())); } - let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut kr = [0u8; 2 * KYBER_SYMBYTES]; - let mut cmp = [0u8; KYBER_CIPHERTEXTBYTES]; + let mut buf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); + let mut kr = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); + let mut cmp = Zeroizing::new([0u8; KYBER_CIPHERTEXTBYTES]); let mut pk = [0u8; KYBER_INDCPA_PUBLICKEYBYTES]; pk.copy_from_slice(&sk[KYBER_INDCPA_SECRETKEYBYTES..][..KYBER_INDCPA_PUBLICKEYBYTES]); - indcpa_dec(&mut buf, ct, sk); + indcpa_dec(&mut buf[..], ct, sk); const START: usize = KYBER_SECRETKEYBYTES - 2 * KYBER_SYMBYTES; const END: usize = KYBER_SECRETKEYBYTES - KYBER_SYMBYTES; buf[KYBER_SYMBYTES..].copy_from_slice(&sk[START..END]); - hash_sha3_512(&mut kr, &buf, 2 * KYBER_SYMBYTES); + hash_sha3_512(&mut kr[..], &buf[..], 2 * KYBER_SYMBYTES); - indcpa_enc(&mut cmp, &buf, &pk, &kr[KYBER_SYMBYTES..]); - let fail = verify(ct, &cmp, KYBER_CIPHERTEXTBYTES); + indcpa_enc(&mut cmp[..], &buf[..], &pk, &kr[KYBER_SYMBYTES..]); + // The reject flag is secret: it never leaves this function and is wiped on drop. + let fail = Zeroizing::new(verify(ct, &cmp[..], KYBER_CIPHERTEXTBYTES)); hash_sha3_256(&mut kr[KYBER_SYMBYTES..], ct, KYBER_CIPHERTEXTBYTES); - cmov(&mut kr, &sk[END..], KYBER_SYMBYTES, fail); - shake256_kdf(ss, &kr, 2 * KYBER_SYMBYTES); + cmov(&mut kr[..], &sk[END..], KYBER_SYMBYTES, *fail); + shake256_kdf(ss, &kr[..], 2 * KYBER_SYMBYTES); - buf.zeroize(); - kr.zeroize(); - cmp.zeroize(); + // buf (m'), kr, cmp and the flag are `Zeroizing`: wiped on every exit, unwinding included. pk.zeroize(); Ok(()) } @@ -125,6 +186,14 @@ pub fn crypto_kem_keypair_512( where R: RngCore + CryptoRng, { + check_keypair_args( + pk, + sk, + _seed, + KYBER_512_PUBLICKEYBYTES, + KYBER_512_SECRETKEYBYTES, + )?; + const PK_START: usize = KYBER_512_SECRETKEYBYTES - (2 * KYBER_SYMBYTES); const SK_START: usize = KYBER_512_SECRETKEYBYTES - KYBER_SYMBYTES; const END: usize = KYBER_512_INDCPA_PUBLICKEYBYTES + KYBER_512_INDCPA_SECRETKEYBYTES; @@ -138,7 +207,14 @@ where if let Some(s) = _seed { sk[SK_START..].copy_from_slice(s.1) } else { - _rng.fill_bytes(&mut sk[SK_START..SK_START + KYBER_SYMBYTES]); + if _rng + .try_fill_bytes(&mut sk[SK_START..SK_START + KYBER_SYMBYTES]) + .is_err() + { + // sk already holds the IND-CPA secret key; do not hand it back. + sk.zeroize(); + return Err(KyberError::RandomBytesGeneration); + } } Ok(()) } @@ -153,17 +229,27 @@ pub fn crypto_kem_enc_512( where R: RngCore + CryptoRng, { + check_enc_args( + ct, + ss, + pk, + _seed, + KYBER_512_CIPHERTEXTBYTES, + KYBER_512_PUBLICKEYBYTES, + )?; + let mut kr = [0u8; 2 * KYBER_SYMBYTES]; let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut randbuf = [0u8; 2 * KYBER_SYMBYTES]; + let mut randbuf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); if let Some(s) = _seed { randbuf[..KYBER_SYMBYTES].copy_from_slice(s); } else { - _rng.fill_bytes(&mut randbuf); + _rng.try_fill_bytes(&mut randbuf[..]) + .map_err(|_| KyberError::RandomBytesGeneration)?; } - hash_sha3_256(&mut buf, &randbuf, KYBER_SYMBYTES); + hash_sha3_256(&mut buf, &randbuf[..], KYBER_SYMBYTES); hash_sha3_256(&mut buf[KYBER_SYMBYTES..], pk, KYBER_512_PUBLICKEYBYTES); hash_sha3_512(&mut kr, &buf, 2 * KYBER_SYMBYTES); @@ -197,29 +283,28 @@ pub fn crypto_kem_dec_512(ss: &mut [u8], ct: &[u8], sk: &[u8]) -> Result<(), Kyb return Err(KyberError::invalid_input("ss", KYBER_SSBYTES, ss.len())); } - let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut kr = [0u8; 2 * KYBER_SYMBYTES]; - let mut cmp = [0u8; KYBER_512_CIPHERTEXTBYTES]; + let mut buf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); + let mut kr = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); + let mut cmp = Zeroizing::new([0u8; KYBER_512_CIPHERTEXTBYTES]); let mut pk = [0u8; KYBER_512_INDCPA_PUBLICKEYBYTES]; pk.copy_from_slice(&sk[KYBER_512_INDCPA_SECRETKEYBYTES..][..KYBER_512_INDCPA_PUBLICKEYBYTES]); - indcpa_dec_512(&mut buf, ct, sk); + indcpa_dec_512(&mut buf[..], ct, sk); const START: usize = KYBER_512_SECRETKEYBYTES - 2 * KYBER_SYMBYTES; const END: usize = KYBER_512_SECRETKEYBYTES - KYBER_SYMBYTES; buf[KYBER_SYMBYTES..].copy_from_slice(&sk[START..END]); - hash_sha3_512(&mut kr, &buf, 2 * KYBER_SYMBYTES); + hash_sha3_512(&mut kr[..], &buf[..], 2 * KYBER_SYMBYTES); - indcpa_enc_512(&mut cmp, &buf, &pk, &kr[KYBER_SYMBYTES..]); - let fail = verify(ct, &cmp, KYBER_512_CIPHERTEXTBYTES); + indcpa_enc_512(&mut cmp[..], &buf[..], &pk, &kr[KYBER_SYMBYTES..]); + // The reject flag is secret: it never leaves this function and is wiped on drop. + let fail = Zeroizing::new(verify(ct, &cmp[..], KYBER_512_CIPHERTEXTBYTES)); hash_sha3_256(&mut kr[KYBER_SYMBYTES..], ct, KYBER_512_CIPHERTEXTBYTES); - cmov(&mut kr, &sk[END..], KYBER_SYMBYTES, fail); - shake256_kdf(ss, &kr, 2 * KYBER_SYMBYTES); + cmov(&mut kr[..], &sk[END..], KYBER_SYMBYTES, *fail); + shake256_kdf(ss, &kr[..], 2 * KYBER_SYMBYTES); - buf.zeroize(); - kr.zeroize(); - cmp.zeroize(); + // buf (m'), kr, cmp and the flag are `Zeroizing`: wiped on every exit, unwinding included. pk.zeroize(); Ok(()) } @@ -233,6 +318,14 @@ pub fn crypto_kem_keypair_1024( where R: RngCore + CryptoRng, { + check_keypair_args( + pk, + sk, + _seed, + KYBER_1024_PUBLICKEYBYTES, + KYBER_1024_SECRETKEYBYTES, + )?; + const PK_START: usize = KYBER_1024_SECRETKEYBYTES - (2 * KYBER_SYMBYTES); const SK_START: usize = KYBER_1024_SECRETKEYBYTES - KYBER_SYMBYTES; const END: usize = KYBER_1024_INDCPA_PUBLICKEYBYTES + KYBER_1024_INDCPA_SECRETKEYBYTES; @@ -246,7 +339,14 @@ where if let Some(s) = _seed { sk[SK_START..].copy_from_slice(s.1) } else { - _rng.fill_bytes(&mut sk[SK_START..SK_START + KYBER_SYMBYTES]); + if _rng + .try_fill_bytes(&mut sk[SK_START..SK_START + KYBER_SYMBYTES]) + .is_err() + { + // sk already holds the IND-CPA secret key; do not hand it back. + sk.zeroize(); + return Err(KyberError::RandomBytesGeneration); + } } Ok(()) } @@ -261,17 +361,27 @@ pub fn crypto_kem_enc_1024( where R: RngCore + CryptoRng, { + check_enc_args( + ct, + ss, + pk, + _seed, + KYBER_1024_CIPHERTEXTBYTES, + KYBER_1024_PUBLICKEYBYTES, + )?; + let mut kr = [0u8; 2 * KYBER_SYMBYTES]; let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut randbuf = [0u8; 2 * KYBER_SYMBYTES]; + let mut randbuf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); if let Some(s) = _seed { randbuf[..KYBER_SYMBYTES].copy_from_slice(s); } else { - _rng.fill_bytes(&mut randbuf); + _rng.try_fill_bytes(&mut randbuf[..]) + .map_err(|_| KyberError::RandomBytesGeneration)?; } - hash_sha3_256(&mut buf, &randbuf, KYBER_SYMBYTES); + hash_sha3_256(&mut buf, &randbuf[..], KYBER_SYMBYTES); hash_sha3_256(&mut buf[KYBER_SYMBYTES..], pk, KYBER_1024_PUBLICKEYBYTES); hash_sha3_512(&mut kr, &buf, 2 * KYBER_SYMBYTES); @@ -305,29 +415,28 @@ pub fn crypto_kem_dec_1024(ss: &mut [u8], ct: &[u8], sk: &[u8]) -> Result<(), Ky return Err(KyberError::invalid_input("ss", KYBER_SSBYTES, ss.len())); } - let mut buf = [0u8; 2 * KYBER_SYMBYTES]; - let mut kr = [0u8; 2 * KYBER_SYMBYTES]; - let mut cmp = [0u8; KYBER_1024_CIPHERTEXTBYTES]; + let mut buf = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); + let mut kr = Zeroizing::new([0u8; 2 * KYBER_SYMBYTES]); + let mut cmp = Zeroizing::new([0u8; KYBER_1024_CIPHERTEXTBYTES]); let mut pk = [0u8; KYBER_1024_INDCPA_PUBLICKEYBYTES]; pk.copy_from_slice(&sk[KYBER_1024_INDCPA_SECRETKEYBYTES..][..KYBER_1024_INDCPA_PUBLICKEYBYTES]); - indcpa_dec_1024(&mut buf, ct, sk); + indcpa_dec_1024(&mut buf[..], ct, sk); const START: usize = KYBER_1024_SECRETKEYBYTES - 2 * KYBER_SYMBYTES; const END: usize = KYBER_1024_SECRETKEYBYTES - KYBER_SYMBYTES; buf[KYBER_SYMBYTES..].copy_from_slice(&sk[START..END]); - hash_sha3_512(&mut kr, &buf, 2 * KYBER_SYMBYTES); + hash_sha3_512(&mut kr[..], &buf[..], 2 * KYBER_SYMBYTES); - indcpa_enc_1024(&mut cmp, &buf, &pk, &kr[KYBER_SYMBYTES..]); - let fail = verify(ct, &cmp, KYBER_1024_CIPHERTEXTBYTES); + indcpa_enc_1024(&mut cmp[..], &buf[..], &pk, &kr[KYBER_SYMBYTES..]); + // The reject flag is secret: it never leaves this function and is wiped on drop. + let fail = Zeroizing::new(verify(ct, &cmp[..], KYBER_1024_CIPHERTEXTBYTES)); hash_sha3_256(&mut kr[KYBER_SYMBYTES..], ct, KYBER_1024_CIPHERTEXTBYTES); - cmov(&mut kr, &sk[END..], KYBER_SYMBYTES, fail); - shake256_kdf(ss, &kr, 2 * KYBER_SYMBYTES); + cmov(&mut kr[..], &sk[END..], KYBER_SYMBYTES, *fail); + shake256_kdf(ss, &kr[..], 2 * KYBER_SYMBYTES); - buf.zeroize(); - kr.zeroize(); - cmp.zeroize(); + // buf (m'), kr, cmp and the flag are `Zeroizing`: wiped on every exit, unwinding included. pk.zeroize(); Ok(()) } diff --git a/src/kyber/poly.rs b/src/kyber/poly.rs index 9e3c21e..3b26418 100644 --- a/src/kyber/poly.rs +++ b/src/kyber/poly.rs @@ -15,7 +15,8 @@ use super::params::*; use super::reduce::{barrett_reduce, montgomery_reduce}; use super::symmetric::shake256_prf_expand; -use zeroize::Zeroize; +use core::hint::black_box; +use zeroize::{Zeroize, Zeroizing}; #[derive(Clone, Zeroize)] #[zeroize(drop)] @@ -132,30 +133,34 @@ pub fn poly_frombytes(r: &mut Poly, a: &[u8]) { pub fn poly_getnoise_eta1(r: &mut Poly, seed: &[u8], nonce: u8) { const LENGTH: usize = KYBER_ETA1 * KYBER_N / 4; - let mut buf = [0u8; LENGTH]; - shake256_prf_expand(&mut buf, LENGTH, seed, nonce); - poly_cbd_eta1(r, &buf); + // The PRF output is the secret noise (the key in key generation, r in encryption). + let mut buf = Zeroizing::new([0u8; LENGTH]); + shake256_prf_expand(&mut buf[..], LENGTH, seed, nonce); + poly_cbd_eta1(r, &buf[..]); } pub fn poly_getnoise_eta1_512(r: &mut Poly, seed: &[u8], nonce: u8) { const LENGTH: usize = KYBER_512_ETA1 * KYBER_N / 4; - let mut buf = [0u8; LENGTH]; - shake256_prf_expand(&mut buf, LENGTH, seed, nonce); - poly_cbd_eta1_512(r, &buf); + // The PRF output is the secret noise (the key in key generation, r in encryption). + let mut buf = Zeroizing::new([0u8; LENGTH]); + shake256_prf_expand(&mut buf[..], LENGTH, seed, nonce); + poly_cbd_eta1_512(r, &buf[..]); } pub fn poly_getnoise_eta2(r: &mut Poly, seed: &[u8], nonce: u8) { const LENGTH: usize = KYBER_ETA2 * KYBER_N / 4; - let mut buf = [0u8; LENGTH]; - shake256_prf_expand(&mut buf, LENGTH, seed, nonce); - poly_cbd_eta2(r, &buf); + // The PRF output is the secret noise (the key in key generation, r in encryption). + let mut buf = Zeroizing::new([0u8; LENGTH]); + shake256_prf_expand(&mut buf[..], LENGTH, seed, nonce); + poly_cbd_eta2(r, &buf[..]); } pub fn poly_getnoise_eta2_512(r: &mut Poly, seed: &[u8], nonce: u8) { const LENGTH: usize = KYBER_512_ETA2 * KYBER_N / 4; - let mut buf = [0u8; LENGTH]; - shake256_prf_expand(&mut buf, LENGTH, seed, nonce); - poly_cbd_eta2(r, &buf); + // The PRF output is the secret noise (the key in key generation, r in encryption). + let mut buf = Zeroizing::new([0u8; LENGTH]); + shake256_prf_expand(&mut buf[..], LENGTH, seed, nonce); + poly_cbd_eta2(r, &buf[..]); } pub fn poly_ntt(r: &mut Poly) { @@ -210,11 +215,19 @@ pub fn poly_sub(r: &mut Poly, a: &Poly) { } } +/// Expands each message bit to 0 or (q + 1) / 2. +/// +/// The mask `-(bit)` is the pattern behind CVE-2024-37880 ("Clangover"), where +/// the compiler turned it into a branch on the message bit. `black_box` is an +/// optimization barrier: the compiler may not assume what the bit or the mask +/// is, so it cannot rewrite the mask into a conditional. It is best effort, not a +/// guarantee (see SECURITY.md). pub fn poly_frommsg(r: &mut Poly, msg: &[u8]) { let mut mask; for i in 0..KYBER_N / 8 { for j in 0..8 { - mask = ((msg[i] as u16 >> j) & 1).wrapping_neg(); + let bit = black_box((msg[i] as u16 >> j) & 1); + mask = black_box(bit.wrapping_neg()); r.coeffs[8 * i + j] = (mask & KYBER_Q.div_ceil(2) as u16) as i16; } } diff --git a/src/kyber/verify.rs b/src/kyber/verify.rs index ef9f7fd..629727b 100644 --- a/src/kyber/verify.rs +++ b/src/kyber/verify.rs @@ -1,15 +1,35 @@ +//! Comparison and conditional move for the round-3 Kyber re-encryption check. +//! +//! The standard library has no constant-time guarantee. `black_box` is an +//! optimization barrier: the compiler may not assume what the accumulated +//! difference or the select mask is, so it cannot prove them to be 0/1 and +//! rewrite the select as a branch (the pattern behind CVE-2024-37880, +//! "Clangover"). It is best effort, so the release assembly is checked (see +//! SECURITY.md). The ML-KEM side does the same in `mlkem::ct`. `len` is public +//! and must not exceed either slice. + +use core::hint::black_box; + +/// Shift that moves the top bit of a `u16` to bit 0. +const FLAG_SHIFT: u32 = u16::BITS - 1; + +/// Returns 0 if the first `len` bytes of `a` and `b` are equal, 1 otherwise. pub fn verify(a: &[u8], b: &[u8], len: usize) -> u8 { - let mut r = 0u64; + let mut r = 0u8; for i in 0..len { - r |= (a[i] ^ b[i]) as u64; + r |= a[i] ^ b[i]; } - r = r.wrapping_neg() >> 63; - r as u8 + // bit = 1 if r != 0 else 0: for r in 1..=255 the top bit of `-r` as a u16 is set. + let wide = u16::from(black_box(r)); + black_box(((wide | wide.wrapping_neg()) >> FLAG_SHIFT) as u8) } -pub fn cmov(r: &mut [u8], x: &[u8], len: usize, mut b: u8) { - b = b.wrapping_neg(); +/// Copies the first `len` bytes of `x` into `r` if `b` is 1, and leaves `r` +/// alone if `b` is 0. `b` must be 0 or 1. +pub fn cmov(r: &mut [u8], x: &[u8], len: usize, b: u8) { + // 0x00 (keep r) or 0xff (take x). + let mask = black_box(black_box(b).wrapping_neg()); for i in 0..len { - r[i] ^= b & (x[i] ^ r[i]); + r[i] ^= mask & (x[i] ^ r[i]); } } diff --git a/src/lib.rs b/src/lib.rs index ecc1406..076f1f6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,34 +10,77 @@ use kyber::{ }; use pyo3::exceptions::PyException; use pyo3::prelude::*; -use rand::thread_rng; +use pyo3::types::PyBytes; +use rand::rngs::OsRng; +use zeroize::{Zeroize, Zeroizing}; fn make_kyber_error(msg: &str) -> PyErr { PyException::new_err(msg.to_string()) } +/// Secret bytes (a secret key or a shared secret) on their way to Python. The +/// Python `bytes` object is built from a copy, and this Rust buffer is wiped +/// when it is dropped right after, instead of being freed with the secret in it. +/// The Python object itself cannot be wiped. +struct SecretBytes(Zeroizing>); + +impl SecretBytes { + fn new(bytes: Vec) -> Self { + SecretBytes(Zeroizing::new(bytes)) + } + + /// Copies a fixed-size secret and wipes the original. + fn from_array(mut bytes: [u8; N]) -> Self { + let copy = SecretBytes::new(bytes.to_vec()); + bytes.zeroize(); + copy + } +} + +impl<'py> IntoPyObject<'py> for SecretBytes { + type Target = PyBytes; + type Output = Bound<'py, PyBytes>; + type Error = std::convert::Infallible; + + fn into_pyobject(self, py: Python<'py>) -> Result { + Ok(PyBytes::new(py, &self.0)) + } +} + +/// A failed OS random generator is raised as `pykyber.RandomnessError` (a +/// `KyberError` subclass), like the ML-KEM functions do. Every other error keeps +/// the plain exception the legacy functions have always raised. +fn legacy_err(error: &KyberError) -> PyErr { + match error { + KyberError::RandomBytesGeneration => { + Python::attach(|py| pymlkem::to_py_err(py, KyberError::RandomBytesGeneration)) + } + _ => make_kyber_error(&error.to_string()), + } +} + #[pyfunction] -fn generate_keypair(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { +fn generate_keypair(seed: Option<&[u8]>) -> PyResult<(Vec, SecretBytes)> { if let Some(s) = seed { if s.len() != 64 { return Err(make_kyber_error("Seed must be 64 bytes")); } kyber::derive(s) - .map(|keys| (keys.public.to_vec(), keys.secret.to_vec())) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|keys| (keys.public.to_vec(), SecretBytes::new(keys.secret.to_vec()))) + .map_err(|e| legacy_err(&e)) } else { - let mut rng = thread_rng(); + let mut rng = OsRng; keypair(&mut rng) - .map(|keys| (keys.public.to_vec(), keys.secret.to_vec())) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|keys| (keys.public.to_vec(), SecretBytes::new(keys.secret.to_vec()))) + .map_err(|e| legacy_err(&e)) } } #[pyfunction] -fn generate_keypair_512(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { - let mut rng = thread_rng(); +fn generate_keypair_512(seed: Option<&[u8]>) -> PyResult<(Vec, SecretBytes)> { + let mut rng = OsRng; let mut pk = vec![0u8; KYBER_512_PUBLICKEYBYTES]; - let mut sk = vec![0u8; KYBER_512_SECRETKEYBYTES]; + let mut sk = Zeroizing::new(vec![0u8; KYBER_512_SECRETKEYBYTES]); let s_opt = if let Some(s) = seed { if s.len() != 64 { return Err(make_kyber_error("Seed must be 64 bytes")); @@ -47,15 +90,15 @@ fn generate_keypair_512(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { None }; crypto_kem_keypair_512(&mut pk, &mut sk, &mut rng, s_opt) - .map(|_| (pk, sk)) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|_| (pk, SecretBytes(sk))) + .map_err(|e| legacy_err(&e)) } #[pyfunction] -fn generate_keypair_1024(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { - let mut rng = thread_rng(); +fn generate_keypair_1024(seed: Option<&[u8]>) -> PyResult<(Vec, SecretBytes)> { + let mut rng = OsRng; let mut pk = vec![0u8; KYBER_1024_PUBLICKEYBYTES]; - let mut sk = vec![0u8; KYBER_1024_SECRETKEYBYTES]; + let mut sk = Zeroizing::new(vec![0u8; KYBER_1024_SECRETKEYBYTES]); let s_opt = if let Some(s) = seed { if s.len() != 64 { return Err(make_kyber_error("Seed must be 64 bytes")); @@ -65,57 +108,57 @@ fn generate_keypair_1024(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { None }; crypto_kem_keypair_1024(&mut pk, &mut sk, &mut rng, s_opt) - .map(|_| (pk, sk)) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|_| (pk, SecretBytes(sk))) + .map_err(|e| legacy_err(&e)) } #[pyfunction] -fn encapsulate_key(pk: &[u8]) -> PyResult<(Vec, Vec)> { - let mut rng = thread_rng(); +fn encapsulate_key(pk: &[u8]) -> PyResult<(Vec, SecretBytes)> { + let mut rng = OsRng; encapsulate(pk, &mut rng) - .map(|(ct, ss)| (ct.to_vec(), ss.to_vec())) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|(ct, ss)| (ct.to_vec(), SecretBytes::from_array(ss))) + .map_err(|e| legacy_err(&e)) } #[pyfunction] -fn decapsulate_key(ct: &[u8], sk: &[u8]) -> PyResult> { +fn decapsulate_key(ct: &[u8], sk: &[u8]) -> PyResult { decapsulate(ct, sk) - .map(|ss| ss.to_vec()) + .map(SecretBytes::from_array) .map_err(|e| make_kyber_error(&e.to_string())) } #[pyfunction] -fn encapsulate_key_512(pk: &[u8]) -> PyResult<(Vec, Vec)> { +fn encapsulate_key_512(pk: &[u8]) -> PyResult<(Vec, SecretBytes)> { if pk.len() != KYBER_512_PUBLICKEYBYTES { return Err(make_kyber_error( &KyberError::invalid_input("pk", KYBER_512_PUBLICKEYBYTES, pk.len()).to_string(), )); } - let mut rng = thread_rng(); + let mut rng = OsRng; let mut ct = vec![0u8; KYBER_512_CIPHERTEXTBYTES]; - let mut ss = vec![0u8; 32]; + let mut ss = Zeroizing::new(vec![0u8; 32]); crypto_kem_enc_512(&mut ct, &mut ss, pk, &mut rng, None) - .map(|_| (ct, ss)) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|_| (ct, SecretBytes(ss))) + .map_err(|e| legacy_err(&e)) } #[pyfunction] -fn encapsulate_key_1024(pk: &[u8]) -> PyResult<(Vec, Vec)> { +fn encapsulate_key_1024(pk: &[u8]) -> PyResult<(Vec, SecretBytes)> { if pk.len() != KYBER_1024_PUBLICKEYBYTES { return Err(make_kyber_error( &KyberError::invalid_input("pk", KYBER_1024_PUBLICKEYBYTES, pk.len()).to_string(), )); } - let mut rng = thread_rng(); + let mut rng = OsRng; let mut ct = vec![0u8; KYBER_1024_CIPHERTEXTBYTES]; - let mut ss = vec![0u8; 32]; + let mut ss = Zeroizing::new(vec![0u8; 32]); crypto_kem_enc_1024(&mut ct, &mut ss, pk, &mut rng, None) - .map(|_| (ct, ss)) - .map_err(|e| make_kyber_error(&e.to_string())) + .map(|_| (ct, SecretBytes(ss))) + .map_err(|e| legacy_err(&e)) } #[pyfunction] -fn decapsulate_key_512(ct: &[u8], sk: &[u8]) -> PyResult> { +fn decapsulate_key_512(ct: &[u8], sk: &[u8]) -> PyResult { if ct.len() != KYBER_512_CIPHERTEXTBYTES { return Err(make_kyber_error( &KyberError::invalid_input("ct", KYBER_512_CIPHERTEXTBYTES, ct.len()).to_string(), @@ -126,14 +169,14 @@ fn decapsulate_key_512(ct: &[u8], sk: &[u8]) -> PyResult> { &KyberError::invalid_input("sk", KYBER_512_SECRETKEYBYTES, sk.len()).to_string(), )); } - let mut ss = vec![0u8; 32]; + let mut ss = Zeroizing::new(vec![0u8; 32]); crypto_kem_dec_512(&mut ss, ct, sk) - .map(|_| ss) + .map(|_| SecretBytes(ss)) .map_err(|e| make_kyber_error(&e.to_string())) } #[pyfunction] -fn decapsulate_key_1024(ct: &[u8], sk: &[u8]) -> PyResult> { +fn decapsulate_key_1024(ct: &[u8], sk: &[u8]) -> PyResult { if ct.len() != KYBER_1024_CIPHERTEXTBYTES { return Err(make_kyber_error( &KyberError::invalid_input("ct", KYBER_1024_CIPHERTEXTBYTES, ct.len()).to_string(), @@ -144,9 +187,9 @@ fn decapsulate_key_1024(ct: &[u8], sk: &[u8]) -> PyResult> { &KyberError::invalid_input("sk", KYBER_1024_SECRETKEYBYTES, sk.len()).to_string(), )); } - let mut ss = vec![0u8; 32]; + let mut ss = Zeroizing::new(vec![0u8; 32]); crypto_kem_dec_1024(&mut ss, ct, sk) - .map(|_| ss) + .map(|_| SecretBytes(ss)) .map_err(|e| make_kyber_error(&e.to_string())) } @@ -162,62 +205,62 @@ mod _pykyber { }; #[pyfunction] - pub fn _generate_keypair(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { + pub fn _generate_keypair(seed: Option<&[u8]>) -> PyResult<(Vec, super::SecretBytes)> { super::generate_keypair(seed) } #[pyfunction] - pub fn _encapsulate(pk: &[u8]) -> PyResult<(Vec, Vec)> { + pub fn _encapsulate(pk: &[u8]) -> PyResult<(Vec, super::SecretBytes)> { super::encapsulate_key(pk) } #[pyfunction] - pub fn _decapsulate(ct: &[u8], sk: &[u8]) -> PyResult> { + pub fn _decapsulate(ct: &[u8], sk: &[u8]) -> PyResult { super::decapsulate_key(ct, sk) } #[pyfunction] - pub fn _keypair_512(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { + pub fn _keypair_512(seed: Option<&[u8]>) -> PyResult<(Vec, super::SecretBytes)> { super::generate_keypair_512(seed) } #[pyfunction] - pub fn _keypair_768(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { + pub fn _keypair_768(seed: Option<&[u8]>) -> PyResult<(Vec, super::SecretBytes)> { super::generate_keypair(seed) } #[pyfunction] - pub fn _keypair_1024(seed: Option<&[u8]>) -> PyResult<(Vec, Vec)> { + pub fn _keypair_1024(seed: Option<&[u8]>) -> PyResult<(Vec, super::SecretBytes)> { super::generate_keypair_1024(seed) } #[pyfunction] - pub fn _encapsulate_512(pk: &[u8]) -> PyResult<(Vec, Vec)> { + pub fn _encapsulate_512(pk: &[u8]) -> PyResult<(Vec, super::SecretBytes)> { super::encapsulate_key_512(pk) } #[pyfunction] - pub fn _encapsulate_768(pk: &[u8]) -> PyResult<(Vec, Vec)> { + pub fn _encapsulate_768(pk: &[u8]) -> PyResult<(Vec, super::SecretBytes)> { super::encapsulate_key(pk) } #[pyfunction] - pub fn _encapsulate_1024(pk: &[u8]) -> PyResult<(Vec, Vec)> { + pub fn _encapsulate_1024(pk: &[u8]) -> PyResult<(Vec, super::SecretBytes)> { super::encapsulate_key_1024(pk) } #[pyfunction] - pub fn _decapsulate_512(ct: &[u8], sk: &[u8]) -> PyResult> { + pub fn _decapsulate_512(ct: &[u8], sk: &[u8]) -> PyResult { super::decapsulate_key_512(ct, sk) } #[pyfunction] - pub fn _decapsulate_768(ct: &[u8], sk: &[u8]) -> PyResult> { + pub fn _decapsulate_768(ct: &[u8], sk: &[u8]) -> PyResult { super::decapsulate_key(ct, sk) } #[pyfunction] - pub fn _decapsulate_1024(ct: &[u8], sk: &[u8]) -> PyResult> { + pub fn _decapsulate_1024(ct: &[u8], sk: &[u8]) -> PyResult { super::decapsulate_key_1024(ct, sk) } } diff --git a/src/mlkem/ct.rs b/src/mlkem/ct.rs index 3de5e2e..6cc8321 100644 --- a/src/mlkem/ct.rs +++ b/src/mlkem/ct.rs @@ -11,7 +11,7 @@ //! accumulator or the mask is and turning the select back into a branch (the //! pattern behind CVE-2024-37880, "Clangover"). It is best effort, so the release //! assembly is checked (see the PR for WP-3 and the gate of WP-8). This is not -//! `kyber::verify`, which has no barrier and trusts a caller-supplied length. +//! `kyber::verify`, which trusts a caller-supplied length. use crate::kyber::params::KYBER_SSBYTES; use core::hint::black_box; diff --git a/src/mlkem/mod.rs b/src/mlkem/mod.rs index ab92f1b..0d04d20 100644 --- a/src/mlkem/mod.rs +++ b/src/mlkem/mod.rs @@ -51,8 +51,9 @@ //! on x86_64 or aarch64 (checked in release builds), but nothing guarantees //! that for other compilers or flags, so ML-KEM needs its own version that //! passes the bit through an optimization barrier: `compress::poly_from_msg`. -//! - `verify` / `cmov` (`verify.rs`): no optimization barrier and a -//! caller-supplied length. ML-KEM needs its own compare and select. +//! - `verify` / `cmov` (`verify.rs`): a caller-supplied length (the legacy code +//! has had an optimization barrier here since the WP-9 pass). ML-KEM keeps +//! its own compare and select. //! - `poly_getnoise_*`: eta is hard-coded per function. Call the PRF and //! `cbd2` / `cbd3` with `ParamSet::ETA1` / `ETA2` instead. //! - `gen_matrix*`, `indcpa_*`, `crypto_kem_*`, the `Polyvec{512,768,1024}` diff --git a/src/pymlkem.rs b/src/pymlkem.rs index 5b50bfb..4b859a7 100644 --- a/src/pymlkem.rs +++ b/src/pymlkem.rs @@ -31,7 +31,7 @@ fn error_class(error: &KyberError) -> &'static str { } } -fn to_py_err(py: Python<'_>, error: KyberError) -> PyErr { +pub(crate) fn to_py_err(py: Python<'_>, error: KyberError) -> PyErr { py.import(ERROR_MODULE) .and_then(|module| module.getattr(error_class(&error))) .and_then(|class| class.call1((error.to_string(),))) diff --git a/tests/legacy_decaps_rejection.rs b/tests/legacy_decaps_rejection.rs new file mode 100644 index 0000000..e59debd --- /dev/null +++ b/tests/legacy_decaps_rejection.rs @@ -0,0 +1,130 @@ +//! Implicit rejection in the legacy (round-3) decapsulation: a ciphertext that +//! does not re-encrypt to itself must give `KDF(z || H(c))` (not an error, and +//! not the real shared secret). The expected value is derived here from `z` and +//! `c` with the public hash functions, independently of the code under test, so +//! it covers `verify`, `cmov` and the rest of the decapsulation tail. + +use pykyber::kyber::fips202::{sha3_256, shake256}; +use pykyber::kyber::params::*; +use pykyber::kyber::{ + crypto_kem_dec, crypto_kem_dec_1024, crypto_kem_dec_512, crypto_kem_enc, crypto_kem_enc_1024, + crypto_kem_enc_512, crypto_kem_keypair, crypto_kem_keypair_1024, crypto_kem_keypair_512, +}; +use rand_core::{CryptoRng, Error, RngCore}; + +/// Seeded paths never draw randomness; this generator must not be asked to. +struct NoRandomness; + +impl RngCore for NoRandomness { + fn next_u32(&mut self) -> u32 { + unreachable!() + } + fn next_u64(&mut self) -> u64 { + unreachable!() + } + fn fill_bytes(&mut self, _dest: &mut [u8]) { + unreachable!() + } + fn try_fill_bytes(&mut self, _dest: &mut [u8]) -> Result<(), Error> { + unreachable!() + } +} + +impl CryptoRng for NoRandomness {} + +/// Fixed seeds `d`, `z` and `m` (arbitrary test data). +const D_SEED: [u8; KYBER_SYMBYTES] = [0x11; KYBER_SYMBYTES]; +const Z_SEED: [u8; KYBER_SYMBYTES] = [0x22; KYBER_SYMBYTES]; +const M_SEED: [u8; KYBER_SYMBYTES] = [0x33; KYBER_SYMBYTES]; + +/// `KDF(z || H(c))`, the shared secret of an implicitly rejected ciphertext. +fn rejection_secret(z: &[u8], ct: &[u8]) -> [u8; KYBER_SSBYTES] { + let mut h = [0u8; KYBER_SYMBYTES]; + sha3_256(&mut h, ct, ct.len()); + let mut input = [0u8; 2 * KYBER_SYMBYTES]; + input[..KYBER_SYMBYTES].copy_from_slice(z); + input[KYBER_SYMBYTES..].copy_from_slice(&h); + let mut ss = [0u8; KYBER_SSBYTES]; + shake256(&mut ss, KYBER_SSBYTES, &input, input.len()); + ss +} + +type Decaps = fn(&mut [u8], &[u8], &[u8]) -> Result<(), pykyber::kyber::KyberError>; + +/// Byte positions to damage: the first, the last and a few in between, +/// covering the compressed vector part and the compressed `v` part. +fn damaged_positions(ct_len: usize) -> [usize; 5] { + [0, 1, ct_len / 3, ct_len - KYBER_SYMBYTES, ct_len - 1] +} + +fn check(pk: &[u8], sk: &[u8], ct: &[u8], real_ss: &[u8], dec: Decaps) { + let z = &sk[sk.len() - KYBER_SYMBYTES..]; + + let mut ss = [0u8; KYBER_SSBYTES]; + dec(&mut ss, ct, sk).unwrap(); + assert_eq!(ss, real_ss, "valid ciphertext (pk length {})", pk.len()); + + for pos in damaged_positions(ct.len()) { + for bit in [0u8, 7u8] { + let mut bad = ct.to_vec(); + bad[pos] ^= 1 << bit; + let mut got = [0u8; KYBER_SSBYTES]; + dec(&mut got, &bad, sk).expect("implicit rejection is not an error"); + assert_ne!(got, real_ss, "byte {pos} bit {bit} gave the real secret"); + assert_eq!( + got, + rejection_secret(z, &bad), + "byte {pos} bit {bit} of a ciphertext of {} bytes", + ct.len() + ); + } + } +} + +#[test] +fn kyber_512_rejects_implicitly() { + let mut pk = vec![0u8; KYBER_512_PUBLICKEYBYTES]; + let mut sk = vec![0u8; KYBER_512_SECRETKEYBYTES]; + crypto_kem_keypair_512( + &mut pk, + &mut sk, + &mut NoRandomness, + Some((&D_SEED, &Z_SEED)), + ) + .unwrap(); + let (mut ct, mut ss) = (vec![0u8; KYBER_512_CIPHERTEXTBYTES], [0u8; KYBER_SSBYTES]); + crypto_kem_enc_512(&mut ct, &mut ss, &pk, &mut NoRandomness, Some(&M_SEED)).unwrap(); + check(&pk, &sk, &ct, &ss, crypto_kem_dec_512); +} + +#[test] +fn kyber_768_rejects_implicitly() { + let mut pk = vec![0u8; KYBER_PUBLICKEYBYTES]; + let mut sk = vec![0u8; KYBER_SECRETKEYBYTES]; + crypto_kem_keypair( + &mut pk, + &mut sk, + &mut NoRandomness, + Some((&D_SEED, &Z_SEED)), + ) + .unwrap(); + let (mut ct, mut ss) = (vec![0u8; KYBER_CIPHERTEXTBYTES], [0u8; KYBER_SSBYTES]); + crypto_kem_enc(&mut ct, &mut ss, &pk, &mut NoRandomness, Some(&M_SEED)).unwrap(); + check(&pk, &sk, &ct, &ss, crypto_kem_dec); +} + +#[test] +fn kyber_1024_rejects_implicitly() { + let mut pk = vec![0u8; KYBER_1024_PUBLICKEYBYTES]; + let mut sk = vec![0u8; KYBER_1024_SECRETKEYBYTES]; + crypto_kem_keypair_1024( + &mut pk, + &mut sk, + &mut NoRandomness, + Some((&D_SEED, &Z_SEED)), + ) + .unwrap(); + let (mut ct, mut ss) = (vec![0u8; KYBER_1024_CIPHERTEXTBYTES], [0u8; KYBER_SSBYTES]); + crypto_kem_enc_1024(&mut ct, &mut ss, &pk, &mut NoRandomness, Some(&M_SEED)).unwrap(); + check(&pk, &sk, &ct, &ss, crypto_kem_dec_1024); +} diff --git a/tests/legacy_length_checks.rs b/tests/legacy_length_checks.rs new file mode 100644 index 0000000..8405012 --- /dev/null +++ b/tests/legacy_length_checks.rs @@ -0,0 +1,108 @@ +//! The legacy keypair and encapsulation functions index caller-supplied buffers +//! and seeds, so they must reject a wrong length with `InvalidInput` instead of +//! panicking (decapsulation already did). + +use pykyber::kyber::error::KyberError; +use pykyber::kyber::params::*; +use pykyber::kyber::{ + crypto_kem_enc, crypto_kem_enc_1024, crypto_kem_enc_512, crypto_kem_keypair, + crypto_kem_keypair_1024, crypto_kem_keypair_512, derive, +}; +use rand::rngs::OsRng; + +fn is_invalid_input(result: Result, param: &str) -> bool { + matches!(result, Err(KyberError::InvalidInput { param: p, .. }) if p == param) +} + +/// `(public key length, secret key length, ciphertext length, keypair, encapsulate)` per level. +type Keypair = + fn(&mut [u8], &mut [u8], &mut OsRng, Option<(&[u8], &[u8])>) -> Result<(), KyberError>; +type Encaps = fn(&mut [u8], &mut [u8], &[u8], &mut OsRng, Option<&[u8]>) -> Result<(), KyberError>; + +fn levels() -> [(usize, usize, usize, Keypair, Encaps); 3] { + [ + ( + KYBER_512_PUBLICKEYBYTES, + KYBER_512_SECRETKEYBYTES, + KYBER_512_CIPHERTEXTBYTES, + crypto_kem_keypair_512::, + crypto_kem_enc_512::, + ), + ( + KYBER_PUBLICKEYBYTES, + KYBER_SECRETKEYBYTES, + KYBER_CIPHERTEXTBYTES, + crypto_kem_keypair::, + crypto_kem_enc::, + ), + ( + KYBER_1024_PUBLICKEYBYTES, + KYBER_1024_SECRETKEYBYTES, + KYBER_1024_CIPHERTEXTBYTES, + crypto_kem_keypair_1024::, + crypto_kem_enc_1024::, + ), + ] +} + +#[test] +fn keypair_rejects_wrong_buffer_and_seed_lengths() { + let half = [0u8; KYBER_SYMBYTES]; + for (pk_len, sk_len, _, keypair, _) in levels() { + let (mut pk, mut sk) = (vec![0u8; pk_len], vec![0u8; sk_len]); + let mut rng = OsRng; + assert!(is_invalid_input( + keypair(&mut pk[1..], &mut sk, &mut rng, None), + "pk" + )); + assert!(is_invalid_input( + keypair(&mut pk, &mut sk[1..], &mut rng, None), + "sk" + )); + assert!(is_invalid_input( + keypair(&mut pk, &mut sk, &mut rng, Some((&half[1..], &half))), + "seed d" + )); + assert!(is_invalid_input( + keypair(&mut pk, &mut sk, &mut rng, Some((&half, &[]))), + "seed z" + )); + // The right sizes still work, seeded and not. + keypair(&mut pk, &mut sk, &mut rng, Some((&half, &half))).unwrap(); + keypair(&mut pk, &mut sk, &mut rng, None).unwrap(); + } +} + +#[test] +fn encapsulate_rejects_wrong_buffer_and_seed_lengths() { + let m = [0u8; KYBER_SYMBYTES]; + for (pk_len, sk_len, ct_len, keypair, enc) in levels() { + let (mut pk, mut sk) = (vec![0u8; pk_len], vec![0u8; sk_len]); + keypair(&mut pk, &mut sk, &mut OsRng, None).unwrap(); + let (mut ct, mut ss) = (vec![0u8; ct_len], [0u8; KYBER_SSBYTES]); + let mut rng = OsRng; + assert!(is_invalid_input( + enc(&mut ct[1..], &mut ss, &pk, &mut rng, None), + "ct" + )); + assert!(is_invalid_input( + enc(&mut ct, &mut ss[1..], &pk, &mut rng, None), + "ss" + )); + assert!(is_invalid_input( + enc(&mut ct, &mut ss, &pk[1..], &mut rng, None), + "pk" + )); + assert!(is_invalid_input( + enc(&mut ct, &mut ss, &pk, &mut rng, Some(&m[1..])), + "seed" + )); + enc(&mut ct, &mut ss, &pk, &mut rng, Some(&m)).unwrap(); + } +} + +#[test] +fn derive_rejects_a_wrong_seed_length() { + assert!(is_invalid_input(derive(&[0u8; 63]).map(|_| ()), "seed")); + assert!(derive(&[0u8; 64]).is_ok()); +} diff --git a/tests/legacy_rng_failure.rs b/tests/legacy_rng_failure.rs new file mode 100644 index 0000000..e14c4c7 --- /dev/null +++ b/tests/legacy_rng_failure.rs @@ -0,0 +1,122 @@ +//! A failing random generator must surface as `KyberError::RandomBytesGeneration` +//! from every legacy keypair and encapsulation function (no panic, no fallback +//! to another source), and a failure after the IND-CPA secret key was written must +//! not leave that key in the caller's buffer. + +use pykyber::kyber::error::KyberError; +use pykyber::kyber::params::*; +use pykyber::kyber::{ + crypto_kem_enc, crypto_kem_enc_1024, crypto_kem_enc_512, crypto_kem_keypair, + crypto_kem_keypair_1024, crypto_kem_keypair_512, keypair, +}; +use rand_core::{CryptoRng, Error, RngCore}; + +/// Byte every successful fill writes. +const FILL: u8 = 0x42; +/// Keypair generation asks the generator twice: `d` first, then `z`. +const CALLS_BEFORE_Z: usize = 1; + +/// Generator that fills `ok_calls` requests and then reports a failure. +struct FailingRng { + ok_calls: usize, +} + +impl RngCore for FailingRng { + fn next_u32(&mut self) -> u32 { + panic!("the legacy code must use try_fill_bytes"); + } + fn next_u64(&mut self) -> u64 { + panic!("the legacy code must use try_fill_bytes"); + } + fn fill_bytes(&mut self, _dest: &mut [u8]) { + panic!("the legacy code must use try_fill_bytes"); + } + fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { + if self.ok_calls == 0 { + return Err(Error::new("simulated failure of the OS generator")); + } + self.ok_calls -= 1; + dest.fill(FILL); + Ok(()) + } +} + +impl CryptoRng for FailingRng {} + +fn assert_randomness_error(result: Result) { + assert_eq!(result.unwrap_err(), KyberError::RandomBytesGeneration); +} + +type KeypairFn = + fn(&mut [u8], &mut [u8], &mut FailingRng, Option<(&[u8], &[u8])>) -> Result<(), KyberError>; + +/// `(public key length, secret key length, function)` for each level. +fn keypair_levels() -> [(usize, usize, KeypairFn); 3] { + [ + ( + KYBER_512_PUBLICKEYBYTES, + KYBER_512_SECRETKEYBYTES, + crypto_kem_keypair_512::, + ), + ( + KYBER_PUBLICKEYBYTES, + KYBER_SECRETKEYBYTES, + crypto_kem_keypair::, + ), + ( + KYBER_1024_PUBLICKEYBYTES, + KYBER_1024_SECRETKEYBYTES, + crypto_kem_keypair_1024::, + ), + ] +} + +#[test] +fn keypair_fails_when_d_cannot_be_drawn() { + for (pk_len, sk_len, run) in keypair_levels() { + let (mut pk, mut sk) = (vec![0u8; pk_len], vec![0u8; sk_len]); + let mut rng = FailingRng { ok_calls: 0 }; + assert_randomness_error(run(&mut pk, &mut sk, &mut rng, None)); + } +} + +#[test] +fn keypair_scrubs_the_secret_key_when_z_cannot_be_drawn() { + for (pk_len, sk_len, run) in keypair_levels() { + let (mut pk, mut sk) = (vec![0u8; pk_len], vec![0u8; sk_len]); + let mut rng = FailingRng { + ok_calls: CALLS_BEFORE_Z, + }; + assert_randomness_error(run(&mut pk, &mut sk, &mut rng, None)); + assert!( + sk.iter().all(|&b| b == 0), + "sk length {sk_len} not scrubbed" + ); + } +} + +#[test] +fn encapsulation_fails_when_m_cannot_be_drawn() { + let mut rng = FailingRng { ok_calls: 0 }; + let mut ss = [0u8; KYBER_SSBYTES]; + + let pk = [0u8; KYBER_512_PUBLICKEYBYTES]; + let mut ct = [0u8; KYBER_512_CIPHERTEXTBYTES]; + assert_randomness_error(crypto_kem_enc_512(&mut ct, &mut ss, &pk, &mut rng, None)); + + let pk = [0u8; KYBER_PUBLICKEYBYTES]; + let mut ct = [0u8; KYBER_CIPHERTEXTBYTES]; + assert_randomness_error(crypto_kem_enc(&mut ct, &mut ss, &pk, &mut rng, None)); + + let pk = [0u8; KYBER_1024_PUBLICKEYBYTES]; + let mut ct = [0u8; KYBER_1024_CIPHERTEXTBYTES]; + assert_randomness_error(crypto_kem_enc_1024(&mut ct, &mut ss, &pk, &mut rng, None)); +} + +#[test] +fn safe_api_reports_the_failure() { + assert_randomness_error(keypair(&mut FailingRng { ok_calls: 0 })); + assert_randomness_error(keypair(&mut FailingRng { + ok_calls: CALLS_BEFORE_Z, + })); +}