allow_private_network_connections no longer blocks the reverse_proxy

CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- `allow_private_network_connections = false` no longer blocks the `[reverse_proxy]` `server_address`: the origin server address comes from the endpoint configuration, so a loopback or private address stays reachable while tunneled client traffic to private networks remains forbidden.
 - `is_global_ipv6` incorrectly classified global unicast IPv6 addresses (e.g. `2001:4860:4860::8888`) as non-global.
 
 ### Security

CONFIGURATION.md

@@ -115,7 +115,9 @@ listen_address = "0.0.0.0:443"
 # Whether IPv6 connections can be routed
 ipv6_available = true
 
-# Whether connections to private network of the endpoint are allowed
+# Whether connections to private network of the endpoint are allowed.
+# Applies to traffic tunneled by clients only, the reverse proxy
+# `server_address` is always reachable.
 allow_private_network_connections = false
 
 # Timeout of an incoming TLS handshake (seconds)
@@ -371,6 +373,8 @@ h3_backward_compatibility = false
 
 The reverse proxy translates HTTP/x traffic to HTTP/1.1 towards the origin server. Translated requests include the `X-Original-Protocol` header (`HTTP1` or `HTTP3`).
 
+`server_address` is configured by the endpoint operator and is therefore not affected by `allow_private_network_connections`: an origin server on a loopback or private address stays reachable even when tunneled client traffic to private networks is forbidden.
+
 ### ICMP Settings
 
 Optional. Enables ICMP forwarding. Requires superuser privileges on some systems.

lib/src/reverse_proxy.rs

