-
Notifications
You must be signed in to change notification settings - Fork 41
API: add failure mode support for randombytes() #1331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
L-series
commented
Dec 1, 2025
- API: add failure mode support for randombytes()
- autogen: run to update randombytes declaration
- randombytes: add example to test failure
7733e4f to
6bbabfa
Compare
|
@L-series Do you need help debugging the CBMC and AWS-LC failures in CI, or are you fine investigating this? |
e42d879 to
226364d
Compare
|
Hi @hanno-becker, the CBMC proofs are fixed and the integration should now be on par with mldsa, however im still seeing an issue with the aws-lc tests after writing a post_import.patch file. Please let me know if you have any idea why the tests re failing. |
hanno-becker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AWS-LC tests likely fail because RAND_bytes has a different error code convention.
5389be5 to
8d9e8bb
Compare
CBMC Results (ML-KEM-512)Full Results (139 proofs)
|
CBMC Results (ML-KEM-768)
Full Results (139 proofs)
|
CBMC Results (ML-KEM-1024)
Full Results (139 proofs)
|
6d5ceed to
efec853
Compare
|
@L-series could you also incorporate pq-code-package/mldsa-native#864 ? |
Change randombytes() to return int (0 on success, non-zero on failure) instead of void, allowing callers to detect and handle RNG failures. This commit: * Updates function signatures. * All call sites to check return values. * Changes test files to use CHECK macro. * Adds documentation of the new failure mode to sign.h and mlkem_native.h * Adds a new error code MLK_ERR_RNG_FAIL. * Declares src/randombytes with MLK_MUST_CHECK_RETURN_VALUE. Signed-off-by: Andreas Hatziiliou <[email protected]>
Tests that crypto_kem_enc and crypto_kem_keypair, correctly return MLD_ERR_RNG_FAIL when randombytes() fails. We systematically inject failures at each invocation point. This test is based off the work from the test_alloc implementation. Signed-off-by: Andreas Hatziiliou <[email protected]>
Done! |
Add the rng failure test to the CI. Signed-off-by: Andreas Hatziiliou <[email protected]>
| extra_env: 'ASAN_OPTIONS=detect_leaks=1' | ||
| examples: false # Some examples use a custom config themselves | ||
| alloc: false # Requires custom config | ||
| rng_fail: false # Requires custom config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not true anymore?
| examples: false | ||
| stack: false | ||
| alloc: ${{ matrix.target.alloc }} | ||
| rng_fail: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be set to true if we incorporate pq-code-package/mldsa-native#900
| # Special rule for test_rng_fail - link against rng_fail libraries with custom randombytes config | ||
| define ADD_SOURCE_RNG_FAIL | ||
| $(BUILD_DIR)/$(1)/bin/test_rng_fail$(subst mlkem,,$(1)): LDLIBS += -L$(BUILD_DIR) -l$(1) | ||
| $(BUILD_DIR)/$(1)/bin/test_rng_fail$(subst mlkem,,$(1)): $(BUILD_DIR)/$(1)/test/src/test_rng_fail.c.o $(BUILD_DIR)/lib$(1).a | ||
| endef | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you follow pq-code-package/mldsa-native#900 here?
hanno-becker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @L-series!
The rng_fail test no longer requires a custom config so can be run in more contexts.
We also need to include pq-code-package/mldsa-native#900 which enables rng_fail for baremetal contexts, and should add it to the baremetal CI.