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

test: add minimal openssl-3.0-fips test #5081

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
34 changes: 34 additions & 0 deletions codebuild/spec/buildspec_openssl3fips.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"). You may not use
# this file except in compliance with the License. A copy of the License is
# located at
#
# http://aws.amazon.com/apache2.0/
#
# or in the "license" file accompanying this file. This file is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied. See the License for the specific language governing permissions and
# limitations under the License.

version: 0.2

phases:
build:
on-failure: ABORT
commands:
- |
cmake . -Bbuild \
-DCMAKE_PREFIX_PATH=/usr/local/openssl-3.0-fips \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DASAN=ON \
-DUBSAN=ON
- cmake --build ./build -- -j $(nproc)
post_build:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for separating build from test... we'll need to be more pedantic about this going forward...

on-failure: ABORT
commands:
- export CTEST_OUTPUT_ON_FAILURE=1
- export CTEST_PARALLEL_LEVEL=$(nproc)
# openssl3fips is still a work-in-progress. Not all tests pass.
- make -C build test -- ARGS="-R 's2n_build_test|s2n_fips_test'"
Comment on lines +33 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea on the strategy to review this! I was worried we would need a feature branch or something.

17 changes: 16 additions & 1 deletion crypto/s2n_fips.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,31 @@ bool s2n_libcrypto_is_fips(void)
if (FIPS_mode() == 1) {
return true;
}
#elif S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)
return EVP_default_properties_is_fips_enabled(NULL);
#endif
return false;
}

int s2n_fips_init(void)
{
s2n_fips_mode_enabled = s2n_libcrypto_is_fips();
#if defined(OPENSSL_FIPS)

/* When using Openssl, ONLY 3.0 currently supports FIPS.
* openssl-1.0.2-fips is no longer supported.
* openssl > 3.0 will likely have a FIPS 140-3 certificate instead of a
* FIPS 140-2 certificate, which will require additional review in order
* to properly integrate.
*/
#if defined(OPENSSL_FIPS) || S2N_OPENSSL_VERSION_AT_LEAST(3, 1, 0)
POSIX_ENSURE(!s2n_fips_mode_enabled, S2N_ERR_FIPS_MODE_UNSUPPORTED);
#endif

/* For now, openssl is only supported for testing */
if (s2n_libcrypto_is_openssl_fips()) {
POSIX_ENSURE(s2n_in_unit_test(), S2N_ERR_FIPS_MODE_UNSUPPORTED);
}

return S2N_SUCCESS;
}

Expand Down
10 changes: 10 additions & 0 deletions crypto/s2n_libcrypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ bool s2n_libcrypto_is_openssl(void)
return !is_other_libcrypto_variant;
}

bool s2n_libcrypto_is_openssl_fips(void)
{
return s2n_libcrypto_is_openssl() && s2n_is_in_fips_mode();
}

bool s2n_libcrypto_is_awslc()
{
#if defined(OPENSSL_IS_AWSLC)
Expand All @@ -134,6 +139,11 @@ bool s2n_libcrypto_is_awslc()
#endif
}

bool s2n_libcrypto_is_awslc_fips(void)
{
return s2n_libcrypto_is_awslc() && s2n_is_in_fips_mode();
}