@@ -111,7 +111,9 @@ async fn handle_stream(
     let (request, respond) = stream.split();
     log_id!(trace, log_id, "Received request: {:?}", request.request());
 
-    let forwarder = Box::new(TcpForwarder::new(context.clone()));
+    let forwarder = Box::new(TcpForwarder::new_for_configured_destination(
+        context.clone(),
+    ));
     let settings = context.settings.reverse_proxy.as_ref().unwrap();
     let (mut server_source, mut server_sink) = forwarder
         .connect(

lib/src/settings.rs

@@ -84,7 +84,10 @@ pub struct Settings {
     /// Whether IPv6 connections can be routed or rejected with unreachable status
     #[serde(default = "Settings::default_ipv6_available")]
     pub(crate) ipv6_available: bool,
-    /// Whether connections to private network of the endpoint are allowed
+    /// Whether connections to private network of the endpoint are allowed.
+    /// Applies to the traffic tunneled by clients only: destinations taken from
+    /// the endpoint configuration, such as [`ReverseProxySettings::server_address`],
+    /// are always allowed.
     #[serde(default = "Settings::default_allow_private_network_connections")]
     pub(crate) allow_private_network_connections: bool,
     /// Timeout of an incoming TLS handshake
@@ -299,7 +302,8 @@ pub struct TlsHostsSettings {
 #[derive(Serialize, Deserialize)]
 #[cfg_attr(feature = "rt_doc", derive(RuntimeDoc))]
 pub struct ReverseProxySettings {
-    /// The origin server address
+    /// The origin server address. It is reachable regardless of
+    /// [`Settings::allow_private_network_connections`].
     pub(crate) server_address: SocketAddr,
     /// Connections to [the main hosts](TlsHostsSettings.main_hosts) with
     /// paths starting with this mask are routed to the reverse proxy server.

lib/src/tcp_forwarder.rs

@@ -14,6 +14,9 @@ use tokio::net::TcpStream;
 
 pub(crate) struct TcpForwarder {
     context: Arc<core::Context>,
+    /// Whether the destination may belong to the endpoint's private network regardless
+    /// of [`crate::settings::Settings::allow_private_network_connections`]
+    allow_private_network: bool,
 }
 
 struct StreamRx {
@@ -34,7 +37,21 @@ struct StreamTx {
 
 impl TcpForwarder {
     pub fn new(context: Arc<core::Context>) -> Self {
-        Self { context }
+        Self {
+            context,
+            allow_private_network: false,
+        }
+    }
+
+    /// Creates a forwarder which is allowed to reach the endpoint's private network
+    /// regardless of [`crate::settings::Settings::allow_private_network_connections`].
+    /// Intended for destinations taken from the endpoint configuration (and thus trusted),
+    /// not from a client request.
+    pub(crate) fn new_for_configured_destination(context: Arc<core::Context>) -> Self {
+        Self {
+            context,
+            allow_private_network: true,
+        }
     }
 
     pub(crate) fn pipe_from_stream(
@@ -65,12 +82,12 @@ impl TcpConnector for TcpForwarder {
         id: log_utils::IdChain<u64>,
         meta: forwarder::TcpConnectionMeta,
     ) -> Result<(Box<dyn pipe::Source>, Box<dyn pipe::Sink>), tunnel::ConnectionError> {
+        let allow_private_network =
+            self.allow_private_network || self.context.settings.allow_private_network_connections;
         let peer = match meta.destination {
             TcpDestination::Address(peer) => {
                 let peer_ip = peer.ip();
-                if !self.context.settings.allow_private_network_connections
-                    && !net_utils::is_global_ip(&peer_ip)
-                {
+                if !allow_private_network && !net_utils::is_global_ip(&peer_ip) {
                     if peer_ip.is_loopback() {
                         return Err(tunnel::ConnectionError::DnsLoopback);
                     }
@@ -99,9 +116,7 @@ impl TcpConnector for TcpForwarder {
                         continue;
                     }
 
-                    if net_utils::is_global_ip(&ip)
-                        || self.context.settings.allow_private_network_connections
-                    {
+                    if net_utils::is_global_ip(&ip) || allow_private_network {
                         status = Some(SelectionStatus::Suitable(a));
                         break;
                     }
@@ -330,6 +345,32 @@ mod tests {
         assert!(matches!(err, tunnel::ConnectionError::DnsNonroutable));
     }
 
+    #[tokio::test]
+    async fn test_connect_allows_loopback_for_configured_destination() {
+        let listener = tokio::net::TcpListener::bind((Ipv4Addr::LOCALHOST, 0))
+            .await
+            .unwrap();
+        let listen_address = listener.local_addr().unwrap();
+
+        let context = make_test_context_disallow_private_network();
+        let connector: Box<dyn TcpConnector> =
+            Box::new(TcpForwarder::new_for_configured_destination(context));
+
+        let meta = forwarder::TcpConnectionMeta {
+            client_address: IpAddr::V4(Ipv4Addr::new(203, 0, 113, 1)),
+            destination: TcpDestination::Address(listen_address),
+            auth: None,
+            tls_domain: String::new(),
+            user_agent: None,
+        };
+
+        connector
+            .connect(log_utils::IdChain::empty(), meta)
+            .await
+            .map_err(|e| format!("{}", e))
+            .expect("Expected connection to be allowed");
+    }
+
     #[tokio::test]
     async fn test_connect_denies_ipv4_mapped_loopback() {
         let context = make_test_context_disallow_private_network();

lib/tests/reverse_proxy.rs

@@ -35,7 +35,7 @@ macro_rules! reverse_proxy_tests {
                 assert_eq!(response.status, http::StatusCode::OK);
                 assert_body_matches(&body);
             };
-            let endpoint_task = run_endpoint(&endpoint_address, &proxy_address);
+            let endpoint_task = run_endpoint(&endpoint_address, &proxy_address, true);
 
             // Pin tasks so they can be polled across multiple select! invocations
             // without being dropped (dropping run_endpoint mid-transfer would tear
@@ -84,7 +84,7 @@ async fn path_h2_chunked() {
         assert_eq!(response.status, http::StatusCode::OK);
         assert_body_matches(&body);
     };
-    let endpoint_task = run_endpoint(&endpoint_address, &proxy_address);
+    let endpoint_task = run_endpoint(&endpoint_address, &proxy_address, true);
 
     tokio::pin!(client_task);
     tokio::pin!(proxy_task);
@@ -118,7 +118,44 @@ async fn path_h3_chunked() {
         assert_eq!(response.status, http::StatusCode::OK);
         assert_body_matches(&body);
     };
-    let endpoint_task = run_endpoint(&endpoint_address, &proxy_address);
+    let endpoint_task = run_endpoint(&endpoint_address, &proxy_address, true);
+
+    tokio::pin!(client_task);
+    tokio::pin!(proxy_task);
+    tokio::pin!(endpoint_task);
+
+    tokio::select! {
+        _ = &mut endpoint_task => unreachable!(),
+        _ = tokio::time::sleep(Duration::from_secs(10)) => panic!("Timed out"),
+        _ = &mut client_task => (),
+        _ = &mut proxy_task => {
+            tokio::select! {
+                _ = client_task => (),
+                _ = &mut endpoint_task => unreachable!(),
+                _ = tokio::time::sleep(Duration::from_secs(5)) => {
+                    panic!("Client timed out after proxy completed")
+                }
+            }
+        },
+    }
+}
+
+// The reverse proxy server address comes from the endpoint configuration, so it must be
+// reachable even if it belongs to a private network and client connections to private
+// networks are forbidden.
+#[tokio::test]
+async fn path_h1_private_network_disallowed() {
+    common::set_up_logger();
+    let endpoint_address = common::make_endpoint_address();
+    let (proxy_address, proxy_task) = run_proxy();
+
+    let client_task = async {
+        tokio::time::sleep(Duration::from_secs(1)).await;
+        let (response, body) = path_h1_client(&endpoint_address).await;
+        assert_eq!(response.status, http::StatusCode::OK);
+        assert_body_matches(&body);
+    };
+    let endpoint_task = run_endpoint(&endpoint_address, &proxy_address, false);
 
     tokio::pin!(client_task);
     tokio::pin!(proxy_task);
@@ -246,7 +283,11 @@ async fn path_h3_client(endpoint_address: &SocketAddr) -> (http::response::Parts
     .await
 }
 
-async fn run_endpoint(endpoint_address: &SocketAddr, proxy_address: &SocketAddr) {
+async fn run_endpoint(
+    endpoint_address: &SocketAddr,
+    proxy_address: &SocketAddr,
+    allow_private_network_connections: bool,
+) {
     let settings = Settings::builder()
         .listen_address(endpoint_address)
         .unwrap()
@@ -263,7 +304,7 @@ async fn run_endpoint(endpoint_address: &SocketAddr, proxy_address: &SocketAddr)
                 .build()
                 .unwrap(),
         )
-        .allow_private_network_connections(true)
+        .allow_private_network_connections(allow_private_network_connections)
         .build()
         .unwrap();