Skip to content
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.

Sign up for GitHub

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

Update pairwise consistency test failures to support gracefully continiung #2201

Merged
merged 1 commit into from
Feb 19, 2025
+5 −6
Merged

Update pairwise consistency test failures to support gracefully continiung #2201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions crypto/fipsmodule/ec/ec_key.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,9 +550,8 @@ int EC_KEY_generate_key_fips(EC_KEY *eckey) {

#if defined(AWSLC_FIPS)
AWS_LC_FIPS_failure("EC keygen checks failed");
#else
return 0;
#endif
return 0;
Comment on lines -553 to +554
Copy link
Member

@skmcgrail skmcgrail Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Does this affect any unreachable-code analysis? As the return 0; could be seen as unreachable due to AWS_LC_FIPS_failure triggering an abort?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In theory yes, but nothing seems to be catching the issue. Looks like GCC removed support for the check a while ago https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82100#c1, Clang has an option but we don't have it turned on https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code.

I enabled that option for clang and it correctly flags the exit(1) is unreachable after the abort. But it doesn't catch this new unreachable code. Reading the bug reports it looks like detecting unreachable code is a hard problem and very dependent on compiler versions and optimizations.

Debug build:

bcm.c:398:5: error: code will never be executed [-Werror,-Wunreachable-code]
  398 |     exit(1);
      |     ^~~~

Release build didn't find anything else with Clang.

}

int EC_KEY_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
Expand Down
7 changes: 4 additions & 3 deletions crypto/fipsmodule/ml_dsa/ml_dsa_ref/sign.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ static int ml_dsa_keypair_pct(ml_dsa_params *params,
* array of CRYPTO_SECRETKEYBYTES bytes)
* - const uint8_t *rnd: pointer to random seed
*
* Returns 0 (success)
* Returns 0 (success) -1 on failure or abort depending on FIPS mode
**************************************************/
int ml_dsa_keypair_internal(ml_dsa_params *params,
uint8_t *pk,
Expand Down Expand Up @@ -114,6 +114,7 @@ int ml_dsa_keypair_internal(ml_dsa_params *params,
// Abort in case of PCT failure.
if (!ml_dsa_keypair_pct(params, pk, sk)) {
AWS_LC_FIPS_failure("ML-DSA keygen PCT failed");
return -1;
}
#endif
return 0;
Expand All @@ -138,9 +139,9 @@ int ml_dsa_keypair(ml_dsa_params *params, uint8_t *pk, uint8_t *sk) {
if (!RAND_bytes(seed, ML_DSA_SEEDBYTES)) {
return -1;
}
ml_dsa_keypair_internal(params, pk, sk, seed);
int result = ml_dsa_keypair_internal(params, pk, sk, seed);
OPENSSL_cleanse(seed, sizeof(seed));
return 0;
return result;
}

/*************************************************
Expand Down
1 change: 0 additions & 1 deletion crypto/fipsmodule/ml_kem/ml_kem_ref/kem.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ int crypto_kem_keypair_derand(ml_kem_params *params,
memcpy(sk+params->secret_key_bytes-KYBER_SYMBYTES, coins+KYBER_SYMBYTES, KYBER_SYMBYTES);

#if defined(AWSLC_FIPS)
// Abort in case of PCT failure.
if (keygen_pct(params, pk, sk)) {
return -1;
}
Expand Down
Loading