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.

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

Add runtime options to break the pairwise consistency test for Ed, ML-KEM, and ML-DSA #2192

Merged
merged 1 commit into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 0 deletions crypto/fipsmodule/FIPS.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ Some FIPS tests cannot be broken by replacing a known string in the binary. For

1. `RSA_PWCT`
2. `ECDSA_PWCT`
3. `EDDSA_PWCT`
4. `MLKEM_PWCT`
5. `MLDSA_PWCT`

## Running ACVP tests

Expand Down
11 changes: 9 additions & 2 deletions crypto/fipsmodule/curve25519/curve25519.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,15 @@ static void ed25519_keypair_pct(uint8_t public_key[ED25519_PUBLIC_KEY_LEN],
#if defined(AWSLC_FIPS)
uint8_t msg[16] = {16};
uint8_t out_sig[ED25519_SIGNATURE_LEN];
if (ED25519_sign_no_self_test(out_sig, msg, 16, private_key) != 1 ||
ED25519_verify_no_self_test(msg, 16, out_sig, public_key) != 1) {
if (ED25519_sign_no_self_test(out_sig, msg, 16, private_key) != 1) {
// This should never happen and static analysis will say that ED25519_sign_no_self_test
// always returns 1
AWS_LC_FIPS_failure("Ed25519 keygen PCT failed");
}
if (boringssl_fips_break_test("EDDSA_PWCT")) {
msg[0] = ~msg[0];
}
if (ED25519_verify_no_self_test(msg, 16, out_sig, public_key) != 1) {
AWS_LC_FIPS_failure("Ed25519 keygen PCT failed");
}
#endif
Expand Down
9 changes: 6 additions & 3 deletions crypto/fipsmodule/ml_dsa/ml_dsa_ref/sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@
static int ml_dsa_keypair_pct(ml_dsa_params *params,
uint8_t *pk,
uint8_t *sk) {
uint8_t message[1] = {0};
uint8_t signature[MLDSA87_SIGNATURE_BYTES];
uint8_t empty_msg[1] = {0};
int ret = ml_dsa_sign(params, signature, &params->bytes, empty_msg, 0, NULL, 0, sk);
int ret = ml_dsa_sign(params, signature, &params->bytes, message, sizeof(message), NULL, 0, sk);
if (ret < 0) {
return 0;
}
return ml_dsa_verify(params, signature, params->bytes, empty_msg, 0, NULL, 0, pk) == 0;
if (boringssl_fips_break_test("MLDSA_PWCT")) {
message[0] = ~message[0];
}
return ml_dsa_verify(params, signature, params->bytes, message, sizeof(message), NULL, 0, pk) == 0;
}
#endif

Expand Down
4 changes: 4 additions & 0 deletions crypto/fipsmodule/ml_kem/ml_kem_ref/kem.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ static int keygen_pct(ml_kem_params *params, const uint8_t *ek, const uint8_t *d
crypto_kem_enc(params, ct, ss_enc, ek);
crypto_kem_dec(params, ss_dec, ct, dk);

if (boringssl_fips_break_test("MLKEM_PWCT")) {
ss_enc[0] = ~ss_enc[0];
}

return verify(ss_enc, ss_dec, KYBER_SSBYTES);
}
#endif
Expand Down
1 change: 1 addition & 0 deletions tests/ci/run_fips_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ if static_linux_supported || static_openbsd_supported; then
echo "Testing AWS-LC static breakable release build"
run_build -DFIPS=1 -DCMAKE_C_FLAGS="-DBORINGSSL_FIPS_BREAK_TESTS"
./util/fipstools/test-break-kat.sh
./util/fipstools/test-runtime-pwct.sh
export BORINGSSL_FIPS_BREAK_TEST="RSA_PWCT"
${BUILD_ROOT}/crypto/crypto_test --gtest_filter="RSADeathTest.KeygenFailAndDie"
unset BORINGSSL_FIPS_BREAK_TEST
Expand Down
48 changes: 48 additions & 0 deletions util/fipstools/test-runtime-pwct.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0 OR ISC

# This script attempts to break each of the key generation pair wise consistency tests and checks that doing so
# seems to work and at least mentions the correct KAT in the output.

set -x
set -e

TEST_FIPS_BIN="test_build_dir/util/fipstools/test_fips"

if [ ! -f $TEST_FIPS_BIN ]; then
echo "$TEST_FIPS_BIN is missing. Run this script from the top level of a"
echo "BoringSSL checkout and ensure that ./build-fips-break-test-binaries.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
echo "BoringSSL checkout and ensure that ./build-fips-break-test-binaries.sh"
echo "AWS-LC checkout and ensure that ./build-fips-break-test-binaries.sh"

?

echo "has been run first."
exit 1
fi

check_test_output() {
local test_name="$1"
local output="$2"
case "$test_name" in
"ECDSA_PWCT") expected="EC keygen checks failed" ;;
"RSA_PWCT") expected="RSA keygen checks failed" ;;
"MLKEM_PWCT") expected="ML-KEM keygen PCT failed" ;;
"MLDSA_PWCT") expected="ML-DSA keygen PCT failed" ;;
"EDDSA_PWCT") expected="Ed25519 keygen PCT failed" ;;
*) echo "Unknown test: $test_name"; return 1 ;;
esac

