From f6647b674fcc3ab532f81fd245423c2651b3fcd6 Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Fri, 7 Feb 2025 17:33:26 -0500 Subject: [PATCH 01/19] chore: bindings release 0.3.11 (#5098) --- bindings/rust/extended/s2n-tls-sys/Cargo.toml | 2 +- bindings/rust/extended/s2n-tls-sys/templates/Cargo.template | 2 +- bindings/rust/extended/s2n-tls-tokio/Cargo.toml | 4 ++-- bindings/rust/extended/s2n-tls/Cargo.toml | 4 ++-- bindings/rust/standard/s2n-tls-hyper/Cargo.toml | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bindings/rust/extended/s2n-tls-sys/Cargo.toml b/bindings/rust/extended/s2n-tls-sys/Cargo.toml index 7ace1f23ec5..9db1524aeb5 100644 --- a/bindings/rust/extended/s2n-tls-sys/Cargo.toml +++ b/bindings/rust/extended/s2n-tls-sys/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-sys" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.10" +version = "0.3.11" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" diff --git a/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template b/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template index 541d9a99fe6..c0f8b423ec2 100644 --- a/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template +++ b/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template @@ -1,7 +1,7 @@ [package] name = "s2n-tls-sys" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.10" +version = "0.3.11" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" diff --git a/bindings/rust/extended/s2n-tls-tokio/Cargo.toml b/bindings/rust/extended/s2n-tls-tokio/Cargo.toml index b2a0a82635e..11a5ca87f60 100644 --- a/bindings/rust/extended/s2n-tls-tokio/Cargo.toml +++ b/bindings/rust/extended/s2n-tls-tokio/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-tokio" description = "An implementation of TLS streams for Tokio built on top of s2n-tls" -version = "0.3.10" +version = "0.3.11" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -16,7 +16,7 @@ errno = { version = "0.3" } # A minimum libc version of 0.2.121 is required by aws-lc-sys 0.14.0. libc = { version = "0.2.121" } pin-project-lite = { version = "0.2" } -s2n-tls = { version = "=0.3.10", path = "../s2n-tls" } +s2n-tls = { version = "=0.3.11", path = "../s2n-tls" } tokio = { version = "1", features = ["net", "time"] } [dev-dependencies] diff --git a/bindings/rust/extended/s2n-tls/Cargo.toml b/bindings/rust/extended/s2n-tls/Cargo.toml index 1a3de03546a..b50bea59f67 100644 --- a/bindings/rust/extended/s2n-tls/Cargo.toml +++ b/bindings/rust/extended/s2n-tls/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.10" +version = "0.3.11" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -22,7 +22,7 @@ unstable-testing = [] errno = { version = "0.3" } # A minimum libc version of 0.2.121 is required by aws-lc-sys 0.14.0. libc = "0.2.121" -s2n-tls-sys = { version = "=0.3.10", path = "../s2n-tls-sys", features = ["internal"] } +s2n-tls-sys = { version = "=0.3.11", path = "../s2n-tls-sys", features = ["internal"] } pin-project-lite = "0.2" hex = "0.4" diff --git a/bindings/rust/standard/s2n-tls-hyper/Cargo.toml b/bindings/rust/standard/s2n-tls-hyper/Cargo.toml index ee31cb65e54..5219422aa0a 100644 --- a/bindings/rust/standard/s2n-tls-hyper/Cargo.toml +++ b/bindings/rust/standard/s2n-tls-hyper/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-hyper" description = "A compatbility crate allowing s2n-tls to be used with the hyper HTTP library" -version = "0.0.2" +version = "0.0.3" authors = ["AWS s2n"] edition = "2021" rust-version = "1.74.0" @@ -12,8 +12,8 @@ license = "Apache-2.0" default = [] [dependencies] -s2n-tls = { version = "=0.3.10", path = "../../extended/s2n-tls" } -s2n-tls-tokio = { version = "=0.3.10", path = "../../extended/s2n-tls-tokio" } +s2n-tls = { version = "=0.3.11", path = "../../extended/s2n-tls" } +s2n-tls-tokio = { version = "=0.3.11", path = "../../extended/s2n-tls-tokio" } # A minimum hyper version of 1.3 is required by hyper-util 0.1.4: # https://github.com/hyperium/hyper-util/blob/3f6a92ecd019b8d534d2945564d3ab8a92ff1f41/Cargo.toml#L34 hyper = { version = "1.3" } From 203cc5c480d048e39e8561e508b6902afc1bf10a Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Mon, 10 Feb 2025 13:39:16 -0500 Subject: [PATCH 02/19] fix(integrationv2): Skip unsupported client auth tests (#5096) Co-authored-by: James Mayclin --- .../spec/buildspec_ubuntu_integrationv2.yml | 2 +- tests/integrationv2/providers.py | 40 ++++++++++++------- tests/integrationv2/utils.py | 10 +++-- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/codebuild/spec/buildspec_ubuntu_integrationv2.yml b/codebuild/spec/buildspec_ubuntu_integrationv2.yml index 109b6757c69..13ee3ab609e 100644 --- a/codebuild/spec/buildspec_ubuntu_integrationv2.yml +++ b/codebuild/spec/buildspec_ubuntu_integrationv2.yml @@ -36,7 +36,7 @@ batch: - openssl-1.1.1_gcc9 - openssl-3.0 INTEGV2_TEST: - - "test_dynamic_record_sizes test_sslyze test_sslv2_client_hello" + - "test_client_authentication test_dynamic_record_sizes test_sslyze test_sslv2_client_hello" - "test_happy_path" - "test_cross_compatibility" - "test_early_data test_well_known_endpoints test_hello_retry_requests test_sni_match test_pq_handshake test_fragmentation test_key_update" diff --git a/tests/integrationv2/providers.py b/tests/integrationv2/providers.py index 6e8bcd049d6..5b5a7e29faf 100644 --- a/tests/integrationv2/providers.py +++ b/tests/integrationv2/providers.py @@ -5,7 +5,7 @@ import pytest import threading -from common import ProviderOptions, Ciphers, Curves, Protocols, Signatures +from common import ProviderOptions, Ciphers, Curves, Protocols, Signatures, Cert from global_flags import get_flag, S2N_PROVIDER_VERSION, S2N_FIPS_MODE from stat import S_IMODE @@ -72,7 +72,7 @@ def get_send_marker(cls): return None @classmethod - def supports_protocol(cls, protocol, with_cert=None): + def supports_protocol(cls, protocol): raise NotImplementedError @classmethod @@ -94,6 +94,10 @@ def set_provider_ready(self): self._provider_ready = True self._provider_ready_condition.notify() + @classmethod + def supports_certificate(cls, cert: Cert): + return True + class Tcpdump(Provider): """ @@ -147,7 +151,7 @@ def get_send_marker(cls): return 's2n is ready' @classmethod - def supports_protocol(cls, protocol, with_cert=None): + def _pss_supported(cls): # RSA-PSS is unsupported for openssl-1.0 # libressl and boringssl are disabled because of configuration issues # see https://github.com/aws/s2n-tls/issues/3250 @@ -156,16 +160,22 @@ def supports_protocol(cls, protocol, with_cert=None): "boringssl", "openssl-1.0" } - pss_is_unsupported = any([ + for libcrypto in PSS_UNSUPPORTED_LIBCRYPTOS: # e.g. "openssl-1.0" in "openssl-1.0.2-fips" - libcrypto in get_flag(S2N_PROVIDER_VERSION) - for libcrypto in PSS_UNSUPPORTED_LIBCRYPTOS - ]) - if pss_is_unsupported: - if protocol == Protocols.TLS13: - return False - if with_cert and with_cert.algorithm == 'RSAPSS': + if libcrypto in get_flag(S2N_PROVIDER_VERSION): return False + return True + + @classmethod + def supports_certificate(cls, cert: Cert): + if not cls._pss_supported() and cert.algorithm == 'RSAPSS': + return False + return True + + @classmethod + def supports_protocol(cls, protocol): + if not cls._pss_supported() and protocol == Protocols.TLS13: + return False # SSLv3 cannot be negotiated in FIPS mode with libcryptos other than AWS-LC. if all([ @@ -391,7 +401,7 @@ def get_version(cls): return get_flag(S2N_PROVIDER_VERSION) @classmethod - def supports_protocol(cls, protocol, with_cert=None): + def supports_protocol(cls, protocol): if protocol is Protocols.SSLv3: return False @@ -552,7 +562,7 @@ def _override_libssl(self, options: ProviderOptions): options.env_overrides = override_env_vars @classmethod - def supports_protocol(cls, protocol, with_cert=None): + def supports_protocol(cls, protocol): if protocol is Protocols.SSLv3: return True return False @@ -572,7 +582,7 @@ def get_send_marker(cls): return "Starting handshake" @classmethod - def supports_protocol(cls, protocol, with_cert=None): + def supports_protocol(cls, protocol): # https://aws.amazon.com/blogs/opensource/tls-1-0-1-1-changes-in-openjdk-and-amazon-corretto/ if protocol is Protocols.SSLv3 or protocol is Protocols.TLS10 or protocol is Protocols.TLS11: return False @@ -879,7 +889,7 @@ def setup_server(self): return cmd_line @classmethod - def supports_protocol(cls, protocol, with_cert=None): + def supports_protocol(cls, protocol): return GnuTLS.protocol_to_priority_str(protocol) is not None @classmethod diff --git a/tests/integrationv2/utils.py b/tests/integrationv2/utils.py index 47842fd3bdb..615a154b4bd 100644 --- a/tests/integrationv2/utils.py +++ b/tests/integrationv2/utils.py @@ -72,6 +72,8 @@ def invalid_test_parameters(*args, **kwargs): # Always consider S2N providers.append(S2N) + certificates = [cert for cert in [certificate, client_certificate] if cert] + # Older versions do not support RSA-PSS-PSS certificates if protocol and protocol < Protocols.TLS12: if client_certificate and client_certificate.algorithm == 'RSAPSS': @@ -83,6 +85,10 @@ def invalid_test_parameters(*args, **kwargs): if not provider_.supports_protocol(protocol): return True + for certificate_ in certificates: + if not provider_.supports_certificate(certificate_): + return True + if cipher is not None: # If the selected protocol doesn't allow the cipher, don't test if protocol is not None: @@ -105,10 +111,6 @@ def invalid_test_parameters(*args, **kwargs): # If we are using a cipher that depends on a specific certificate algorithm # deselect the test if the wrong certificate is used. if certificate is not None: - if protocol is not None: - for provider_ in providers: - if provider_.supports_protocol(protocol, with_cert=certificate) is False: - return True if cipher is not None and certificate.compatible_with_cipher(cipher) is False: return True From 36636673e8440239bef1dabb19501310f56933b4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 11 Feb 2025 09:37:04 -0800 Subject: [PATCH 03/19] build(deps): bump aws-actions/configure-aws-credentials from 4.0.2 to 4.1.0 in /.github/workflows in the all-gha-updates group across 1 directory (#5107) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/bench.yml | 2 +- .github/workflows/usage_guide.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index eb83cfa4fdb..abf30a40f2a 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -41,7 +41,7 @@ jobs: - name: Configure AWS Credentials # Only continue with the workflow to emit metrics on code that has been merged to main. if: github.event_name != 'pull_request' - uses: aws-actions/configure-aws-credentials@v4.0.2 + uses: aws-actions/configure-aws-credentials@v4.1.0 with: role-to-assume: arn:aws:iam::024603541914:role/GitHubOIDCRole role-session-name: s2ntlsghabenchsession diff --git a/.github/workflows/usage_guide.yml b/.github/workflows/usage_guide.yml index ffe7d3ba39b..2c990a898cc 100644 --- a/.github/workflows/usage_guide.yml +++ b/.github/workflows/usage_guide.yml @@ -52,7 +52,7 @@ jobs: folder: docs/usage-guide/book - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v4.0.2 + uses: aws-actions/configure-aws-credentials@v4.1.0 if: github.event_name == 'push' || github.repository == github.event.pull_request.head.repo.full_name with: role-to-assume: arn:aws:iam::024603541914:role/GitHubOIDCRole From e4a5a74b0492767ac996e93a7257d533087015dd Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Tue, 11 Feb 2025 11:36:38 -0800 Subject: [PATCH 04/19] refactor: remove s2n_hmac_is_available (#5104) --- crypto/s2n_hmac.c | 26 ---------- crypto/s2n_hmac.h | 1 - .../proofs/s2n_hmac_is_available/Makefile | 31 ------------ .../s2n_hmac_is_available/cbmc-proof.txt | 1 - .../s2n_hmac_is_available_harness.c | 48 ------------------- tests/unit/s2n_hmac_test.c | 12 ++--- 6 files changed, 6 insertions(+), 113 deletions(-) delete mode 100644 tests/cbmc/proofs/s2n_hmac_is_available/Makefile delete mode 100644 tests/cbmc/proofs/s2n_hmac_is_available/cbmc-proof.txt delete mode 100644 tests/cbmc/proofs/s2n_hmac_is_available/s2n_hmac_is_available_harness.c diff --git a/crypto/s2n_hmac.c b/crypto/s2n_hmac.c index 3b519c1f72a..48842259671 100644 --- a/crypto/s2n_hmac.c +++ b/crypto/s2n_hmac.c @@ -75,28 +75,6 @@ int s2n_hmac_digest_size(s2n_hmac_algorithm hmac_alg, uint8_t *out) return S2N_SUCCESS; } -/* Return 1 if hmac algorithm is available, 0 otherwise. */ -bool s2n_hmac_is_available(s2n_hmac_algorithm hmac_alg) -{ - switch(hmac_alg) { - case S2N_HMAC_MD5: - case S2N_HMAC_SSLv3_MD5: - /* Some libcryptos, such as OpenSSL, disable MD5 by default when in FIPS mode, which is - * required in order to negotiate SSLv3. However, this is supported in AWS-LC. - */ - return !s2n_is_in_fips_mode() || s2n_libcrypto_is_awslc(); - case S2N_HMAC_SSLv3_SHA1: - case S2N_HMAC_NONE: - case S2N_HMAC_SHA1: - case S2N_HMAC_SHA224: - case S2N_HMAC_SHA256: - case S2N_HMAC_SHA384: - case S2N_HMAC_SHA512: - return true; - } - return false; -} - static int s2n_sslv3_mac_init(struct s2n_hmac_state *state, s2n_hmac_algorithm alg, const void *key, uint32_t klen) { for (int i = 0; i < state->xor_pad_size; i++) { @@ -205,10 +183,6 @@ S2N_RESULT s2n_hmac_state_validate(struct s2n_hmac_state *state) int s2n_hmac_init(struct s2n_hmac_state *state, s2n_hmac_algorithm alg, const void *key, uint32_t klen) { POSIX_ENSURE_REF(state); - if (!s2n_hmac_is_available(alg)) { - /* Prevent hmacs from being used if they are not available. */ - POSIX_BAIL(S2N_ERR_HMAC_INVALID_ALGORITHM); - } state->alg = alg; POSIX_GUARD(s2n_hmac_hash_block_size(alg, &state->hash_block_size)); diff --git a/crypto/s2n_hmac.h b/crypto/s2n_hmac.h index 81b96c06ea6..de1379d83ec 100644 --- a/crypto/s2n_hmac.h +++ b/crypto/s2n_hmac.h @@ -61,7 +61,6 @@ struct s2n_hmac_evp_backup { }; int s2n_hmac_digest_size(s2n_hmac_algorithm alg, uint8_t *out); -bool s2n_hmac_is_available(s2n_hmac_algorithm alg); int s2n_hmac_hash_alg(s2n_hmac_algorithm hmac_alg, s2n_hash_algorithm *out); int s2n_hash_hmac_alg(s2n_hash_algorithm hash_alg, s2n_hmac_algorithm *out); diff --git a/tests/cbmc/proofs/s2n_hmac_is_available/Makefile b/tests/cbmc/proofs/s2n_hmac_is_available/Makefile deleted file mode 100644 index 0cd6a1a078b..00000000000 --- a/tests/cbmc/proofs/s2n_hmac_is_available/Makefile +++ /dev/null @@ -1,31 +0,0 @@ -# -# -# 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. - -# Expected runtime is less than 5 seconds. - -CBMCFLAGS += - -PROOF_UID = s2n_hmac_is_available -HARNESS_ENTRY = $(PROOF_UID)_harness -HARNESS_FILE = $(HARNESS_ENTRY).c - -PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) -PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_libcrypto_is_awslc.c - -PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hmac.c - -UNWINDSET += - -include ../Makefile.common diff --git a/tests/cbmc/proofs/s2n_hmac_is_available/cbmc-proof.txt b/tests/cbmc/proofs/s2n_hmac_is_available/cbmc-proof.txt deleted file mode 100644 index 6ed46f1258c..00000000000 --- a/tests/cbmc/proofs/s2n_hmac_is_available/cbmc-proof.txt +++ /dev/null @@ -1 +0,0 @@ -# This file marks this directory as containing a CBMC proof. diff --git a/tests/cbmc/proofs/s2n_hmac_is_available/s2n_hmac_is_available_harness.c b/tests/cbmc/proofs/s2n_hmac_is_available/s2n_hmac_is_available_harness.c deleted file mode 100644 index 9c9fba4ec5a..00000000000 --- a/tests/cbmc/proofs/s2n_hmac_is_available/s2n_hmac_is_available_harness.c +++ /dev/null @@ -1,48 +0,0 @@ -/* - * 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. - */ - -#include - -#include "crypto/s2n_fips.h" -#include "crypto/s2n_hmac.h" -#include "crypto/s2n_openssl.h" - -#include - -void s2n_hmac_is_available_harness() -{ - /* Non-deterministic inputs. */ - s2n_hmac_algorithm hmac_alg; - - /* Operation under verification. */ - bool is_available = s2n_hmac_is_available(hmac_alg); - - /* Postconditions. */ - switch (hmac_alg) { - case S2N_HASH_MD5: - case S2N_HMAC_SSLv3_MD5: - assert(is_available == !s2n_is_in_fips_mode() || s2n_libcrypto_is_awslc()); break; - case S2N_HMAC_SSLv3_SHA1: - case S2N_HASH_NONE: - case S2N_HASH_SHA1: - case S2N_HASH_SHA224: - case S2N_HASH_SHA256: - case S2N_HASH_SHA384: - case S2N_HASH_SHA512: - assert(is_available); break; - default: - __CPROVER_assert(!is_available, "Unsupported algorithm."); - } -} diff --git a/tests/unit/s2n_hmac_test.c b/tests/unit/s2n_hmac_test.c index 74fe6fc9033..11020d3c909 100644 --- a/tests/unit/s2n_hmac_test.c +++ b/tests/unit/s2n_hmac_test.c @@ -47,8 +47,8 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_hmac_new(©)); EXPECT_SUCCESS(s2n_hmac_new(&cmac)); - if (s2n_hmac_is_available(S2N_HMAC_SSLv3_MD5)) { - /* Try SSLv3 MD5 */ + /* Try SSLv3 MD5 */ + { uint8_t hmac_sslv3_md5_size = 0; POSIX_GUARD(s2n_hmac_digest_size(S2N_HMAC_SSLv3_MD5, &hmac_sslv3_md5_size)); EXPECT_EQUAL(hmac_sslv3_md5_size, 16); @@ -80,8 +80,8 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_hmac_reset(&hmac)); } - if (s2n_hmac_is_available(S2N_HMAC_SSLv3_SHA1)) { - /* Try SSLv3 SHA1 */ + /* Try SSLv3 SHA1 */ + { uint8_t hmac_sslv3_sha1_size = 0; POSIX_GUARD(s2n_hmac_digest_size(S2N_HMAC_SSLv3_SHA1, &hmac_sslv3_sha1_size)); EXPECT_EQUAL(hmac_sslv3_sha1_size, 20); @@ -113,8 +113,8 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_hmac_reset(&hmac)); } - if (s2n_hmac_is_available(S2N_HMAC_MD5)) { - /* Try MD5 */ + /* Try MD5 */ + { uint8_t hmac_md5_size = 0; POSIX_GUARD(s2n_hmac_digest_size(S2N_HMAC_MD5, &hmac_md5_size)); EXPECT_EQUAL(hmac_md5_size, 16); From f3ae011d3a47575237f019ffe3b9c5ae20ac47bc Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Wed, 12 Feb 2025 15:04:04 -0800 Subject: [PATCH 05/19] refactor: remove unused evp support for md5+sha1 (#5106) --- crypto/s2n_evp_signing.c | 51 +++++++++++-- crypto/s2n_hash.c | 75 +++---------------- crypto/s2n_hash.h | 2 - tests/cbmc/include/cbmc_proof/cbmc_utils.h | 5 -- .../Makefile | 2 +- tests/cbmc/proofs/s2n_hash_copy/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_digest/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_free/Makefile | 2 +- .../s2n_hash_free/s2n_hash_free_harness.c | 10 +-- .../Makefile | 2 +- tests/cbmc/proofs/s2n_hash_init/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_new/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_reset/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_update/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_copy/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_digest/Makefile | 2 +- .../Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_free/Makefile | 2 +- .../s2n_hmac_free/s2n_hmac_free_harness.c | 16 +--- tests/cbmc/proofs/s2n_hmac_init/Makefile | 1 + tests/cbmc/proofs/s2n_hmac_new/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_reset/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_update/Makefile | 2 +- tests/cbmc/sources/cbmc_utils.c | 13 +--- .../cbmc/sources/make_common_datastructures.c | 5 -- tests/cbmc/stubs/s2n_evp_signing_supported.c | 28 +++++++ 26 files changed, 109 insertions(+), 129 deletions(-) create mode 100644 tests/cbmc/stubs/s2n_evp_signing_supported.c diff --git a/crypto/s2n_evp_signing.c b/crypto/s2n_evp_signing.c index bd0b9c5972d..d8ec6e31539 100644 --- a/crypto/s2n_evp_signing.c +++ b/crypto/s2n_evp_signing.c @@ -16,6 +16,7 @@ #include "crypto/s2n_evp_signing.h" #include "crypto/s2n_evp.h" +#include "crypto/s2n_fips.h" #include "crypto/s2n_pkey.h" #include "crypto/s2n_rsa_pss.h" #include "error/s2n_errno.h" @@ -50,18 +51,58 @@ static S2N_RESULT s2n_evp_pkey_set_rsa_pss_saltlen(EVP_PKEY_CTX *pctx) #endif } -bool s2n_evp_signing_supported() +static bool s2n_evp_md5_sha1_is_supported() +{ +#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) + return true; +#else + return false; +#endif +} + +static bool s2n_evp_md_ctx_set_pkey_ctx_is_supported() { #ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX - /* We can only use EVP signing if the hash state has an EVP_MD_CTX - * that we can pass to the EVP signing methods. - */ - return s2n_hash_evp_fully_supported(); + return true; #else return false; #endif } +bool s2n_evp_signing_supported() +{ + /* We must use the FIPS-approved EVP APIs in FIPS mode, + * but we could also use the EVP APIs outside of FIPS mode. + * Only using the EVP APIs in FIPS mode was a choice made to reduce + * the impact of adding support for the EVP APIs. + * We should consider instead making the EVP APIs the default. + */ + if (!s2n_is_in_fips_mode()) { + return false; + } + + /* Our EVP signing logic is intended to support FIPS 140-3. + * FIPS 140-3 does not allow externally calculated digests (except for + * signing, but not verifying, with ECDSA). + * See https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Digital-Signatures, + * and note that "component" tests only exist for ECDSA sign. + * + * We currently work around that restriction by calling EVP_MD_CTX_set_pkey_ctx, + * which lets us set a key on an existing hash state. This is important + * when we need to handle signing the TLS1.2 client cert verify message, + * which requires signing the entire message transcript. If EVP_MD_CTX_set_pkey_ctx + * is unavailable (true for openssl-1.0.2), our current EVP logic will not work. + * + * FIPS 140-3 is also not possible if EVP_md5_sha1() isn't available + * (again true for openssl-1.0.2). In that case, we use two separate hash + * states to track the md5 and sha1 parts of the hash separately. That means + * that we also have to calculate the digests separately, then combine the + * result. We therefore only have an externally calculated digest available + * for signing or verifying. + */ + return s2n_evp_md_ctx_set_pkey_ctx_is_supported() && s2n_evp_md5_sha1_is_supported(); +} + /* If using EVP signing, override the sign and verify pkey methods. * The EVP methods can handle all pkey types / signature algorithms. */ diff --git a/crypto/s2n_hash.c b/crypto/s2n_hash.c index 8bd57ba563b..bc78b436a96 100644 --- a/crypto/s2n_hash.c +++ b/crypto/s2n_hash.c @@ -15,29 +15,22 @@ #include "crypto/s2n_hash.h" -#include "crypto/s2n_fips.h" +#include "crypto/s2n_evp_signing.h" #include "crypto/s2n_hmac.h" #include "crypto/s2n_openssl.h" #include "error/s2n_errno.h" #include "utils/s2n_safety.h" -static bool s2n_use_custom_md5_sha1() -{ -#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) - return false; -#else - return true; -#endif -} - static bool s2n_use_evp_impl() { - return s2n_is_in_fips_mode(); -} - -bool s2n_hash_evp_fully_supported() -{ - return s2n_use_evp_impl() && !s2n_use_custom_md5_sha1(); + /* Our current EVP signing implementation requires EVP hashing. + * + * We could use the EVP hashing impl with legacy signing, but that would + * unnecessarily complicate the logic. The only known libcrypto that + * doesn't support EVP signing is openssl-1.0.2. Just let legacy + * libcryptos use legacy methods. + */ + return s2n_evp_signing_supported(); } const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg) @@ -289,13 +282,8 @@ static int s2n_low_level_hash_free(struct s2n_hash_state *state) static int s2n_evp_hash_new(struct s2n_hash_state *state) { POSIX_ENSURE_REF(state->digest.high_level.evp.ctx = S2N_EVP_MD_CTX_NEW()); - if (s2n_use_custom_md5_sha1()) { - POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx = S2N_EVP_MD_CTX_NEW()); - } - state->is_ready_for_input = 0; state->currently_in_hash = 0; - return S2N_SUCCESS; } @@ -311,13 +299,6 @@ static int s2n_evp_hash_init(struct s2n_hash_state *state, s2n_hash_algorithm al return S2N_SUCCESS; } - if (alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { - POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx); - POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, EVP_sha1(), NULL), S2N_ERR_HASH_INIT_FAILED); - POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp_md5_secondary.ctx, EVP_md5(), NULL), S2N_ERR_HASH_INIT_FAILED); - return S2N_SUCCESS; - } - POSIX_ENSURE_REF(s2n_hash_alg_to_evp_md(alg)); POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, s2n_hash_alg_to_evp_md(alg), NULL), S2N_ERR_HASH_INIT_FAILED); return S2N_SUCCESS; @@ -335,12 +316,6 @@ static int s2n_evp_hash_update(struct s2n_hash_state *state, const void *data, u POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp.ctx)); POSIX_GUARD_OSSL(EVP_DigestUpdate(state->digest.high_level.evp.ctx, data, size), S2N_ERR_HASH_UPDATE_FAILED); - - if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { - POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp_md5_secondary.ctx)); - POSIX_GUARD_OSSL(EVP_DigestUpdate(state->digest.high_level.evp_md5_secondary.ctx, data, size), S2N_ERR_HASH_UPDATE_FAILED); - } - return S2N_SUCCESS; } @@ -362,23 +337,6 @@ static int s2n_evp_hash_digest(struct s2n_hash_state *state, void *out, uint32_t POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp.ctx)); - if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { - POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp_md5_secondary.ctx)); - - uint8_t sha1_digest_size = 0; - POSIX_GUARD(s2n_hash_digest_size(S2N_HASH_SHA1, &sha1_digest_size)); - - unsigned int sha1_primary_digest_size = sha1_digest_size; - unsigned int md5_secondary_digest_size = digest_size - sha1_primary_digest_size; - - POSIX_ENSURE(EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= sha1_digest_size, S2N_ERR_HASH_DIGEST_FAILED); - POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp_md5_secondary.ctx) <= md5_secondary_digest_size, S2N_ERR_HASH_DIGEST_FAILED); - - POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, ((uint8_t *) out) + MD5_DIGEST_LENGTH, &sha1_primary_digest_size), S2N_ERR_HASH_DIGEST_FAILED); - POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp_md5_secondary.ctx, out, &md5_secondary_digest_size), S2N_ERR_HASH_DIGEST_FAILED); - return S2N_SUCCESS; - } - POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= digest_size, S2N_ERR_HASH_DIGEST_FAILED); POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, out, &digest_size), S2N_ERR_HASH_DIGEST_FAILED); return S2N_SUCCESS; @@ -397,21 +355,12 @@ static int s2n_evp_hash_copy(struct s2n_hash_state *to, struct s2n_hash_state *f POSIX_ENSURE_REF(to->digest.high_level.evp.ctx); POSIX_GUARD_OSSL(EVP_MD_CTX_copy_ex(to->digest.high_level.evp.ctx, from->digest.high_level.evp.ctx), S2N_ERR_HASH_COPY_FAILED); - - if (from->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { - POSIX_ENSURE_REF(to->digest.high_level.evp_md5_secondary.ctx); - POSIX_GUARD_OSSL(EVP_MD_CTX_copy_ex(to->digest.high_level.evp_md5_secondary.ctx, from->digest.high_level.evp_md5_secondary.ctx), S2N_ERR_HASH_COPY_FAILED); - } - return S2N_SUCCESS; } static int s2n_evp_hash_reset(struct s2n_hash_state *state) { POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(state->digest.high_level.evp.ctx), S2N_ERR_HASH_WIPE_FAILED); - if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { - POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(state->digest.high_level.evp_md5_secondary.ctx), S2N_ERR_HASH_WIPE_FAILED); - } /* hash_init resets the ready_for_input and currently_in_hash fields. */ return s2n_evp_hash_init(state, state->alg); @@ -421,12 +370,6 @@ static int s2n_evp_hash_free(struct s2n_hash_state *state) { S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp.ctx); state->digest.high_level.evp.ctx = NULL; - - if (s2n_use_custom_md5_sha1()) { - S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp_md5_secondary.ctx); - state->digest.high_level.evp_md5_secondary.ctx = NULL; - } - state->is_ready_for_input = 0; return S2N_SUCCESS; } diff --git a/crypto/s2n_hash.h b/crypto/s2n_hash.h index 03643a03e9c..8c0e7385af0 100644 --- a/crypto/s2n_hash.h +++ b/crypto/s2n_hash.h @@ -54,8 +54,6 @@ union s2n_hash_low_level_digest { /* The evp_digest stores all OpenSSL structs to be used with OpenSSL's EVP hash API's. */ struct s2n_hash_evp_digest { struct s2n_evp_digest evp; - /* Always store a secondary evp_digest to allow resetting a hash_state to MD5_SHA1 from another alg. */ - struct s2n_evp_digest evp_md5_secondary; }; /* s2n_hash_state stores the s2n_hash implementation being used (low-level or EVP), diff --git a/tests/cbmc/include/cbmc_proof/cbmc_utils.h b/tests/cbmc/include/cbmc_proof/cbmc_utils.h index 19c2b60b0e7..4c58d4d5451 100644 --- a/tests/cbmc/include/cbmc_proof/cbmc_utils.h +++ b/tests/cbmc/include/cbmc_proof/cbmc_utils.h @@ -47,13 +47,8 @@ struct rc_keys_from_evp_pkey_ctx { int pkey_eckey_refs; }; -/** - * In the `rc_keys_from_hash_state`, we store two `rc_keys_from_evp_pkey_ctx` objects: - * one for the `evp` field and another for `evp_md5_secondary` field. - */ struct rc_keys_from_hash_state { struct rc_keys_from_evp_pkey_ctx evp; - struct rc_keys_from_evp_pkey_ctx evp_md5; }; /** diff --git a/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile b/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile index 3e88a91d608..2f7df8c00fa 100644 --- a/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile +++ b/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile @@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_copy/Makefile b/tests/cbmc/proofs/s2n_hash_copy/Makefile index 02e2c6c99be..82cb3b54e39 100644 --- a/tests/cbmc/proofs/s2n_hash_copy/Makefile +++ b/tests/cbmc/proofs/s2n_hash_copy/Makefile @@ -25,7 +25,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_ensure.c diff --git a/tests/cbmc/proofs/s2n_hash_digest/Makefile b/tests/cbmc/proofs/s2n_hash_digest/Makefile index f4e8cd51fe0..53a8f450683 100644 --- a/tests/cbmc/proofs/s2n_hash_digest/Makefile +++ b/tests/cbmc/proofs/s2n_hash_digest/Makefile @@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c diff --git a/tests/cbmc/proofs/s2n_hash_free/Makefile b/tests/cbmc/proofs/s2n_hash_free/Makefile index 676317f1dc2..a873b020619 100644 --- a/tests/cbmc/proofs/s2n_hash_free/Makefile +++ b/tests/cbmc/proofs/s2n_hash_free/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c b/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c index 9c646a0b64f..f8fc362ad92 100644 --- a/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c +++ b/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "crypto/s2n_fips.h" +#include "crypto/s2n_evp_signing.h" #include "crypto/s2n_hash.h" #include @@ -35,9 +35,8 @@ void s2n_hash_free_harness() assert(s2n_hash_free(state) == S2N_SUCCESS); if (state != NULL) { assert(state->hash_impl->free != NULL); - if (s2n_is_in_fips_mode()) { + if (s2n_evp_signing_supported()) { assert(state->digest.high_level.evp.ctx == NULL); - assert(state->digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_hash_state); } else { assert_rc_unchanged_on_hash_state(&saved_hash_state); @@ -47,11 +46,10 @@ void s2n_hash_free_harness() /* Cleanup after expected error cases, for memory leak check. */ if (state != NULL) { - /* 1. `free` leftover EVP_MD_CTX objects if `s2n_is_in_fips_mode`, + /* 1. `free` leftover EVP_MD_CTX objects if `s2n_evp_signing_supported`, since `s2n_hash_free` is a NO-OP in that case. */ - if (!s2n_is_in_fips_mode()) { + if (!s2n_evp_signing_supported()) { S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp.ctx); - S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp_md5_secondary.ctx); } /* 2. `free` leftover reference-counted keys (i.e. those with non-zero ref-count), diff --git a/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile b/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile index dd6491d63a4..7be94f92f02 100644 --- a/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile +++ b/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile @@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_init/Makefile b/tests/cbmc/proofs/s2n_hash_init/Makefile index 5d905e7c2ef..f7ae8963b2b 100644 --- a/tests/cbmc/proofs/s2n_hash_init/Makefile +++ b/tests/cbmc/proofs/s2n_hash_init/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_new/Makefile b/tests/cbmc/proofs/s2n_hash_new/Makefile index da1f324b484..c156cd61aaa 100644 --- a/tests/cbmc/proofs/s2n_hash_new/Makefile +++ b/tests/cbmc/proofs/s2n_hash_new/Makefile @@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_reset/Makefile b/tests/cbmc/proofs/s2n_hash_reset/Makefile index 55e08b1269c..b0f408b03eb 100644 --- a/tests/cbmc/proofs/s2n_hash_reset/Makefile +++ b/tests/cbmc/proofs/s2n_hash_reset/Makefile @@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c diff --git a/tests/cbmc/proofs/s2n_hash_update/Makefile b/tests/cbmc/proofs/s2n_hash_update/Makefile index f9239e1946e..13e35003793 100644 --- a/tests/cbmc/proofs/s2n_hash_update/Makefile +++ b/tests/cbmc/proofs/s2n_hash_update/Makefile @@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c diff --git a/tests/cbmc/proofs/s2n_hmac_copy/Makefile b/tests/cbmc/proofs/s2n_hmac_copy/Makefile index 87231753868..e3e1c211460 100644 --- a/tests/cbmc/proofs/s2n_hmac_copy/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_copy/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_digest/Makefile b/tests/cbmc/proofs/s2n_hmac_digest/Makefile index 90781cff1ba..7012a41ea8c 100644 --- a/tests/cbmc/proofs/s2n_hmac_digest/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_digest/Makefile @@ -19,7 +19,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_copy.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_digest.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_update.c diff --git a/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile b/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile index 7fdb8a76da8..014cc1d9955 100644 --- a/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile @@ -19,7 +19,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_copy.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_digest.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_reset.c diff --git a/tests/cbmc/proofs/s2n_hmac_free/Makefile b/tests/cbmc/proofs/s2n_hmac_free/Makefile index fd14d427e94..8791513153f 100644 --- a/tests/cbmc/proofs/s2n_hmac_free/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_free/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c b/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c index aeff29282ad..fbc0f9f3625 100644 --- a/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c +++ b/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "crypto/s2n_fips.h" +#include "crypto/s2n_evp_signing.h" #include "crypto/s2n_hash.h" #include @@ -44,21 +44,17 @@ void s2n_hmac_free_harness() assert(state->outer.hash_impl->free != NULL); assert(state->outer_just_key.hash_impl->free != NULL); - if (s2n_is_in_fips_mode()) { + if (s2n_evp_signing_supported()) { assert(state->inner.digest.high_level.evp.ctx == NULL); - assert(state->inner.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_inner_hash_state); assert(state->inner_just_key.digest.high_level.evp.ctx == NULL); - assert(state->inner_just_key.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_inner_just_key_hash_state); assert(state->outer.digest.high_level.evp.ctx == NULL); - assert(state->outer.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_outer_hash_state); assert(state->outer_just_key.digest.high_level.evp.ctx == NULL); - assert(state->outer_just_key.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_outer_just_key_hash_state); } else { assert_rc_unchanged_on_hash_state(&saved_inner_hash_state); @@ -75,17 +71,13 @@ void s2n_hmac_free_harness() /* Cleanup after expected error cases, for memory leak check. */ if (state != NULL) { - /* 1. `free` leftover EVP_MD_CTX objects if `s2n_is_in_fips_mode`, + /* 1. `free` leftover EVP_MD_CTX objects if `s2n_evp_signing_supported`, since `s2n_hash_free` is a NO-OP in that case. */ - if (!s2n_is_in_fips_mode()) { + if (!s2n_evp_signing_supported()) { S2N_EVP_MD_CTX_FREE(state->inner.digest.high_level.evp.ctx); - S2N_EVP_MD_CTX_FREE(state->inner.digest.high_level.evp_md5_secondary.ctx); S2N_EVP_MD_CTX_FREE(state->inner_just_key.digest.high_level.evp.ctx); - S2N_EVP_MD_CTX_FREE(state->inner_just_key.digest.high_level.evp_md5_secondary.ctx); S2N_EVP_MD_CTX_FREE(state->outer.digest.high_level.evp.ctx); - S2N_EVP_MD_CTX_FREE(state->outer.digest.high_level.evp_md5_secondary.ctx); S2N_EVP_MD_CTX_FREE(state->outer_just_key.digest.high_level.evp.ctx); - S2N_EVP_MD_CTX_FREE(state->outer_just_key.digest.high_level.evp_md5_secondary.ctx); } /* 2. `free` leftover reference-counted keys (i.e. those with non-zero ref-count), diff --git a/tests/cbmc/proofs/s2n_hmac_init/Makefile b/tests/cbmc/proofs/s2n_hmac_init/Makefile index 4931abbccda..964e84d9e6e 100644 --- a/tests/cbmc/proofs/s2n_hmac_init/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_init/Makefile @@ -24,6 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOF_STUB)/s2n_libcrypto_is_awslc.c PROOF_SOURCES += $(PROOF_STUB)/darwin_check_fd_set_overflow.c diff --git a/tests/cbmc/proofs/s2n_hmac_new/Makefile b/tests/cbmc/proofs/s2n_hmac_new/Makefile index b553599d793..b503264b6d6 100644 --- a/tests/cbmc/proofs/s2n_hmac_new/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_new/Makefile @@ -21,7 +21,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_reset/Makefile b/tests/cbmc/proofs/s2n_hmac_reset/Makefile index c74c27914fd..066dcc3489c 100644 --- a/tests/cbmc/proofs/s2n_hmac_reset/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_reset/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_update/Makefile b/tests/cbmc/proofs/s2n_hmac_update/Makefile index 0098edb5975..eb16a293d47 100644 --- a/tests/cbmc/proofs/s2n_hmac_update/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_update/Makefile @@ -26,7 +26,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hmac.c diff --git a/tests/cbmc/sources/cbmc_utils.c b/tests/cbmc/sources/cbmc_utils.c index 733c87fb9e1..f8eb8943627 100644 --- a/tests/cbmc/sources/cbmc_utils.c +++ b/tests/cbmc/sources/cbmc_utils.c @@ -220,13 +220,11 @@ void assert_rc_unchanged_on_evp_pkey_ctx(struct rc_keys_from_evp_pkey_ctx *stora void assert_rc_decrement_on_hash_state(struct rc_keys_from_hash_state *storage) { assert_rc_decrement_on_evp_pkey_ctx(&storage->evp); - assert_rc_decrement_on_evp_pkey_ctx(&storage->evp_md5); } void assert_rc_unchanged_on_hash_state(struct rc_keys_from_hash_state *storage) { assert_rc_unchanged_on_evp_pkey_ctx(&storage->evp); - assert_rc_unchanged_on_evp_pkey_ctx(&storage->evp_md5); } void save_abstract_evp_ctx(const EVP_PKEY_CTX *pctx, struct rc_keys_from_evp_pkey_ctx *storage) @@ -245,9 +243,7 @@ void save_rc_keys_from_hash_state(const struct s2n_hash_state *state, struct rc_ { storage->evp.pkey = NULL; storage->evp.pkey_eckey = NULL; - storage->evp_md5.pkey = NULL; - storage->evp_md5.pkey_eckey = NULL; - storage->evp.pkey_refs = storage->evp.pkey_eckey_refs = storage->evp_md5.pkey_refs = storage->evp_md5.pkey_eckey_refs = 0; + storage->evp.pkey_refs = storage->evp.pkey_eckey_refs = 0; if (state) { if (state->digest.high_level.evp.ctx) { @@ -255,12 +251,6 @@ void save_rc_keys_from_hash_state(const struct s2n_hash_state *state, struct rc_ save_abstract_evp_ctx(state->digest.high_level.evp.ctx->pctx, &storage->evp); } } - - if (state->digest.high_level.evp_md5_secondary.ctx) { - if (state->digest.high_level.evp_md5_secondary.ctx->pctx) { - save_abstract_evp_ctx(state->digest.high_level.evp_md5_secondary.ctx->pctx, &storage->evp_md5); - } - } } } @@ -279,5 +269,4 @@ void free_rc_keys_from_evp_pkey_ctx(struct rc_keys_from_evp_pkey_ctx *pctx) void free_rc_keys_from_hash_state(struct rc_keys_from_hash_state *storage) { free_rc_keys_from_evp_pkey_ctx(&storage->evp); - free_rc_keys_from_evp_pkey_ctx(&storage->evp_md5); } diff --git a/tests/cbmc/sources/make_common_datastructures.c b/tests/cbmc/sources/make_common_datastructures.c index 59c7590f10d..b159986ad8f 100644 --- a/tests/cbmc/sources/make_common_datastructures.c +++ b/tests/cbmc/sources/make_common_datastructures.c @@ -270,7 +270,6 @@ void cbmc_populate_s2n_hash_state(struct s2n_hash_state* state) * If required, this initialization should be done in the validation function. */ cbmc_populate_s2n_evp_digest(&state->digest.high_level.evp); - cbmc_populate_s2n_evp_digest(&state->digest.high_level.evp_md5_secondary); } struct s2n_hash_state* cbmc_allocate_s2n_hash_state() @@ -300,13 +299,9 @@ void cbmc_populate_s2n_hmac_evp_backup(struct s2n_hmac_evp_backup *backup) { CBMC_ENSURE_REF(backup); cbmc_populate_s2n_evp_digest(&backup->inner.evp); - cbmc_populate_s2n_evp_digest(&backup->inner.evp_md5_secondary); cbmc_populate_s2n_evp_digest(&backup->inner_just_key.evp); - cbmc_populate_s2n_evp_digest(&backup->inner_just_key.evp_md5_secondary); cbmc_populate_s2n_evp_digest(&backup->outer.evp); - cbmc_populate_s2n_evp_digest(&backup->outer.evp_md5_secondary); cbmc_populate_s2n_evp_digest(&backup->outer_just_key.evp); - cbmc_populate_s2n_evp_digest(&backup->outer_just_key.evp_md5_secondary); } struct s2n_hmac_evp_backup* cbmc_allocate_s2n_hmac_evp_backup() diff --git a/tests/cbmc/stubs/s2n_evp_signing_supported.c b/tests/cbmc/stubs/s2n_evp_signing_supported.c new file mode 100644 index 00000000000..32d709a1167 --- /dev/null +++ b/tests/cbmc/stubs/s2n_evp_signing_supported.c @@ -0,0 +1,28 @@ +/* + * 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. + */ + +#include + +static bool is_set = false; +static bool is_evp_signing_supported = false; + +bool s2n_evp_signing_supported() +{ + if (!is_set) { + is_evp_signing_supported = nondet_bool(); + is_set = true; + } + return is_evp_signing_supported; +} From 18adf0298fb606059d3286209bcaaa252b87c9b7 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Wed, 12 Feb 2025 17:35:50 -0800 Subject: [PATCH 06/19] fix: allow b64 decoding using libcrypto for sidechannel resistance (#5103) Co-authored-by: Sam Clark <3758302+goatgoose@users.noreply.github.com> Co-authored-by: Doug Chapman <54039637+dougch@users.noreply.github.com> --- stuffer/s2n_stuffer.h | 9 +- stuffer/s2n_stuffer_base64.c | 226 ++++++------------ .../cbmc/aws-verification-model-for-libcrypto | 2 +- tests/cbmc/proofs/s2n_is_base64_char/Makefile | 1 + .../s2n_stuffer_certificate_from_pem/Makefile | 1 + .../s2n_stuffer_dhparams_from_pem/Makefile | 1 + .../s2n_stuffer_private_key_from_pem/Makefile | 1 + .../proofs/s2n_stuffer_read_base64/Makefile | 3 +- .../s2n_stuffer_read_base64_harness.c | 7 +- .../proofs/s2n_stuffer_write_base64/Makefile | 1 + .../s2n_stuffer_write_base64_harness.c | 7 +- tests/unit/s2n_stuffer_base64_test.c | 110 ++++++++- 12 files changed, 197 insertions(+), 172 deletions(-) diff --git a/stuffer/s2n_stuffer.h b/stuffer/s2n_stuffer.h index f3d8f29cb3c..f63fce1a8bf 100644 --- a/stuffer/s2n_stuffer.h +++ b/stuffer/s2n_stuffer.h @@ -176,8 +176,15 @@ S2N_RESULT s2n_stuffer_write_uint8_hex(struct s2n_stuffer *stuffer, uint8_t u); S2N_RESULT s2n_stuffer_read_uint16_hex(struct s2n_stuffer *stuffer, uint16_t *u); S2N_RESULT s2n_stuffer_write_uint16_hex(struct s2n_stuffer *stuffer, uint16_t u); -/* Read and write base64 */ +/** + * Given base64 data in `stuffer`, write the decoded (binary) data into `out`. + * + * DANGER: If the data to be read is not a multiple of 4, any trailing bytes will + * be silently ignored. + */ int s2n_stuffer_read_base64(struct s2n_stuffer *stuffer, struct s2n_stuffer *out); + +/* Given some binary data in `in`, write the encoded (base64) data to `stuffer`. */ int s2n_stuffer_write_base64(struct s2n_stuffer *stuffer, struct s2n_stuffer *in); /* Useful for text manipulation ... */ diff --git a/stuffer/s2n_stuffer_base64.c b/stuffer/s2n_stuffer_base64.c index f87ccdc6571..e923de39b97 100644 --- a/stuffer/s2n_stuffer_base64.c +++ b/stuffer/s2n_stuffer_base64.c @@ -13,124 +13,72 @@ * permissions and limitations under the License. */ +#include #include #include "error/s2n_errno.h" #include "stuffer/s2n_stuffer.h" #include "utils/s2n_safety.h" -static const uint8_t b64[64] = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', - 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', - 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '+', - '/' }; - -/* Generated with this python: - * - * b64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/="; - * - * for i in range(0, 256): - * if chr(i) in b64: - * print str(b64.index(chr(i))) + ", ", - * else: - * print "255, ", - * - * if (i + 1) % 16 == 0: - * print - * - * Note that '=' maps to 64. - */ -static const uint8_t b64_inverse[256] = { 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 62, 255, 255, 255, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 255, 255, 255, 64, 255, 255, - 255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 255, 255, 255, - 255, 255, 255, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, - 51, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255 }; - bool s2n_is_base64_char(unsigned char c) { - return (b64_inverse[*((uint8_t *) (&c))] != 255); + /* use bitwise operations to minimize branching */ + uint8_t out = 0; + out ^= (c >= 'A') & (c <= 'Z'); + out ^= (c >= 'a') & (c <= 'z'); + out ^= (c >= '0') & (c <= '9'); + out ^= c == '+'; + out ^= c == '/'; + out ^= c == '='; + + return out == 1; } -/** - * NOTE: - * In general, shift before masking. This avoids needing to worry about how the - * signed bit may be handled. +/* We use the base64 decoding implementation from the libcrypto to allow for + * sidechannel-resistant base64 decoding. While OpenSSL doesn't support this, + * AWS-LC does. */ int s2n_stuffer_read_base64(struct s2n_stuffer *stuffer, struct s2n_stuffer *out) { POSIX_PRECONDITION(s2n_stuffer_validate(stuffer)); POSIX_PRECONDITION(s2n_stuffer_validate(out)); - int bytes_this_round = 3; - s2n_stack_blob(o, 4, 4); - - do { - if (s2n_stuffer_data_available(stuffer) < o.size) { - break; - } - - POSIX_GUARD(s2n_stuffer_read(stuffer, &o)); - - uint8_t value1 = b64_inverse[o.data[0]]; - uint8_t value2 = b64_inverse[o.data[1]]; - uint8_t value3 = b64_inverse[o.data[2]]; - uint8_t value4 = b64_inverse[o.data[3]]; - - /* We assume the entire thing is base64 data, thus, terminate cleanly if we encounter a non-base64 character */ - if (value1 == 255) { - /* Undo the read */ - stuffer->read_cursor -= o.size; - POSIX_BAIL(S2N_ERR_INVALID_BASE64); - } - - /* The first two characters can never be '=' and in general - * everything has to be a valid character. - */ - POSIX_ENSURE(!(value1 == 64 || value2 == 64 || value2 == 255 || value3 == 255 || value4 == 255), - S2N_ERR_INVALID_BASE64); - - if (o.data[2] == '=') { - /* If there is only one output byte, then the second value - * should have none of its bottom four bits set. - */ - POSIX_ENSURE(!(o.data[3] != '=' || value2 & 0x0f), S2N_ERR_INVALID_BASE64); - bytes_this_round = 1; - value3 = 0; - value4 = 0; - } else if (o.data[3] == '=') { - /* The last two bits of the final value should be unset */ - POSIX_ENSURE(!(value3 & 0x03), S2N_ERR_INVALID_BASE64); - - bytes_this_round = 2; - value4 = 0; - } - - /* Advance by bytes_this_round, and then fill in the data */ - POSIX_GUARD(s2n_stuffer_skip_write(out, bytes_this_round)); - uint8_t *ptr = out->blob.data + out->write_cursor - bytes_this_round; - - /* value1 maps to the first 6 bits of the first data byte */ - /* value2's top two bits are the rest */ - *ptr = ((value1 << 2) & 0xfc) | ((value2 >> 4) & 0x03); - - if (bytes_this_round > 1) { - /* Put the next four bits in the second data byte */ - /* Put the next four bits in the third data byte */ - ptr++; - *ptr = ((value2 << 4) & 0xf0) | ((value3 >> 2) & 0x0f); - } - - if (bytes_this_round > 2) { - /* Put the next two bits in the third data byte */ - /* Put the next six bits in the fourth data byte */ - ptr++; - *ptr = ((value3 << 6) & 0xc0) | (value4 & 0x3f); - } - } while (bytes_this_round == 3); + + int base64_groups = s2n_stuffer_data_available(stuffer) / 4; + if (base64_groups == 0) { + return S2N_SUCCESS; + } + int base64_data_size = base64_groups * 4; + int binary_output_size = base64_groups * 3; + + const uint32_t base64_data_offset = stuffer->read_cursor; + POSIX_GUARD(s2n_stuffer_skip_read(stuffer, base64_data_size)); + const uint8_t *start_of_base64_data = stuffer->blob.data + base64_data_offset; + + const uint32_t binary_output_offset = out->write_cursor; + POSIX_GUARD(s2n_stuffer_skip_write(out, binary_output_size)); + uint8_t *start_of_binary_output = out->blob.data + binary_output_offset; + + /* https://docs.openssl.org/master/man3/EVP_EncodeInit/ + * > This function will return the length of the data decoded or -1 on error. */ + int res = EVP_DecodeBlock(start_of_binary_output, start_of_base64_data, base64_data_size); + POSIX_ENSURE(res == binary_output_size, S2N_ERR_INVALID_BASE64); + + /* https://docs.openssl.org/1.1.1/man3/EVP_EncodeInit/ + * > The output will be padded with 0 bits if necessary to ensure that the + * > output is always 3 bytes for every 4 input bytes. + * FFFF -> 0x14 0x51 0x45 + * FFF= -> 0x14 0x51 0x00 + * FF== -> 0x14 0x00 0x00 + * F=== -> INVALID + */ + /* manually unrolled loop to prevent CBMC errors */ + POSIX_ENSURE_GTE(stuffer->read_cursor, 2); + if (stuffer->blob.data[stuffer->read_cursor - 1] == '=') { + out->write_cursor -= 1; + } + if (stuffer->blob.data[stuffer->read_cursor - 2] == '=') { + out->write_cursor -= 1; + } return S2N_SUCCESS; } @@ -139,61 +87,35 @@ int s2n_stuffer_write_base64(struct s2n_stuffer *stuffer, struct s2n_stuffer *in { POSIX_PRECONDITION(s2n_stuffer_validate(stuffer)); POSIX_PRECONDITION(s2n_stuffer_validate(in)); - s2n_stack_blob(o, 4, 4); - s2n_stack_blob(i, 3, 3); - - while (s2n_stuffer_data_available(in) > 2) { - POSIX_GUARD(s2n_stuffer_read(in, &i)); - /* Take the top 6-bits of the first data byte */ - o.data[0] = b64[(i.data[0] >> 2) & 0x3f]; + int binary_data_size = s2n_stuffer_data_available(in); + if (binary_data_size == 0) { + return S2N_SUCCESS; + } - /* Take the bottom 2-bits of the first data byte - 0b00110000 = 0x30 - * and take the top 4-bits of the second data byte - 0b00001111 = 0x0f - */ - o.data[1] = b64[((i.data[0] << 4) & 0x30) | ((i.data[1] >> 4) & 0x0f)]; + int base64_groups = binary_data_size / 3; + /* we will need to add a final padded block */ + if (binary_data_size % 3 != 0) { + base64_groups++; + } - /* Take the bottom 4-bits of the second data byte - 0b00111100 = 0x3c - * and take the top 2-bits of the third data byte - 0b00000011 = 0x03 - */ - o.data[2] = b64[((i.data[1] << 2) & 0x3c) | ((i.data[2] >> 6) & 0x03)]; + int base64_output_size = base64_groups * 4; + /* Null terminator is added */ + base64_output_size += 1; - /* Take the bottom 6-bits of the second data byte - 0b00111111 = 0x3f - */ - o.data[3] = b64[i.data[2] & 0x3f]; + const uint32_t binary_data_offset = in->read_cursor; + POSIX_GUARD(s2n_stuffer_skip_read(in, binary_data_size)); + const uint8_t *start_of_binary_data = in->blob.data + binary_data_offset; - POSIX_GUARD(s2n_stuffer_write(stuffer, &o)); - } + const uint32_t base64_output_offset = stuffer->write_cursor; + POSIX_GUARD(s2n_stuffer_skip_write(stuffer, base64_output_size)); + uint8_t *start_of_base64_output = stuffer->blob.data + base64_output_offset; - if (s2n_stuffer_data_available(in)) { - /* Read just one byte */ - i.size = 1; - POSIX_GUARD(s2n_stuffer_read(in, &i)); - uint8_t c = i.data[0]; - - /* We at least one data byte left to encode, encode - * its first six bits - */ - o.data[0] = b64[(c >> 2) & 0x3f]; - - /* And our end has to be an equals */ - o.data[3] = '='; - - /* How many bytes are actually left? */ - if (s2n_stuffer_data_available(in) == 0) { - /* We just have the last two bits to deal with */ - o.data[1] = b64[(c << 4) & 0x30]; - o.data[2] = '='; - } else { - /* Read the last byte */ - POSIX_GUARD(s2n_stuffer_read(in, &i)); - - o.data[1] = b64[((c << 4) & 0x30) | ((i.data[0] >> 4) & 0x0f)]; - o.data[2] = b64[((i.data[0] << 2) & 0x3c)]; - } - - POSIX_GUARD(s2n_stuffer_write(stuffer, &o)); - } + /* https://docs.openssl.org/master/man3/EVP_EncodeInit/ + * > The length of the data generated without the NUL terminator is returned from the function. */ + int res = EVP_EncodeBlock(start_of_base64_output, start_of_binary_data, binary_data_size); + POSIX_ENSURE(res == base64_output_size - 1, S2N_ERR_INVALID_BASE64); + POSIX_GUARD(s2n_stuffer_wipe_n(stuffer, 1)); return S2N_SUCCESS; } diff --git a/tests/cbmc/aws-verification-model-for-libcrypto b/tests/cbmc/aws-verification-model-for-libcrypto index d732f7257fc..53f8f8918f3 160000 --- a/tests/cbmc/aws-verification-model-for-libcrypto +++ b/tests/cbmc/aws-verification-model-for-libcrypto @@ -1 +1 @@ -Subproject commit d732f7257fc569e1be32d34037b111af0a058ab0 +Subproject commit 53f8f8918f3aca5127937becd8efd9bfd1a860c2 diff --git a/tests/cbmc/proofs/s2n_is_base64_char/Makefile b/tests/cbmc/proofs/s2n_is_base64_char/Makefile index f5733fa1aac..a3739ad8448 100644 --- a/tests/cbmc/proofs/s2n_is_base64_char/Makefile +++ b/tests/cbmc/proofs/s2n_is_base64_char/Makefile @@ -19,6 +19,7 @@ HARNESS_ENTRY = $(PROOF_UID)_harness HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) +PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROJECT_SOURCES += $(SRCDIR)/stuffer/s2n_stuffer_base64.c diff --git a/tests/cbmc/proofs/s2n_stuffer_certificate_from_pem/Makefile b/tests/cbmc/proofs/s2n_stuffer_certificate_from_pem/Makefile index 06e5363c302..739031caf9a 100644 --- a/tests/cbmc/proofs/s2n_stuffer_certificate_from_pem/Makefile +++ b/tests/cbmc/proofs/s2n_stuffer_certificate_from_pem/Makefile @@ -22,6 +22,7 @@ HARNESS_ENTRY = $(PROOF_UID)_harness HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) +PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c diff --git a/tests/cbmc/proofs/s2n_stuffer_dhparams_from_pem/Makefile b/tests/cbmc/proofs/s2n_stuffer_dhparams_from_pem/Makefile index 6e6394579cc..1cf337b7d76 100644 --- a/tests/cbmc/proofs/s2n_stuffer_dhparams_from_pem/Makefile +++ b/tests/cbmc/proofs/s2n_stuffer_dhparams_from_pem/Makefile @@ -22,6 +22,7 @@ HARNESS_ENTRY = $(PROOF_UID)_harness HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) +PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c diff --git a/tests/cbmc/proofs/s2n_stuffer_private_key_from_pem/Makefile b/tests/cbmc/proofs/s2n_stuffer_private_key_from_pem/Makefile index a3cfe4789c8..199de86856f 100644 --- a/tests/cbmc/proofs/s2n_stuffer_private_key_from_pem/Makefile +++ b/tests/cbmc/proofs/s2n_stuffer_private_key_from_pem/Makefile @@ -23,6 +23,7 @@ HARNESS_ENTRY = $(PROOF_UID)_harness HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) +PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c diff --git a/tests/cbmc/proofs/s2n_stuffer_read_base64/Makefile b/tests/cbmc/proofs/s2n_stuffer_read_base64/Makefile index 519c534678f..c34a55c43f8 100644 --- a/tests/cbmc/proofs/s2n_stuffer_read_base64/Makefile +++ b/tests/cbmc/proofs/s2n_stuffer_read_base64/Makefile @@ -22,6 +22,7 @@ HARNESS_ENTRY = $(PROOF_UID)_harness HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) +PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/mlock.c @@ -44,6 +45,4 @@ REMOVE_FUNCTION_BODY += __CPROVER_file_local_s2n_mem_c_s2n_mem_cleanup_impl REMOVE_FUNCTION_BODY += s2n_blob_slice REMOVE_FUNCTION_BODY += s2n_stuffer_wipe -UNWINDSET += s2n_stuffer_read_base64.19:$(MAX_BLOB_SIZE) - include ../Makefile.common diff --git a/tests/cbmc/proofs/s2n_stuffer_read_base64/s2n_stuffer_read_base64_harness.c b/tests/cbmc/proofs/s2n_stuffer_read_base64/s2n_stuffer_read_base64_harness.c index 292c90fe0d0..13821553a02 100644 --- a/tests/cbmc/proofs/s2n_stuffer_read_base64/s2n_stuffer_read_base64_harness.c +++ b/tests/cbmc/proofs/s2n_stuffer_read_base64/s2n_stuffer_read_base64_harness.c @@ -31,7 +31,7 @@ void s2n_stuffer_read_base64_harness() __CPROVER_assume(s2n_result_is_ok(s2n_stuffer_validate(out))); /* Save previous state from stuffer. */ - struct s2n_stuffer old_stuffer = *stuffer; + struct s2n_stuffer old_stuffer = *stuffer; struct store_byte_from_buffer old_byte_from_stuffer; save_byte_from_blob(&stuffer->blob, &old_byte_from_stuffer); @@ -42,11 +42,6 @@ void s2n_stuffer_read_base64_harness() if (s2n_stuffer_read_base64(stuffer, out) == S2N_SUCCESS) { assert(s2n_result_is_ok(s2n_stuffer_validate(out))); - if (s2n_stuffer_data_available(&old_stuffer) >= 4) { - size_t idx; - __CPROVER_assume(idx >= old_stuffer.read_cursor && idx < old_stuffer.write_cursor); - assert(s2n_is_base64_char(stuffer->blob.data[ idx ])); - } } assert_stuffer_immutable_fields_after_read(stuffer, &old_stuffer, &old_byte_from_stuffer); diff --git a/tests/cbmc/proofs/s2n_stuffer_write_base64/Makefile b/tests/cbmc/proofs/s2n_stuffer_write_base64/Makefile index 363bbca7fb1..fab5be76593 100644 --- a/tests/cbmc/proofs/s2n_stuffer_write_base64/Makefile +++ b/tests/cbmc/proofs/s2n_stuffer_write_base64/Makefile @@ -22,6 +22,7 @@ HARNESS_ENTRY = $(PROOF_UID)_harness HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) +PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/mlock.c diff --git a/tests/cbmc/proofs/s2n_stuffer_write_base64/s2n_stuffer_write_base64_harness.c b/tests/cbmc/proofs/s2n_stuffer_write_base64/s2n_stuffer_write_base64_harness.c index e6c818c89f2..2f73ce6f0c2 100644 --- a/tests/cbmc/proofs/s2n_stuffer_write_base64/s2n_stuffer_write_base64_harness.c +++ b/tests/cbmc/proofs/s2n_stuffer_write_base64/s2n_stuffer_write_base64_harness.c @@ -34,7 +34,7 @@ void s2n_stuffer_write_base64_harness() struct s2n_stuffer old_stuffer = *stuffer; /* Save previous state from out. */ - struct s2n_stuffer old_in = *in; + struct s2n_stuffer old_in = *in; struct store_byte_from_buffer old_byte_from_in; save_byte_from_blob(&in->blob, &old_byte_from_in); @@ -42,11 +42,6 @@ void s2n_stuffer_write_base64_harness() if (s2n_stuffer_write_base64(stuffer, in) == S2N_SUCCESS) { assert(s2n_result_is_ok(s2n_stuffer_validate(stuffer))); - if (s2n_stuffer_data_available(&old_stuffer) >= 2) { - size_t idx; - __CPROVER_assume(idx >= old_stuffer.write_cursor && idx < stuffer->write_cursor); - assert(s2n_is_base64_char(stuffer->blob.data[ idx ])); - } } assert_stuffer_immutable_fields_after_read(in, &old_in, &old_byte_from_in); diff --git a/tests/unit/s2n_stuffer_base64_test.c b/tests/unit/s2n_stuffer_base64_test.c index 65104f8c88c..09b4caa7c3a 100644 --- a/tests/unit/s2n_stuffer_base64_test.c +++ b/tests/unit/s2n_stuffer_base64_test.c @@ -20,8 +20,104 @@ #include "stuffer/s2n_stuffer.h" #include "utils/s2n_random.h" +/* Generated with this python: + * + * b64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/="; + * + * for i in range(0, 256): + * if chr(i) in b64: + * print str(b64.index(chr(i))) + ", ", + * else: + * print "255, ", + * + * if (i + 1) % 16 == 0: + * print + * + * Note that '=' maps to 64. + */ +static const uint8_t b64_inverse[256] = { 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 62, 255, 255, 255, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 255, 255, 255, 64, 255, 255, + 255, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 255, 255, 255, + 255, 255, 255, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, + 51, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, + 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255 }; + +bool s2n_is_base64_char_alternate(unsigned char c) +{ + return (b64_inverse[*((uint8_t *) (&c))] != 255); +} + int main(int argc, char **argv) { + BEGIN_TEST(); + + /* s2n_is_base64_char */ + for (uint8_t i = 0; i < 255; i++) { + EXPECT_EQUAL(s2n_is_base64_char(i), s2n_is_base64_char_alternate(i)); + }; + + /* safety: s2n_stuffer_read/write_base64 */ + { + struct s2n_stuffer a = { 0 }; + struct s2n_stuffer b = { 0 }; + EXPECT_SUCCESS(s2n_stuffer_write_base64(&a, &b)); + EXPECT_SUCCESS(s2n_stuffer_read_base64(&a, &b)); + } + + /* known-value base64 read/write tests using byte strings of `0` */ + struct { + uint8_t bytes; + const char *expected; + } test_cases[] = { + { + .bytes = 1, + .expected = "AA==", + }, + { + .bytes = 2, + .expected = "AAA=", + }, + { + .bytes = 3, + .expected = "AAAA", + }, + { + .bytes = 4, + .expected = "AAAAAA==", + }, + }; + for (int i = 0; i < s2n_array_len(test_cases); i++) { + DEFER_CLEANUP(struct s2n_stuffer binary = { 0 }, s2n_stuffer_free); + DEFER_CLEANUP(struct s2n_stuffer base64 = { 0 }, s2n_stuffer_free); + DEFER_CLEANUP(struct s2n_stuffer mirror = { 0 }, s2n_stuffer_free); + + uint32_t base64_groups = test_cases[i].bytes / 3; + if (test_cases[i].bytes % 3 != 0) { + base64_groups++; + } + EXPECT_SUCCESS(s2n_stuffer_alloc(&binary, test_cases[i].bytes)); + /* +1 for null terminator */ + EXPECT_SUCCESS(s2n_stuffer_alloc(&base64, base64_groups * 4 + 1)); + EXPECT_SUCCESS(s2n_stuffer_alloc(&mirror, base64_groups * 3)); + + for (int b = 0; b < test_cases[i].bytes; b++) { + EXPECT_SUCCESS(s2n_stuffer_write_uint8(&binary, 0)); + } + + EXPECT_SUCCESS(s2n_stuffer_write_base64(&base64, &binary)); + EXPECT_EQUAL(s2n_stuffer_data_available(&base64), strlen(test_cases[i].expected)); + EXPECT_BYTEARRAY_EQUAL(base64.blob.data, test_cases[i].expected, strlen(test_cases[i].expected)); + + EXPECT_SUCCESS(s2n_stuffer_read_base64(&base64, &mirror)); + EXPECT_EQUAL(s2n_stuffer_data_available(&mirror), test_cases[i].bytes); + EXPECT_BYTEARRAY_EQUAL(binary.blob.data, mirror.blob.data, test_cases[i].bytes); + }; + char hello_world[] = "Hello world!"; uint8_t hello_world_base64[] = "SGVsbG8gd29ybGQhAA=="; struct s2n_stuffer stuffer = { 0 }, known_data = { 0 }, scratch = { 0 }, entropy = { 0 }, mirror = { 0 }; @@ -29,9 +125,6 @@ int main(int argc, char **argv) struct s2n_blob r = { 0 }; EXPECT_SUCCESS(s2n_blob_init(&r, pad, sizeof(pad))); - BEGIN_TEST(); - EXPECT_SUCCESS(s2n_disable_tls13_in_test()); - /* Create a 100 byte stuffer */ EXPECT_SUCCESS(s2n_stuffer_alloc(&stuffer, 1000)); @@ -52,7 +145,8 @@ int main(int argc, char **argv) * where size % 3 == 0, 1, 2 */ EXPECT_SUCCESS(s2n_stuffer_alloc(&entropy, 50)); - EXPECT_SUCCESS(s2n_stuffer_alloc(&mirror, 50)); + /* +1 to give space for the null terminator written by EVP_EncodeBlock */ + EXPECT_SUCCESS(s2n_stuffer_alloc(&mirror, 50 + 1)); for (size_t i = entropy.blob.size; i > 0; i--) { EXPECT_SUCCESS(s2n_stuffer_wipe(&stuffer)); @@ -68,6 +162,14 @@ int main(int argc, char **argv) /* Read it back, decoded */ EXPECT_SUCCESS(s2n_stuffer_write_base64(&stuffer, &entropy)); + /* s2n_is_base64_char: should be true for all bytes in the stuffer */ + while (s2n_stuffer_data_available(&stuffer) > 0) { + uint8_t byte = 0; + EXPECT_SUCCESS(s2n_stuffer_read_uint8(&stuffer, &byte)); + EXPECT_TRUE(s2n_is_base64_char(byte)); + } + EXPECT_SUCCESS(s2n_stuffer_reread(&stuffer)); + /* Should be (i / 3) * 4 + a carry */ EXPECT_EQUAL((i / 3) * 4 + ((i % 3) ? 4 : 0), s2n_stuffer_data_available(&stuffer)); From 910665bebad1856f1799d627acb3b85584c46ee3 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Wed, 12 Feb 2025 21:05:19 -0800 Subject: [PATCH 07/19] fix: don't enable custom random for openssl fips (#5093) Co-authored-by: Sam Clark <3758302+goatgoose@users.noreply.github.com> --- tests/unit/s2n_openssl_test.c | 8 ++++++++ utils/s2n_random.c | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/unit/s2n_openssl_test.c b/tests/unit/s2n_openssl_test.c index d48cc8b4bd9..e2f8d61183d 100644 --- a/tests/unit/s2n_openssl_test.c +++ b/tests/unit/s2n_openssl_test.c @@ -16,6 +16,7 @@ #include "crypto/s2n_openssl.h" #include "s2n_test.h" +#include "utils/s2n_random.h" int main(int argc, char** argv) { @@ -53,5 +54,12 @@ int main(int argc, char** argv) FAIL_MSG("Testing with an unexpected libcrypto."); } + /* Ensure that custom rand is not enabled for OpenSSL 1.0.2 Fips to match + * historical behavior + */ + if (strcmp("openssl-1.0.2-fips", env_libcrypto) == 0) { + EXPECT_FALSE(s2n_supports_custom_rand()); + } + END_TEST(); } diff --git a/utils/s2n_random.c b/utils/s2n_random.c index 233a76b3060..bb7280f74dd 100644 --- a/utils/s2n_random.c +++ b/utils/s2n_random.c @@ -554,7 +554,9 @@ static int s2n_rand_init_cb_impl(void) bool s2n_supports_custom_rand(void) { -#if !defined(S2N_LIBCRYPTO_SUPPORTS_ENGINE) +#if !defined(S2N_LIBCRYPTO_SUPPORTS_ENGINE) || defined(OPENSSL_FIPS) + /* OpenSSL 1.0.2-fips is excluded to match historical behavior */ + /* OPENSSL_FIPS is only defined for 1.0.2-fips, not 3.x-fips */ return false; #else return s2n_libcrypto_is_openssl() && !s2n_is_in_fips_mode(); From 82012056bb47dd710edb06324f8605969d5fa293 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Thu, 13 Feb 2025 13:21:51 -0800 Subject: [PATCH 08/19] ci: add default provider to openssl-3.0-fips (#5114) --- codebuild/bin/s2n_fips_openssl.cnf | 4 ++-- codebuild/bin/start_codebuild.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/codebuild/bin/s2n_fips_openssl.cnf b/codebuild/bin/s2n_fips_openssl.cnf index 1a7d93a8817..514c7de24f0 100644 --- a/codebuild/bin/s2n_fips_openssl.cnf +++ b/codebuild/bin/s2n_fips_openssl.cnf @@ -56,10 +56,10 @@ alg_section = algorithm_sect # List of providers to load [provider_sect] -base = base_sect +default = default_sect fips = fips_sect -[base_sect] +[default_sect] activate = 1 [algorithm_sect] diff --git a/codebuild/bin/start_codebuild.sh b/codebuild/bin/start_codebuild.sh index af85005cc90..ab6413ed4ec 100755 --- a/codebuild/bin/start_codebuild.sh +++ b/codebuild/bin/start_codebuild.sh @@ -28,6 +28,7 @@ BUILDS=( "s2nUnitNix" "Integv2NixBatchBF1FB83F-7tcZOiMDWPH0 us-east-2 batch" "kTLS us-west-2 no-batch" + "Openssl3fipsWIP us-west-2 no-batch" ) usage() { From 7a4d1e7f774da92ff0dd99379ebfc1dc1e552dd0 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Mon, 17 Feb 2025 15:01:28 -0800 Subject: [PATCH 09/19] Revert "refactor: remove unused evp support for md5+sha1 (#5106)" (#5118) --- crypto/s2n_evp_signing.c | 51 ++----------- crypto/s2n_hash.c | 75 ++++++++++++++++--- crypto/s2n_hash.h | 2 + tests/cbmc/include/cbmc_proof/cbmc_utils.h | 5 ++ .../Makefile | 2 +- tests/cbmc/proofs/s2n_hash_copy/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_digest/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_free/Makefile | 2 +- .../s2n_hash_free/s2n_hash_free_harness.c | 10 ++- .../Makefile | 2 +- tests/cbmc/proofs/s2n_hash_init/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_new/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_reset/Makefile | 2 +- tests/cbmc/proofs/s2n_hash_update/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_copy/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_digest/Makefile | 2 +- .../Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_free/Makefile | 2 +- .../s2n_hmac_free/s2n_hmac_free_harness.c | 16 +++- tests/cbmc/proofs/s2n_hmac_init/Makefile | 1 - tests/cbmc/proofs/s2n_hmac_new/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_reset/Makefile | 2 +- tests/cbmc/proofs/s2n_hmac_update/Makefile | 2 +- tests/cbmc/sources/cbmc_utils.c | 13 +++- .../cbmc/sources/make_common_datastructures.c | 5 ++ tests/cbmc/stubs/s2n_evp_signing_supported.c | 28 ------- 26 files changed, 129 insertions(+), 109 deletions(-) delete mode 100644 tests/cbmc/stubs/s2n_evp_signing_supported.c diff --git a/crypto/s2n_evp_signing.c b/crypto/s2n_evp_signing.c index d8ec6e31539..bd0b9c5972d 100644 --- a/crypto/s2n_evp_signing.c +++ b/crypto/s2n_evp_signing.c @@ -16,7 +16,6 @@ #include "crypto/s2n_evp_signing.h" #include "crypto/s2n_evp.h" -#include "crypto/s2n_fips.h" #include "crypto/s2n_pkey.h" #include "crypto/s2n_rsa_pss.h" #include "error/s2n_errno.h" @@ -51,58 +50,18 @@ static S2N_RESULT s2n_evp_pkey_set_rsa_pss_saltlen(EVP_PKEY_CTX *pctx) #endif } -static bool s2n_evp_md5_sha1_is_supported() -{ -#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) - return true; -#else - return false; -#endif -} - -static bool s2n_evp_md_ctx_set_pkey_ctx_is_supported() +bool s2n_evp_signing_supported() { #ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_MD_CTX_SET_PKEY_CTX - return true; + /* We can only use EVP signing if the hash state has an EVP_MD_CTX + * that we can pass to the EVP signing methods. + */ + return s2n_hash_evp_fully_supported(); #else return false; #endif } -bool s2n_evp_signing_supported() -{ - /* We must use the FIPS-approved EVP APIs in FIPS mode, - * but we could also use the EVP APIs outside of FIPS mode. - * Only using the EVP APIs in FIPS mode was a choice made to reduce - * the impact of adding support for the EVP APIs. - * We should consider instead making the EVP APIs the default. - */ - if (!s2n_is_in_fips_mode()) { - return false; - } - - /* Our EVP signing logic is intended to support FIPS 140-3. - * FIPS 140-3 does not allow externally calculated digests (except for - * signing, but not verifying, with ECDSA). - * See https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Digital-Signatures, - * and note that "component" tests only exist for ECDSA sign. - * - * We currently work around that restriction by calling EVP_MD_CTX_set_pkey_ctx, - * which lets us set a key on an existing hash state. This is important - * when we need to handle signing the TLS1.2 client cert verify message, - * which requires signing the entire message transcript. If EVP_MD_CTX_set_pkey_ctx - * is unavailable (true for openssl-1.0.2), our current EVP logic will not work. - * - * FIPS 140-3 is also not possible if EVP_md5_sha1() isn't available - * (again true for openssl-1.0.2). In that case, we use two separate hash - * states to track the md5 and sha1 parts of the hash separately. That means - * that we also have to calculate the digests separately, then combine the - * result. We therefore only have an externally calculated digest available - * for signing or verifying. - */ - return s2n_evp_md_ctx_set_pkey_ctx_is_supported() && s2n_evp_md5_sha1_is_supported(); -} - /* If using EVP signing, override the sign and verify pkey methods. * The EVP methods can handle all pkey types / signature algorithms. */ diff --git a/crypto/s2n_hash.c b/crypto/s2n_hash.c index bc78b436a96..8bd57ba563b 100644 --- a/crypto/s2n_hash.c +++ b/crypto/s2n_hash.c @@ -15,22 +15,29 @@ #include "crypto/s2n_hash.h" -#include "crypto/s2n_evp_signing.h" +#include "crypto/s2n_fips.h" #include "crypto/s2n_hmac.h" #include "crypto/s2n_openssl.h" #include "error/s2n_errno.h" #include "utils/s2n_safety.h" +static bool s2n_use_custom_md5_sha1() +{ +#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) + return false; +#else + return true; +#endif +} + static bool s2n_use_evp_impl() { - /* Our current EVP signing implementation requires EVP hashing. - * - * We could use the EVP hashing impl with legacy signing, but that would - * unnecessarily complicate the logic. The only known libcrypto that - * doesn't support EVP signing is openssl-1.0.2. Just let legacy - * libcryptos use legacy methods. - */ - return s2n_evp_signing_supported(); + return s2n_is_in_fips_mode(); +} + +bool s2n_hash_evp_fully_supported() +{ + return s2n_use_evp_impl() && !s2n_use_custom_md5_sha1(); } const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg) @@ -282,8 +289,13 @@ static int s2n_low_level_hash_free(struct s2n_hash_state *state) static int s2n_evp_hash_new(struct s2n_hash_state *state) { POSIX_ENSURE_REF(state->digest.high_level.evp.ctx = S2N_EVP_MD_CTX_NEW()); + if (s2n_use_custom_md5_sha1()) { + POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx = S2N_EVP_MD_CTX_NEW()); + } + state->is_ready_for_input = 0; state->currently_in_hash = 0; + return S2N_SUCCESS; } @@ -299,6 +311,13 @@ static int s2n_evp_hash_init(struct s2n_hash_state *state, s2n_hash_algorithm al return S2N_SUCCESS; } + if (alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { + POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx); + POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, EVP_sha1(), NULL), S2N_ERR_HASH_INIT_FAILED); + POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp_md5_secondary.ctx, EVP_md5(), NULL), S2N_ERR_HASH_INIT_FAILED); + return S2N_SUCCESS; + } + POSIX_ENSURE_REF(s2n_hash_alg_to_evp_md(alg)); POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, s2n_hash_alg_to_evp_md(alg), NULL), S2N_ERR_HASH_INIT_FAILED); return S2N_SUCCESS; @@ -316,6 +335,12 @@ static int s2n_evp_hash_update(struct s2n_hash_state *state, const void *data, u POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp.ctx)); POSIX_GUARD_OSSL(EVP_DigestUpdate(state->digest.high_level.evp.ctx, data, size), S2N_ERR_HASH_UPDATE_FAILED); + + if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { + POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp_md5_secondary.ctx)); + POSIX_GUARD_OSSL(EVP_DigestUpdate(state->digest.high_level.evp_md5_secondary.ctx, data, size), S2N_ERR_HASH_UPDATE_FAILED); + } + return S2N_SUCCESS; } @@ -337,6 +362,23 @@ static int s2n_evp_hash_digest(struct s2n_hash_state *state, void *out, uint32_t POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp.ctx)); + if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { + POSIX_ENSURE_REF(EVP_MD_CTX_md(state->digest.high_level.evp_md5_secondary.ctx)); + + uint8_t sha1_digest_size = 0; + POSIX_GUARD(s2n_hash_digest_size(S2N_HASH_SHA1, &sha1_digest_size)); + + unsigned int sha1_primary_digest_size = sha1_digest_size; + unsigned int md5_secondary_digest_size = digest_size - sha1_primary_digest_size; + + POSIX_ENSURE(EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= sha1_digest_size, S2N_ERR_HASH_DIGEST_FAILED); + POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp_md5_secondary.ctx) <= md5_secondary_digest_size, S2N_ERR_HASH_DIGEST_FAILED); + + POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, ((uint8_t *) out) + MD5_DIGEST_LENGTH, &sha1_primary_digest_size), S2N_ERR_HASH_DIGEST_FAILED); + POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp_md5_secondary.ctx, out, &md5_secondary_digest_size), S2N_ERR_HASH_DIGEST_FAILED); + return S2N_SUCCESS; + } + POSIX_ENSURE((size_t) EVP_MD_CTX_size(state->digest.high_level.evp.ctx) <= digest_size, S2N_ERR_HASH_DIGEST_FAILED); POSIX_GUARD_OSSL(EVP_DigestFinal_ex(state->digest.high_level.evp.ctx, out, &digest_size), S2N_ERR_HASH_DIGEST_FAILED); return S2N_SUCCESS; @@ -355,12 +397,21 @@ static int s2n_evp_hash_copy(struct s2n_hash_state *to, struct s2n_hash_state *f POSIX_ENSURE_REF(to->digest.high_level.evp.ctx); POSIX_GUARD_OSSL(EVP_MD_CTX_copy_ex(to->digest.high_level.evp.ctx, from->digest.high_level.evp.ctx), S2N_ERR_HASH_COPY_FAILED); + + if (from->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { + POSIX_ENSURE_REF(to->digest.high_level.evp_md5_secondary.ctx); + POSIX_GUARD_OSSL(EVP_MD_CTX_copy_ex(to->digest.high_level.evp_md5_secondary.ctx, from->digest.high_level.evp_md5_secondary.ctx), S2N_ERR_HASH_COPY_FAILED); + } + return S2N_SUCCESS; } static int s2n_evp_hash_reset(struct s2n_hash_state *state) { POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(state->digest.high_level.evp.ctx), S2N_ERR_HASH_WIPE_FAILED); + if (state->alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { + POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(state->digest.high_level.evp_md5_secondary.ctx), S2N_ERR_HASH_WIPE_FAILED); + } /* hash_init resets the ready_for_input and currently_in_hash fields. */ return s2n_evp_hash_init(state, state->alg); @@ -370,6 +421,12 @@ static int s2n_evp_hash_free(struct s2n_hash_state *state) { S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp.ctx); state->digest.high_level.evp.ctx = NULL; + + if (s2n_use_custom_md5_sha1()) { + S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp_md5_secondary.ctx); + state->digest.high_level.evp_md5_secondary.ctx = NULL; + } + state->is_ready_for_input = 0; return S2N_SUCCESS; } diff --git a/crypto/s2n_hash.h b/crypto/s2n_hash.h index 8c0e7385af0..03643a03e9c 100644 --- a/crypto/s2n_hash.h +++ b/crypto/s2n_hash.h @@ -54,6 +54,8 @@ union s2n_hash_low_level_digest { /* The evp_digest stores all OpenSSL structs to be used with OpenSSL's EVP hash API's. */ struct s2n_hash_evp_digest { struct s2n_evp_digest evp; + /* Always store a secondary evp_digest to allow resetting a hash_state to MD5_SHA1 from another alg. */ + struct s2n_evp_digest evp_md5_secondary; }; /* s2n_hash_state stores the s2n_hash implementation being used (low-level or EVP), diff --git a/tests/cbmc/include/cbmc_proof/cbmc_utils.h b/tests/cbmc/include/cbmc_proof/cbmc_utils.h index 4c58d4d5451..19c2b60b0e7 100644 --- a/tests/cbmc/include/cbmc_proof/cbmc_utils.h +++ b/tests/cbmc/include/cbmc_proof/cbmc_utils.h @@ -47,8 +47,13 @@ struct rc_keys_from_evp_pkey_ctx { int pkey_eckey_refs; }; +/** + * In the `rc_keys_from_hash_state`, we store two `rc_keys_from_evp_pkey_ctx` objects: + * one for the `evp` field and another for `evp_md5_secondary` field. + */ struct rc_keys_from_hash_state { struct rc_keys_from_evp_pkey_ctx evp; + struct rc_keys_from_evp_pkey_ctx evp_md5; }; /** diff --git a/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile b/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile index 2f7df8c00fa..3e88a91d608 100644 --- a/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile +++ b/tests/cbmc/proofs/s2n_hash_const_time_get_currently_in_hash_block/Makefile @@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_copy/Makefile b/tests/cbmc/proofs/s2n_hash_copy/Makefile index 82cb3b54e39..02e2c6c99be 100644 --- a/tests/cbmc/proofs/s2n_hash_copy/Makefile +++ b/tests/cbmc/proofs/s2n_hash_copy/Makefile @@ -25,7 +25,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_ensure.c diff --git a/tests/cbmc/proofs/s2n_hash_digest/Makefile b/tests/cbmc/proofs/s2n_hash_digest/Makefile index 53a8f450683..f4e8cd51fe0 100644 --- a/tests/cbmc/proofs/s2n_hash_digest/Makefile +++ b/tests/cbmc/proofs/s2n_hash_digest/Makefile @@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c diff --git a/tests/cbmc/proofs/s2n_hash_free/Makefile b/tests/cbmc/proofs/s2n_hash_free/Makefile index a873b020619..676317f1dc2 100644 --- a/tests/cbmc/proofs/s2n_hash_free/Makefile +++ b/tests/cbmc/proofs/s2n_hash_free/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c b/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c index f8fc362ad92..9c646a0b64f 100644 --- a/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c +++ b/tests/cbmc/proofs/s2n_hash_free/s2n_hash_free_harness.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "crypto/s2n_evp_signing.h" +#include "crypto/s2n_fips.h" #include "crypto/s2n_hash.h" #include @@ -35,8 +35,9 @@ void s2n_hash_free_harness() assert(s2n_hash_free(state) == S2N_SUCCESS); if (state != NULL) { assert(state->hash_impl->free != NULL); - if (s2n_evp_signing_supported()) { + if (s2n_is_in_fips_mode()) { assert(state->digest.high_level.evp.ctx == NULL); + assert(state->digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_hash_state); } else { assert_rc_unchanged_on_hash_state(&saved_hash_state); @@ -46,10 +47,11 @@ void s2n_hash_free_harness() /* Cleanup after expected error cases, for memory leak check. */ if (state != NULL) { - /* 1. `free` leftover EVP_MD_CTX objects if `s2n_evp_signing_supported`, + /* 1. `free` leftover EVP_MD_CTX objects if `s2n_is_in_fips_mode`, since `s2n_hash_free` is a NO-OP in that case. */ - if (!s2n_evp_signing_supported()) { + if (!s2n_is_in_fips_mode()) { S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp.ctx); + S2N_EVP_MD_CTX_FREE(state->digest.high_level.evp_md5_secondary.ctx); } /* 2. `free` leftover reference-counted keys (i.e. those with non-zero ref-count), diff --git a/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile b/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile index 7be94f92f02..dd6491d63a4 100644 --- a/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile +++ b/tests/cbmc/proofs/s2n_hash_get_currently_in_hash_total/Makefile @@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_init/Makefile b/tests/cbmc/proofs/s2n_hash_init/Makefile index f7ae8963b2b..5d905e7c2ef 100644 --- a/tests/cbmc/proofs/s2n_hash_init/Makefile +++ b/tests/cbmc/proofs/s2n_hash_init/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_new/Makefile b/tests/cbmc/proofs/s2n_hash_new/Makefile index c156cd61aaa..da1f324b484 100644 --- a/tests/cbmc/proofs/s2n_hash_new/Makefile +++ b/tests/cbmc/proofs/s2n_hash_new/Makefile @@ -22,7 +22,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hash_reset/Makefile b/tests/cbmc/proofs/s2n_hash_reset/Makefile index b0f408b03eb..55e08b1269c 100644 --- a/tests/cbmc/proofs/s2n_hash_reset/Makefile +++ b/tests/cbmc/proofs/s2n_hash_reset/Makefile @@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c diff --git a/tests/cbmc/proofs/s2n_hash_update/Makefile b/tests/cbmc/proofs/s2n_hash_update/Makefile index 13e35003793..f9239e1946e 100644 --- a/tests/cbmc/proofs/s2n_hash_update/Makefile +++ b/tests/cbmc/proofs/s2n_hash_update/Makefile @@ -27,7 +27,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c diff --git a/tests/cbmc/proofs/s2n_hmac_copy/Makefile b/tests/cbmc/proofs/s2n_hmac_copy/Makefile index e3e1c211460..87231753868 100644 --- a/tests/cbmc/proofs/s2n_hmac_copy/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_copy/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_digest/Makefile b/tests/cbmc/proofs/s2n_hmac_digest/Makefile index 7012a41ea8c..90781cff1ba 100644 --- a/tests/cbmc/proofs/s2n_hmac_digest/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_digest/Makefile @@ -19,7 +19,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_copy.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_digest.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_update.c diff --git a/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile b/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile index 014cc1d9955..7fdb8a76da8 100644 --- a/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_digest_two_compression_rounds/Makefile @@ -19,7 +19,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_copy.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_digest.c PROOF_SOURCES += $(PROOF_STUB)/s2n_hash_reset.c diff --git a/tests/cbmc/proofs/s2n_hmac_free/Makefile b/tests/cbmc/proofs/s2n_hmac_free/Makefile index 8791513153f..fd14d427e94 100644 --- a/tests/cbmc/proofs/s2n_hmac_free/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_free/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c b/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c index fbc0f9f3625..aeff29282ad 100644 --- a/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c +++ b/tests/cbmc/proofs/s2n_hmac_free/s2n_hmac_free_harness.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "crypto/s2n_evp_signing.h" +#include "crypto/s2n_fips.h" #include "crypto/s2n_hash.h" #include @@ -44,17 +44,21 @@ void s2n_hmac_free_harness() assert(state->outer.hash_impl->free != NULL); assert(state->outer_just_key.hash_impl->free != NULL); - if (s2n_evp_signing_supported()) { + if (s2n_is_in_fips_mode()) { assert(state->inner.digest.high_level.evp.ctx == NULL); + assert(state->inner.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_inner_hash_state); assert(state->inner_just_key.digest.high_level.evp.ctx == NULL); + assert(state->inner_just_key.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_inner_just_key_hash_state); assert(state->outer.digest.high_level.evp.ctx == NULL); + assert(state->outer.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_outer_hash_state); assert(state->outer_just_key.digest.high_level.evp.ctx == NULL); + assert(state->outer_just_key.digest.high_level.evp_md5_secondary.ctx == NULL); assert_rc_decrement_on_hash_state(&saved_outer_just_key_hash_state); } else { assert_rc_unchanged_on_hash_state(&saved_inner_hash_state); @@ -71,13 +75,17 @@ void s2n_hmac_free_harness() /* Cleanup after expected error cases, for memory leak check. */ if (state != NULL) { - /* 1. `free` leftover EVP_MD_CTX objects if `s2n_evp_signing_supported`, + /* 1. `free` leftover EVP_MD_CTX objects if `s2n_is_in_fips_mode`, since `s2n_hash_free` is a NO-OP in that case. */ - if (!s2n_evp_signing_supported()) { + if (!s2n_is_in_fips_mode()) { S2N_EVP_MD_CTX_FREE(state->inner.digest.high_level.evp.ctx); + S2N_EVP_MD_CTX_FREE(state->inner.digest.high_level.evp_md5_secondary.ctx); S2N_EVP_MD_CTX_FREE(state->inner_just_key.digest.high_level.evp.ctx); + S2N_EVP_MD_CTX_FREE(state->inner_just_key.digest.high_level.evp_md5_secondary.ctx); S2N_EVP_MD_CTX_FREE(state->outer.digest.high_level.evp.ctx); + S2N_EVP_MD_CTX_FREE(state->outer.digest.high_level.evp_md5_secondary.ctx); S2N_EVP_MD_CTX_FREE(state->outer_just_key.digest.high_level.evp.ctx); + S2N_EVP_MD_CTX_FREE(state->outer_just_key.digest.high_level.evp_md5_secondary.ctx); } /* 2. `free` leftover reference-counted keys (i.e. those with non-zero ref-count), diff --git a/tests/cbmc/proofs/s2n_hmac_init/Makefile b/tests/cbmc/proofs/s2n_hmac_init/Makefile index 964e84d9e6e..4931abbccda 100644 --- a/tests/cbmc/proofs/s2n_hmac_init/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_init/Makefile @@ -24,7 +24,6 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOF_STUB)/s2n_libcrypto_is_awslc.c PROOF_SOURCES += $(PROOF_STUB)/darwin_check_fd_set_overflow.c diff --git a/tests/cbmc/proofs/s2n_hmac_new/Makefile b/tests/cbmc/proofs/s2n_hmac_new/Makefile index b503264b6d6..b553599d793 100644 --- a/tests/cbmc/proofs/s2n_hmac_new/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_new/Makefile @@ -21,7 +21,7 @@ HARNESS_FILE = $(HARNESS_ENTRY).c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_reset/Makefile b/tests/cbmc/proofs/s2n_hmac_reset/Makefile index 066dcc3489c..c74c27914fd 100644 --- a/tests/cbmc/proofs/s2n_hmac_reset/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_reset/Makefile @@ -24,7 +24,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/ec_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/evp_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hash.c diff --git a/tests/cbmc/proofs/s2n_hmac_update/Makefile b/tests/cbmc/proofs/s2n_hmac_update/Makefile index eb16a293d47..0098edb5975 100644 --- a/tests/cbmc/proofs/s2n_hmac_update/Makefile +++ b/tests/cbmc/proofs/s2n_hmac_update/Makefile @@ -26,7 +26,7 @@ PROOF_SOURCES += $(OPENSSL_SOURCE)/md5_override.c PROOF_SOURCES += $(OPENSSL_SOURCE)/sha_override.c PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c PROOF_SOURCES += $(PROOF_STUB)/s2n_calculate_stacktrace.c -PROOF_SOURCES += $(PROOF_STUB)/s2n_evp_signing_supported.c +PROOF_SOURCES += $(PROOF_STUB)/s2n_is_in_fips_mode.c PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) PROJECT_SOURCES += $(SRCDIR)/crypto/s2n_hmac.c diff --git a/tests/cbmc/sources/cbmc_utils.c b/tests/cbmc/sources/cbmc_utils.c index f8eb8943627..733c87fb9e1 100644 --- a/tests/cbmc/sources/cbmc_utils.c +++ b/tests/cbmc/sources/cbmc_utils.c @@ -220,11 +220,13 @@ void assert_rc_unchanged_on_evp_pkey_ctx(struct rc_keys_from_evp_pkey_ctx *stora void assert_rc_decrement_on_hash_state(struct rc_keys_from_hash_state *storage) { assert_rc_decrement_on_evp_pkey_ctx(&storage->evp); + assert_rc_decrement_on_evp_pkey_ctx(&storage->evp_md5); } void assert_rc_unchanged_on_hash_state(struct rc_keys_from_hash_state *storage) { assert_rc_unchanged_on_evp_pkey_ctx(&storage->evp); + assert_rc_unchanged_on_evp_pkey_ctx(&storage->evp_md5); } void save_abstract_evp_ctx(const EVP_PKEY_CTX *pctx, struct rc_keys_from_evp_pkey_ctx *storage) @@ -243,7 +245,9 @@ void save_rc_keys_from_hash_state(const struct s2n_hash_state *state, struct rc_ { storage->evp.pkey = NULL; storage->evp.pkey_eckey = NULL; - storage->evp.pkey_refs = storage->evp.pkey_eckey_refs = 0; + storage->evp_md5.pkey = NULL; + storage->evp_md5.pkey_eckey = NULL; + storage->evp.pkey_refs = storage->evp.pkey_eckey_refs = storage->evp_md5.pkey_refs = storage->evp_md5.pkey_eckey_refs = 0; if (state) { if (state->digest.high_level.evp.ctx) { @@ -251,6 +255,12 @@ void save_rc_keys_from_hash_state(const struct s2n_hash_state *state, struct rc_ save_abstract_evp_ctx(state->digest.high_level.evp.ctx->pctx, &storage->evp); } } + + if (state->digest.high_level.evp_md5_secondary.ctx) { + if (state->digest.high_level.evp_md5_secondary.ctx->pctx) { + save_abstract_evp_ctx(state->digest.high_level.evp_md5_secondary.ctx->pctx, &storage->evp_md5); + } + } } } @@ -269,4 +279,5 @@ void free_rc_keys_from_evp_pkey_ctx(struct rc_keys_from_evp_pkey_ctx *pctx) void free_rc_keys_from_hash_state(struct rc_keys_from_hash_state *storage) { free_rc_keys_from_evp_pkey_ctx(&storage->evp); + free_rc_keys_from_evp_pkey_ctx(&storage->evp_md5); } diff --git a/tests/cbmc/sources/make_common_datastructures.c b/tests/cbmc/sources/make_common_datastructures.c index b159986ad8f..59c7590f10d 100644 --- a/tests/cbmc/sources/make_common_datastructures.c +++ b/tests/cbmc/sources/make_common_datastructures.c @@ -270,6 +270,7 @@ void cbmc_populate_s2n_hash_state(struct s2n_hash_state* state) * If required, this initialization should be done in the validation function. */ cbmc_populate_s2n_evp_digest(&state->digest.high_level.evp); + cbmc_populate_s2n_evp_digest(&state->digest.high_level.evp_md5_secondary); } struct s2n_hash_state* cbmc_allocate_s2n_hash_state() @@ -299,9 +300,13 @@ void cbmc_populate_s2n_hmac_evp_backup(struct s2n_hmac_evp_backup *backup) { CBMC_ENSURE_REF(backup); cbmc_populate_s2n_evp_digest(&backup->inner.evp); + cbmc_populate_s2n_evp_digest(&backup->inner.evp_md5_secondary); cbmc_populate_s2n_evp_digest(&backup->inner_just_key.evp); + cbmc_populate_s2n_evp_digest(&backup->inner_just_key.evp_md5_secondary); cbmc_populate_s2n_evp_digest(&backup->outer.evp); + cbmc_populate_s2n_evp_digest(&backup->outer.evp_md5_secondary); cbmc_populate_s2n_evp_digest(&backup->outer_just_key.evp); + cbmc_populate_s2n_evp_digest(&backup->outer_just_key.evp_md5_secondary); } struct s2n_hmac_evp_backup* cbmc_allocate_s2n_hmac_evp_backup() diff --git a/tests/cbmc/stubs/s2n_evp_signing_supported.c b/tests/cbmc/stubs/s2n_evp_signing_supported.c deleted file mode 100644 index 32d709a1167..00000000000 --- a/tests/cbmc/stubs/s2n_evp_signing_supported.c +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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. - */ - -#include - -static bool is_set = false; -static bool is_evp_signing_supported = false; - -bool s2n_evp_signing_supported() -{ - if (!is_set) { - is_evp_signing_supported = nondet_bool(); - is_set = true; - } - return is_evp_signing_supported; -} From f2e918118c2ca88c21a1ca62bfd6318970d95950 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Mon, 17 Feb 2025 18:03:53 -0500 Subject: [PATCH 10/19] Add new security policy (20250211) (#5111) --- tests/pems/gen_self_signed_cert.sh | 2 +- tests/unit/s2n_security_policies_test.c | 25 +++++++++++++++++++++++++ tls/s2n_cipher_preferences.c | 11 +++++++++++ tls/s2n_cipher_preferences.h | 1 + tls/s2n_security_policies.c | 20 ++++++++++++++++++++ tls/s2n_security_policies.h | 1 + 6 files changed, 59 insertions(+), 1 deletion(-) diff --git a/tests/pems/gen_self_signed_cert.sh b/tests/pems/gen_self_signed_cert.sh index 058934644b3..5bc4e3e7046 100755 --- a/tests/pems/gen_self_signed_cert.sh +++ b/tests/pems/gen_self_signed_cert.sh @@ -139,7 +139,7 @@ if [ "$KEY_TYPE" == "rsa" ]; then openssl req -x509 -config "$cert_conf_path" -newkey rsa:${RSA_KEY_SIZE} -${HASH_ALG} -nodes -keyout ${PREFIX}rsa_key.pem -out ${PREFIX}rsa_cert.pem -days 36500 elif [ "$KEY_TYPE" == "ecdsa" ]; then openssl ecparam -out "${PREFIX}ecdsa_key.pem" -name "$CURVE_NAME" -genkey - openssl req -new -config "$cert_conf_path" -days 36500 -nodes -x509 -key "${PREFIX}ecdsa_key.pem" -out "${PREFIX}ecdsa_cert.pem" + openssl req -new -config "$cert_conf_path" -${HASH_ALG} -days 36500 -nodes -x509 -key "${PREFIX}ecdsa_key.pem" -out "${PREFIX}ecdsa_cert.pem" else echo "Incorrect key-type: $KEY_TYPE" usage ; diff --git a/tests/unit/s2n_security_policies_test.c b/tests/unit/s2n_security_policies_test.c index d310c2b4bd6..7545162efc1 100644 --- a/tests/unit/s2n_security_policies_test.c +++ b/tests/unit/s2n_security_policies_test.c @@ -105,6 +105,9 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&ecdsa_chain_and_key, S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN, S2N_DEFAULT_ECDSA_TEST_PRIVATE_KEY)); + DEFER_CLEANUP(struct s2n_cert_chain_and_key *ecdsa_sha384_chain_and_key = NULL, s2n_cert_chain_and_key_ptr_free); + EXPECT_SUCCESS(s2n_test_cert_permutation_load_server_chain(&ecdsa_sha384_chain_and_key, "ec", "ecdsa", "p384", "sha384")); + DEFER_CLEANUP(struct s2n_cert_chain_and_key *rsa_pss_chain_and_key = NULL, s2n_cert_chain_and_key_ptr_free); if (s2n_is_rsa_pss_certs_supported()) { EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&rsa_pss_chain_and_key, @@ -1019,6 +1022,28 @@ int main(int argc, char **argv) "test_all_tls13", ecdsa_chain_and_key), S2N_ERR_CIPHER_NOT_SUPPORTED); }; + + /* We know of customers that expect to move between the policies in + * this section without multi-phased rollouts, so avoid inadvertant + * breakage by verifying compatibility. + */ + if (s2n_is_tls13_fully_supported()) { + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_rfc9151, "default_tls13", ecdsa_sha384_chain_and_key)); + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_rfc9151, "default_fips", ecdsa_sha384_chain_and_key)); + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_rfc9151, "20250211", ecdsa_sha384_chain_and_key)); + + /* default_tls13 is currently 20240503 */ + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240503, "rfc9151", ecdsa_sha384_chain_and_key)); + /* default_fips is currently 20240502 */ + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240502, "rfc9151", ecdsa_sha384_chain_and_key)); + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20250211, "rfc9151", ecdsa_sha384_chain_and_key)); + + /* default_tls13 > 20250211 + * note this does not require a sha384 key. + */ + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240503, "20250211", ecdsa_chain_and_key)); + EXPECT_OK(s2n_test_security_policies_compatible(&security_policy_20240503, "default_tls13", ecdsa_chain_and_key)); + }; }; /* Sanity check that changes to default security policies are not completely diff --git a/tls/s2n_cipher_preferences.c b/tls/s2n_cipher_preferences.c index 13d7bcd8a21..307eaffa0de 100644 --- a/tls/s2n_cipher_preferences.c +++ b/tls/s2n_cipher_preferences.c @@ -2180,4 +2180,15 @@ const struct s2n_cipher_preferences cipher_preferences_rfc9151 = { .allow_chacha20_boosting = false, }; +struct s2n_cipher_suite *cipher_suites_20250211[] = { + /* TLS1.3 */ + &s2n_tls13_aes_256_gcm_sha384, +}; + +const struct s2n_cipher_preferences cipher_preferences_20250211 = { + .count = s2n_array_len(cipher_suites_20250211), + .suites = cipher_suites_20250211, + .allow_chacha20_boosting = false, +}; + /* clang-format on */ diff --git a/tls/s2n_cipher_preferences.h b/tls/s2n_cipher_preferences.h index 132b52052b8..4d200f81062 100644 --- a/tls/s2n_cipher_preferences.h +++ b/tls/s2n_cipher_preferences.h @@ -62,6 +62,7 @@ extern const struct s2n_cipher_preferences cipher_preferences_20240603; extern const struct s2n_cipher_preferences cipher_preferences_20241008; extern const struct s2n_cipher_preferences cipher_preferences_20241008_gcm; extern const struct s2n_cipher_preferences cipher_preferences_20241009; +extern const struct s2n_cipher_preferences cipher_preferences_20250211; extern const struct s2n_cipher_preferences cipher_preferences_default_fips; diff --git a/tls/s2n_security_policies.c b/tls/s2n_security_policies.c index edb0f7868a5..28e6f297475 100644 --- a/tls/s2n_security_policies.c +++ b/tls/s2n_security_policies.c @@ -1131,6 +1131,25 @@ const struct s2n_security_policy security_policy_rfc9151 = { .certificate_preferences_apply_locally = true, }; +/* + * This security policy is a mix of default_tls13 (20240503) and rfc9151, with + * a primary requirement that AES-256 is the ciphersuite chosen. Other + * requirements are generally picked to raise minimum thresholds (e.g., + * requiring TLS 1.3) where possible without losing compatibility with modern + * default_tls13 clients or servers. + */ +const struct s2n_security_policy security_policy_20250211 = { + .minimum_protocol_version = S2N_TLS13, + .cipher_preferences = &cipher_preferences_20250211, + .kem_preferences = &kem_preferences_null, + .signature_preferences = &s2n_signature_preferences_rfc9151, + .certificate_signature_preferences = &s2n_certificate_signature_preferences_20201110, + .ecc_preferences = &s2n_ecc_preferences_20210816, + .rules = { + [S2N_PERFECT_FORWARD_SECRECY] = true, + }, +}; + const struct s2n_security_policy security_policy_test_all = { .minimum_protocol_version = S2N_SSLv3, .cipher_preferences = &cipher_preferences_test_all, @@ -1331,6 +1350,7 @@ struct s2n_security_policy_selection security_policy_selection[] = { { .version = "20210816", .security_policy = &security_policy_20210816, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "20210816_GCM", .security_policy = &security_policy_20210816_gcm, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "20240603", .security_policy = &security_policy_20240603, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, + { .version = "20250211", .security_policy = &security_policy_20250211, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "rfc9151", .security_policy = &security_policy_rfc9151, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "test_all", .security_policy = &security_policy_test_all, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "test_all_fips", .security_policy = &security_policy_test_all_fips, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, diff --git a/tls/s2n_security_policies.h b/tls/s2n_security_policies.h index 762c15fdca8..095353aa22e 100644 --- a/tls/s2n_security_policies.h +++ b/tls/s2n_security_policies.h @@ -129,6 +129,7 @@ extern const struct s2n_security_policy security_policy_20240603; extern const struct s2n_security_policy security_policy_20240730; extern const struct s2n_security_policy security_policy_20241001; extern const struct s2n_security_policy security_policy_20241001_pq_mixed; +extern const struct s2n_security_policy security_policy_20250211; extern const struct s2n_security_policy security_policy_rfc9151; extern const struct s2n_security_policy security_policy_test_all; From dcbec3ea679397cd5270fda606701f77ca078cfe Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Mon, 17 Feb 2025 16:21:50 -0800 Subject: [PATCH 11/19] refactor: move "s2n_libcrypto_is" methods into s2n_libcrypto.h (#5117) --- crypto/s2n_drbg.c | 1 + crypto/s2n_ecdsa.c | 1 - crypto/s2n_fips.c | 2 ++ crypto/s2n_hash.c | 1 - crypto/s2n_libcrypto.h | 9 ++++++++- crypto/s2n_openssl.h | 9 --------- crypto/s2n_pq.c | 2 -- crypto/s2n_rsa.c | 1 + crypto/s2n_rsa_pss.c | 1 - crypto/s2n_rsa_signing.h | 1 - .../cbmc/include/cbmc_proof/make_common_datastructures.h | 1 + tests/cbmc/stubs/s2n_libcrypto_is_awslc.c | 1 - tests/unit/s2n_build_test.c | 2 +- tests/unit/s2n_cert_authorities_test.c | 1 + tests/unit/s2n_config_test.c | 2 ++ tests/unit/s2n_evp_signing_test.c | 1 + tests/unit/s2n_fips_test.c | 1 + tests/unit/s2n_hash_test.c | 1 + tests/unit/s2n_hkdf_test.c | 1 + tests/unit/{s2n_openssl_test.c => s2n_libcrypto_test.c} | 2 +- tests/unit/s2n_locking_test.c | 1 + tests/unit/s2n_pq_mlkem_test.c | 1 - tests/unit/s2n_random_test.c | 1 + tests/unit/s2n_tls13_secrets_rfc8448_test.c | 1 + tests/unit/s2n_tls_prf_test.c | 2 +- .../s2n_x509_validator_certificate_signatures_test.c | 1 - tests/unit/s2n_x509_validator_test.c | 2 ++ tests/unit/s2n_x509_validator_time_verification_test.c | 1 + tls/s2n_prf.c | 1 - tls/s2n_prf.h | 1 - tls/s2n_x509_validator.c | 1 - utils/s2n_random.c | 1 + 32 files changed, 30 insertions(+), 25 deletions(-) rename tests/unit/{s2n_openssl_test.c => s2n_libcrypto_test.c} (98%) diff --git a/crypto/s2n_drbg.c b/crypto/s2n_drbg.c index 99e4c682e2d..869591b6671 100644 --- a/crypto/s2n_drbg.c +++ b/crypto/s2n_drbg.c @@ -18,6 +18,7 @@ #include #include +#include "crypto/s2n_openssl.h" #include "utils/s2n_blob.h" #include "utils/s2n_random.h" #include "utils/s2n_safety.h" diff --git a/crypto/s2n_ecdsa.c b/crypto/s2n_ecdsa.c index 36558f75345..cefd76af232 100644 --- a/crypto/s2n_ecdsa.c +++ b/crypto/s2n_ecdsa.c @@ -22,7 +22,6 @@ #include "crypto/s2n_ecc_evp.h" #include "crypto/s2n_evp_signing.h" #include "crypto/s2n_hash.h" -#include "crypto/s2n_openssl.h" #include "crypto/s2n_pkey.h" #include "error/s2n_errno.h" #include "stuffer/s2n_stuffer.h" diff --git a/crypto/s2n_fips.c b/crypto/s2n_fips.c index 85b03ca955e..fd2e03057ad 100644 --- a/crypto/s2n_fips.c +++ b/crypto/s2n_fips.c @@ -17,6 +17,8 @@ #include +#include "crypto/s2n_libcrypto.h" +#include "crypto/s2n_openssl.h" #include "utils/s2n_init.h" #include "utils/s2n_safety.h" diff --git a/crypto/s2n_hash.c b/crypto/s2n_hash.c index 8bd57ba563b..001c3bc547d 100644 --- a/crypto/s2n_hash.c +++ b/crypto/s2n_hash.c @@ -17,7 +17,6 @@ #include "crypto/s2n_fips.h" #include "crypto/s2n_hmac.h" -#include "crypto/s2n_openssl.h" #include "error/s2n_errno.h" #include "utils/s2n_safety.h" diff --git a/crypto/s2n_libcrypto.h b/crypto/s2n_libcrypto.h index c2232b9bead..2ac736a3ad4 100644 --- a/crypto/s2n_libcrypto.h +++ b/crypto/s2n_libcrypto.h @@ -17,7 +17,14 @@ #include "utils/s2n_result.h" +bool s2n_libcrypto_is_openssl(void); +bool s2n_libcrypto_is_openssl_fips(void); +bool s2n_libcrypto_is_awslc(void); +bool s2n_libcrypto_is_awslc_fips(void); +bool s2n_libcrypto_is_boringssl(void); +bool s2n_libcrypto_is_libressl(void); + uint64_t s2n_libcrypto_awslc_api_version(void); S2N_RESULT s2n_libcrypto_validate_runtime(void); const char *s2n_libcrypto_get_version_name(void); -bool s2n_libcrypto_supports_flag_no_check_time(); +bool s2n_libcrypto_supports_flag_no_check_time(void); diff --git a/crypto/s2n_openssl.h b/crypto/s2n_openssl.h index cd2763a8fe2..7d05bed132b 100644 --- a/crypto/s2n_openssl.h +++ b/crypto/s2n_openssl.h @@ -15,8 +15,6 @@ #pragma once -#include - /** * openssl with OPENSSL_VERSION_NUMBER < 0x10100003L made data type details unavailable * libressl use openssl with data type details available, but mandatorily set @@ -49,10 +47,3 @@ #define s2n_evp_ctx_init(ctx) EVP_CIPHER_CTX_init(ctx) #define RESULT_EVP_CTX_INIT(ctx) EVP_CIPHER_CTX_init(ctx) #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); -bool s2n_libcrypto_is_boringssl(); -bool s2n_libcrypto_is_libressl(); diff --git a/crypto/s2n_pq.c b/crypto/s2n_pq.c index cb9795311a3..d7bfc238a5e 100644 --- a/crypto/s2n_pq.c +++ b/crypto/s2n_pq.c @@ -15,8 +15,6 @@ #include "s2n_pq.h" -#include "crypto/s2n_openssl.h" - bool s2n_libcrypto_supports_evp_kem() { /* S2N_LIBCRYPTO_SUPPORTS_EVP_KEM will be auto-detected and #defined if diff --git a/crypto/s2n_rsa.c b/crypto/s2n_rsa.c index a89b59612b7..76cd277d6c9 100644 --- a/crypto/s2n_rsa.c +++ b/crypto/s2n_rsa.c @@ -22,6 +22,7 @@ #include "crypto/s2n_drbg.h" #include "crypto/s2n_evp_signing.h" #include "crypto/s2n_hash.h" +#include "crypto/s2n_openssl.h" #include "crypto/s2n_pkey.h" #include "crypto/s2n_rsa_signing.h" #include "error/s2n_errno.h" diff --git a/crypto/s2n_rsa_pss.c b/crypto/s2n_rsa_pss.c index e99da354473..528656d234c 100644 --- a/crypto/s2n_rsa_pss.c +++ b/crypto/s2n_rsa_pss.c @@ -21,7 +21,6 @@ #include "crypto/s2n_evp_signing.h" #include "crypto/s2n_hash.h" -#include "crypto/s2n_openssl.h" #include "crypto/s2n_pkey.h" #include "crypto/s2n_rsa.h" #include "crypto/s2n_rsa_signing.h" diff --git a/crypto/s2n_rsa_signing.h b/crypto/s2n_rsa_signing.h index dd56d7b576a..e9cc74c5609 100644 --- a/crypto/s2n_rsa_signing.h +++ b/crypto/s2n_rsa_signing.h @@ -16,7 +16,6 @@ #pragma once #include "api/s2n.h" -#include "crypto/s2n_openssl.h" #include "crypto/s2n_rsa.h" #include "utils/s2n_blob.h" diff --git a/tests/cbmc/include/cbmc_proof/make_common_datastructures.h b/tests/cbmc/include/cbmc_proof/make_common_datastructures.h index 1e310caa434..f60ee2b0401 100644 --- a/tests/cbmc/include/cbmc_proof/make_common_datastructures.h +++ b/tests/cbmc/include/cbmc_proof/make_common_datastructures.h @@ -23,6 +23,7 @@ #include "crypto/s2n_evp.h" #include "crypto/s2n_hash.h" #include "crypto/s2n_hmac.h" +#include "crypto/s2n_libcrypto.h" #include "stuffer/s2n_stuffer.h" #include "tls/s2n_config.h" #include "tls/s2n_connection.h" diff --git a/tests/cbmc/stubs/s2n_libcrypto_is_awslc.c b/tests/cbmc/stubs/s2n_libcrypto_is_awslc.c index 6e74ecf193f..216bea4dc45 100644 --- a/tests/cbmc/stubs/s2n_libcrypto_is_awslc.c +++ b/tests/cbmc/stubs/s2n_libcrypto_is_awslc.c @@ -16,7 +16,6 @@ #include #include -#include "crypto/s2n_openssl.h" static int flag = 0; static bool s2n_awslc_flag = 0; diff --git a/tests/unit/s2n_build_test.c b/tests/unit/s2n_build_test.c index 7c161e7b4dd..7a2355c7630 100644 --- a/tests/unit/s2n_build_test.c +++ b/tests/unit/s2n_build_test.c @@ -26,7 +26,7 @@ #include #include -#include "crypto/s2n_openssl.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #define MAX_LIBCRYPTO_NAME_LEN 100 diff --git a/tests/unit/s2n_cert_authorities_test.c b/tests/unit/s2n_cert_authorities_test.c index 1cae4570acb..79f46264c42 100644 --- a/tests/unit/s2n_cert_authorities_test.c +++ b/tests/unit/s2n_cert_authorities_test.c @@ -15,6 +15,7 @@ #include "tls/extensions/s2n_cert_authorities.h" +#include "crypto/s2n_libcrypto.h" #include "crypto/s2n_rsa_pss.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_config_test.c b/tests/unit/s2n_config_test.c index 03e97581efa..14776188967 100644 --- a/tests/unit/s2n_config_test.c +++ b/tests/unit/s2n_config_test.c @@ -20,6 +20,8 @@ #include "api/s2n.h" #include "crypto/s2n_fips.h" +#include "crypto/s2n_libcrypto.h" +#include "crypto/s2n_openssl.h" #include "crypto/s2n_pq.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_evp_signing_test.c b/tests/unit/s2n_evp_signing_test.c index 375be90afd0..0085ed8bd23 100644 --- a/tests/unit/s2n_evp_signing_test.c +++ b/tests/unit/s2n_evp_signing_test.c @@ -17,6 +17,7 @@ #include "crypto/s2n_ecdsa.h" #include "crypto/s2n_fips.h" +#include "crypto/s2n_libcrypto.h" #include "crypto/s2n_rsa_pss.h" #include "crypto/s2n_rsa_signing.h" #include "s2n_test.h" diff --git a/tests/unit/s2n_fips_test.c b/tests/unit/s2n_fips_test.c index 31274231d1b..37509855a31 100644 --- a/tests/unit/s2n_fips_test.c +++ b/tests/unit/s2n_fips_test.c @@ -16,6 +16,7 @@ #include "crypto/s2n_fips.h" #include "api/s2n.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" int main() diff --git a/tests/unit/s2n_hash_test.c b/tests/unit/s2n_hash_test.c index 929108a2c40..1ee7aa10339 100644 --- a/tests/unit/s2n_hash_test.c +++ b/tests/unit/s2n_hash_test.c @@ -18,6 +18,7 @@ #include #include "crypto/s2n_fips.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "stuffer/s2n_stuffer.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_hkdf_test.c b/tests/unit/s2n_hkdf_test.c index 0a095c5fe36..21418006c21 100644 --- a/tests/unit/s2n_hkdf_test.c +++ b/tests/unit/s2n_hkdf_test.c @@ -19,6 +19,7 @@ #include "crypto/s2n_fips.h" #include "crypto/s2n_hmac.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "stuffer/s2n_stuffer.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_openssl_test.c b/tests/unit/s2n_libcrypto_test.c similarity index 98% rename from tests/unit/s2n_openssl_test.c rename to tests/unit/s2n_libcrypto_test.c index e2f8d61183d..a8e2962bbcb 100644 --- a/tests/unit/s2n_openssl_test.c +++ b/tests/unit/s2n_libcrypto_test.c @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -#include "crypto/s2n_openssl.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "utils/s2n_random.h" diff --git a/tests/unit/s2n_locking_test.c b/tests/unit/s2n_locking_test.c index 4ae66ed6114..88f4b558015 100644 --- a/tests/unit/s2n_locking_test.c +++ b/tests/unit/s2n_locking_test.c @@ -17,6 +17,7 @@ #include +#include "crypto/s2n_openssl.h" #include "s2n_test.h" #define LOCK_N 1 diff --git a/tests/unit/s2n_pq_mlkem_test.c b/tests/unit/s2n_pq_mlkem_test.c index 1bec449a960..7a43e9a837a 100644 --- a/tests/unit/s2n_pq_mlkem_test.c +++ b/tests/unit/s2n_pq_mlkem_test.c @@ -15,7 +15,6 @@ #include "api/s2n.h" #include "crypto/s2n_libcrypto.h" -#include "crypto/s2n_openssl.h" #include "crypto/s2n_pq.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_random_test.c b/tests/unit/s2n_random_test.c index 0fbbfb823e8..80577f485f4 100644 --- a/tests/unit/s2n_random_test.c +++ b/tests/unit/s2n_random_test.c @@ -32,6 +32,7 @@ #include "api/s2n.h" #include "crypto/s2n_fips.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "utils/s2n_fork_detection.h" diff --git a/tests/unit/s2n_tls13_secrets_rfc8448_test.c b/tests/unit/s2n_tls13_secrets_rfc8448_test.c index a98a3243563..f81e3e9cfc9 100644 --- a/tests/unit/s2n_tls13_secrets_rfc8448_test.c +++ b/tests/unit/s2n_tls13_secrets_rfc8448_test.c @@ -16,6 +16,7 @@ /* Needed to set up X25519 key shares */ #include +#include "crypto/s2n_openssl.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" #include "tls/s2n_cipher_suites.h" diff --git a/tests/unit/s2n_tls_prf_test.c b/tests/unit/s2n_tls_prf_test.c index 7c1ce076b8c..faca2f3b543 100644 --- a/tests/unit/s2n_tls_prf_test.c +++ b/tests/unit/s2n_tls_prf_test.c @@ -17,7 +17,7 @@ #include #include "api/s2n.h" -#include "crypto/s2n_openssl.h" +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "stuffer/s2n_stuffer.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_x509_validator_certificate_signatures_test.c b/tests/unit/s2n_x509_validator_certificate_signatures_test.c index 06bbf4f8093..f5c3309e710 100644 --- a/tests/unit/s2n_x509_validator_certificate_signatures_test.c +++ b/tests/unit/s2n_x509_validator_certificate_signatures_test.c @@ -19,7 +19,6 @@ #include #include "api/s2n.h" -#include "crypto/s2n_openssl.h" #include "crypto/s2n_openssl_x509.h" #include "error/s2n_errno.h" #include "s2n_test.h" diff --git a/tests/unit/s2n_x509_validator_test.c b/tests/unit/s2n_x509_validator_test.c index 9439abd99f5..3da8d8a5805 100644 --- a/tests/unit/s2n_x509_validator_test.c +++ b/tests/unit/s2n_x509_validator_test.c @@ -13,6 +13,8 @@ * permissions and limitations under the License. */ +#include "crypto/s2n_libcrypto.h" +#include "crypto/s2n_openssl.h" #include "crypto/s2n_openssl_x509.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" diff --git a/tests/unit/s2n_x509_validator_time_verification_test.c b/tests/unit/s2n_x509_validator_time_verification_test.c index e5bf7317447..fd9d0ded442 100644 --- a/tests/unit/s2n_x509_validator_time_verification_test.c +++ b/tests/unit/s2n_x509_validator_time_verification_test.c @@ -13,6 +13,7 @@ * permissions and limitations under the License. */ +#include "crypto/s2n_libcrypto.h" #include "s2n_test.h" #include "testlib/s2n_testlib.h" diff --git a/tls/s2n_prf.c b/tls/s2n_prf.c index 168cd06379e..763c8058f2e 100644 --- a/tls/s2n_prf.c +++ b/tls/s2n_prf.c @@ -24,7 +24,6 @@ #include "crypto/s2n_fips.h" #include "crypto/s2n_hash.h" #include "crypto/s2n_hmac.h" -#include "crypto/s2n_openssl.h" #include "error/s2n_errno.h" #include "stuffer/s2n_stuffer.h" #include "tls/s2n_cipher_suites.h" diff --git a/tls/s2n_prf.h b/tls/s2n_prf.h index 2db2db5e2e3..4b3b99d3702 100644 --- a/tls/s2n_prf.h +++ b/tls/s2n_prf.h @@ -19,7 +19,6 @@ #include "crypto/s2n_hash.h" #include "crypto/s2n_hmac.h" -#include "crypto/s2n_openssl.h" #include "utils/s2n_blob.h" /* Enough to support TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, 2*SHA384_DIGEST_LEN + 2*AES256_KEY_SIZE */ diff --git a/tls/s2n_x509_validator.c b/tls/s2n_x509_validator.c index 4de3c03b1ff..ad9ac61a3fd 100644 --- a/tls/s2n_x509_validator.c +++ b/tls/s2n_x509_validator.c @@ -20,7 +20,6 @@ #include #include "crypto/s2n_libcrypto.h" -#include "crypto/s2n_openssl.h" #include "crypto/s2n_openssl_x509.h" #include "crypto/s2n_pkey.h" #include "tls/extensions/s2n_extension_list.h" diff --git a/utils/s2n_random.c b/utils/s2n_random.c index bb7280f74dd..bcb69ec6f81 100644 --- a/utils/s2n_random.c +++ b/utils/s2n_random.c @@ -71,6 +71,7 @@ #include "api/s2n.h" #include "crypto/s2n_drbg.h" #include "crypto/s2n_fips.h" +#include "crypto/s2n_libcrypto.h" #include "error/s2n_errno.h" #include "s2n_io.h" #include "stuffer/s2n_stuffer.h" From 4c5e35afc47f9218f0f75d1d4902a70974c9b33d Mon Sep 17 00:00:00 2001 From: Boquan Fang Date: Tue, 18 Feb 2025 10:27:48 -0800 Subject: [PATCH 12/19] bindings: unpin openssl crate from a specific patch version (#5120) Co-authored-by: Boquan Fang --- bindings/rust/extended/s2n-tls/Cargo.toml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bindings/rust/extended/s2n-tls/Cargo.toml b/bindings/rust/extended/s2n-tls/Cargo.toml index b50bea59f67..7f26b49d5b5 100644 --- a/bindings/rust/extended/s2n-tls/Cargo.toml +++ b/bindings/rust/extended/s2n-tls/Cargo.toml @@ -28,9 +28,7 @@ hex = "0.4" [dev-dependencies] futures-test = "0.3" -# The openssl crate broke MSRV with 0.10.67 -# TODO unpin this once fixed - https://github.com/sfackler/rust-openssl/issues/2317 -openssl = "<0.10.67" +openssl = "0.10" openssl-sys = "0.9" foreign-types = "0.3" temp-env = "0.3" From 551efe0273bf37a0b5d2ef6307cafd7f881d7e10 Mon Sep 17 00:00:00 2001 From: Boquan Fang Date: Tue, 18 Feb 2025 11:46:57 -0800 Subject: [PATCH 13/19] chore: fix a typo in API comments (#5123) Co-authored-by: Boquan Fang --- api/s2n.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/s2n.h b/api/s2n.h index e7e2a634716..8273b2bcefc 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -1774,7 +1774,7 @@ S2N_API extern int s2n_connection_get_write_fd(struct s2n_connection *conn, int S2N_API extern int s2n_connection_use_corked_io(struct s2n_connection *conn); /** - * Function pointer for a user provided send callback. + * Function pointer for a user provided recv callback. */ typedef int s2n_recv_fn(void *io_context, uint8_t *buf, uint32_t len); From 117d24e11da1b2176e3cb330b4944b5345c64634 Mon Sep 17 00:00:00 2001 From: Boquan Fang Date: Tue, 18 Feb 2025 15:07:01 -0800 Subject: [PATCH 14/19] build(deps): update rand requirement (#5125) Co-authored-by: Boquan Fang --- bindings/rust/extended/s2n-tls-tokio/Cargo.toml | 2 +- bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/rust/extended/s2n-tls-tokio/Cargo.toml b/bindings/rust/extended/s2n-tls-tokio/Cargo.toml index 11a5ca87f60..64559377912 100644 --- a/bindings/rust/extended/s2n-tls-tokio/Cargo.toml +++ b/bindings/rust/extended/s2n-tls-tokio/Cargo.toml @@ -21,6 +21,6 @@ tokio = { version = "1", features = ["net", "time"] } [dev-dependencies] s2n-tls = { path = "../s2n-tls", features = ["unstable-testing"] } -rand = { version = "0.8" } +rand = { version = "0.9" } tokio = { version = "1", features = [ "io-std", "io-util", "macros", "net", "rt-multi-thread", "test-util", "time"] } tokio-macros = "=2.3.0" diff --git a/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs b/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs index 6ad15116601..0a13f25602b 100644 --- a/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs +++ b/bindings/rust/extended/s2n-tls-tokio/tests/handshake.rs @@ -62,7 +62,7 @@ async fn handshake_with_pool_multithread() -> Result<(), Box Date: Tue, 18 Feb 2025 15:21:50 -0800 Subject: [PATCH 15/19] fix(bindings): make Context borrow immutable (#5071) --- .../rust/extended/s2n-tls/src/callbacks.rs | 6 +- bindings/rust/extended/s2n-tls/src/config.rs | 112 +++++++++++------- .../rust/extended/s2n-tls/src/renegotiate.rs | 30 +++-- 3 files changed, 88 insertions(+), 60 deletions(-) diff --git a/bindings/rust/extended/s2n-tls/src/callbacks.rs b/bindings/rust/extended/s2n-tls/src/callbacks.rs index c1d476ec1b8..1a710673604 100644 --- a/bindings/rust/extended/s2n-tls/src/callbacks.rs +++ b/bindings/rust/extended/s2n-tls/src/callbacks.rs @@ -48,7 +48,7 @@ pub use pkey::*; /// callbacks were configured through the Rust bindings. pub(crate) unsafe fn with_context(conn_ptr: *mut s2n_connection, action: F) -> T where - F: FnOnce(&mut Connection, &mut Context) -> T, + F: FnOnce(&mut Connection, &Context) -> T, { let raw = NonNull::new(conn_ptr).expect("connection should not be null"); // Since this is a callback, it receives a pointer to the connection @@ -57,8 +57,8 @@ where // We must make the connection `ManuallyDrop` before `action`, otherwise a panic // in `action` would cause the unwind mechanism to drop the connection. let mut conn = ManuallyDrop::new(Connection::from_raw(raw)); - let mut config = conn.config().expect("config should not be null"); - let context = config.context_mut(); + let config = conn.config().expect("config should not be null"); + let context = config.context(); action(&mut conn, context) } diff --git a/bindings/rust/extended/s2n-tls/src/config.rs b/bindings/rust/extended/s2n-tls/src/config.rs index c289b1bf0a8..75d63bbab47 100644 --- a/bindings/rust/extended/s2n-tls/src/config.rs +++ b/bindings/rust/extended/s2n-tls/src/config.rs @@ -93,14 +93,16 @@ impl Config { /// Retrieve a mutable reference to the [`Context`] stored on the config. /// /// Corresponds to [s2n_config_get_ctx]. - pub(crate) fn context_mut(&mut self) -> &mut Context { + /// + /// SAFETY: There must only ever by mutable reference to `Context` alive at + /// any time. Configs can be shared across threads, so this method is + /// likely not correct for your usecase. + pub(crate) unsafe fn context_mut(&mut self) -> &mut Context { let mut ctx = core::ptr::null_mut(); - unsafe { - s2n_config_get_ctx(self.as_mut_ptr(), &mut ctx) - .into_result() - .unwrap(); - &mut *(ctx as *mut Context) - } + s2n_config_get_ctx(self.as_mut_ptr(), &mut ctx) + .into_result() + .unwrap(); + &mut *(ctx as *mut Context) } #[cfg(test)] @@ -135,7 +137,7 @@ impl Clone for Config { impl Drop for Config { /// Corresponds to [s2n_config_free]. fn drop(&mut self) { - let context = self.context_mut(); + let context = self.context(); let count = context.refcount.fetch_sub(1, Ordering::Release); debug_assert!(count > 0, "refcount should not drop below 1 instance"); @@ -158,18 +160,20 @@ impl Drop for Config { // https://github.com/rust-lang/rust/blob/e012a191d768adeda1ee36a99ef8b92d51920154/library/alloc/src/sync.rs#L1637 std::sync::atomic::fence(Ordering::Acquire); - unsafe { - // This is the last instance so free the context. - let context = Box::from_raw(context); - drop(context); + // This is the last instance so free the context. + let context = unsafe { + // SAFETY: The reference count is verified to be 1, so this is the + // last instance of the config, and the only reference to the context. + Box::from_raw(self.context_mut()) + }; + drop(context); - let _ = s2n_config_free(self.0.as_ptr()).into_result(); - } + let _ = unsafe { s2n_config_free(self.0.as_ptr()).into_result() }; } } pub struct Builder { - config: Config, + pub(crate) config: Config, load_system_certs: bool, enable_ocsp: bool, } @@ -334,7 +338,12 @@ impl Builder { ) .into_result() }; - self.context_mut().application_owned_certs.push(chain); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because the + // Builder owns the only reference to the config. + self.config.context_mut() + }; + context.application_owned_certs.push(chain); result?; Ok(self) @@ -373,9 +382,12 @@ impl Builder { let collected_chains = chain_arrays.into_iter().take(cert_chain_count).flatten(); - self.context_mut() - .application_owned_certs - .extend(collected_chains); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because the + // Builder owns the only reference to the config. + self.config.context_mut() + }; + context.application_owned_certs.extend(collected_chains); unsafe { s2n_config_set_cert_chain_and_key_defaults( @@ -547,12 +559,17 @@ impl Builder { verify_host(host_name, host_name_len, handler) } - self.context_mut().verify_host_callback = Some(Box::new(handler)); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because the + // Builder owns the only reference to the config. + self.config.context_mut() + }; + context.verify_host_callback = Some(Box::new(handler)); unsafe { s2n_config_set_verify_host_callback( self.as_mut_ptr(), Some(verify_host_cb_fn), - self.context_mut() as *mut Context as *mut c_void, + self.config.context() as *const Context as *mut c_void, ) .into_result()?; } @@ -607,7 +624,11 @@ impl Builder { } let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.client_hello_callback = Some(handler); unsafe { @@ -628,7 +649,11 @@ impl Builder { ) -> Result<&mut Self, Error> { // Store callback in config context let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.connection_initializer = Some(handler); Ok(self) } @@ -659,14 +684,18 @@ impl Builder { // Store callback in context let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.session_ticket_callback = Some(handler); unsafe { s2n_config_set_session_ticket_cb( self.as_mut_ptr(), Some(session_ticket_cb), - self.context_mut() as *mut Context as *mut c_void, + self.config.context() as *const Context as *mut c_void, ) .into_result() }?; @@ -698,7 +727,11 @@ impl Builder { } let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.private_key_callback = Some(handler); unsafe { @@ -734,13 +767,17 @@ impl Builder { } let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.wall_clock = Some(handler); unsafe { s2n_config_set_wall_clock( self.as_mut_ptr(), Some(clock_cb), - self.context_mut() as *mut _ as *mut c_void, + self.config.context() as *const _ as *mut c_void, ) .into_result()?; } @@ -773,13 +810,17 @@ impl Builder { } let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.monotonic_clock = Some(handler); unsafe { s2n_config_set_monotonic_clock( self.as_mut_ptr(), Some(clock_cb), - self.context_mut() as *mut _ as *mut c_void, + self.config.context() as *const _ as *mut c_void, ) .into_result()?; } @@ -913,17 +954,6 @@ impl Builder { pub(crate) fn as_mut_ptr(&mut self) -> *mut s2n_config { self.config.as_mut_ptr() } - - /// Retrieve a mutable reference to the [`Context`] stored on the config. - pub(crate) fn context_mut(&mut self) -> &mut Context { - let mut ctx = core::ptr::null_mut(); - unsafe { - s2n_config_get_ctx(self.as_mut_ptr(), &mut ctx) - .into_result() - .unwrap(); - &mut *(ctx as *mut Context) - } - } } #[cfg(feature = "quic")] diff --git a/bindings/rust/extended/s2n-tls/src/renegotiate.rs b/bindings/rust/extended/s2n-tls/src/renegotiate.rs index 215ca630c42..08f5dc8fe2a 100644 --- a/bindings/rust/extended/s2n-tls/src/renegotiate.rs +++ b/bindings/rust/extended/s2n-tls/src/renegotiate.rs @@ -41,7 +41,7 @@ //! //! impl RenegotiateCallback for Callback { //! fn on_renegotiate_request( -//! &mut self, +//! &self, //! conn: &mut Connection, //! ) -> Option { //! let response = match conn.server_name() { @@ -51,7 +51,7 @@ //! Some(response) //! } //! -//! fn on_renegotiate_wipe(&mut self, conn: &mut Connection) -> Result<(), Error> { +//! fn on_renegotiate_wipe(&self, conn: &mut Connection) -> Result<(), Error> { //! conn.set_application_protocol_preference(Some("http"))?; //! Ok(()) //! } @@ -155,23 +155,20 @@ pub trait RenegotiateCallback: 'static + Send + Sync { // // This method returns Option instead of Result because the callback has no mechanism // for surfacing errors to the application, so Result would be somewhat deceptive. - fn on_renegotiate_request( - &mut self, - connection: &mut Connection, - ) -> Option; + fn on_renegotiate_request(&self, connection: &mut Connection) -> Option; /// A callback that triggers after the connection is wiped for renegotiation. /// /// Because renegotiation requires wiping the connection, connection-level /// configuration will need to be set again via this callback. /// See [`Connection::wipe_for_renegotiate()`] for more information. - fn on_renegotiate_wipe(&mut self, _connection: &mut Connection) -> Result<(), Error> { + fn on_renegotiate_wipe(&self, _connection: &mut Connection) -> Result<(), Error> { Ok(()) } } impl RenegotiateCallback for RenegotiateResponse { - fn on_renegotiate_request(&mut self, _conn: &mut Connection) -> Option { + fn on_renegotiate_request(&self, _conn: &mut Connection) -> Option { Some(*self) } } @@ -248,8 +245,8 @@ impl Connection { // We trigger the callback last so that the application can modify any // preserved configuration (like the server name or waker) if necessary. - if let Some(mut config) = self.config() { - if let Some(callback) = config.context_mut().renegotiate.as_mut() { + if let Some(config) = self.config() { + if let Some(callback) = config.context().renegotiate.as_ref() { callback.on_renegotiate_wipe(self)?; } } @@ -390,7 +387,7 @@ impl config::Builder { response: *mut s2n_renegotiate_response::Type, ) -> libc::c_int { with_context(conn_ptr, |conn, context| { - let callback = context.renegotiate.as_mut(); + let callback = context.renegotiate.as_ref(); if let Some(callback) = callback { if let Some(result) = callback.on_renegotiate_request(conn) { // If the callback indicates renegotiation, schedule it. @@ -408,7 +405,11 @@ impl config::Builder { } let handler = Box::new(handler); - let context = self.context_mut(); + let context = unsafe { + // SAFETY: usage of context_mut is safe in the builder, because while + // it is being built, the Builder is the only reference to the config. + self.config.context_mut() + }; context.renegotiate = Some(handler); unsafe { s2n_config_set_renegotiate_request_cb( @@ -717,10 +718,7 @@ mod tests { fn error_callback() -> Result<(), Box> { struct ErrorRenegotiateCallback {} impl RenegotiateCallback for ErrorRenegotiateCallback { - fn on_renegotiate_request( - &mut self, - _: &mut Connection, - ) -> Option { + fn on_renegotiate_request(&self, _: &mut Connection) -> Option { None } } From c936e91d8572177e64d0d6ad5dbd5064e4be6bc2 Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Tue, 18 Feb 2025 19:02:56 -0500 Subject: [PATCH 16/19] feat: Option to disable RAND engine override (#5108) --- CMakeLists.txt | 7 +++ .../spec/buildspec_disable_rand_override.yml | 53 +++++++++++++++++++ codebuild/spec/buildspec_generalbatch.yml | 6 +++ docs/BUILD.md | 14 +++++ tests/unit/s2n_random_test.c | 8 +++ utils/s2n_random.c | 2 + 6 files changed, 90 insertions(+) create mode 100644 codebuild/spec/buildspec_disable_rand_override.yml diff --git a/CMakeLists.txt b/CMakeLists.txt index 08c2bb476a0..6ad07d88bed 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,6 +32,8 @@ loaded in an application with an otherwise conflicting libcrypto version." OFF) option(S2N_LTO, "Enables link time optimizations when building s2n-tls." OFF) option(S2N_STACKTRACE "Enables stacktrace functionality in s2n-tls. Note that this functionality is only available on platforms that support execinfo." ON) +option(S2N_OVERRIDE_LIBCRYPTO_RAND_ENGINE "Allow s2n-tls to override the libcrypto random implementation with the custom +s2n-tls implementation, when appropriate. Disabling this flag is not recommended. See docs/BUILD.md for details." ON) option(COVERAGE "Enable profiling collection for code coverage calculation" OFF) option(BUILD_TESTING "Build tests for s2n-tls. By default only unit tests are built." ON) option(S2N_INTEG_TESTS "Enable the integrationv2 tests" OFF) @@ -247,6 +249,11 @@ if (COVERAGE) target_link_options(${PROJECT_NAME} PUBLIC -fprofile-instr-generate -fcoverage-mapping) endif() +if (NOT S2N_OVERRIDE_LIBCRYPTO_RAND_ENGINE) + message(STATUS "Disabling libcrypto RAND engine override") + add_definitions(-DS2N_DISABLE_RAND_ENGINE_OVERRIDE) +endif() + # For interning, we need to find the static libcrypto library. Cmake configs # can branch on the variable BUILD_SHARED_LIBS to e.g. avoid having to define # multiple targets. An example is AWS-LC: diff --git a/codebuild/spec/buildspec_disable_rand_override.yml b/codebuild/spec/buildspec_disable_rand_override.yml new file mode 100644 index 00000000000..65802da6414 --- /dev/null +++ b/codebuild/spec/buildspec_disable_rand_override.yml @@ -0,0 +1,53 @@ +--- +# 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 + +env: + shell: bash + variables: + # Select a libcrypto where s2n-tls will override the RAND engine by default. + S2N_LIBCRYPTO: "openssl-1.0.2" + CTEST_OUTPUT_ON_FAILURE: 1 + +phases: + build: + on-failure: ABORT + commands: + - | + cmake . -Brand_override_enabled \ + -DCMAKE_PREFIX_PATH=/usr/local/"${S2N_LIBCRYPTO}" \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo + - cmake --build ./rand_override_enabled -- -j $(nproc) + - | + cmake . -Brand_override_disabled \ + -DCMAKE_PREFIX_PATH=/usr/local/"${S2N_LIBCRYPTO}" \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo \ + -DS2N_OVERRIDE_LIBCRYPTO_RAND_ENGINE=0 + - cmake --build ./rand_override_disabled -- -j $(nproc) + post_build: + on-failure: ABORT + commands: + # CTEST_PARALLEL_LEVEL is set outside of env/variables to ensure that `nproc` is evaluated. + - export CTEST_PARALLEL_LEVEL=$(nproc) + # Run the s2n-tls tests with the assumption that the RAND engine override feature will be + # disabled. This will enable tests that ensure it's disabled. + - export S2N_DISABLE_RAND_ENGINE_OVERRIDE_EXPECTED=1 + - make -C rand_override_disabled test + # If the RAND engine override is not actually disabled, tests that expect it to be should fail. + - echo "The following is a negative test, and is expected to fail." + - | + ! make -C rand_override_enabled test -- ARGS="-R 's2n_random_test'" + # The tests should succeed without this assumption. + - unset S2N_DISABLE_RAND_ENGINE_OVERRIDE_EXPECTED + - make -C rand_override_enabled test -- ARGS="-R 's2n_random_test'" diff --git a/codebuild/spec/buildspec_generalbatch.yml b/codebuild/spec/buildspec_generalbatch.yml index 192449a331e..bb9249063e2 100644 --- a/codebuild/spec/buildspec_generalbatch.yml +++ b/codebuild/spec/buildspec_generalbatch.yml @@ -277,3 +277,9 @@ batch: privileged-mode: true compute-type: BUILD_GENERAL1_LARGE image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild + - identifier: DisableRandOverride + buildspec: codebuild/spec/buildspec_disable_rand_override.yml + env: + privileged-mode: true + compute-type: BUILD_GENERAL1_LARGE + image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu22codebuild diff --git a/docs/BUILD.md b/docs/BUILD.md index 1773fa9442c..6da13c33446 100644 --- a/docs/BUILD.md +++ b/docs/BUILD.md @@ -227,3 +227,17 @@ cmake --build ./build -j $(nproc) CTEST_PARALLEL_LEVEL=$(nproc) ctest --test-dir build ``` + +## RAND engine override + +By default, s2n-tls may override the libcrypto random implementation with its custom implementation. This allows the libcrypto APIs invoked by s2n-tls to internally use the s2n-tls random implementation when fetching random bytes. + +The motivation for this behavior is twofold: +1. Ensure that s2n-tls usage is safe when linked to a libcrypto with known issues in its random implementation, such as OpenSSL 1.0.2. See https://wiki.openssl.org/index.php/Random_fork-safety for details. +2. Ensure that s2n-tls usage is safe when used in snapshot environments. Some applications, such as [Lambda SnapStart](https://docs.aws.amazon.com/lambda/latest/dg/snapstart.html), take a snapshot of the memory and disk state, and later restore this state to a virtual machine. Precautions must be taken when running inside a restored snapshot environment to ensure that randomly generated data remains unique between restored snapshots. See the Lambda SnapStart documentation for details: https://docs.aws.amazon.com/lambda/latest/dg/snapstart-uniqueness.html + +When both of the above concerns are known to be mitigated in the linked libcrypto's random implementation, s2n-tls will not override the libcrypto's implementation, as is the case for AWS-LC. + +The s2n-tls RAND engine may conflict with some environments that use the same libcrypto as s2n-tls. For example, other applications or libraries may have certain requirements for the libcrypto RAND engine that the s2n-tls implementation doesn't provide. Other applications or libraries might also need to implement their own custom RAND engines. If the s2n-tls RAND engine conflicts with your environment, consider enabling libcrypto interning with the `S2N_INTERN_LIBCRYPTO` CMake option, which will build s2n-tls with its own copy of the libcrypto that's isolated from the rest of the environment. + +If the s2n-tls RAND engine conflicts with your environment and enabling libcrypto interning is not a viable option, s2n-tls can be forced to disable overriding the RAND engine by setting the `S2N_OVERRIDE_LIBCRYPTO_RAND_ENGINE` CMake flag to false when building s2n-tls. This is not recommended unless both of the concerns described above are confirmed to be inapplicable to your use case. diff --git a/tests/unit/s2n_random_test.c b/tests/unit/s2n_random_test.c index 80577f485f4..41c45493e93 100644 --- a/tests/unit/s2n_random_test.c +++ b/tests/unit/s2n_random_test.c @@ -918,6 +918,14 @@ int main(int argc, char **argv) EXPECT_TRUE(s2n_libcrypto_is_openssl()); EXPECT_FALSE(s2n_is_in_fips_mode()); } + + /* Ensure that disabling the S2N_OVERRIDE_LIBCRYPTO_RAND_ENGINE CMake option disables the + * custom rand override feature. When the S2N_DISABLE_RAND_ENGINE_OVERRIDE_EXPECTED + * environment variable is set, this CMake option is expected to be disabled. + */ + if (getenv("S2N_DISABLE_RAND_ENGINE_OVERRIDE_EXPECTED")) { + EXPECT_FALSE(s2n_supports_custom_rand()); + } }; /* For each test case, creates a child process that runs the test case. diff --git a/utils/s2n_random.c b/utils/s2n_random.c index bcb69ec6f81..f3a01706f4f 100644 --- a/utils/s2n_random.c +++ b/utils/s2n_random.c @@ -559,6 +559,8 @@ bool s2n_supports_custom_rand(void) /* OpenSSL 1.0.2-fips is excluded to match historical behavior */ /* OPENSSL_FIPS is only defined for 1.0.2-fips, not 3.x-fips */ return false; +#elif defined(S2N_DISABLE_RAND_ENGINE_OVERRIDE) + return false; #else return s2n_libcrypto_is_openssl() && !s2n_is_in_fips_mode(); #endif From 493248b0dc7330360d6061088d812c5cb21dda83 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Wed, 19 Feb 2025 10:55:38 -0800 Subject: [PATCH 17/19] refactor: use EVP_MD_fetch() if available (#5116) Co-authored-by: Sam Clark <3758302+goatgoose@users.noreply.github.com> --- codebuild/spec/buildspec_openssl3fips.yml | 7 +- crypto/s2n_hash.c | 98 ++++++++++++++----- crypto/s2n_hash.h | 6 +- crypto/s2n_libcrypto.c | 9 ++ crypto/s2n_libcrypto.h | 1 + .../s2n_hash_init/s2n_hash_init_harness.c | 2 + .../s2n_hash_is_available_harness.c | 2 - .../s2n_hash_reset/s2n_hash_reset_harness.c | 2 + .../S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.c | 32 ++++++ .../S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.flags | 0 .../working/s2n-cbc/stubs/s2n_hash.h | 4 +- tests/sidetrail/working/stubs/s2n_hash.h | 4 +- tests/unit/s2n_connection_test.c | 2 +- tests/unit/s2n_evp_signing_test.c | 10 +- tests/unit/s2n_handshake_hashes_test.c | 4 +- tests/unit/s2n_hash_all_algs_test.c | 5 +- tests/unit/s2n_libcrypto_test.c | 5 + tls/s2n_handshake.h | 2 +- tls/s2n_psk.c | 6 +- utils/s2n_init.c | 2 + 20 files changed, 154 insertions(+), 49 deletions(-) create mode 100644 tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.c create mode 100644 tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.flags diff --git a/codebuild/spec/buildspec_openssl3fips.yml b/codebuild/spec/buildspec_openssl3fips.yml index 4ede818eb32..4caef166990 100644 --- a/codebuild/spec/buildspec_openssl3fips.yml +++ b/codebuild/spec/buildspec_openssl3fips.yml @@ -13,6 +13,11 @@ # limitations under the License. version: 0.2 + +env: + variables: + S2N_LIBCRYPTO: "openssl-3.0-fips" + CTEST_OUTPUT_ON_FAILURE: 1 phases: build: @@ -28,7 +33,7 @@ phases: post_build: 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'" + - make -C build test -- ARGS="-R 's2n_hash_test|s2n_hash_all_algs_test|s2n_openssl_test|s2n_init_test'" diff --git a/crypto/s2n_hash.c b/crypto/s2n_hash.c index 001c3bc547d..090deb6f6c1 100644 --- a/crypto/s2n_hash.c +++ b/crypto/s2n_hash.c @@ -20,6 +20,12 @@ #include "error/s2n_errno.h" #include "utils/s2n_safety.h" +#if S2N_LIBCRYPTO_SUPPORTS_PROVIDERS +static EVP_MD *s2n_evp_mds[S2N_HASH_ALGS_COUNT] = { 0 }; +#else +static const EVP_MD *s2n_evp_mds[S2N_HASH_ALGS_COUNT] = { 0 }; +#endif + static bool s2n_use_custom_md5_sha1() { #if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) @@ -39,28 +45,65 @@ bool s2n_hash_evp_fully_supported() return s2n_use_evp_impl() && !s2n_use_custom_md5_sha1(); } -const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg) +S2N_RESULT s2n_hash_algorithms_init() { - switch (alg) { - case S2N_HASH_MD5: - return EVP_md5(); - case S2N_HASH_SHA1: - return EVP_sha1(); - case S2N_HASH_SHA224: - return EVP_sha224(); - case S2N_HASH_SHA256: - return EVP_sha256(); - case S2N_HASH_SHA384: - return EVP_sha384(); - case S2N_HASH_SHA512: - return EVP_sha512(); -#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) - case S2N_HASH_MD5_SHA1: - return EVP_md5_sha1(); +#if S2N_LIBCRYPTO_SUPPORTS_PROVIDERS + /* openssl-3.0 introduced the concept of providers. + * After openssl-3.0, the old EVP_sha256()-style methods will still work, + * but may be inefficient. See + * https://docs.openssl.org/3.4/man7/ossl-guide-libcrypto-introduction/#performance + * + * Additionally, the old style methods do not support property query strings + * to guide which provider to fetch from. This is important for FIPS, where + * the default query string of "fips=yes" will need to be overridden for + * legacy algorithms. + */ + s2n_evp_mds[S2N_HASH_MD5] = EVP_MD_fetch(NULL, "MD5", "-fips"); + s2n_evp_mds[S2N_HASH_MD5_SHA1] = EVP_MD_fetch(NULL, "MD5-SHA1", "-fips"); + s2n_evp_mds[S2N_HASH_SHA1] = EVP_MD_fetch(NULL, "SHA1", NULL); + s2n_evp_mds[S2N_HASH_SHA224] = EVP_MD_fetch(NULL, "SHA224", NULL); + s2n_evp_mds[S2N_HASH_SHA256] = EVP_MD_fetch(NULL, "SHA256", NULL); + s2n_evp_mds[S2N_HASH_SHA384] = EVP_MD_fetch(NULL, "SHA384", NULL); + s2n_evp_mds[S2N_HASH_SHA512] = EVP_MD_fetch(NULL, "SHA512", NULL); +#else + s2n_evp_mds[S2N_HASH_MD5] = EVP_md5(); + s2n_evp_mds[S2N_HASH_SHA1] = EVP_sha1(); + s2n_evp_mds[S2N_HASH_SHA224] = EVP_sha224(); + s2n_evp_mds[S2N_HASH_SHA256] = EVP_sha256(); + s2n_evp_mds[S2N_HASH_SHA384] = EVP_sha384(); + s2n_evp_mds[S2N_HASH_SHA512] = EVP_sha512(); + /* Very old libcryptos like openssl-1.0.2 do not support EVP_MD_md5_sha1(). + * We work around that by manually combining MD5 and SHA1, rather than + * using the composite algorithm. + */ + #if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_MD5_SHA1_HASH) + s2n_evp_mds[S2N_HASH_MD5_SHA1] = EVP_md5_sha1(); + #endif #endif - default: - return NULL; + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_hash_algorithms_cleanup() +{ +#if S2N_LIBCRYPTO_SUPPORTS_PROVIDERS + for (size_t i = 0; i < S2N_HASH_ALGS_COUNT; i++) { + /* https://docs.openssl.org/3.4/man3/EVP_DigestInit/ + * > Decrements the reference count for the fetched EVP_MD structure. + * > If the reference count drops to 0 then the structure is freed. + * > If the argument is NULL, nothing is done. + */ + EVP_MD_free(s2n_evp_mds[i]); + s2n_evp_mds[i] = NULL; } +#endif + return S2N_RESULT_OK; +} + +const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg) +{ + PTR_ENSURE_GTE(alg, 0); + PTR_ENSURE_LT(alg, S2N_HASH_ALGS_COUNT); + return s2n_evp_mds[alg]; } int s2n_hash_digest_size(s2n_hash_algorithm alg, uint8_t *out) @@ -119,7 +162,7 @@ bool s2n_hash_is_available(s2n_hash_algorithm alg) case S2N_HASH_SHA384: case S2N_HASH_SHA512: return true; - case S2N_HASH_SENTINEL: + case S2N_HASH_ALGS_COUNT: return false; } return false; @@ -312,13 +355,20 @@ static int s2n_evp_hash_init(struct s2n_hash_state *state, s2n_hash_algorithm al if (alg == S2N_HASH_MD5_SHA1 && s2n_use_custom_md5_sha1()) { POSIX_ENSURE_REF(state->digest.high_level.evp_md5_secondary.ctx); - POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, EVP_sha1(), NULL), S2N_ERR_HASH_INIT_FAILED); - POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp_md5_secondary.ctx, EVP_md5(), NULL), S2N_ERR_HASH_INIT_FAILED); + POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, + s2n_hash_alg_to_evp_md(S2N_HASH_SHA1), NULL), + S2N_ERR_HASH_INIT_FAILED); + POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp_md5_secondary.ctx, + s2n_hash_alg_to_evp_md(S2N_HASH_MD5), NULL), + S2N_ERR_HASH_INIT_FAILED); return S2N_SUCCESS; } - POSIX_ENSURE_REF(s2n_hash_alg_to_evp_md(alg)); - POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, s2n_hash_alg_to_evp_md(alg), NULL), S2N_ERR_HASH_INIT_FAILED); + const EVP_MD *md = s2n_hash_alg_to_evp_md(alg); + POSIX_ENSURE(md, S2N_ERR_HASH_INVALID_ALGORITHM); + POSIX_GUARD_OSSL(EVP_DigestInit_ex(state->digest.high_level.evp.ctx, md, NULL), + S2N_ERR_HASH_INIT_FAILED); + return S2N_SUCCESS; } diff --git a/crypto/s2n_hash.h b/crypto/s2n_hash.h index 03643a03e9c..6e18f47be69 100644 --- a/crypto/s2n_hash.h +++ b/crypto/s2n_hash.h @@ -33,8 +33,8 @@ typedef enum { S2N_HASH_SHA384, S2N_HASH_SHA512, S2N_HASH_MD5_SHA1, - /* Don't add any hash algorithms below S2N_HASH_SENTINEL */ - S2N_HASH_SENTINEL + /* Don't add any hash algorithms below S2N_HASH_ALGS_COUNT */ + S2N_HASH_ALGS_COUNT } s2n_hash_algorithm; /* The low_level_digest stores all OpenSSL structs that are alg-specific to be used with OpenSSL's low-level hash API's. */ @@ -85,6 +85,8 @@ struct s2n_hash { int (*free)(struct s2n_hash_state *state); }; +S2N_RESULT s2n_hash_algorithms_init(); +S2N_RESULT s2n_hash_algorithms_cleanup(); bool s2n_hash_evp_fully_supported(); const EVP_MD *s2n_hash_alg_to_evp_md(s2n_hash_algorithm alg); int s2n_hash_digest_size(s2n_hash_algorithm alg, uint8_t *out); diff --git a/crypto/s2n_libcrypto.c b/crypto/s2n_libcrypto.c index 3497af60881..1be12658159 100644 --- a/crypto/s2n_libcrypto.c +++ b/crypto/s2n_libcrypto.c @@ -226,3 +226,12 @@ bool s2n_libcrypto_supports_flag_no_check_time() return false; #endif } + +bool s2n_libcrypto_supports_providers(void) +{ +#if S2N_LIBCRYPTO_SUPPORTS_PROVIDERS + return true; +#else + return false; +#endif +} diff --git a/crypto/s2n_libcrypto.h b/crypto/s2n_libcrypto.h index 2ac736a3ad4..e4eb1f43df0 100644 --- a/crypto/s2n_libcrypto.h +++ b/crypto/s2n_libcrypto.h @@ -28,3 +28,4 @@ uint64_t s2n_libcrypto_awslc_api_version(void); S2N_RESULT s2n_libcrypto_validate_runtime(void); const char *s2n_libcrypto_get_version_name(void); bool s2n_libcrypto_supports_flag_no_check_time(void); +bool s2n_libcrypto_supports_providers(void); diff --git a/tests/cbmc/proofs/s2n_hash_init/s2n_hash_init_harness.c b/tests/cbmc/proofs/s2n_hash_init/s2n_hash_init_harness.c index ccffdc2e300..c0adf52e093 100644 --- a/tests/cbmc/proofs/s2n_hash_init/s2n_hash_init_harness.c +++ b/tests/cbmc/proofs/s2n_hash_init/s2n_hash_init_harness.c @@ -25,6 +25,8 @@ void s2n_hash_init_harness() struct s2n_hash_state *state = cbmc_allocate_s2n_hash_state(); s2n_hash_algorithm alg; + assert(s2n_result_is_ok(s2n_hash_algorithms_init())); + /* Operation under verification. */ if (s2n_hash_init(state, alg) == S2N_SUCCESS) { /* Post-conditions. */ diff --git a/tests/cbmc/proofs/s2n_hash_is_available/s2n_hash_is_available_harness.c b/tests/cbmc/proofs/s2n_hash_is_available/s2n_hash_is_available_harness.c index 515e8280dfc..f7af5aa2d00 100644 --- a/tests/cbmc/proofs/s2n_hash_is_available/s2n_hash_is_available_harness.c +++ b/tests/cbmc/proofs/s2n_hash_is_available/s2n_hash_is_available_harness.c @@ -39,8 +39,6 @@ void s2n_hash_is_available_harness() case S2N_HASH_SHA384: case S2N_HASH_SHA512: assert(is_available); break; - case S2N_HASH_SENTINEL: - assert(!is_available); break; default: __CPROVER_assert(!is_available, "Unsupported algorithm."); } diff --git a/tests/cbmc/proofs/s2n_hash_reset/s2n_hash_reset_harness.c b/tests/cbmc/proofs/s2n_hash_reset/s2n_hash_reset_harness.c index 795a9aa0f67..10a42cb8b2f 100644 --- a/tests/cbmc/proofs/s2n_hash_reset/s2n_hash_reset_harness.c +++ b/tests/cbmc/proofs/s2n_hash_reset/s2n_hash_reset_harness.c @@ -24,6 +24,8 @@ void s2n_hash_reset_harness() /* Non-deterministic inputs. */ struct s2n_hash_state *state = cbmc_allocate_s2n_hash_state(); + assert(s2n_result_is_ok(s2n_hash_algorithms_init())); + /* Operation under verification. */ if (s2n_hash_reset(state) == S2N_SUCCESS) { diff --git a/tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.c b/tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.c new file mode 100644 index 00000000000..d115dad587f --- /dev/null +++ b/tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.c @@ -0,0 +1,32 @@ +/* + * 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. + */ + +/* + * This feature probe checks if the linked libcrypto has "provider" support: + * https://docs.openssl.org/3.4/man7/provider/ + * Fetching algorithms from providers: + * https://docs.openssl.org/3.4/man7/ossl-guide-libcrypto-introduction/#algorithm-fetching + */ + +#include + +int main() +{ + /* Supports fetching hash algorithms */ + EVP_MD *md = EVP_MD_fetch(NULL, NULL, NULL); + EVP_MD_free(md); + + return 0; +} diff --git a/tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.flags b/tests/features/S2N_LIBCRYPTO_SUPPORTS_PROVIDERS.flags new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/sidetrail/working/s2n-cbc/stubs/s2n_hash.h b/tests/sidetrail/working/s2n-cbc/stubs/s2n_hash.h index 035118b93ad..894c153d091 100644 --- a/tests/sidetrail/working/s2n-cbc/stubs/s2n_hash.h +++ b/tests/sidetrail/working/s2n-cbc/stubs/s2n_hash.h @@ -32,8 +32,8 @@ typedef enum { S2N_HASH_SHA384, S2N_HASH_SHA512, S2N_HASH_MD5_SHA1, - /* Don't add any hash algorithms below S2N_HASH_SENTINEL */ - S2N_HASH_SENTINEL + /* Don't add any hash algorithms below S2N_HASH_ALGS_COUNT */ + S2N_HASH_ALGS_COUNT } s2n_hash_algorithm; struct s2n_hash_state { diff --git a/tests/sidetrail/working/stubs/s2n_hash.h b/tests/sidetrail/working/stubs/s2n_hash.h index 035118b93ad..894c153d091 100644 --- a/tests/sidetrail/working/stubs/s2n_hash.h +++ b/tests/sidetrail/working/stubs/s2n_hash.h @@ -32,8 +32,8 @@ typedef enum { S2N_HASH_SHA384, S2N_HASH_SHA512, S2N_HASH_MD5_SHA1, - /* Don't add any hash algorithms below S2N_HASH_SENTINEL */ - S2N_HASH_SENTINEL + /* Don't add any hash algorithms below S2N_HASH_ALGS_COUNT */ + S2N_HASH_ALGS_COUNT } s2n_hash_algorithm; struct s2n_hash_state { diff --git a/tests/unit/s2n_connection_test.c b/tests/unit/s2n_connection_test.c index 7a1c4e63bdb..023be06f1a6 100644 --- a/tests/unit/s2n_connection_test.c +++ b/tests/unit/s2n_connection_test.c @@ -258,7 +258,7 @@ int main(int argc, char **argv) test_scheme.hash_alg = i; conn->handshake_params.client_cert_sig_scheme = &test_scheme; conn->handshake_params.server_cert_sig_scheme = &test_scheme; - if (i <= S2N_HASH_SENTINEL) { + if (i <= S2N_HASH_ALGS_COUNT) { EXPECT_SUCCESS(s2n_connection_get_selected_client_cert_digest_algorithm(conn, &output)); EXPECT_EQUAL(expected_output[i], output); diff --git a/tests/unit/s2n_evp_signing_test.c b/tests/unit/s2n_evp_signing_test.c index 0085ed8bd23..8869d7a00f6 100644 --- a/tests/unit/s2n_evp_signing_test.c +++ b/tests/unit/s2n_evp_signing_test.c @@ -119,7 +119,7 @@ int main(int argc, char **argv) struct s2n_pkey *pkey = rsa_cert_chain->private_key; for (s2n_signature_algorithm sig_alg = 0; sig_alg <= UINT8_MAX; sig_alg++) { - for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_SENTINEL; hash_alg++) { + for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) { if (s2n_hash_alg_is_supported(sig_alg, hash_alg)) { continue; } @@ -145,7 +145,7 @@ int main(int argc, char **argv) EXPECT_PKEY_USES_EVP_SIGNING(private_key); EXPECT_PKEY_USES_EVP_SIGNING(public_key); - for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_SENTINEL; hash_alg++) { + for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) { if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) { continue; } @@ -183,7 +183,7 @@ int main(int argc, char **argv) EXPECT_PKEY_USES_EVP_SIGNING(private_key); EXPECT_PKEY_USES_EVP_SIGNING(public_key); - for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_SENTINEL; hash_alg++) { + for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) { if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) { continue; } @@ -220,7 +220,7 @@ int main(int argc, char **argv) EXPECT_PKEY_USES_EVP_SIGNING(private_key); EXPECT_PKEY_USES_EVP_SIGNING(public_key); - for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_SENTINEL; hash_alg++) { + for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) { if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) { continue; } @@ -258,7 +258,7 @@ int main(int argc, char **argv) EXPECT_PKEY_USES_EVP_SIGNING(private_key); EXPECT_PKEY_USES_EVP_SIGNING(public_key); - for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_SENTINEL; hash_alg++) { + for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) { if (!s2n_hash_alg_is_supported(sig_alg, hash_alg)) { continue; } diff --git a/tests/unit/s2n_handshake_hashes_test.c b/tests/unit/s2n_handshake_hashes_test.c index caa4cbd9a96..237ca40dd63 100644 --- a/tests/unit/s2n_handshake_hashes_test.c +++ b/tests/unit/s2n_handshake_hashes_test.c @@ -41,7 +41,7 @@ int main(int argc, char **argv) uint8_t data[100] = { 0 }; /* Allocates all hashes */ - for (s2n_hash_algorithm alg = 0; alg < S2N_HASH_SENTINEL; alg++) { + for (s2n_hash_algorithm alg = 0; alg < S2N_HASH_ALGS_COUNT; alg++) { if (alg == S2N_HASH_NONE) { continue; } @@ -74,7 +74,7 @@ int main(int argc, char **argv) uint8_t data[100] = { 0 }; - for (s2n_hash_algorithm alg = 0; alg < S2N_HASH_SENTINEL; alg++) { + for (s2n_hash_algorithm alg = 0; alg < S2N_HASH_ALGS_COUNT; alg++) { if (alg == S2N_HASH_NONE) { continue; } diff --git a/tests/unit/s2n_hash_all_algs_test.c b/tests/unit/s2n_hash_all_algs_test.c index 427e72e2dfe..1eeb72f43f4 100644 --- a/tests/unit/s2n_hash_all_algs_test.c +++ b/tests/unit/s2n_hash_all_algs_test.c @@ -28,7 +28,7 @@ const uint8_t input_data[INPUT_DATA_SIZE] = "hello hash"; * They are useful to validate that the results of the low level implementation * never change and match the results of the EVP implementation. */ -const char *expected_result_hex[S2N_HASH_SENTINEL] = { +const char *expected_result_hex[S2N_HASH_ALGS_COUNT] = { [S2N_HASH_NONE] = "", [S2N_HASH_MD5] = "f5d589043253ca6ae54124c31be43701", [S2N_HASH_SHA1] = "ccf8abd6b03ef5054a4f257e7c712e17f965272d", @@ -134,8 +134,7 @@ int main(int argc, char **argv) { BEGIN_TEST(); - /* Calculate digests when not in FIPS mode. They must match. */ - for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_SENTINEL; hash_alg++) { + for (s2n_hash_algorithm hash_alg = 0; hash_alg < S2N_HASH_ALGS_COUNT; hash_alg++) { struct s2n_blob actual_result = { 0 }; uint8_t actual_result_data[OUTPUT_DATA_SIZE] = { 0 }; EXPECT_SUCCESS(s2n_blob_init(&actual_result, actual_result_data, OUTPUT_DATA_SIZE)); diff --git a/tests/unit/s2n_libcrypto_test.c b/tests/unit/s2n_libcrypto_test.c index a8e2962bbcb..a0186af686d 100644 --- a/tests/unit/s2n_libcrypto_test.c +++ b/tests/unit/s2n_libcrypto_test.c @@ -61,5 +61,10 @@ int main(int argc, char** argv) EXPECT_FALSE(s2n_supports_custom_rand()); } + /* We expect openssl-3.0 to support providers */ + if (strstr(env_libcrypto, "openssl") && strstr(env_libcrypto, "3")) { + EXPECT_TRUE(s2n_libcrypto_supports_providers()); + } + END_TEST(); } diff --git a/tls/s2n_handshake.h b/tls/s2n_handshake.h index da2a121c576..81f426ea16d 100644 --- a/tls/s2n_handshake.h +++ b/tls/s2n_handshake.h @@ -158,7 +158,7 @@ struct s2n_handshake { /* Hash algorithms required for this handshake. The set of required hashes can be reduced as session parameters are * negotiated, i.e. cipher suite and protocol version. */ - uint8_t required_hash_algs[S2N_HASH_SENTINEL]; + uint8_t required_hash_algs[S2N_HASH_ALGS_COUNT]; /* * Data required by the Finished messages. diff --git a/tls/s2n_psk.c b/tls/s2n_psk.c index 28abf69b07f..e33138812cd 100644 --- a/tls/s2n_psk.c +++ b/tls/s2n_psk.c @@ -25,8 +25,6 @@ #include "utils/s2n_mem.h" #include "utils/s2n_safety.h" -#define S2N_HASH_ALG_COUNT S2N_HASH_SENTINEL - S2N_RESULT s2n_psk_init(struct s2n_psk *psk, s2n_psk_type type) { RESULT_ENSURE_MUT(psk); @@ -493,8 +491,8 @@ static S2N_RESULT s2n_psk_write_binder_list(struct s2n_connection *conn, const s /* Setup memory to hold the binder hashes. We potentially need one for * every hash algorithm. */ - uint8_t binder_hashes_data[S2N_HASH_ALG_COUNT][S2N_TLS13_SECRET_MAX_LEN] = { 0 }; - struct s2n_blob binder_hashes[S2N_HASH_ALG_COUNT] = { 0 }; + uint8_t binder_hashes_data[S2N_HASH_ALGS_COUNT][S2N_TLS13_SECRET_MAX_LEN] = { 0 }; + struct s2n_blob binder_hashes[S2N_HASH_ALGS_COUNT] = { 0 }; struct s2n_stuffer_reservation binder_list_size = { 0 }; RESULT_GUARD_POSIX(s2n_stuffer_reserve_uint16(out, &binder_list_size)); diff --git a/utils/s2n_init.c b/utils/s2n_init.c index 1a94fa2e684..10ab5d8a226 100644 --- a/utils/s2n_init.c +++ b/utils/s2n_init.c @@ -76,6 +76,7 @@ int s2n_init(void) POSIX_GUARD_RESULT(s2n_locking_init()); POSIX_GUARD(s2n_fips_init()); POSIX_GUARD_RESULT(s2n_rand_init()); + POSIX_GUARD_RESULT(s2n_hash_algorithms_init()); POSIX_GUARD(s2n_cipher_suites_init()); POSIX_GUARD(s2n_security_policies_init()); POSIX_GUARD(s2n_config_defaults_init()); @@ -109,6 +110,7 @@ static bool s2n_cleanup_atexit_impl(void) s2n_wipe_static_configs(); bool cleaned_up = s2n_result_is_ok(s2n_cipher_suites_cleanup()) + && s2n_result_is_ok(s2n_hash_algorithms_cleanup()) && s2n_result_is_ok(s2n_rand_cleanup_thread()) && s2n_result_is_ok(s2n_rand_cleanup()) && s2n_result_is_ok(s2n_locking_cleanup()) From 2c47d43a71a96802fb49f9f889d1292ef30a832d Mon Sep 17 00:00:00 2001 From: Boquan Fang Date: Wed, 19 Feb 2025 12:45:25 -0800 Subject: [PATCH 18/19] chore: binding release 0.3.12 (#5128) Co-authored-by: Boquan Fang --- bindings/rust/extended/s2n-tls-sys/Cargo.toml | 2 +- bindings/rust/extended/s2n-tls-sys/templates/Cargo.template | 2 +- bindings/rust/extended/s2n-tls-tokio/Cargo.toml | 4 ++-- bindings/rust/extended/s2n-tls/Cargo.toml | 4 ++-- bindings/rust/standard/s2n-tls-hyper/Cargo.toml | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bindings/rust/extended/s2n-tls-sys/Cargo.toml b/bindings/rust/extended/s2n-tls-sys/Cargo.toml index 9db1524aeb5..b4fac59ebc2 100644 --- a/bindings/rust/extended/s2n-tls-sys/Cargo.toml +++ b/bindings/rust/extended/s2n-tls-sys/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-sys" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.11" +version = "0.3.12" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" diff --git a/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template b/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template index c0f8b423ec2..e4c94f5785b 100644 --- a/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template +++ b/bindings/rust/extended/s2n-tls-sys/templates/Cargo.template @@ -1,7 +1,7 @@ [package] name = "s2n-tls-sys" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.11" +version = "0.3.12" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" diff --git a/bindings/rust/extended/s2n-tls-tokio/Cargo.toml b/bindings/rust/extended/s2n-tls-tokio/Cargo.toml index 64559377912..063197053c8 100644 --- a/bindings/rust/extended/s2n-tls-tokio/Cargo.toml +++ b/bindings/rust/extended/s2n-tls-tokio/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-tokio" description = "An implementation of TLS streams for Tokio built on top of s2n-tls" -version = "0.3.11" +version = "0.3.12" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -16,7 +16,7 @@ errno = { version = "0.3" } # A minimum libc version of 0.2.121 is required by aws-lc-sys 0.14.0. libc = { version = "0.2.121" } pin-project-lite = { version = "0.2" } -s2n-tls = { version = "=0.3.11", path = "../s2n-tls" } +s2n-tls = { version = "=0.3.12", path = "../s2n-tls" } tokio = { version = "1", features = ["net", "time"] } [dev-dependencies] diff --git a/bindings/rust/extended/s2n-tls/Cargo.toml b/bindings/rust/extended/s2n-tls/Cargo.toml index 7f26b49d5b5..d53b65d47ff 100644 --- a/bindings/rust/extended/s2n-tls/Cargo.toml +++ b/bindings/rust/extended/s2n-tls/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.11" +version = "0.3.12" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -22,7 +22,7 @@ unstable-testing = [] errno = { version = "0.3" } # A minimum libc version of 0.2.121 is required by aws-lc-sys 0.14.0. libc = "0.2.121" -s2n-tls-sys = { version = "=0.3.11", path = "../s2n-tls-sys", features = ["internal"] } +s2n-tls-sys = { version = "=0.3.12", path = "../s2n-tls-sys", features = ["internal"] } pin-project-lite = "0.2" hex = "0.4" diff --git a/bindings/rust/standard/s2n-tls-hyper/Cargo.toml b/bindings/rust/standard/s2n-tls-hyper/Cargo.toml index 5219422aa0a..09b2cb3e237 100644 --- a/bindings/rust/standard/s2n-tls-hyper/Cargo.toml +++ b/bindings/rust/standard/s2n-tls-hyper/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-hyper" description = "A compatbility crate allowing s2n-tls to be used with the hyper HTTP library" -version = "0.0.3" +version = "0.0.4" authors = ["AWS s2n"] edition = "2021" rust-version = "1.74.0" @@ -12,8 +12,8 @@ license = "Apache-2.0" default = [] [dependencies] -s2n-tls = { version = "=0.3.11", path = "../../extended/s2n-tls" } -s2n-tls-tokio = { version = "=0.3.11", path = "../../extended/s2n-tls-tokio" } +s2n-tls = { version = "=0.3.12", path = "../../extended/s2n-tls" } +s2n-tls-tokio = { version = "=0.3.12", path = "../../extended/s2n-tls-tokio" } # A minimum hyper version of 1.3 is required by hyper-util 0.1.4: # https://github.com/hyperium/hyper-util/blob/3f6a92ecd019b8d534d2945564d3ab8a92ff1f41/Cargo.toml#L34 hyper = { version = "1.3" } From 4ae43ecc538ca7a2ec0931005ab54c7a5d3168b8 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Wed, 19 Feb 2025 12:53:38 -0800 Subject: [PATCH 19/19] fix(bindings): remove mutation behind Arc (#5124) --- .../rust/extended/s2n-tls/src/cert_chain.rs | 106 ++++++++---------- .../rust/extended/s2n-tls/src/connection.rs | 15 ++- 2 files changed, 58 insertions(+), 63 deletions(-) diff --git a/bindings/rust/extended/s2n-tls/src/cert_chain.rs b/bindings/rust/extended/s2n-tls/src/cert_chain.rs index 48a3f57dfd8..4c5790cf654 100644 --- a/bindings/rust/extended/s2n-tls/src/cert_chain.rs +++ b/bindings/rust/extended/s2n-tls/src/cert_chain.rs @@ -13,34 +13,41 @@ use std::{ /// /// [CertificateChain] is internally reference counted. The reference counted `T` /// must have a drop implementation. -struct CertificateChainHandle { - cert: NonNull, +pub(crate) struct CertificateChainHandle<'a> { + pub(crate) cert: NonNull, is_owned: bool, + _lifetime: PhantomData<&'a s2n_cert_chain_and_key>, } // # Safety // // s2n_cert_chain_and_key objects can be sent across threads. -unsafe impl Send for CertificateChainHandle {} -unsafe impl Sync for CertificateChainHandle {} +unsafe impl Send for CertificateChainHandle<'_> {} +unsafe impl Sync for CertificateChainHandle<'_> {} -impl CertificateChainHandle { - fn from_owned(cert: NonNull) -> Self { - Self { - cert, +impl CertificateChainHandle<'_> { + /// Allocate an uninitialized CertificateChainHandle. + /// + /// Corresponds to [s2n_cert_chain_and_key_new]. + pub(crate) fn allocate() -> Result, crate::error::Error> { + crate::init::init(); + Ok(CertificateChainHandle { + cert: unsafe { s2n_cert_chain_and_key_new().into_result() }?, is_owned: true, - } + _lifetime: PhantomData, + }) } fn from_reference(cert: NonNull) -> Self { Self { cert, is_owned: false, + _lifetime: PhantomData, } } } -impl Drop for CertificateChainHandle { +impl Drop for CertificateChainHandle<'_> { /// Corresponds to [s2n_cert_chain_and_key_free]. fn drop(&mut self) { // ignore failures since there's not much we can do about it @@ -53,13 +60,13 @@ impl Drop for CertificateChainHandle { } pub struct Builder { - cert: CertificateChain<'static>, + cert_handle: CertificateChainHandle<'static>, } impl Builder { pub fn new() -> Result { Ok(Self { - cert: CertificateChain::allocate_owned()?, + cert_handle: CertificateChainHandle::allocate()?, }) } @@ -73,7 +80,7 @@ impl Builder { // `private_key_pem` are not modified. // https://github.com/aws/s2n-tls/issues/4140 s2n_cert_chain_and_key_load_pem_bytes( - self.cert.as_mut_ptr(), + self.cert_handle.cert.as_ptr(), chain.as_ptr() as *mut _, chain.len() as u32, key.as_ptr() as *mut _, @@ -95,7 +102,7 @@ impl Builder { // is not modified // https://github.com/aws/s2n-tls/issues/4140 s2n_cert_chain_and_key_load_public_pem_bytes( - self.cert.as_mut_ptr(), + self.cert_handle.cert.as_ptr(), chain.as_ptr() as *mut _, chain.len() as u32, ) @@ -109,7 +116,7 @@ impl Builder { pub fn set_ocsp_data(&mut self, data: &[u8]) -> Result<&mut Self, Error> { unsafe { s2n_cert_chain_and_key_set_ocsp_data( - self.cert.as_mut_ptr(), + self.cert_handle.cert.as_ptr(), data.as_ptr(), data.len() as u32, ) @@ -122,7 +129,7 @@ impl Builder { pub fn build(self) -> Result, Error> { // This method is currently infallible, but returning a result allows // us to add validation in the future. - Ok(self.cert) + Ok(CertificateChain::from_allocated(self.cert_handle)) } } @@ -135,22 +142,16 @@ impl Builder { // safe to mutate CertificateChains. #[derive(Clone)] pub struct CertificateChain<'a> { - ptr: Arc, - _lifetime: PhantomData<&'a s2n_cert_chain_and_key>, + cert_handle: Arc>, } impl CertificateChain<'_> { - /// This allocates a new certificate chain from s2n. - /// - /// Corresponds to [s2n_cert_chain_and_key_new]. - pub(crate) fn allocate_owned() -> Result, Error> { - crate::init::init(); - unsafe { - let ptr = s2n_cert_chain_and_key_new().into_result()?; - Ok(CertificateChain { - ptr: Arc::new(CertificateChainHandle::from_owned(ptr)), - _lifetime: PhantomData, - }) + /// Construct a CertificateChain from an allocated [CertificateChainHandle]. + pub(crate) fn from_allocated( + handle: CertificateChainHandle<'static>, + ) -> CertificateChain<'static> { + CertificateChain { + cert_handle: Arc::new(handle), } } @@ -162,8 +163,7 @@ impl CertificateChain<'_> { let handle = Arc::new(CertificateChainHandle::from_reference(ptr)); CertificateChain { - ptr: handle, - _lifetime: PhantomData, + cert_handle: handle, } } @@ -202,16 +202,8 @@ impl CertificateChain<'_> { self.len() == 0 } - /// SAFETY: Only one instance of `CertificateChain` may exist when this method - /// is called. s2n_cert_chain_and_key is not thread-safe, so it is not safe - /// to mutate the certificate chain if references are held across multiple threads. - pub(crate) unsafe fn as_mut_ptr(&mut self) -> *mut s2n_cert_chain_and_key { - debug_assert_eq!(Arc::strong_count(&self.ptr), 1); - self.ptr.cert.as_ptr() - } - pub(crate) fn as_ptr(&self) -> *const s2n_cert_chain_and_key { - self.ptr.cert.as_ptr() as *const _ + self.cert_handle.cert.as_ptr() as *const _ } } @@ -339,19 +331,19 @@ mod tests { #[test] fn reference_count_increment() -> Result<(), crate::error::Error> { let cert = SniTestCerts::AlligatorRsa.get().into_certificate_chain(); - assert_eq!(Arc::strong_count(&cert.ptr), 1); + assert_eq!(Arc::strong_count(&cert.cert_handle), 1); { let mut server = config::Builder::new(); server.load_chain(cert.clone())?; // after being added, the reference count should have increased - assert_eq!(Arc::strong_count(&cert.ptr), 2); + assert_eq!(Arc::strong_count(&cert.cert_handle), 2); } // after the config goes out of scope and is dropped, the ref count should // decrement - assert_eq!(Arc::strong_count(&cert.ptr), 1); + assert_eq!(Arc::strong_count(&cert.cert_handle), 1); Ok(()) } @@ -359,8 +351,8 @@ mod tests { fn cert_is_dropped() { let weak_ref = { let cert = SniTestCerts::AlligatorEcdsa.get().into_certificate_chain(); - assert_eq!(Arc::strong_count(&cert.ptr), 1); - Arc::downgrade(&cert.ptr) + assert_eq!(Arc::strong_count(&cert.cert_handle), 1); + Arc::downgrade(&cert.cert_handle) }; assert_eq!(weak_ref.strong_count(), 0); assert!(weak_ref.upgrade().is_none()); @@ -377,17 +369,17 @@ mod tests { let mut test_pair_2 = sni_test_pair(vec![cert.clone()], None, &[SniTestCerts::AlligatorRsa])?; - assert_eq!(Arc::strong_count(&cert.ptr), 3); + assert_eq!(Arc::strong_count(&cert.cert_handle), 3); assert!(test_pair_1.handshake().is_ok()); assert!(test_pair_2.handshake().is_ok()); - assert_eq!(Arc::strong_count(&cert.ptr), 3); + assert_eq!(Arc::strong_count(&cert.cert_handle), 3); drop(test_pair_1); - assert_eq!(Arc::strong_count(&cert.ptr), 2); + assert_eq!(Arc::strong_count(&cert.cert_handle), 2); drop(test_pair_2); - assert_eq!(Arc::strong_count(&cert.ptr), 1); + assert_eq!(Arc::strong_count(&cert.cert_handle), 1); Ok(()) } @@ -396,7 +388,7 @@ mod tests { // 5 certs in the maximum allowed, 6 should error. const FAILING_NUMBER: usize = 6; let certs = vec![SniTestCerts::AlligatorRsa.get().into_certificate_chain(); FAILING_NUMBER]; - assert_eq!(Arc::strong_count(&certs[0].ptr), FAILING_NUMBER); + assert_eq!(Arc::strong_count(&certs[0].cert_handle), FAILING_NUMBER); let mut config = config::Builder::new(); let err = config.set_default_chains(certs.clone()).err().unwrap(); @@ -405,7 +397,7 @@ mod tests { // The config should not hold a reference when the error was detected // in the bindings - assert_eq!(Arc::strong_count(&certs[0].ptr), FAILING_NUMBER); + assert_eq!(Arc::strong_count(&certs[0].cert_handle), FAILING_NUMBER); Ok(()) } @@ -430,8 +422,8 @@ mod tests { &test_pair.client.peer_cert_chain().unwrap() )); - assert_eq!(Arc::strong_count(&alligator_cert.ptr), 2); - assert_eq!(Arc::strong_count(&beaver_cert.ptr), 2); + assert_eq!(Arc::strong_count(&alligator_cert.cert_handle), 2); + assert_eq!(Arc::strong_count(&beaver_cert.cert_handle), 2); } // set an explicit default @@ -449,10 +441,10 @@ mod tests { &test_pair.client.peer_cert_chain().unwrap() )); - assert_eq!(Arc::strong_count(&alligator_cert.ptr), 2); + assert_eq!(Arc::strong_count(&alligator_cert.cert_handle), 2); // beaver has an additional reference because it was used in multiple // calls - assert_eq!(Arc::strong_count(&beaver_cert.ptr), 3); + assert_eq!(Arc::strong_count(&beaver_cert.cert_handle), 3); } // set a default without adding it to the store @@ -470,8 +462,8 @@ mod tests { &test_pair.client.peer_cert_chain().unwrap() )); - assert_eq!(Arc::strong_count(&alligator_cert.ptr), 2); - assert_eq!(Arc::strong_count(&beaver_cert.ptr), 2); + assert_eq!(Arc::strong_count(&alligator_cert.cert_handle), 2); + assert_eq!(Arc::strong_count(&beaver_cert.cert_handle), 2); } Ok(()) diff --git a/bindings/rust/extended/s2n-tls/src/connection.rs b/bindings/rust/extended/s2n-tls/src/connection.rs index aa03d97f747..3bd64b6e233 100644 --- a/bindings/rust/extended/s2n-tls/src/connection.rs +++ b/bindings/rust/extended/s2n-tls/src/connection.rs @@ -7,7 +7,7 @@ use crate::renegotiate::RenegotiateState; use crate::{ callbacks::*, - cert_chain::CertificateChain, + cert_chain::{CertificateChain, CertificateChainHandle}, config::Config, enums::*, error::{Error, Fallible, Pollable}, @@ -1219,11 +1219,14 @@ impl Connection { /// Corresponds to [s2n_connection_get_peer_cert_chain]. pub fn peer_cert_chain(&self) -> Result, Error> { unsafe { - let mut chain = CertificateChain::allocate_owned()?; - s2n_connection_get_peer_cert_chain(self.connection.as_ptr(), chain.as_mut_ptr()) - .into_result() - .map(|_| ())?; - Ok(chain) + let chain_handle = CertificateChainHandle::allocate()?; + s2n_connection_get_peer_cert_chain( + self.connection.as_ptr(), + chain_handle.cert.as_ptr(), + ) + .into_result() + .map(|_| ())?; + Ok(CertificateChain::from_allocated(chain_handle)) } }