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

Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_FAILURE_CALLBACK #2266

Merged
merged 2 commits into from
Mar 13, 2025
+108 −56
Merged

Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_FAILURE_CALLBACK #2266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef3db5d
Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_…
andrewhop Mar 12, 2025
55776e2
PR feedback: update documentation and make test clearer
andrewhop Mar 13, 2025
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
18 changes: 4 additions & 14 deletions crypto/fipsmodule/bcm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,6 @@ static void BORINGSSL_bcm_power_on_self_test(void) __attribute__ ((constructor))
#endif

static void BORINGSSL_bcm_power_on_self_test(void) {
#if defined(AWSLC_FIPS_FAILURE_CALLBACK)
if (AWS_LC_fips_failure_callback == NULL) {
fprintf(stderr, "AWS_LC_fips_failure_callback not defined but AWS-LC built with AWSLC_FIPS_FAILURE_CALLBACK\n");
fflush(stderr);
abort();
}
#endif
// TODO: remove !defined(OPENSSL_PPC64BE) from the check below when starting to support
// PPC64BE that has VCRYPTO capability. In that case, add `|| defined(OPENSSL_PPC64BE)`
// to `#if defined(OPENSSL_PPC64LE)` wherever it occurs.
Expand Down Expand Up @@ -408,21 +401,18 @@ int BORINGSSL_integrity_test(void) {

void AWS_LC_FIPS_failure(const char* message) {
#if defined(AWSLC_FIPS_FAILURE_CALLBACK)
if (AWS_LC_fips_failure_callback == NULL) {
fprintf(stderr, "AWS_LC_fips_failure_callback not defined but AWS-LC built with AWSLC_FIPS_FAILURE_CALLBACK. FIPS failure:\n%s", message);
fflush(stderr);
abort();
} else {
if (AWS_LC_fips_failure_callback != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update the documentation here to indicate that a failure (even in FIPS mode) might be ignored by the callback. I.e., the failure doesn't necessarily "prevent any further cryptographic operations by the current process".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, I was leaving it implied because to build AWS-LC in FIPS mode you can't specify the flag to turn the callback on. But I can add something explaining this behavior.

AWS_LC_fips_failure_callback(message);
return;
}
#else
// Fallback to the default behavior if the callback is not defined
#endif
fprintf(stderr, "AWS-LC FIPS failure caused by:\n%s\n", message);
fflush(stderr);
for (;;) {
abort();
exit(1);
}
#endif
}
#else // BORINGSSL_FIPS
void AWS_LC_FIPS_failure(const char* message) {
Expand Down
12 changes: 10 additions & 2 deletions crypto/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1270,8 +1270,16 @@ static inline uint64_t CRYPTO_subc_u64(uint64_t x, uint64_t y, uint64_t borrow,
#if defined(BORINGSSL_FIPS)

// AWS_LC_FIPS_failure is called when a FIPS power-on or continuous test
// fails. If the library is built in FIPS mode it prevents any further
// cryptographic operations by the current process.
// fails. The behavior depends on how AWS-LC is built:
// - When AWS-LC is not in FIPS mode it prints |message| to |stderr|.
// - If AWS-LC is built with FIPS it prints |message| to |stderr| and prevents
// any further cryptographic operations by the current process.
// - If AWS-LC is built with FIPS, AWSLC_FIPS_FAILURE_CALLBACK, and the
// application does not define the AWS_LC_fips_failure_callback function
// the normal behavior FIPS behavior is used.
// - If AWS-LC is built with FIPS, AWSLC_FIPS_FAILURE_CALLBACK, and the
// application defines the AWS_LC_fips_failure_callback function that
// function is called with |message|.
#if defined(AWSLC_FIPS_FAILURE_CALLBACK)
void AWS_LC_FIPS_failure(const char* message);
#else
Expand Down
11 changes: 11 additions & 0 deletions tests/ci/cdk/cdk/codebuild/github_ci_linux_x86_omnibus.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,3 +627,14 @@ batch:
image: 620771051181.dkr.ecr.us-west-2.amazonaws.com/aws-lc-docker-images-linux-x86:ubuntu-10.04_gcc-4.1x_latest
variables:
AWS_LC_CI_TARGET: "tests/ci/run_legacy_build.sh"

- identifier: amazonlinux2_gcc7x_x86_64_fips_callback
buildspec: ./tests/ci/codebuild/common/run_simple_target.yml
env:
type: LINUX_CONTAINER
privileged-mode: true
compute-type: BUILD_GENERAL1_LARGE
image: 620771051181.dkr.ecr.us-west-2.amazonaws.com/aws-lc-docker-images-linux-x86:amazonlinux-2_gcc-7x_latest
variables:
AWS_LC_CI_TARGET: "tests/ci/run_fips_callback_tests.sh"

106 changes: 83 additions & 23 deletions tests/ci/run_fips_callback_tests.sh
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear to me which path in the test is for the case when the build flag AWSLC_FIPS_FAILURE_CALLBACK is set but the callback is undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The callback is only defined in crypto_test, so ssl_test, test_fips, and bssl don't have the callback defined. This is why I was only running crypto_test before: the other tests/executables didn't have the callback defined and if you tried to call them they would abort because the callback was previosuly required.

Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,86 @@ set -ex
# SPDX-License-Identifier: Apache-2.0 OR ISC
source tests/ci/common_posix_setup.sh

original_test="${BUILD_ROOT}/crypto/crypto_test"
broken_test="${BUILD_ROOT}/crypto/crypto_test_broken"

# By default the test should pass
$original_test --gtest_filter=FIPSCallback.PowerOnSelfTests
$original_test --gtest_filter=FIPSCallback.PWCT

# Break the tests
KATS=$(go run "${SRC_ROOT}/util/fipstools/break-kat.go" --list-tests)
for kat in $KATS; do
go run "${SRC_ROOT}/util/fipstools/break-kat.go" "$original_test" "$kat" > "$broken_test"
chmod +x "$broken_test"
export FIPS_CALLBACK_TEST_EXPECTED_FAILURE="$kat"
# When a callback is defined AWS-LC will not abort and the test should exit successfully
$broken_test --gtest_filter=FIPSCallback.PowerOnSelfTests
unset FIPS_CALLBACK_TEST_EXPECTED_FAILURE
done

for TEST in RSA_PWCT ECDSA_PWCT EDDSA_PWCT MLKEM_PWCT MLDSA_PWCT; do
export FIPS_CALLBACK_TEST_EXPECTED_FAILURE="${TEST}"
export BORINGSSL_FIPS_BREAK_TEST="${TEST}"
$original_test --gtest_filter=FIPSCallback.PWCT
done
# This test file is designed to replicate the internal FIPS callback build defined in the build-glue package

# This should follow AWS-LC-Build-GLue/bin/fips_tests.sh maybe_run_fips_tests
function maybe_run_fips_tests() {
expect_fips_mode=1
module_status=$("${BUILD_ROOT}/tool/bssl" isfips)
[[ "${expect_fips_mode}" == "${module_status}" ]] || {
echo >&2 "FIPS Mode validation failed."
exit 1
}
# Mainline AWS-LC does not have the CAVP tests anymore so only run the test_fips branch
"${BUILD_ROOT}/util/fipstools/test_fips"
}

# This should follow AWS-LC-Build-GLue/bin/fips_tests.sh maybe_run_fips_break_tests
function maybe_run_fips_break_tests() {
break_kat_executable="${BUILD_ROOT}/break-kat"
pushd "${SRC_ROOT}"
go build -o "$break_kat_executable" "./util/fipstools/break-kat.go"
"$break_kat_executable" -list-tests

working_bssl="${BUILD_ROOT}/tool/bssl"
broken_bssl="${BUILD_ROOT}/tool/brokenbssl"
"$working_bssl" isfips

# This breaks a local copy of bssl that will not be included in the build artifacts
"$break_kat_executable" "$working_bssl" DRBG >"$broken_bssl"
chmod +x "$broken_bssl"
if ! ("$broken_bssl" isfips 2>&1 >/dev/null || true) |
grep -q "DRBG"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should you do a $working_bssl | grep -q DRBG && /bin/false to assert that breaking bssl is the difference maker on the string "DRBG" being output?

echo "Broken bssl did not mention DRBG failure in startup"
exit 1
fi
popd
}

function run_all_break_tests() {
original_test="${BUILD_ROOT}/crypto/crypto_test"
broken_test="${BUILD_ROOT}/crypto/crypto_test_broken"

# By default the test should pass
$original_test --gtest_filter=FIPSCallback.PowerOnSelfTests
$original_test --gtest_filter=FIPSCallback.PWCT

# Break the tests
KATS=$(go run "${SRC_ROOT}/util/fipstools/break-kat.go" --list-tests)
for kat in $KATS; do
go run "${SRC_ROOT}/util/fipstools/break-kat.go" "$original_test" "$kat" > "$broken_test"
chmod +x "$broken_test"
export FIPS_CALLBACK_TEST_EXPECTED_FAILURE="$kat"
# When a callback is defined AWS-LC will not abort and the test should exit successfully
$broken_test --gtest_filter=FIPSCallback.PowerOnSelfTests
unset FIPS_CALLBACK_TEST_EXPECTED_FAILURE
done

for TEST in RSA_PWCT ECDSA_PWCT EDDSA_PWCT MLKEM_PWCT MLDSA_PWCT; do
export FIPS_CALLBACK_TEST_EXPECTED_FAILURE="${TEST}"
export BORINGSSL_FIPS_BREAK_TEST="${TEST}"
$original_test --gtest_filter=FIPSCallback.PWCT
unset FIPS_CALLBACK_TEST_EXPECTED_FAILURE
unset BORINGSSL_FIPS_BREAK_TEST
done

}

echo "Testing AWS-LC static breakable build with custom callback and Jitter enabled"
build_and_test -DFIPS=1 \
-DCMAKE_C_FLAGS="-DBORINGSSL_FIPS_BREAK_TESTS -DAWSLC_FIPS_FAILURE_CALLBACK" \
-DCMAKE_CXX_FLAGS="-DAWSLC_FIPS_FAILURE_CALLBACK" \
-DENABLE_FIPS_ENTROPY_CPU_JITTER=1

maybe_run_fips_tests
maybe_run_fips_break_tests
run_all_break_tests

echo "Testing AWS-LC static build with custom callback and Jitter enabled"
build_and_test -DFIPS=1 \
-DCMAKE_C_FLAGS="-DAWSLC_FIPS_FAILURE_CALLBACK" \
-DCMAKE_CXX_FLAGS="-DAWSLC_FIPS_FAILURE_CALLBACK" \
-DENABLE_FIPS_ENTROPY_CPU_JITTER=1

maybe_run_fips_tests
# Can't run maybe_run_fips_break_tests or run_all_break_tests since they require BORINGSSL_FIPS_BREAK_TESTS
7 changes: 0 additions & 7 deletions tests/ci/run_fips_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ if static_linux_supported || static_openbsd_supported; then
echo "Testing AWS-LC static library in FIPS Release mode."
fips_build_and_test -DCMAKE_BUILD_TYPE=Release

echo "Testing AWS-LC static breakable build with custom callback enabled"
run_build -DFIPS=1 \
-DCMAKE_C_FLAGS="-DBORINGSSL_FIPS_BREAK_TESTS -DAWSLC_FIPS_FAILURE_CALLBACK" \
-DCMAKE_CXX_FLAGS="-DAWSLC_FIPS_FAILURE_CALLBACK"
shard_gtest ./test_build_dir/crypto/crypto_test
./tests/ci/run_fips_callback_tests.sh

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
Expand Down
10 changes: 0 additions & 10 deletions tool/tool.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,6 @@

#include "internal.h"

#if defined(AWSLC_FIPS_FAILURE_CALLBACK)
extern "C" {
void AWS_LC_fips_failure_callback(const char* message);
}

void AWS_LC_fips_failure_callback(const char* message) {
fprintf(stderr, "FIPS failure:\n%s", message);
}
#endif

static bool version(const std::vector<std::string> &args) {
printf("%s\n", AWSLC_VERSION_NUMBER_STRING);
return true;
Expand Down
Loading