Merge pull request #2808 from fermyon/allow-private-ips

Give outbound http ability to ban private ips

Author: rylev

Cargo.lock

@@ -9,14 +9,15 @@ anyhow = "1.0"
 http = "1.1.0"
 http-body-util = "0.1"
 hyper = "1.4.1"
+ip_network = "0.4"
 reqwest = { version = "0.11", features = ["gzip"] }
 rustls = { version = "0.23", default-features = false, features = ["ring", "std"] }
 spin-factor-outbound-networking = { path = "../factor-outbound-networking" }
 spin-factors = { path = "../factors" }
 spin-telemetry = { path = "../telemetry" }
 spin-world = { path = "../world" }
 terminal = { path = "../terminal" }
-tokio = { version = "1", features = ["macros", "rt"] }
+tokio = { version = "1", features = ["macros", "rt", "net"] }
 tokio-rustls = { version = "0.26", default-features = false, features = ["logging", "tls12"] }
 tracing = { workspace = true }
 wasmtime = { workspace = true }

crates/factor-outbound-http/Cargo.toml

@@ -24,14 +24,24 @@ pub use wasmtime_wasi_http::{
     HttpResult,
 };
 
-#[derive(Default)]
 pub struct OutboundHttpFactor {
-    _priv: (),
+    allow_private_ips: bool,
 }
 
 impl OutboundHttpFactor {
-    pub fn new() -> Self {
-        Self::default()
+    /// Create a new OutboundHttpFactor.
+    ///
+    /// If `allow_private_ips` is true, requests to private IP addresses will be allowed.
+    pub fn new(allow_private_ips: bool) -> Self {
+        Self { allow_private_ips }
+    }
+}
+
+impl Default for OutboundHttpFactor {
+    fn default() -> Self {
+        Self {
+            allow_private_ips: true,
+        }
     }
 }
 
@@ -66,6 +76,7 @@ impl Factor for OutboundHttpFactor {
         Ok(InstanceState {
             wasi_http_ctx: WasiHttpCtx::new(),
             allowed_hosts,
+            allow_private_ips: self.allow_private_ips,
             component_tls_configs,
             self_request_origin: None,
             request_interceptor: None,
@@ -77,6 +88,7 @@ impl Factor for OutboundHttpFactor {
 pub struct InstanceState {
     wasi_http_ctx: WasiHttpCtx,
     allowed_hosts: OutboundAllowedHosts,
+    allow_private_ips: bool,
     component_tls_configs: ComponentTlsConfigs,
     self_request_origin: Option<SelfRequestOrigin>,
     request_interceptor: Option<Box<dyn OutboundHttpInterceptor>>,

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

@@ -1,8 +1,9 @@
-use std::{error::Error, sync::Arc};
+use std::{error::Error, net::IpAddr, sync::Arc};
 
 use anyhow::Context;
 use http::{header::HOST, Request};
 use http_body_util::BodyExt;
+use ip_network::IpNetwork;
 use rustls::ClientConfig;
 use spin_factor_outbound_networking::OutboundAllowedHosts;
 use spin_factors::{wasmtime::component::ResourceTable, RuntimeFactorsInstanceState};
@@ -123,6 +124,7 @@ impl<'a> WasiHttpView for WasiHttpImplInner<'a> {
                     self.state.allowed_hosts.clone(),
                     self.state.self_request_origin.clone(),
                     tls_client_config,
+                    self.state.allow_private_ips,
                 )
                 .in_current_span(),
             ),
@@ -136,6 +138,7 @@ async fn send_request_impl(
     outbound_allowed_hosts: OutboundAllowedHosts,
     self_request_origin: Option<SelfRequestOrigin>,
     tls_client_config: Arc<ClientConfig>,
+    allow_private_ips: bool,
 ) -> anyhow::Result<Result<IncomingResponse, ErrorCode>> {
     if request.uri().authority().is_some() {
         // Absolute URI
@@ -177,7 +180,7 @@ async fn send_request_impl(
         current_span.record("server.port", port.as_u16());
     }
 
-    Ok(send_request_handler(request, config, tls_client_config).await)
+    Ok(send_request_handler(request, config, tls_client_config, allow_private_ips).await)
 }
 
 /// This is a fork of wasmtime_wasi_http::default_send_request_handler function
@@ -192,6 +195,7 @@ async fn send_request_handler(
         between_bytes_timeout,
     }: wasmtime_wasi_http::types::OutgoingRequestConfig,
     tls_client_config: Arc<ClientConfig>,
+    allow_private_ips: bool,
 ) -> Result<wasmtime_wasi_http::types::IncomingResponse, ErrorCode> {
     let authority_str = if let Some(authority) = request.uri().authority() {
         if authority.port().is_some() {
@@ -204,23 +208,26 @@ async fn send_request_handler(
         return Err(ErrorCode::HttpRequestUriInvalid);
     };
 
-    let tcp_stream = timeout(connect_timeout, TcpStream::connect(&authority_str))
+    // Resolve the authority to IP addresses
+    let mut socket_addrs = tokio::net::lookup_host(&authority_str)
+        .await
+        .map_err(|_| dns_error("address not available".into(), 0))?
+        .collect::<Vec<_>>();
+
+    // Potentially filter out private IPs
+    if !allow_private_ips && !socket_addrs.is_empty() {
+        socket_addrs.retain(|addr| !is_private_ip(addr.ip()));
+        if socket_addrs.is_empty() {
+            return Err(ErrorCode::DestinationIpProhibited);
+        }
+    }
+
+    let tcp_stream = timeout(connect_timeout, TcpStream::connect(socket_addrs.as_slice()))
         .await
         .map_err(|_| ErrorCode::ConnectionTimeout)?
         .map_err(|err| match err.kind() {
-            std::io::ErrorKind::AddrNotAvailable => {
-                dns_error("address not available".to_string(), 0)
-            }
-            _ => {
-                if err
-                    .to_string()
-                    .starts_with("failed to lookup address information")
-                {
-                    dns_error("address not available".to_string(), 0)
-                } else {
-                    ErrorCode::ConnectionRefused
-                }
-            }
+            std::io::ErrorKind::AddrNotAvailable => dns_error("address not available".into(), 0),
+            _ => ErrorCode::ConnectionRefused,
         })?;
 
     let (mut sender, worker) = if use_tls {
@@ -337,3 +344,8 @@ fn dns_error(rcode: String, info_code: u16) -> ErrorCode {
         info_code: Some(info_code),
     })
 }
+
+/// Returns true if the IP is a private IP address.
+fn is_private_ip(ip: IpAddr) -> bool {
+    !IpNetwork::from(ip).is_global()
+}

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

@@ -21,7 +21,7 @@ struct TestFactors {
 
 #[tokio::test]
 async fn allowed_host_is_allowed() -> anyhow::Result<()> {
-    let mut state = test_instance_state("https://*").await?;
+    let mut state = test_instance_state("https://*", true).await?;
     let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(&mut state).unwrap();
 
     // [100::] is an IPv6 "black hole", which should always fail
@@ -39,7 +39,7 @@ 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 state = test_instance_state("http://self", true).await?;
     let origin = SelfRequestOrigin::from_uri(&Uri::from_static("http://[100::1]"))?;
     state.http.set_self_request_origin(origin);
 
@@ -58,7 +58,7 @@ async fn self_request_smoke_test() -> anyhow::Result<()> {
 
 #[tokio::test]
 async fn disallowed_host_fails() -> anyhow::Result<()> {
-    let mut state = test_instance_state("https://allowed.test").await?;
+    let mut state = test_instance_state("https://allowed.test", true).await?;
     let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(&mut state).unwrap();
 
     let req = Request::get("https://denied.test").body(Default::default())?;
@@ -71,13 +71,47 @@ async fn disallowed_host_fails() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn disallowed_private_ips_fails() -> anyhow::Result<()> {
+    async fn run_test(allow_private_ips: bool) -> anyhow::Result<()> {
+        let mut state = test_instance_state("http://*", allow_private_ips).await?;
+        let mut wasi_http = OutboundHttpFactor::get_wasi_http_impl(&mut state).unwrap();
+        let req = Request::get("http://localhost").body(Default::default())?;
+        let mut future_resp = wasi_http.send_request(req, test_request_config())?;
+        future_resp.ready().await;
+        match future_resp.unwrap_ready().unwrap() {
+            // If we don't allow private IPs, we should not get a response
+            Ok(_) if !allow_private_ips => bail!("expected Err, got Ok"),
+            // Otherwise, it's fine if the request happens to succeed
+            Ok(_) => {}
+            // If private IPs are disallowed, we should get an error saying the destination is prohibited
+            Err(err) if !allow_private_ips => {
+                assert!(matches!(err, ErrorCode::DestinationIpProhibited))
+            }
+            // Otherwise, we should get some non-DestinationIpProhibited error
+            Err(err) => {
+                assert!(!matches!(err, ErrorCode::DestinationIpProhibited))
+            }
+        };
+        Ok(())
+    }
+
+    // Test with private IPs allowed
+    run_test(true).await?;
+    // Test with private IPs disallowed
+    run_test(false).await?;
+
+    Ok(())
+}
+
 async fn test_instance_state(
     allowed_outbound_hosts: &str,
+    allow_private_ips: bool,
 ) -> anyhow::Result<TestFactorsInstanceState> {
     let factors = TestFactors {
         variables: VariablesFactor::default(),
         networking: OutboundNetworkingFactor::new(),
-        http: OutboundHttpFactor::new(),
+        http: OutboundHttpFactor::new(allow_private_ips),
     };
     let env = TestEnvironment::new(factors).extend_manifest(toml! {
         [component.test-component]

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

@@ -49,7 +49,7 @@ impl TriggerFactors {
             variables: VariablesFactor::default(),
             key_value: KeyValueFactor::new(default_key_value_label_resolver),
             outbound_networking: outbound_networking_factor(),
-            outbound_http: OutboundHttpFactor::new(),
+            outbound_http: OutboundHttpFactor::default(),
             sqlite: SqliteFactor::new(default_sqlite_label_resolver),
             redis: OutboundRedisFactor::new(),
             mqtt: OutboundMqttFactor::new(NetworkedMqttClient::creator()),