uint64_t s2n_libcrypto_awslc_api_version(void)
{
#if defined(OPENSSL_IS_AWSLC)
Expand Down
2 changes: 2 additions & 0 deletions crypto/s2n_openssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
#endif

bool s2n_libcrypto_is_openssl(void);
bool s2n_libcrypto_is_openssl_fips(void);
bool s2n_libcrypto_is_awslc();
bool s2n_libcrypto_is_awslc_fips(void);
Comment on lines +54 to +56
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These methods are just for convenience, but since we generally talk about the fips version of libcryptos like they're separate libcryptos, I think they're helpful and simplify common conditionals.

bool s2n_libcrypto_is_boringssl();
bool s2n_libcrypto_is_libressl();
43 changes: 21 additions & 22 deletions tests/unit/s2n_build_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ S2N_RESULT s2n_check_supported_libcrypto(const char *s2n_libcrypto)
const struct {
const char *libcrypto;
bool is_openssl;
bool is_opensslfips;
} supported_libcrypto[] = {
{ .libcrypto = "awslc", .is_openssl = false },
{ .libcrypto = "awslc-fips", .is_openssl = false },
Expand All @@ -82,24 +83,24 @@ S2N_RESULT s2n_check_supported_libcrypto(const char *s2n_libcrypto)
{ .libcrypto = "openssl-1.0.2", .is_openssl = true },
{ .libcrypto = "openssl-1.1.1", .is_openssl = true },
{ .libcrypto = "openssl-3.0", .is_openssl = true },
{ .libcrypto = "openssl-3.0-fips", .is_openssl = true, .is_opensslfips = true },
Copy link
Contributor

Choose a reason for hiding this comment

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

Is is_opensslfips set to false anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When initializing a struct this way in C, any field not specified is "0", which is false. So everywhere that doesn't set is_openssl_fips gets "false".

{ .libcrypto = "openssl-3.4", .is_openssl = true },
};

for (size_t i = 0; i < s2n_array_len(supported_libcrypto); i++) {
/* The linked libcryto is one of the known supported libcrypto variants */
if (strcmp(s2n_libcrypto, supported_libcrypto[i].libcrypto) == 0) {
if (supported_libcrypto[i].is_openssl) {
EXPECT_TRUE(s2n_libcrypto_is_openssl());
} else {
EXPECT_FALSE(s2n_libcrypto_is_openssl());
}

return S2N_RESULT_OK;
if (strcmp(s2n_libcrypto, supported_libcrypto[i].libcrypto) != 0) {
continue;
}
EXPECT_EQUAL(s2n_libcrypto_is_openssl(), supported_libcrypto[i].is_openssl);
EXPECT_EQUAL(s2n_libcrypto_is_openssl_fips(), supported_libcrypto[i].is_opensslfips);
return S2N_RESULT_OK;
}

/* Testing with an unexpected libcrypto. */
return S2N_RESULT_ERROR;
/* Fail if no matching libcrypto was found.
* Since we check for openssl via process of elimination, we must know
* and test the full list of non-openssl libcryptos.
*/
FAIL_MSG("Unknown libcrypto - update supported_libcrypto list");
}

int main()
Expand Down Expand Up @@ -127,16 +128,13 @@ int main()
END_TEST();
}

/* Ensure that FIPS mode is enabled when linked to AWS-LC-FIPS, and disabled when linked to AWS-LC */
if (strstr(s2n_libcrypto, "awslc") != NULL) {
s2n_fips_mode fips_mode = S2N_FIPS_MODE_DISABLED;
EXPECT_SUCCESS(s2n_get_fips_mode(&fips_mode));

if (strstr(s2n_libcrypto, "fips") != NULL) {
EXPECT_EQUAL(fips_mode, S2N_FIPS_MODE_ENABLED);
} else {
EXPECT_EQUAL(fips_mode, S2N_FIPS_MODE_DISABLED);
}
/* Ensure that FIPS mode is enabled only when linked to a fips library */
s2n_fips_mode fips_mode = S2N_FIPS_MODE_DISABLED;
EXPECT_SUCCESS(s2n_get_fips_mode(&fips_mode));
if (strstr(s2n_libcrypto, "fips") != NULL) {
EXPECT_EQUAL(fips_mode, S2N_FIPS_MODE_ENABLED);
} else {
EXPECT_EQUAL(fips_mode, S2N_FIPS_MODE_DISABLED);
}

char s2n_libcrypto_copy[MAX_LIBCRYPTO_NAME_LEN] = { 0 };
Expand All @@ -150,7 +148,8 @@ int main()
{
if (strstr(name, "awslc") != NULL) {
/* Early versions of awslc's SSLeay_version return an inaccurate value left over
* after its fork from BoringSSL. */
* after its fork from BoringSSL.
*/
Comment on lines -154 to +153
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'm removing a sneaky tab character here :/ I'm surprised clang-format doesn't flag tabs.

EXPECT_TRUE(s2n_libcrypto_is_awslc());
} else {
/* Any other library should have the name of the library (modulo case) in its version string. */
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/s2n_fips_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,17 @@ int main()
}
}

/* Test: FIPS mode not currently supported for openssl outside of tests. */
if (s2n_libcrypto_is_openssl_fips()) {
/* s2n_fips_init was already called by BEGIN_TEST.
* Call it again to confirm that we are allowed to call it repeatedly.
*/
EXPECT_SUCCESS(s2n_fips_init());

EXPECT_SUCCESS(s2n_in_unit_test_set(false));
EXPECT_FAILURE_WITH_ERRNO(s2n_fips_init(), S2N_ERR_FIPS_MODE_UNSUPPORTED);
EXPECT_SUCCESS(s2n_in_unit_test_set(true));
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
}

END_TEST();
}
Loading