From 3c0dfeebaf54f65276a68177c76cf64baa67750d Mon Sep 17 00:00:00 2001 From: Doug Chapman <54039637+dougch@users.noreply.github.com> Date: Fri, 26 Jul 2024 08:51:08 -0700 Subject: [PATCH 1/7] ci(nix): Setup a head build for the cross_compatibility integ test (#4567) --- codebuild/bin/install_s2n_head.sh | 52 ++++++++++++++++++++++--------- nix/shell.sh | 15 +++++---- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/codebuild/bin/install_s2n_head.sh b/codebuild/bin/install_s2n_head.sh index ebad50d12b2..3fbb3f609be 100755 --- a/codebuild/bin/install_s2n_head.sh +++ b/codebuild/bin/install_s2n_head.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"). @@ -12,31 +12,53 @@ # express or implied. See the License for the specific language governing # permissions and limitations under the License. -set -ex -pushd "$(pwd)" +set -eu usage() { echo "install_s2n_head.sh build_dir" exit 1 } +BUILD_DIR=$1 +SRC_ROOT=${SRC_ROOT:-$(pwd)} + if [ "$#" -ne "1" ]; then usage fi -BUILD_DIR=$1 -source codebuild/bin/jobs.sh -cd "$BUILD_DIR" - -# Clone the most recent s2n commit -git clone --depth=1 https://github.com/aws/s2n-tls s2n_head -cmake ./s2n_head -Bbuild -DCMAKE_PREFIX_PATH="$LIBCRYPTO_ROOT" -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_SHARED_LIBS=on -DBUILD_TESTING=on -cmake --build ./build -- -j $JOBS +clone(){ + git clone --branch main --single-branch . "$SRC_ROOT"/s2n_head +} -# Copy new executables to bin directory -cp -f "$BUILD_DIR"/build/bin/s2nc "$BASE_S2N_DIR"/bin/s2nc_head -cp -f "$BUILD_DIR"/build/bin/s2nd "$BASE_S2N_DIR"/bin/s2nd_head +# CMake(nix) and Make are using different directory structures. +set +u +if [[ "$IN_NIX_SHELL" ]]; then + export DEST_DIR="$SRC_ROOT"/build/bin + export EXTRA_BUILD_FLAGS="" +else + export DEST_DIR="$SRC_ROOT"/bin + export EXTRA_BUILD_FLAGS="-DCMAKE_PREFIX_PATH=$LIBCRYPTO_ROOT" +fi +set -u -popd +# Cleanup any stale s2n_head clones. +if [[ -d "$SRC_ROOT/s2n_head" ]]; then + now=$(date +%s) + last_modified=$(stat -c %Y s2n_head) + days_old=$(( (now - last_modified) / 86400)) + if ((days_old > 1 )); then + echo "s2n_head is $days_old days old, removing and cloning again." + rm -rf s2n_head + clone + else + echo "s2n_head already exists and is $days_old days old." + fi +else + clone +fi +cmake "$SRC_ROOT"/s2n_head -B"$BUILD_DIR" "$EXTRA_BUILD_FLAGS" -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_SHARED_LIBS=on -DBUILD_TESTING=on +cmake --build "$BUILD_DIR" -- -j "$(nproc)" +cp -f "$BUILD_DIR"/bin/s2nc "$DEST_DIR"/s2nc_head +cp -f "$BUILD_DIR"/bin/s2nd "$DEST_DIR"/s2nd_head exit 0 diff --git a/nix/shell.sh b/nix/shell.sh index bed64de1d4c..75072d244cd 100644 --- a/nix/shell.sh +++ b/nix/shell.sh @@ -28,7 +28,7 @@ libcrypto_alias libressl "${LIBRESSL_INSTALL_DIR}/bin/openssl" function clean { banner "Cleanup ./build" - rm -rf ./build + rm -rf ./build ./s2n_head } function configure { @@ -47,6 +47,10 @@ function build { banner "Running Build" javac tests/integrationv2/bin/SSLSocketClient.java cmake --build ./build -j $(nproc) + # Build s2n from HEAD + if [[ -z "${S2N_KTLS_TESTING_EXPECTED}" ]]; then + $SRC_ROOT/codebuild/bin/install_s2n_head.sh $(mktemp -d) + fi } function unit { @@ -60,21 +64,16 @@ function unit { function integ { if [ "$1" == "help" ]; then echo "The following tests are not supported:" - echo " - cross_compatibility" - echo " This test depends on s2nc_head and s2nd_head. To run" - echo " the test build s2n-tls from the main branch on github." - echo " Change the names of s2n[cd] to s2n[cd]_head and add those" - echo " binaries to \$PATH." echo "- renegotiate_apache" echo " This test requires apache to be running. See codebuild/bin/s2n_apache.sh" echo " for more info." return fi if [[ -z "$1" ]]; then - banner "Running all integ tests except cross_compatibility, renegotiate_apache." + banner "Running all integ tests except renegotiate_apache." (cd $SRC_ROOT/build; ctest -L integrationv2 -E "(integrationv2_cross_compatibility|integrationv2_renegotiate_apache)" --verbose) else - banner "Warning: cross_compatibility & renegotiate_apache are not supported in nix for various reasons integ help for more info." + banner "Warning: renegotiate_apache is not supported in nix for various reasons integ help for more info." for test in $@; do ctest --test-dir ./build -L integrationv2 --no-tests=error --output-on-failure -R "$test" --verbose if [ "$?" -ne 0 ]; then From fff7b157eb43cff62b39a34c8aa1c89145888167 Mon Sep 17 00:00:00 2001 From: kaukabrizvi <100529019+kaukabrizvi@users.noreply.github.com> Date: Fri, 26 Jul 2024 11:01:08 -0700 Subject: [PATCH 2/7] Set up regression benchmark for scalar performance (#4649) --- .gitignore | 2 + bindings/rust/s2n-tls/Cargo.toml | 1 + bindings/rust/s2n-tls/src/lib.rs | 4 +- tests/regression/Cargo.toml | 14 +++ tests/regression/README.md | 106 +++++++++++++++++++ tests/regression/src/lib.rs | 170 +++++++++++++++++++++++++++++++ 6 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 tests/regression/Cargo.toml create mode 100644 tests/regression/README.md create mode 100644 tests/regression/src/lib.rs diff --git a/.gitignore b/.gitignore index ad5ed8373c1..6eecfc650fe 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,5 @@ build/ result result-* *.class +# Exclude rust build directories +*target/ diff --git a/bindings/rust/s2n-tls/Cargo.toml b/bindings/rust/s2n-tls/Cargo.toml index a678b4cd4c3..aa064a2bd04 100644 --- a/bindings/rust/s2n-tls/Cargo.toml +++ b/bindings/rust/s2n-tls/Cargo.toml @@ -15,6 +15,7 @@ unstable-ktls = ["s2n-tls-sys/unstable-ktls"] quic = ["s2n-tls-sys/quic"] fips = ["s2n-tls-sys/fips"] pq = ["s2n-tls-sys/pq"] +unstable-testing = [] [dependencies] errno = { version = "0.3" } diff --git a/bindings/rust/s2n-tls/src/lib.rs b/bindings/rust/s2n-tls/src/lib.rs index 00c58200cba..27dc0f6f534 100644 --- a/bindings/rust/s2n-tls/src/lib.rs +++ b/bindings/rust/s2n-tls/src/lib.rs @@ -27,5 +27,5 @@ pub mod security; pub use s2n_tls_sys as ffi; -#[cfg(test)] -mod testing; +#[cfg(any(feature = "unstable-testing", test))] +pub mod testing; diff --git a/tests/regression/Cargo.toml b/tests/regression/Cargo.toml new file mode 100644 index 00000000000..d8f07d9ab63 --- /dev/null +++ b/tests/regression/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "regression" +version = "0.1.0" +edition = "2021" + +[dependencies] +s2n-tls = { path = "../../bindings/rust/s2n-tls", features = ["unstable-testing"] } +bytes = { version = "1", optional = true } +errno = { version = "0.3" } +libc = "0.2" +crabgrind = "0.1" +futures-test = "0.3.30" +[profile.release] +debug = true diff --git a/tests/regression/README.md b/tests/regression/README.md new file mode 100644 index 00000000000..f79c0964da9 --- /dev/null +++ b/tests/regression/README.md @@ -0,0 +1,106 @@ +# Regression Testing for s2n-tls + +This folder contains regression tests and benchmarking tools for the `s2n-tls` library. The tests focus on various aspects of TLS connections. + +## Testing Philosophy + +Currently, s2n-tls implements a wall clock benchmarking tool which measures end-to-end handshake performance to compare s2n-tls with rustls and OpenSSL. In the past, s2n-tls has tried benchmarking to detect regressions through criterion in Rust, but the subprocess and spin-up time contributed to performance measurement which made the results inaccurate and difficult to use in CI. The project has a slightly different focus, learning from these existing tools. Performance assertion in s2n-tls focuses on a benchmarking tool that can detail performance by API path and do so with enough repeatability and accuracy to detect regressions between two versions of s2n-tls so that performance analysis can occur at PR time. This means that the scope of each harness is limited and mutually exclusive of other harnesses since we are intersted in measuring the performance of the important paths a TLS connection typically follows. +## Contents + +1. **lib.rs** + - **test_set_config**: Builds a new s2n-tls config with a security policy, host callback and certs + - **test_rsa_handshake**: Performs an RSA handshake in s2n-tls. + +2. **Cargo.toml** + - The configuration file for building and running the regression tests using Cargo. + + +## Prerequisites + +Ensure you have the following installed: +- Rust (with Cargo) +- Valgrind (for cachegrind instrumentation) + +## Running the Harnesses with Valgrind (scalar performance) +To run the harnesses with Valgrind and store the annotated results, run: + +``` +ENABLE_VALGRIND = true cargo test +``` + +This will recursively call all tests with valgrind enabled so the performance output is generated and stored +## Running the tests w/o Valgrind + +``` +cargo test +``` + +This will run the tests without valgrind to test if the process completes as expected +## Sample Output for Valgrind test + +Running the test will run all harnesses and fail if any number of harnesses exceed the performance threshold. For example, a regression test faliure could look like: +``` +---- tests::test_set_security_policy_and_build stdout ---- +Running command: valgrind --tool=cachegrind --cachegrind-out-file=cachegrind_test_set_security_policy_and_build.out /home/ubuntu/proj/s2n/tests/regression/target/debug/deps/regression-7c7d86aeafe3b426 test_set_security_policy_and_build +Running command: cg_annotate cachegrind_test_set_security_policy_and_build.out > perf_outputs/test_set_security_policy_and_build.annotated.txt +thread 'tests::test_set_security_policy_and_build' panicked at src/lib.rs:174:9: +Instruction count difference in test_set_security_policy_and_build exceeds the threshold, regression of 13975865 instructions +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + +---- tests::test_rsa_handshake stdout ---- +Running command: valgrind --tool=cachegrind --cachegrind-out-file=cachegrind_test_rsa_handshake.out /home/ubuntu/proj/s2n/tests/regression/target/debug/deps/regression-7c7d86aeafe3b426 test_rsa_handshake +Running command: cg_annotate cachegrind_test_rsa_handshake.out > perf_outputs/test_rsa_handshake.annotated.txt +thread 'tests::test_rsa_handshake' panicked at src/lib.rs:174:9: +Instruction count difference in test_rsa_handshake exceeds the threshold, regression of 51176459 instructions + + +failures: + tests::test_rsa_handshake + tests::test_set_security_policy_and_build +``` + +It also produces annotated cachegrind files stored in the `perf_ouput` directory which detail the instruction counts, how many instructions a particular file/function account for, and the contribution of individual lines of code to the overall instruction count. For example, these are the first few lines of the output generated for 'test_rsa_handshake.annotated.txt': + +``` +-------------------------------------------------------------------------------- +-- Summary +-------------------------------------------------------------------------------- +Ir_________________ + +79,270,744 (100.0%) PROGRAM TOTALS + +-------------------------------------------------------------------------------- +-- File:function summary +-------------------------------------------------------------------------------- + Ir_______________________ file:function + +< 71,798,872 (90.6%, 90.6%) /home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.19.0/aws-lc/generated-src/linux-x86_64/crypto/fipsmodule/x86_64-mont5.S: + 54,908,926 (69.3%) aws_lc_0_19_0_bn_sqr8x_internal + 15,699,024 (19.8%) mul4x_internal + 1,114,840 (1.4%) __bn_post4x_internal + +< 1,551,316 (2.0%, 92.5%) /home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.19.0/aws-lc/generated-src/linux-x86_64/crypto/fipsmodule/p256-x86_64-asm.S: + 676,336 (0.9%) __ecp_nistz256_mul_montq + 475,750 (0.6%) __ecp_nistz256_sqr_montq + 95,732 (0.1%) aws_lc_0_19_0_ecp_nistz256_point_double + +< 833,553 (1.1%, 93.6%) /home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.19.0/aws-lc/generated-src/linux-x86_64/crypto/fipsmodule/sha256-x86_64.S: + 830,671 (1.0%) sha256_block_data_order_avx + +< 557,697 (0.7%, 94.3%) /home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.19.0/aws-lc/generated-src/linux-x86_64/crypto/fipsmodule/x86_64-mont.S: + 493,032 (0.6%) bn_mul4x_mont + +``` + +### Understanding the Annotated Output +The total instruction counts are listed at the top, and segmented by file:function beneath it. When comparing versions of s2n-tls (during PR workflow or otherwise) this can be useful to pinpoint the source of instruction count difference to inform you on how changes to the code impact performance. This [link](https://valgrind.org/docs/manual/cg-manual.html#cg-manual.running-cg_annotate:~:text=Information%20Source%20Code%20Documentation%20Contact%20How%20to%20Help%20Gallery,5.2.3.%C2%A0Running%20cg_annotate,-Before%20using%20cg_annotate) provides a more detailed description to fully understand the output file. + +## Test Details + +### test_set_config + +Configures and creates a new s2n-tls configuration with a specified security policy and loads a certificate key pair. Ensures the configuration is valid and can be built. + +### test_rsa_handshake + +Performs an RSA handshake in s2n-tls and validates the handshake process utilizing rsa_4096_sha512. diff --git a/tests/regression/src/lib.rs b/tests/regression/src/lib.rs new file mode 100644 index 00000000000..6a748b778ee --- /dev/null +++ b/tests/regression/src/lib.rs @@ -0,0 +1,170 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use s2n_tls::{ + config::Builder, + security, + testing::{CertKeyPair, InsecureAcceptAllCertificatesHandler}, +}; +type Error = s2n_tls::error::Error; + +/// Function to create default config with specified parameters. +pub fn set_config( + cipher_prefs: &security::Policy, + keypair: CertKeyPair, +) -> Result { + let mut builder = Builder::new(); + builder + .set_security_policy(cipher_prefs) + .expect("Unable to set config cipher preferences"); + builder + .set_verify_host_callback(InsecureAcceptAllCertificatesHandler {}) + .expect("Unable to set a host verify callback."); + builder + .load_pem(keypair.cert(), keypair.key()) + .expect("Unable to load cert/pem"); + builder.trust_pem(keypair.cert()).expect("load cert pem"); + builder.build() +} + +#[cfg(test)] +mod tests { + use super::*; + use crabgrind as cg; + use s2n_tls::testing::TestPair; + use std::{ + env, + fs::{create_dir_all, File}, + io::{self, BufRead, Write}, + path::Path, + process::Command, + }; + + /// Configurable threshold for regression testing. + /// Tests will fail if the instruction count difference is greater than the value of this constant. + const MAX_DIFF: u64 = 1_000_000; + + struct InstrumentationControl; + + impl InstrumentationControl { + fn stop_instrumentation(&self) { + cg::cachegrind::stop_instrumentation(); + } + + fn start_instrumentation(&self) { + cg::cachegrind::start_instrumentation(); + } + } + /// Environment variable to determine whether to run under valgrind or solely test functionality. + fn is_running_under_valgrind() -> bool { + env::var("ENABLE_VALGRIND").is_ok() + } + + fn valgrind_test(test_name: &str, test_body: F) -> Result<(), s2n_tls::error::Error> + where + F: FnOnce(&InstrumentationControl) -> Result<(), s2n_tls::error::Error>, + { + if !is_running_under_valgrind() { + let ctrl = InstrumentationControl; + test_body(&ctrl) + } else { + run_valgrind_test(test_name); + Ok(()) + } + } + + /// Test to create new config, set security policy, host_callback information, load/trust certs, and build config. + #[test] + fn test_set_config() { + valgrind_test("test_set_config", |ctrl| { + ctrl.stop_instrumentation(); + ctrl.start_instrumentation(); + let keypair_rsa = CertKeyPair::default(); + let _config = + set_config(&security::DEFAULT_TLS13, keypair_rsa).expect("Failed to build config"); + Ok(()) + }) + .unwrap(); + } + + /// Test which creates a TestPair from config using `rsa_4096_sha512`. Only measures a pair handshake. + #[test] + fn test_rsa_handshake() { + valgrind_test("test_rsa_handshake", |ctrl| { + ctrl.stop_instrumentation(); + // Example usage with RSA keypair (default) + let keypair_rsa = CertKeyPair::default(); + let config = set_config(&security::DEFAULT_TLS13, keypair_rsa)?; + // Create a pair (client + server) using that config, start handshake measurement + let mut pair = TestPair::from_config(&config); + // Assert a successful handshake + ctrl.start_instrumentation(); + assert!(pair.handshake().is_ok()); + ctrl.stop_instrumentation(); + Ok(()) + }) + .unwrap(); + } + /// Function to run specified test using valgrind + fn run_valgrind_test(test_name: &str) { + let exe_path = std::env::args().next().unwrap(); + create_dir_all(Path::new("target/cg_artifacts")).unwrap(); + let output_file = format!("target/cg_artifacts/cachegrind_{}.out", test_name); + let output_command = format!("--cachegrind-out-file={}", &output_file); + let mut command = Command::new("valgrind"); + command + .args(["--tool=cachegrind", &output_command, &exe_path, test_name]) + // Ensures that the recursive call is made to the actual harness code block rather than back to this function + .env_remove("ENABLE_VALGRIND"); + + println!("Running command: {:?}", command); + let status = command.status().expect("Failed to execute valgrind"); + + if !status.success() { + panic!("Valgrind failed"); + } + + let annotate_output = Command::new("cg_annotate") + .arg(&output_file) + .output() + .expect("Failed to run cg_annotate"); + + if !annotate_output.status.success() { + panic!("cg_annotate failed"); + } + create_dir_all(Path::new("target/perf_outputs")).unwrap(); + let annotate_file = format!("target/perf_outputs/{}.annotated.txt", test_name); + let mut file = File::create(&annotate_file).expect("Failed to create annotation file"); + file.write_all(&annotate_output.stdout) + .expect("Failed to write annotation file"); + + let count = find_instruction_count(&annotate_file) + .expect("Failed to get instruction count from file"); + // This is temporary code to showcase the future diff functionality, here the code regresses by 10% each time so this test will almost always fail + let new_count = count + count / 10; + let diff = new_count - count; + assert!(diff <= self::MAX_DIFF, "Instruction count difference in {} exceeds the threshold, regression of {} instructions", test_name, diff); + } + + /// Parses the annotated file for the overall instruction count total + fn find_instruction_count(file_path: &str) -> Result { + let path = Path::new(file_path); + let file = File::open(path)?; + let reader = io::BufReader::new(file); + // Example of the line being parsed: + // "79,278,369 (100.0%) PROGRAM TOTALS" + for line in reader.lines() { + let line = line?; + if line.contains("PROGRAM TOTALS") { + if let Some(instructions) = line.split_whitespace().next() { + return instructions + .replace(',', "") + .parse::() + .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)); + } + } + } + + panic!("Failed to find instruction count in annotated file"); + } +} From b483357680577fdd5f94c14cbc4e92a7563f0160 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Sat, 27 Jul 2024 14:18:20 -0700 Subject: [PATCH 3/7] refactor: clean up other hex methods (#4664) --- .../proofs/s2n_hex_string_to_bytes/Makefile | 48 -------------- .../s2n_hex_string_to_bytes/cbmc-proof.txt | 1 - .../s2n_hex_string_to_bytes_harness.c | 53 --------------- tests/testlib/s2n_hex_testlib.c | 66 +++++++++++++++++++ tests/testlib/s2n_resumption_testlib.c | 2 +- tests/testlib/s2n_stuffer_testlib.c | 35 ---------- tests/testlib/s2n_testlib.h | 12 +++- tests/unit/s2n_blob_test.c | 55 ---------------- tests/unit/s2n_client_hello_retry_test.c | 2 +- .../s2n_client_key_share_extension_pq_test.c | 2 +- tests/unit/s2n_drbg_test.c | 12 ++-- tests/unit/s2n_hash_all_algs_test.c | 5 +- tests/unit/s2n_override_openssl_random_test.c | 4 +- tests/unit/s2n_rsa_pss_rsae_test.c | 4 +- tests/unit/s2n_self_talk_key_log_test.c | 4 +- .../s2n_server_key_share_extension_test.c | 14 ++-- tests/unit/s2n_testlib_test.c | 25 +++++++ tests/unit/s2n_tls13_server_cert_test.c | 7 +- tls/s2n_key_log.c | 38 +++++------ tls/s2n_key_log.h | 1 - utils/s2n_blob.c | 56 ---------------- utils/s2n_blob.h | 13 ---- 22 files changed, 142 insertions(+), 317 deletions(-) delete mode 100644 tests/cbmc/proofs/s2n_hex_string_to_bytes/Makefile delete mode 100644 tests/cbmc/proofs/s2n_hex_string_to_bytes/cbmc-proof.txt delete mode 100644 tests/cbmc/proofs/s2n_hex_string_to_bytes/s2n_hex_string_to_bytes_harness.c create mode 100644 tests/testlib/s2n_hex_testlib.c delete mode 100644 tests/testlib/s2n_stuffer_testlib.c diff --git a/tests/cbmc/proofs/s2n_hex_string_to_bytes/Makefile b/tests/cbmc/proofs/s2n_hex_string_to_bytes/Makefile deleted file mode 100644 index 0fe32ed3e37..00000000000 --- a/tests/cbmc/proofs/s2n_hex_string_to_bytes/Makefile +++ /dev/null @@ -1,48 +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. - -# Enough to get full coverage with 10 seconds of runtime. -MAX_STRING_LEN = 10 -DEFINES += -DMAX_STRING_LEN=$(MAX_STRING_LEN) - -CBMCFLAGS += - -PROOF_UID = s2n_hex_string_to_bytes -HARNESS_ENTRY = $(PROOF_UID)_harness -HARNESS_FILE = $(HARNESS_ENTRY).c - -PROOF_SOURCES += $(PROOFDIR)/$(HARNESS_FILE) -PROOF_SOURCES += $(PROOF_SOURCE)/cbmc_utils.c -PROOF_SOURCES += $(PROOF_SOURCE)/make_common_datastructures.c - -PROJECT_SOURCES += $(SRCDIR)/utils/s2n_blob.c -PROJECT_SOURCES += $(SRCDIR)/utils/s2n_safety.c -PROJECT_SOURCES += $(SRCDIR)/error/s2n_errno.c -PROJECT_SOURCES += $(SRCDIR)/utils/s2n_result.c - -# We abstract this function because manual inspection demonstrates it is unreachable. -REMOVE_FUNCTION_BODY += s2n_calculate_stacktrace - -UNWINDSET += strlen.0:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes_harness.0:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.2:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.3:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.6:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.9:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.12:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.13:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.15:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.16:$(shell echo $$((1 + $(MAX_STRING_LEN)))) -UNWINDSET += s2n_hex_string_to_bytes.17:$(shell echo $$((1 + $(MAX_STRING_LEN)))) - -include ../Makefile.common diff --git a/tests/cbmc/proofs/s2n_hex_string_to_bytes/cbmc-proof.txt b/tests/cbmc/proofs/s2n_hex_string_to_bytes/cbmc-proof.txt deleted file mode 100644 index 6ed46f1258c..00000000000 --- a/tests/cbmc/proofs/s2n_hex_string_to_bytes/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_hex_string_to_bytes/s2n_hex_string_to_bytes_harness.c b/tests/cbmc/proofs/s2n_hex_string_to_bytes/s2n_hex_string_to_bytes_harness.c deleted file mode 100644 index 6a144e5bbbe..00000000000 --- a/tests/cbmc/proofs/s2n_hex_string_to_bytes/s2n_hex_string_to_bytes_harness.c +++ /dev/null @@ -1,53 +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 -#include - -#include "error/s2n_errno.h" -#include "utils/s2n_blob.h" - -#include - -void s2n_hex_string_to_bytes_harness() -{ - /* Non-deterministic inputs. */ - struct s2n_blob *blob = cbmc_allocate_s2n_blob(); - __CPROVER_assume(s2n_result_is_ok(s2n_blob_validate(blob))); - char *str = ensure_c_str_is_allocated(MAX_STRING_LEN); - - /* Save previous state. */ - struct s2n_blob old_blob = *blob; - struct store_byte_from_buffer old_byte_from_blob; - save_byte_from_blob(blob, &old_byte_from_blob); - - /* Operation under verification. */ - if (s2n_hex_string_to_bytes(str, blob) == S2N_SUCCESS) { - size_t strLength = 0; - for (size_t i = 0; i < strlen(str); i++) { - if (str[i] != ' ') { - strLength++; - } - } - assert(strLength % 2 == 0); - assert(blob->size == (strLength / 2)); - } else { - assert(blob->allocated == old_blob.allocated); - assert(blob->growable == old_blob.growable); - assert(blob->size == old_blob.size); - } - assert(s2n_result_is_ok(s2n_blob_validate(blob))); -} diff --git a/tests/testlib/s2n_hex_testlib.c b/tests/testlib/s2n_hex_testlib.c new file mode 100644 index 00000000000..0f33b871e25 --- /dev/null +++ b/tests/testlib/s2n_hex_testlib.c @@ -0,0 +1,66 @@ +/* + * 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 "error/s2n_errno.h" +#include "stuffer/s2n_stuffer.h" +#include "testlib/s2n_testlib.h" +#include "utils/s2n_safety.h" + +S2N_RESULT s2n_stuffer_alloc_from_hex(struct s2n_stuffer *bytes_out, const char *hex_cstr) +{ + RESULT_ENSURE_REF(bytes_out); + RESULT_ENSURE_REF(hex_cstr); + + DEFER_CLEANUP(struct s2n_blob hex = { 0 }, s2n_free); + /* Copying the hex into heap memory to handle the 'const' isn't exactly efficient, + * but for a testlib method it is sufficient. + */ + RESULT_GUARD_POSIX(s2n_alloc(&hex, strlen(hex_cstr))); + RESULT_CHECKED_MEMCPY(hex.data, hex_cstr, hex.size); + + RESULT_GUARD_POSIX(s2n_stuffer_alloc(bytes_out, strlen(hex_cstr) / 2)); + RESULT_GUARD(s2n_stuffer_read_hex(bytes_out, &hex)); + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_blob_alloc_from_hex_with_whitespace(struct s2n_blob *bytes_out, const char *hex_cstr) +{ + RESULT_ENSURE_REF(bytes_out); + RESULT_ENSURE_REF(hex_cstr); + + DEFER_CLEANUP(struct s2n_stuffer hex_in = { 0 }, s2n_stuffer_free); + RESULT_GUARD_POSIX(s2n_stuffer_alloc(&hex_in, strlen(hex_cstr))); + for (size_t i = 0; i < strlen(hex_cstr); i++) { + if (hex_cstr[i] == ' ') { + continue; + } + RESULT_GUARD_POSIX(s2n_stuffer_write_uint8(&hex_in, hex_cstr[i])); + } + uint32_t hex_in_size = s2n_stuffer_data_available(&hex_in); + hex_in.blob.size = hex_in_size; + + DEFER_CLEANUP(struct s2n_blob bytes_out_mem = { 0 }, s2n_free); + RESULT_GUARD_POSIX(s2n_alloc(&bytes_out_mem, hex_in_size / 2)); + + struct s2n_stuffer bytes_out_stuffer = { 0 }; + RESULT_GUARD_POSIX(s2n_stuffer_init(&bytes_out_stuffer, &bytes_out_mem)); + RESULT_GUARD(s2n_stuffer_read_hex(&bytes_out_stuffer, &hex_in.blob)); + + *bytes_out = bytes_out_mem; + ZERO_TO_DISABLE_DEFER_CLEANUP(bytes_out_mem); + return S2N_RESULT_OK; +} diff --git a/tests/testlib/s2n_resumption_testlib.c b/tests/testlib/s2n_resumption_testlib.c index 561f231a85c..c219baa8658 100644 --- a/tests/testlib/s2n_resumption_testlib.c +++ b/tests/testlib/s2n_resumption_testlib.c @@ -21,7 +21,7 @@ S2N_RESULT s2n_resumption_test_ticket_key_setup(struct s2n_config *config) *# PRK = 0x077709362c2e32df0ddc3f0dc47bba63 *# 90b6c73bb50f9c3122ec844ad7c2b3e5 (32 octets) **/ - S2N_RESULT_BLOB_FROM_HEX(ticket_key, + S2N_CHECKED_BLOB_FROM_HEX(ticket_key, RESULT_GUARD, "077709362c2e32df0ddc3f0dc47bba63" "90b6c73bb50f9c3122ec844ad7c2b3e5"); diff --git a/tests/testlib/s2n_stuffer_testlib.c b/tests/testlib/s2n_stuffer_testlib.c deleted file mode 100644 index 60e911fb854..00000000000 --- a/tests/testlib/s2n_stuffer_testlib.c +++ /dev/null @@ -1,35 +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 "error/s2n_errno.h" -#include "stuffer/s2n_stuffer.h" -#include "testlib/s2n_testlib.h" -#include "utils/s2n_safety.h" - -int s2n_stuffer_alloc_ro_from_hex_string(struct s2n_stuffer *stuffer, const char *str) -{ - DEFER_CLEANUP(struct s2n_blob hex = { 0 }, s2n_free); - /* Copying the hex into heap memory to handle the 'const' isn't exactly efficient, - * but for a testlib method it is sufficient. - */ - POSIX_GUARD(s2n_alloc(&hex, strlen(str))); - POSIX_CHECKED_MEMCPY(hex.data, str, hex.size); - - POSIX_GUARD(s2n_stuffer_alloc(stuffer, strlen(str) / 2)); - POSIX_GUARD_RESULT(s2n_stuffer_read_hex(stuffer, &hex)); - return S2N_SUCCESS; -} diff --git a/tests/testlib/s2n_testlib.h b/tests/testlib/s2n_testlib.h index eea219b71dc..cc9d5614035 100644 --- a/tests/testlib/s2n_testlib.h +++ b/tests/testlib/s2n_testlib.h @@ -23,8 +23,11 @@ extern const struct s2n_ecc_preferences ecc_preferences_for_retry; extern const struct s2n_security_policy security_policy_test_tls13_retry; -/* Stuffer methods for testing */ -int s2n_stuffer_alloc_ro_from_hex_string(struct s2n_stuffer *stuffer, const char *str); +/* Hex methods for testing */ +S2N_RESULT s2n_stuffer_alloc_from_hex(struct s2n_stuffer *stuffer, const char *str); +/* Unlike other hex methods, the hex string read here may include spaces. + * This is useful for hex strings with odd whitespace for readability purposes. */ +S2N_RESULT s2n_blob_alloc_from_hex_with_whitespace(struct s2n_blob *bytes_out, const char *str); void s2n_print_connection(struct s2n_connection *conn, const char *marker); @@ -287,3 +290,8 @@ int s2n_kem_recv_ciphertext_fuzz_test(const uint8_t *buf, size_t len, struct s2n int s2n_kem_recv_ciphertext_fuzz_test_init(const char *kat_file_path, struct s2n_kem_params *kem_params); S2N_RESULT s2n_resumption_test_ticket_key_setup(struct s2n_config *config); + +#define S2N_BLOB_FROM_HEX(name, hex) S2N_CHECKED_BLOB_FROM_HEX(name, POSIX_GUARD_RESULT, hex) +#define S2N_CHECKED_BLOB_FROM_HEX(name, check, hex) \ + DEFER_CLEANUP(struct s2n_blob name = { 0 }, s2n_free); \ + check(s2n_blob_alloc_from_hex_with_whitespace(&name, (const char *) hex)); diff --git a/tests/unit/s2n_blob_test.c b/tests/unit/s2n_blob_test.c index 48aed4ea74f..5db247d4b32 100644 --- a/tests/unit/s2n_blob_test.c +++ b/tests/unit/s2n_blob_test.c @@ -104,60 +104,5 @@ int main(int argc, char **argv) EXPECT_EQUAL(memcmp(g8.data, world, sizeof(world)), 0); EXPECT_EQUAL(g8.size, sizeof(world)); - /* Test s2n_hex_string_to_bytes */ - { - uint8_t test_mem[10] = { 0 }; - - /* Test with output buffer too small */ - { - const uint8_t long_input_str[] = "abcdef123456"; - struct s2n_blob output_blob = { 0 }; - - /* Succeeds with output blob of the right size */ - EXPECT_SUCCESS(s2n_blob_init(&output_blob, test_mem, sizeof(long_input_str) / 2)); - EXPECT_SUCCESS(s2n_hex_string_to_bytes(long_input_str, &output_blob)); - - /* Fails with output blob that's too small */ - EXPECT_SUCCESS(s2n_blob_init(&output_blob, test_mem, 1)); - EXPECT_FAILURE_WITH_ERRNO(s2n_hex_string_to_bytes(long_input_str, &output_blob), - S2N_ERR_INVALID_HEX); - }; - - /* Test with invalid characters */ - { - struct s2n_blob output_blob = { 0 }; - EXPECT_SUCCESS(s2n_blob_init(&output_blob, test_mem, sizeof(test_mem))); - - EXPECT_SUCCESS(s2n_hex_string_to_bytes((const uint8_t *) "12", &output_blob)); - EXPECT_FAILURE_WITH_ERRNO(s2n_hex_string_to_bytes((const uint8_t *) "#2", &output_blob), - S2N_ERR_INVALID_HEX); - EXPECT_FAILURE_WITH_ERRNO(s2n_hex_string_to_bytes((const uint8_t *) "1#", &output_blob), - S2N_ERR_INVALID_HEX); - }; - - struct { - const char *input; - size_t expected_output_size; - uint8_t expected_output[sizeof(test_mem)]; - } test_cases[] = { - { .input = "abcd", .expected_output = { 171, 205 }, .expected_output_size = 2 }, - { .input = "ab cd", .expected_output = { 171, 205 }, .expected_output_size = 2 }, - { .input = " abcd", .expected_output = { 171, 205 }, .expected_output_size = 2 }, - { .input = "abcd ", .expected_output = { 171, 205 }, .expected_output_size = 2 }, - { .input = " ab cd ", .expected_output = { 171, 205 }, .expected_output_size = 2 }, - { .input = "", .expected_output = { 0 }, .expected_output_size = 0 }, - { .input = " ", .expected_output = { 0 }, .expected_output_size = 0 }, - { .input = "12 34 56 78 90", .expected_output = { 18, 52, 86, 120, 144 }, .expected_output_size = 5 }, - { .input = "1234567890", .expected_output = { 18, 52, 86, 120, 144 }, .expected_output_size = 5 }, - }; - for (size_t i = 0; i < s2n_array_len(test_cases); i++) { - struct s2n_blob actual_output = { 0 }; - EXPECT_SUCCESS(s2n_blob_init(&actual_output, test_mem, sizeof(test_mem))); - - EXPECT_SUCCESS(s2n_hex_string_to_bytes((const uint8_t *) test_cases[i].input, &actual_output)); - EXPECT_BYTEARRAY_EQUAL(actual_output.data, test_cases[i].expected_output, test_cases[i].expected_output_size); - } - }; - END_TEST(); } diff --git a/tests/unit/s2n_client_hello_retry_test.c b/tests/unit/s2n_client_hello_retry_test.c index 643e6d12bf7..222bd59c33a 100644 --- a/tests/unit/s2n_client_hello_retry_test.c +++ b/tests/unit/s2n_client_hello_retry_test.c @@ -284,7 +284,7 @@ int main(int argc, char **argv) /* Server responds with HRR indicating x25519+Kyber as choice for negotiation; * the last 6 bytes (0033 0002 2F39) are the key share extension with x25519+Kyber */ DEFER_CLEANUP(struct s2n_stuffer hrr = { 0 }, s2n_stuffer_free); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&hrr, + EXPECT_OK(s2n_stuffer_alloc_from_hex(&hrr, "0303CF21AD74E59A6111BE1D8C021E65B891C2A211167ABB8C5E079E09E2C8A8339C00130200000C002B00020304003300022F39")); EXPECT_SUCCESS(s2n_stuffer_copy(&hrr, &conn->handshake.io, s2n_stuffer_data_available(&hrr))); diff --git a/tests/unit/s2n_client_key_share_extension_pq_test.c b/tests/unit/s2n_client_key_share_extension_pq_test.c index 0308cde026a..6d397e3bfaf 100644 --- a/tests/unit/s2n_client_key_share_extension_pq_test.c +++ b/tests/unit/s2n_client_key_share_extension_pq_test.c @@ -435,7 +435,7 @@ int main() DEFER_CLEANUP(struct s2n_stuffer key_share_extension = { 0 }, s2n_stuffer_free); /* The key shares in this extension are fake - that's OK, the server should ignore the * KEM group ID and skip the share. */ - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&key_share_extension, + EXPECT_OK(s2n_stuffer_alloc_from_hex(&key_share_extension, /* Shares size: 12 bytes */ "000C" /* IANA ID for secp256r1_sikep434r3 */ diff --git a/tests/unit/s2n_drbg_test.c b/tests/unit/s2n_drbg_test.c index a90715f933b..4595ff79b6f 100644 --- a/tests/unit/s2n_drbg_test.c +++ b/tests/unit/s2n_drbg_test.c @@ -282,9 +282,9 @@ int check_drgb_version(s2n_drbg_mode mode, int (*generator)(void *, uint32_t), i DEFER_CLEANUP(struct s2n_stuffer personalization = { 0 }, s2n_stuffer_free); DEFER_CLEANUP(struct s2n_stuffer returned_bits = { 0 }, s2n_stuffer_free); DEFER_CLEANUP(struct s2n_stuffer reference_values = { 0 }, s2n_stuffer_free); - POSIX_GUARD(s2n_stuffer_alloc_ro_from_hex_string(&personalization, personalization_hex)); - POSIX_GUARD(s2n_stuffer_alloc_ro_from_hex_string(&returned_bits, returned_bits_hex)); - POSIX_GUARD(s2n_stuffer_alloc_ro_from_hex_string(&reference_values, reference_values_hex)); + POSIX_GUARD_RESULT(s2n_stuffer_alloc_from_hex(&personalization, personalization_hex)); + POSIX_GUARD_RESULT(s2n_stuffer_alloc_from_hex(&returned_bits, returned_bits_hex)); + POSIX_GUARD_RESULT(s2n_stuffer_alloc_from_hex(&reference_values, reference_values_hex)); for (int i = 0; i < 14; i++) { uint8_t ps[S2N_DRBG_MAX_SEED_SIZE] = { 0 }; @@ -428,7 +428,7 @@ int main(int argc, char **argv) EXPECT_OK(s2n_drbg_wipe(&aes256_pr_drbg)); /* Check everything against the NIST AES 128 vectors with prediction resistance */ - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&nist_aes128_reference_entropy, nist_aes128_reference_entropy_hex)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&nist_aes128_reference_entropy, nist_aes128_reference_entropy_hex)); EXPECT_SUCCESS(check_drgb_version(S2N_AES_128_CTR_NO_DF_PR, &nist_fake_128_entropy_data, 32, nist_aes128_reference_personalization_strings_hex, nist_aes128_reference_values_hex, nist_aes128_reference_returned_bits_hex)); @@ -438,8 +438,8 @@ int main(int argc, char **argv) /* Combine nist_aes256_reference_entropy_hex_part1 and nist_aes256_reference_entropy_hex_part2 to avoid C99 * string length limit. */ - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&temp1, nist_aes256_reference_entropy_hex_part1)); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&temp2, nist_aes256_reference_entropy_hex_part2)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&temp1, nist_aes256_reference_entropy_hex_part1)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&temp2, nist_aes256_reference_entropy_hex_part2)); EXPECT_SUCCESS(s2n_stuffer_alloc(&nist_aes256_reference_entropy, temp1.write_cursor + temp2.write_cursor)); EXPECT_SUCCESS(s2n_stuffer_copy(&temp1, &nist_aes256_reference_entropy, temp1.write_cursor)); EXPECT_SUCCESS(s2n_stuffer_copy(&temp2, &nist_aes256_reference_entropy, temp2.write_cursor)); diff --git a/tests/unit/s2n_hash_all_algs_test.c b/tests/unit/s2n_hash_all_algs_test.c index af4dc4963ff..f38ae72002e 100644 --- a/tests/unit/s2n_hash_all_algs_test.c +++ b/tests/unit/s2n_hash_all_algs_test.c @@ -145,10 +145,7 @@ int main(int argc, char **argv) uint8_t actual_result_data[OUTPUT_DATA_SIZE] = { 0 }; EXPECT_SUCCESS(s2n_blob_init(&actual_result, actual_result_data, OUTPUT_DATA_SIZE)); - struct s2n_blob expected_result = { 0 }; - uint8_t expected_result_data[OUTPUT_DATA_SIZE] = { 0 }; - EXPECT_SUCCESS(s2n_blob_init(&expected_result, expected_result_data, OUTPUT_DATA_SIZE)); - EXPECT_SUCCESS(s2n_hex_string_to_bytes((const uint8_t *) expected_result_hex[hash_alg], &expected_result)); + S2N_CHECKED_BLOB_FROM_HEX(expected_result, EXPECT_OK, expected_result_hex[hash_alg]); EXPECT_OK(s2n_hash_test(hash_alg, &actual_result)); EXPECT_EQUAL(expected_result.size, actual_result.size); diff --git a/tests/unit/s2n_override_openssl_random_test.c b/tests/unit/s2n_override_openssl_random_test.c index 823cecf9315..765d806b525 100644 --- a/tests/unit/s2n_override_openssl_random_test.c +++ b/tests/unit/s2n_override_openssl_random_test.c @@ -98,7 +98,7 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_pkcs3_to_dh_params(&dh_params, &b)); /* Set s2n_random to use a new fixed DRBG to test that other known answer tests with s2n_random and OpenSSL are deterministic */ - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&test_entropy, reference_entropy_hex)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&test_entropy, reference_entropy_hex)); struct s2n_drbg drbg; EXPECT_SUCCESS(s2n_rand_set_callbacks(s2n_entropy_init_cleanup, s2n_entropy_init_cleanup, s2n_entropy_generator, s2n_entropy_generator)); @@ -119,7 +119,7 @@ int main(int argc, char **argv) EXPECT_EQUAL(bytes_used, 352); DEFER_CLEANUP(struct s2n_stuffer dhe_key_stuffer = { 0 }, s2n_stuffer_free); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&dhe_key_stuffer, expected_dhe_key_hex)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&dhe_key_stuffer, expected_dhe_key_hex)); EXPECT_EQUAL(dhe_key_stuffer.blob.size, 519); EXPECT_EQUAL(out_blob.size, 519); diff --git a/tests/unit/s2n_rsa_pss_rsae_test.c b/tests/unit/s2n_rsa_pss_rsae_test.c index 132ffc0c5d0..fe0c46d8f6d 100644 --- a/tests/unit/s2n_rsa_pss_rsae_test.c +++ b/tests/unit/s2n_rsa_pss_rsae_test.c @@ -318,8 +318,8 @@ int main(int argc, char **argv) RSA_free(rsa_key_copy); struct s2n_stuffer message_stuffer = { 0 }, signature_stuffer = { 0 }; - s2n_stuffer_alloc_ro_from_hex_string(&message_stuffer, test_case.message); - s2n_stuffer_alloc_ro_from_hex_string(&signature_stuffer, test_case.signature); + POSIX_GUARD_RESULT(s2n_stuffer_alloc_from_hex(&message_stuffer, test_case.message)); + POSIX_GUARD_RESULT(s2n_stuffer_alloc_from_hex(&signature_stuffer, test_case.signature)); hash_state_for_alg_new(verify_hash, test_case.hash_alg, message_stuffer.blob); int ret_val = rsa_public_key.verify(&rsa_public_key, S2N_SIGNATURE_RSA_PSS_RSAE, diff --git a/tests/unit/s2n_self_talk_key_log_test.c b/tests/unit/s2n_self_talk_key_log_test.c index b0269bf0175..a74d76db310 100644 --- a/tests/unit/s2n_self_talk_key_log_test.c +++ b/tests/unit/s2n_self_talk_key_log_test.c @@ -176,7 +176,9 @@ int main(int argc, char **argv) DEFER_CLEANUP(struct s2n_stuffer encoded, s2n_stuffer_free); EXPECT_SUCCESS(s2n_stuffer_alloc(&encoded, sizeof(bytes) * 2)); - EXPECT_OK(s2n_key_log_hex_encode(&encoded, bytes, sizeof(bytes))); + struct s2n_blob raw_bytes = { 0 }; + EXPECT_SUCCESS(s2n_blob_init(&raw_bytes, bytes, sizeof(bytes))); + EXPECT_OK(s2n_stuffer_write_hex(&encoded, &raw_bytes)); DEFER_CLEANUP(struct s2n_stuffer decoded, s2n_stuffer_free); EXPECT_SUCCESS(s2n_stuffer_alloc(&decoded, sizeof(bytes))); diff --git a/tests/unit/s2n_server_key_share_extension_test.c b/tests/unit/s2n_server_key_share_extension_test.c index 962ffbd77f0..7f2d46f15fe 100644 --- a/tests/unit/s2n_server_key_share_extension_test.c +++ b/tests/unit/s2n_server_key_share_extension_test.c @@ -259,7 +259,7 @@ int main(int argc, char **argv) const char *payload = key_share_payloads[i]; EXPECT_NULL(client_conn->kex_params.server_ecc_evp_params.negotiated_curve); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&extension_stuffer, payload)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&extension_stuffer, payload)); client_conn->kex_params.client_ecc_evp_params.negotiated_curve = ecc_pref->ecc_curves[i]; EXPECT_SUCCESS(s2n_ecc_evp_generate_ephemeral_key(&client_conn->kex_params.client_ecc_evp_params)); @@ -289,7 +289,7 @@ int main(int argc, char **argv) const char *payload = key_share_payloads[0]; EXPECT_NULL(client_conn->kex_params.server_ecc_evp_params.negotiated_curve); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&extension_stuffer, payload)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&extension_stuffer, payload)); client_conn->kex_params.client_ecc_evp_params.negotiated_curve = ecc_pref->ecc_curves[0]; EXPECT_SUCCESS(s2n_ecc_evp_generate_ephemeral_key(&client_conn->kex_params.client_ecc_evp_params)); @@ -318,7 +318,7 @@ int main(int argc, char **argv) const char *p256 = "001700410474cfd75c0ab7b57247761a277e1c92b5810dacb251bb758f43e9d15aaf292c4a2be43e886425ba55653ebb7a4f32fe368bacce3df00c618645cf1eb6"; EXPECT_NULL(client_conn->kex_params.server_ecc_evp_params.negotiated_curve); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&extension_stuffer, p256)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&extension_stuffer, p256)); EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_extension.recv(client_conn, &extension_stuffer), S2N_ERR_BAD_KEY_SHARE); @@ -339,7 +339,7 @@ int main(int argc, char **argv) const char *p256 = "001700410474cfd75c0ab7b57247761a277e1c92b5810dacb251bb758f43e9d15aaf292c4a2be43e886425ba55653ebb7a4f32fe368bacce3df00c618645cf1eb646f22552"; EXPECT_NULL(client_conn->kex_params.server_ecc_evp_params.negotiated_curve); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&extension_stuffer, p256)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&extension_stuffer, p256)); /* If s2n_is_evp_apis_supported is not supported, the ecc_prefs->ecc_curves contains only p-256, p-384 curves. */ int p_384_index = s2n_is_evp_apis_supported() ? 2 : 1; @@ -647,7 +647,7 @@ int main(int argc, char **argv) /* In the HRR, the server indicated p256+Kyber as it's choice in the key share extension */ const struct s2n_kem_group *kem_group = &s2n_secp256r1_kyber_512_r3; DEFER_CLEANUP(struct s2n_stuffer key_share_payload = { 0 }, s2n_stuffer_free); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&key_share_payload, "2F3A")); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&key_share_payload, "2F3A")); /* Client should successfully parse the indicated group */ EXPECT_SUCCESS(s2n_server_key_share_extension.recv(client_conn, &key_share_payload)); @@ -689,14 +689,14 @@ int main(int argc, char **argv) /* Server sends a named group identifier that isn't in the client's KEM preferences */ const char *bad_group = "2F2C"; /* IANA ID for secp256r1_threebears-babybear-r2 (not imported into s2n) */ DEFER_CLEANUP(struct s2n_stuffer bad_group_stuffer = { 0 }, s2n_stuffer_free); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&bad_group_stuffer, bad_group)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&bad_group_stuffer, bad_group)); EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_extension.recv(client_conn, &bad_group_stuffer), S2N_ERR_ECDHE_UNSUPPORTED_CURVE); /* Server sends a key share that is in the client's KEM preferences, but client didn't send a key share */ const char *wrong_share = "2F1F"; /* Full extension truncated - not necessary */ DEFER_CLEANUP(struct s2n_stuffer wrong_share_stuffer = { 0 }, s2n_stuffer_free); - EXPECT_SUCCESS(s2n_stuffer_alloc_ro_from_hex_string(&wrong_share_stuffer, wrong_share)); + EXPECT_OK(s2n_stuffer_alloc_from_hex(&wrong_share_stuffer, wrong_share)); EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_extension.recv(client_conn, &wrong_share_stuffer), S2N_ERR_ECDHE_UNSUPPORTED_CURVE); diff --git a/tests/unit/s2n_testlib_test.c b/tests/unit/s2n_testlib_test.c index 7dbd2da11e4..f5f7b8b9df0 100644 --- a/tests/unit/s2n_testlib_test.c +++ b/tests/unit/s2n_testlib_test.c @@ -43,5 +43,30 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_connection_free(server_conn)); }; + /* Test s2n_blob_alloc_from_hex_with_whitespace */ + { + struct { + const char *input; + size_t expected_output_size; + uint8_t expected_output[100]; + } test_cases[] = { + { .input = "abcd", .expected_output = { 171, 205 }, .expected_output_size = 2 }, + { .input = "ab cd", .expected_output = { 171, 205 }, .expected_output_size = 2 }, + { .input = " abcd", .expected_output = { 171, 205 }, .expected_output_size = 2 }, + { .input = "abcd ", .expected_output = { 171, 205 }, .expected_output_size = 2 }, + { .input = " ab cd ", .expected_output = { 171, 205 }, .expected_output_size = 2 }, + { .input = "", .expected_output = { 0 }, .expected_output_size = 0 }, + { .input = " ", .expected_output = { 0 }, .expected_output_size = 0 }, + { .input = "12 34 56 78 90", .expected_output = { 18, 52, 86, 120, 144 }, .expected_output_size = 5 }, + { .input = "1234567890", .expected_output = { 18, 52, 86, 120, 144 }, .expected_output_size = 5 }, + }; + for (size_t i = 0; i < s2n_array_len(test_cases); i++) { + DEFER_CLEANUP(struct s2n_blob actual_output = { 0 }, s2n_free); + EXPECT_OK(s2n_blob_alloc_from_hex_with_whitespace(&actual_output, test_cases[i].input)); + EXPECT_EQUAL(actual_output.size, test_cases[i].expected_output_size); + EXPECT_BYTEARRAY_EQUAL(actual_output.data, test_cases[i].expected_output, actual_output.size); + } + }; + END_TEST(); } diff --git a/tests/unit/s2n_tls13_server_cert_test.c b/tests/unit/s2n_tls13_server_cert_test.c index da9e6e6994d..c18ab419dfb 100644 --- a/tests/unit/s2n_tls13_server_cert_test.c +++ b/tests/unit/s2n_tls13_server_cert_test.c @@ -123,11 +123,8 @@ int main(int argc, char **argv) EXPECT_NOT_NULL(tls13_cert_chain_hex = malloc(S2N_MAX_TEST_PEM_SIZE)); strcpy(tls13_cert_chain_hex, tls13_cert_chain_header_hex); strcat(tls13_cert_chain_hex, tls13_cert_hex); - /* convert certificate chain hex to bytes*/ - struct s2n_blob tls13_cert = { 0 }; - EXPECT_SUCCESS(s2n_alloc(&tls13_cert, strlen(tls13_cert_chain_hex) / 2)); - POSIX_GUARD(s2n_hex_string_to_bytes((uint8_t *) tls13_cert_chain_hex, &tls13_cert)); + S2N_BLOB_FROM_HEX(tls13_cert, tls13_cert_chain_hex); S2N_BLOB_FROM_HEX(tls13_cert_chain, tls13_cert_hex); struct s2n_connection *conn = NULL; @@ -164,8 +161,6 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_connection_free(conn)); free(tls13_cert_chain_hex); - /* free memory allocated in s2n_alloc*/ - free(tls13_cert.data); } /* Test server sends cert and client receives cert for tls 1.3 */ diff --git a/tls/s2n_key_log.c b/tls/s2n_key_log.c index 75ef3db618f..f135ea31ba1 100644 --- a/tls/s2n_key_log.c +++ b/tls/s2n_key_log.c @@ -50,24 +50,6 @@ /* hex requires 2 chars per byte */ #define HEX_ENCODING_SIZE 2 -S2N_RESULT s2n_key_log_hex_encode(struct s2n_stuffer *output, uint8_t *bytes, size_t len) -{ - RESULT_ENSURE_MUT(output); - RESULT_ENSURE_REF(bytes); - - const uint8_t chars[] = "0123456789abcdef"; - - for (size_t i = 0; i < len; i++) { - uint8_t upper = bytes[i] >> 4; - uint8_t lower = bytes[i] & 0x0f; - - RESULT_GUARD_POSIX(s2n_stuffer_write_uint8(output, chars[upper])); - RESULT_GUARD_POSIX(s2n_stuffer_write_uint8(output, chars[lower])); - } - - return S2N_RESULT_OK; -} - S2N_RESULT s2n_key_log_tls13_secret(struct s2n_connection *conn, const struct s2n_blob *secret, s2n_secret_type_t secret_type) { RESULT_ENSURE_REF(conn); @@ -127,10 +109,14 @@ S2N_RESULT s2n_key_log_tls13_secret(struct s2n_connection *conn, const struct s2 DEFER_CLEANUP(struct s2n_stuffer output, s2n_stuffer_free); RESULT_GUARD_POSIX(s2n_stuffer_alloc(&output, len)); + struct s2n_blob client_random = { 0 }; + RESULT_GUARD_POSIX(s2n_blob_init(&client_random, conn->handshake_params.client_random, + sizeof(conn->handshake_params.client_random))); + RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&output, label, label_size)); - RESULT_GUARD(s2n_key_log_hex_encode(&output, conn->handshake_params.client_random, S2N_TLS_RANDOM_DATA_LEN)); + RESULT_GUARD(s2n_stuffer_write_hex(&output, &client_random)); RESULT_GUARD_POSIX(s2n_stuffer_write_uint8(&output, ' ')); - RESULT_GUARD(s2n_key_log_hex_encode(&output, secret->data, secret->size)); + RESULT_GUARD(s2n_stuffer_write_hex(&output, secret)); uint8_t *data = s2n_stuffer_raw_read(&output, len); RESULT_ENSURE_REF(data); @@ -162,10 +148,18 @@ S2N_RESULT s2n_key_log_tls12_secret(struct s2n_connection *conn) DEFER_CLEANUP(struct s2n_stuffer output, s2n_stuffer_free); RESULT_GUARD_POSIX(s2n_stuffer_alloc(&output, len)); + struct s2n_blob client_random = { 0 }; + RESULT_GUARD_POSIX(s2n_blob_init(&client_random, conn->handshake_params.client_random, + sizeof(conn->handshake_params.client_random))); + + struct s2n_blob master_secret = { 0 }; + RESULT_GUARD_POSIX(s2n_blob_init(&master_secret, conn->secrets.version.tls12.master_secret, + sizeof(conn->secrets.version.tls12.master_secret))); + RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&output, label, label_size)); - RESULT_GUARD(s2n_key_log_hex_encode(&output, conn->handshake_params.client_random, S2N_TLS_RANDOM_DATA_LEN)); + RESULT_GUARD(s2n_stuffer_write_hex(&output, &client_random)); RESULT_GUARD_POSIX(s2n_stuffer_write_uint8(&output, ' ')); - RESULT_GUARD(s2n_key_log_hex_encode(&output, conn->secrets.version.tls12.master_secret, S2N_TLS_SECRET_LEN)); + RESULT_GUARD(s2n_stuffer_write_hex(&output, &master_secret)); uint8_t *data = s2n_stuffer_raw_read(&output, len); RESULT_ENSURE_REF(data); diff --git a/tls/s2n_key_log.h b/tls/s2n_key_log.h index 02d39a8e704..9f07dfb5531 100644 --- a/tls/s2n_key_log.h +++ b/tls/s2n_key_log.h @@ -21,6 +21,5 @@ #include "utils/s2n_blob.h" #include "utils/s2n_safety.h" -S2N_RESULT s2n_key_log_hex_encode(struct s2n_stuffer *output, uint8_t *bytes, size_t len); S2N_RESULT s2n_key_log_tls12_secret(struct s2n_connection *conn); S2N_RESULT s2n_key_log_tls13_secret(struct s2n_connection *conn, const struct s2n_blob *secret, s2n_secret_type_t secret_type); diff --git a/utils/s2n_blob.c b/utils/s2n_blob.c index 2c3997ae746..58cd7b0a72b 100644 --- a/utils/s2n_blob.c +++ b/utils/s2n_blob.c @@ -78,59 +78,3 @@ int s2n_blob_char_to_lower(struct s2n_blob *b) POSIX_POSTCONDITION(s2n_blob_validate(b)); return S2N_SUCCESS; } - -/* An inverse map from an ascii value to a hexidecimal nibble value - * accounts for all possible char values, where 255 is invalid value */ -static const uint8_t hex_inverse[256] = { - /* clang-format off */ - 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, - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 255, 255, 255, 255, 255, 255, - 255, 10, 11, 12, 13, 14, 15, 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, 10, 11, 12, 13, 14, 15, 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, 255, 255, 255, 255, - 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255 - /* clang-format on */ -}; - -/* takes a hex string and writes values in the s2n_blob - * string needs to a valid hex and blob needs to be large enough */ -int s2n_hex_string_to_bytes(const uint8_t *str, struct s2n_blob *blob) -{ - POSIX_ENSURE_REF(str); - POSIX_PRECONDITION(s2n_blob_validate(blob)); - uint32_t len_with_spaces = strlen((const char *) str); - - size_t i = 0, j = 0; - while (j < len_with_spaces) { - if (str[j] == ' ') { - j++; - continue; - } - - uint8_t high_nibble = hex_inverse[str[j]]; - POSIX_ENSURE(high_nibble != 255, S2N_ERR_INVALID_HEX); - - uint8_t low_nibble = hex_inverse[str[j + 1]]; - POSIX_ENSURE(low_nibble != 255, S2N_ERR_INVALID_HEX); - - POSIX_ENSURE(i < blob->size, S2N_ERR_INVALID_HEX); - blob->data[i] = high_nibble << 4 | low_nibble; - - i++; - j += 2; - } - blob->size = i; - - POSIX_POSTCONDITION(s2n_blob_validate(blob)); - return S2N_SUCCESS; -} diff --git a/utils/s2n_blob.h b/utils/s2n_blob.h index 8f9481affbc..25e6b7a7646 100644 --- a/utils/s2n_blob.h +++ b/utils/s2n_blob.h @@ -43,7 +43,6 @@ S2N_RESULT s2n_blob_validate(const struct s2n_blob *b); int S2N_RESULT_MUST_USE s2n_blob_init(struct s2n_blob *b, uint8_t *data, uint32_t size); int s2n_blob_zero(struct s2n_blob *b); int S2N_RESULT_MUST_USE s2n_blob_char_to_lower(struct s2n_blob *b); -int S2N_RESULT_MUST_USE s2n_hex_string_to_bytes(const uint8_t *str, struct s2n_blob *blob); int S2N_RESULT_MUST_USE s2n_blob_slice(const struct s2n_blob *b, struct s2n_blob *slice, uint32_t offset, uint32_t size); #define s2n_stack_blob(name, requested_size, maximum) \ @@ -63,15 +62,3 @@ int S2N_RESULT_MUST_USE s2n_blob_slice(const struct s2n_blob *b, struct s2n_blob #define S2N_BLOB_LABEL(name, str) \ static uint8_t name##_data[] = str; \ const struct s2n_blob name = { .data = name##_data, .size = sizeof(name##_data) - 1 }; - -/* The S2N_BLOB_FROM_HEX macro creates a s2n_blob with the contents of a hex string. - * It is allocated on a stack so there no need to free after use. - * hex should be a const char[]. This function checks against using char*, - * because sizeof needs to refer to the buffer length rather than a pointer size */ -#define S2N_BLOB_FROM_HEX(name, hex) \ - s2n_stack_blob(name, (sizeof(hex) - 1) / 2, (sizeof(hex) - 1) / 2); \ - POSIX_GUARD(s2n_hex_string_to_bytes((const uint8_t *) hex, &name)); - -#define S2N_RESULT_BLOB_FROM_HEX(name, hex) \ - RESULT_STACK_BLOB(name, (sizeof(hex) - 1) / 2, (sizeof(hex) - 1) / 2); \ - RESULT_GUARD_POSIX(s2n_hex_string_to_bytes((const uint8_t *) hex, &name)); From f68135d1df103181b5f7b4b5d986b39626648e6a Mon Sep 17 00:00:00 2001 From: Jou Ho <43765840+jouho@users.noreply.github.com> Date: Mon, 29 Jul 2024 16:20:22 -0700 Subject: [PATCH 4/7] fix: add missing corpus files for s2n_deserialize_resumption_state_test (#4672) --- .../02bc84873b5095b759ee81641fe996cc5509aed3 | 2 ++ .../1a21e37ecaf58800139e4e842869198ce56b64ad | Bin 0 -> 12 bytes .../1c9d6750f4a924316459ac4568b629e37bf7c611 | Bin 0 -> 12 bytes .../20be0191bdefe3895b52db2cd2d67e7f98b9d5e4 | Bin 0 -> 12 bytes .../22c8a8a4473d80c05d8c30ae33d504f8542337e6 | 1 + .../24c77c33dab6e242decf9b0d98181580d7c1af7a | 1 + .../2833a5a490217ab03b7d0516f9597911a4b216ac | 1 + .../3897ffc3953e2f699d86ef3f713e197bee1ee3c9 | 2 ++ .../3cac2f0d0a9d30394cecf7fd48e8924c20a16f0d | 1 + .../42034c895d06d6f914deac94ca1d87cb39a8cd32 | 1 + .../4da2a0efa65934dbc2fbf965700243546966b5e0 | 1 + .../71853c6197a6a7f222db0f1978c7cb232b87c5ee | 2 ++ .../7369bc69b25973cd1e7a570257adcd9cc12ff959 | Bin 0 -> 4 bytes .../7ce183d59193fa5feb13c2ae159931bb91b92ee0 | Bin 0 -> 4 bytes .../85e53271e14006f0265921d02d4d736cdc580b0b | 1 + .../89096f0b81069aba25c5349783bb4861c03c20c2 | 2 ++ .../8ce8a112516d5fac4f8cdf82c52ee0c7e760347e | 2 ++ .../96539ce423b89f70ffdad05fdf57192556117438 | Bin 0 -> 4 bytes .../990aa904709d8c174de24c48f43a2164a8c60fdc | Bin 0 -> 12 bytes .../9a3461893a6ad8efb7167a5499e9f8c15b542b5f | Bin 0 -> 4 bytes .../9b6539615bb49873505a3139b22b357b3ee2bce9 | 1 + .../9d8b7499014b282c6a5ad2bfbd8c196c57d712f7 | 1 + .../adc83b19e793491b1c6ea0fd8b46cd9f32e592fc | 1 + .../ae67a4f5c81fed57fe9545c429d9760753c753c5 | 2 ++ .../b220a3cbb136bbd2bbdb46fd3869c4d817394dd4 | Bin 0 -> 4 bytes .../c4ea21bb365bbeeaf5f2c654883e56d11e43c44e | 1 + .../cb2698b08ed53e85b181ef4099d4c3b2f76491e6 | 1 + .../e2caca4f6b83f4e6358878d56f1c625efb7abc8f | 2 ++ 28 files changed, 26 insertions(+) create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/02bc84873b5095b759ee81641fe996cc5509aed3 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/1a21e37ecaf58800139e4e842869198ce56b64ad create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/1c9d6750f4a924316459ac4568b629e37bf7c611 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/20be0191bdefe3895b52db2cd2d67e7f98b9d5e4 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/22c8a8a4473d80c05d8c30ae33d504f8542337e6 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/24c77c33dab6e242decf9b0d98181580d7c1af7a create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/2833a5a490217ab03b7d0516f9597911a4b216ac create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3897ffc3953e2f699d86ef3f713e197bee1ee3c9 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3cac2f0d0a9d30394cecf7fd48e8924c20a16f0d create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/42034c895d06d6f914deac94ca1d87cb39a8cd32 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/4da2a0efa65934dbc2fbf965700243546966b5e0 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/71853c6197a6a7f222db0f1978c7cb232b87c5ee create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/7369bc69b25973cd1e7a570257adcd9cc12ff959 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/7ce183d59193fa5feb13c2ae159931bb91b92ee0 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/85e53271e14006f0265921d02d4d736cdc580b0b create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/89096f0b81069aba25c5349783bb4861c03c20c2 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/8ce8a112516d5fac4f8cdf82c52ee0c7e760347e create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/96539ce423b89f70ffdad05fdf57192556117438 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/990aa904709d8c174de24c48f43a2164a8c60fdc create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9a3461893a6ad8efb7167a5499e9f8c15b542b5f create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9b6539615bb49873505a3139b22b357b3ee2bce9 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9d8b7499014b282c6a5ad2bfbd8c196c57d712f7 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/adc83b19e793491b1c6ea0fd8b46cd9f32e592fc create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/ae67a4f5c81fed57fe9545c429d9760753c753c5 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/b220a3cbb136bbd2bbdb46fd3869c4d817394dd4 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/c4ea21bb365bbeeaf5f2c654883e56d11e43c44e create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/cb2698b08ed53e85b181ef4099d4c3b2f76491e6 create mode 100644 tests/fuzz/corpus/s2n_deserialize_resumption_state_test/e2caca4f6b83f4e6358878d56f1c625efb7abc8f diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/02bc84873b5095b759ee81641fe996cc5509aed3 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/02bc84873b5095b759ee81641fe996cc5509aed3 new file mode 100644 index 00000000000..e1b424f239d --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/02bc84873b5095b759ee81641fe996cc5509aed3 @@ -0,0 +1,2 @@ + + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/1a21e37ecaf58800139e4e842869198ce56b64ad b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/1a21e37ecaf58800139e4e842869198ce56b64ad new file mode 100644 index 0000000000000000000000000000000000000000..745b4b683dfd5ae0a9223857434169c18c0bd05e GIT binary patch literal 12 Tcmd;D!NA3(q;-JvKLY~*5itVW literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/1c9d6750f4a924316459ac4568b629e37bf7c611 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/1c9d6750f4a924316459ac4568b629e37bf7c611 new file mode 100644 index 0000000000000000000000000000000000000000..b39d53537a2dcf7cd75fa85837f53ded6251d7c6 GIT binary patch literal 12 TcmcEe&%mY0Foo;CaJ&Ej7zhK5 literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/20be0191bdefe3895b52db2cd2d67e7f98b9d5e4 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/20be0191bdefe3895b52db2cd2d67e7f98b9d5e4 new file mode 100644 index 0000000000000000000000000000000000000000..c908eb959b0c2cd6cd3fcb27d085423e5d637588 GIT binary patch literal 12 TcmZ3tj)99&xP^g1xSjz36yySt literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/22c8a8a4473d80c05d8c30ae33d504f8542337e6 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/22c8a8a4473d80c05d8c30ae33d504f8542337e6 new file mode 100644 index 00000000000..fe34e8292a5 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/22c8a8a4473d80c05d8c30ae33d504f8542337e6 @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/24c77c33dab6e242decf9b0d98181580d7c1af7a b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/24c77c33dab6e242decf9b0d98181580d7c1af7a new file mode 100644 index 00000000000..fcf511f5311 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/24c77c33dab6e242decf9b0d98181580d7c1af7a @@ -0,0 +1 @@ +' \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/2833a5a490217ab03b7d0516f9597911a4b216ac b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/2833a5a490217ab03b7d0516f9597911a4b216ac new file mode 100644 index 00000000000..d6122807c8f --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/2833a5a490217ab03b7d0516f9597911a4b216ac @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3897ffc3953e2f699d86ef3f713e197bee1ee3c9 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3897ffc3953e2f699d86ef3f713e197bee1ee3c9 new file mode 100644 index 00000000000..cdda0c416ab --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3897ffc3953e2f699d86ef3f713e197bee1ee3c9 @@ -0,0 +1,2 @@ + +; \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3cac2f0d0a9d30394cecf7fd48e8924c20a16f0d b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3cac2f0d0a9d30394cecf7fd48e8924c20a16f0d new file mode 100644 index 00000000000..39115c34d5b --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/3cac2f0d0a9d30394cecf7fd48e8924c20a16f0d @@ -0,0 +1 @@ +:' \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/42034c895d06d6f914deac94ca1d87cb39a8cd32 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/42034c895d06d6f914deac94ca1d87cb39a8cd32 new file mode 100644 index 00000000000..1bd8bf60573 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/42034c895d06d6f914deac94ca1d87cb39a8cd32 @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/4da2a0efa65934dbc2fbf965700243546966b5e0 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/4da2a0efa65934dbc2fbf965700243546966b5e0 new file mode 100644 index 00000000000..b8f9651e2ed --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/4da2a0efa65934dbc2fbf965700243546966b5e0 @@ -0,0 +1 @@ +.. \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/71853c6197a6a7f222db0f1978c7cb232b87c5ee b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/71853c6197a6a7f222db0f1978c7cb232b87c5ee new file mode 100644 index 00000000000..139597f9cb0 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/71853c6197a6a7f222db0f1978c7cb232b87c5ee @@ -0,0 +1,2 @@ + + diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/7369bc69b25973cd1e7a570257adcd9cc12ff959 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/7369bc69b25973cd1e7a570257adcd9cc12ff959 new file mode 100644 index 0000000000000000000000000000000000000000..b7826fb948b32f649e6a29c3354e57d7b19b1ce8 GIT binary patch literal 4 Lcmca)%`guD1e*b@ literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/7ce183d59193fa5feb13c2ae159931bb91b92ee0 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/7ce183d59193fa5feb13c2ae159931bb91b92ee0 new file mode 100644 index 0000000000000000000000000000000000000000..303bed3dc76608e13b0c62b06ee7ba007a9c35fa GIT binary patch literal 4 LcmcC2U@!&%0&f7` literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/85e53271e14006f0265921d02d4d736cdc580b0b b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/85e53271e14006f0265921d02d4d736cdc580b0b new file mode 100644 index 00000000000..ce542efaa51 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/85e53271e14006f0265921d02d4d736cdc580b0b @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/89096f0b81069aba25c5349783bb4861c03c20c2 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/89096f0b81069aba25c5349783bb4861c03c20c2 new file mode 100644 index 00000000000..2aa437d0d0b --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/89096f0b81069aba25c5349783bb4861c03c20c2 @@ -0,0 +1,2 @@ + + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/8ce8a112516d5fac4f8cdf82c52ee0c7e760347e b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/8ce8a112516d5fac4f8cdf82c52ee0c7e760347e new file mode 100644 index 00000000000..1240ce70614 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/8ce8a112516d5fac4f8cdf82c52ee0c7e760347e @@ -0,0 +1,2 @@ + +nn~ \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/96539ce423b89f70ffdad05fdf57192556117438 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/96539ce423b89f70ffdad05fdf57192556117438 new file mode 100644 index 0000000000000000000000000000000000000000..cfd4068ff0a18b45798a02b7cbacf99efab21687 GIT binary patch literal 4 Lcmd;TVwej60#5-d literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/990aa904709d8c174de24c48f43a2164a8c60fdc b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/990aa904709d8c174de24c48f43a2164a8c60fdc new file mode 100644 index 0000000000000000000000000000000000000000..19622275bbed5b0b594a993aa75db649126d49e6 GIT binary patch literal 12 QcmebBI>34AKLZ#502|B$CjbBd literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9a3461893a6ad8efb7167a5499e9f8c15b542b5f b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9a3461893a6ad8efb7167a5499e9f8c15b542b5f new file mode 100644 index 0000000000000000000000000000000000000000..fcb2d502add2149e59128efa582c69b8c490a09c GIT binary patch literal 4 LcmcDrW|$8E0z3f~ literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9b6539615bb49873505a3139b22b357b3ee2bce9 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9b6539615bb49873505a3139b22b357b3ee2bce9 new file mode 100644 index 00000000000..8ba3180e838 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9b6539615bb49873505a3139b22b357b3ee2bce9 @@ -0,0 +1 @@ +>J \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9d8b7499014b282c6a5ad2bfbd8c196c57d712f7 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9d8b7499014b282c6a5ad2bfbd8c196c57d712f7 new file mode 100644 index 00000000000..1680a4eeab4 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/9d8b7499014b282c6a5ad2bfbd8c196c57d712f7 @@ -0,0 +1 @@ +'' \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/adc83b19e793491b1c6ea0fd8b46cd9f32e592fc b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/adc83b19e793491b1c6ea0fd8b46cd9f32e592fc new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/adc83b19e793491b1c6ea0fd8b46cd9f32e592fc @@ -0,0 +1 @@ + diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/ae67a4f5c81fed57fe9545c429d9760753c753c5 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/ae67a4f5c81fed57fe9545c429d9760753c753c5 new file mode 100644 index 00000000000..c7a004c50be --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/ae67a4f5c81fed57fe9545c429d9760753c753c5 @@ -0,0 +1,2 @@ + +x \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/b220a3cbb136bbd2bbdb46fd3869c4d817394dd4 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/b220a3cbb136bbd2bbdb46fd3869c4d817394dd4 new file mode 100644 index 0000000000000000000000000000000000000000..39af2f7eeb5c76594a12bd3a8b4aa2c7fa34c523 GIT binary patch literal 4 Lcmd1HV_*RQ0}24) literal 0 HcmV?d00001 diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/c4ea21bb365bbeeaf5f2c654883e56d11e43c44e b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/c4ea21bb365bbeeaf5f2c654883e56d11e43c44e new file mode 100644 index 00000000000..25cb955ba23 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/c4ea21bb365bbeeaf5f2c654883e56d11e43c44e @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/cb2698b08ed53e85b181ef4099d4c3b2f76491e6 b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/cb2698b08ed53e85b181ef4099d4c3b2f76491e6 new file mode 100644 index 00000000000..9d445689335 --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/cb2698b08ed53e85b181ef4099d4c3b2f76491e6 @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/e2caca4f6b83f4e6358878d56f1c625efb7abc8f b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/e2caca4f6b83f4e6358878d56f1c625efb7abc8f new file mode 100644 index 00000000000..a326a196e1c --- /dev/null +++ b/tests/fuzz/corpus/s2n_deserialize_resumption_state_test/e2caca4f6b83f4e6358878d56f1c625efb7abc8f @@ -0,0 +1,2 @@ + +rit \ No newline at end of file From 2f5f8ec5904581e70073234fe9b3fc413fd1dc78 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Tue, 30 Jul 2024 03:08:30 -0700 Subject: [PATCH 5/7] fix: default s2nc should accept default s2nd cert (#4670) Co-authored-by: Wesley Rosenblum <55108558+WesleyRosenblum@users.noreply.github.com> --- bin/common.c | 12 +++++++++--- bin/s2nc.c | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/bin/common.c b/bin/common.c index 155acfd07a3..ff19ace9da6 100644 --- a/bin/common.c +++ b/bin/common.c @@ -489,9 +489,15 @@ uint8_t unsafe_verify_host(const char *host_name, size_t host_name_len, void *da return (uint8_t) (strcasecmp(suffix, host_name + 1) == 0); } - if (strcasecmp(host_name, "localhost") == 0 || strcasecmp(host_name, "127.0.0.1") == 0) { - return (uint8_t) (strcasecmp(verify_data->trusted_host, "localhost") == 0 - || strcasecmp(verify_data->trusted_host, "127.0.0.1") == 0); + /* If we're connecting to localhost, accept any values that represent localhost */ + bool is_localhost = (strcasecmp(verify_data->trusted_host, "localhost") == 0); + is_localhost |= (strcasecmp(verify_data->trusted_host, "127.0.0.1") == 0); + if (is_localhost) { + bool match = (strcasecmp(host_name, "localhost") == 0); + match |= (strcasecmp(host_name, "127.0.0.1") == 0); + /* Some of our older test certificates use odd common names */ + match |= (strcasecmp(host_name, "s2nTestServer") == 0); + return (uint8_t) match; } return (uint8_t) (strcasecmp(host_name, verify_data->trusted_host) == 0); diff --git a/bin/s2nc.c b/bin/s2nc.c index 1c7421d2df4..d96e094026d 100644 --- a/bin/s2nc.c +++ b/bin/s2nc.c @@ -47,6 +47,28 @@ #define OPT_SERIALIZE_OUT 1008 #define OPT_DESERIALIZE_IN 1009 +/* This should match the final cert in the s2nd default_certificate_chain */ +const char default_trusted_cert[] = + "-----BEGIN CERTIFICATE-----" + "MIIC/jCCAeagAwIBAgIUFFjxpSf0mUsrVbyLPQhccDYfixowDQYJKoZIhvcNAQEL" + "BQAwFjEUMBIGA1UEAwwLczJuVGVzdFJvb3QwIBcNMjAwMTI0MDEwODIyWhgPMjEx" + "OTEyMzEwMTA4MjJaMBYxFDASBgNVBAMMC3MyblRlc3RSb290MIIBIjANBgkqhkiG" + "9w0BAQEFAAOCAQ8AMIIBCgKCAQEAz3AaOAlkcxJHryCI9SfwB9q4PA53hv5tz4ZL" + "be37b69v58mfP+D18cWIBHUmkmN6gWWoWZ/9hv75pxcNXW0zPn7+wOVvXLUjtmkq" + "1IGT/mykhasw00viaBFAuBHZ5iLwfc4/cjUFAPVCKLmfv5Xs7TJVzWA/0mR4r1h8" + "uFqqXczkVMklIbsOIrlZXz8ifQs3DpFA2FeoziEh+Pcb4c3QBPgCHFDEGyTSdqo9" + "+NbS+iRlw0T6tqUOpC0DdKXo/3mJNBmy4XPahTi9zgsu7b+UVqemL7eXXf/iSr5y" + "iwJKJjz+N/rLpcF1VJtF8q0fpHagzljQaN7/emjg7BplUUyLawIDAQABo0IwQDAP" + "BgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTDmXkyQEJ7ZciyE4KF7wAJKDxMfDAO" + "BgNVHQ8BAf8EBAMCAYYwDQYJKoZIhvcNAQELBQADggEBAFobyhsc7mYoGaA7N4Pp" + "it+MQZZNzWte5vWal/3/2V7ZGrJsgeCPwLblzzTmey85RilX6ovMQHEqT1vBFSHq" + "nntMZnHkEl2QLU8XopJWR4MXK7LzjjQYaXiZhGbJbtylVSfATAa/ZzdgjBx1C8aD" + "IM1+ELGCP/UHD0YEJkFoxSUwXGAXoV8I+cPDAWHC6VnC4mY8qubhx95FpX02ERnz" + "1Cw2YWtntyO8P52dEJD1+0EJjtVX4Bj5wwgJHHbDkPP1IzFrR/uBC2LCjtRY+UtZ" + "kfoDfWu2tslkLK7/LaC5qZyCPKnpPHLLz8gUWKlvbuejM99FTlBg/tcH+bv5x7WB" + "MZ8=" + "-----END CERTIFICATE-----"; + /* * s2nc is an example client that uses many s2n-tls APIs. * It is intended for testing purposes only, and should not be used in production. @@ -616,6 +638,8 @@ int main(int argc, char *const *argv) GUARD_EXIT(s2n_config_add_cert_chain_and_key_to_store(config, chain_and_key), "Error setting certificate/key"); } + GUARD_EXIT(s2n_config_add_pem_to_trust_store(config, default_trusted_cert), + "Error adding default cert to trust store."); if (ca_file || ca_dir) { GUARD_EXIT(s2n_config_wipe_trust_store(config), "Error wiping trust store"); if (s2n_config_set_verification_ca_location(config, ca_file, ca_dir) < 0) { From dec975b1611fe4b500a9b617e9cdb85e4e9aa024 Mon Sep 17 00:00:00 2001 From: Jou Ho <43765840+jouho@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:29:27 -0700 Subject: [PATCH 6/7] ci: move fuzz corpus to S3 (#4665) --- tests/fuzz/Makefile | 6 +++++- tests/fuzz/runFuzzTest.sh | 25 +++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/fuzz/Makefile b/tests/fuzz/Makefile index 266fa280e87..b2bd95dba32 100644 --- a/tests/fuzz/Makefile +++ b/tests/fuzz/Makefile @@ -28,6 +28,10 @@ ifndef FUZZ_TIMEOUT_SEC export FUZZ_TIMEOUT_SEC=120 endif +ifndef CORPUS_UPLOAD_LOC + export CORPUS_UPLOAD_LOC="none" +endif + ifndef FUZZ_TESTS export FUZZ_TESTS=${TESTS} endif @@ -68,7 +72,7 @@ run_tests:: $(FUZZ_TESTS) ld-preload export LD_LIBRARY_PATH=${LD_LIBRARY_PATH}; \ export DYLD_LIBRARY_PATH=${DYLD_LIBRARY_PATH}; \ export LIBCRYPTO_ROOT=${LIBCRYPTO_ROOT}; \ - ./runFuzzTest.sh $${test_name} ${FUZZ_TIMEOUT_SEC}; done; \ + ./runFuzzTest.sh $${test_name} ${FUZZ_TIMEOUT_SEC} ${CORPUS_UPLOAD_LOC}; done; \ } ./calcTotalCov.sh diff --git a/tests/fuzz/runFuzzTest.sh b/tests/fuzz/runFuzzTest.sh index 2e7312bfe05..f3d723d8537 100755 --- a/tests/fuzz/runFuzzTest.sh +++ b/tests/fuzz/runFuzzTest.sh @@ -22,12 +22,13 @@ usage() { exit 1 } -if [ "$#" -ne "2" ]; then +if [ "$#" -ne "3" ]; then usage fi TEST_NAME=$1 FUZZ_TIMEOUT_SEC=$2 +CORPUS_UPLOAD_LOC=$3 MIN_TEST_PER_SEC="1000" MIN_FEATURES_COVERED="100" @@ -75,8 +76,20 @@ ACTUAL_TEST_FAILURE=0 # Copy existing Corpus to a temp directory so that new inputs from fuzz tests runs will add new inputs to the temp directory. # This allows us to minimize new inputs before merging to the original corpus directory. +# If s3 directory is specified, use corpuses stored in S3 bucket instead. TEMP_CORPUS_DIR="$(mktemp -d)" -cp -r ./corpus/${TEST_NAME}/. "${TEMP_CORPUS_DIR}" +if [ "$CORPUS_UPLOAD_LOC" != "none" ]; then + ( + # Clean the environment before copying corpuses from the S3 bucket. + # The LD variables interferes with certificate validation when communicating with AWS S3. + unset LD_PRELOAD + unset LD_LIBRARY_PATH + printf "Copying corpus files from S3 bucket...\n" + aws s3 sync $CORPUS_UPLOAD_LOC/${TEST_NAME}/ "${TEMP_CORPUS_DIR}" + ) +else + cp -r ./corpus/${TEST_NAME}/. "${TEMP_CORPUS_DIR}" +fi # Run AFL instead of libfuzzer if AFL_FUZZ is set. Not compatible with fuzz coverage. if [[ ${AFL_FUZZ} == "true" && ${FUZZ_COVERAGE} != "true" ]]; then @@ -207,6 +220,14 @@ then printf "\033[31;1mERROR!\033[0m ${TEST_NAME} only covers ${FEATURE_COVERAGE} features, which is below ${MIN_FEATURES_COVERED}! This may be due to missing corpus files or a bug.\n" exit -1; fi + + # Store generated corpus files in the S3 bucket. + unset LD_PRELOAD + unset LD_LIBRARY_PATH + if [ "$CORPUS_UPLOAD_LOC" != "none" ]; then + printf "Uploading corpus files to S3 bucket...\n" + aws s3 sync ./corpus/${TEST_NAME}/ $CORPUS_UPLOAD_LOC/${TEST_NAME}/ + fi fi else From ffe7b354650c475f176edb50c97ad3d4826dbf37 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Tue, 30 Jul 2024 17:16:14 -0700 Subject: [PATCH 7/7] feat(bindings): add renegotiate to the rust bindings (#4668) --- bindings/rust/s2n-tls/Cargo.toml | 3 + bindings/rust/s2n-tls/src/callbacks/pkey.rs | 13 +- bindings/rust/s2n-tls/src/config.rs | 41 +- bindings/rust/s2n-tls/src/connection.rs | 119 ++-- bindings/rust/s2n-tls/src/lib.rs | 2 + bindings/rust/s2n-tls/src/renegotiate.rs | 618 +++++++++++++++++++ bindings/rust/s2n-tls/src/testing.rs | 96 +-- bindings/rust/s2n-tls/src/testing/s2n_tls.rs | 6 +- 8 files changed, 782 insertions(+), 116 deletions(-) create mode 100644 bindings/rust/s2n-tls/src/renegotiate.rs diff --git a/bindings/rust/s2n-tls/Cargo.toml b/bindings/rust/s2n-tls/Cargo.toml index aa064a2bd04..e10757cc1e2 100644 --- a/bindings/rust/s2n-tls/Cargo.toml +++ b/bindings/rust/s2n-tls/Cargo.toml @@ -12,6 +12,7 @@ license = "Apache-2.0" default = [] unstable-fingerprint = ["s2n-tls-sys/unstable-fingerprint"] unstable-ktls = ["s2n-tls-sys/unstable-ktls"] +unstable-renegotiate = ["s2n-tls-sys/unstable-renegotiate"] quic = ["s2n-tls-sys/quic"] fips = ["s2n-tls-sys/fips"] pq = ["s2n-tls-sys/pq"] @@ -27,6 +28,8 @@ hex = "0.4" [dev-dependencies] futures-test = "0.3" openssl = "0.10" +openssl-sys = "0.9" +foreign-types = "0.3" temp-env = "0.3" checkers = "0.6" # newer versions require rust 1.66, see https://github.com/aws/s2n-tls/issues/4241 diff --git a/bindings/rust/s2n-tls/src/callbacks/pkey.rs b/bindings/rust/s2n-tls/src/callbacks/pkey.rs index e4977b0cf94..1c9953e8d31 100644 --- a/bindings/rust/s2n-tls/src/callbacks/pkey.rs +++ b/bindings/rust/s2n-tls/src/callbacks/pkey.rs @@ -73,7 +73,7 @@ impl PrivateKeyOperation { /// The size of the slice returned by [`input()`] pub fn input_size(&self) -> Result { let mut size = 0; - unsafe { s2n_async_pkey_op_get_input_size(self.raw.as_ptr(), &mut size) }.into_result()?; + unsafe { s2n_async_pkey_op_get_input_size(self.as_ptr(), &mut size) }.into_result()?; size.try_into().map_err(|_| Error::INVALID_INPUT) } @@ -84,8 +84,7 @@ impl PrivateKeyOperation { pub fn input(&self, buf: &mut [u8]) -> Result<(), Error> { let buf_len: u32 = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?; let buf_ptr = buf.as_ptr() as *mut u8; - unsafe { s2n_async_pkey_op_get_input(self.raw.as_ptr(), buf_ptr, buf_len) } - .into_result()?; + unsafe { s2n_async_pkey_op_get_input(self.as_ptr(), buf_ptr, buf_len) }.into_result()?; Ok(()) } @@ -94,11 +93,15 @@ impl PrivateKeyOperation { let buf_len: u32 = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?; let buf_ptr = buf.as_ptr(); unsafe { - s2n_async_pkey_op_set_output(self.raw.as_ptr(), buf_ptr, buf_len).into_result()?; - s2n_async_pkey_op_apply(self.raw.as_ptr(), conn.as_ptr()).into_result()?; + s2n_async_pkey_op_set_output(self.as_ptr(), buf_ptr, buf_len).into_result()?; + s2n_async_pkey_op_apply(self.as_ptr(), conn.as_ptr()).into_result()?; } Ok(()) } + + pub(crate) fn as_ptr(&self) -> *mut s2n_async_pkey_op { + self.raw.as_ptr() + } } impl Drop for PrivateKeyOperation { diff --git a/bindings/rust/s2n-tls/src/config.rs b/bindings/rust/s2n-tls/src/config.rs index 15485a0a73b..59e33175915 100644 --- a/bindings/rust/s2n-tls/src/config.rs +++ b/bindings/rust/s2n-tls/src/config.rs @@ -1,6 +1,8 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +#[cfg(feature = "unstable-renegotiate")] +use crate::renegotiate::RenegotiateCallback; use crate::{ callbacks::*, enums::*, @@ -436,12 +438,12 @@ impl Builder { verify_host(host_name, host_name_len, handler) } - self.config.context_mut().verify_host_callback = Some(Box::new(handler)); + self.context_mut().verify_host_callback = Some(Box::new(handler)); unsafe { s2n_config_set_verify_host_callback( self.as_mut_ptr(), Some(verify_host_cb_fn), - self.config.context_mut() as *mut Context as *mut c_void, + self.context_mut() as *mut Context as *mut c_void, ) .into_result()?; } @@ -490,7 +492,7 @@ impl Builder { } let handler = Box::new(handler); - let context = self.config.context_mut(); + let context = self.context_mut(); context.client_hello_callback = Some(handler); unsafe { @@ -511,7 +513,7 @@ impl Builder { ) -> Result<&mut Self, Error> { // Store callback in config context let handler = Box::new(handler); - let context = self.config.context_mut(); + let context = self.context_mut(); context.connection_initializer = Some(handler); Ok(self) } @@ -540,14 +542,14 @@ impl Builder { // Store callback in context let handler = Box::new(handler); - let context = self.config.context_mut(); + let context = self.context_mut(); context.session_ticket_callback = Some(handler); unsafe { s2n_config_set_session_ticket_cb( self.as_mut_ptr(), Some(session_ticket_cb), - self.config.context_mut() as *mut Context as *mut c_void, + self.context_mut() as *mut Context as *mut c_void, ) .into_result() }?; @@ -577,7 +579,7 @@ impl Builder { } let handler = Box::new(handler); - let context = self.config.context_mut(); + let context = self.context_mut(); context.private_key_callback = Some(handler); unsafe { @@ -611,13 +613,13 @@ impl Builder { } let handler = Box::new(handler); - let context = self.config.context_mut(); + let context = self.context_mut(); context.wall_clock = Some(handler); unsafe { s2n_config_set_wall_clock( self.as_mut_ptr(), Some(clock_cb), - self.config.context_mut() as *mut _ as *mut c_void, + self.context_mut() as *mut _ as *mut c_void, ) .into_result()?; } @@ -648,13 +650,13 @@ impl Builder { } let handler = Box::new(handler); - let context = self.config.context_mut(); + let context = self.context_mut(); context.monotonic_clock = Some(handler); unsafe { s2n_config_set_monotonic_clock( self.as_mut_ptr(), Some(clock_cb), - self.config.context_mut() as *mut _ as *mut c_void, + self.context_mut() as *mut _ as *mut c_void, ) .into_result()?; } @@ -762,9 +764,20 @@ impl Builder { Ok(self.config) } - fn as_mut_ptr(&mut self) -> *mut s2n_config { + 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")] @@ -797,6 +810,8 @@ pub(crate) struct Context { pub(crate) connection_initializer: Option>, pub(crate) wall_clock: Option>, pub(crate) monotonic_clock: Option>, + #[cfg(feature = "unstable-renegotiate")] + pub(crate) renegotiate: Option>, } impl Default for Context { @@ -814,6 +829,8 @@ impl Default for Context { connection_initializer: None, wall_clock: None, monotonic_clock: None, + #[cfg(feature = "unstable-renegotiate")] + renegotiate: None, } } } diff --git a/bindings/rust/s2n-tls/src/connection.rs b/bindings/rust/s2n-tls/src/connection.rs index 770c556df13..766a248d5ff 100644 --- a/bindings/rust/s2n-tls/src/connection.rs +++ b/bindings/rust/s2n-tls/src/connection.rs @@ -394,6 +394,14 @@ impl Connection { Ok(self) } + /// # Safety + /// + /// The `context` pointer must live at least as long as the connection + pub unsafe fn set_send_context(&mut self, context: *mut c_void) -> Result<&mut Self, Error> { + s2n_connection_set_send_ctx(self.connection.as_ptr(), context).into_result()?; + Ok(self) + } + /// Sets the callback to use for verifying that a hostname from an X.509 certificate is /// trusted. /// @@ -428,14 +436,6 @@ impl Connection { Ok(self) } - /// # Safety - /// - /// The `context` pointer must live at least as long as the connection - pub unsafe fn set_send_context(&mut self, context: *mut c_void) -> Result<&mut Self, Error> { - s2n_connection_set_send_ctx(self.connection.as_ptr(), context).into_result()?; - Ok(self) - } - /// Connections prefering low latency will be encrypted using small record sizes that /// can be decrypted sooner by the recipient. pub fn prefer_low_latency(&mut self) -> Result<&mut Self, Error> { @@ -463,13 +463,10 @@ impl Connection { Ok(self) } - /// wipes an existing connection and allows it to be reused. - /// - /// This method erases all data associated with a connection including pending reads. - /// This function should be called after all I/O is completed and s2n_shutdown has been - /// called. Reusing the same connection handle(s) is more performant than repeatedly - /// calling s2n_connection_new and s2n_connection_free - pub fn wipe(&mut self) -> Result<&mut Self, Error> { + pub(crate) fn wipe_method(&mut self, wipe: F) -> Result<(), Error> + where + F: FnOnce(&mut Self) -> Result, + { let mode = self.mode(); unsafe { // Wiping the connection will wipe the pointer to the context, @@ -477,24 +474,25 @@ impl Connection { let ctx = self.context_mut(); drop(Box::from_raw(ctx)); - s2n_connection_wipe(self.connection.as_ptr()).into_result() + wipe(self) }?; self.init_context(mode); - Ok(self) + Ok(()) } - /// Performs the TLS handshake to completion - /// - /// Multiple callbacks can be configured for a connection and config, but - /// [`Self::poll_negotiate()`] can only execute and block on one callback at a time. - /// The handshake is sequential, not concurrent, and stops execution when - /// it encounters an async callback. + /// wipes an existing connection and allows it to be reused. /// - /// The handshake does not continue execution (and therefore can't call - /// any other callbacks) until the blocking async task reports completion. - pub fn poll_negotiate(&mut self) -> Poll> { - let mut blocked = s2n_blocked_status::NOT_BLOCKED; + /// This method erases all data associated with a connection including pending reads. + /// This function should be called after all I/O is completed and s2n_shutdown has been + /// called. Reusing the same connection handle(s) is more performant than repeatedly + /// calling s2n_connection_new and s2n_connection_free + pub fn wipe(&mut self) -> Result<&mut Self, Error> { + self.wipe_method(|conn| unsafe { s2n_connection_wipe(conn.as_ptr()).into_result() })?; + Ok(self) + } + + fn trigger_initializer(&mut self) { if !core::mem::replace(&mut self.context_mut().connection_initialized, true) { if let Some(config) = self.config() { if let Some(callback) = config.context().connection_initializer.as_ref() { @@ -503,40 +501,6 @@ impl Connection { } } } - - loop { - // check if an async task exists and poll it to completion - if let Some(fut) = self.poll_async_task() { - match fut { - Poll::Ready(Ok(())) => { - // happy case: - // continue and call s2n_negotiate to make progress on the handshake - } - Poll::Ready(Err(err)) => { - // error case: - // if the callback returned an error then abort the handshake - return Poll::Ready(Err(err)); - } - Poll::Pending => return Poll::Pending, - } - } - - let res = unsafe { s2n_negotiate(self.connection.as_ptr(), &mut blocked).into_poll() }; - - match res { - Poll::Ready(res) => { - let res = res.map(|_| self); - return Poll::Ready(res); - } - Poll::Pending => { - // if there is no connection_future then return, otherwise continue - // looping and polling the future - if self.context_mut().async_callback.is_none() { - return Poll::Pending; - } - } - } - } } // Poll the connection future if it exists. @@ -557,6 +521,39 @@ impl Connection { }) } + pub(crate) fn poll_negotiate_method(&mut self, negotiate: F) -> Poll> + where + F: FnOnce(&mut Connection) -> Poll>, + { + self.trigger_initializer(); + + // Check whether renegotiate is blocked by any async callbacks + match self.poll_async_task().unwrap_or(Poll::Ready(Ok(()))) { + Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), + Poll::Pending => return Poll::Pending, + Poll::Ready(Ok(_)) => {} + }; + + negotiate(self).map_ok(|_| ()) + } + + /// Performs the TLS handshake to completion + /// + /// Multiple callbacks can be configured for a connection and config, but + /// [`Self::poll_negotiate()`] can only execute and block on one callback at a time. + /// The handshake is sequential, not concurrent, and stops execution when + /// it encounters an async callback. + /// + /// The handshake does not continue execution (and therefore can't call + /// any other callbacks) until the blocking async task reports completion. + pub fn poll_negotiate(&mut self) -> Poll> { + let mut blocked = s2n_blocked_status::NOT_BLOCKED; + self.poll_negotiate_method(|conn| unsafe { + s2n_negotiate(conn.as_ptr(), &mut blocked).into_poll() + }) + .map_ok(|_| self) + } + /// Encrypts and sends data on a connection where /// [negotiate](`Self::poll_negotiate`) has succeeded. /// diff --git a/bindings/rust/s2n-tls/src/lib.rs b/bindings/rust/s2n-tls/src/lib.rs index 27dc0f6f534..c9da760c0f9 100644 --- a/bindings/rust/s2n-tls/src/lib.rs +++ b/bindings/rust/s2n-tls/src/lib.rs @@ -23,6 +23,8 @@ pub mod enums; pub mod fingerprint; pub mod init; pub mod pool; +#[cfg(feature = "unstable-renegotiate")] +pub mod renegotiate; pub mod security; pub use s2n_tls_sys as ffi; diff --git a/bindings/rust/s2n-tls/src/renegotiate.rs b/bindings/rust/s2n-tls/src/renegotiate.rs new file mode 100644 index 00000000000..a1832618eeb --- /dev/null +++ b/bindings/rust/s2n-tls/src/renegotiate.rs @@ -0,0 +1,618 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Methods to perform renegotiation. +//! +//! The use of renegotiation is strongly discouraged. +//! See [the C API documentation](https://github.com/aws/s2n-tls/blob/main/api/unstable/renegotiate.h). + +use s2n_tls_sys::*; + +use crate::{ + callbacks::with_context, + config, + connection::Connection, + enums::CallbackResult, + error::{Error, Fallible, Pollable}, +}; +use std::task::Poll; + +/// How to handle a renegotiation request. +/// +/// See s2n_renegotiate_response in [the C API documentation](https://github.com/aws/s2n-tls/blob/main/api/unstable/renegotiate.h). +#[derive(Debug, PartialEq, Copy, Clone)] +pub enum RenegotiateResponse { + Ignore, + Reject, + Accept, +} + +impl From for s2n_renegotiate_response::Type { + fn from(input: RenegotiateResponse) -> s2n_renegotiate_response::Type { + match input { + RenegotiateResponse::Ignore => s2n_renegotiate_response::RENEGOTIATE_IGNORE, + RenegotiateResponse::Reject => s2n_renegotiate_response::RENEGOTIATE_REJECT, + RenegotiateResponse::Accept => s2n_renegotiate_response::RENEGOTIATE_ACCEPT, + } + } +} + +/// A callback that triggers when the server requests renegotiation. +/// +/// Returning "None" will result in the C callback returning an error, +/// canceling the connection. +/// +/// See s2n_renegotiate_request_cb in [the C API documentation](https://github.com/aws/s2n-tls/blob/main/api/unstable/renegotiate.h). +// +// 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. +pub trait RenegotiateCallback: 'static + Send + Sync { + fn on_renegotiate_request( + &mut self, + connection: &mut Connection, + ) -> Option; +} + +impl RenegotiateCallback for RenegotiateResponse { + fn on_renegotiate_request(&mut self, _conn: &mut Connection) -> Option { + Some(*self) + } +} + +impl Connection { + /// Reset the connection so that it can be renegotiated. + /// + /// Returning "None" will result in the C callback returning an error, + /// canceling the connection. + /// + /// See s2n_renegotiate_wipe in [the C API documentation](https://github.com/aws/s2n-tls/blob/main/api/unstable/renegotiate.h). + /// The Rust equivalent of the connection-specific methods listed are: + /// - Methods to set the file descriptors: not currently supported by rust bindings + /// - Methods to set the send callback: + /// ([Connection::set_send_callback()], [Connection::set_send_context()]) + /// - Methods to set the recv callback: + /// ([Connection::set_receive_callback()], [Connection::set_receive_context()]) + pub fn wipe_for_renegotiate(&mut self) -> Result<(), Error> { + self.wipe_method(|conn| unsafe { s2n_renegotiate_wipe(conn.as_ptr()).into_result() }) + } + + /// Perform a new handshake on an already established connection. + /// + /// The first element of the returned pair represents progress on the new + /// handshake, like [Connection::poll_negotiate()]. + /// + /// If any application data is received during the new handshake, the number + /// of bytes received is returned as the second element of the returned pair, + /// and the data is written to `buf`. + /// + /// See s2n_renegotiate in [the C API documentation](https://github.com/aws/s2n-tls/blob/main/api/unstable/renegotiate.h). + pub fn poll_renegotiate(&mut self, buf: &mut [u8]) -> (Poll>, usize) { + let mut blocked = s2n_blocked_status::NOT_BLOCKED; + let buf_len: isize = buf.len().try_into().unwrap_or(0); + let buf_ptr = buf.as_mut_ptr(); + let mut read: isize = 0; + + let r = self.poll_negotiate_method(|conn| { + unsafe { s2n_renegotiate(conn.as_ptr(), buf_ptr, buf_len, &mut read, &mut blocked) } + .into_poll() + }); + (r, read.try_into().unwrap()) + } +} + +impl config::Builder { + /// Sets a method to be called when the client receives a request to renegotiate. + /// + /// See s2n_config_set_renegotiate_request_cb in [the C API documentation](https://github.com/aws/s2n-tls/blob/main/api/unstable/renegotiate.h). + pub fn set_renegotiate_callback( + &mut self, + handler: T, + ) -> Result<&mut Self, Error> { + unsafe extern "C" fn renegotiate_cb( + conn_ptr: *mut s2n_connection, + _context: *mut libc::c_void, + response: *mut s2n_renegotiate_response::Type, + ) -> libc::c_int { + with_context(conn_ptr, |conn, context| { + let callback = context.renegotiate.as_mut(); + if let Some(callback) = callback { + if let Some(result) = callback.on_renegotiate_request(conn) { + *response = result.into(); + return CallbackResult::Success.into(); + } + } + return CallbackResult::Failure.into(); + }) + } + + let handler = Box::new(handler); + let context = self.context_mut(); + context.renegotiate = Some(handler); + unsafe { + s2n_config_set_renegotiate_request_cb( + self.as_mut_ptr(), + Some(renegotiate_cb), + std::ptr::null_mut(), + ) + .into_result()?; + } + Ok(self) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + callbacks::{ + ConnectionFuture, ConnectionFutureResult, PrivateKeyCallback, PrivateKeyOperation, + }, + config::ConnectionInitializer, + testing::{CertKeyPair, InsecureAcceptAllCertificatesHandler, TestPair, TestPairIO}, + }; + use foreign_types::ForeignTypeRef; + use futures_test::task::new_count_waker; + use openssl::ssl::{ + ErrorCode, NameType, Ssl, SslContext, SslFiletype, SslMethod, SslStream, SslVerifyMode, + SslVersion, + }; + use std::{ + error::Error, + io::{Read, Write}, + pin::Pin, + task::Poll::{Pending, Ready}, + }; + + // Currently renegotiation is not available from the openssl-sys bindings + extern "C" { + fn SSL_renegotiate(s: *mut openssl_sys::SSL) -> libc::size_t; + fn SSL_renegotiate_pending(s: *mut openssl_sys::SSL) -> libc::size_t; + } + + // std::task::ready is unstable + fn unwrap_poll( + poll: Poll>, + ) -> Result<(), crate::error::Error> { + if let Ready(value) = poll { + return value.map(|_| ()); + } + panic!("Poll not Ready"); + } + + #[derive(Debug)] + struct ServerTestStream(TestPairIO); + + // For server testing purposes, we read from the client output stream + impl Read for ServerTestStream { + fn read(&mut self, buf: &mut [u8]) -> Result { + let result = self.0.client_tx_stream.borrow_mut().read(buf); + if let Ok(0) = result { + // Treat no data as blocking instead of EOF + Err(std::io::Error::new( + std::io::ErrorKind::WouldBlock, + "blocking", + )) + } else { + result + } + } + } + + // For server testing purposes, we write to the server output stream + impl Write for ServerTestStream { + fn write(&mut self, buf: &[u8]) -> Result { + self.0.server_tx_stream.borrow_mut().write(buf) + } + + fn flush(&mut self) -> Result<(), std::io::Error> { + self.0.server_tx_stream.borrow_mut().flush() + } + } + + // s2n-tls doesn't support sending client hello requests. + // This makes it impossible to test renegotiation without direct access to + // s2n-tls internals like the methods for sending arbitrary records. + // Instead, we need to use openssl as our server. + // + // The openssl SslStream::new method requires an owned Stream, + // so the openssl server owns the TestPairIO. This is possible because the + // s2n-tls client only references the TestPairIO via C callbacks. + struct RenegotiateTestPair { + client: Connection, + server: SslStream, + } + + impl RenegotiateTestPair { + fn from(mut builder: config::Builder) -> Result> { + // openssl and s2n-tls must be configured to accept each other's + // certificates. Some tests will require client auth. + // + // openssl also requires a properly configured CA cert, which the + // default TestPair does not include. + let certs_dir = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../../tests/pems/permutations/rsae_pkcs_4096_sha384/" + ); + let certs = CertKeyPair::from(certs_dir, "server-chain", "server-key", "ca-cert"); + + // Build the s2n-tls client. + builder.load_pem(certs.cert(), certs.key())?; + builder.trust_pem(certs.cert())?; + builder.set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})?; + let config = builder.build()?; + let s2n_pair = TestPair::from_config(&config); + let client = s2n_pair.client; + + // Build the openssl server. + let mut ctx_builder = SslContext::builder(SslMethod::tls_server())?; + ctx_builder.set_max_proto_version(Some(SslVersion::TLS1_2))?; + ctx_builder.set_min_proto_version(Some(SslVersion::TLS1_2))?; + ctx_builder.set_certificate_chain_file(certs.cert_path())?; + ctx_builder.set_private_key_file(certs.key_path(), SslFiletype::PEM)?; + ctx_builder.set_ca_file(certs.ca_path())?; + ctx_builder.set_verify(SslVerifyMode::PEER); + let openssl_ctx = ctx_builder.build(); + let openssl_ssl = Ssl::new(&openssl_ctx)?; + + // Connect the openssl server to the same IO that the s2n-tls + // client was constructed to use. + let server_stream = ServerTestStream(s2n_pair.io); + let server = SslStream::new(openssl_ssl, server_stream)?; + + Ok(Self { client, server }) + } + + // Translate the output of openssl's `accept` to match s2n-tls's `poll_negotiate`. + fn poll_openssl_negotiate( + server: &mut SslStream, + ) -> Poll>> { + match server.accept() { + Ok(_) => Ready(Ok(())), + Err(err) if err.code() == ErrorCode::WANT_READ => Pending, + Err(err) => Ready(Err(err.into())), + } + } + + // Perform a handshake with the s2n-tls client and openssl server + fn handshake(&mut self) -> Result<(), Box> { + loop { + match ( + self.client.poll_negotiate(), + Self::poll_openssl_negotiate(&mut self.server), + ) { + (Poll::Ready(Ok(_)), Poll::Ready(Ok(_))) => return Ok(()), + // Error on the server + (_, Poll::Ready(Err(e))) => return Err(e), + // Error on the client + (Poll::Ready(Err(e)), _) => return Err(Box::new(e)), + _ => continue, + } + } + } + + // Send and receive the hello request message, triggering renegotiate. + // The result of s2n-tls receiving the request is returned. + fn trigger_renegotiate(&mut self) -> Result<(), crate::error::Error> { + let openssl_ptr = self.server.ssl().as_ptr(); + + // Sanity check that renegotiation is not initially scheduled + let requested = unsafe { SSL_renegotiate_pending(openssl_ptr) }; + assert_eq!(requested, 0, "Renegotiation should not be pending"); + + // Schedule renegotiation + unsafe { SSL_renegotiate(openssl_ptr) }; + + // Verify that openssl scheduled the renegotiation + let requested = unsafe { SSL_renegotiate_pending(openssl_ptr) }; + assert_eq!(requested, 1, "Renegotiation should be pending"); + + // SSL_renegotiate doesn't actually send the message. + // Like s2n-tls, a call to send / write is required. + let to_send = [0; 1]; + self.server + .write(&to_send) + .expect("Failed to write hello request"); + + // s2n-tls needs to attempt to read data to receive the message + let mut recv_buffer = [0; 1]; + unwrap_poll(self.client.poll_recv(&mut recv_buffer)) + } + + // Send and receive application data. + // We have to ensure that application data continues to work during / after + // the renegotiate. + fn send_and_receive(&mut self) -> Result<(), Box> { + let to_send = [0; 1]; + let mut recv_buffer = [0; 1]; + self.server.write_all(&to_send)?; + unwrap_poll(self.client.poll_recv(&mut recv_buffer))?; + unwrap_poll(self.client.poll_send(&to_send))?; + self.server.read(&mut recv_buffer)?; + Ok(()) + } + + fn assert_renegotiate(&mut self) { + let mut empty = [0; 0]; + let mut buf = [0; 1]; + let mut result = Pending; + while result.is_pending() { + // openssl can only make progress by sending and receiving a 0-length array. + // Both operations can fail for a number of irrelevant reasons while + // still making progress, so we just ignore the results. + _ = self.server.write(&empty); + _ = self.server.read(&mut empty); + + let (r, n) = self.client.poll_renegotiate(&mut buf); + assert_eq!(n, 0, "Unexpected application data"); + result = r; + } + unwrap_poll(result).expect("Renegotiate"); + } + } + + #[test] + fn ignore_callback() -> Result<(), Box> { + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(RenegotiateResponse::Ignore)?; + let mut pair = RenegotiateTestPair::from(builder)?; + + pair.handshake().expect("Initial handshake"); + + // Expect receiving the hello request to be successful + pair.trigger_renegotiate().expect("Trigger renegotiate"); + pair.send_and_receive().expect("Application data"); + + Ok(()) + } + + #[test] + fn error_callback() -> Result<(), Box> { + struct ErrorRenegotiateCallback {} + impl RenegotiateCallback for ErrorRenegotiateCallback { + fn on_renegotiate_request( + &mut self, + _: &mut Connection, + ) -> Option { + None + } + } + + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(ErrorRenegotiateCallback {})?; + let mut pair = RenegotiateTestPair::from(builder)?; + + pair.handshake().expect("Initial handshake"); + // Expect receiving the hello request to be an error + let error = pair.trigger_renegotiate().unwrap_err(); + assert_eq!(error.name(), "S2N_ERR_CANCELLED"); + + Ok(()) + } + + #[test] + fn reject_callback() -> Result<(), Box> { + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(RenegotiateResponse::Reject)?; + let mut pair = RenegotiateTestPair::from(builder)?; + + pair.handshake().expect("Initial handshake"); + // Expect handling the hello request to succeed. + // s2n-tls doesn't fail when it rejects renegotiatation, it just sends + // a warning alert. The peer chooses how to handle that alert. + pair.trigger_renegotiate().expect("Trigger renegotiate"); + // The openssl server receives the alert on its next read. + // Openssl considers the alert an error. + let openssl_error = pair.send_and_receive().unwrap_err(); + assert!(openssl_error.to_string().contains("no renegotiation")); + + Ok(()) + } + + #[test] + fn do_renegotiate() -> Result<(), Box> { + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(RenegotiateResponse::Accept)?; + let mut pair = RenegotiateTestPair::from(builder)?; + + pair.handshake().expect("Initial handshake"); + pair.trigger_renegotiate().expect("Trigger renegotiate"); + pair.send_and_receive() + .expect("Application data before renegotiate"); + pair.client + .wipe_for_renegotiate() + .expect("Wipe for renegotiate"); + + pair.assert_renegotiate(); + + pair.send_and_receive() + .expect("Application data after renegotiate"); + Ok(()) + } + + #[test] + fn do_renegotiate_with_app_data() -> Result<(), Box> { + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(RenegotiateResponse::Accept)?; + let mut pair = RenegotiateTestPair::from(builder)?; + + pair.handshake().expect("Initial handshake"); + pair.trigger_renegotiate().expect("Trigger renegotiate"); + pair.send_and_receive() + .expect("Application data before renegotiate"); + pair.client + .wipe_for_renegotiate() + .expect("Wipe for renegotiate"); + + let to_write = "hello world"; + let mut buf = [0; 100]; + pair.server + .write(to_write.as_bytes()) + .expect("Application data during renegotiate"); + let (result, n) = pair.client.poll_renegotiate(&mut buf); + assert!(result.is_pending()); + assert_eq!(n, to_write.len(), "Incorrect application data"); + assert_eq!(&buf[..n], to_write.as_bytes()); + + Ok(()) + } + + #[test] + fn do_renegotiate_with_async_callback() -> Result<(), Box> { + // To test how renegotiate handles blocking on async callbacks, + // we need an async callback that triggers on the client. + // Currently our only option is the async pkey callback. + struct TestAsyncCallback { + count: usize, + op: Option, + } + impl PrivateKeyCallback for TestAsyncCallback { + fn handle_operation( + &self, + _: &mut Connection, + operation: PrivateKeyOperation, + ) -> ConnectionFutureResult { + Ok(Some(Box::pin(TestAsyncCallback { + count: self.count, + op: Some(operation), + }))) + } + } + impl ConnectionFuture for TestAsyncCallback { + fn poll( + self: Pin<&mut Self>, + conn: &mut Connection, + ctx: &mut core::task::Context, + ) -> Poll> { + ctx.waker().wake_by_ref(); + let this = self.get_mut(); + if this.count > 1 { + // Repeatedly block the handshake in order to verify + // that renegotiate can handle Pending callbacks. + this.count = this.count - 1; + Pending + } else { + // Perform the pkey operation with the selected cert / key pair. + let op = this.op.take().unwrap(); + let opt_ptr = op.as_ptr(); + let chain_ptr = conn.selected_cert().unwrap().as_mut_ptr().as_ptr(); + unsafe { + let key_ptr = s2n_cert_chain_and_key_get_private_key(chain_ptr) + .into_result()? + .as_ptr(); + s2n_async_pkey_op_perform(opt_ptr, key_ptr).into_result()?; + s2n_async_pkey_op_apply(opt_ptr, conn.as_ptr()).into_result()?; + } + Ready(Ok(())) + } + } + } + + let count_per_handshake = 10; + let async_callback = TestAsyncCallback { + count: count_per_handshake, + op: None, + }; + + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(RenegotiateResponse::Accept)?; + builder.set_private_key_callback(async_callback)?; + let mut pair = RenegotiateTestPair::from(builder)?; + + let (waker, wake_count) = new_count_waker(); + pair.client.set_waker(Some(&waker))?; + + pair.handshake().expect("Initial handshake"); + assert_eq!(wake_count, count_per_handshake); + pair.trigger_renegotiate().expect("Trigger renegotiate"); + pair.send_and_receive() + .expect("Application data before renegotiate"); + pair.client + .wipe_for_renegotiate() + .expect("Wipe for renegotiate"); + // Reset the waker + pair.client.set_waker(Some(&waker))?; + + pair.assert_renegotiate(); + assert_eq!(wake_count, count_per_handshake * 2); + + Ok(()) + } + + #[test] + fn do_renegotiate_with_async_init() -> Result<(), Box> { + // To test that the initializer method triggers again on the second + // handshake, we need to set an easily verified connection-level value. + // The server name is convenient. + #[derive(Clone)] + struct TestInitializer { + count: usize, + server_name: String, + } + impl ConnectionInitializer for TestInitializer { + fn initialize_connection( + &self, + _: &mut crate::connection::Connection, + ) -> ConnectionFutureResult { + Ok(Some(Box::pin(self.clone()))) + } + } + impl ConnectionFuture for TestInitializer { + fn poll( + self: Pin<&mut Self>, + conn: &mut Connection, + ctx: &mut core::task::Context, + ) -> Poll> { + ctx.waker().wake_by_ref(); + let this = self.get_mut(); + if this.count > 1 { + // Repeatedly block the handshake in order to verify + // that renegotiate can handle Pending callbacks. + this.count = this.count - 1; + Pending + } else { + conn.set_server_name(&this.server_name)?; + Ready(Ok(())) + } + } + } + + let count_per_handshake = 10; + let expected_server_name = "helloworld"; + let initializer = TestInitializer { + count: count_per_handshake, + server_name: expected_server_name.to_owned(), + }; + + let mut builder = config::Builder::new(); + builder.set_renegotiate_callback(RenegotiateResponse::Accept)?; + builder.set_connection_initializer(initializer)?; + let mut pair = RenegotiateTestPair::from(builder)?; + + let (waker, wake_count) = new_count_waker(); + pair.client.set_waker(Some(&waker))?; + + pair.handshake().expect("Initial handshake"); + assert_eq!(wake_count, count_per_handshake); + pair.trigger_renegotiate().expect("Trigger renegotiate"); + pair.send_and_receive() + .expect("Application data before renegotiate"); + pair.client + .wipe_for_renegotiate() + .expect("Wipe for renegotiate"); + // Verify that the wipe cleared the server name + assert!(pair.client.server_name().is_none()); + // Reset the waker + pair.client.set_waker(Some(&waker))?; + + pair.assert_renegotiate(); + assert_eq!(wake_count, count_per_handshake * 2); + + // Both the client and server should have the correct server name + let server_name = pair.client.server_name(); + assert_eq!(Some(expected_server_name), server_name); + let server_name = pair.server.ssl().servername(NameType::HOST_NAME); + assert_eq!(Some(expected_server_name), server_name); + + Ok(()) + } +} diff --git a/bindings/rust/s2n-tls/src/testing.rs b/bindings/rust/s2n-tls/src/testing.rs index 2626cb07c23..5ba35b3c33f 100644 --- a/bindings/rust/s2n-tls/src/testing.rs +++ b/bindings/rust/s2n-tls/src/testing.rs @@ -60,35 +60,59 @@ impl Default for Counter { } pub struct CertKeyPair { - cert_path: &'static str, - cert: &'static [u8], - key: &'static [u8], + cert_path: String, + key_path: String, + ca_path: String, + cert: Vec, + key: Vec, } impl Default for CertKeyPair { fn default() -> Self { - CertKeyPair { - cert_path: concat!( - env!("CARGO_MANIFEST_DIR"), - "/../../../tests/pems/rsa_4096_sha512_client_cert.pem", - ), - cert: &include_bytes!("../../../../tests/pems/rsa_4096_sha512_client_cert.pem")[..], - key: &include_bytes!("../../../../tests/pems/rsa_4096_sha512_client_key.pem")[..], - } + let prefix = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../../tests/pems/rsa_4096_sha512_client_" + ); + Self::from(prefix, "cert", "key", "cert") } } impl CertKeyPair { - pub fn cert_path(&self) -> &'static str { - self.cert_path + pub fn from(prefix: &str, chain: &str, key: &str, ca: &str) -> Self { + let cert_path = format!("{prefix}{chain}.pem"); + let key_path = format!("{prefix}{key}.pem"); + let ca_path = format!("{prefix}{ca}.pem"); + let cert = std::fs::read(&cert_path) + .unwrap_or_else(|_| panic!("Failed to read cert at {cert_path}")); + let key = + std::fs::read(&key_path).unwrap_or_else(|_| panic!("Failed to read key at {key_path}")); + CertKeyPair { + cert_path, + key_path, + ca_path, + cert, + key, + } + } + + pub fn cert_path(&self) -> &str { + &self.cert_path + } + + pub fn key_path(&self) -> &str { + &self.key_path + } + + pub fn ca_path(&self) -> &str { + &self.ca_path } - pub fn cert(&self) -> &'static [u8] { - self.cert + pub fn cert(&self) -> &[u8] { + &self.cert } - pub fn key(&self) -> &'static [u8] { - self.key + pub fn key(&self) -> &[u8] { + &self.key } } @@ -130,6 +154,22 @@ pub fn config_builder(cipher_prefs: &security::Policy) -> Result>; +#[derive(Debug)] +// We allow dead_code, because otherwise the compiler complains about the tx_streams +// never being read. This is because it can't reason through the pointers that were +// passed into the s2n-tls connection io contexts. +#[allow(dead_code)] +pub struct TestPairIO { + // Pin: since we are dereferencing this pointer (because it is passed as the send/recv ctx) + // we need to ensure that the pointer remains in the same place + // Box: A Vec (or VecDeque) may be moved or reallocated, so we need another layer of + // indirection to have a stable (pinned) reference + /// a data buffer that the server writes to and the client reads from + pub server_tx_stream: Pin>, + /// a data buffer that the client writes to and the server reads from + pub client_tx_stream: Pin>, +} + /// TestPair is a testing utility used to easily test handshakes and send data. /// /// SAFETY: if the server or client connection is moved outside of the struct, IO @@ -155,23 +195,10 @@ type LocalDataBuffer = RefCell>; // // The doctest is `ignore`d because testing utilities are not publicly exported // and therefore can't be referenced in a doc comment. -// -// We allow dead_code, because otherwise the compiler complains about the tx_streams -// never being read. This is because it can't reason through the pointers that were -// passed into the s2n-tls connection io contexts. -#[allow(dead_code)] pub struct TestPair { pub server: connection::Connection, pub client: connection::Connection, - - // Pin: since we are dereferencing this pointer (because it is passed as the send/recv ctx) - // we need to ensure that the pointer remains in the same place - // Box: A Vec (or VecDeque) may be moved or reallocated, so we need another layer of - // indirection to have a stable (pinned) reference - /// a data buffer that the server writes to and the client reads from - server_tx_stream: Pin>, - /// a data buffer that the client writes to and the server reads from - client_tx_stream: Pin>, + pub io: TestPairIO, } impl TestPair { @@ -212,12 +239,11 @@ impl TestPair { ) .unwrap(); - Self { - server, - client, + let io = TestPairIO { server_tx_stream, client_tx_stream, - } + }; + Self { server, client, io } } /// create a connection ready for harness IO diff --git a/bindings/rust/s2n-tls/src/testing/s2n_tls.rs b/bindings/rust/s2n-tls/src/testing/s2n_tls.rs index 8d670a99e2c..1da872fa277 100644 --- a/bindings/rust/s2n-tls/src/testing/s2n_tls.rs +++ b/bindings/rust/s2n-tls/src/testing/s2n_tls.rs @@ -618,8 +618,8 @@ mod tests { builder .set_client_hello_callback(TestClientHelloHandler {}) .unwrap(); - builder.load_pem(keypair.cert, keypair.key).unwrap(); - builder.trust_pem(keypair.cert).unwrap(); + builder.load_pem(keypair.cert(), keypair.key()).unwrap(); + builder.trust_pem(keypair.cert()).unwrap(); builder.build().unwrap() }; let mut pair = TestPair::from_config(&config); @@ -702,7 +702,7 @@ mod tests { let mut pair = TestPair::from_config(&config); drop(pair.client); - let mut client_tx_stream = pair.client_tx_stream.borrow_mut(); + let mut client_tx_stream = pair.io.client_tx_stream.borrow_mut(); client_tx_stream.write_all(SSLV2_CLIENT_HELLO_HEADER)?; client_tx_stream.write_all(SSLV2_CLIENT_HELLO_PREFIX)?; client_tx_stream.write_all(SSLV2_CLIENT_HELLO_CIPHER_SUITES)?;