Merge branch 'main' into work-flow-overview

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,19 +1,30 @@
+### Release Summary:
+<!-- If this is a feature or bug that impacts customers and is significant enough to include in the "Summary" section of the next version release, please include a brief (1-2 sentences) description of the change. The audience of this summary is future customers, not maintainers or reviewers. See https://github.com/aws/s2n-tls/releases/tag/v1.5.7 for an example. Otherwise, leave this section blank -->
+
 ### Resolved issues:
 
-Resolves #ISSUE-NUMBER1, resolves #ISSUE-NUMBER2, etc.
+resolves #ISSUE-NUMBER1, resolves #ISSUE-NUMBER2, etc.
 
 ### Description of changes: 
 
 Describe s2n’s current behavior and how your code changes that behavior. If there are no issues this PR is resolving, explain why this change is necessary.
 
 ### 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.

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

.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

.github/workflows/dependencies.yml

@@ -0,0 +1,42 @@
+---
+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 * * *"
+
+env:
+  ROOT_PATH: bindings/rust
+
+jobs:
+  audit:
+    runs-on: ubuntu-latest
+    permissions:
+      issues: write   # Open/update issues.
+      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/[email protected]
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
+          working-directory: ${{env.ROOT_PATH}}

CMakeLists.txt

@@ -194,9 +194,13 @@ endif()
 
 target_compile_options(${PROJECT_NAME} PUBLIC -fPIC)
 
-add_definitions(-D_POSIX_C_SOURCE=200809L)
-if(CMAKE_BUILD_TYPE MATCHES Release)
-    add_definitions(-D_FORTIFY_SOURCE=2)
+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
+if(CMAKE_BUILD_TYPE MATCHES Rel)
+    add_definitions(-DS2N_BUILD_RELEASE)
 endif()
 
 if(NO_STACK_PROTECTOR)
@@ -331,7 +335,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
     )
@@ -475,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")
@@ -505,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")
 
@@ -633,7 +640,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()

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" {
@@ -234,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
  */
@@ -244,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/integration/Cargo.toml

@@ -5,20 +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" }
-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
+[dev-dependencies]
+tokio = { version = "1", features = ["macros", "test-util"] }
 
-[[bench]]
-name = "s2nd"
-harness = false
+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"
 
-[dev-dependencies]
-regex = "=1.9.6" # newer versions require rust 1.65, see https://github.com/aws/s2n-tls/issues/4242
+http = "1.1"
+http-body-util = "0.1"
+bytes = "1.8"
+hyper = "1.5"
+hyper-util = "0.1"

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<dyn std::error::Error>> {
+        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(())
+    }
+}