Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add rust well-known-endpoint tests #4884

Merged
merged 15 commits into from
Nov 16, 2024
Merged
4 changes: 4 additions & 0 deletions .github/workflows/ci_rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ jobs:
working-directory: ${{env.ROOT_PATH}}
run: cargo test --features unstable-renegotiate

- name: Network Tests
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
working-directory: ${{env.ROOT_PATH}}/integration
run: 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
Expand Down
21 changes: 21 additions & 0 deletions bindings/rust/integration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,18 @@ authors = ["AWS s2n"]
edition = "2021"
publish = false

[features]
# Network tests are useful but relatively slow and inherently flaky. So they are
# behind this feature flag.
network-tests = []

[dependencies]
s2n-tls = { path = "../s2n-tls"}
s2n-tls-hyper = { path = "../s2n-tls-hyper" }
s2n-tls-tokio = { path = "../s2n-tls-tokio" }
s2n-tls-sys = { path = "../s2n-tls-sys" }

# s2nc/d criterion harness dependencies
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
Expand All @@ -21,4 +30,16 @@ name = "s2nd"
harness = false

[dev-dependencies]
tokio = { version = "1", features = ["macros", "test-util"] }

tracing = "0.1"
tracing-subscriber = "0.3"
tracing-appender = "0.2"
test-log = "0.2"

http = "1.1"
http-body-util = "0.1"
bytes = "1.8"
hyper-util = "0.1"

regex = "=1.9.6" # newer versions require rust 1.65, see https://github.com/aws/s2n-tls/issues/4242
107 changes: 107 additions & 0 deletions bindings/rust/integration/tests/internet_http_client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

/// Purpose: 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.
#[cfg(feature = "network-tests")]
mod http_get {
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
use bytes::Bytes;
use http::{StatusCode, Uri};
use http_body_util::{BodyExt, Empty};
use hyper_util::{client::legacy::Client, rt::TokioExecutor};
use s2n_tls::{config::Config, security};
use s2n_tls_hyper::connector::HttpsConnector;
use std::{error::Error, str::FromStr};
use tracing_subscriber::filter::LevelFilter;

#[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] = &[
// Akamai hangs indefinitely. This is also observed with curl and chrome
// https://github.com/aws/s2n-tls/issues/4883
// TestCase::new("https://www.akamai.com/", 200),
jmayclin marked this conversation as resolved.
Show resolved Hide resolved

// 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),
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
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),
];

#[tokio::test]
async fn http_get_test() -> Result<(), Box<dyn std::error::Error>> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any retry mechanism-- is that handled by hyper automatically, or do you plan to add that in another PR? How flaky are these tests? Same for the PQ tests, since s2n-tls-tokio definitely doesn't handle retries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add more information about this in the callouts, but

  1. it is not handled by hyper automatically
  2. I do plan on adding it in another PR, but not until actually observing some failures.
  3. I don't have any concrete data, but I have not seen any failures locally.

We don't currently have a good understanding of whether the flakiness comes from. TCP? TLS? DNS? Being able to actually observe these failures will allow us to

  1. document where the flakiness actually comes from
  2. have tightly scoped retries to catch other (real) transient errors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting. Do you suspect that the flakiness we attributed to the network was actually coming from the IO parsing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't expect that, but I certainly wouldn't be surprised by that outcome 🤷 .

I'm also wondering whether the amount of queries that we were doing (e.g. 30 failed TLS handshakes for each domain per test run) might have led some domains to start refusing connections.

async fn get(test_case: &TestCase) -> Result<(), Box<dyn Error>> {
for p in [security::DEFAULT, security::DEFAULT_TLS13] {
tracing::info!("executing test case {:#?} with {:?}", test_case, p);

let mut config = Config::builder();
config.set_security_policy(&p)?;

let connector = HttpsConnector::new(config.build()?);
let client: Client<_, Empty<Bytes>> =
Client::builder(TokioExecutor::new()).build(connector);

let uri = Uri::from_str(test_case.query_target)?;
let response = client.get(uri).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(())
}

// enable tracing metrics. hyper/http has extensive logging, so these logs
// are very useful if failures happen in CI.
tracing_subscriber::fmt()
.with_max_level(LevelFilter::TRACE)
.with_test_writer()
.init();

for case in TEST_CASES {
get(case).await?;
}

Ok(())
}
}
93 changes: 93 additions & 0 deletions bindings/rust/integration/tests/internet_kms_pq_client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

