From 4237a81e6eebf94578a5a1cc52449479f359bf9a Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Wed, 21 Aug 2024 11:00:08 -0400 Subject: [PATCH 1/3] factors: Update outbound networking Signed-off-by: Lann Martin --- Cargo.lock | 188 ++++++++++++------ crates/factor-outbound-http/src/lib.rs | 11 +- crates/factor-outbound-http/src/wasi.rs | 65 +++--- .../factor-outbound-http/tests/factor_test.rs | 4 +- .../factor-outbound-mqtt/tests/factor_test.rs | 2 +- .../tests/factor_test.rs | 2 +- crates/factor-outbound-networking/src/lib.rs | 90 ++++++--- .../src/runtime_config.rs | 2 +- .../src/runtime_config/spin.rs | 7 +- .../tests/factor_test.rs | 4 +- .../factor-outbound-pg/tests/factor_test.rs | 2 +- .../tests/factor_test.rs | 2 +- crates/factors/tests/smoke.rs | 4 +- crates/outbound-networking/src/lib.rs | 37 ++-- crates/trigger2/Cargo.toml | 1 + crates/trigger2/src/factors.rs | 30 ++- 16 files changed, 294 insertions(+), 157 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 20e822f0e7..860bb418db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -566,7 +566,7 @@ dependencies = [ "pin-project-lite", "rustversion", "serde 1.0.197", - "sync_wrapper", + "sync_wrapper 0.1.2", "tower", "tower-layer", "tower-service", @@ -606,7 +606,7 @@ dependencies = [ "paste", "pin-project", "rand 0.8.5", - "reqwest 0.12.4", + "reqwest 0.12.7", "rustc_version", "serde 1.0.197", "serde_json", @@ -1266,7 +1266,7 @@ dependencies = [ "num-traits 0.2.18", "serde 1.0.197", "wasm-bindgen", - "windows-targets 0.52.4", + "windows-targets 0.52.6", ] [[package]] @@ -1505,7 +1505,7 @@ dependencies = [ "flate2", "json5", "libtest-mimic 0.7.3", - "reqwest 0.12.4", + "reqwest 0.12.7", "serde 1.0.197", "tar", "test-environment", @@ -3472,6 +3472,23 @@ dependencies = [ "webpki-roots 0.26.1", ] +[[package]] +name = "hyper-rustls" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ee4be2c948921a1a5320b629c4193916ed787a7f7f293fd3f7f5a6c9de74155" +dependencies = [ + "futures-util", + "http 1.1.0", + "hyper 1.4.1", + "hyper-util", + "rustls 0.23.7", + "rustls-pki-types", + "tokio", + "tokio-rustls 0.26.0", + "tower-service", +] + [[package]] name = "hyper-timeout" version = "0.4.1" @@ -4022,7 +4039,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c2a198fb6b0eada2a8df47933734e6d35d350665a33a3593d7164fa52c75c19" dependencies = [ "cfg-if", - "windows-targets 0.52.4", + "windows-targets 0.52.6", ] [[package]] @@ -5034,7 +5051,7 @@ dependencies = [ "lazy_static 1.4.0", "olpc-cjson", "regex", - "reqwest 0.12.4", + "reqwest 0.12.7", "serde 1.0.197", "serde_json", "sha2", @@ -5058,7 +5075,7 @@ dependencies = [ "lazy_static 1.4.0", "olpc-cjson", "regex", - "reqwest 0.12.4", + "reqwest 0.12.7", "serde 1.0.197", "serde_json", "sha2", @@ -6420,8 +6437,8 @@ dependencies = [ "serde 1.0.197", "serde_json", "serde_urlencoded", - "sync_wrapper", - "system-configuration", + "sync_wrapper 0.1.2", + "system-configuration 0.5.1", "tokio", "tokio-native-tls", "tokio-rustls 0.24.1", @@ -6433,14 +6450,14 @@ dependencies = [ "wasm-streams", "web-sys", "webpki-roots 0.25.4", - "winreg 0.50.0", + "winreg", ] [[package]] name = "reqwest" -version = "0.12.4" +version = "0.12.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "566cafdd92868e0939d3fb961bd0dc25fcfaaed179291093b3d43e6b3150ea10" +checksum = "f8f4955649ef5c38cc7f9e8aa41761d48fb9677197daea9984dc54f56aad5e63" dependencies = [ "base64 0.22.0", "bytes", @@ -6453,6 +6470,7 @@ dependencies = [ "http-body 1.0.0", "http-body-util", "hyper 1.4.1", + "hyper-rustls 0.27.2", "hyper-tls 0.6.0", "hyper-util", "ipnet", @@ -6467,8 +6485,8 @@ dependencies = [ "serde 1.0.197", "serde_json", "serde_urlencoded", - "sync_wrapper", - "system-configuration", + "sync_wrapper 1.0.1", + "system-configuration 0.6.0", "tokio", "tokio-native-tls", "tokio-socks", @@ -6479,7 +6497,7 @@ dependencies = [ "wasm-bindgen-futures", "wasm-streams", "web-sys", - "winreg 0.52.0", + "windows-registry", ] [[package]] @@ -7446,7 +7464,7 @@ dependencies = [ "rand 0.8.5", "redis 0.24.0", "regex", - "reqwest 0.12.4", + "reqwest 0.12.7", "rpassword", "runtime-tests", "semver", @@ -8470,6 +8488,7 @@ dependencies = [ "spin-factors-executor", "spin-runtime-config", "spin-telemetry", + "terminal", "tokio", "tracing", ] @@ -8640,6 +8659,15 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" +[[package]] +name = "sync_wrapper" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7065abeca94b6a8a577f9bd45aa0867a2238b74e8eb67cf10d492bc39351394" +dependencies = [ + "futures-core", +] + [[package]] name = "synstructure" version = "0.12.6" @@ -8660,7 +8688,18 @@ checksum = "ba3a3adc5c275d719af8cb4272ea1c4a6d668a777f37e115f6d11ddbc1c8e0e7" dependencies = [ "bitflags 1.3.2", "core-foundation", - "system-configuration-sys", + "system-configuration-sys 0.5.0", +] + +[[package]] +name = "system-configuration" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "658bc6ee10a9b4fcf576e9b0819d95ec16f4d2c02d39fd83ac1c8789785c4a42" +dependencies = [ + "bitflags 2.5.0", + "core-foundation", + "system-configuration-sys 0.6.0", ] [[package]] @@ -8673,6 +8712,16 @@ dependencies = [ "libc", ] +[[package]] +name = "system-configuration-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "system-interface" version = "0.27.2" @@ -8794,7 +8843,7 @@ dependencies = [ "anyhow", "fslock", "regex", - "reqwest 0.12.4", + "reqwest 0.12.7", "temp-dir", "tokio", ] @@ -8810,7 +8859,7 @@ dependencies = [ "log", "nix 0.26.4", "regex", - "reqwest 0.12.4", + "reqwest 0.12.7", "spin-app", "spin-factors-executor", "spin-http", @@ -9724,7 +9773,7 @@ dependencies = [ "once_cell", "pathdiff", "ptree", - "reqwest 0.12.4", + "reqwest 0.12.7", "secrecy", "semver", "serde 1.0.197", @@ -10051,7 +10100,7 @@ dependencies = [ "anyhow", "dirs 5.0.1", "http 1.1.0", - "reqwest 0.12.4", + "reqwest 0.12.7", "semver", "serde 1.0.197", "serde_json", @@ -10747,7 +10796,37 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" dependencies = [ - "windows-targets 0.52.4", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-registry" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e400001bb720a623c1c69032f8e3e4cf09984deec740f007dd2b03ec864804b0" +dependencies = [ + "windows-result", + "windows-strings", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-result" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d1043d8214f791817bab27572aaa8af63732e11bf84aa21a45a78d6c317ae0e" +dependencies = [ + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-strings" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd9b125c486025df0eabcb585e62173c6c9eddcec5d117d3b6e8c30e2ee4d10" +dependencies = [ + "windows-result", + "windows-targets 0.52.6", ] [[package]] @@ -10774,7 +10853,7 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets 0.52.4", + "windows-targets 0.52.6", ] [[package]] @@ -10809,17 +10888,18 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dd37b7e5ab9018759f893a1952c9420d060016fc19a472b4bb20d1bdd694d1b" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm 0.52.4", - "windows_aarch64_msvc 0.52.4", - "windows_i686_gnu 0.52.4", - "windows_i686_msvc 0.52.4", - "windows_x86_64_gnu 0.52.4", - "windows_x86_64_gnullvm 0.52.4", - "windows_x86_64_msvc 0.52.4", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", + "windows_i686_gnullvm", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] [[package]] @@ -10836,9 +10916,9 @@ checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bcf46cf4c365c6f2d1cc93ce535f2c8b244591df96ceee75d8e83deb70a9cac9" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" [[package]] name = "windows_aarch64_msvc" @@ -10854,9 +10934,9 @@ checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" [[package]] name = "windows_aarch64_msvc" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da9f259dd3bcf6990b55bffd094c4f7235817ba4ceebde8e6d11cd0c5633b675" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" [[package]] name = "windows_i686_gnu" @@ -10872,9 +10952,15 @@ checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" [[package]] name = "windows_i686_gnu" -version = "0.52.4" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b474d8268f99e0995f25b9f095bc7434632601028cf86590aea5c8a5cb7801d3" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" [[package]] name = "windows_i686_msvc" @@ -10890,9 +10976,9 @@ checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" [[package]] name = "windows_i686_msvc" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1515e9a29e5bed743cb4415a9ecf5dfca648ce85ee42e15873c3cd8610ff8e02" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" [[package]] name = "windows_x86_64_gnu" @@ -10908,9 +10994,9 @@ checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" [[package]] name = "windows_x86_64_gnu" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5eee091590e89cc02ad514ffe3ead9eb6b660aedca2183455434b93546371a03" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" [[package]] name = "windows_x86_64_gnullvm" @@ -10926,9 +11012,9 @@ checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" [[package]] name = "windows_x86_64_gnullvm" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77ca79f2451b49fa9e2af39f0747fe999fcda4f5e241b2898624dca97a1f2177" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" [[package]] name = "windows_x86_64_msvc" @@ -10944,9 +11030,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "windows_x86_64_msvc" -version = "0.52.4" +version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "winnow" @@ -10976,16 +11062,6 @@ dependencies = [ "windows-sys 0.48.0", ] -[[package]] -name = "winreg" -version = "0.52.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a277a57398d4bfa075df44f501a17cfdf8542d224f0d36095a2adc7aee4ef0a5" -dependencies = [ - "cfg-if", - "windows-sys 0.48.0", -] - [[package]] name = "winx" version = "0.36.3" diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs index 7db483bb00..3e72a3763e 100644 --- a/crates/factor-outbound-http/src/lib.rs +++ b/crates/factor-outbound-http/src/lib.rs @@ -25,7 +25,16 @@ pub use wasmtime_wasi_http::{ HttpResult, }; -pub struct OutboundHttpFactor; +#[derive(Default)] +pub struct OutboundHttpFactor { + _priv: (), +} + +impl OutboundHttpFactor { + pub fn new() -> Self { + Self::default() + } +} impl Factor for OutboundHttpFactor { type RuntimeConfig = (); diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index 9f0fffeebb..8fb0a17d83 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -1,13 +1,13 @@ use std::{error::Error, sync::Arc}; use anyhow::Context; -use http::{header::HOST, uri::Authority, Request, Uri}; +use http::{header::HOST, Request}; use http_body_util::BodyExt; use rustls::ClientConfig; -use spin_factor_outbound_networking::{OutboundAllowedHosts, OutboundUrl}; +use spin_factor_outbound_networking::OutboundAllowedHosts; use spin_factors::{wasmtime::component::ResourceTable, RuntimeFactorsInstanceState}; use tokio::{net::TcpStream, time::timeout}; -use tracing::Instrument; +use tracing::{field::Empty, instrument, Instrument}; use wasmtime_wasi_http::{ bindings::http::types::ErrorCode, body::HyperOutgoingBody, @@ -68,6 +68,19 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> { self.table } + #[instrument( + name = "spin_outbound_http.send_request", + skip_all, + fields( + otel.kind = "client", + url.full = %request.uri(), + http.request.method = %request.method(), + otel.name = %request.method(), + http.response.status_code = Empty, + server.address = Empty, + server.port = Empty, + ), + )] fn send_request( &mut self, mut request: Request, @@ -104,15 +117,24 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> { async fn send_request_impl( mut request: Request, mut config: wasmtime_wasi_http::types::OutgoingRequestConfig, - allowed_hosts: OutboundAllowedHosts, + outbound_allowed_hosts: OutboundAllowedHosts, tls_client_config: Arc, ) -> anyhow::Result> { - let allowed_hosts = allowed_hosts.resolve().await?; - - let is_relative_url = request.uri().authority().is_none(); - if is_relative_url { + if request.uri().authority().is_some() { + // Absolute URI + let is_allowed = outbound_allowed_hosts + .check_url(&request.uri().to_string(), "https") + .await + .map_err(|_| ErrorCode::HttpRequestUriInvalid)?; + if !is_allowed { + return Ok(Err(ErrorCode::HttpRequestDenied)); + } + } else { + // Relative URI ("self" request) + let allowed_hosts = outbound_allowed_hosts.resolve().await?; if !allowed_hosts.allows_relative_url(&["http", "https"]) { - return Ok(handle_not_allowed(request.uri(), true)); + outbound_allowed_hosts.report_disallowed_host("http", "self"); + return Ok(Err(ErrorCode::HttpRequestDenied)); } let origin = request @@ -127,12 +149,6 @@ async fn send_request_impl( let path_and_query = request.uri().path_and_query().cloned(); *request.uri_mut() = origin.into_uri(path_and_query); - } else { - let outbound_url = OutboundUrl::parse(request.uri().to_string(), "https") - .map_err(|_| ErrorCode::HttpRequestUriInvalid)?; - if !allowed_hosts.allows(&outbound_url) { - return Ok(handle_not_allowed(request.uri(), false)); - } } if let Some(authority) = request.uri().authority() { @@ -146,25 +162,6 @@ async fn send_request_impl( Ok(send_request_handler(request, config, tls_client_config).await) } -// TODO(factors): Move to some callback on spin-factor-outbound-networking (?) -fn handle_not_allowed(uri: &Uri, is_relative: bool) -> Result { - tracing::error!("Destination not allowed!: {uri}"); - let allowed_host_example = if is_relative { - terminal::warn!("A component tried to make a HTTP request to the same component but it does not have permission."); - "http://self".to_string() - } else { - let host = format!( - "{scheme}://{authority}", - scheme = uri.scheme_str().unwrap_or_default(), - authority = uri.authority().map(Authority::as_str).unwrap_or_default() - ); - terminal::warn!("A component tried to make a HTTP request to non-allowed host '{host}'."); - host - }; - eprintln!("To allow requests, add 'allowed_outbound_hosts = [\"{allowed_host_example}\"]' to the manifest component section."); - Err(ErrorCode::HttpRequestDenied) -} - /// This is a fork of wasmtime_wasi_http::default_send_request_handler function /// forked from bytecodealliance/wasmtime commit-sha 29a76b68200fcfa69c8fb18ce6c850754279a05b /// This fork provides the ability to configure client cert auth for mTLS diff --git a/crates/factor-outbound-http/tests/factor_test.rs b/crates/factor-outbound-http/tests/factor_test.rs index 3de9ac7c15..caa1b65c05 100644 --- a/crates/factor-outbound-http/tests/factor_test.rs +++ b/crates/factor-outbound-http/tests/factor_test.rs @@ -77,8 +77,8 @@ async fn test_instance_state( ) -> anyhow::Result { let factors = TestFactors { variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, - http: OutboundHttpFactor, + networking: OutboundNetworkingFactor::new(), + http: OutboundHttpFactor::new(), }; let env = TestEnvironment::new(factors).extend_manifest(toml! { [component.test-component] diff --git a/crates/factor-outbound-mqtt/tests/factor_test.rs b/crates/factor-outbound-mqtt/tests/factor_test.rs index 1d88b3e574..e507d65207 100644 --- a/crates/factor-outbound-mqtt/tests/factor_test.rs +++ b/crates/factor-outbound-mqtt/tests/factor_test.rs @@ -46,7 +46,7 @@ struct TestFactors { fn factors() -> TestFactors { TestFactors { variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, + networking: OutboundNetworkingFactor::new(), mqtt: OutboundMqttFactor::new(Arc::new(MockMqttClient {})), } } diff --git a/crates/factor-outbound-mysql/tests/factor_test.rs b/crates/factor-outbound-mysql/tests/factor_test.rs index d392a0dff8..b9261377b2 100644 --- a/crates/factor-outbound-mysql/tests/factor_test.rs +++ b/crates/factor-outbound-mysql/tests/factor_test.rs @@ -20,7 +20,7 @@ struct TestFactors { fn factors() -> TestFactors { TestFactors { variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, + networking: OutboundNetworkingFactor::new(), mysql: OutboundMysqlFactor::::new(), } } diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index abf270bb0b..f5e1c783ef 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -22,7 +22,24 @@ pub use runtime_config::ComponentTlsConfigs; pub type SharedFutureResult = Shared, Arc>>>; -pub struct OutboundNetworkingFactor; +#[derive(Default)] +pub struct OutboundNetworkingFactor { + disallowed_host_callback: Option, +} + +pub type DisallowedHostCallback = fn(scheme: &str, authority: &str); + +impl OutboundNetworkingFactor { + pub fn new() -> Self { + Self::default() + } + + /// Sets a function to be called when a request is disallowed by an + /// instance's configured `allowed_outbound_hosts`. + pub fn set_disallowed_host_callback(&mut self, callback: DisallowedHostCallback) { + self.disallowed_host_callback = Some(callback); + } +} impl Factor for OutboundNetworkingFactor { type RuntimeConfig = RuntimeConfig; @@ -87,26 +104,24 @@ impl Factor for OutboundNetworkingFactor { match builders.get_mut::() { Ok(wasi_builder) => { // Update Wasi socket allowed ports - let hosts_future = allowed_hosts_future.clone(); + let allowed_hosts = OutboundAllowedHosts { + allowed_hosts_future: allowed_hosts_future.clone(), + disallowed_host_callback: self.disallowed_host_callback, + }; wasi_builder.outbound_socket_addr_check(move |addr, addr_use| { - let hosts_future = hosts_future.clone(); + let allowed_hosts = allowed_hosts.clone(); async move { - match hosts_future.await { - Ok(allowed_hosts) => { - // TODO: validate against existing spin-core behavior - let scheme = match addr_use { - SocketAddrUse::TcpBind => return false, - SocketAddrUse::TcpConnect => "tcp", - SocketAddrUse::UdpBind | SocketAddrUse::UdpConnect | SocketAddrUse::UdpOutgoingDatagram => "udp", - }; - spin_outbound_networking::check_url(&addr.to_string(),scheme, &allowed_hosts) - } - Err(err) => { - // TODO: should this trap (somehow)? - tracing::error!(%err, "allowed_outbound_hosts variable resolution failed"); - false - } - } + // TODO: validate against existing spin-core behavior + let scheme = match addr_use { + SocketAddrUse::TcpBind => return false, + SocketAddrUse::TcpConnect => "tcp", + SocketAddrUse::UdpBind | SocketAddrUse::UdpConnect | SocketAddrUse::UdpOutgoingDatagram => "udp", + }; + allowed_hosts.check_url(&addr.to_string(), scheme).await.unwrap_or_else(|err| { + // TODO: should this trap (somehow)? + tracing::error!(%err, "allowed_outbound_hosts variable resolution failed"); + false + }) } }); } @@ -122,6 +137,7 @@ impl Factor for OutboundNetworkingFactor { Ok(InstanceBuilder { allowed_hosts_future, component_tls_configs, + disallowed_host_callback: self.disallowed_host_callback, }) } } @@ -134,12 +150,14 @@ pub struct AppState { pub struct InstanceBuilder { allowed_hosts_future: SharedFutureResult, component_tls_configs: ComponentTlsConfigs, + disallowed_host_callback: Option, } impl InstanceBuilder { pub fn allowed_hosts(&self) -> OutboundAllowedHosts { OutboundAllowedHosts { allowed_hosts_future: self.allowed_hosts_future.clone(), + disallowed_host_callback: self.disallowed_host_callback, } } @@ -160,6 +178,7 @@ impl FactorInstanceBuilder for InstanceBuilder { #[derive(Clone)] pub struct OutboundAllowedHosts { allowed_hosts_future: SharedFutureResult, + disallowed_host_callback: Option, } impl OutboundAllowedHosts { @@ -170,16 +189,39 @@ impl OutboundAllowedHosts { }) } + /// Checks if the given URL is allowed by this component's + /// `allowed_outbound_hosts`. pub async fn allows(&self, url: &OutboundUrl) -> anyhow::Result { Ok(self.resolve().await?.allows(url)) } + /// Report that an outbound connection has been disallowed by e.g. + /// [`OutboundAllowedHosts::allows`] returning `false`. + /// + /// Calls the [`DisallowedHostCallback`] if set. + pub fn report_disallowed_host(&self, scheme: &str, authority: &str) { + if let Some(disallowed_host_callback) = self.disallowed_host_callback { + disallowed_host_callback(scheme, authority); + } + } + + /// Checks address against allowed hosts + /// + /// Calls the [`DisallowedHostCallback`] if set and URL is disallowed. pub async fn check_url(&self, url: &str, scheme: &str) -> anyhow::Result { + let Ok(url) = OutboundUrl::parse(url, scheme) else { + tracing::warn!( + "A component tried to make a request to a url that could not be parsed: {url}", + ); + return Ok(false); + }; + let allowed_hosts = self.resolve().await?; - Ok(spin_outbound_networking::check_url( - url, - scheme, - &allowed_hosts, - )) + + let is_allowed = allowed_hosts.allows(&url); + if !is_allowed { + self.report_disallowed_host(url.scheme(), &url.authority()); + } + Ok(is_allowed) } } diff --git a/crates/factor-outbound-networking/src/runtime_config.rs b/crates/factor-outbound-networking/src/runtime_config.rs index 9ea5d7d8f4..fad06edd1d 100644 --- a/crates/factor-outbound-networking/src/runtime_config.rs +++ b/crates/factor-outbound-networking/src/runtime_config.rs @@ -253,7 +253,7 @@ mod tests { Ok(()) } - const TESTDATA_DIR: &'static str = concat!(env!("CARGO_MANIFEST_DIR"), "/testdata"); + const TESTDATA_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/testdata"); fn test_certs() -> anyhow::Result>> { let file = std::fs::File::open(Path::new(TESTDATA_DIR).join("valid-cert.pem"))?; diff --git a/crates/factor-outbound-networking/src/runtime_config/spin.rs b/crates/factor-outbound-networking/src/runtime_config/spin.rs index 580d0e6c71..c8e2a3fc57 100644 --- a/crates/factor-outbound-networking/src/runtime_config/spin.rs +++ b/crates/factor-outbound-networking/src/runtime_config/spin.rs @@ -154,7 +154,10 @@ impl SpinTlsRuntimeConfig { .ok_or_else(|| { io::Error::new( io::ErrorKind::InvalidInput, - format!("private key file '{}' contains no private keys", path.display()), + format!( + "private key file '{}' contains no private keys", + path.display() + ), ) })?) } @@ -184,7 +187,7 @@ fn deserialize_hosts<'de, D: Deserializer<'de>>(deserializer: D) -> Result anyhow::Result<()> { diff --git a/crates/factor-outbound-networking/tests/factor_test.rs b/crates/factor-outbound-networking/tests/factor_test.rs index 693ac6600c..d4bd1a51c8 100644 --- a/crates/factor-outbound-networking/tests/factor_test.rs +++ b/crates/factor-outbound-networking/tests/factor_test.rs @@ -17,7 +17,7 @@ async fn configures_wasi_socket_addr_check() -> anyhow::Result<()> { let factors = TestFactors { wasi: WasiFactor::new(DummyFilesMounter), variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, + networking: OutboundNetworkingFactor::new(), }; let env = TestEnvironment::new(factors).extend_manifest(toml! { [component.test-component] @@ -58,7 +58,7 @@ async fn wasi_factor_is_optional() -> anyhow::Result<()> { } TestEnvironment::new(WithoutWasi { variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, + networking: OutboundNetworkingFactor::new(), }) .build_instance_state() .await?; diff --git a/crates/factor-outbound-pg/tests/factor_test.rs b/crates/factor-outbound-pg/tests/factor_test.rs index e189b9d2af..b765d805f6 100644 --- a/crates/factor-outbound-pg/tests/factor_test.rs +++ b/crates/factor-outbound-pg/tests/factor_test.rs @@ -21,7 +21,7 @@ struct TestFactors { fn factors() -> TestFactors { TestFactors { variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, + networking: OutboundNetworkingFactor::new(), pg: OutboundPgFactor::::new(), } } diff --git a/crates/factor-outbound-redis/tests/factor_test.rs b/crates/factor-outbound-redis/tests/factor_test.rs index 70a09f0be9..6f2f7b051a 100644 --- a/crates/factor-outbound-redis/tests/factor_test.rs +++ b/crates/factor-outbound-redis/tests/factor_test.rs @@ -17,7 +17,7 @@ struct TestFactors { async fn no_outbound_hosts_fails() -> anyhow::Result<()> { let factors = TestFactors { variables: VariablesFactor::default(), - networking: OutboundNetworkingFactor, + networking: OutboundNetworkingFactor::new(), redis: OutboundRedisFactor::new(), }; let env = TestEnvironment::new(factors).extend_manifest(toml! { diff --git a/crates/factors/tests/smoke.rs b/crates/factors/tests/smoke.rs index 9925c59b6f..3b31c701d2 100644 --- a/crates/factors/tests/smoke.rs +++ b/crates/factors/tests/smoke.rs @@ -60,8 +60,8 @@ async fn smoke_test_works() -> anyhow::Result<()> { let mut factors = Factors { wasi: WasiFactor::new(DummyFilesMounter), variables: VariablesFactor::default(), - outbound_networking: OutboundNetworkingFactor, - outbound_http: OutboundHttpFactor, + outbound_networking: OutboundNetworkingFactor::new(), + outbound_http: OutboundHttpFactor::new(), key_value: KeyValueFactor::new(key_value_resolver.clone()), }; diff --git a/crates/outbound-networking/src/lib.rs b/crates/outbound-networking/src/lib.rs index c2c3095b85..aec793419c 100644 --- a/crates/outbound-networking/src/lib.rs +++ b/crates/outbound-networking/src/lib.rs @@ -8,31 +8,6 @@ pub const ALLOWED_HOSTS_KEY: MetadataKey> = MetadataKey::new("allowe pub const SERVICE_CHAINING_DOMAIN: &str = "spin.internal"; pub const SERVICE_CHAINING_DOMAIN_SUFFIX: &str = ".spin.internal"; -/// Checks address against allowed hosts -/// -/// Emits several warnings -pub fn check_url(url: &str, scheme: &str, allowed_hosts: &AllowedHostsConfig) -> bool { - let Ok(url) = OutboundUrl::parse(url, scheme) else { - terminal::warn!( - "A component tried to make a request to an url that could not be parsed {url}.", - ); - return false; - }; - let is_allowed = allowed_hosts.allows(&url); - - if !is_allowed { - terminal::warn!("A component tried to make a request to non-allowed url '{url}'."); - let (scheme, host, port) = (url.scheme, url.host, url.port); - let msg = if let Some(port) = port { - format!("`allowed_outbound_hosts = [\"{scheme}://{host}:{port}\"]`") - } else { - format!("`allowed_outbound_hosts = [\"{scheme}://{host}:$PORT\"]` (where $PORT is the correct port number)") - }; - eprintln!("To allow requests, add {msg} to the manifest component section."); - } - is_allowed -} - /// An address is a url-like string that contains a host, a port, and an optional scheme #[derive(Eq, Debug, Clone)] pub struct AllowedHostConfig { @@ -431,6 +406,18 @@ impl OutboundUrl { original, }) } + + pub fn scheme(&self) -> &str { + &self.scheme + } + + pub fn authority(&self) -> String { + if let Some(port) = self.port { + format!("{}:{port}", self.host) + } else { + self.host.clone() + } + } } impl std::fmt::Display for OutboundUrl { diff --git a/crates/trigger2/Cargo.toml b/crates/trigger2/Cargo.toml index 70a7588ac0..a989095345 100644 --- a/crates/trigger2/Cargo.toml +++ b/crates/trigger2/Cargo.toml @@ -32,6 +32,7 @@ spin-factors = { path = "../factors" } spin-factors-executor = { path = "../factors-executor" } spin-telemetry = { path = "../telemetry" } tokio = { version = "1.23", features = ["fs"] } +terminal = { path = "../terminal" } tracing = { workspace = true } [lints] diff --git a/crates/trigger2/src/factors.rs b/crates/trigger2/src/factors.rs index 897151863f..e4719e5060 100644 --- a/crates/trigger2/src/factors.rs +++ b/crates/trigger2/src/factors.rs @@ -28,19 +28,41 @@ impl TriggerFactors { default_key_value_label_resolver: impl spin_factor_key_value::DefaultLabelResolver + 'static, default_sqlite_label_resolver: impl spin_factor_sqlite::DefaultLabelResolver + 'static, ) -> Self { - let files_mounter = SpinFilesMounter::new(working_dir, allow_transient_writes); Self { - wasi: WasiFactor::new(files_mounter), + wasi: wasi_factor(working_dir, allow_transient_writes), variables: VariablesFactor::default(), key_value: KeyValueFactor::new(default_key_value_label_resolver), - outbound_networking: OutboundNetworkingFactor, - outbound_http: OutboundHttpFactor, + outbound_networking: outbound_networking_factor(), + outbound_http: OutboundHttpFactor::new(), sqlite: SqliteFactor::new(default_sqlite_label_resolver), redis: OutboundRedisFactor::new(), } } } +fn wasi_factor(working_dir: impl Into, allow_transient_writes: bool) -> WasiFactor { + WasiFactor::new(SpinFilesMounter::new(working_dir, allow_transient_writes)) +} + +fn outbound_networking_factor() -> OutboundNetworkingFactor { + fn disallowed_host_callback(scheme: &str, authority: &str) { + let host_pattern = format!("{scheme}://{authority}"); + tracing::error!("Outbound network destination not allowed: {host_pattern}"); + if scheme.starts_with("http") && authority == "self" { + terminal::warn!("A component tried to make an HTTP request to its own app but it does not have permission."); + } else { + terminal::warn!( + "A component tried to make an outbound network connection to disallowed destination '{host_pattern}'." + ); + }; + eprintln!("To allow this request, add 'allowed_outbound_hosts = [\"{host_pattern}\"]' to the manifest component section."); + } + + let mut factor = OutboundNetworkingFactor::new(); + factor.set_disallowed_host_callback(disallowed_host_callback); + factor +} + impl TryFrom> for TriggerFactorsRuntimeConfig { type Error = anyhow::Error; From a8b4c8a05ec49f27e76d155948ed83e4395f8c56 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Wed, 21 Aug 2024 11:36:35 -0400 Subject: [PATCH 2/3] factors: Refactor OutboundHttpFactor SelfRequestOrigin Signed-off-by: Lann Martin --- crates/factor-outbound-http/src/lib.rs | 10 ++++++++++ crates/factor-outbound-http/src/wasi.rs | 7 +++---- crates/factor-outbound-http/tests/factor_test.rs | 9 ++++----- crates/trigger-http2/src/outbound_http.rs | 8 +++----- crates/trigger-http2/src/server.rs | 7 +++---- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs index 3e72a3763e..b0c18baf8a 100644 --- a/crates/factor-outbound-http/src/lib.rs +++ b/crates/factor-outbound-http/src/lib.rs @@ -69,6 +69,7 @@ impl Factor for OutboundHttpFactor { wasi_http_ctx: WasiHttpCtx::new(), allowed_hosts, component_tls_configs, + self_request_origin: None, request_interceptor: None, }) } @@ -78,10 +79,19 @@ pub struct InstanceState { wasi_http_ctx: WasiHttpCtx, allowed_hosts: OutboundAllowedHosts, component_tls_configs: ComponentTlsConfigs, + self_request_origin: Option, request_interceptor: Option>, } impl InstanceState { + /// Sets the [`SelfRequestOrigin`] for this instance. + /// + /// This is used to handle outbound requests to relative URLs. If unset, + /// those requests will fail. + pub fn set_self_request_origin(&mut self, origin: SelfRequestOrigin) { + self.self_request_origin = Some(origin); + } + /// Sets a [`OutboundHttpInterceptor`] for this instance. /// /// Returns an error if it has already been called for this instance. diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index 8fb0a17d83..925c2f9f99 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -106,6 +106,7 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> { request, config, self.state.allowed_hosts.clone(), + self.state.self_request_origin.clone(), tls_client_config, ) .in_current_span(), @@ -118,6 +119,7 @@ async fn send_request_impl( mut request: Request, mut config: wasmtime_wasi_http::types::OutgoingRequestConfig, outbound_allowed_hosts: OutboundAllowedHosts, + self_request_origin: Option, tls_client_config: Arc, ) -> anyhow::Result> { if request.uri().authority().is_some() { @@ -137,10 +139,7 @@ async fn send_request_impl( return Ok(Err(ErrorCode::HttpRequestDenied)); } - let origin = request - .extensions() - .get::() - .cloned() + let origin = self_request_origin .context("cannot send relative outbound request; no 'origin' set by host")?; config.use_tls = origin.use_tls(); diff --git a/crates/factor-outbound-http/tests/factor_test.rs b/crates/factor-outbound-http/tests/factor_test.rs index caa1b65c05..42bfe7c5ac 100644 --- a/crates/factor-outbound-http/tests/factor_test.rs +++ b/crates/factor-outbound-http/tests/factor_test.rs @@ -40,12 +40,11 @@ async fn allowed_host_is_allowed() -> anyhow::Result<()> { #[tokio::test] async fn self_request_smoke_test() -> anyhow::Result<()> { let mut state = test_instance_state("http://self").await?; - let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(&mut state).unwrap(); + let origin = SelfRequestOrigin::from_uri(&Uri::from_static("http://[100::1]"))?; + state.http.set_self_request_origin(origin); - let mut req = Request::get("/self-request").body(Default::default())?; - let origin = Uri::from_static("http://[100::1]"); - req.extensions_mut() - .insert(SelfRequestOrigin::from_uri(&origin).unwrap()); + let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(&mut state).unwrap(); + let req = Request::get("/self-request").body(Default::default())?; let mut future_resp = wasi_http.send_request(req, test_request_config())?; future_resp.ready().await; diff --git a/crates/trigger-http2/src/outbound_http.rs b/crates/trigger-http2/src/outbound_http.rs index 45e63ab751..a722bc315d 100644 --- a/crates/trigger-http2/src/outbound_http.rs +++ b/crates/trigger-http2/src/outbound_http.rs @@ -5,7 +5,7 @@ use std::{ use http::uri::Scheme; use spin_factor_outbound_http::{ - HostFutureIncomingResponse, InterceptOutcome, OutgoingRequestConfig, Request, SelfRequestOrigin, + HostFutureIncomingResponse, InterceptOutcome, OutgoingRequestConfig, Request, }; use spin_http::routes::RouteMatch; use spin_outbound_networking::parse_service_chaining_target; @@ -16,12 +16,11 @@ use crate::HttpServer; /// An outbound HTTP interceptor that handles service chaining requests. pub struct OutboundHttpInterceptor { server: Arc, - origin: SelfRequestOrigin, } impl OutboundHttpInterceptor { - pub fn new(server: Arc, origin: SelfRequestOrigin) -> Self { - Self { server, origin } + pub fn new(server: Arc) -> Self { + Self { server } } } @@ -58,7 +57,6 @@ impl spin_factor_outbound_http::OutboundHttpInterceptor for OutboundHttpIntercep let resp = HostFutureIncomingResponse::pending(wasmtime_wasi::runtime::spawn(resp_fut)); InterceptOutcome::Complete(Ok(resp)) } else { - request.extensions_mut().insert(self.origin.clone()); InterceptOutcome::Continue } } diff --git a/crates/trigger-http2/src/server.rs b/crates/trigger-http2/src/server.rs index c01327fda3..3923bc9c49 100644 --- a/crates/trigger-http2/src/server.rs +++ b/crates/trigger-http2/src/server.rs @@ -188,11 +188,10 @@ impl HttpServer { let mut instance_builder = self.trigger_app.prepare(component_id)?; // Set up outbound HTTP request origin and service chaining + let outbound_http = instance_builder.factor_builders().outbound_http(); let origin = SelfRequestOrigin::create(server_scheme, &self.listen_addr)?; - instance_builder - .factor_builders() - .outbound_http() - .set_request_interceptor(OutboundHttpInterceptor::new(self.clone(), origin))?; + outbound_http.set_self_request_origin(origin); + outbound_http.set_request_interceptor(OutboundHttpInterceptor::new(self.clone()))?; // Prepare HTTP executor let trigger_config = self.component_trigger_configs.get(component_id).unwrap(); From f09aaffa6457eb1d7b2ff80b729c72dcde8fcd41 Mon Sep 17 00:00:00 2001 From: Lann Martin Date: Wed, 21 Aug 2024 12:46:14 -0400 Subject: [PATCH 3/3] Address review feedback Signed-off-by: Lann Martin --- crates/factor-outbound-http/src/spin.rs | 4 +- crates/factor-outbound-http/src/wasi.rs | 27 ++++--- crates/factor-outbound-networking/src/lib.rs | 81 +++++++++++--------- crates/trigger2/src/factors.rs | 4 +- 4 files changed, 64 insertions(+), 52 deletions(-) diff --git a/crates/factor-outbound-http/src/spin.rs b/crates/factor-outbound-http/src/spin.rs index 22ea723f2f..c9dd6f1862 100644 --- a/crates/factor-outbound-http/src/spin.rs +++ b/crates/factor-outbound-http/src/spin.rs @@ -1,4 +1,3 @@ -use spin_factor_outbound_networking::OutboundUrl; use spin_world::{ async_trait, v1::http, @@ -9,8 +8,7 @@ use spin_world::{ impl http::Host for crate::InstanceState { async fn send_request(&mut self, req: Request) -> Result { // FIXME(lann): This is all just a stub to test allowed_outbound_hosts - let outbound_url = OutboundUrl::parse(&req.uri, "https").or(Err(HttpError::InvalidUrl))?; - match self.allowed_hosts.allows(&outbound_url).await { + match self.allowed_hosts.check_url(&req.uri, "https").await { Ok(true) => (), _ => { return Err(HttpError::DestinationNotAllowed); diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index 925c2f9f99..e63f40b532 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -127,20 +127,24 @@ async fn send_request_impl( let is_allowed = outbound_allowed_hosts .check_url(&request.uri().to_string(), "https") .await - .map_err(|_| ErrorCode::HttpRequestUriInvalid)?; + .unwrap_or(false); if !is_allowed { return Ok(Err(ErrorCode::HttpRequestDenied)); } } else { // Relative URI ("self" request) - let allowed_hosts = outbound_allowed_hosts.resolve().await?; - if !allowed_hosts.allows_relative_url(&["http", "https"]) { - outbound_allowed_hosts.report_disallowed_host("http", "self"); + let is_allowed = outbound_allowed_hosts + .check_relative_url(&["http", "https"]) + .await + .unwrap_or(false); + if !is_allowed { return Ok(Err(ErrorCode::HttpRequestDenied)); } - let origin = self_request_origin - .context("cannot send relative outbound request; no 'origin' set by host")?; + let Some(origin) = self_request_origin else { + tracing::error!("Couldn't handle outbound HTTP request to relative URI; no origin set"); + return Ok(Err(ErrorCode::HttpRequestUriInvalid)); + }; config.use_tls = origin.use_tls(); @@ -150,12 +154,11 @@ async fn send_request_impl( *request.uri_mut() = origin.into_uri(path_and_query); } - if let Some(authority) = request.uri().authority() { - let current_span = tracing::Span::current(); - current_span.record("server.address", authority.host()); - if let Some(port) = authority.port() { - current_span.record("server.port", port.as_u16()); - } + let authority = request.uri().authority().context("authority not set")?; + let current_span = tracing::Span::current(); + current_span.record("server.address", authority.host()); + if let Some(port) = authority.port() { + current_span.record("server.port", port.as_u16()); } Ok(send_request_handler(request, config, tls_client_config).await) diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index f5e1c783ef..7af5f34f14 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -24,20 +24,18 @@ pub type SharedFutureResult = Shared, Arc, + disallowed_host_handler: Option>, } -pub type DisallowedHostCallback = fn(scheme: &str, authority: &str); - impl OutboundNetworkingFactor { pub fn new() -> Self { Self::default() } - /// Sets a function to be called when a request is disallowed by an + /// Sets a handler to be called when a request is disallowed by an /// instance's configured `allowed_outbound_hosts`. - pub fn set_disallowed_host_callback(&mut self, callback: DisallowedHostCallback) { - self.disallowed_host_callback = Some(callback); + pub fn set_disallowed_host_handler(&mut self, handler: impl DisallowedHostHandler + 'static) { + self.disallowed_host_handler = Some(Arc::new(handler)); } } @@ -106,7 +104,7 @@ impl Factor for OutboundNetworkingFactor { // Update Wasi socket allowed ports let allowed_hosts = OutboundAllowedHosts { allowed_hosts_future: allowed_hosts_future.clone(), - disallowed_host_callback: self.disallowed_host_callback, + disallowed_host_handler: self.disallowed_host_handler.clone(), }; wasi_builder.outbound_socket_addr_check(move |addr, addr_use| { let allowed_hosts = allowed_hosts.clone(); @@ -137,7 +135,7 @@ impl Factor for OutboundNetworkingFactor { Ok(InstanceBuilder { allowed_hosts_future, component_tls_configs, - disallowed_host_callback: self.disallowed_host_callback, + disallowed_host_handler: self.disallowed_host_handler.clone(), }) } } @@ -150,14 +148,14 @@ pub struct AppState { pub struct InstanceBuilder { allowed_hosts_future: SharedFutureResult, component_tls_configs: ComponentTlsConfigs, - disallowed_host_callback: Option, + disallowed_host_handler: Option>, } impl InstanceBuilder { pub fn allowed_hosts(&self) -> OutboundAllowedHosts { OutboundAllowedHosts { allowed_hosts_future: self.allowed_hosts_future.clone(), - disallowed_host_callback: self.disallowed_host_callback, + disallowed_host_handler: self.disallowed_host_handler.clone(), } } @@ -178,33 +176,10 @@ impl FactorInstanceBuilder for InstanceBuilder { #[derive(Clone)] pub struct OutboundAllowedHosts { allowed_hosts_future: SharedFutureResult, - disallowed_host_callback: Option, + disallowed_host_handler: Option>, } impl OutboundAllowedHosts { - pub async fn resolve(&self) -> anyhow::Result> { - self.allowed_hosts_future.clone().await.map_err(|err| { - // TODO: better way to handle this? - anyhow::Error::msg(err) - }) - } - - /// Checks if the given URL is allowed by this component's - /// `allowed_outbound_hosts`. - pub async fn allows(&self, url: &OutboundUrl) -> anyhow::Result { - Ok(self.resolve().await?.allows(url)) - } - - /// Report that an outbound connection has been disallowed by e.g. - /// [`OutboundAllowedHosts::allows`] returning `false`. - /// - /// Calls the [`DisallowedHostCallback`] if set. - pub fn report_disallowed_host(&self, scheme: &str, authority: &str) { - if let Some(disallowed_host_callback) = self.disallowed_host_callback { - disallowed_host_callback(scheme, authority); - } - } - /// Checks address against allowed hosts /// /// Calls the [`DisallowedHostCallback`] if set and URL is disallowed. @@ -217,11 +192,47 @@ impl OutboundAllowedHosts { }; let allowed_hosts = self.resolve().await?; - let is_allowed = allowed_hosts.allows(&url); if !is_allowed { self.report_disallowed_host(url.scheme(), &url.authority()); } Ok(is_allowed) } + + /// Checks if allowed hosts permit relative requests + /// + /// Calls the [`DisallowedHostCallback`] if set and relative requests are + /// disallowed. + pub async fn check_relative_url(&self, schemes: &[&str]) -> anyhow::Result { + let allowed_hosts = self.resolve().await?; + let is_allowed = allowed_hosts.allows_relative_url(schemes); + if !is_allowed { + let scheme = schemes.first().unwrap_or(&""); + self.report_disallowed_host(scheme, "self"); + } + Ok(is_allowed) + } + + async fn resolve(&self) -> anyhow::Result> { + self.allowed_hosts_future.clone().await.map_err(|err| { + tracing::error!("Error resolving allowed_outbound_hosts variables: {err}"); + anyhow::Error::msg(err) + }) + } + + fn report_disallowed_host(&self, scheme: &str, authority: &str) { + if let Some(handler) = &self.disallowed_host_handler { + handler.handle_disallowed_host(scheme, authority); + } + } +} + +pub trait DisallowedHostHandler: Send + Sync { + fn handle_disallowed_host(&self, scheme: &str, authority: &str); +} + +impl DisallowedHostHandler for F { + fn handle_disallowed_host(&self, scheme: &str, authority: &str) { + self(scheme, authority); + } } diff --git a/crates/trigger2/src/factors.rs b/crates/trigger2/src/factors.rs index e4719e5060..42b2dae138 100644 --- a/crates/trigger2/src/factors.rs +++ b/crates/trigger2/src/factors.rs @@ -45,7 +45,7 @@ fn wasi_factor(working_dir: impl Into, allow_transient_writes: bool) -> } fn outbound_networking_factor() -> OutboundNetworkingFactor { - fn disallowed_host_callback(scheme: &str, authority: &str) { + fn disallowed_host_handler(scheme: &str, authority: &str) { let host_pattern = format!("{scheme}://{authority}"); tracing::error!("Outbound network destination not allowed: {host_pattern}"); if scheme.starts_with("http") && authority == "self" { @@ -59,7 +59,7 @@ fn outbound_networking_factor() -> OutboundNetworkingFactor { } let mut factor = OutboundNetworkingFactor::new(); - factor.set_disallowed_host_callback(disallowed_host_callback); + factor.set_disallowed_host_handler(disallowed_host_handler); factor }