Merge branch 'main' into dependabot/github_actions/dot-github/workflo…

…ws/aws-actions/configure-aws-credentials-4.0.2

.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/[email protected]
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}

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")
 

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);

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" }

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"

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]

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"
 

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;
 }

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()

codebuild/bin/test_exec_leak.sh

@@ -51,7 +51,7 @@ cat <<EOF > 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);

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

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.

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])

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)

tests/s2n_test.h

@@ -15,18 +15,18 @@
 
 #pragma once
 #include <errno.h>
+#include <openssl/crypto.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-#include <openssl/crypto.h>
-
 #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[]) \
 { \