#[cfg(feature = "network-tests")]
mod pq_kms {
use s2n_tls::{config::Config, security::Policy};
use s2n_tls_tokio::TlsConnector;
use tokio::net::TcpStream;
use tracing::level_filters::LevelFilter;

#[derive(Debug)]
struct TestCase {
s2n_security_policy: &'static str,
expected_cipher: &'static str,
expected_kem: Option<&'static str>,
}

// When KMS moves to standardized ML-KEM, this test will fail. The test should
// then be updated with the new security policy that supports ML-KEM.
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
const TEST_CASES: &[TestCase] = &[
// positive case: negotiates kyber
TestCase {
s2n_security_policy: "KMS-PQ-TLS-1-0-2020-07",
expected_cipher: "ECDHE-KYBER-RSA-AES256-GCM-SHA384",
expected_kem: Some("kyber512r3"),
},
// negative cases: these policies support a variety of early kyber drafts.
// We want to confirm that non-supported kyber drafts successfully fall
// back to a full handshake.
TestCase {
s2n_security_policy: "KMS-PQ-TLS-1-0-2019-06",
expected_cipher: "ECDHE-RSA-AES256-GCM-SHA384",
expected_kem: None,
},
TestCase {
s2n_security_policy: "PQ-SIKE-TEST-TLS-1-0-2019-11",
expected_cipher: "ECDHE-RSA-AES256-GCM-SHA384",
expected_kem: None,
},
TestCase {
s2n_security_policy: "KMS-PQ-TLS-1-0-2020-02",
expected_cipher: "ECDHE-RSA-AES256-GCM-SHA384",
expected_kem: None,
},
TestCase {
s2n_security_policy: "PQ-SIKE-TEST-TLS-1-0-2020-02",
expected_cipher: "ECDHE-RSA-AES256-GCM-SHA384",
expected_kem: None,
},
];

/// Purpose: ensure that we remain compatible with existing pq AWS deployments.
///
/// This test makes network calls over the public internet.
///
/// KMS is has PQ support. Assert that we successfully negotiate PQ key exchange.
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
// Everything is included in the same block to prevent "unused" errors when
// "network-tests" aren't enabled.
#[tokio::test]
async fn pq_kms_test() -> Result<(), Box<dyn std::error::Error>> {
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
const DOMAIN: &str = "kms.us-east-1.amazonaws.com";
const SOCKET_ADDR: (&str, u16) = (DOMAIN, 443);

async fn test(test_case: &TestCase) -> Result<(), Box<dyn std::error::Error>> {
tracing::info!("executing test case: {:#?}", test_case);

let mut config = Config::builder();
config.set_security_policy(&Policy::from_version(test_case.s2n_security_policy)?)?;

let client = TlsConnector::new(config.build()?);
// open the TCP stream
let stream = TcpStream::connect(SOCKET_ADDR).await?;
// complete the TLS handshake
let tls = client.connect(DOMAIN, stream).await?;

assert_eq!(tls.as_ref().cipher_suite()?, test_case.expected_cipher);
assert_eq!(tls.as_ref().kem_name()?, test_case.expected_kem);

Ok(())
}

tracing_subscriber::fmt()
.with_max_level(LevelFilter::TRACE)
.with_test_writer()
.init();

for test_case in TEST_CASES {
test(test_case).await?
}

Ok(())
}
}
25 changes: 0 additions & 25 deletions bindings/rust/s2n-tls-hyper/tests/web_client.rs

This file was deleted.

18 changes: 18 additions & 0 deletions bindings/rust/s2n-tls/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,24 @@ impl Connection {
}
}

pub fn kem_name(&self) -> Result<Option<&str>, Error> {
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
let name_bytes =
unsafe { s2n_connection_get_kem_name(self.connection.as_ptr()).into_result()? };
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") => Ok(None),
Ok(name) => Ok(Some(name)),
Err(e) => Err(e),
}
}

pub fn selected_curve(&self) -> Result<&str, Error> {
let curve = unsafe { s2n_connection_get_curve(self.connection.as_ptr()).into_result()? };
unsafe {
Expand Down
29 changes: 29 additions & 0 deletions bindings/rust/s2n-tls/src/testing/s2n_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)?;
Expand Down
Loading
Loading