factors: Update outbound networking

Signed-off-by: Lann Martin <[email protected]>

Author: lann

Cargo.lock

@@ -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 = ();

crates/factor-outbound-http/src/lib.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<wasmtime_wasi_http::body::HyperOutgoingBody>,
@@ -104,15 +117,24 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> {
 async fn send_request_impl(
     mut request: Request<wasmtime_wasi_http::body::HyperOutgoingBody>,
     mut config: wasmtime_wasi_http::types::OutgoingRequestConfig,
-    allowed_hosts: OutboundAllowedHosts,
+    outbound_allowed_hosts: OutboundAllowedHosts,
     tls_client_config: Arc<ClientConfig>,
 ) -> anyhow::Result<Result<IncomingResponse, ErrorCode>> {
-    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<IncomingResponse, ErrorCode> {
-    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

crates/factor-outbound-http/src/wasi.rs

@@ -77,8 +77,8 @@ async fn test_instance_state(
 ) -> anyhow::Result<TestFactorsInstanceState> {
     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]

crates/factor-outbound-http/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 {})),
     }
 }

crates/factor-outbound-mqtt/tests/factor_test.rs

@@ -20,7 +20,7 @@ struct TestFactors {
 fn factors() -> TestFactors {
     TestFactors {
         variables: VariablesFactor::default(),
-        networking: OutboundNetworkingFactor,
+        networking: OutboundNetworkingFactor::new(),
         mysql: OutboundMysqlFactor::<MockClient>::new(),
     }
 }

crates/factor-outbound-mysql/tests/factor_test.rs

@@ -22,7 +22,24 @@ pub use runtime_config::ComponentTlsConfigs;
 
 pub type SharedFutureResult<T> = Shared<BoxFuture<'static, Result<Arc<T>, Arc<anyhow::Error>>>>;
 
-pub struct OutboundNetworkingFactor;
+#[derive(Default)]
+pub struct OutboundNetworkingFactor {
+    disallowed_host_callback: Option<DisallowedHostCallback>,
+}
+
+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::<WasiFactor>() {
             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<AllowedHostsConfig>,
     component_tls_configs: ComponentTlsConfigs,
+    disallowed_host_callback: Option<DisallowedHostCallback>,
 }
 
 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<AllowedHostsConfig>,
+    disallowed_host_callback: Option<DisallowedHostCallback>,
 }
 
 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<bool> {
         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<bool> {
+        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)
     }
 }

crates/factor-outbound-networking/src/lib.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<Vec<CertificateDer<'static>>> {
         let file = std::fs::File::open(Path::new(TESTDATA_DIR).join("valid-cert.pem"))?;

crates/factor-outbound-networking/src/runtime_config.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<Vec<S
 mod tests {
     use super::*;
 
-    const TESTDATA_DIR: &'static str = concat!(env!("CARGO_MANIFEST_DIR"), "/testdata");
+    const TESTDATA_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/testdata");
 
     #[test]
     fn test_min_config() -> anyhow::Result<()> {

crates/factor-outbound-networking/src/runtime_config/spin.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?;