From c93d1ba7c49b30e9f1342467507c2b60187ad58e Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Fri, 1 Nov 2024 17:10:36 -0400 Subject: [PATCH 01/18] chore: bindings release 0.3.6 (#4867) --- bindings/rust/s2n-tls-hyper/Cargo.toml | 4 ++-- bindings/rust/s2n-tls-sys/templates/Cargo.template | 2 +- bindings/rust/s2n-tls-tokio/Cargo.toml | 4 ++-- bindings/rust/s2n-tls/Cargo.toml | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bindings/rust/s2n-tls-hyper/Cargo.toml b/bindings/rust/s2n-tls-hyper/Cargo.toml index 075e25b4614..8c474a9e739 100644 --- a/bindings/rust/s2n-tls-hyper/Cargo.toml +++ b/bindings/rust/s2n-tls-hyper/Cargo.toml @@ -13,8 +13,8 @@ publish = false default = [] [dependencies] -s2n-tls = { version = "=0.3.5", path = "../s2n-tls" } -s2n-tls-tokio = { version = "=0.3.5", path = "../s2n-tls-tokio" } +s2n-tls = { version = "=0.3.6", path = "../s2n-tls" } +s2n-tls-tokio = { version = "=0.3.6", path = "../s2n-tls-tokio" } hyper = { version = "1" } hyper-util = { version = "0.1", features = ["client-legacy", "tokio", "http1"] } tower-service = { version = "0.3" } diff --git a/bindings/rust/s2n-tls-sys/templates/Cargo.template b/bindings/rust/s2n-tls-sys/templates/Cargo.template index a1a5091c63d..fb4a5bd960e 100644 --- a/bindings/rust/s2n-tls-sys/templates/Cargo.template +++ b/bindings/rust/s2n-tls-sys/templates/Cargo.template @@ -1,7 +1,7 @@ [package] name = "s2n-tls-sys" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.5" +version = "0.3.6" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" diff --git a/bindings/rust/s2n-tls-tokio/Cargo.toml b/bindings/rust/s2n-tls-tokio/Cargo.toml index bf1bb075c3e..2a3b3073034 100644 --- a/bindings/rust/s2n-tls-tokio/Cargo.toml +++ b/bindings/rust/s2n-tls-tokio/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-tokio" description = "An implementation of TLS streams for Tokio built on top of s2n-tls" -version = "0.3.5" +version = "0.3.6" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -15,7 +15,7 @@ default = [] errno = { version = "0.3" } libc = { version = "0.2" } pin-project-lite = { version = "0.2" } -s2n-tls = { version = "=0.3.5", path = "../s2n-tls" } +s2n-tls = { version = "=0.3.6", path = "../s2n-tls" } tokio = { version = "1", features = ["net", "time"] } [dev-dependencies] diff --git a/bindings/rust/s2n-tls/Cargo.toml b/bindings/rust/s2n-tls/Cargo.toml index 8c9c60e9416..ab895344473 100644 --- a/bindings/rust/s2n-tls/Cargo.toml +++ b/bindings/rust/s2n-tls/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.5" +version = "0.3.6" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -21,7 +21,7 @@ unstable-testing = [] [dependencies] errno = { version = "0.3" } libc = "0.2" -s2n-tls-sys = { version = "=0.3.5", path = "../s2n-tls-sys", features = ["internal"] } +s2n-tls-sys = { version = "=0.3.6", path = "../s2n-tls-sys", features = ["internal"] } pin-project-lite = "0.2" hex = "0.4" From 53691f9feba06ca371ea03f371391bbbb7b227d5 Mon Sep 17 00:00:00 2001 From: Cameron Bytheway Date: Mon, 4 Nov 2024 08:38:45 -0700 Subject: [PATCH 02/18] build: add s2n_prelude.h to consolidate defines (#4465) --- CMakeLists.txt | 11 +++--- api/s2n.h | 9 ++--- bindings/rust/s2n-tls-sys/build.rs | 16 ++++++--- s2n.mk | 5 +-- tests/features/S2N_MADVISE_SUPPORTED.c | 12 +------ tests/features/S2N_MINHERIT_SUPPORTED.c | 10 +----- tests/unit/s2n_build_test.c | 6 ++++ tls/s2n_config.c | 5 +++ utils/s2n_fork_detection.c | 32 +++--------------- utils/s2n_fork_detection_features.h | 43 +++++++++++++++++++++++ utils/s2n_prelude.h | 45 +++++++++++++++++++++++++ 11 files changed, 128 insertions(+), 66 deletions(-) create mode 100644 utils/s2n_fork_detection_features.h create mode 100644 utils/s2n_prelude.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 77e76acc1a9..6ff981fa001 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -192,11 +192,12 @@ if(S2N_BLOCK_NONPORTABLE_OPTIMIZATIONS) target_compile_options(${PROJECT_NAME} PUBLIC -DS2N_BLOCK_NONPORTABLE_OPTIMIZATIONS=1) endif() -target_compile_options(${PROJECT_NAME} PUBLIC -fPIC) +target_compile_options(${PROJECT_NAME} PUBLIC -fPIC -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h") -add_definitions(-D_POSIX_C_SOURCE=200809L) -if(CMAKE_BUILD_TYPE MATCHES Release) - add_definitions(-D_FORTIFY_SOURCE=2) +# Match on Release, RelWithDebInfo and MinSizeRel +# See: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html#variable:CMAKE_BUILD_TYPE +if(CMAKE_BUILD_TYPE MATCHES Rel) + add_definitions(-DS2N_BUILD_RELEASE) endif() if(NO_STACK_PROTECTOR) @@ -331,7 +332,7 @@ function(feature_probe PROBE_NAME) SOURCES "${CMAKE_CURRENT_LIST_DIR}/tests/features/${PROBE_NAME}.c" LINK_LIBRARIES ${LINK_LIB} ${OS_LIBS} CMAKE_FLAGS ${ADDITIONAL_FLAGS} - COMPILE_DEFINITIONS -c ${GLOBAL_FLAGS} ${PROBE_FLAGS} + COMPILE_DEFINITIONS -I "${CMAKE_CURRENT_LIST_DIR}" -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h" -c ${GLOBAL_FLAGS} ${PROBE_FLAGS} ${ARGN} OUTPUT_VARIABLE TRY_COMPILE_OUTPUT ) diff --git a/api/s2n.h b/api/s2n.h index 48f70a6a41a..f104a5ee2af 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -22,17 +22,12 @@ #pragma once -#if ((__GNUC__ >= 4) || defined(__clang__)) && defined(S2N_EXPORTS) - /** - * Marks a function as belonging to the public s2n API. - */ - #define S2N_API __attribute__((visibility("default"))) -#else +#ifndef S2N_API /** * Marks a function as belonging to the public s2n API. */ #define S2N_API -#endif /* __GNUC__ >= 4 || defined(__clang__) */ +#endif #ifdef __cplusplus extern "C" { diff --git a/bindings/rust/s2n-tls-sys/build.rs b/bindings/rust/s2n-tls-sys/build.rs index 32b4efda1ea..d3473a03427 100644 --- a/bindings/rust/s2n-tls-sys/build.rs +++ b/bindings/rust/s2n-tls-sys/build.rs @@ -85,9 +85,14 @@ fn build_vendored() { build.files(include!("./files.rs")); - if env("PROFILE") == "release" { - // fortify source is only available in release mode - build.define("_FORTIFY_SOURCE", "2"); + // https://doc.rust-lang.org/cargo/reference/environment-variables.html + // * OPT_LEVEL, DEBUG — values of the corresponding variables for the profile currently being built. + // * PROFILE — release for release builds, debug for other builds. This is determined based on if + // the profile inherits from the dev or release profile. Using this environment variable is not + // recommended. Using other environment variables like OPT_LEVEL provide a more correct view of + // the actual settings being used. + if env("OPT_LEVEL") != "0" { + build.define("S2N_BUILD_RELEASE", "1"); build.define("NDEBUG", "1"); // build s2n-tls with LTO if supported @@ -166,6 +171,8 @@ fn builder(libcrypto: &Libcrypto) -> cc::Build { }; build + .flag("-include") + .flag("lib/utils/s2n_prelude.h") .flag("-std=c11") .flag("-fgnu89-inline") // make sure the stack is non-executable @@ -173,8 +180,7 @@ fn builder(libcrypto: &Libcrypto) -> cc::Build { .flag_if_supported("-z now") .flag_if_supported("-z noexecstack") // we use some deprecated libcrypto features so don't warn here - .flag_if_supported("-Wno-deprecated-declarations") - .define("_POSIX_C_SOURCE", "200112L"); + .flag_if_supported("-Wno-deprecated-declarations"); build } diff --git a/s2n.mk b/s2n.mk index 0195388eec2..a5c3bdaff93 100644 --- a/s2n.mk +++ b/s2n.mk @@ -49,9 +49,10 @@ endif DEFAULT_CFLAGS += -pedantic -Wall -Werror -Wimplicit -Wunused -Wcomment -Wchar-subscripts -Wuninitialized \ -Wshadow -Wcast-align -Wwrite-strings -fPIC -Wno-missing-braces\ - -D_POSIX_C_SOURCE=200809L -O2 -I$(LIBCRYPTO_ROOT)/include/ \ + -O2 -I$(LIBCRYPTO_ROOT)/include/ \ + -DS2N_BUILD_RELEASE -include utils/s2n_prelude.h \ -I$(S2N_ROOT)/api/ -I$(S2N_ROOT) -Wno-deprecated-declarations -Wno-unknown-pragmas -Wformat-security \ - -D_FORTIFY_SOURCE=2 -fgnu89-inline -fvisibility=hidden -DS2N_EXPORTS + -fgnu89-inline -fvisibility=hidden -DS2N_EXPORTS COVERAGE_CFLAGS = -fprofile-arcs -ftest-coverage COVERAGE_LDFLAGS = --coverage diff --git a/tests/features/S2N_MADVISE_SUPPORTED.c b/tests/features/S2N_MADVISE_SUPPORTED.c index 5b98cd23a38..312f84627fa 100644 --- a/tests/features/S2N_MADVISE_SUPPORTED.c +++ b/tests/features/S2N_MADVISE_SUPPORTED.c @@ -13,17 +13,7 @@ * permissions and limitations under the License. */ -/* Keep in sync with utils/s2n_fork_detection.c */ -#if defined(__FreeBSD__) - /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) - * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ - #undef _POSIX_C_SOURCE -#elif !defined(__APPLE__) && !defined(_GNU_SOURCE) - #define _GNU_SOURCE -#endif - -#include -#include +#include "utils/s2n_fork_detection_features.h" int main() { madvise(NULL, 0, 0); diff --git a/tests/features/S2N_MINHERIT_SUPPORTED.c b/tests/features/S2N_MINHERIT_SUPPORTED.c index 13ce6975ae8..fc662367f36 100644 --- a/tests/features/S2N_MINHERIT_SUPPORTED.c +++ b/tests/features/S2N_MINHERIT_SUPPORTED.c @@ -13,15 +13,7 @@ * permissions and limitations under the License. */ -/* Keep in sync with utils/s2n_fork_detection.c */ -#if defined(__FreeBSD__) - /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) - * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ - #undef _POSIX_C_SOURCE -#endif - -#include -#include +#include "utils/s2n_fork_detection_features.h" int main() { minherit(NULL, 0, 0); diff --git a/tests/unit/s2n_build_test.c b/tests/unit/s2n_build_test.c index e7f40949d15..87001ab3078 100644 --- a/tests/unit/s2n_build_test.c +++ b/tests/unit/s2n_build_test.c @@ -12,6 +12,12 @@ * express or implied. See the License for the specific language governing * permissions and limitations under the License. */ + +#ifndef _S2N_PRELUDE_INCLUDED + /* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ + #error "Expected s2n_prelude.h to be included as part of the compiler flags" +#endif + #define _GNU_SOURCE #include #include diff --git a/tls/s2n_config.c b/tls/s2n_config.c index 4db579dd465..ccc1940c0ac 100644 --- a/tls/s2n_config.c +++ b/tls/s2n_config.c @@ -13,6 +13,11 @@ * permissions and limitations under the License. */ +#ifndef _S2N_PRELUDE_INCLUDED + /* make sure s2n_prelude.h is includes as part of the compiler flags, if not then fail the build */ + #error "Expected s2n_prelude.h to be included as part of the compiler flags" +#endif + #include #include diff --git a/utils/s2n_fork_detection.c b/utils/s2n_fork_detection.c index 87d66ba4c63..92d82cac604 100644 --- a/utils/s2n_fork_detection.c +++ b/utils/s2n_fork_detection.c @@ -13,36 +13,14 @@ * permissions and limitations under the License. */ -/* This captures Darwin specialities. This is the only APPLE flavor we care about. - * Here we also capture varius required feature test macros. - */ -#if defined(__APPLE__) -typedef struct _opaque_pthread_once_t __darwin_pthread_once_t; -typedef __darwin_pthread_once_t pthread_once_t; - #define _DARWIN_C_SOURCE -#elif defined(__FreeBSD__) || defined(__OpenBSD__) - /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) - * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ - #undef _POSIX_C_SOURCE -#elif !defined(_GNU_SOURCE) - /* Keep in sync with feature probe tests/features/madvise.c */ - #define _GNU_SOURCE -#endif +/* force the internal header to be included first, since it modifies _GNU_SOURCE/_POSIX_C_SOURCE */ +/* clang-format off */ +#include "utils/s2n_fork_detection_features.h" +/* clang-format on */ -#include - -/* Not always defined for Darwin */ -#if !defined(MAP_ANONYMOUS) - #define MAP_ANONYMOUS MAP_ANON -#endif - -#include -#include -#include -#include +#include "utils/s2n_fork_detection.h" #include "error/s2n_errno.h" -#include "utils/s2n_fork_detection.h" #include "utils/s2n_safety.h" #if defined(S2N_MADVISE_SUPPORTED) && defined(MADV_WIPEONFORK) diff --git a/utils/s2n_fork_detection_features.h b/utils/s2n_fork_detection_features.h new file mode 100644 index 00000000000..e0d78691b0a --- /dev/null +++ b/utils/s2n_fork_detection_features.h @@ -0,0 +1,43 @@ +/* + * 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. + */ + +#pragma once + +/* This captures Darwin specialities. This is the only APPLE flavor we care about. + * Here we also capture various required feature test macros. + */ +#if defined(__APPLE__) +typedef struct _opaque_pthread_once_t __darwin_pthread_once_t; +typedef __darwin_pthread_once_t pthread_once_t; + #define _DARWIN_C_SOURCE +#elif defined(__FreeBSD__) || defined(__OpenBSD__) + /* FreeBSD requires POSIX compatibility off for its syscalls (enables __BSD_VISIBLE) + * Without the below line, cannot be imported (it requires __BSD_VISIBLE) */ + #undef _POSIX_C_SOURCE +#elif !defined(_GNU_SOURCE) + #define _GNU_SOURCE +#endif + +#include + +/* Not always defined for Darwin */ +#if !defined(MAP_ANONYMOUS) + #define MAP_ANONYMOUS MAP_ANON +#endif + +#include +#include +#include +#include diff --git a/utils/s2n_prelude.h b/utils/s2n_prelude.h new file mode 100644 index 00000000000..364cc82ca87 --- /dev/null +++ b/utils/s2n_prelude.h @@ -0,0 +1,45 @@ +/* + * 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. + */ + +#pragma once + +/* Let modules know that the prelude was included */ +#define _S2N_PRELUDE_INCLUDED + +/* Define the POSIX API we are targeting */ +#ifndef _POSIX_C_SOURCE + #define _POSIX_C_SOURCE 200809L +#endif + +/** + * If we're building in release mode make sure _FORTIFY_SOURCE is set + * See: https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html + * https://man7.org/linux/man-pages/man7/feature_test_macros.7.html + * + * NOTE: _FORTIFY_SOURCE can only be set when optimizations are enabled. + * https://sourceware.org/git/?p=glibc.git;a=commit;f=include/features.h;h=05c2c9618f583ea4acd69b3fe5ae2a2922dd2ddc + */ +#if !defined(_FORTIFY_SOURCE) && defined(S2N_BUILD_RELEASE) + #define _FORTIFY_SOURCE 2 +#endif + +#if ((__GNUC__ >= 4) || defined(__clang__)) && defined(S2N_EXPORTS) + /** + * Marks a function as belonging to the public s2n API. + * + * See: https://gcc.gnu.org/wiki/Visibility + */ + #define S2N_API __attribute__((visibility("default"))) +#endif From 01e90bb34829aa2d63c2c74534e33474085c3422 Mon Sep 17 00:00:00 2001 From: Cameron Bytheway Date: Thu, 7 Nov 2024 10:38:53 -0700 Subject: [PATCH 03/18] fix: move prelude inclusion as PRIVATE (#4876) --- CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ff981fa001..e272660c67e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -192,7 +192,10 @@ if(S2N_BLOCK_NONPORTABLE_OPTIMIZATIONS) target_compile_options(${PROJECT_NAME} PUBLIC -DS2N_BLOCK_NONPORTABLE_OPTIMIZATIONS=1) endif() -target_compile_options(${PROJECT_NAME} PUBLIC -fPIC -include "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h") +target_compile_options(${PROJECT_NAME} PUBLIC -fPIC) + +set(S2N_PRELUDE "${CMAKE_CURRENT_LIST_DIR}/utils/s2n_prelude.h") +target_compile_options(${PROJECT_NAME} PRIVATE -include "${S2N_PRELUDE}") # Match on Release, RelWithDebInfo and MinSizeRel # See: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html#variable:CMAKE_BUILD_TYPE @@ -476,6 +479,8 @@ if (BUILD_TESTING) add_library(testss2n STATIC ${TESTLIB_SRC} ${EXAMPLES_SRC}) target_include_directories(testss2n PUBLIC tests) target_compile_options(testss2n PRIVATE -std=gnu99) + # make sure all linked tests include the prelude + target_compile_options(testss2n PUBLIC -include "${S2N_PRELUDE}") target_link_libraries(testss2n PUBLIC ${PROJECT_NAME}) if (SECCOMP) message(STATUS "Linking tests with seccomp") From 12b140a98a60c7d43eb89f891eab8c7f8d3de614 Mon Sep 17 00:00:00 2001 From: toidiu Date: Fri, 8 Nov 2024 15:28:53 -0800 Subject: [PATCH 04/18] ci: remove www.mozilla.com from well-known to unblock CI (#4880) --- tests/integrationv2/test_well_known_endpoints.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integrationv2/test_well_known_endpoints.py b/tests/integrationv2/test_well_known_endpoints.py index 684f6d19ab6..21869520da0 100644 --- a/tests/integrationv2/test_well_known_endpoints.py +++ b/tests/integrationv2/test_well_known_endpoints.py @@ -38,7 +38,8 @@ "www.github.com", "www.ibm.com", "www.microsoft.com", - "www.mozilla.org", + # https://github.com/aws/s2n-tls/issues/4879 + # "www.mozilla.org", "www.netflix.com", "www.openssl.org", "www.samsung.com", From b589fa420687ea6d6a774fe0df5bc4d5dfe8331a Mon Sep 17 00:00:00 2001 From: Doug Chapman <54039637+dougch@users.noreply.github.com> Date: Mon, 11 Nov 2024 10:24:02 -0800 Subject: [PATCH 05/18] ci: Clean dup source tree for CRT (#4882) --- codebuild/bin/build_aws_crt_cpp.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/codebuild/bin/build_aws_crt_cpp.sh b/codebuild/bin/build_aws_crt_cpp.sh index aae7a2ee773..5947a10300a 100755 --- a/codebuild/bin/build_aws_crt_cpp.sh +++ b/codebuild/bin/build_aws_crt_cpp.sh @@ -30,6 +30,8 @@ source codebuild/bin/s2n_setup_env.sh BUILD_DIR=$1 INSTALL_DIR=$2 +# Make sure there isn't another source tree hanging around. +rm -rf /opt/s2n-tls || true mkdir -p "$BUILD_DIR/s2n" # In case $BUILD_DIR is a subdirectory of current directory for file in *;do test "$file" != "$BUILD_DIR" && cp -r "$file" "$BUILD_DIR/s2n";done From 84561bbce17e1300693cb666962704a990203244 Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Mon, 11 Nov 2024 12:28:04 -0800 Subject: [PATCH 06/18] chore: remove unused benchmarks (#4869) --- CMakeLists.txt | 2 +- bindings/rust/integration/Cargo.toml | 13 --- bindings/rust/integration/benches/s2nc.rs | 30 ------- bindings/rust/integration/benches/s2nd.rs | 30 ------- codebuild/bin/criterion_baseline.sh | 66 -------------- codebuild/bin/criterion_delta.sh | 53 ----------- .../buildspec_ubuntu_integv2criterion.yml | 59 ------------- ...dspec_ubuntu_integv2criterion_baseline.yml | 30 ------- tests/integrationv2/README.md | 36 -------- tests/integrationv2/conftest.py | 5 +- tests/integrationv2/fixtures.py | 9 +- tests/integrationv2/global_flags.py | 10 --- tests/integrationv2/providers.py | 87 ------------------- .../test_well_known_endpoints.py | 6 +- tests/integrationv2/tox.ini | 1 - 15 files changed, 5 insertions(+), 432 deletions(-) delete mode 100644 bindings/rust/integration/benches/s2nc.rs delete mode 100644 bindings/rust/integration/benches/s2nd.rs delete mode 100755 codebuild/bin/criterion_baseline.sh delete mode 100755 codebuild/bin/criterion_delta.sh delete mode 100644 codebuild/spec/buildspec_ubuntu_integv2criterion.yml delete mode 100644 codebuild/spec/buildspec_ubuntu_integv2criterion_baseline.yml diff --git a/CMakeLists.txt b/CMakeLists.txt index e272660c67e..d16e5f5f053 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -639,7 +639,7 @@ if (BUILD_TESTING) pytest -x -n=${N} --reruns=2 --durations=10 --cache-clear -rpfsq -o log_cli=true --log-cli-level=DEBUG --provider-version=$ENV{S2N_LIBCRYPTO} - --provider-criterion=off --fips-mode=0 ${test_file_path} + --fips-mode=0 ${test_file_path} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests/integrationv2 ) else() diff --git a/bindings/rust/integration/Cargo.toml b/bindings/rust/integration/Cargo.toml index a229b2df586..684cc4714ac 100644 --- a/bindings/rust/integration/Cargo.toml +++ b/bindings/rust/integration/Cargo.toml @@ -8,17 +8,4 @@ publish = false [dependencies] s2n-tls = { path = "../s2n-tls"} s2n-tls-sys = { path = "../s2n-tls-sys" } -criterion = { version = "0.3", features = ["html_reports"] } -anyhow = "1" -unicode-width = "=0.1.13" # newer versions require newer rust, see https://github.com/aws/s2n-tls/issues/4786 -[[bench]] -name = "s2nc" -harness = false - -[[bench]] -name = "s2nd" -harness = false - -[dev-dependencies] -regex = "=1.9.6" # newer versions require rust 1.65, see https://github.com/aws/s2n-tls/issues/4242 diff --git a/bindings/rust/integration/benches/s2nc.rs b/bindings/rust/integration/benches/s2nc.rs deleted file mode 100644 index d2fe97ff0db..00000000000 --- a/bindings/rust/integration/benches/s2nc.rs +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -use criterion::{criterion_group, criterion_main, Criterion}; -use std::{env, process::Command, time::Duration}; - -pub fn s2nc(c: &mut Criterion) { - let mut group = c.benchmark_group("s2nc"); - let s2nc_env: &str = &env::var("S2NC_ARGS").unwrap(); - let s2nc_test_name: &str = &env::var("S2NC_TEST_NAME").unwrap(); - let test_name = format!("s2nc_{}", s2nc_test_name); - let s2nc_split = s2nc_env.split(' ').collect::>(); - group.bench_function(test_name, move |b| { - b.iter(|| { - let s2nc_argvec = s2nc_split.clone(); - let status = Command::new("/usr/local/bin/s2nc") - .args(s2nc_argvec) - .status() - .expect("failed to execute process"); - assert!(status.success()); - }); - }); - - group.finish(); -} - -criterion_group!(name = benches; - config = Criterion::default().sample_size(10).measurement_time(Duration::from_secs(1)); - targets = s2nc); -criterion_main!(benches); diff --git a/bindings/rust/integration/benches/s2nd.rs b/bindings/rust/integration/benches/s2nd.rs deleted file mode 100644 index 8e2d06235fa..00000000000 --- a/bindings/rust/integration/benches/s2nd.rs +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -use criterion::{criterion_group, criterion_main, Criterion}; -use std::{env, process::Command, time::Duration}; - -pub fn s2nd(c: &mut Criterion) { - let mut group = c.benchmark_group("s2nd"); - let s2nd_env: &str = &env::var("S2ND_ARGS").unwrap(); - let s2nd_test_name: &str = &env::var("S2ND_TEST_NAME").unwrap(); - let test_name = format!("s2nd_{}", s2nd_test_name); - let s2nd_split = s2nd_env.split(' ').collect::>(); - group.bench_function(test_name, move |b| { - b.iter(|| { - let s2nd_argvec = s2nd_split.clone(); - let status = Command::new("/usr/local/bin/s2nd") - .args(s2nd_argvec) - .status() - .expect("failed to execute process"); - assert!(status.success()); - }); - }); - - group.finish(); -} - -criterion_group!(name = benches; - config = Criterion::default().sample_size(10).measurement_time(Duration::from_secs(1)); - targets = s2nd); -criterion_main!(benches); diff --git a/codebuild/bin/criterion_baseline.sh b/codebuild/bin/criterion_baseline.sh deleted file mode 100755 index fa644b0a80a..00000000000 --- a/codebuild/bin/criterion_baseline.sh +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/env bash -# 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. - -set -eu -source codebuild/bin/s2n_setup_env.sh -source codebuild/bin/utils.sh - -# Limit the number of child processes in the test run -export RUST_BACKTRACE=1 -export TOX_TEST_NAME="$INTEGV2_TEST" - -# There can be only one artifact config per batch job, -# so we're scipting the baseline upload steps here. -upload_artifacts(){ - cd tests/integrationv2/target/criterion - echo "Creating zip ${AWS_S3_PATH}" - zip -r "${AWS_S3_PATH}" ./* - aws s3 cp "${AWS_S3_PATH}" "${AWS_S3_URL}" - echo "S3 upload complete" -} - -if [ -d "third-party-src" ]; then - echo "Not running against c.a.c." - return 0 -fi - -# setting LOCAL_TESTING disables a check for an existing baseline. -if [ -z "${LOCAL_TESTING:-}" ]; then - # Fetch creds and the latest release number. - gh_login s2n_codebuild_PRs - LATEST_RELEASE_VER=$(get_latest_release) - # Build a specific filename for this release - AWS_S3_PATH="integv2criterion_${INTEGV2_TEST}_${LATEST_RELEASE_VER}.zip" - zip_count=$(aws s3 ls "${AWS_S3_URL}${AWS_S3_PATH}"|wc -l||true) - - # Only do the baseline if an artifact for the current release doesn't exist. - if [ "$zip_count" -eq 0 ]; then - echo "File ${AWS_S3_URL}${AWS_S3_PATH} not found" - criterion_install_deps - ORIGINAL_COMMIT=$(git rev-parse HEAD) - git fetch --tags - git checkout "$LATEST_RELEASE_VER" - S2N_USE_CRITERION=baseline make -C tests/integrationv2 "$INTEGV2_TEST" - upload_artifacts - git reset --hard ${ORIGINAL_COMMIT} - else - echo "Found existing artifact for ${LATEST_RELEASE_VER}, not rebuilding." - exit 0 - fi -else - echo "Local testing enabled; baselining without checking s3" - criterion_install_deps - S2N_USE_CRITERION=baseline make -C tests/integrationv2 "$INTEGV2_TEST" -fi - diff --git a/codebuild/bin/criterion_delta.sh b/codebuild/bin/criterion_delta.sh deleted file mode 100755 index b2a28fecec7..00000000000 --- a/codebuild/bin/criterion_delta.sh +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env bash -# 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. -set -eu -source ./codebuild/bin/utils.sh -export AWS_S3_BUCKET="s3://s2n-tls-logs/" -# Limit the number of child processes in the test run -export RUST_BACKTRACE=1 - -export GIT_COMMIT=$(git log -n 1 --format="%h") -export AWS_S3_REPORT_PATH="reports/${INTEGV2_TEST}/$(date +%Y%m%d_%H%M_${GIT_COMMIT})" - -# CodeBuild artifacts are too limited; -# scipting the baseline download steps here. -download_artifacts(){ - mkdir -p ./tests/integrationv2/target/criterion || true - echo "Downloading ${AWS_S3_BUCKET}${1}/${2}" - pushd ./tests/integrationv2/target/criterion/ - aws s3 cp "${AWS_S3_BUCKET}${1}/${2}" . - unzip -o "${2}" - echo "S3 download complete" - popd -} - -upload_report(){ - cd tests/integrationv2/target/criterion - echo "Uploading report to ${AWS_S3_BUCKET}/${AWS_S3_REPORT_PATH}" - aws s3 sync . "${AWS_S3_BUCKET}${AWS_S3_REPORT_PATH}" - echo "S3 upload complete" -} - -# Fetch creds and the latest release number. -gh_login s2n_codebuild_PRs -LATEST_RELEASE_VER=$(get_latest_release) -AWS_ZIPFILE="integv2criterion_${INTEGV2_TEST}_${LATEST_RELEASE_VER}.zip" -AWS_S3_BASE_PATH="release" -criterion_install_deps -download_artifacts ${AWS_S3_BASE_PATH} ${AWS_ZIPFILE} - -echo "Current dir: $(pwd)" -S2N_USE_CRITERION=delta make -C tests/integrationv2 "$INTEGV2_TEST" -upload_report - diff --git a/codebuild/spec/buildspec_ubuntu_integv2criterion.yml b/codebuild/spec/buildspec_ubuntu_integv2criterion.yml deleted file mode 100644 index 0b737941c73..00000000000 --- a/codebuild/spec/buildspec_ubuntu_integv2criterion.yml +++ /dev/null @@ -1,59 +0,0 @@ ---- -version: 0.2 - -env: - variables: - # CODEBUILD_ is a reserved namespace. - CB_BIN_DIR: "./codebuild/bin" - -# Doc for batch https://docs.aws.amazon.com/codebuild/latest/userguide/batch-build-buildspec.html#build-spec.batch.build-list -batch: - build-graph: - - identifier: s2nIntegrationv2WellKnownEndpointsBaseline - buildspec: codebuild/spec/buildspec_ubuntu_integv2criterion_baseline.yml - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - variables: - INTEGV2_TEST: test_well_known_endpoints - S2N_USE_CRITERION: 2 - TESTS: integrationv2crit - GCC_VERSION: 6 - RUST_BACKTRACE: 1 - - identifier: s2nIntegrationv2WellKnownEndpoints - debug-session: true - env: - privileged-mode: true - compute-type: BUILD_GENERAL1_LARGE - variables: - INTEGV2_TEST: test_well_known_endpoints - S2N_USE_CRITERION: 1 - TESTS: integrationv2crit - GCC_VERSION: 6 - ARTIFACT_BUCKET: s3://s2n-tls-logs/release - ARTIFACT_FILE: integv2criterion - depend-on: - - s2nIntegrationv2WellKnownEndpointsBaseline - -phases: - install: - runtime-versions: - python: 3.x - commands: - - $CB_BIN_DIR/install_ubuntu_dependencies.sh - - $CB_BIN_DIR/utils.sh gh_login s2n_codebuild_PRs - - export LATEST_RELEASE_VER=$($CB_BIN_DIR/utils.sh get_latest_release) - - mkdir -p tests/integrationv2/target/criterion || true - - aws s3 cp ${ARTIFACT_BUCKET}/${ARTIFACT_FILE}_${INTEGV2_TEST}_${LATEST_RELEASE_VER}.zip . - - unzip -o ${ARTIFACT_FILE}_${INTEGV2_TEST}_${LATEST_RELEASE_VER}.zip -d ./tests/integrationv2/target/criterion - build: - commands: - - codebuild-breakpoint - - $CB_BIN_DIR/criterion_delta.sh -artifacts: - files: - - "**/index.html" - - "**/*.svg" - - "**/*.json" - base-directory: "tests/integrationv2/target/criterion" - discard-paths: no diff --git a/codebuild/spec/buildspec_ubuntu_integv2criterion_baseline.yml b/codebuild/spec/buildspec_ubuntu_integv2criterion_baseline.yml deleted file mode 100644 index f6b43b2c535..00000000000 --- a/codebuild/spec/buildspec_ubuntu_integv2criterion_baseline.yml +++ /dev/null @@ -1,30 +0,0 @@ ---- -version: 0.2 -env: - variables: - # CODEBUILD_ is a reserved namespace. - CB_BIN_DIR: "./codebuild/bin" -phases: - install: - runtime-versions: - python: 3.x - commands: - - | - if [ -d "third-party-src" ]; then - cd third-party-src; - fi - - $CB_BIN_DIR/install_ubuntu_dependencies.sh - build: - commands: - - $CB_BIN_DIR/criterion_baseline.sh - - mkdir -p tests/integrationv2/target/criterion - - echo "{id:$CODEBUILD_BUILD_ID}" >> tests/integrationv2/target/criterion/artifact.json - -artifacts: - files: - - "**/index.html" - - "**/*.svg" - - "**/*.json" - base-directory: "tests/integrationv2/target/criterion" - discard-paths: no - diff --git a/tests/integrationv2/README.md b/tests/integrationv2/README.md index a6ef767eea7..ed3cb41bb5b 100644 --- a/tests/integrationv2/README.md +++ b/tests/integrationv2/README.md @@ -156,39 +156,3 @@ An example of how to test that the server and the client can send and receive ap **INTERNALERROR> OSError: cannot send to ** An error similar to this is caused by a runtime error in a test. In `tox.ini` change `-n8` to `-n0` to see the actual error causing the OSError. - - -# Criterion - -### Why - -We wanted to use the rust criterion project to benchmark s2n-tls, without re-writing all the integration tests. -To accomplish this, we created some criterion benchmarks that use s2nc and s2nd, and a new provider, CriterionS2N, in the python testing framework. - -### Prerequisites - -Normally, you'd run criterion with `cargo bench --bench `, but cargo does some checks to see if it needs to rebuild -the benchmark and other housekeeping that slows things down a bit. Running `cargo bench --no-run` is the benchmark equivalent to `cargo build` and will produce a binary executable. - -The CI will run `make install` and `make -C ./bindings/rust` to create and install the s2nc/d binaries in a system-wide location, then build the cargo criterion binary handlers for s2nc and s2nd. - - -### Running locally - -The Criterion CodeBuild scripts can be used to run these locally, by setting LOCAL_TESTING the s3/github interactions are disabled. Tooling needed includes python3.9, rust, and write permissions to `/usr/local/bin|lib` (or use sudo) - in addition to the traditional C build tooling. - -``` -export LOCAL_TESTING=true -INTEGV2_TEST=test_well_known_endpoints ./codebuild/bin/criterion_baseline.sh -INTEGV2_TEST=test_well_known_endpoints ./codebuild/bin/criterion_delta.sh -``` - -The resulting reports are viewable via `tests/integrationv2/target/criterion/report/index.html` - - - -## Troubleshooting CriterionS2N - -The most direct trouble-shooting is done using the [interactive troubleshooting CodeBuild](https://docs.aws.amazon.com/codebuild/latest/userguide/session-manager.html#ssm-pause-build) `codebuild-break` line in the buildspec. Put the break right before the main build step and run interactively. - -As mentioned above, in order to get more output from the tests, set the `-n` or `XDIST_WORKER` environment variable to 0 and add a -v to the pytest command line in tox.ini. diff --git a/tests/integrationv2/conftest.py b/tests/integrationv2/conftest.py index d71ff1e9401..55b992ef698 100644 --- a/tests/integrationv2/conftest.py +++ b/tests/integrationv2/conftest.py @@ -1,6 +1,6 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -from global_flags import set_flag, S2N_PROVIDER_VERSION, S2N_FIPS_MODE, S2N_USE_CRITERION +from global_flags import set_flag, S2N_PROVIDER_VERSION, S2N_FIPS_MODE def pytest_addoption(parser): @@ -8,8 +8,6 @@ def pytest_addoption(parser): default=None, type=str, help="Set the version of the TLS provider") parser.addoption("--fips-mode", action="store", dest="fips-mode", default=False, type=int, help="S2N is running in FIPS mode") - parser.addoption("--provider-criterion", action="store", dest="provider-criterion", - default="off", type=str, choices=['off', 'baseline', 'delta'], help="Use Criterion provider in one of 3 modes: [off,baseline,delta]") def pytest_configure(config): @@ -26,7 +24,6 @@ def pytest_configure(config): set_flag(S2N_FIPS_MODE, True) set_flag(S2N_PROVIDER_VERSION, config.getoption('provider-version', None)) - set_flag(S2N_USE_CRITERION, config.getoption('provider-criterion', "off")) def pytest_collection_modifyitems(config, items): diff --git a/tests/integrationv2/fixtures.py b/tests/integrationv2/fixtures.py index 4ec0c3a3e96..dad2b7afbed 100644 --- a/tests/integrationv2/fixtures.py +++ b/tests/integrationv2/fixtures.py @@ -4,9 +4,9 @@ import pytest import subprocess -from global_flags import get_flag, is_criterion_on, S2N_USE_CRITERION +from global_flags import get_flag from processes import ManagedProcess -from providers import Provider, CriterionS2N, S2N +from providers import Provider, S2N from common import ProviderOptions @@ -25,11 +25,6 @@ def managed_process(): def _fn(provider_class: Provider, options: ProviderOptions, timeout=5, send_marker=None, close_marker=None, expect_stderr=None, kill_marker=None, send_with_newline=None): - if provider_class == S2N and is_criterion_on(): - provider_class = CriterionS2N - # This comes from the number of iterations specific in the rust benchmark handler(s). - # currently set at 10 iterations, so give us 10x more time. - timeout = timeout * 10 provider = provider_class(options) cmd_line = provider.get_cmd_line() # The process will default to send markers in the providers.py file diff --git a/tests/integrationv2/global_flags.py b/tests/integrationv2/global_flags.py index 149a22eacfc..853cd3feb02 100644 --- a/tests/integrationv2/global_flags.py +++ b/tests/integrationv2/global_flags.py @@ -11,16 +11,6 @@ # (set from the S2N_LIBCRYPTO env var, which is how the original integration test works) S2N_PROVIDER_VERSION = 's2n_provider_version' -# From S2N_USE_CRITERION env var. -S2N_USE_CRITERION = 's2n_use_criterion' - - -def is_criterion_on(): - if _flags.get(S2N_USE_CRITERION, "off") in ["baseline", "delta"]: - return True - return False - - _flags = {} diff --git a/tests/integrationv2/providers.py b/tests/integrationv2/providers.py index 55b1efc628d..ae203af6e6c 100644 --- a/tests/integrationv2/providers.py +++ b/tests/integrationv2/providers.py @@ -7,7 +7,6 @@ from common import ProviderOptions, Ciphers, Curves, Protocols, Signatures from global_flags import get_flag, S2N_PROVIDER_VERSION, S2N_FIPS_MODE -from global_flags import S2N_USE_CRITERION from stat import S_IMODE @@ -332,92 +331,6 @@ def setup_server(self): return cmd_line -class CriterionS2N(S2N): - """ - Wrap the S2N provider in criterion-rust - The CriterionS2N provider modifies the test command being run by pytest to be the criterion benchmark binary - and moves the s2nc/d command line arguments into an environment variable. The binary created by - `cargo bench --no-run` has a unique name and must be searched for, which the CriterionS2N provider finds - and replaces in the final testing command. The arguments to s2nc/d are moved to the environment variables - `S2NC_ARGS` or `S2ND_ARGS`, along with the test name, which are read by the rust benchmark when spawning - s2nc/d as subprocesses. - """ - criterion_off = 'off' - criterion_delta = 'delta' - criterion_baseline = 'baseline' - # Figure out what mode to run in: baseline or delta - criterion_mode = get_flag(S2N_USE_CRITERION) - - def _find_s2n_benchmark(self, pattern): - # Use executable bit to find the correct file. - result = find_files(pattern, root_dir=self.cargo_root, modes=['0o775', '0o755']) - if len(result) != 1: - raise FileNotFoundError( - f"Exactly one s2n criterion benchmark not found. Found {result}.") - else: - return result[0] - - def _find_cargo(self): - return os.path.abspath("../../bindings/rust") - - def _cargo_bench(self): - """ - Find the handlers - """ - self.s2nc_bench = self._find_s2n_benchmark("s2nc-") - self.s2nd_bench = self._find_s2n_benchmark("s2nd-") - - def __init__(self, options: ProviderOptions): - super().__init__(options) - # Set cargo root - self.cargo_root = self._find_cargo() - - # Find the criterion binaries - self._cargo_bench() - - # strip off the s2nc/d at the front because criterion - if 's2nc' in self.cmd_line[0] or 's2nd' in self.cmd_line[0]: - self.cmd_line = self.cmd_line[1:] - - # strip off the s2nc -e argument, criterion handler isn't sending any STDIN characters, - # and makes it look like s2nc is hanging. - # WARNING: this is a blocker for any test that requires STDIN. - if '-e' in self.cmd_line: - self.cmd_line.remove('-e') - print(f"***** cmd_line is now {self.cmd_line}") - - # Copy the command arguments to an environment variable for the harness to read. - if self.options.mode == Provider.ServerMode: - self.options.env_overrides.update( - {'S2ND_ARGS': ' '.join(self.cmd_line), - 'S2ND_TEST_NAME': f"{self.options.cipher}_{self.options.host}"}) - self.capture_server_args() - elif self.options.mode == Provider.ClientMode: - self.options.env_overrides.update( - {'S2NC_ARGS': ' '.join(self.cmd_line), - 'S2NC_TEST_NAME': f"{self.options.cipher}_{self.options.host}"}) - if self.criterion_mode == CriterionS2N.criterion_baseline: - self.capture_client_args_baseline() - if self.criterion_mode == CriterionS2N.criterion_delta: - self.capture_client_args_delta() - - def capture_server_args(self): - self.cmd_line = [self.s2nd_bench, "--bench", - "s2nd", "--save-baseline", "main"] - - # Saves baseline data with the tag "main" - # see https://bheisler.github.io/criterion.rs/book/user_guide/command_line_options.html - def capture_client_args_baseline(self): - self.cmd_line = [self.s2nc_bench, "--bench", - "s2nc", "--save-baseline", "main"] - - # "By default, Criterion.rs will compare the measurements against the previous run" - # This run is stored with the tag "new" - # https://bheisler.github.io/criterion.rs/book/user_guide/command_line_options.html - def capture_client_args_delta(self): - self.cmd_line = [self.s2nc_bench, "--bench", "s2nc", "--plotting-backend", "plotters", "--baseline", "main"] - - class OpenSSL(Provider): _version = get_flag(S2N_PROVIDER_VERSION) diff --git a/tests/integrationv2/test_well_known_endpoints.py b/tests/integrationv2/test_well_known_endpoints.py index 21869520da0..7bce5c2248a 100644 --- a/tests/integrationv2/test_well_known_endpoints.py +++ b/tests/integrationv2/test_well_known_endpoints.py @@ -6,7 +6,7 @@ from configuration import PROTOCOLS from common import ProviderOptions, Ciphers, pq_enabled from fixtures import managed_process # lgtm [py/unused-import] -from global_flags import get_flag, is_criterion_on, S2N_FIPS_MODE, S2N_USE_CRITERION +from global_flags import get_flag, S2N_FIPS_MODE from providers import Provider, S2N from test_pq_handshake import PQ_ENABLED_FLAG from utils import invalid_test_parameters, get_parameter_name, to_bytes @@ -110,10 +110,6 @@ def test_well_known_endpoints(managed_process, protocol, endpoint, provider, cip if get_flag(S2N_FIPS_MODE) is True: client_options.trust_store = TRUST_STORE_TRUSTED_BUNDLE - # TODO: Understand the failure with criterion and this endpoint. - if is_criterion_on() and 'www.netflix.com' in endpoint: - pytest.skip() - # expect_stderr=True because S2N sometimes receives OCSP responses: # https://github.com/aws/s2n-tls/blob/14ed186a13c1ffae7fbb036ed5d2849ce7c17403/bin/echo.c#L180-L184 client = managed_process(provider, client_options, diff --git a/tests/integrationv2/tox.ini b/tests/integrationv2/tox.ini index fd0705b5192..eda3ebfbece 100644 --- a/tests/integrationv2/tox.ini +++ b/tests/integrationv2/tox.ini @@ -25,6 +25,5 @@ commands = --durations=10 \ -o log_cli=true --log-cli-level=INFO \ --provider-version={env:S2N_LIBCRYPTO} \ - --provider-criterion={env:S2N_USE_CRITERION:"off"} \ --fips-mode={env:S2N_TEST_IN_FIPS_MODE:"0"} \ {env:TOX_TEST_NAME:""} From bcbf0d99c9633dd3702baeb40bb06c6fee7eb5e0 Mon Sep 17 00:00:00 2001 From: toidiu Date: Mon, 11 Nov 2024 15:17:01 -0800 Subject: [PATCH 07/18] feat: add new security policy `20241106` (#4874) --- tls/s2n_security_policies.c | 10 ++++++++++ tls/s2n_security_policies.h | 1 + 2 files changed, 11 insertions(+) diff --git a/tls/s2n_security_policies.c b/tls/s2n_security_policies.c index 3a331b9c6dc..54bfc431762 100644 --- a/tls/s2n_security_policies.c +++ b/tls/s2n_security_policies.c @@ -351,6 +351,15 @@ const struct s2n_security_policy security_policy_cloudfront_tls_1_0_2016 = { .ecc_preferences = &s2n_ecc_preferences_20200310, }; +/* Same as security_policy_cloudfront_tls_1_0_2016, but with TLS 1.2 as minimum */ +const struct s2n_security_policy security_policy_20241106 = { + .minimum_protocol_version = S2N_TLS12, + .cipher_preferences = &cipher_preferences_cloudfront_tls_1_0_2016, + .kem_preferences = &kem_preferences_null, + .signature_preferences = &s2n_signature_preferences_20200207, + .ecc_preferences = &s2n_ecc_preferences_20200310, +}; + const struct s2n_security_policy security_policy_cloudfront_tls_1_1_2016 = { .minimum_protocol_version = S2N_TLS11, .cipher_preferences = &cipher_preferences_cloudfront_tls_1_1_2016, @@ -1222,6 +1231,7 @@ struct s2n_security_policy_selection security_policy_selection[] = { { .version = "default_tls13", .security_policy = &security_policy_20240503, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "default_fips", .security_policy = &security_policy_20240502, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "default_pq", .security_policy = &security_policy_20241001, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, + { .version = "20241106", .security_policy = &security_policy_20241106, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "20240501", .security_policy = &security_policy_20240501, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "20240502", .security_policy = &security_policy_20240502, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, { .version = "20240503", .security_policy = &security_policy_20240503, .ecc_extension_required = 0, .pq_kem_extension_required = 0 }, diff --git a/tls/s2n_security_policies.h b/tls/s2n_security_policies.h index e27f7140736..8387831449e 100644 --- a/tls/s2n_security_policies.h +++ b/tls/s2n_security_policies.h @@ -99,6 +99,7 @@ extern const struct s2n_security_policy security_policy_20240501; extern const struct s2n_security_policy security_policy_20240502; extern const struct s2n_security_policy security_policy_20240503; +extern const struct s2n_security_policy security_policy_20241106; extern const struct s2n_security_policy security_policy_20140601; extern const struct s2n_security_policy security_policy_20141001; extern const struct s2n_security_policy security_policy_20150202; From 8dd481501af1ec3c631a8d646e31d7d2403593f5 Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Mon, 11 Nov 2024 17:02:30 -0800 Subject: [PATCH 08/18] chore: update github PR template (#4885) --- .github/PULL_REQUEST_TEMPLATE.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 23d3a172f86..4204ebf88ab 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,6 +1,9 @@ +### Release Summary: + + ### Resolved issues: -Resolves #ISSUE-NUMBER1, resolves #ISSUE-NUMBER2, etc. +resolves #ISSUE-NUMBER1, resolves #ISSUE-NUMBER2, etc. ### Description of changes: @@ -8,12 +11,20 @@ Describe s2n’s current behavior and how your code changes that behavior. If th ### Call-outs: -Address any potentially confusing code. Is there code added that needs to be cleaned up later? Is there code that is missing because it’s still in development? +Address any potentially confusing code. Is there code added that needs to be cleaned up later? Is there code that is missing because it’s still in development? If a callout is specific to a section of code, it might make more sense to leave a comment on your own PR file diff. ### Testing: -How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer? - +How is this change tested (unit tests, fuzz tests, etc.)? What manual testing was performed? Are there any testing steps to be verified by the reviewer? +How can you convince your reviewers that this PR is safe and effective? Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed? +Remember: +* Any change to the library source code should at least include unit tests. +* Any change to the core stuffer or blob methods should include [CBMC proofs](https://github.com/aws/s2n-tls/tree/main/tests/cbmc). +* Any change to the CI or tests should: + 1. prove that the test succeeds for good input + 2. prove that the test fails for bad input (eg, a test for memory leaks fails when a memory leak is committed) + + By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. From db1589d3a139247bebdd9026d1e0d43b0053feb3 Mon Sep 17 00:00:00 2001 From: Boquan Fang Date: Mon, 11 Nov 2024 20:35:15 -0800 Subject: [PATCH 09/18] fix: fix open AF_INET sockets in s2n_self_talk_ktls_test.c (#4852) --- tests/unit/s2n_self_talk_ktls_test.c | 53 ++++++++++++++-------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/tests/unit/s2n_self_talk_ktls_test.c b/tests/unit/s2n_self_talk_ktls_test.c index 6bcf686f68b..454ecb77284 100644 --- a/tests/unit/s2n_self_talk_ktls_test.c +++ b/tests/unit/s2n_self_talk_ktls_test.c @@ -82,16 +82,10 @@ static S2N_RESULT s2n_new_inet_socket_pair(struct s2n_test_io_pair *io_pair) io_pair->client = socket(AF_INET, SOCK_STREAM, 0); RESULT_ENSURE_GT(io_pair->client, 0); - fflush(stdout); - pid_t pid = fork(); - RESULT_ENSURE_GTE(pid, 0); - if (pid == 0) { - RESULT_ENSURE_EQ(connect(io_pair->client, (struct sockaddr *) &saddr, addrlen), 0); - ZERO_TO_DISABLE_DEFER_CLEANUP(io_pair); - exit(0); - } + RESULT_ENSURE_EQ(connect(io_pair->client, (struct sockaddr *) &saddr, addrlen), 0); io_pair->server = accept(listener, NULL, NULL); RESULT_ENSURE_GT(io_pair->server, 0); + RESULT_ENSURE_EQ(close(listener), 0); return S2N_RESULT_OK; } @@ -134,12 +128,6 @@ int main(int argc, char **argv) sizeof(test_data), }; - uint8_t file_test_data[100] = { 0 }; - int file = open(argv[0], O_RDONLY); - EXPECT_TRUE(file > 0); - int file_read = pread(file, file_test_data, sizeof(file_test_data), 0); - EXPECT_EQUAL(file_read, sizeof(file_test_data)); - DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free); EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain_and_key)); EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(config)); @@ -171,6 +159,7 @@ int main(int argc, char **argv) * to be able to test ktls. */ EXPECT_FALSE(ktls_expected); + EXPECT_SUCCESS(s2n_io_pair_close(&io_pair)); END_TEST(); } EXPECT_OK(s2n_setup_connections(server, client, &io_pair)); @@ -270,22 +259,32 @@ int main(int argc, char **argv) }; /* Test: s2n_sendfile */ - for (size_t offset_i = 0; offset_i < s2n_array_len(test_offsets); offset_i++) { - const size_t offset = test_offsets[offset_i]; - const size_t expected_written = sizeof(test_data) - offset; + { + uint8_t file_test_data[100] = { 0 }; + int file = open(argv[0], O_RDONLY); + EXPECT_TRUE(file > 0); + int file_read = pread(file, file_test_data, sizeof(file_test_data), 0); + EXPECT_EQUAL(file_read, sizeof(file_test_data)); + + for (size_t offset_i = 0; offset_i < s2n_array_len(test_offsets); offset_i++) { + const size_t offset = test_offsets[offset_i]; + const size_t expected_written = sizeof(test_data) - offset; + + size_t written = 0; + EXPECT_SUCCESS(s2n_sendfile(writer, file, offset, expected_written, + &written, &blocked)); + EXPECT_EQUAL(written, expected_written); + EXPECT_EQUAL(blocked, S2N_NOT_BLOCKED); - size_t written = 0; - EXPECT_SUCCESS(s2n_sendfile(writer, file, offset, expected_written, - &written, &blocked)); - EXPECT_EQUAL(written, expected_written); - EXPECT_EQUAL(blocked, S2N_NOT_BLOCKED); + uint8_t buffer[sizeof(file_test_data)] = { 0 }; + int read = s2n_recv(reader, buffer, expected_written, &blocked); + EXPECT_EQUAL(read, expected_written); + EXPECT_EQUAL(blocked, S2N_NOT_BLOCKED); - uint8_t buffer[sizeof(file_test_data)] = { 0 }; - int read = s2n_recv(reader, buffer, expected_written, &blocked); - EXPECT_EQUAL(read, expected_written); - EXPECT_EQUAL(blocked, S2N_NOT_BLOCKED); + EXPECT_BYTEARRAY_EQUAL(file_test_data + offset, buffer, read); + } - EXPECT_BYTEARRAY_EQUAL(file_test_data + offset, buffer, read); + EXPECT_SUCCESS(close(file)); } /* Test: s2n_shutdown */ From 38b273f36eff3719a7996d8204be7c5c7635d296 Mon Sep 17 00:00:00 2001 From: Doug Chapman <54039637+dougch@users.noreply.github.com> Date: Tue, 12 Nov 2024 08:31:28 -0800 Subject: [PATCH 10/18] chore: configure dependabot (#4861) --- .github/dependabot.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000000..9c9ee516393 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,22 @@ +# This configuration file tells Dependabot which +# package ecosystems to update and where the package manifests are located. +# https://docs.github.com/en/enterprise-cloud@latest/code-security/dependabot/dependabot-version-updates/configuring-dependabot-version-updates + +version: 2 +updates: + # Maintain dependencies for GitHub Actions + # https://github.com/dependabot/dependabot-core/pull/6189 + - package-ecosystem: "github-actions" + directory: "/.github/workflows" + schedule: + interval: "daily" + + # Maintain dependencies for cargo + - package-ecosystem: "cargo" + directories: + - "/bindings/rust" + - "/bindings/rust-examples" + - "/tests/pcap" + - "/tests/regression" + schedule: + interval: "daily" From 0f9654871e0f09a0016cf990d425e7464028435a Mon Sep 17 00:00:00 2001 From: Doug Chapman <54039637+dougch@users.noreply.github.com> Date: Tue, 12 Nov 2024 10:37:13 -0800 Subject: [PATCH 11/18] chore: broaden use of flaky mark (#4865) --- tests/integrationv2/test_happy_path.py | 3 ++- tests/integrationv2/test_renegotiate.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/integrationv2/test_happy_path.py b/tests/integrationv2/test_happy_path.py index 540aac871a2..f306b3ec913 100644 --- a/tests/integrationv2/test_happy_path.py +++ b/tests/integrationv2/test_happy_path.py @@ -11,6 +11,7 @@ from utils import invalid_test_parameters, get_parameter_name, get_expected_s2n_version, to_bytes +@pytest.mark.flaky(reruns=5, reruns_delay=2) @pytest.mark.uncollect_if(func=invalid_test_parameters) @pytest.mark.parametrize("cipher", ALL_TEST_CIPHERS, ids=get_parameter_name) @pytest.mark.parametrize("provider", [S2N, OpenSSL, GnuTLS, JavaSSL, SSLv3Provider]) @@ -69,7 +70,7 @@ def test_s2n_server_happy_path(managed_process, cipher, provider, curve, protoco cipher.name)) in server_results.stdout -@pytest.mark.flaky(reruns=5, reruns_delay=2, condition=platform.machine().startswith("aarch")) +@pytest.mark.flaky(reruns=5, reruns_delay=2) @pytest.mark.uncollect_if(func=invalid_test_parameters) @pytest.mark.parametrize("cipher", ALL_TEST_CIPHERS, ids=get_parameter_name) @pytest.mark.parametrize("provider", [S2N, OpenSSL, GnuTLS, SSLv3Provider]) diff --git a/tests/integrationv2/test_renegotiate.py b/tests/integrationv2/test_renegotiate.py index 84c12aa7cb1..8d53645c0d2 100644 --- a/tests/integrationv2/test_renegotiate.py +++ b/tests/integrationv2/test_renegotiate.py @@ -295,7 +295,7 @@ def test_s2n_client_renegotiate_with_openssl(managed_process, cipher, curve, cer """ -@pytest.mark.flaky(reruns=3, reruns_delay=1, condition=platform.machine().startswith("aarch")) +@pytest.mark.flaky(reruns=3, reruns_delay=1) @pytest.mark.uncollect_if(func=invalid_test_parameters) @pytest.mark.parametrize("cipher", ALL_TEST_CIPHERS, ids=get_parameter_name) @pytest.mark.parametrize("curve", ALL_TEST_CURVES, ids=get_parameter_name) @@ -345,7 +345,7 @@ def test_s2n_client_renegotiate_with_client_auth_with_openssl(managed_process, c """ -@pytest.mark.flaky(reruns=3, reruns_delay=1, condition=platform.machine().startswith("aarch")) +@pytest.mark.flaky(reruns=3, reruns_delay=1) @pytest.mark.uncollect_if(func=invalid_test_parameters) @pytest.mark.parametrize("cipher", ALL_TEST_CIPHERS, ids=get_parameter_name) @pytest.mark.parametrize("curve", ALL_TEST_CURVES, ids=get_parameter_name) From 493b77167dc367c394de23cfe78a029298e2a254 Mon Sep 17 00:00:00 2001 From: maddeleine <59030281+maddeleine@users.noreply.github.com> Date: Tue, 12 Nov 2024 12:32:04 -0800 Subject: [PATCH 12/18] feat: Reworking cleanup behavior (#4871) --- api/s2n.h | 10 ++---- codebuild/bin/s2n_dynamic_load_test.c | 15 +++++--- codebuild/bin/test_exec_leak.sh | 2 +- .../usage-guide/topics/ch02-initialization.md | 13 +++++-- tests/s2n_test.h | 27 +++++++------- tests/unit/s2n_init_test.c | 36 ++++++++----------- tests/unit/s2n_random_test.c | 7 ++-- utils/s2n_init.c | 10 ++---- utils/s2n_init.h | 1 + 9 files changed, 56 insertions(+), 65 deletions(-) diff --git a/api/s2n.h b/api/s2n.h index f104a5ee2af..e7e2a634716 100644 --- a/api/s2n.h +++ b/api/s2n.h @@ -229,8 +229,8 @@ S2N_API extern unsigned long s2n_get_openssl_version(void); S2N_API extern int s2n_init(void); /** - * Cleans up any internal resources used by s2n-tls. This function should be called from each thread or process - * that is created subsequent to calling `s2n_init` when that thread or process is done calling other s2n-tls functions. + * Cleans up thread-local resources used by s2n-tls. Does not perform a full library cleanup. To fully + * clean up the library use s2n_cleanup_final(). * * @returns S2N_SUCCESS on success. S2N_FAILURE on failure */ @@ -239,12 +239,6 @@ S2N_API extern int s2n_cleanup(void); /* * Performs a complete deinitialization and cleanup of the s2n-tls library. * - * s2n_cleanup_final will always perform a complete cleanup. In contrast, - * s2n_cleanup will only perform a complete cleanup if the atexit handler - * is disabled and s2n_cleanup is called by the thread that called s2n_init. - * Therefore s2n_cleanup_final should be used instead of s2n_cleanup in cases - * where the user needs full control over when the complete cleanup executes. - * * @returns S2N_SUCCESS on success. S2N_FAILURE on failure */ S2N_API extern int s2n_cleanup_final(void); diff --git a/codebuild/bin/s2n_dynamic_load_test.c b/codebuild/bin/s2n_dynamic_load_test.c index bab76c8aa16..d8bc929982a 100644 --- a/codebuild/bin/s2n_dynamic_load_test.c +++ b/codebuild/bin/s2n_dynamic_load_test.c @@ -37,10 +37,10 @@ static void *s2n_load_dynamic_lib(void *ctx) exit(1); } - int (*s2n_cleanup_dl)(void) = NULL; - *(void **) (&s2n_cleanup_dl) = dlsym(s2n_so, "s2n_cleanup"); + int (*s2n_cleanup_final_dl)(void) = NULL; + *(void **) (&s2n_cleanup_final_dl) = dlsym(s2n_so, "s2n_cleanup_final"); if (dlerror()) { - printf("Error dynamically loading s2n_cleanup\n"); + printf("Error dynamically loading s2n_cleanup_final\n"); exit(1); } @@ -63,17 +63,22 @@ static void *s2n_load_dynamic_lib(void *ctx) fprintf(stderr, "Error calling s2n_init: '%s'\n", (*s2n_strerror_debug_dl)(s2n_errno, "EN")); exit(1); } - if ((*s2n_cleanup_dl)()) { + if ((*s2n_cleanup_final_dl)()) { int s2n_errno = (*s2n_errno_location_dl)(); - fprintf(stderr, "Error calling s2n_cleanup: '%s'\n", (*s2n_strerror_debug_dl)(s2n_errno, "EN")); + fprintf(stderr, "Error calling s2n_cleanup_final: '%s'\n", (*s2n_strerror_debug_dl)(s2n_errno, "EN")); exit(1); } + /* TODO: https://github.com/aws/s2n-tls/issues/4827 + * This dlclose call invokes the pthread key destructor that + * asserts that the s2n-tls library is initialized, which at this point + * is not, due to the s2n_cleanup_final call. This is a bug. if (dlclose(s2n_so)) { printf("Error closing libs2n\n"); printf("%s\n", dlerror()); exit(1); } + */ return NULL; } diff --git a/codebuild/bin/test_exec_leak.sh b/codebuild/bin/test_exec_leak.sh index 6dc50416438..367652546a8 100755 --- a/codebuild/bin/test_exec_leak.sh +++ b/codebuild/bin/test_exec_leak.sh @@ -51,7 +51,7 @@ cat < build/detect_exec_leak_finish.c int main() { s2n_init(); - s2n_cleanup(); + s2n_cleanup_final(); /* close std* file descriptors so valgrind output is less noisy */ fclose(stdin); diff --git a/docs/usage-guide/topics/ch02-initialization.md b/docs/usage-guide/topics/ch02-initialization.md index 467c0f7567c..f4f60e6247b 100644 --- a/docs/usage-guide/topics/ch02-initialization.md +++ b/docs/usage-guide/topics/ch02-initialization.md @@ -1,10 +1,17 @@ # Initialization and Teardown -The s2n-tls library must be initialized with `s2n_init()` before calling most library functions. `s2n_init()` MUST NOT be called more than once, even when an application uses multiple threads or processes. s2n attempts to clean up its thread-local memory at thread-exit and all other memory at process-exit. However, this may not work if you are using a thread library other than pthreads or other threads using s2n outlive the thread that called `s2n_init()`. In that case you should call `s2n_cleanup_thread()` from every thread or process created after `s2n_init()`. -> Note: `s2n_cleanup_thread()` is currently considered unstable, meaning the API is subject to change in a future release. To access this API, include `api/unstable/cleanup.h`. +## Initialization +The s2n-tls library must be initialized with `s2n_init()` before calling most library functions. `s2n_init()` will error if it is called more than once, even when an application uses multiple threads. Initialization can be modified by calling `s2n_crypto_disable_init()` or `s2n_disable_atexit()` before `s2n_init()`. -An application can override s2n-tls’s internal memory management by calling `s2n_mem_set_callbacks` before calling s2n_init. +An application can override s2n-tls’s internal memory management by calling `s2n_mem_set_callbacks()` before calling `s2n_init()`. If you are trying to use FIPS mode, you must enable FIPS in your libcrypto library (probably by calling `FIPS_mode_set(1)`) before calling `s2n_init()`. + +## Teardown +### Thread-local Memory +We recommend calling `s2n_cleanup()` from every thread created after `s2n_init()` to ensure there are no memory leaks. s2n-tls has thread-local memory that it attempts to clean up automatically at thread-exit. However, this is done using pthread destructors and may not work if you are using a threads library other than pthreads. + +### Library Cleanup +A full cleanup and de-initialization of the library can be done by calling `s2n_cleanup_final()`. s2n-tls allocates some memory at initialization that is intended to live for the duration of the process, but can be cleaned up earlier with `s2n_cleanup_final()`. Not calling this method may cause tools like ASAN or valgrind to detect memory leaks, as the memory will still be allocated when the process exits. diff --git a/tests/s2n_test.h b/tests/s2n_test.h index 3ad2c168783..e03b21ff7ba 100644 --- a/tests/s2n_test.h +++ b/tests/s2n_test.h @@ -15,18 +15,18 @@ #pragma once #include +#include #include #include #include #include -#include - #include "error/s2n_errno.h" -#include "utils/s2n_safety.h" -#include "utils/s2n_result.h" #include "tls/s2n_alerts.h" #include "tls/s2n_tls13.h" +#include "utils/s2n_init.h" +#include "utils/s2n_result.h" +#include "utils/s2n_safety.h" int test_count; @@ -64,14 +64,15 @@ bool s2n_use_color_in_output = true; * number of independent childs at the start of a unit test and where you want * each child to have its own independently initialised s2n. */ -#define BEGIN_TEST_NO_INIT() \ - do { \ - test_count = 0; \ - fprintf(stdout, "Running %-50s ... ", __FILE__); \ - fflush(stdout); \ - EXPECT_SUCCESS_WITHOUT_COUNT(s2n_in_unit_test_set(true)); \ - S2N_TEST_OPTIONALLY_ENABLE_FIPS_MODE(); \ - } while(0) +#define BEGIN_TEST_NO_INIT() \ + do { \ + test_count = 0; \ + fprintf(stdout, "Running %-50s ... ", __FILE__); \ + fflush(stdout); \ + EXPECT_SUCCESS_WITHOUT_COUNT(s2n_in_unit_test_set(true)); \ + S2N_TEST_OPTIONALLY_ENABLE_FIPS_MODE(); \ + EXPECT_SUCCESS(s2n_enable_atexit()); \ + } while (0) #define END_TEST_NO_INIT() \ do { \ @@ -261,7 +262,7 @@ void s2n_test__fuzz_cleanup() \ if (fuzz_cleanup) { \ ((void (*)()) fuzz_cleanup)(); \ } \ - s2n_cleanup(); \ + s2n_cleanup_final(); \ } \ int LLVMFuzzerInitialize(int *argc, char **argv[]) \ { \ diff --git a/tests/unit/s2n_init_test.c b/tests/unit/s2n_init_test.c index 091427c86bc..27a7b9f8095 100644 --- a/tests/unit/s2n_init_test.c +++ b/tests/unit/s2n_init_test.c @@ -19,7 +19,6 @@ #include "s2n_test.h" bool s2n_is_initialized(void); -int s2n_enable_atexit(void); static void *s2n_init_fail_cb(void *_unused_arg) { @@ -49,24 +48,18 @@ int main(int argc, char **argv) { BEGIN_TEST_NO_INIT(); - /* Disabling the atexit handler makes it easier for us to test s2n_init and s2n_cleanup - * behavior. Otherwise we'd have to create and exit a bunch of processes to test this - * interaction. */ - EXPECT_SUCCESS(s2n_disable_atexit()); - /* Calling s2n_init twice in a row will cause an error */ EXPECT_SUCCESS(s2n_init()); EXPECT_FAILURE_WITH_ERRNO(s2n_init(), S2N_ERR_INITIALIZED); - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); - /* Second call to s2n_cleanup will fail, since the full cleanup is not idempotent. - * This behavior only exists when atexit is disabled. */ - EXPECT_FAILURE_WITH_ERRNO(s2n_cleanup(), S2N_ERR_NOT_INITIALIZED); + /* Second call to s2n_cleanup_final will fail, since the full cleanup is not idempotent. */ + EXPECT_FAILURE_WITH_ERRNO(s2n_cleanup_final(), S2N_ERR_NOT_INITIALIZED); /* Clean up and init multiple times */ for (size_t i = 0; i < 10; i++) { EXPECT_SUCCESS(s2n_init()); - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); } /* Calling s2n_init again after creating a process will cause an error */ @@ -78,16 +71,16 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_cleanup()); return 0; } - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); /* Calling s2n_init again after creating a thread will cause an error */ EXPECT_SUCCESS(s2n_init()); pthread_t init_thread = { 0 }; EXPECT_EQUAL(pthread_create(&init_thread, NULL, s2n_init_fail_cb, NULL), 0); EXPECT_EQUAL(pthread_join(init_thread, NULL), 0); - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); - /* Calling s2n_init/s2n_cleanup in a different thread than s2n_cleanup_thread is called cleans up properly */ + /* Calling s2n_init/s2n_cleanup_final in a different thread than s2n_cleanup_thread is called cleans up properly */ { EXPECT_SUCCESS(s2n_init()); EXPECT_TRUE(s2n_is_initialized()); @@ -98,10 +91,10 @@ int main(int argc, char **argv) /* Calling s2n_cleanup_thread in the main thread leaves s2n initialized. */ EXPECT_SUCCESS(s2n_cleanup_thread()); EXPECT_TRUE(s2n_is_initialized()); - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); EXPECT_FALSE(s2n_is_initialized()); - /* Second call to s2n_cleanup will fail, since the full cleanup is not idempotent. */ - EXPECT_FAILURE_WITH_ERRNO(s2n_cleanup(), S2N_ERR_NOT_INITIALIZED); + /* Second call to s2n_cleanup_final will fail, since the full cleanup is not idempotent. */ + EXPECT_FAILURE_WITH_ERRNO(s2n_cleanup_final(), S2N_ERR_NOT_INITIALIZED); } /* s2n_cleanup_final */ @@ -112,11 +105,11 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_cleanup_final()); EXPECT_FALSE(s2n_is_initialized()); - /* s2n_cleanup fully cleans up the library when the atexit handler is disabled. - * Therefore, calling s2n_cleanup_final after s2n_cleanup will error */ + /* s2n_cleanup only cleans up thread-local storage. + * Therefore, calling s2n_cleanup_final after s2n_cleanup will succeed */ EXPECT_SUCCESS(s2n_init()); EXPECT_SUCCESS(s2n_cleanup()); - EXPECT_FAILURE_WITH_ERRNO(s2n_cleanup_final(), S2N_ERR_NOT_INITIALIZED); + EXPECT_SUCCESS(s2n_cleanup_final()); /* s2n_cleanup_thread only cleans up thread-local storage. * Therefore calling s2n_cleanup_final after s2n_cleanup_thread will succeed */ @@ -130,8 +123,7 @@ int main(int argc, char **argv) /* Initializing s2n on a child thread without calling s2n_cleanup on that * thread will not result in a memory leak. This is because we register - * thread-local memory to be cleaned up at thread-exit - * and then our atexit handler cleans up the rest at proccess-exit. */ + * thread-local memory to be cleaned up at thread-exit. */ pthread_t init_success_thread = { 0 }; EXPECT_EQUAL(pthread_create(&init_success_thread, NULL, s2n_init_success_cb, NULL), 0); EXPECT_EQUAL(pthread_join(init_success_thread, NULL), 0); diff --git a/tests/unit/s2n_random_test.c b/tests/unit/s2n_random_test.c index 5c031c87be6..802693d6f81 100644 --- a/tests/unit/s2n_random_test.c +++ b/tests/unit/s2n_random_test.c @@ -782,9 +782,8 @@ static int s2n_random_noop_destructor_test_cb(struct random_test_case *test_case static int s2n_random_rand_bytes_after_cleanup_cb(struct random_test_case *test_case) { - s2n_disable_atexit(); EXPECT_SUCCESS(s2n_init()); - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); unsigned char rndbytes[16]; EXPECT_EQUAL(RAND_bytes(rndbytes, sizeof(rndbytes)), 1); @@ -818,8 +817,6 @@ static int s2n_random_rand_bytes_before_init(struct random_test_case *test_case) static int s2n_random_invalid_urandom_fd_cb(struct random_test_case *test_case) { - EXPECT_SUCCESS(s2n_disable_atexit()); - struct s2n_rand_device *dev_urandom = NULL; EXPECT_OK(s2n_rand_get_urandom_for_test(&dev_urandom)); EXPECT_NOT_NULL(dev_urandom); @@ -871,7 +868,7 @@ static int s2n_random_invalid_urandom_fd_cb(struct random_test_case *test_case) EXPECT_TRUE(public_bytes_used > 0); } - EXPECT_SUCCESS(s2n_cleanup()); + EXPECT_SUCCESS(s2n_cleanup_final()); } return S2N_SUCCESS; diff --git a/utils/s2n_init.c b/utils/s2n_init.c index 82608d78e4e..1a94fa2e684 100644 --- a/utils/s2n_init.c +++ b/utils/s2n_init.c @@ -37,7 +37,7 @@ static void s2n_cleanup_atexit(void); static pthread_t main_thread = 0; static bool initialized = false; -static bool atexit_cleanup = true; +static bool atexit_cleanup = false; int s2n_disable_atexit(void) { POSIX_ENSURE(!initialized, S2N_ERR_INITIALIZED); @@ -139,13 +139,7 @@ int s2n_cleanup(void) { POSIX_GUARD(s2n_cleanup_thread()); - /* If this is the main thread and atexit cleanup is disabled, - * perform final cleanup now */ - if (pthread_equal(pthread_self(), main_thread) && !atexit_cleanup) { - POSIX_GUARD(s2n_cleanup_final()); - } - - return 0; + return S2N_SUCCESS; } static void s2n_cleanup_atexit(void) diff --git a/utils/s2n_init.h b/utils/s2n_init.h index 7936b85a76e..6a9deed372c 100644 --- a/utils/s2n_init.h +++ b/utils/s2n_init.h @@ -20,3 +20,4 @@ int s2n_init(void); int s2n_cleanup(void); bool s2n_is_initialized(void); +int s2n_enable_atexit(void); From 0807885e704a1dc58b12e639a2a341796fa545bf Mon Sep 17 00:00:00 2001 From: Boquan Fang Date: Tue, 12 Nov 2024 14:59:46 -0800 Subject: [PATCH 13/18] ci: add open fds valgrind check (#4851) --- CMakeLists.txt | 1 + codebuild/bin/s2n_open_fds_test.py | 65 +++++++++++++++++++++++++++ codebuild/spec/buildspec_valgrind.yml | 4 +- 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 codebuild/bin/s2n_open_fds_test.py diff --git a/CMakeLists.txt b/CMakeLists.txt index d16e5f5f053..8dfaa4e2d80 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -511,6 +511,7 @@ if (BUILD_TESTING) --error-limit=no \ --num-callers=40 \ --undef-value-errors=no \ + --track-fds=yes \ --log-fd=2 \ --suppressions=valgrind.suppressions") diff --git a/codebuild/bin/s2n_open_fds_test.py b/codebuild/bin/s2n_open_fds_test.py new file mode 100644 index 00000000000..06cf5bffee9 --- /dev/null +++ b/codebuild/bin/s2n_open_fds_test.py @@ -0,0 +1,65 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +# This script parses the LastDynamicAnalysis file generated by Valgrind running through CTest memcheck. +# It identifies any leaking file descriptors and triggers an error when detected. +# This enhances the capabilities of existing Valgrind checks. +# Output snippet for open file descriptors: +# ==6652== FILE DESCRIPTORS: 6 open (3 std) at exit. +# ==6652== Open AF_INET socket 6: 127.0.0.1:36915 <-> unbound +# ==6652== at 0x498B2EB: socket (syscall-template.S:120) +# ==6652== by 0x16CD16: s2n_new_inet_socket_pair (s2n_self_talk_ktls_test.c:69) +# ==6652== by 0x15DBB2: main (s2n_self_talk_ktls_test.c:168) +# ==6652== +import os +import sys + +EXIT_SUCCESS = 0 +# Exit with error code 1 if leaking fds are detected. +ERROR_EXIT_CODE = 1 +# This test is designed to be informational only, so we only print fifteen lines of error messages when a leak is detected. +NUM_OF_LINES_TO_PRINT = 15 + + +def find_log_file(path): + for f in os.listdir(path): + if "LastDynamicAnalysis" in f: + return os.path.join(path, f) + + raise FileNotFoundError("LastDynamicAnalysis log file is not found!") + + +def detect_leak(file): + fd_leak_detected = False + lines = file.readlines() + for i in range(len(lines)): + if "FILE DESCRIPTORS:" in lines[i]: + # Example line: `==6096== FILE DESCRIPTORS: 4 open (3 std) at exit.` + line_elements = lines[i].split() + open_fd_count = line_elements[line_elements.index("DESCRIPTORS:") + 1] + std_fd_count = line_elements[line_elements.index("std)") - 1][1:] + # CTest memcheck writes to a LastDynamicAnslysis log file. + # We allow that fd to remain opened. + if int(open_fd_count) > int(std_fd_count) + 1: + for j in range(NUM_OF_LINES_TO_PRINT): + print(lines[i + j], end="") + print() + fd_leak_detected = True + return fd_leak_detected + + +def main(): + # Print banner of the test + print("############################################################################") + print("################# Test for Leaking File Descriptors ########################") + print("############################################################################") + + with open(find_log_file(sys.argv[1]), 'r') as file: + if detect_leak(file): + sys.exit(ERROR_EXIT_CODE) + + return EXIT_SUCCESS + + +if __name__ == '__main__': + main() diff --git a/codebuild/spec/buildspec_valgrind.yml b/codebuild/spec/buildspec_valgrind.yml index 1ca359d3f42..c26570d8380 100644 --- a/codebuild/spec/buildspec_valgrind.yml +++ b/codebuild/spec/buildspec_valgrind.yml @@ -32,7 +32,7 @@ batch: - identifier: gcc_openssl_1_1_1 env: compute-type: BUILD_GENERAL1_LARGE - image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu18codebuild + image: 024603541914.dkr.ecr.us-west-2.amazonaws.com/docker:ubuntu24 variables: S2N_LIBCRYPTO: openssl-1.1.1 COMPILER: gcc @@ -70,3 +70,5 @@ phases: CTEST_OUTPUT_ON_FAILURE=1 \ cmake --build build/ --target test \ -- ARGS="--test-action memcheck" + - cd codebuild/bin + - python3 s2n_open_fds_test.py $CODEBUILD_SRC_DIR/build/Testing/Temporary From b4c8e6c01af2af59fb39ba3e8b0b95885d87ad41 Mon Sep 17 00:00:00 2001 From: Doug Chapman <54039637+dougch@users.noreply.github.com> Date: Tue, 12 Nov 2024 17:11:46 -0800 Subject: [PATCH 14/18] chore: add a cargo audit action (#4862) --- .github/workflows/dependencies.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/dependencies.yml diff --git a/.github/workflows/dependencies.yml b/.github/workflows/dependencies.yml new file mode 100644 index 00000000000..eaff63bef72 --- /dev/null +++ b/.github/workflows/dependencies.yml @@ -0,0 +1,29 @@ +name: dependencies + +on: + # Because of permissions issues with forked PRs, + # Only run on a schedule or pushes to main. + push: + branches: + - main + # Only run if these files were touched. + paths: + - "**/Cargo.toml" + - "**/Cargo.lock" + - ".github/workflows/dependencies.yml" + + schedule: + # Run every day at 1800 UTC. + - cron: "0 18 * * *" + +jobs: + audit: + runs-on: ubuntu-latest + permissions: + issues: write # Open/update issues. + checks: write # Create/update a check run. + steps: + - uses: actions/checkout@v4 + - uses: rustsec/audit-check@v2.0.0 + with: + token: ${{ secrets.GITHUB_TOKEN }} From 2ba3dda0eadb334cc7de8c3da8a44859e52bbb0f Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Wed, 13 Nov 2024 13:59:32 -0800 Subject: [PATCH 15/18] chore: bindings release 0.3.7 (#4894) --- bindings/rust/s2n-tls-hyper/Cargo.toml | 4 ++-- bindings/rust/s2n-tls-sys/templates/Cargo.template | 2 +- bindings/rust/s2n-tls-tokio/Cargo.toml | 4 ++-- bindings/rust/s2n-tls/Cargo.toml | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bindings/rust/s2n-tls-hyper/Cargo.toml b/bindings/rust/s2n-tls-hyper/Cargo.toml index 8c474a9e739..a83970e12d3 100644 --- a/bindings/rust/s2n-tls-hyper/Cargo.toml +++ b/bindings/rust/s2n-tls-hyper/Cargo.toml @@ -13,8 +13,8 @@ publish = false default = [] [dependencies] -s2n-tls = { version = "=0.3.6", path = "../s2n-tls" } -s2n-tls-tokio = { version = "=0.3.6", path = "../s2n-tls-tokio" } +s2n-tls = { version = "=0.3.7", path = "../s2n-tls" } +s2n-tls-tokio = { version = "=0.3.7", path = "../s2n-tls-tokio" } hyper = { version = "1" } hyper-util = { version = "0.1", features = ["client-legacy", "tokio", "http1"] } tower-service = { version = "0.3" } diff --git a/bindings/rust/s2n-tls-sys/templates/Cargo.template b/bindings/rust/s2n-tls-sys/templates/Cargo.template index fb4a5bd960e..6c497c5633b 100644 --- a/bindings/rust/s2n-tls-sys/templates/Cargo.template +++ b/bindings/rust/s2n-tls-sys/templates/Cargo.template @@ -1,7 +1,7 @@ [package] name = "s2n-tls-sys" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.6" +version = "0.3.7" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" diff --git a/bindings/rust/s2n-tls-tokio/Cargo.toml b/bindings/rust/s2n-tls-tokio/Cargo.toml index 2a3b3073034..c4cb389943b 100644 --- a/bindings/rust/s2n-tls-tokio/Cargo.toml +++ b/bindings/rust/s2n-tls-tokio/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls-tokio" description = "An implementation of TLS streams for Tokio built on top of s2n-tls" -version = "0.3.6" +version = "0.3.7" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -15,7 +15,7 @@ default = [] errno = { version = "0.3" } libc = { version = "0.2" } pin-project-lite = { version = "0.2" } -s2n-tls = { version = "=0.3.6", path = "../s2n-tls" } +s2n-tls = { version = "=0.3.7", path = "../s2n-tls" } tokio = { version = "1", features = ["net", "time"] } [dev-dependencies] diff --git a/bindings/rust/s2n-tls/Cargo.toml b/bindings/rust/s2n-tls/Cargo.toml index ab895344473..498df276255 100644 --- a/bindings/rust/s2n-tls/Cargo.toml +++ b/bindings/rust/s2n-tls/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "s2n-tls" description = "A C99 implementation of the TLS/SSL protocols" -version = "0.3.6" +version = "0.3.7" authors = ["AWS s2n"] edition = "2021" rust-version = "1.63.0" @@ -21,7 +21,7 @@ unstable-testing = [] [dependencies] errno = { version = "0.3" } libc = "0.2" -s2n-tls-sys = { version = "=0.3.6", path = "../s2n-tls-sys", features = ["internal"] } +s2n-tls-sys = { version = "=0.3.7", path = "../s2n-tls-sys", features = ["internal"] } pin-project-lite = "0.2" hex = "0.4" From 06015f9843c8cec020bc7fae140476ac4d8039fd Mon Sep 17 00:00:00 2001 From: James Mayclin Date: Fri, 15 Nov 2024 18:57:13 -0800 Subject: [PATCH 16/18] test: add rust well-known-endpoint tests (#4884) --- .github/workflows/ci_rust.yml | 4 + bindings/rust/integration/Cargo.toml | 29 +++- bindings/rust/integration/src/lib.rs | 31 ++++ .../integration/src/network/https_client.rs | 97 +++++++++++++ bindings/rust/integration/src/network/mod.rs | 5 + .../integration/src/network/tls_client.rs | 95 ++++++++++++ .../rust/s2n-tls-hyper/tests/web_client.rs | 25 ---- bindings/rust/s2n-tls/src/connection.rs | 28 ++++ bindings/rust/s2n-tls/src/testing/s2n_tls.rs | 29 ++++ .../test_well_known_endpoints.py | 135 ++---------------- 10 files changed, 326 insertions(+), 152 deletions(-) create mode 100644 bindings/rust/integration/src/lib.rs create mode 100644 bindings/rust/integration/src/network/https_client.rs create mode 100644 bindings/rust/integration/src/network/mod.rs create mode 100644 bindings/rust/integration/src/network/tls_client.rs delete mode 100644 bindings/rust/s2n-tls-hyper/tests/web_client.rs diff --git a/.github/workflows/ci_rust.yml b/.github/workflows/ci_rust.yml index 97637855b09..2ced1e01d7e 100644 --- a/.github/workflows/ci_rust.yml +++ b/.github/workflows/ci_rust.yml @@ -58,6 +58,10 @@ jobs: working-directory: ${{env.ROOT_PATH}} run: cargo test --features unstable-renegotiate + - name: Network-enabled integration tests + working-directory: ${{env.ROOT_PATH}}/integration + run: RUST_LOG=TRACE cargo test --features network-tests + - name: Test external build # if this test is failing, make sure that api headers are appropriately # included. For a symbol to be visible in a shared lib, the diff --git a/bindings/rust/integration/Cargo.toml b/bindings/rust/integration/Cargo.toml index 684cc4714ac..8c1f75470a0 100644 --- a/bindings/rust/integration/Cargo.toml +++ b/bindings/rust/integration/Cargo.toml @@ -5,7 +5,34 @@ authors = ["AWS s2n"] edition = "2021" publish = false +[features] +default = ["pq"] + +# Network tests are useful but relatively slow and inherently flaky. So they are +# behind this feature flag. +network-tests = [] + +# Not all libcryptos support PQ capabilities. Tests relying on PQ functionality +# can be disabled by turning off this feature. +pq = [] + [dependencies] -s2n-tls = { path = "../s2n-tls"} +s2n-tls = { path = "../s2n-tls", features = ["unstable-testing"]} +s2n-tls-hyper = { path = "../s2n-tls-hyper" } +s2n-tls-tokio = { path = "../s2n-tls-tokio" } s2n-tls-sys = { path = "../s2n-tls-sys" } +[dev-dependencies] +tokio = { version = "1", features = ["macros", "test-util"] } + +tracing = "0.1" +tracing-subscriber = "0.3" +# TODO: Unpin when s2n-tls MSRV >= 1.71, https://github.com/aws/s2n-tls/issues/4893 +test-log = { version = "=0.2.14", default-features = false, features = ["trace"]} +test-log-macros = "=0.2.14" + +http = "1.1" +http-body-util = "0.1" +bytes = "1.8" +hyper = "1.5" +hyper-util = "0.1" diff --git a/bindings/rust/integration/src/lib.rs b/bindings/rust/integration/src/lib.rs new file mode 100644 index 00000000000..612f362e2dd --- /dev/null +++ b/bindings/rust/integration/src/lib.rs @@ -0,0 +1,31 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +#[cfg(all(feature = "network-tests", test))] +mod network; + +#[cfg(test)] +mod tests { + use s2n_tls::{ + security::Policy, + testing::{self, TestPair}, + }; + + /// This test provides a helpful debug message if the PQ feature is incorrectly + /// configured. + #[cfg(feature = "pq")] + #[test] + fn pq_sanity_check() -> Result<(), Box> { + let config = testing::build_config(&Policy::from_version("KMS-PQ-TLS-1-0-2020-07")?)?; + let mut pair = TestPair::from_config(&config); + pair.handshake()?; + + if pair.client.kem_name().is_none() { + panic!( + "PQ tests are enabled, but PQ functionality is unavailable. \ + Are you sure that the libcrypto supports PQ?" + ); + } + Ok(()) + } +} diff --git a/bindings/rust/integration/src/network/https_client.rs b/bindings/rust/integration/src/network/https_client.rs new file mode 100644 index 00000000000..84c90e5a10a --- /dev/null +++ b/bindings/rust/integration/src/network/https_client.rs @@ -0,0 +1,97 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use bytes::Bytes; +use http::{Response, StatusCode, Uri}; +use http_body_util::{BodyExt, Empty}; +use hyper::body::Incoming; +use hyper_util::{client::legacy::Client, rt::TokioExecutor}; +use s2n_tls::{ + config::Config, + security::{self, Policy}, +}; +use s2n_tls_hyper::connector::HttpsConnector; +use std::str::FromStr; + +#[derive(Debug)] +struct TestCase { + pub query_target: &'static str, + pub expected_status_code: u16, +} + +impl TestCase { + const fn new(domain: &'static str, expected_status_code: u16) -> Self { + TestCase { + query_target: domain, + expected_status_code, + } + } +} + +const TEST_CASES: &[TestCase] = &[ + // this is a link to the s2n-tls unit test coverage report, hosted on cloudfront + TestCase::new("https://dx1inn44oyl7n.cloudfront.net/main/index.html", 200), + // this is a link to a non-existent S3 item + TestCase::new("https://notmybucket.s3.amazonaws.com/folder/afile.jpg", 403), + TestCase::new("https://www.amazon.com", 200), + TestCase::new("https://www.apple.com", 200), + TestCase::new("https://www.att.com", 200), + TestCase::new("https://www.cloudflare.com", 200), + TestCase::new("https://www.ebay.com", 200), + TestCase::new("https://www.google.com", 200), + TestCase::new("https://www.mozilla.org", 200), + TestCase::new("https://www.netflix.com", 200), + TestCase::new("https://www.openssl.org", 200), + TestCase::new("https://www.t-mobile.com", 200), + TestCase::new("https://www.verizon.com", 200), + TestCase::new("https://www.wikipedia.org", 200), + TestCase::new("https://www.yahoo.com", 200), + TestCase::new("https://www.youtube.com", 200), + TestCase::new("https://www.github.com", 301), + TestCase::new("https://www.samsung.com", 301), + TestCase::new("https://www.twitter.com", 301), + TestCase::new("https://www.facebook.com", 302), + TestCase::new("https://www.microsoft.com", 302), + TestCase::new("https://www.ibm.com", 303), + TestCase::new("https://www.f5.com", 403), +]; + +/// perform an HTTP GET request against `uri` using an s2n-tls config with +/// `security_policy`. +async fn https_get( + uri: &str, + security_policy: &Policy, +) -> Result, hyper_util::client::legacy::Error> { + let mut config = Config::builder(); + config.set_security_policy(security_policy).unwrap(); + + let connector = HttpsConnector::new(config.build().unwrap()); + let client: Client<_, Empty> = Client::builder(TokioExecutor::new()).build(connector); + + let uri = Uri::from_str(uri).unwrap(); + client.get(uri).await +} + +/// Ensure that s2n-tls is compatible with other http/TLS implementations. +/// +/// This test uses s2n-tls-hyper to make http requests over a TLS connection to +/// a number of well known http sites. +#[test_log::test(tokio::test)] +async fn http_get_test() -> Result<(), Box> { + for test_case in TEST_CASES { + for policy in [security::DEFAULT, security::DEFAULT_TLS13] { + tracing::info!("executing test case {:#?} with {:?}", test_case, policy); + + let response = https_get(test_case.query_target, &policy).await?; + let expected_status = StatusCode::from_u16(test_case.expected_status_code).unwrap(); + assert_eq!(response.status(), expected_status); + + if expected_status == StatusCode::OK { + let body = response.into_body().collect().await?.to_bytes(); + assert!(!body.is_empty()); + } + } + } + + Ok(()) +} diff --git a/bindings/rust/integration/src/network/mod.rs b/bindings/rust/integration/src/network/mod.rs new file mode 100644 index 00000000000..07d0cabf7de --- /dev/null +++ b/bindings/rust/integration/src/network/mod.rs @@ -0,0 +1,5 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +mod https_client; +mod tls_client; diff --git a/bindings/rust/integration/src/network/tls_client.rs b/bindings/rust/integration/src/network/tls_client.rs new file mode 100644 index 00000000000..c94325a7692 --- /dev/null +++ b/bindings/rust/integration/src/network/tls_client.rs @@ -0,0 +1,95 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use s2n_tls::{config::Config, enums::Version, security::Policy}; +use s2n_tls_tokio::{TlsConnector, TlsStream}; +use tokio::net::TcpStream; + +/// Perform a TLS handshake with port 443 of `domain`. +/// +/// * `domain`: The domain to perform the handshake with +/// * `security_policy`: The security policy to set on the handshaking client. +/// +/// Returns an open `TlsStream` if the handshake was successful, otherwise an +/// `Err``. +async fn handshake_with_domain( + domain: &str, + security_policy: &str, +) -> Result, Box> { + tracing::info!("querying {domain} with {security_policy}"); + const PORT: u16 = 443; + + let mut config = Config::builder(); + config.set_security_policy(&Policy::from_version(security_policy)?)?; + + let client = TlsConnector::new(config.build()?); + // open the TCP stream + let stream = TcpStream::connect((domain, PORT)).await?; + // complete the TLS handshake + Ok(client.connect(domain, stream).await?) +} + +#[cfg(feature = "pq")] +mod kms_pq { + use super::*; + + const DOMAIN: &str = "kms.us-east-1.amazonaws.com"; + + // confirm that we successfully negotiate a supported PQ key exchange. + // + // Note: In the future KMS will deprecate kyber_r3 in favor of ML-KEM. + // At that point this test should be updated with a security policy that + // supports ML-KEM. + #[test_log::test(tokio::test)] + async fn pq_handshake() -> Result<(), Box> { + let tls = handshake_with_domain(DOMAIN, "KMS-PQ-TLS-1-0-2020-07").await?; + + assert_eq!( + tls.as_ref().cipher_suite()?, + "ECDHE-KYBER-RSA-AES256-GCM-SHA384" + ); + assert_eq!(tls.as_ref().kem_name(), Some("kyber512r3")); + + Ok(()) + } + + // We want to confirm that non-supported kyber drafts successfully fall + // back to a full handshake. + #[test_log::test(tokio::test)] + async fn early_draft_falls_back_to_classical() -> Result<(), Box> { + const EARLY_DRAFT_PQ_POLICIES: &[&str] = &[ + "KMS-PQ-TLS-1-0-2019-06", + "PQ-SIKE-TEST-TLS-1-0-2019-11", + "KMS-PQ-TLS-1-0-2020-02", + "PQ-SIKE-TEST-TLS-1-0-2020-02", + ]; + + for security_policy in EARLY_DRAFT_PQ_POLICIES { + let tls = handshake_with_domain(DOMAIN, security_policy).await?; + + assert_eq!(tls.as_ref().cipher_suite()?, "ECDHE-RSA-AES256-GCM-SHA384"); + assert_eq!(tls.as_ref().kem_name(), None); + } + Ok(()) + } +} + +#[test_log::test(tokio::test)] +async fn tls_client() -> Result<(), Box> { + // The akamai request should be in internet_https_client.rs but Akamai + // http requests hang indefinitely. This behavior is also observed with + // curl and chrome. https://github.com/aws/s2n-tls/issues/4883 + const DOMAINS: &[&str] = &["www.akamai.com"]; + + for domain in DOMAINS { + tracing::info!("querying {domain}"); + + let tls12 = handshake_with_domain(domain, "default").await?; + assert_eq!(tls12.as_ref().actual_protocol_version()?, Version::TLS12); + + let tls13 = handshake_with_domain(domain, "default_tls13").await?; + assert_eq!(tls13.as_ref().actual_protocol_version()?, Version::TLS13); + } + + Ok(()) +} diff --git a/bindings/rust/s2n-tls-hyper/tests/web_client.rs b/bindings/rust/s2n-tls-hyper/tests/web_client.rs deleted file mode 100644 index 41a72970ed3..00000000000 --- a/bindings/rust/s2n-tls-hyper/tests/web_client.rs +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -use bytes::Bytes; -use http::{status, Uri}; -use http_body_util::{BodyExt, Empty}; -use hyper_util::{client::legacy::Client, rt::TokioExecutor}; -use s2n_tls::config::Config; -use s2n_tls_hyper::connector::HttpsConnector; -use std::{error::Error, str::FromStr}; - -#[tokio::test] -async fn test_get_request() -> Result<(), Box> { - let connector = HttpsConnector::new(Config::default()); - let client: Client<_, Empty> = Client::builder(TokioExecutor::new()).build(connector); - - let uri = Uri::from_str("https://www.amazon.com")?; - let response = client.get(uri).await?; - assert_eq!(response.status(), status::StatusCode::OK); - - let body = response.into_body().collect().await?.to_bytes(); - assert!(!body.is_empty()); - - Ok(()) -} diff --git a/bindings/rust/s2n-tls/src/connection.rs b/bindings/rust/s2n-tls/src/connection.rs index 0ac0a389650..e65225cf111 100644 --- a/bindings/rust/s2n-tls/src/connection.rs +++ b/bindings/rust/s2n-tls/src/connection.rs @@ -972,6 +972,34 @@ impl Connection { } } + pub fn kem_name(&self) -> Option<&str> { + let name_bytes = { + let name = unsafe { s2n_connection_get_kem_name(self.connection.as_ptr()) }; + if name.is_null() { + return None; + } + name + }; + + let name_str = unsafe { + // SAFETY: The data is null terminated because it is declared as a C + // string literal. + // SAFETY: kem_name has a static lifetime because it lives on a const + // struct s2n_kem with file scope. + const_str!(name_bytes) + }; + + match name_str { + Ok("NONE") => None, + Ok(name) => Some(name), + Err(_) => { + // Unreachable: This would indicate a non-utf-8 string literal in + // the s2n-tls C codebase. + None + } + } + } + pub fn selected_curve(&self) -> Result<&str, Error> { let curve = unsafe { s2n_connection_get_curve(self.connection.as_ptr()).into_result()? }; unsafe { diff --git a/bindings/rust/s2n-tls/src/testing/s2n_tls.rs b/bindings/rust/s2n-tls/src/testing/s2n_tls.rs index a641eb993f0..1d5d920c384 100644 --- a/bindings/rust/s2n-tls/src/testing/s2n_tls.rs +++ b/bindings/rust/s2n-tls/src/testing/s2n_tls.rs @@ -12,6 +12,7 @@ mod tests { use alloc::sync::Arc; use core::sync::atomic::Ordering; use futures_test::task::{new_count_waker, noop_waker}; + use security::Policy; use std::{fs, path::Path, pin::Pin, sync::atomic::AtomicUsize}; #[test] @@ -26,6 +27,34 @@ mod tests { assert!(TestPair::handshake_with_config(&config).is_ok()); } + #[test] + fn kem_name_retrieval() -> Result<(), Error> { + // PQ isn't supported + { + let policy = Policy::from_version("20240501")?; + let config = build_config(&policy)?; + let mut pair = TestPair::from_config(&config); + + // before negotiation, kem_name is none + assert!(pair.client.kem_name().is_none()); + + pair.handshake().unwrap(); + assert!(pair.client.kem_name().is_none()); + } + + // PQ is supported + { + let policy = Policy::from_version("KMS-PQ-TLS-1-0-2020-07")?; + let config = build_config(&policy)?; + let mut pair = TestPair::from_config(&config); + + pair.handshake().unwrap(); + assert_eq!(pair.client.kem_name(), Some("kyber512r3")); + } + + Ok(()) + } + #[test] fn default_config_and_clone_interaction() -> Result<(), Error> { let config = build_config(&security::DEFAULT_TLS13)?; diff --git a/tests/integrationv2/test_well_known_endpoints.py b/tests/integrationv2/test_well_known_endpoints.py index 7bce5c2248a..6d5d7e9d5a3 100644 --- a/tests/integrationv2/test_well_known_endpoints.py +++ b/tests/integrationv2/test_well_known_endpoints.py @@ -1,130 +1,13 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -import pytest -from constants import TRUST_STORE_BUNDLE, TRUST_STORE_TRUSTED_BUNDLE -from configuration import PROTOCOLS -from common import ProviderOptions, Ciphers, pq_enabled -from fixtures import managed_process # lgtm [py/unused-import] -from global_flags import get_flag, S2N_FIPS_MODE -from providers import Provider, S2N -from test_pq_handshake import PQ_ENABLED_FLAG -from utils import invalid_test_parameters, get_parameter_name, to_bytes +def test_well_known_endpoints(): + ''' + This is a stub test, which allows the existing CI to continue passing while + https://github.com/aws/s2n-tls/pull/4884 is merged in. - -ENDPOINTS = [ - "www.akamai.com", - "www.amazon.com", - "kms.us-east-1.amazonaws.com", - "s3.us-west-2.amazonaws.com", - "www.apple.com", - "www.att.com", - # "www.badssl.com", - # "mozilla-intermediate.badssl.com", - # "mozilla-modern.badssl.com", - # "rsa2048.badssl.com", - # "rsa4096.badssl.com", - # "sha256.badssl.com", - # "sha384.badssl.com", - # "sha512.badssl.com", - # "tls-v1-0.badssl.com", - # "tls-v1-1.badssl.com", - # "tls-v1-2.badssl.com", - "www.cloudflare.com", - "www.ebay.com", - "www.f5.com", - "www.facebook.com", - "www.google.com", - "www.github.com", - "www.ibm.com", - "www.microsoft.com", - # https://github.com/aws/s2n-tls/issues/4879 - # "www.mozilla.org", - "www.netflix.com", - "www.openssl.org", - "www.samsung.com", - "www.t-mobile.com", - "www.twitter.com", - "www.verizon.com", - "www.wikipedia.org", - "www.yahoo.com", - "www.youtube.com", -] - -CIPHERS = [ - None, # `None` will default to the appropriate `test_all` cipher preference in the S2N client provider - Ciphers.KMS_PQ_TLS_1_0_2019_06, - Ciphers.PQ_SIKE_TEST_TLS_1_0_2019_11, - Ciphers.KMS_PQ_TLS_1_0_2020_07, - Ciphers.KMS_PQ_TLS_1_0_2020_02, - Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02 -] - - -if pq_enabled(): - EXPECTED_RESULTS = { - ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2019_06): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2019_11): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_07): - {"cipher": "ECDHE-KYBER-RSA-AES256-GCM-SHA384", "kem": "kyber512r3"}, - ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_02): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - } -else: - EXPECTED_RESULTS = { - ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2019_06): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2019_11): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_07): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.KMS_PQ_TLS_1_0_2020_02): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - ("kms.us-east-1.amazonaws.com", Ciphers.PQ_SIKE_TEST_TLS_1_0_2020_02): - {"cipher": "ECDHE-RSA-AES256-GCM-SHA384", "kem": None}, - } - - -@pytest.mark.uncollect_if(func=invalid_test_parameters) -@pytest.mark.parametrize("protocol", PROTOCOLS, ids=get_parameter_name) -@pytest.mark.parametrize("endpoint", ENDPOINTS, ids=get_parameter_name) -@pytest.mark.parametrize("provider", [S2N], ids=get_parameter_name) -@pytest.mark.parametrize("cipher", CIPHERS, ids=get_parameter_name) -@pytest.mark.flaky(reruns=5, reruns_delay=4) -def test_well_known_endpoints(managed_process, protocol, endpoint, provider, cipher): - port = "443" - - client_options = ProviderOptions( - mode=Provider.ClientMode, - host=endpoint, - port=port, - insecure=False, - trust_store=TRUST_STORE_BUNDLE, - protocol=protocol, - cipher=cipher) - - if get_flag(S2N_FIPS_MODE) is True: - client_options.trust_store = TRUST_STORE_TRUSTED_BUNDLE - - # expect_stderr=True because S2N sometimes receives OCSP responses: - # https://github.com/aws/s2n-tls/blob/14ed186a13c1ffae7fbb036ed5d2849ce7c17403/bin/echo.c#L180-L184 - client = managed_process(provider, client_options, - timeout=5, expect_stderr=True) - - expected_result = EXPECTED_RESULTS.get((endpoint, cipher), None) - - for results in client.get_results(): - results.assert_success() - - if expected_result is not None: - assert to_bytes(expected_result['cipher']) in results.stdout - if expected_result['kem']: - assert to_bytes(expected_result['kem']) in results.stdout - assert to_bytes(PQ_ENABLED_FLAG) in results.stdout - else: - assert to_bytes('KEM:') not in results.stdout - assert to_bytes(PQ_ENABLED_FLAG) not in results.stdout + Once the PR is merged, the Codebuild spec for NixIntegV2Batch will be updated + to remove the "well_known_endpoints" argument (manual process) and then this test + can be fully removed (PR). + ''' + assert 1 == 1 From 45efb09afdea44706b774fcc324d0b6baabf40d8 Mon Sep 17 00:00:00 2001 From: Sam Clark <3758302+goatgoose@users.noreply.github.com> Date: Mon, 18 Nov 2024 12:24:18 -0500 Subject: [PATCH 17/18] test(s2n-tls-hyper): Add localhost http tests (#4838) --- bindings/rust/s2n-tls-hyper/Cargo.toml | 1 + .../rust/s2n-tls-hyper/tests/common/echo.rs | 85 +++++++++++ .../rust/s2n-tls-hyper/tests/common/mod.rs | 27 ++++ bindings/rust/s2n-tls-hyper/tests/http.rs | 140 ++++++++++++++++++ 4 files changed, 253 insertions(+) create mode 100644 bindings/rust/s2n-tls-hyper/tests/common/echo.rs create mode 100644 bindings/rust/s2n-tls-hyper/tests/common/mod.rs create mode 100644 bindings/rust/s2n-tls-hyper/tests/http.rs diff --git a/bindings/rust/s2n-tls-hyper/Cargo.toml b/bindings/rust/s2n-tls-hyper/Cargo.toml index a83970e12d3..8cdf850b3bd 100644 --- a/bindings/rust/s2n-tls-hyper/Cargo.toml +++ b/bindings/rust/s2n-tls-hyper/Cargo.toml @@ -23,4 +23,5 @@ http = { version= "1" } [dev-dependencies] tokio = { version = "1", features = ["macros", "test-util"] } http-body-util = "0.1" +hyper-util = { version = "0.1", features = ["server"] } bytes = "1" diff --git a/bindings/rust/s2n-tls-hyper/tests/common/echo.rs b/bindings/rust/s2n-tls-hyper/tests/common/echo.rs new file mode 100644 index 00000000000..044a99d775d --- /dev/null +++ b/bindings/rust/s2n-tls-hyper/tests/common/echo.rs @@ -0,0 +1,85 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use bytes::Bytes; +use http::{Request, Response}; +use http_body_util::{combinators::BoxBody, BodyExt}; +use hyper::service::service_fn; +use hyper_util::rt::{TokioExecutor, TokioIo}; +use s2n_tls::connection::Builder; +use s2n_tls_tokio::TlsAcceptor; +use std::{error::Error, future::Future}; +use tokio::net::TcpListener; + +async fn echo( + req: Request, +) -> Result>, hyper::Error> { + Ok(Response::new(req.into_body().boxed())) +} + +async fn serve_echo( + tcp_listener: TcpListener, + builder: B, +) -> Result<(), Box> +where + B: Builder, + ::Output: Unpin + Send + Sync + 'static, +{ + let (tcp_stream, _) = tcp_listener.accept().await?; + let acceptor = TlsAcceptor::new(builder); + let tls_stream = acceptor.accept(tcp_stream).await?; + let io = TokioIo::new(tls_stream); + + let server = hyper_util::server::conn::auto::Builder::new(TokioExecutor::new()); + if let Err(err) = server.serve_connection(io, service_fn(echo)).await { + // The hyper client doesn't gracefully terminate by waiting for the server's shutdown. + // Instead, the client sends its shutdown and then immediately closes the socket. This can + // cause a NotConnected error to be emitted when the server attempts to send its shutdown. + // + // For now, NotConnected errors are ignored. After the hyper client can be configured to + // gracefully shutdown, this exception can be removed: + // https://github.com/aws/s2n-tls/issues/4855 + // + // Also, it's possible that a NotConnected error could occur during some operation other + // than a shutdown. Ideally, these NotConnected errors wouldn't be ignored. However, it's + // not currently possible to distinguish between shutdown vs non-shutdown errors: + // https://github.com/aws/s2n-tls/issues/4856 + if let Some(hyper_err) = err.downcast_ref::() { + if let Some(source) = hyper_err.source() { + if let Some(io_err) = source.downcast_ref::() { + if io_err.kind() == tokio::io::ErrorKind::NotConnected { + return Ok(()); + } + } + } + } + + return Err(err); + } + + Ok(()) +} + +pub async fn make_echo_request( + server_builder: B, + send_client_request: F, +) -> Result<(), Box> +where + B: Builder + Send + Sync + 'static, + ::Output: Unpin + Send + Sync + 'static, + F: FnOnce(u16) -> Fut, + Fut: Future>> + Send + 'static, +{ + let listener = TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; + + let mut tasks = tokio::task::JoinSet::new(); + tasks.spawn(serve_echo(listener, server_builder)); + tasks.spawn(send_client_request(addr.port())); + + while let Some(res) = tasks.join_next().await { + res.unwrap()?; + } + + Ok(()) +} diff --git a/bindings/rust/s2n-tls-hyper/tests/common/mod.rs b/bindings/rust/s2n-tls-hyper/tests/common/mod.rs new file mode 100644 index 00000000000..148462d2d12 --- /dev/null +++ b/bindings/rust/s2n-tls-hyper/tests/common/mod.rs @@ -0,0 +1,27 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use s2n_tls::{callbacks::VerifyHostNameCallback, config, error::Error, security::DEFAULT_TLS13}; + +pub mod echo; + +/// NOTE: this certificate and key are used for testing purposes only! +pub static CERT_PEM: &[u8] = + include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../certs/cert.pem")); +pub static KEY_PEM: &[u8] = + include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/../certs/key.pem")); + +pub fn config() -> Result { + let mut builder = config::Config::builder(); + builder.set_security_policy(&DEFAULT_TLS13)?; + builder.trust_pem(CERT_PEM)?; + builder.load_pem(CERT_PEM, KEY_PEM)?; + Ok(builder) +} + +pub struct InsecureAcceptAllCertificatesHandler {} +impl VerifyHostNameCallback for InsecureAcceptAllCertificatesHandler { + fn verify_host_name(&self, _host_name: &str) -> bool { + true + } +} diff --git a/bindings/rust/s2n-tls-hyper/tests/http.rs b/bindings/rust/s2n-tls-hyper/tests/http.rs new file mode 100644 index 00000000000..0a5469b45de --- /dev/null +++ b/bindings/rust/s2n-tls-hyper/tests/http.rs @@ -0,0 +1,140 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +use crate::common::InsecureAcceptAllCertificatesHandler; +use bytes::Bytes; +use http::{Method, Request, Uri}; +use http_body_util::{BodyExt, Empty, Full}; +use hyper_util::{client::legacy::Client, rt::TokioExecutor}; +use s2n_tls::{ + callbacks::{ClientHelloCallback, ConnectionFuture}, + connection::Connection, +}; +use s2n_tls_hyper::connector::HttpsConnector; +use std::{error::Error, pin::Pin, str::FromStr}; + +pub mod common; + +const TEST_DATA: &[u8] = "hello world".as_bytes(); + +// The maximum TLS record payload is 2^14 bytes. +// Send more to ensure multiple records. +const LARGE_TEST_DATA: &[u8] = &[5; (1 << 15)]; + +#[tokio::test] +async fn test_get_request() -> Result<(), Box> { + let config = common::config()?.build()?; + common::echo::make_echo_request(config.clone(), |port| async move { + let connector = HttpsConnector::new(config.clone()); + let client: Client<_, Empty> = + Client::builder(TokioExecutor::new()).build(connector); + + let uri = Uri::from_str(format!("https://localhost:{}", port).as_str())?; + let response = client.get(uri).await?; + assert_eq!(response.status(), 200); + + Ok(()) + }) + .await?; + + Ok(()) +} + +#[tokio::test] +async fn test_http_methods() -> Result<(), Box> { + let methods = [Method::GET, Method::POST, Method::PUT, Method::DELETE]; + for method in methods { + let config = common::config()?.build()?; + common::echo::make_echo_request(config.clone(), |port| async move { + let connector = HttpsConnector::new(config.clone()); + let client: Client<_, Full> = + Client::builder(TokioExecutor::new()).build(connector); + let request: Request> = Request::builder() + .method(method) + .uri(Uri::from_str( + format!("https://localhost:{}", port).as_str(), + )?) + .body(Full::from(TEST_DATA))?; + + let response = client.request(request).await?; + assert_eq!(response.status(), 200); + + let body = response.into_body().collect().await?.to_bytes(); + assert_eq!(body.to_vec().as_slice(), TEST_DATA); + + Ok(()) + }) + .await?; + } + + Ok(()) +} + +#[tokio::test] +async fn test_large_request() -> Result<(), Box> { + let config = common::config()?.build()?; + common::echo::make_echo_request(config.clone(), |port| async move { + let connector = HttpsConnector::new(config.clone()); + let client: Client<_, Full> = Client::builder(TokioExecutor::new()).build(connector); + let request: Request> = Request::builder() + .method(Method::POST) + .uri(Uri::from_str( + format!("https://localhost:{}", port).as_str(), + )?) + .body(Full::from(LARGE_TEST_DATA))?; + + let response = client.request(request).await?; + assert_eq!(response.status(), 200); + + let body = response.into_body().collect().await?.to_bytes(); + assert_eq!(body.to_vec().as_slice(), LARGE_TEST_DATA); + + Ok(()) + }) + .await?; + + Ok(()) +} + +#[tokio::test] +async fn test_sni() -> Result<(), Box> { + struct TestClientHelloHandler { + expected_server_name: &'static str, + } + impl ClientHelloCallback for TestClientHelloHandler { + fn on_client_hello( + &self, + connection: &mut Connection, + ) -> Result>>, s2n_tls::error::Error> { + let server_name = connection.server_name().unwrap(); + assert_eq!(server_name, self.expected_server_name); + Ok(None) + } + } + + for hostname in ["localhost", "127.0.0.1"] { + let mut config = common::config()?; + config.set_client_hello_callback(TestClientHelloHandler { + // Ensure that the HttpsConnector correctly sets the SNI according to the hostname in + // the URI. + expected_server_name: hostname, + })?; + config.set_verify_host_callback(InsecureAcceptAllCertificatesHandler {})?; + let config = config.build()?; + + common::echo::make_echo_request(config.clone(), |port| async move { + let connector = HttpsConnector::new(config.clone()); + let client: Client<_, Empty> = + Client::builder(TokioExecutor::new()).build(connector); + + let uri = Uri::from_str(format!("https://{}:{}", hostname, port).as_str())?; + let response = client.get(uri).await?; + assert_eq!(response.status(), 200); + + Ok(()) + }) + .await?; + } + + Ok(()) +} From 535af43e66a24cdb3c847e9cab954ca5127975ce Mon Sep 17 00:00:00 2001 From: Doug Chapman <54039637+dougch@users.noreply.github.com> Date: Mon, 18 Nov 2024 13:19:46 -0800 Subject: [PATCH 18/18] ci: fixes for cargo audit (#4895) --- .github/workflows/dependencies.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/dependencies.yml b/.github/workflows/dependencies.yml index eaff63bef72..cc5fd9d2e05 100644 --- a/.github/workflows/dependencies.yml +++ b/.github/workflows/dependencies.yml @@ -1,3 +1,4 @@ +--- name: dependencies on: @@ -16,6 +17,9 @@ on: # Run every day at 1800 UTC. - cron: "0 18 * * *" +env: + ROOT_PATH: bindings/rust + jobs: audit: runs-on: ubuntu-latest @@ -24,6 +28,15 @@ jobs: checks: write # Create/update a check run. steps: - uses: actions/checkout@v4 + - name: Install Rust toolchain + id: toolchain + run: | + rustup toolchain install stable + rustup override set stable + - uses: camshaft/rust-cache@v1 + - name: Generate + run: ${{env.ROOT_PATH}}/generate.sh - uses: rustsec/audit-check@v2.0.0 with: token: ${{ secrets.GITHUB_TOKEN }} + working-directory: ${{env.ROOT_PATH}}