Skip to content

Abort on RAND_bytes failure#3078

Merged
justsmth merged 2 commits intoaws:mainfrom
justsmth:AWSLC_ABORT_IF_NOT_ONE
Mar 13, 2026
Merged

Abort on RAND_bytes failure#3078
justsmth merged 2 commits intoaws:mainfrom
justsmth:AWSLC_ABORT_IF_NOT_ONE

Conversation

@justsmth
Copy link
Contributor

@justsmth justsmth commented Mar 6, 2026

Description of changes:

RAND_bytes in AWS-LC always returns 1 and cannot actually fail. However, call sites across the codebase handle its return value inconsistently:

  1. Ignored entirely — bare RAND_bytes(...) calls with no check at all.
  2. Error propagatedif (!RAND_bytes(...)) { goto err; } patterns that
    attempt graceful recovery.
  3. Abort on failureif (RAND_bytes(...) != 1) { abort(); } (only one
    call site).

For a cryptographic library, silently continuing with a failed RNG is never the right choice. This change introduces an AWSLC_ABORT_IF_NOT_ONE macro and applies it uniformly to all RAND_bytes call sites within crypto/. If RAND_bytes ever returns a non-1 value in the future, the process will abort immediately rather than operating with potentially uninitialized randomness.

Call-outs:

  • RAND_bytes calls in ssl/ and test files are not converted in this change.
  • The ml_dsa.c diff includes some incidental trailing-whitespace cleanup on blank lines.
  • In PKCS12_create (pkcs8_x509.c), the RAND_bytes call is now evaluated before CBB_flush rather than being short-circuited behind it. This is benign since RAND_bytes has no dependency on CBB state.

Testing:

Existing tests. No behavioral change under normal operation since RAND_bytes always returns 1 today.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

@@ -157,7 +157,7 @@ int BN_rand(BIGNUM *rnd, int bits, int top, int bottom) {
// |RAND_bytes| calls within the fipsmodule should be wrapped with state lock
// functions to avoid updating the service indicator with the DRBG functions.
FIPS_service_indicator_lock_state();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: call to undeclared function 'FIPS_service_indicator_lock_state'; ISO C99 and later do not support implicit function declarations [clang-diagnostic-implicit-function-declaration]

  FIPS_service_indicator_lock_state();
  ^

FIPS_service_indicator_lock_state();
RAND_bytes((uint8_t *)rnd->d, words * sizeof(BN_ULONG));
AWSLC_ABORT_IF_NOT_ONE(RAND_bytes((uint8_t *)rnd->d, words * sizeof(BN_ULONG)));
FIPS_service_indicator_unlock_state();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: call to undeclared function 'FIPS_service_indicator_unlock_state'; ISO C99 and later do not support implicit function declarations [clang-diagnostic-implicit-function-declaration]

  FIPS_service_indicator_unlock_state();
  ^

#include <openssl/rand.h>
static MLK_INLINE void mlk_randombytes(void *ptr, size_t len) {
RAND_bytes(ptr, len);
AWSLC_ABORT_IF_NOT_ONE(RAND_bytes(ptr, len));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no matching function for call to 'RAND_bytes' [clang-diagnostic-error]

    AWSLC_ABORT_IF_NOT_ONE(RAND_bytes(ptr, len));
                           ^
Additional context

crypto/internal.h:1459: expanded from macro 'AWSLC_ABORT_IF_NOT_ONE'

x), abort())
^

crypto/internal.h:1431: expanded from macro '__AWS_LC_ENSURE'

            \
                            ^

include/openssl/rand.h:31: candidate function not viable: cannot convert argument of incomplete type 'void *' to 'uint8_t *' (aka 'unsigned char *') for 1st argument

OPENSSL_EXPORT int RAND_bytes(uint8_t *buf, size_t len);
                   ^

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.38%. Comparing base (092453f) to head (d7df5ee).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
crypto/hrss/hrss.c 0.00% 3 Missing ⚠️
crypto/rand_extra/rand_extra.c 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3078      +/-   ##
==========================================
+ Coverage   78.36%   78.38%   +0.01%     
==========================================
  Files         689      689              
  Lines      121144   121129      -15     
  Branches    16973    16964       -9     
==========================================
+ Hits        94935    94947      +12     
+ Misses      25314    25287      -27     
  Partials      895      895              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@justsmth justsmth requested a review from nebeid March 9, 2026 13:22
@justsmth justsmth requested a review from nebeid March 13, 2026 12:26
@justsmth justsmth merged commit a0589d2 into aws:main Mar 13, 2026
448 of 455 checks passed
@justsmth justsmth deleted the AWSLC_ABORT_IF_NOT_ONE branch March 13, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants