Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 75 additions & 7 deletions .github/workflows/ci_windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,35 @@ jobs:
rust-bindings:
runs-on: windows-latest
name: Rust Bindings (${{matrix.sys}})
env:
# msys2 ships clang, not gcc, but cc-rs defaults to gcc for *-gnu.
CC: clang
CXX: clang++
# gnullvm doesn't auto-link winpthreads, so s2n's pthread/clock_gettime/
# nanosleep symbols are undefined; -lpthread links it explicitly. Both vars
# are needed: RUSTFLAGS for the normal link, RUSTDOCFLAGS for doctests
CARGO_TARGET_X86_64_PC_WINDOWS_GNULLVM_RUSTFLAGS: -Clink-arg=-lpthread
CARGO_TARGET_X86_64_PC_WINDOWS_GNULLVM_RUSTDOCFLAGS: -Clink-arg=-lpthread
strategy:
fail-fast: false
matrix:
include:
- sys: ucrt64
pkg-prefix: mingw-w64-ucrt-x86_64
env-path: /ucrt64
rust-target: x86_64-pc-windows-gnu
rust-host: x86_64-pc-windows-gnu
- sys: mingw64
pkg-prefix: mingw-w64-x86_64
env-path: /mingw64
rust-target: x86_64-pc-windows-gnu
rust-host: x86_64-pc-windows-gnu
- sys: clang64
pkg-prefix: mingw-w64-clang-x86_64
env-path: /clang64
rust-target: x86_64-pc-windows-gnullvm
# clang64 targets gnullvm, but pinned Rust 1.89 has no gnullvm host
# toolchain. rust-host is the host we install (gnu); cargo-target
# is what we cross-compile for (gnullvm).
rust-host: x86_64-pc-windows-gnu
cargo-target: x86_64-pc-windows-gnullvm
steps:
- uses: actions/checkout@v6

Expand All @@ -120,16 +133,71 @@ jobs:
make
git
diffutils
jq

- name: Install Rust toolchain
shell: msys2 {0}
run: |
rustup default stable-${{matrix.rust-target}}
set -eu
# Force the MinGW gnu host; otherwise rustup picks the runner's MSVC
# host, which can't build build.rs's GCC/Clang flags.
rustup set default-host ${{matrix.rust-host}}
rustup default stable-${{matrix.rust-host}}
rustup component add rustfmt

- name: Generate and build Rust bindings
# clang64 has no gnullvm host toolchain, so cross-compile to gnullvm
# from the gnu host.
if [ -n "${{matrix.cargo-target}}" ]; then
# Add the gnullvm std to both toolchains that need it: the "1.89"
# workspaces (both pin it, so one add covers both) and the repo-root
# "stable" default used by the cargo install below.
( cd bindings/rust/extended && rustup target add ${{matrix.cargo-target}} )
rustup target add ${{matrix.cargo-target}}
echo "CARGO_BUILD_TARGET=${{matrix.cargo-target}}" >> "$GITHUB_ENV"
# aws-lc-sys has no prebuilt bindings for gnullvm, so it runs bindgen
# at build time. Install bindgen-cli into the msys2 prefix (on PATH);
# build it for gnullvm so it runs in the clang64 environment.
cargo install --locked --target ${{matrix.cargo-target}} \
--root "${{matrix.env-path}}" bindgen-cli
fi

# openssl-src doesn't pass `-j` to make on Windows, so the vendored OpenSSL
# build runs single-threaded. Export MAKEFLAGS so make parallelizes
# which speeds up Openssl Vendered version build.
- name: Enable parallel make for vendored OpenSSL
shell: msys2 {0}
run: echo "MAKEFLAGS=-j$(nproc)" >> "$GITHUB_ENV"

- name: Generate Rust bindings
shell: msys2 {0}
run: |
set -eu
./bindings/rust/extended/generate.sh

- name: Test extended workspace
shell: msys2 {0}
working-directory: bindings/rust/extended
run: |
set -eu
cargo test

- name: "Feature Tests: Fingerprint, kTLS, QUIC, PQ, and unstable-custom_x509_extensions"
shell: msys2 {0}
working-directory: bindings/rust/extended
run: |
set -eu
cargo test --features unstable-fingerprint,unstable-ktls,quic,pq,unstable-custom_x509_extensions
Comment thread
boquan-fang marked this conversation as resolved.

- name: "Feature Test: Renegotiate"
shell: msys2 {0}
working-directory: bindings/rust/extended
run: |
set -eu
cargo test --features unstable-renegotiate

- name: Test integration crate
shell: msys2 {0}
working-directory: bindings/rust/standard
run: |
set -eu
./bindings/rust/extended/generate.sh --skip-tests
cargo build --manifest-path bindings/rust/extended/s2n-tls/Cargo.toml
cargo test -p integration
25 changes: 23 additions & 2 deletions bindings/rust/extended/generate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ set -xe
# cd into the script directory so it can be executed from anywhere
pushd "$(dirname "${BASH_SOURCE[0]}")"

# Helper function to check if the rust bindings is running on Windows
is_windows() {
local os
os="$(uname -s)"
if [[ "$os" == MINGW* || "$os" == MSYS* || "$os" == CYGWIN* ]]; then
true
else
false
fi
}

# delete the existing copy in case we have extra files
rm -rf s2n-tls-sys/lib
mkdir -p s2n-tls-sys/lib
Expand Down Expand Up @@ -49,10 +60,20 @@ fi;
# make sure everything builds and passes sanity checks
pushd s2n-tls-sys
cargo test
cargo test --all-features
cargo test --release
cargo publish --dry-run --allow-dirty
cargo publish --dry-run --allow-dirty --all-features
if is_windows; then
# `fips` is the one feature that can't build on Windows/MinGW, since aws-lc-fips-sys can't be built on MSYS2 MinGW.
# Test every other feature instead of `--all-features`.
# The list is read from the manifest, so newly added features are covered automatically.
windows_features=$(cargo metadata --no-deps --format-version 1 \
| jq -r '.packages[] | select(.name == "s2n-tls-sys").features | keys - ["default", "fips"] | join(",")')
cargo test --features "$windows_features"
cargo publish --dry-run --allow-dirty --features "$windows_features"
else
cargo test --all-features
cargo publish --dry-run --allow-dirty --all-features
fi
popd

pushd ../standard/integration
Expand Down
27 changes: 26 additions & 1 deletion bindings/rust/extended/s2n-tls-tokio/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

#[cfg(not(target_os = "windows"))]
use errno::{set_errno, Errno};
use s2n_tls::{
config::Config,
Expand Down Expand Up @@ -260,13 +261,37 @@ where
match res {
Poll::Ready(Ok(len)) => len as c_int,
Poll::Pending => {
set_errno(Errno(libc::EWOULDBLOCK));
Self::set_io_would_block();
CallbackResult::Failure.into()
}
_ => CallbackResult::Failure.into(),
}
}

/// Signal a "would block" to s2n's C IO layer by setting the CRT `errno` to
/// EWOULDBLOCK. `s2n_io.c` reads `errno` to distinguish a retriable blocked
/// read/write from a fatal IO error.
#[cfg(not(target_os = "windows"))]
fn set_io_would_block() {
// The `errno` crate writes the CRT errno, which is what s2n reads.
set_errno(Errno(libc::EWOULDBLOCK));
}

/// Windows variant: the `errno` crate writes the Win32 last-error, not the
/// CRT `errno` that s2n reads, so set the CRT errno directly. s2n and this
/// code share one statically linked CRT, so `_set_errno` and `errno` hit the
/// same thread-local.
#[cfg(target_os = "windows")]
fn set_io_would_block() {
extern "C" {
fn _set_errno(value: core::ffi::c_int) -> core::ffi::c_int;
}
// SAFETY: `_set_errno` only writes the thread-local CRT errno.
unsafe {
let _ = _set_errno(libc::EWOULDBLOCK);
}
}