if ! echo "$output" | grep -q "$expected"; then
echo "Failure for ${test_name} did not contain expected message: '${expected}'"
echo "Actual output was: '${output}'"
return 1
fi
return 0
}

for runtime_test in ECDSA_PWCT RSA_PWCT EDDSA_PWCT MLKEM_PWCT MLDSA_PWCT; do
output=$(2>&1 BORINGSSL_FIPS_BREAK_TEST=$runtime_test $TEST_FIPS_BIN 2>&1 >/dev/null || true)
echo $output
if ! check_test_output "$runtime_test" "$output"; then
exit 1
fi
done

echo "All tests broken as expected"

32 changes: 24 additions & 8 deletions util/fipstools/test_fips.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

#include "../../crypto/fipsmodule/evp/internal.h"
#include "../../crypto/fipsmodule/kem/internal.h"
#include "../../crypto/fipsmodule/pqdsa/internal.h"
Copy link
Member

Choose a reason for hiding this comment

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

Broader question, what is the purpose of test_fips.c. My recollection is that it is an abomination of a file with a very large main function, and I'm pretty sure doesn't include support for everything that is FIPS approved. Honestly it feels a bit duplicative to what should be in crypto_test?

Copy link
Member

Choose a reason for hiding this comment

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

For example I'm pretty sure this doesn't have ed25519ph tested :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, test_fips is a big file that turns into an executable with some of the algorithms. I think this file was used by BoringSSL to demonstrate certain things to their lab. We now use it in our CI to run/verify the break tests work as expected. In this case I needed to add algorithms to it to trigger the lazy self tests.

The break-tests could use crypto_test, but it's a little hard to write tests for the FIPS failure abort cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't need ed25519ph because this change is only adding support to break the pairwise consistency tests and all the Ed stuff uses the same keygen function. However I did need to add the the pre-hash changes to test breaking the self test and that's already checked in https://github.com/aws/aws-lc/blob/main/util/fipstools/test_fips.c#L416-L430.

#include "../../crypto/fipsmodule/rand/internal.h"
#include "../../crypto/internal.h"

Expand Down Expand Up @@ -431,18 +432,33 @@ int main(int argc, char **argv) {

/* ML-KEM */
printf("About to Generate ML-KEM key\n");
EVP_PKEY *raw = NULL;
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_KEM, NULL);
if (ctx == NULL || !EVP_PKEY_CTX_kem_set_params(ctx, NID_MLKEM512) ||
!EVP_PKEY_keygen_init(ctx) ||
!EVP_PKEY_keygen(ctx, &raw)) {
EVP_PKEY *kem_raw = NULL;
EVP_PKEY_CTX *kem_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_KEM, NULL);
if (kem_ctx == NULL || !EVP_PKEY_CTX_kem_set_params(kem_ctx, NID_MLKEM512) ||
!EVP_PKEY_keygen_init(kem_ctx) ||
!EVP_PKEY_keygen(kem_ctx, &kem_raw)) {
printf("ML-KEM keygen failed.\n");
goto err;
}
printf("Generated public key: ");
hexdump(raw->pkey.kem_key->public_key, raw->pkey.kem_key->kem->public_key_len);
EVP_PKEY_free(raw);
EVP_PKEY_CTX_free(ctx);
hexdump(kem_raw->pkey.kem_key->public_key, kem_raw->pkey.kem_key->kem->public_key_len);
EVP_PKEY_free(kem_raw);
EVP_PKEY_CTX_free(kem_ctx);

/* ML-DSA */
printf("About to Generate ML-DSA key\n");
EVP_PKEY *dsa_raw = NULL;
EVP_PKEY_CTX *dsa_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_PQDSA, NULL);
if (dsa_ctx == NULL || !EVP_PKEY_CTX_pqdsa_set_params(dsa_ctx, NID_MLDSA44) ||
!EVP_PKEY_keygen_init(dsa_ctx) ||
!EVP_PKEY_keygen(dsa_ctx, &dsa_raw)) {
printf("ML-DSA keygen failed.\n");
goto err;
}
printf("Generated public key: ");
hexdump(dsa_raw->pkey.pqdsa_key->public_key, dsa_raw->pkey.pqdsa_key->pqdsa->public_key_len);
EVP_PKEY_free(dsa_raw);
EVP_PKEY_CTX_free(dsa_ctx);

/* DBRG */
CTR_DRBG_STATE drbg;
Expand Down
Loading