Repository navigation
fix: isolate reference implementation RNGs - #31
Draft
exocognosis wants to merge 1 commit into
Draft
exocognosis wants to merge 1 commit into
exocognosis wants to merge 1 commit into
Conversation
cryptoquick
approved these changes
Aug 1, 2026
cryptoquick
left a comment
Owner
There was a problem hiding this comment.
While I wasn't really wanting to modify the reference implementations here, I appreciate this work, and I will also be working to reimplement libbitcoinpqc entirely in Systems Lean once I have that work finished.
Much appreciated.
Author
|
No problem. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Isolate every randomness path used by the production libbitcoinpqc targets and make the entropy contract explicit at the reference implementation boundary.
This change:
randombytes()adapter, its global entropy state, its buffer-cycling behavior, and its/dev/urandomfallbackoptrandSLH-DSA signing entry pointrandombytes()hookRoot cause
The initial entropy refactor centralized the two reference implementations behind
src/randombytes_custom.c. That adapter allowed the public API to inject caller-provided bytes, but it also retained hidden process-global state and silently read/dev/urandomwhenever that state was absent.This created several problems:
/dev/urandomproduced zero-filled output while the voidrandombytes()interface could not report the failureThe existing user entropy documentation described the intended public behavior, but the build did not structurally enforce that behavior.
Proposed solution
The CMake library targets now define
LIBBITCOINPQC_EXPLICIT_ENTROPY. This mode compiles only reference implementation entry points whose randomness inputs are explicit.aux_rand32 = NULLcrypto_sign_seed_keypair()crypto_sign_seed_keypair()optrandThe default standalone behavior of the vendored Dilithium and SPHINCS+ trees remains available when
LIBBITCOINPQC_EXPLICIT_ENTROPYis not defined. Their originalrandombytes.cand NIST KAT RNG files remain in the source tree for upstream Makefiles and test-vector tooling, but they are not part of the production CMake targets.The public
bitcoin_pqc_*API is unchanged. Existing ML-DSA and SLH-DSA key and signature golden vectors remain unchanged.Regression coverage
The new
rng_isolationtest provides its own trap implementation ofrandombytes()and exercises key generation, signing, and verification for secp256k1 Schnorr, ML-DSA-44, and SLH-DSA-SHA2-128s. The test fails if any public algorithm path calls the legacy hook.The entropy documentation now records:
Impact
Verification
randombytes,/dev/urandom,getrandom, orarc4randomdependencygit diff --checkValidation notes
-Wall -Wextra -Werror.mem_eq_hexhelper intests/secp256k1_schnorr_test.c. That file is outside this change.nixandjustare not installed. The host CMake path used by the non-Nix CI job passed.Fixes #12