unsafe extern "C" fn recv_io_cb(ctx: *mut c_void, buf: *mut u8, len: u32) -> c_int {
Self::poll_io(ctx, |stream, async_context| {
let len: usize = len.try_into().unwrap();
Expand Down
12 changes: 11 additions & 1 deletion bindings/rust/extended/s2n-tls/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,19 @@ mod tests {
// this value if the definition of an IO error changes might be easier.
const S2N_IO_ERROR_CODE: s2n_status_code::Type = 1 << 26;

// The value that decodes to `ConnectionReset` differs by platform:
// `from_raw_os_error` reads a POSIX errno on non-Windows and a Winsock code
// on Windows.
#[cfg(not(target_os = "windows"))]
const OS_CONNECTION_RESET: i32 = libc::ECONNRESET;
// 10054 is WSAECONNRESET ("Connection reset by peer").
// See https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
#[cfg(target_os = "windows")]
const OS_CONNECTION_RESET: i32 = 10054;

#[test]
fn s2n_io_error_to_std_io_error() -> Result<(), Box<dyn std::error::Error>> {
set_errno(Errno(libc::ECONNRESET));
set_errno(Errno(OS_CONNECTION_RESET));
unsafe {
let s2n_errno_ptr = s2n_errno_location();
*s2n_errno_ptr = S2N_IO_ERROR_CODE;
Expand Down
4 changes: 4 additions & 0 deletions bindings/rust/extended/s2n-tls/src/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,8 @@ mod tests {
Ok(())
}

// Relies on the `checkers` global allocator, which is not installed on Windows.
#[cfg(not(target_os = "windows"))]
#[test]
fn hash_does_not_allocate_memory() -> Result<(), Box<dyn Error>> {
let client_hello = ClientHello::parse_client_hello(CLIENT_HELLO_BYTES)?;
Expand All @@ -593,6 +595,8 @@ mod tests {
Ok(())
}

// Relies on the `checkers` global allocator, which is not installed on Windows.
#[cfg(not(target_os = "windows"))]
#[test]
fn raw_may_allocate_memory() -> Result<(), Box<dyn Error>> {
let client_hello = ClientHello::parse_client_hello(CLIENT_HELLO_BYTES)?;
Expand Down
7 changes: 6 additions & 1 deletion bindings/rust/extended/s2n-tls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ extern crate alloc;
// Ensure memory is correctly managed in tests
// tests invoked using the checkers::test macro have additional
// memory sanity checks that occur
#[cfg(test)]
//
// This allocator is not installed on Windows. When it is registered as the
// global allocator there, the test binary overflows its stack during startup.
// Checkers doesn't seem to work well on Windows. We only use checkers for
// these allocation-sanity checks. The allocator is gated to non-Windows targets.
#[cfg(all(test, not(target_os = "windows")))]
#[global_allocator]
static ALLOCATOR: checkers::Allocator = checkers::Allocator::system();

Expand Down
4 changes: 3 additions & 1 deletion bindings/rust/extended/s2n-tls/src/renegotiate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,9 @@ mod tests {
_: *const u8,
_: u32,
) -> libc::c_int {
errno::set_errno(errno::Errno(libc::EWOULDBLOCK));
// Signal a blocked send to s2n. See crate::testing::set_io_would_block
// for the platform errno-channel details.
crate::testing::set_io_would_block();
-1
}

Expand Down
26 changes: 25 additions & 1 deletion bindings/rust/extended/s2n-tls/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ impl TestPair {
// TCP Close) which would not be correct here. To just communicate
// that there is no more data, we instead set the errno to
// WouldBlock and return -1.
errno::set_errno(errno::Errno(libc::EWOULDBLOCK));
set_io_would_block();
-1
} else {
len as c_int
Expand All @@ -404,6 +404,30 @@ impl TestPair {
}
}

/// Signal a "would block" to s2n's C IO layer by setting the CRT `errno` to
/// EWOULDBLOCK. `s2n_io.c` reads `errno` to distinguish a retriable blocked
/// read/write from a fatal IO error. Shared by this crate's test IO callbacks.
#[cfg(not(target_os = "windows"))]
pub(crate) fn set_io_would_block() {
Comment thread
boquan-fang marked this conversation as resolved.
// The `errno` crate writes the CRT errno, which is what s2n reads.
errno::set_errno(errno::Errno(libc::EWOULDBLOCK));
}

/// Windows variant: the `errno` crate writes the Win32 last-error, not the CRT
/// `errno` that s2n reads, so set the CRT errno directly. s2n and this code
/// share one statically linked CRT, so `_set_errno` and `errno` hit the same
/// thread-local.
#[cfg(target_os = "windows")]
pub(crate) fn set_io_would_block() {
extern "C" {
fn _set_errno(value: core::ffi::c_int) -> core::ffi::c_int;
}
// SAFETY: `_set_errno` only writes the thread-local CRT errno.
unsafe {
let _ = _set_errno(libc::EWOULDBLOCK);
}
}

type SessionState = Vec<u8>;

/// This is a simple struct to enable session resumption in unit tests.
Expand Down
7 changes: 7 additions & 0 deletions bindings/rust/extended/s2n-tls/src/testing/s2n_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ mod tests {
Ok(())
}

// Uses `SSL_CERT_FILE`, which s2n reads via the C `getenv`. On Windows the
// `temp_env` var below only updates the Win32 environment, which `getenv`
// doesn't see, so the cert never loads. Gate the test out since there is a test limitation.
#[cfg(not(target_os = "windows"))]
#[test]
fn system_certs_loaded_by_default() -> Result<(), Error> {
let keypair = CertKeyPair::default();
Expand All @@ -440,6 +444,9 @@ mod tests {
})
}

// See system_certs_loaded_by_default: the `temp_env` `SSL_CERT_FILE` var
// isn't visible to the C `getenv` that s2n uses on Windows.
#[cfg(not(target_os = "windows"))]
#[test]
fn disable_loading_system_certs() -> Result<(), Error> {
let keypair = CertKeyPair::default();
Expand Down
10 changes: 7 additions & 3 deletions bindings/rust/standard/integration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,17 @@ hyper-util = "0.1"

dhat = "0.3.3"
tabled = "0.21.0"
serde_json = "1"

# The most recent published metrics subscriber assumes the Linux binding types for the
# s2n_tls_extension_type enum (u32 vs i32), so it can't compile on Windows.
[target.'cfg(not(target_os = "windows"))'.dev-dependencies]
# This should be the latest version of metrics subscriber available on crates.io
old-metrics-subscriber = { package = "s2n-tls-metrics-subscriber", version = "0.0.3" }
serde_json = "1"

# NOTE: BoringSSL is disabled on macOS to avoid symbol collisions with
# OpenSSL; see https://github.com/aws/s2n-tls/pull/5659 for details.
[target.'cfg(not(target_os = "macos"))'.dev-dependencies.boring]
# OpenSSL (see https://github.com/aws/s2n-tls/pull/5659), and on Windows because BoringSSL can't be built under the MSYS2/MinGW toolchain.
[target.'cfg(not(any(target_os = "macos", target_os = "windows")))'.dev-dependencies.boring]
git = "https://github.com/kaukabrizvi/boring.git"
features = ["prefix-symbols"]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ fn register_async_pkey_verify_offload(
// callback. The callback will reclaim ownership using Box::from_raw to prevent leaks.
unsafe {
let raw = raw_config(s2n_cfg);
// bindgen generates this enum differently based on platform: u32 on
// Linux, i32 on Windows. The `allow_list` param is always u32, so cast
// on Windows to match. https://github.com/rust-lang/rust-bindgen/issues/1907
let allowed_types = s2n_async_offload_op_type::OFFLOAD_PKEY_VERIFY;
#[cfg(target_os = "windows")]
let allowed_types = allowed_types as u32;

let result = s2n_config_set_async_offload_callback(
raw,
Expand Down
5 changes: 5 additions & 0 deletions bindings/rust/standard/integration/tests/memory.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

// The dhat global allocator below recurses during process startup on Windows,
// overflowing the main thread's stack before any test runs. This
// profiling test is therefore limited to non-Windows targets.
#![cfg(not(target_os = "windows"))]

#[global_allocator]
static ALLOC: dhat::Alloc = dhat::Alloc;
#[cfg(not(feature = "no-sensitive-tests"))]
Expand Down
4 changes: 4 additions & 0 deletions bindings/rust/standard/integration/tests/metrics_stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
//! compatibility with the released subscriber. The fix is to restore the
//! missing APIs (with #[deprecated] annotations) in the bindings.

// The s2n-tls-metrics-subscriber assumes the Linux binding types and does not compile on Windows,
// so this compatibility test is limited to non-Windows targets (see integration/Cargo.toml).
#![cfg(not(target_os = "windows"))]

use old_metrics_subscriber::{
AggregatedMetricsSubscriber, Attribution, MetricRecord, TelemetrySink,
};
Expand Down
2 changes: 1 addition & 1 deletion bindings/rust/standard/rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
[toolchain]
channel = "1.91.0"
channel = "1.91"
Loading
Loading