From bffcb61d78e547046a4fd54afc26d9f1a5f38b64 Mon Sep 17 00:00:00 2001 From: "Sean P. Kelly" Date: Mon, 21 Aug 2023 16:16:45 -0700 Subject: [PATCH 1/5] github: run CI on windows This also disables Rust incremental builds in the CI for a few reasons: * Incremental builds need a lot of disk space, which we need to protect for Windows builds. * Only the first invocation for each cache key is stored, so the delta between incremental builds grows larger over time. See https://github.com/dtolnay/rust-toolchain/issues/26 for more details. --- .github/cache_bust | 2 +- .github/workflows/rust.yml | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/cache_bust b/.github/cache_bust index a6869102..a433a844 100644 --- a/.github/cache_bust +++ b/.github/cache_bust @@ -1,4 +1,4 @@ # this file provides a manual way to clear out github actions caches. any change # to this file will cause all github action caches to miss. increment the number # below by 1 if you need to clear the caches. -3 +4 diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 952ef520..551a3f36 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -5,23 +5,29 @@ on: - "**.md" - ".github/dependabot.yml" branches: [develop] + +env: + # From-scratch builds with incremental compilation enabled adds unneeded performance and disk overhead. + CARGO_INCREMENTAL: "0" + jobs: build: strategy: + fail-fast: false matrix: make_target: ["check-licenses", "build", "integ"] - runs-on: ubuntu-latest + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 - uses: actions/cache@v3 with: path: | .cargo - target # you can edit the .github/cache_bust file if you need to clear the cache key: ${{ hashFiles('.github/cache_bust') }}-${{ runner.os }}-${{ matrix.make_target }}-${{ hashFiles('**/Cargo.lock') }} restore-keys: | - ${{ hashFiles('.github/cache_bust') }}-${{ runner.os }}-${{ matrix.make_target }}- + ${{ hashFiles('.github/cache_bust') }}-${{ runner.os }}-${{ matrix.make_target }} - run: rustup default 1.71.1 - run: rustup component add rustfmt - run: rustup component add clippy From 55a40cc833d8ef3d555fceca0baf3b06601caf59 Mon Sep 17 00:00:00 2001 From: "Sean P. Kelly" Date: Mon, 21 Aug 2023 22:19:07 -0700 Subject: [PATCH 2/5] tough: always use unix-style paths in clean_name Using `std::path::Path` to manipulate `Url` paths only has the correct behavior on unix systems where the properties of the paths are nearly identical. This change moves to using unix-style path manipulation regardless of platform. It also adds a utility for safely creating filesystem paths based on `Url` paths for both unix and windows (which is complicated by drive lettering in absolute paths). --- Cargo.lock | 26 ++++---------- tough/Cargo.toml | 2 +- tough/src/error.rs | 6 ++++ tough/src/lib.rs | 2 ++ tough/src/target_name.rs | 18 ++++------ tough/src/transport.rs | 7 ++-- tough/src/urlpath.rs | 73 ++++++++++++++++++++++++++++++++++++++++ tuftool/src/error.rs | 3 ++ tuftool/src/source.rs | 24 +++++++------ 9 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 tough/src/urlpath.rs diff --git a/Cargo.lock b/Cargo.lock index 7c075c3a..ff717488 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1427,24 +1427,6 @@ dependencies = [ "windows-targets", ] -[[package]] -name = "path-absolutize" -version = "3.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43eb3595c63a214e1b37b44f44b0a84900ef7ae0b4c5efce59e123d246d7a0de" -dependencies = [ - "path-dedot", -] - -[[package]] -name = "path-dedot" -version = "3.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d55e486337acb9973cdea3ec5638c1b3bcb22e573b2b7b41969e0c744d5a15e" -dependencies = [ - "once_cell", -] - [[package]] name = "pem" version = "1.1.1" @@ -2184,7 +2166,6 @@ dependencies = [ "log", "maplit", "olpc-cjson", - "path-absolutize", "pem", "percent-encoding", "reqwest", @@ -2194,6 +2175,7 @@ dependencies = [ "serde_plain", "snafu", "tempfile", + "typed-path", "untrusted", "url", "walkdir", @@ -2327,6 +2309,12 @@ dependencies = [ "walkdir", ] +[[package]] +name = "typed-path" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "deb9adcf07a4ad4233bc70d3a609dd1cc831da1f8d851cf62d6327769d08179f" + [[package]] name = "typenum" version = "1.16.0" diff --git a/tough/Cargo.toml b/tough/Cargo.toml index 3b7e85ca..e7cbb75f 100644 --- a/tough/Cargo.toml +++ b/tough/Cargo.toml @@ -15,7 +15,6 @@ globset = { version = "0.4" } hex = "0.4" log = "0.4" olpc-cjson = { version = "0.1", path = "../olpc-cjson" } -path-absolutize = "3" pem = "1" percent-encoding = "2" reqwest = { version = "0.11", optional = true, default-features = false, features = ["blocking"] } @@ -25,6 +24,7 @@ serde_json = "1" serde_plain = "1" snafu = "0.7" tempfile = "3" +typed-path = "0.4" untrusted = "0.7" url = "2" walkdir = "2" diff --git a/tough/src/error.rs b/tough/src/error.rs index b7616821..345747b9 100644 --- a/tough/src/error.rs +++ b/tough/src/error.rs @@ -306,6 +306,12 @@ pub enum Error { #[snafu(display("Path {} is not valid UTF-8", path.display()))] PathUtf8 { path: PathBuf, backtrace: Backtrace }, + #[snafu(display("Path {} is not valid UTF-8", path.display()))] + UnixPathUtf8 { + path: typed_path::UnixPathBuf, + backtrace: Backtrace, + }, + #[snafu(display("Failed to remove existing target path '{}': {}", path.display(), source))] RemoveTarget { path: PathBuf, diff --git a/tough/src/lib.rs b/tough/src/lib.rs index 3f406f5e..9be298af 100644 --- a/tough/src/lib.rs +++ b/tough/src/lib.rs @@ -43,6 +43,7 @@ pub mod schema; pub mod sign; mod target_name; mod transport; +mod urlpath; use crate::datastore::Datastore; use crate::error::Result; @@ -57,6 +58,7 @@ pub use crate::target_name::TargetName; pub use crate::transport::{ DefaultTransport, FilesystemTransport, Transport, TransportError, TransportErrorKind, }; +pub use crate::urlpath::SafeUrlPath; use chrono::{DateTime, Utc}; use log::warn; use percent_encoding::{utf8_percent_encode, AsciiSet, NON_ALPHANUMERIC}; diff --git a/tough/src/target_name.rs b/tough/src/target_name.rs index c742f97e..e1efdc42 100644 --- a/tough/src/target_name.rs +++ b/tough/src/target_name.rs @@ -1,11 +1,10 @@ use crate::error::{self, Result}; -use path_absolutize::Absolutize; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use snafu::{ensure, OptionExt, ResultExt}; +use snafu::{ensure, OptionExt}; use std::convert::TryFrom; -use std::path::PathBuf; use std::str::FromStr; +use typed_path::{Component, UnixPath, UnixPathBuf}; /// Represents the name of a target in the repository. Path-like constructs are resolved (e.g. /// `foo/../bar` becomes `bar`). Certain unsafe names are rejected when constructing a `TargetName`. @@ -113,13 +112,11 @@ fn clean_name(name: &str) -> Result { ensure!(!name.is_empty(), error::UnsafeTargetNameEmptySnafu { name }); // If our name starts with absolute, then we need to remember this so we can restore it later. - let name_path = PathBuf::from(name); + let name_path = UnixPathBuf::from(name); let absolute = name_path.is_absolute(); let clean = { - let proposed = name_path - .absolutize_from(&PathBuf::from("/")) - .context(error::TargetNameResolveSnafu { name })?; + let proposed = UnixPath::new("/").join(name_path).normalize(); // `absolutize_from` will give us a path that starts with `/`, so we remove it if the // original name did not start with `/` @@ -135,12 +132,12 @@ fn clean_name(name: &str) -> Result { .next() // If this error occurs then there is a bug or behavior change in absolutize_from. .context(error::TargetNameComponentsEmptySnafu { name })? - .as_os_str(); + .as_bytes(); // If the first component isn't the main separator ( unix `/`, windows '\' ) // then there is a bug or behavior change in absolutize_from. ensure!( - first_component == std::path::MAIN_SEPARATOR_STR, + first_component == typed_path::unix::SEPARATOR_STR.as_bytes(), error::TargetNameRootMissingSnafu { name } ); @@ -149,9 +146,8 @@ fn clean_name(name: &str) -> Result { }; let final_name = clean - .as_os_str() .to_str() - .context(error::PathUtf8Snafu { path: &clean })? + .context(error::UnixPathUtf8Snafu { path: &clean })? .to_string(); // Check again to make sure we didn't end up with an empty string. diff --git a/tough/src/transport.rs b/tough/src/transport.rs index d72b681a..b75f1f59 100644 --- a/tough/src/transport.rs +++ b/tough/src/transport.rs @@ -1,10 +1,10 @@ +use crate::SafeUrlPath; #[cfg(feature = "http")] use crate::{HttpTransport, HttpTransportBuilder}; use dyn_clone::DynClone; use std::error::Error; use std::fmt::{Debug, Display, Formatter}; use std::io::{ErrorKind, Read}; -use std::path::PathBuf; use url::Url; /// A trait to abstract over the method/protocol by which files are obtained. @@ -149,10 +149,7 @@ impl Transport for FilesystemTransport { )); } - // Convert the file URL into a file path. We need to use url.path() and not - // url.to_file_path() because to_file_path will decode the percent encoding which could - // restore path traversal characters. - let file_path = PathBuf::from(url.path()); + let file_path = url.safe_url_filepath(); // And open the file let f = std::fs::File::open(file_path).map_err(|e| { diff --git a/tough/src/urlpath.rs b/tough/src/urlpath.rs new file mode 100644 index 00000000..d348249a --- /dev/null +++ b/tough/src/urlpath.rs @@ -0,0 +1,73 @@ +//! This module contains utilities for mapping URL paths to local Paths. +use std::path::PathBuf; +use url::Url; + +/// Converts a file URL into a file path. +/// Needed because `url.to_file_path()` will decode any percent encoding, which could restore path +/// traversal characters, and `url.path()` roots paths to '/' on Windows. +pub trait SafeUrlPath { + /// Returns the path component of a URL as a filesystem path. + fn safe_url_filepath(&self) -> PathBuf; +} + +#[cfg(windows)] +impl SafeUrlPath for Url { + fn safe_url_filepath(&self) -> PathBuf { + let url_path = self.path(); + + // Windows filepaths when written as `file://` URLs have path components prefixed with a /. + PathBuf::from(if let Some(stripped) = url_path.strip_prefix('/') { + stripped + } else { + url_path + }) + } +} + +#[cfg(unix)] +impl SafeUrlPath for Url { + fn safe_url_filepath(&self) -> PathBuf { + PathBuf::from(self.path()) + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::encode_filename; + use std::path::PathBuf; + + fn manifest_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + } + + #[test] + fn test_safe_simple() { + let cargo_toml = manifest_dir().join("Cargo.toml"); + let cargo_toml_url = Url::from_file_path(&cargo_toml) + .expect("Could not create URL from Cargo.toml filepath"); + + let safe_url_path = cargo_toml_url.safe_url_filepath(); + + assert_eq!(cargo_toml, safe_url_path); + assert!(safe_url_path.is_absolute()); + } + + #[test] + fn test_safe_traversals() { + let url_base = Url::from_directory_path(manifest_dir()) + .expect("Could not create URL from CARGO_MANIFEST_DIR"); + + let escaped_test_path = encode_filename("a/../b/././c/.."); + let traversal_url = url_base.join(&escaped_test_path).expect(&format!( + "Could not create URL from unusual traveral path '{}' + '{}'", + url_base.to_string(), + escaped_test_path + )); + + assert_eq!( + manifest_dir().join("a%2F..%2Fb%2F.%2F.%2Fc%2F.."), + traversal_url.safe_url_filepath(), + ); + } +} diff --git a/tuftool/src/error.rs b/tuftool/src/error.rs index 5a281dd2..d8f22ef1 100644 --- a/tuftool/src/error.rs +++ b/tuftool/src/error.rs @@ -127,6 +127,9 @@ pub(crate) enum Error { backtrace: Backtrace, }, + #[snafu(display("Can't build URL from path '{}'", path.display()))] + FileUrl { path: PathBuf, backtrace: Backtrace }, + #[snafu(display("Failed to write to {}: {}", path.display(), source))] FileWrite { path: PathBuf, diff --git a/tuftool/src/source.rs b/tuftool/src/source.rs index 27f688a3..45556852 100644 --- a/tuftool/src/source.rs +++ b/tuftool/src/source.rs @@ -34,9 +34,10 @@ //! "aws-ssm:///a/key" (notice the 3 slashes after the colon) use crate::error::{self, Result}; -use snafu::ResultExt; -use std::path::PathBuf; +use snafu::{OptionExt, ResultExt}; +use std::path::Path; use tough::key_source::{KeySource, LocalKeySource}; +use tough::SafeUrlPath; use tough_kms::{KmsKeySource, KmsSigningAlgorithm}; use tough_ssm::SsmKeySource; use url::Url; @@ -50,16 +51,19 @@ use url::Url; /// the `KeySource` trait in the `tough` library. A user can then add /// to this parser to support them in `tuftool`. pub(crate) fn parse_key_source(input: &str) -> Result> { - let pwd_url = - Url::from_directory_path(std::env::current_dir().context(error::CurrentDirSnafu)?) - .expect("expected current directory to be absolute"); - let url = Url::options() - .base_url(Some(&pwd_url)) - .parse(input) - .context(error::UrlParseSnafu { url: input })?; + let input_as_path = Path::new(input); + let url = if input_as_path.exists() { + Url::from_file_path(input) + .ok() // dump unhelpful `()` error + .context(error::FileUrlSnafu { + path: input_as_path.to_owned(), + })? + } else { + Url::parse(input).context(error::UrlParseSnafu { url: input })? + }; match url.scheme() { "file" => Ok(Box::new(LocalKeySource { - path: PathBuf::from(url.path()), + path: url.safe_url_filepath(), })), #[cfg(any(feature = "aws-sdk-rust-native-tls", feature = "aws-sdk-rust-rustls"))] "aws-ssm" => Ok(Box::new(SsmKeySource { From 967e02dfd19c74506111b22f600ef9cb360bc270 Mon Sep 17 00:00:00 2001 From: Sean P Kelly Date: Tue, 22 Aug 2023 03:04:31 -0700 Subject: [PATCH 3/5] Refrain from using docker for integration testing Github actions on Windows do not allow running Linux containers. In order to run the same CI on Windows as Linux, we compile toxic HTTP services at test time. --- Cargo.lock | 353 +++++++++++++++++- Makefile | 19 +- README.md | 9 +- deny.toml | 2 + integ/failure-server/.gitattributes | 4 - integ/failure-server/Cargo.toml | 19 + integ/failure-server/Dockerfile.toxiproxycli | 6 - integ/failure-server/Dockerfile.toxy | 4 - integ/failure-server/run.sh | 90 ----- integ/failure-server/src/lib.rs | 80 ++++ integ/failure-server/src/toxic/http_server.rs | 96 +++++ integ/failure-server/src/toxic/mod.rs | 35 ++ integ/failure-server/src/toxic/tcp_proxy.rs | 127 +++++++ integ/failure-server/teardown.sh | 15 - integ/failure-server/toxiproxy/setup.sh | 32 -- integ/failure-server/toxy/.gitignore | 1 - integ/failure-server/toxy/index.js | 29 -- integ/failure-server/toxy/package.json | 13 - tough/Cargo.toml | 4 +- tough/README.md | 2 +- tough/src/lib.rs | 2 +- tough/tests/http.rs | 137 +++---- 22 files changed, 777 insertions(+), 302 deletions(-) delete mode 100644 integ/failure-server/.gitattributes create mode 100644 integ/failure-server/Cargo.toml delete mode 100644 integ/failure-server/Dockerfile.toxiproxycli delete mode 100644 integ/failure-server/Dockerfile.toxy delete mode 100755 integ/failure-server/run.sh create mode 100644 integ/failure-server/src/lib.rs create mode 100644 integ/failure-server/src/toxic/http_server.rs create mode 100644 integ/failure-server/src/toxic/mod.rs create mode 100644 integ/failure-server/src/toxic/tcp_proxy.rs delete mode 100755 integ/failure-server/teardown.sh delete mode 100644 integ/failure-server/toxiproxy/setup.sh delete mode 100644 integ/failure-server/toxy/.gitignore delete mode 100644 integ/failure-server/toxy/index.js delete mode 100644 integ/failure-server/toxy/package.json diff --git a/Cargo.lock b/Cargo.lock index ff717488..28befb0a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -50,6 +50,12 @@ dependencies = [ "libc", ] +[[package]] +name = "anyhow" +version = "1.0.75" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" + [[package]] name = "assert-json-diff" version = "1.1.0" @@ -75,6 +81,17 @@ dependencies = [ "wait-timeout", ] +[[package]] +name = "async-trait" +version = "0.1.73" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc00ceb34980c03614e35a3a4e218276a0a824e911d07651cd0d858a51e8c0f0" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.28", +] + [[package]] name = "atty" version = "0.2.14" @@ -362,7 +379,7 @@ dependencies = [ "pin-project-lite", "pin-utils", "tokio", - "tokio-util", + "tokio-util 0.7.8", "tracing", ] @@ -454,6 +471,55 @@ dependencies = [ "tracing", ] +[[package]] +name = "axum" +version = "0.6.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b829e4e32b91e643de6eafe82b1d90675f5874230191a4ffbc1b336dec4d6bf" +dependencies = [ + "async-trait", + "axum-core", + "bitflags 1.3.2", + "bytes", + "futures-util", + "http", + "http-body", + "hyper", + "itoa", + "matchit", + "memchr", + "mime", + "percent-encoding", + "pin-project-lite", + "rustversion", + "serde", + "serde_json", + "serde_path_to_error", + "serde_urlencoded", + "sync_wrapper", + "tokio", + "tower", + "tower-layer", + "tower-service", +] + +[[package]] +name = "axum-core" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "759fa577a247914fd3f7f76d62972792636412fbfd634cd452f6a385a74d2d2c" +dependencies = [ + "async-trait", + "bytes", + "futures-util", + "http", + "http-body", + "mime", + "rustversion", + "tower-layer", + "tower-service", +] + [[package]] name = "backtrace" version = "0.3.68" @@ -512,6 +578,17 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bmrng" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9758e48498ae13d49b51a979d553d254e67021b203d9597e82a04ebd81025b2" +dependencies = [ + "futures", + "loom", + "tokio", +] + [[package]] name = "bstr" version = "0.2.17" @@ -779,6 +856,22 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "failure-server" +version = "0.1.0" +dependencies = [ + "anyhow", + "axum", + "noxious-client", + "rand", + "serde_json", + "tempfile", + "tokio", + "tower", + "tower-fault", + "tower-http", +] + [[package]] name = "fastrand" version = "1.9.0" @@ -832,6 +925,7 @@ checksum = "23342abe12aba583913b2e62f22225ff9c950774065e4bfb61a19cd9770fec40" dependencies = [ "futures-channel", "futures-core", + "futures-executor", "futures-io", "futures-sink", "futures-task", @@ -854,6 +948,17 @@ version = "0.3.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4bca583b7e26f571124fe5b7561d49cb2868d79116cfa0eefce955557c6fee8c" +[[package]] +name = "futures-executor" +version = "0.3.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccecee823288125bd88b4d7f565c9e58e41858e47ab72e8ea2d64e93624386e0" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.28" @@ -901,6 +1006,19 @@ dependencies = [ "slab", ] +[[package]] +name = "generator" +version = "0.6.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "061d3be1afec479d56fa3bd182bf966c7999ec175fcfdb87ac14d417241366c6" +dependencies = [ + "cc", + "libc", + "log", + "rustversion", + "winapi", +] + [[package]] name = "generic-array" version = "0.14.7" @@ -911,6 +1029,17 @@ dependencies = [ "version_check", ] +[[package]] +name = "getrandom" +version = "0.2.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be4136b2a15dd319360be1c07d9933517ccf0be8f16bf62a3bee4f0d618df427" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + [[package]] name = "gimli" version = "0.27.3" @@ -945,7 +1074,7 @@ dependencies = [ "indexmap", "slab", "tokio", - "tokio-util", + "tokio-util 0.7.8", "tracing", ] @@ -1019,6 +1148,12 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "http-range-header" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "add0ab9360ddbd88cfeb3bd9574a1d85cfdfa14db10b3e21d3700dbc4328758f" + [[package]] name = "httparse" version = "1.8.0" @@ -1221,12 +1356,32 @@ version = "0.4.19" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b06a4cde4c0f271a446782e3eff8de789548ce57dbc8eca9292c27f4a42004b4" +[[package]] +name = "loom" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "27a6650b2f722ae8c0e2ebc46d07f80c9923464fc206d962332f1eff83143530" +dependencies = [ + "cfg-if", + "futures-util", + "generator", + "scoped-tls", + "serde", + "serde_json", +] + [[package]] name = "maplit" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d" +[[package]] +name = "matchit" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed1202b2a6f884ae56f04cff409ab315c5ce26b5e58d7412e484f01fd52f52ef" + [[package]] name = "memchr" version = "2.5.0" @@ -1248,6 +1403,16 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "mime_guess" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4192263c238a5f0d0c6bfd21f336a313a4ce1c450542449ca191bb657b4642ef" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "miniz_oxide" version = "0.7.1" @@ -1268,6 +1433,18 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "mockall_double" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dffc15b97456ecc84d2bde8c1df79145e154f45225828c4361f676e1b82acd6" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "native-tls" version = "0.2.11" @@ -1286,6 +1463,39 @@ dependencies = [ "tempfile", ] +[[package]] +name = "noxious" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e68998924150ba54dbf1adf4c3f7f7c10bb5d3c6789ab71af11e34fe4c667970" +dependencies = [ + "async-trait", + "bmrng", + "bytes", + "futures", + "mockall_double", + "pin-project-lite", + "rand", + "serde", + "thiserror", + "tokio", + "tokio-util 0.6.10", + "tracing", +] + +[[package]] +name = "noxious-client" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b7ab7a9efb5768cd07e2b2455f80b3998d7397be76398c2ac03a52a42b652e7" +dependencies = [ + "noxious", + "reqwest", + "serde", + "thiserror", + "tokio", +] + [[package]] name = "num-integer" version = "0.1.45" @@ -1427,6 +1637,12 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "paste" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" + [[package]] name = "pem" version = "1.1.1" @@ -1480,6 +1696,12 @@ version = "0.3.27" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26072860ba924cbfa98ea39c8c19b4dd6a4a25423dbdf219c1eca91aa0cf6964" +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + [[package]] name = "predicates" version = "2.1.5" @@ -1559,6 +1781,36 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "rayon" version = "1.7.0" @@ -1629,10 +1881,12 @@ dependencies = [ "http-body", "hyper", "hyper-rustls", + "hyper-tls", "ipnet", "js-sys", "log", "mime", + "native-tls", "once_cell", "percent-encoding", "pin-project-lite", @@ -1642,6 +1896,7 @@ dependencies = [ "serde_json", "serde_urlencoded", "tokio", + "tokio-native-tls", "tokio-rustls", "tower-service", "url", @@ -1737,6 +1992,12 @@ dependencies = [ "base64 0.21.2", ] +[[package]] +name = "rustversion" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ffc183a10b4478d04cbbbfc96d0873219d962dd5accaff2ffbd4ceb7df837f4" + [[package]] name = "ryu" version = "1.0.15" @@ -1761,6 +2022,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "scoped-tls" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1cf6437eb19a8f4a6cc0f7dca544973b0b78843adbfeb3683d1a94a0024a294" + [[package]] name = "scopeguard" version = "1.2.0" @@ -1837,6 +2104,16 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_path_to_error" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4beec8bce849d58d06238cb50db2e1c417cfeafa4c63f692b15c82b7c80f8335" +dependencies = [ + "itoa", + "serde", +] + [[package]] name = "serde_plain" version = "1.0.1" @@ -1977,11 +2254,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "sync_wrapper" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" + [[package]] name = "tempfile" -version = "3.7.0" +version = "3.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5486094ee78b2e5038a6382ed7645bc084dc2ec433426ca4c3cb61e2007b8998" +checksum = "cb94d2f3cc536af71caac6b6fcebf65860b347e7ce0cc9ebe8f70d3e521054ef" dependencies = [ "cfg-if", "fastrand 2.0.0", @@ -2139,6 +2422,20 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-util" +version = "0.6.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "36943ee01a6d67977dd3f84a5a1d2efeb4ada3a1ae771cadfaa535d9d9fc6507" +dependencies = [ + "bytes", + "futures-core", + "futures-sink", + "log", + "pin-project-lite", + "tokio", +] + [[package]] name = "tokio-util" version = "0.7.8" @@ -2159,6 +2456,7 @@ version = "0.14.0" dependencies = [ "chrono", "dyn-clone", + "failure-server", "globset", "hex", "hex-literal", @@ -2175,6 +2473,7 @@ dependencies = [ "serde_plain", "snafu", "tempfile", + "tokio", "typed-path", "untrusted", "url", @@ -2228,6 +2527,43 @@ dependencies = [ "tracing", ] +[[package]] +name = "tower-fault" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e91fcf38cbb8c7c68e8fe245437c64e654a2db6004318c5d893d01b5d9210bb8" +dependencies = [ + "paste", + "rand", + "tokio", + "tower", +] + +[[package]] +name = "tower-http" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55ae70283aba8d2a8b411c695c437fe25b8b5e44e23e780662002fc72fb47a82" +dependencies = [ + "bitflags 2.3.3", + "bytes", + "futures-core", + "futures-util", + "http", + "http-body", + "http-range-header", + "httpdate", + "mime", + "mime_guess", + "percent-encoding", + "pin-project-lite", + "tokio", + "tokio-util 0.7.8", + "tower-layer", + "tower-service", + "tracing", +] + [[package]] name = "tower-layer" version = "0.3.2" @@ -2321,6 +2657,15 @@ version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "497961ef93d974e23eb6f433eb5fe1b7930b659f06d12dec6fc44a8f554c0bba" +[[package]] +name = "unicase" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7d2d4dafb69621809a81864c9c1b864479e1235c0dd4e199924b9742439ed89" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-bidi" version = "0.3.13" diff --git a/Makefile b/Makefile index 087c4763..706ad8a5 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # Use directory-local cargo root to install version-specific executables into. export CARGO_HOME = $(shell pwd)/.cargo -# the series of builds, tests and checks that runs for pull requests. requires docker. +# the series of builds, tests and checks that runs for pull requests. .PHONY: ci ci: check-licenses build integ @@ -28,9 +28,18 @@ build: cargo build --locked -p tuftool cargo test --locked -# checks tough tests with and without the http feature. http testing requires docker. + +# installs noxious-server +# We currently build from a forked version, until such a point that the following are resolved: +# https://github.com/oguzbilgener/noxious/issues/13 +# https://github.com/oguzbilgener/noxious/pull/14 +.PHONY: noxious +noxious: + cargo install --locked --git https://github.com/cbgbt/noxious.git --tag v1.0.5 + +# checks tough tests with and without the http feature. http testing requires noxious-server. .PHONY: integ -integ: +integ: noxious set +e - cd tough && cargo test --features '' --locked - cd tough && cargo test --all-features --locked + cargo test --manifest-path tough/Cargo.toml --features '' --locked + cargo test --manifest-path tough/Cargo.toml --all-features --locked diff --git a/README.md b/README.md index 904bc4f1..3741bf82 100644 --- a/README.md +++ b/README.md @@ -5,14 +5,7 @@ **tuftool** is a Rust command-line utility for generating and signing TUF repositories. ## Integration Testing -Integration tests require `docker`. - -### Windowsâť— Warnings -- Tests can break on Windows if Git's `autocrlf` feature changes line endings. - This is due to the fact that some tests require files to have a *precise* byte size and hash signature. - *We have mitigated this with a `.gitattributes` file in the test data directory*. - -- Cygwin **must** be installed at `C:\cygwin64\` and have the `make` package installed for integration tests to work properly. +Integration tests require, `noxious`, which is installed when running `make integ`. ## Documentation See [tough - Rust](https://docs.rs/tough/) for the latest `tough` library documentation. diff --git a/deny.toml b/deny.toml index 2db6933d..02d8d2b6 100644 --- a/deny.toml +++ b/deny.toml @@ -73,6 +73,8 @@ skip = [ { name = "fastrand", version = "=1.9" }, # several dependencies are using an old version of bitflags { name = "bitflags", version = "=1.3" }, + # noxious, used for testing, is using an old version of tokio-util + { name = "tokio-util", version = "=0.6.10" }, ] skip-tree = [ diff --git a/integ/failure-server/.gitattributes b/integ/failure-server/.gitattributes deleted file mode 100644 index 8a6231a3..00000000 --- a/integ/failure-server/.gitattributes +++ /dev/null @@ -1,4 +0,0 @@ -# Denote bash scripts as binary so they will not be modified by autocrlf. -# (Bash doens't seem to like CRLF-encoded scripts | Just to be sure) -run.sh binary -teardown.sh binary diff --git a/integ/failure-server/Cargo.toml b/integ/failure-server/Cargo.toml new file mode 100644 index 00000000..1ad9550a --- /dev/null +++ b/integ/failure-server/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "failure-server" +version = "0.1.0" +edition = "2021" +license = "MIT OR Apache-2.0" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +axum = "0.6" +anyhow = "1.0" +noxious-client = "1.0" +rand = "0.8" +serde_json = "1.0" +tempfile = "3.8" +tokio = "1.0" +tower = { version = "0.4", features = ["util"] } +tower-fault = "0.0.5" +tower-http = { version = "0.4", features = ["fs"] } diff --git a/integ/failure-server/Dockerfile.toxiproxycli b/integ/failure-server/Dockerfile.toxiproxycli deleted file mode 100644 index 649424fd..00000000 --- a/integ/failure-server/Dockerfile.toxiproxycli +++ /dev/null @@ -1,6 +0,0 @@ -# a one-shot process that sets up toxiproxy for our test -FROM shopify/toxiproxy:2.1.4 -ENV PATH "/go/bin:${PATH}" -COPY ./toxiproxy/setup.sh ./ -RUN chmod +x ./setup.sh -ENTRYPOINT ./setup.sh diff --git a/integ/failure-server/Dockerfile.toxy b/integ/failure-server/Dockerfile.toxy deleted file mode 100644 index 6ef14746..00000000 --- a/integ/failure-server/Dockerfile.toxy +++ /dev/null @@ -1,4 +0,0 @@ -FROM node:12-stretch -COPY ./toxy/index.js ./toxy/package.json ./ -RUN yarn -ENTRYPOINT yarn start diff --git a/integ/failure-server/run.sh b/integ/failure-server/run.sh deleted file mode 100755 index eb28d16d..00000000 --- a/integ/failure-server/run.sh +++ /dev/null @@ -1,90 +0,0 @@ -#!/usr/bin/env bash -set -eo pipefail - -# this script sets up three http servers. -# * fileserver: serves TUF repo files on port 10101. -# * toxiproxy: serves as a proxy to fileserver on port 10102. introduces mid-response aborts. -# * toxy: serves as a proxy to toxiproxy on port 10103. introduces 5XX failures. -# -# port 10103 is bound on the host so that the 'toxic' TUF repo can be found at: -# * http://localhost:10103/metadata -# * http://localhost:10103/targets - -# get the directory where this script is located -DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)" -TUF_REFERENCE_REPO="${DIR}/../../tough/tests/data/tuf-reference-impl" - -# if we are under cygwin , set a windows style path -# (docker for windows requires windows style paths to work ,it doesn't understand cygwin-style paths) -if [[ "$OSTYPE" == "cygwin" ]]; then - DIR="$(cygpath --windows ${DIR})" - TUF_REFERENCE_REPO="$(cygpath --windows ${TUF_REFERENCE_REPO})" -fi - - -function waitforit() { - echo "waiting $1 seconds for $2 to start" - sleep $1 -} - -# dismantle everything and force rebuilds by deleting images -"${DIR}/teardown.sh" - -# rebuild the toxiproxy image -docker build -f "${DIR}/Dockerfile.toxiproxycli" \ - -t toxiproxy_cli_img:latest \ - "${DIR}" - -# rebuild the toxi image -docker build -f "${DIR}/Dockerfile.toxy" \ - -t toxy_srv_img:latest \ - "${DIR}" - -# create a shared network -docker network create \ - --driver=bridge \ - tough_test_network - -# a fileserver that is serving the tuf reference impl repo on port 10101 -docker run -d \ - -v "${TUF_REFERENCE_REPO}/targets:/content/targets" \ - -v "${TUF_REFERENCE_REPO}/metadata:/content/metadata" \ - -e FOLDER=/content \ - -e SHOW_LISTING=true \ - -e PORT="10101" \ - --expose "10101" \ - --network tough_test_network \ - --name tuf_srv_ctr \ - --network-alias "fileserver" \ - halverneus/static-file-server:latest -waitforit 1 fileserver - -# start the toxiproxy server that will provide mid-response aborts - points to the fileserver -# this container will serve http on 10102. the service can be controlled at 8472. -docker run -d \ - --expose "8474" \ - --expose "10102" \ - --name toxiproxy_srv_ctr \ - --network tough_test_network \ - --network-alias "toxiproxy" \ - shopify/toxiproxy:2.1.4 -waitforit 1 toxiproxy - -# run a one-shot container that sets up the toxiproxy with http calls -docker run \ - --name toxiproxy_cli_ctr \ - --network tough_test_network \ - toxiproxy_cli_img - -# run another server 'in front' of toxiproxy, this one will return occasional 503's -# and occasionally abort with no data at all. serves on port 10103 which is bound -# to the host. -docker run -d \ - -p "10103:3000" \ - --name toxy_srv_ctr \ - --network tough_test_network \ - toxy_srv_img:latest -waitforit 1 toxy - -echo "**********************************************************************" -echo "the toxic tuf repo is available at http://localhost:10103" diff --git a/integ/failure-server/src/lib.rs b/integ/failure-server/src/lib.rs new file mode 100644 index 00000000..3b80c8f2 --- /dev/null +++ b/integ/failure-server/src/lib.rs @@ -0,0 +1,80 @@ +//! This module sets up 2 HTTP servers. +//! * ToxicStaticHttpServer: serves TUF repo files on port 10101, with occasional random 503s. +//! * ToxicTcpProxy: proxies to the TUF repo on port 10102, with occasional toxic behavior. +use anyhow::Result; +use noxious_client::{StreamDirection, Toxic, ToxicKind}; +use std::path::Path; +use std::thread::sleep; +use std::time::Duration; +use toxic::{ToxicStaticHttpServer, ToxicTcpProxy}; + +mod toxic; + +const STATIC_HTTP_SERVER_LISTEN: &str = "127.0.0.1:10101"; +const TCP_PROXY_LISTEN: &str = "127.0.0.1:10102"; +const TCP_PROXY_CONFIG_API_LISTEN: &str = "127.0.0.1:8472"; + +pub struct IntegServers { + toxic_tcp_proxy: ToxicTcpProxy, + toxic_static_http_server: ToxicStaticHttpServer, +} + +impl IntegServers { + pub fn new>(tuf_reference_repo: P) -> Result { + let tuf_reference_repo = tuf_reference_repo.as_ref().to_owned(); + + let toxic_tcp_proxy = ToxicTcpProxy::new( + "toxictuf".to_string(), + TCP_PROXY_LISTEN, + STATIC_HTTP_SERVER_LISTEN, + TCP_PROXY_CONFIG_API_LISTEN, + )? + .with_toxic(Toxic { + name: "slowclose".to_string(), + kind: ToxicKind::SlowClose { delay: 500 }, + toxicity: 0.75, + direction: StreamDirection::Downstream, + }) + .with_toxic(Toxic { + name: "timeout".to_string(), + kind: ToxicKind::Timeout { timeout: 100 }, + toxicity: 0.5, + direction: StreamDirection::Downstream, + }); + + let toxic_static_http_server = + ToxicStaticHttpServer::new(STATIC_HTTP_SERVER_LISTEN, tuf_reference_repo)?; + + Ok(Self { + toxic_tcp_proxy, + toxic_static_http_server, + }) + } + + pub async fn run(&mut self) -> Result<()> { + // Make sure we're starting from scratch + self.teardown()?; + + self.toxic_static_http_server.start()?; + self.toxic_tcp_proxy.start().await?; + sleep(Duration::from_secs(1)); // give the servers a chance to start + + println!("**********************************************************************"); + println!("the toxic tuf repo is available at {TCP_PROXY_LISTEN}"); + + Ok(()) + } + + pub fn teardown(&mut self) -> Result<()> { + self.toxic_tcp_proxy.stop()?; + self.toxic_static_http_server.stop()?; + + Ok(()) + } +} + +impl Drop for IntegServers { + fn drop(&mut self) { + self.teardown().ok(); + } +} diff --git a/integ/failure-server/src/toxic/http_server.rs b/integ/failure-server/src/toxic/http_server.rs new file mode 100644 index 00000000..091c6447 --- /dev/null +++ b/integ/failure-server/src/toxic/http_server.rs @@ -0,0 +1,96 @@ +//! A simple filesystem HTTP server that introduces chaos at the HTTP layer. +//! +//! Chaos includes: +//! * Occasional additional request latency +//! * Occasional 503 responses +use super::ToSocketAddrsExt; +use anyhow::{Context, Result}; +use axum::{ + http::{Request, StatusCode}, + middleware::{self, Next}, + response::Response, + Router, +}; +use std::fmt::Debug; +use std::net::{SocketAddr, ToSocketAddrs}; +use std::path::{Path, PathBuf}; +use tower_fault::latency::LatencyLayer; +use tower_http::services::ServeDir; + +const ERR_503_PROBABILITY: f64 = 0.5; +const LATENCY_PROBABILITY: f64 = 0.1; + +/// An HTTP server which serves static files from a directory. +/// +/// The server implementation is "toxic" in that it introduces artificial faults at the HTTP layer. +#[derive(Debug)] +pub(crate) struct ToxicStaticHttpServer { + /// The proxy's listen address. Written to `ProxyConfig`. + listen: SocketAddr, + + /// The path to serve static content from. + serve_dir: PathBuf, + + /// Running server, if any + running_server: Option>>, +} + +impl ToxicStaticHttpServer { + pub(crate) fn new(listen: T, serve_dir: P) -> Result + where + T: ToSocketAddrs + Debug, + P: AsRef, + { + let listen = listen.parse_only_one_address()?; + let serve_dir = serve_dir.as_ref().to_owned(); + let running_server = None; + + Ok(Self { + listen, + serve_dir, + running_server, + }) + } + + /// Starts the HTTP server. + pub(crate) fn start(&mut self) -> Result<()> { + // Stop any existing server + self.stop().ok(); + + // Chance to inject 50 to 200 milliseconds of latency + let latency_layer = LatencyLayer::new(LATENCY_PROBABILITY, 50..200); + // Chance to return an HTTP 503 error + let error_layer = middleware::from_fn(maybe_return_error); + + let app = Router::new() + .nest_service("/", ServeDir::new(&self.serve_dir)) + .layer(error_layer) + .layer(latency_layer); + let server = axum::Server::bind(&self.listen).serve(app.into_make_service()); + + self.running_server = Some(tokio::spawn(async { + server.await.context("Failed to run ToxicStaticHttpServer") + })); + + Ok(()) + } + + /// Attempts to kill the running server, if there is one. + /// + /// Succeeds if the server is killed successfully or if it isn't/was never running. + pub(crate) fn stop(&mut self) -> Result<()> { + if let Some(server) = self.running_server.take() { + server.abort(); + } + Ok(()) + } +} + +/// Middleware for chaotically returning a 503 error. +async fn maybe_return_error(req: Request, next: Next) -> Result { + if rand::random::() < ERR_503_PROBABILITY { + Err(StatusCode::SERVICE_UNAVAILABLE) + } else { + Ok(next.run(req).await) + } +} diff --git a/integ/failure-server/src/toxic/mod.rs b/integ/failure-server/src/toxic/mod.rs new file mode 100644 index 00000000..3728ff39 --- /dev/null +++ b/integ/failure-server/src/toxic/mod.rs @@ -0,0 +1,35 @@ +use anyhow::{Context, Result}; +use std::fmt::Debug; +use std::net::{SocketAddr, ToSocketAddrs}; + +pub(crate) use http_server::ToxicStaticHttpServer; +pub(crate) use tcp_proxy::ToxicTcpProxy; + +mod http_server; +mod tcp_proxy; + +/// Attempts to read exactly one `SocketAddr` from a `ToSocketAddrs`. +/// +/// Returns an error if more than one SocketAddr is present. +trait ToSocketAddrsExt { + fn parse_only_one_address(self) -> Result; +} + +impl ToSocketAddrsExt for T { + fn parse_only_one_address(self) -> Result { + let mut addresses = self + .to_socket_addrs() + .context(format!("Failed to parse {self:?} as socket address"))?; + + let address = addresses + .next() + .context(format!("Did not parse any addresses from {self:?}"))?; + + anyhow::ensure!( + addresses.next().is_none(), + format!("Listen address ({:?}) must parse to one address.", address) + ); + + Ok(address) + } +} diff --git a/integ/failure-server/src/toxic/tcp_proxy.rs b/integ/failure-server/src/toxic/tcp_proxy.rs new file mode 100644 index 00000000..265cfaa0 --- /dev/null +++ b/integ/failure-server/src/toxic/tcp_proxy.rs @@ -0,0 +1,127 @@ +//! Wrapper around running [`noxious-server`](https://github.com/oguzbilgener/noxious) +//! +//! `noxious-server` is a TCP proxy that introduces chaos at the TCP layer. +use super::ToSocketAddrsExt; +use anyhow::{Context, Result}; +use noxious_client::{Client, Toxic}; +use std::net::ToSocketAddrs; +use std::process::Command; +use std::{fmt::Debug, net::SocketAddr}; +use tempfile::NamedTempFile; + +/// A TCP proxy server that introduces artificial faults at the TCP layer. +#[derive(Debug)] +pub(crate) struct ToxicTcpProxy { + /// The name of the noxious proxy. Written to `ProxyConfig`. + name: String, + /// The proxy's listen address. Written to `ProxyConfig`. + listen: SocketAddr, + /// The upstream's listen address. Written to `ProxyConfig`. + upstream: SocketAddr, + /// The proxy's control API address. + api_listen: SocketAddr, + /// The running server process. + running_server: Option, + /// The list of toxics to apply to connections. + toxics: Vec, +} + +impl ToxicTcpProxy { + pub(crate) fn new( + name: String, + listen: T1, + upstream: T2, + api_listen: T3, + ) -> Result + where + T1: ToSocketAddrs + Debug, + T2: ToSocketAddrs + Debug, + T3: ToSocketAddrs + Debug, + { + let listen = listen.parse_only_one_address()?; + let upstream = upstream.parse_only_one_address()?; + let api_listen = api_listen.parse_only_one_address()?; + let running_server = None; + let toxics = Vec::new(); + + Ok(Self { + name, + listen, + upstream, + api_listen, + running_server, + toxics, + }) + } + + pub(crate) fn with_toxic(mut self, toxic: Toxic) -> Self { + self.toxics.push(toxic); + self + } + + /// Starts the noxious-server. + /// + /// If the server is already running, it will be restarted. + pub(crate) async fn start(&mut self) -> Result<()> { + // Stop any existing server + self.stop().ok(); + + // Configure and start the server + let proxy_config = serde_json::json!([{ + "name": &self.name, + "listen": self.listen.to_string(), + "upstream": self.upstream.to_string(), + }]); + + let config_tmpfile = + NamedTempFile::new().context("Failed to create tmpfile for noxious proxy config")?; + serde_json::to_writer(&config_tmpfile, &proxy_config) + .context("Failed to write proxy config file for noxious")?; + + #[rustfmt::skip] + let noxious_process = Command::new("noxious-server") + .args([ + "--config", &config_tmpfile.path().to_string_lossy(), + "--host", &self.api_listen.ip().to_string(), + "--port", &self.api_listen.port().to_string(), + ]) + .spawn() + .context("Failed to start noxious server")?; + + self.running_server = Some(noxious_process); + + // Configure toxics + let client = Client::new(&self.api_listen.to_string()); + let proxy = client.proxy(&self.name).await.context(format!( + "Failed to find our configured proxy '{}'", + self.name + ))?; + for toxic in &self.toxics { + proxy.add_toxic(toxic).await.context(format!( + "Failed to apply toxic {:?} to proxy '{}'", + toxic, self.name + ))?; + } + + Ok(()) + } + + /// Attempts to kill the running server, if there is one. + /// + /// Succeeds if the server is killed successfully or if it isn't/was never running. + pub(crate) fn stop(&mut self) -> Result<()> { + self.running_server + .as_mut() + .map(std::process::Child::kill) + .transpose() + .context("Failed to kill noxious server.")?; + self.running_server = None; + Ok(()) + } +} + +impl Drop for ToxicTcpProxy { + fn drop(&mut self) { + self.stop().ok(); + } +} diff --git a/integ/failure-server/teardown.sh b/integ/failure-server/teardown.sh deleted file mode 100755 index 26afdb15..00000000 --- a/integ/failure-server/teardown.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/env bash -set -e - -# delete everything in the right order. errors are ignored so that this -# script can be used whether or not all of the artifacts exist. -docker stop tuf_srv_ctr || true -docker stop toxiproxy_srv_ctr || true -docker stop toxiproxy_cli_ctr || true -docker rm -f tuf_srv_ctr || true -docker rm -f toxiproxy_srv_ctr || true -docker rm -f toxiproxy_cli_ctr || true -docker rm -f toxy_srv_ctr || true -docker network rm tough_test_network || true -docker rmi -f toxiproxy_cli_img || true -docker rmi -f toxy_srv_img || true diff --git a/integ/failure-server/toxiproxy/setup.sh b/integ/failure-server/toxiproxy/setup.sh deleted file mode 100644 index 0d1e7c6f..00000000 --- a/integ/failure-server/toxiproxy/setup.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env sh -set -eo - -# sets up toxiproxy for our test with aborts and timeouts - -# create the proxy connection to the tuf fileserver -toxiproxy-cli --host http://toxiproxy:8474 \ - create tuf \ - --listen toxiproxy:10102 \ - --upstream fileserver:10101 - -# add an abort failure -toxiproxy-cli --host http://toxiproxy:8474 \ - toxic add tuf \ - --toxicName abort \ - --type limit_data \ - --toxicity 0.75 \ - --attribute bytes=500 \ - --downstream - -# add a timeout failure (timeout is in milliseconds) -toxiproxy-cli --host http://toxiproxy:8474 \ - toxic add tuf \ - --toxicName timeout \ - --type timeout \ - --toxicity 0.5 \ - --attribute timeout=100 \ - --downstream - -# list the setup -toxiproxy-cli --host http://toxiproxy:8474 \ - list diff --git a/integ/failure-server/toxy/.gitignore b/integ/failure-server/toxy/.gitignore deleted file mode 100644 index 3c3629e6..00000000 --- a/integ/failure-server/toxy/.gitignore +++ /dev/null @@ -1 +0,0 @@ -node_modules diff --git a/integ/failure-server/toxy/index.js b/integ/failure-server/toxy/index.js deleted file mode 100644 index 2913fe65..00000000 --- a/integ/failure-server/toxy/index.js +++ /dev/null @@ -1,29 +0,0 @@ -const toxy = require('toxy') -const poisons = toxy.poisons -const rules = toxy.rules - -// Create a new toxy proxy -const proxy = toxy() -proxy - .forward('http://toxiproxy:10102') - -// Register poisons and rules -proxy - .get('/metadata/snapshot.json') - .poison(poisons.abort({delay: 100})) - .rule(rules.probability(50)) - -proxy - .get('/metadata/targets.json') - .poison(poisons.inject({ - code: 503, - body: '{"error": "toxy injected error"}', - headers: {'Content-Type': 'application/json'} - })) - .rule(rules.probability(50)) - -proxy - .get('/*') - -proxy.listen(3000) -console.log('Server listening on port:', 3000) diff --git a/integ/failure-server/toxy/package.json b/integ/failure-server/toxy/package.json deleted file mode 100644 index caa78108..00000000 --- a/integ/failure-server/toxy/package.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "name": "toxic-server", - "version": "1.0.0", - "description": "a server for testing tough retry mechanisms", - "main": "index.js", - "license": "MIT", - "scripts": { - "start":"node index.js" - }, - "dependencies": { - "toxy": "^0.3.16" - } -} diff --git a/tough/Cargo.toml b/tough/Cargo.toml index e7cbb75f..de4a7ef0 100644 --- a/tough/Cargo.toml +++ b/tough/Cargo.toml @@ -30,12 +30,14 @@ url = "2" walkdir = "2" [dev-dependencies] +failure-server = { path = "../integ/failure-server", version = "0.1.0" } hex-literal = "0.4" httptest = "0.15" maplit = "1" +tokio = { version = "1.0", features = ["rt-multi-thread"] } [features] http = ["reqwest"] -# The `integ` feature enables integration tests. These tests require docker to be running on the host. +# The `integ` feature enables integration tests. These tests require `noxious-server` to be installed on the host. integ = [] diff --git a/tough/README.md b/tough/README.md index d47625d9..5aa0ddac 100644 --- a/tough/README.md +++ b/tough/README.md @@ -5,5 +5,5 @@ For more information see the documentation and [the repository](https://github.c ## Testing Unit tests are run in the usual manner: `cargo test`. -Integration tests require docker and are disabled by default behind a feature named `integ`. +Integration tests require `noxious-server` and are disabled by default behind a feature named `integ`. To run all tests, including integration tests: `cargo test --all-features` or `cargo test --features 'http,integ'`. diff --git a/tough/src/lib.rs b/tough/src/lib.rs index 9be298af..38e85d02 100644 --- a/tough/src/lib.rs +++ b/tough/src/lib.rs @@ -14,7 +14,7 @@ //! # Testing //! //! Unit tests are run in the usual manner: `cargo test`. -//! Integration tests require docker and are disabled by default behind a feature named `integ`. +//! Integration tests require `noxious-server` and are disabled by default behind a feature named `integ`. //! To run all tests, including integration tests: `cargo test --all-features` or //! `cargo test --features 'http,integ'`. diff --git a/tough/tests/http.rs b/tough/tests/http.rs index 888c08db..1f83d055 100644 --- a/tough/tests/http.rs +++ b/tough/tests/http.rs @@ -95,31 +95,12 @@ mod http_happy { #[cfg(feature = "integ")] mod http_integ { use crate::test_utils::test_data; + use failure_server::IntegServers; use std::fs::File; use std::path::PathBuf; - use std::process::{Command, Stdio}; use tough::{HttpTransportBuilder, RepositoryLoader}; use url::Url; - pub fn integ_dir() -> PathBuf { - let mut p = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - p.pop(); - p = p.join("integ"); - p - } - - /// Returns a command object that runs the provided script under BASH, whether we are under cygwin or unix. - pub fn bash_base() -> Command { - // if under cygwin, run the bash script under cygwin64 bash - if cfg!(target_os = "windows") { - let mut command = Command::new("c:\\cygwin64\\bin\\bash"); - command.arg("-l"); - return command; - } else { - return Command::new("bash"); - } - } - pub fn tuf_reference_impl() -> PathBuf { test_data().join("tuf-reference-impl") } @@ -134,73 +115,53 @@ mod http_integ { /// Test `tough` using faulty HTTP connections. /// - /// This test requires `docker` and should be disabled for PRs because it will not work with our - /// current CI setup. It works by starting HTTP services in containers which serve the tuf- - /// reference-impl through fault-ridden HTTP. We load the repo many times in a loop, and - /// statistically exercise many of the retry code paths. In particular, the server aborts during - /// the send which exercises the range-header retry in the `Read` loop, and 5XX's are also sent - /// triggering retries in the `fetch` loop. - #[test] - fn test_retries() { - use std::ffi::OsString; - // run docker images to create a faulty http representation of tuf-reference-impl - - // Get the "run.sh" path - let script_path = integ_dir() - .join("failure-server") - .join("run.sh") - .into_os_string() - .into_string() - .unwrap(); - - // Run it under BASH - let output = bash_base() - .arg(OsString::from(script_path)) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .output() - .expect("failed to start server with docker containers"); - - if !output.status.success() { - panic!("Failed to run integration test HTTP servers, is docker running?"); - } - - // load the tuf-reference-impl repo via http repeatedly through faulty proxies - for i in 0..5 { - let transport = HttpTransportBuilder::new() - // the service we have created is very toxic with many failures, so we will do a - // large number of retries, enough that we can be reasonably assured that we will - // always succeed. - .tries(200) - // we don't want the test to take forever so we use small pauses - .initial_backoff(std::time::Duration::from_nanos(100)) - .max_backoff(std::time::Duration::from_millis(1)) - .build(); - let root_path = tuf_reference_impl_root_json(); - - RepositoryLoader::new( - File::open(&root_path).unwrap(), - Url::parse("http://localhost:10103/metadata").unwrap(), - Url::parse("http://localhost:10103/targets").unwrap(), - ) - .transport(transport) - .load() - .unwrap(); - println!("{}:{} SUCCESSFULLY LOADED THE REPO {}", file!(), line!(), i,); - } - - // stop and delete the docker containers, images and network - let output = bash_base() - .arg( - integ_dir() - .join("failure-server") - .join("teardown.sh") - .into_os_string(), - ) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .output() - .expect("failed to delete docker objects"); - assert!(output.status.success()); + /// This works by starting HTTP services which serve the tuf-reference-impl through fault-ridden + /// HTTP. We load the repo many times in a loop, and statistically exercise many of the retry + /// code paths. In particular, the server aborts during the send which exercises the + /// range-header retry in the `Read` loop, and 5XX's are also sent triggering retries in the + /// `fetch` loop. + #[tokio::test] + async fn test_retries() { + // create a faulty http representation of tuf-reference-impl + let tuf_reference_path = tuf_reference_impl(); + let mut integ_servers = IntegServers::new(tuf_reference_path).unwrap(); + integ_servers + .run() + .await + .expect("Failed to run integration test HTTP servers"); + + // Load the tuf-reference-impl repo via http repeatedly through faulty proxies. + // We avoid nested tokio runtimes from `reqwest::blocking` by sequestering it to another + // thread in a blocking task. + tokio::task::spawn_blocking(move || { + for i in 0..5 { + let transport = HttpTransportBuilder::new() + // the service we have created is very toxic with many failures, so we will do a + // large number of retries, enough that we can be reasonably assured that we + // will always succeed. + .tries(200) + // we don't want the test to take forever so we use small pauses + .initial_backoff(std::time::Duration::from_nanos(100)) + .max_backoff(std::time::Duration::from_millis(1)) + .build(); + let root_path = tuf_reference_impl_root_json(); + + RepositoryLoader::new( + File::open(&root_path).unwrap(), + Url::parse("http://localhost:10102/metadata").unwrap(), + Url::parse("http://localhost:10102/targets").unwrap(), + ) + .transport(transport) + .load() + .unwrap(); + println!("{}:{} SUCCESSFULLY LOADED THE REPO {}", file!(), line!(), i,); + } + }) + .await + .expect("Failed to load the repo through faulty proxies"); + + integ_servers + .teardown() + .expect("failed to stop HTTP servers"); } } From c4bfd0d73d9f7a327c553b300c5faee857ebd4a9 Mon Sep 17 00:00:00 2001 From: "Sean P. Kelly" Date: Wed, 23 Aug 2023 20:55:19 +0000 Subject: [PATCH 4/5] github: run CI on macos --- .github/workflows/rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 551a3f36..7de42c36 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -16,7 +16,7 @@ jobs: fail-fast: false matrix: make_target: ["check-licenses", "build", "integ"] - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 From a18164be4a435b42b2b5bca087c510d4b137527a Mon Sep 17 00:00:00 2001 From: "Sean P. Kelly" Date: Wed, 23 Aug 2023 21:10:23 +0000 Subject: [PATCH 5/5] integ: use retries when applying toxics The forked noxious-server takes a little longer to start on MacOS. Using retries ensures we keep trying until the server is ready. --- Cargo.lock | 12 +++++++++ integ/failure-server/Cargo.toml | 1 + integ/failure-server/src/toxic/tcp_proxy.rs | 27 +++++++++++++++------ 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28befb0a..555dbb52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -867,6 +867,7 @@ dependencies = [ "serde_json", "tempfile", "tokio", + "tokio-retry", "tower", "tower-fault", "tower-http", @@ -2400,6 +2401,17 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-retry" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f57eb36ecbe0fc510036adff84824dd3c24bb781e21bfa67b69d556aa85214f" +dependencies = [ + "pin-project", + "rand", + "tokio", +] + [[package]] name = "tokio-rustls" version = "0.23.4" diff --git a/integ/failure-server/Cargo.toml b/integ/failure-server/Cargo.toml index 1ad9550a..2d5a9176 100644 --- a/integ/failure-server/Cargo.toml +++ b/integ/failure-server/Cargo.toml @@ -14,6 +14,7 @@ rand = "0.8" serde_json = "1.0" tempfile = "3.8" tokio = "1.0" +tokio-retry = "0.3" tower = { version = "0.4", features = ["util"] } tower-fault = "0.0.5" tower-http = { version = "0.4", features = ["fs"] } diff --git a/integ/failure-server/src/toxic/tcp_proxy.rs b/integ/failure-server/src/toxic/tcp_proxy.rs index 265cfaa0..c72a255a 100644 --- a/integ/failure-server/src/toxic/tcp_proxy.rs +++ b/integ/failure-server/src/toxic/tcp_proxy.rs @@ -8,6 +8,7 @@ use std::net::ToSocketAddrs; use std::process::Command; use std::{fmt::Debug, net::SocketAddr}; use tempfile::NamedTempFile; +use tokio_retry::{strategy::ExponentialBackoff, Retry}; /// A TCP proxy server that introduces artificial faults at the TCP layer. #[derive(Debug)] @@ -26,6 +27,10 @@ pub(crate) struct ToxicTcpProxy { toxics: Vec, } +fn retry_strategy() -> impl Iterator { + ExponentialBackoff::from_millis(500).take(10) +} + impl ToxicTcpProxy { pub(crate) fn new( name: String, @@ -92,15 +97,21 @@ impl ToxicTcpProxy { // Configure toxics let client = Client::new(&self.api_listen.to_string()); - let proxy = client.proxy(&self.name).await.context(format!( - "Failed to find our configured proxy '{}'", - self.name - ))?; + let proxy = Retry::spawn(retry_strategy(), || async { + client.proxy(&self.name).await.context(format!( + "Failed to find our configured proxy '{}'", + self.name + )) + }) + .await?; for toxic in &self.toxics { - proxy.add_toxic(toxic).await.context(format!( - "Failed to apply toxic {:?} to proxy '{}'", - toxic, self.name - ))?; + Retry::spawn(retry_strategy(), || async { + proxy.add_toxic(toxic).await.context(format!( + "Failed to apply toxic {:?} to proxy '{}'", + toxic, self.name + )) + }) + .await?; } Ok(())