Merge pull request #2737 from lann/factors-update-outbound-net

factors: Update outbound networking

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 = ();
@@ -60,6 +69,7 @@ impl Factor for OutboundHttpFactor {
             wasi_http_ctx: WasiHttpCtx::new(),
             allowed_hosts,
             component_tls_configs,
+            self_request_origin: None,
             request_interceptor: None,
         })
     }
@@ -69,10 +79,19 @@ pub struct InstanceState {
     wasi_http_ctx: WasiHttpCtx,
     allowed_hosts: OutboundAllowedHosts,
     component_tls_configs: ComponentTlsConfigs,
+    self_request_origin: Option<SelfRequestOrigin>,
     request_interceptor: Option<Box<dyn OutboundHttpInterceptor>>,
 }
 
 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.

crates/factor-outbound-http/src/lib.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<Response, HttpError> {
         // 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);

crates/factor-outbound-http/src/spin.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>,
@@ -93,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(),
@@ -104,67 +118,52 @@ 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,
+    self_request_origin: Option<SelfRequestOrigin>,
     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 !allowed_hosts.allows_relative_url(&["http", "https"]) {
-            return Ok(handle_not_allowed(request.uri(), true));
+    if request.uri().authority().is_some() {
+        // Absolute URI
+        let is_allowed = outbound_allowed_hosts
+            .check_url(&request.uri().to_string(), "https")
+            .await
+            .unwrap_or(false);
+        if !is_allowed {
+            return Ok(Err(ErrorCode::HttpRequestDenied));
+        }
+    } else {
+        // Relative URI ("self" request)
+        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 = request
-            .extensions()
-            .get::<SelfRequestOrigin>()
-            .cloned()
-            .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();
 
         request.headers_mut().insert(HOST, origin.host_header());
 
         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() {
-        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)
 }
 
-// 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

@@ -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;
 
@@ -77,8 +76,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(),
     }
 }