DG26-11: Gateway setup - Lack of server-side data validation

crates/defguard_common/src/utils.rs

@@ -1,5 +1,7 @@
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
+use url::Url;
+
 use ipnetwork::IpNetwork;
 use serde::Serialize;
 
@@ -12,6 +14,37 @@ pub fn strip_scheme(s: &str) -> &str {
         .unwrap_or(s)
 }
 
+/// Validates that `ip_or_domain` is a bare hostname or IP address with no embedded port,
+/// path, query string, or fragment.
+/// Intended for the `ip_or_domain` fields of Gateway and Edge setup requests.
+pub fn validate_host_only(ip_or_domain: &str) -> Result<(), String> {
+    let test_url = format!("http://{ip_or_domain}/");
+    let url = Url::parse(&test_url)
+        .map_err(|_| format!("'{ip_or_domain}' is not a valid hostname or IP address"))?;
+    if url.host_str().is_none() {
+        return Err(format!(
+            "'{ip_or_domain}' is not a valid hostname or IP address"
+        ));
+    }
+    if url.port().is_some() {
+        return Err(format!(
+            "'{ip_or_domain}' must not include a port; provide the port in the grpc_port field"
+        ));
+    }
+    if url.path() != "/" {
+        return Err(format!(
+            "'{ip_or_domain}' must not include a path component"
+        ));
+    }
+    if url.query().is_some() {
+        return Err(format!("'{ip_or_domain}' must not include a query string"));
+    }
+    if url.fragment().is_some() {
+        return Err(format!("'{ip_or_domain}' must not include a fragment"));
+    }
+    Ok(())
+}
+
 /// Parse a string with comma-separated IP addresses.
 /// Invalid addresses will be silently ignored.
 #[must_use]
@@ -165,4 +198,60 @@ mod test {
         assert_eq!(net.modifiable_part, "aaaa");
         assert_eq!(net.network_prefix, "125");
     }
+
+    #[test]
+    fn test_validate_host_only_accepts_bare_ipv4() {
+        assert!(validate_host_only("192.168.1.1").is_ok());
+        assert!(validate_host_only("10.0.0.1").is_ok());
+    }
+
+    #[test]
+    fn test_validate_host_only_accepts_bare_ipv6() {
+        assert!(validate_host_only("[::1]").is_ok());
+        assert!(validate_host_only("[2001:db8::1]").is_ok());
+    }
+
+    #[test]
+    fn test_validate_host_only_accepts_bare_hostname() {
+        assert!(validate_host_only("gateway.example.com").is_ok());
+        assert!(validate_host_only("my-gateway").is_ok());
+        assert!(validate_host_only("edge01.vpn.internal").is_ok());
+    }
+
+    #[test]
+    fn test_validate_host_only_rejects_embedded_port() {
+        let err = validate_host_only("192.168.1.1:4444").unwrap_err();
+        assert!(err.contains("must not include a port"), "got: {err}");
+
+        let err = validate_host_only("gateway.example.com:8443").unwrap_err();
+        assert!(err.contains("must not include a port"), "got: {err}");
+    }
+
+    #[test]
+    fn test_validate_host_only_rejects_path() {
+        let err = validate_host_only("192.168.1.1/testpath").unwrap_err();
+        assert!(err.contains("must not include a path"), "got: {err}");
+    }
+
+    #[test]
+    fn test_validate_host_only_rejects_query_string() {
+        // A bare query string without a path separator is not reachable via URL
+        // parsing in practice, but a port+path+query is. The port check fires first.
+        let err = validate_host_only("192.168.1.1:4444/path?a=b").unwrap_err();
+        assert!(err.contains("must not include a port"), "got: {err}");
+    }
+
+    #[test]
+    fn dg26_11_test_validate_host_only_rejects_fragment() {
+        let err = validate_host_only("192.168.1.1#frag").unwrap_err();
+        assert!(err.contains("must not include a fragment"), "got: {err}");
+    }
+
+    #[test]
+    fn dg26_11_test_validate_host_only_rejects_poc_input() {
+        // Exact input from the DG26-11 audit report (after URL-decode of %23 -> #).
+        // The embedded port (:4444) is the first violation the validator encounters.
+        let err = validate_host_only("46.101.217.165:4444/testpath?a=b#").unwrap_err();
+        assert!(err.contains("must not include a port"), "got: {err}");
+    }
 }

crates/defguard_core/src/handlers/component_setup.rs

@@ -28,7 +28,7 @@ use defguard_common::{
         },
     },
     types::proxy::ProxyControlMessage,
-    utils::strip_scheme,
+    utils::{strip_scheme, validate_host_only},
 };
 use defguard_proto::{
     common::{CertBundle, CertificateInfo},
@@ -265,6 +265,17 @@ pub async fn setup_proxy_tls_stream(
 
         let ip_or_domain = strip_scheme(&request.ip_or_domain);
 
+        if let Err(msg) = validate_host_only(ip_or_domain) {
+            yield Ok(flow.error(&msg));
+            return;
+        }
+
+        // `u16` covers the expected port range, so we just have to check for 0
+        if request.grpc_port == 0 {
+            yield Ok(flow.error("grpc_port must not be 0"));
+            return;
+        }
+
         // Step 1: Check configuration
         yield Ok(flow.step(SetupStep::CheckingConfiguration));
         match Proxy::find_by_address_port(&pool, ip_or_domain, i32::from(request.grpc_port)).await {
@@ -703,6 +714,13 @@ async fn perform_gateway_adoption(
 
     let ip_or_domain = strip_scheme(&ip_or_domain_raw);
 
+    validate_host_only(ip_or_domain)?;
+
+    // `u16` covers the expected port range, so we just have to check for 0
+    if grpc_port == 0 {
+        return Err("grpc_port must not be 0".to_string());
+    }
+
     // License check: non-enterprise installs are limited to one gateway per network.
     if !is_enterprise_license_active() {
         let gateways = Gateway::find_by_location_id(pool, network_id)

crates/defguard_core/tests/integration/api/component_setup.rs

@@ -443,8 +443,8 @@ async fn test_adopt_gateway_rest_bad_address(_: PgPoolOptions, options: PgConnec
     assert!(
         body.get("msg")
             .and_then(Value::as_str)
-            .is_some_and(|msg| msg.contains("Invalid URL")),
-        "expected invalid URL error, got: {body}"
+            .is_some_and(|msg| msg.contains("not a valid hostname or IP address")),
+        "expected invalid host error, got: {body}"
     );
 }
 
@@ -579,3 +579,212 @@ async fn test_adopt_gateway_sse(_: PgPoolOptions, options: PgConnectOptions) {
         "expected gateway in DB to have a certificate serial"
     );
 }
+
+#[sqlx::test]
+async fn dg26_11_test_gateway_setup_sse_rejects_host_with_path(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (client, network_id) = setup_test_no_ca(&pool).await;
+
+    // Exact PoC from DG26-11: embedded port, path, query string, and a fragment
+    // that would swallow grpc_port when naively concatenated into a URL.
+    // %3A = ':', %2F = '/', %3F = '?', %3D = '=', %23 = '#' (percent-encoded so
+    // the query parser passes them through into ip_or_domain rather than treating
+    // them as separate query parameters).
+    let response = client
+        .get(format!(
+            "/api/v1/network/{network_id}/gateways/setup\
+             ?ip_or_domain=46.101.217.165%3A4444%2Ftestpath%3Fa%3Db%23\
+             &grpc_port=50061&common_name=test"
+        ))
+        .send()
+        .await;
+
+    assert_eq!(response.status(), StatusCode::OK); // SSE always 200
+
+    let body = response.text().await;
+    let events = parse_sse_data_events(&body);
+
+    let error_event = events
+        .iter()
+        .find(|e| e.get("error") == Some(&Value::Bool(true)))
+        .expect("expected an error SSE event");
+
+    let msg = error_event
+        .get("message")
+        .and_then(Value::as_str)
+        .unwrap_or("");
+    assert!(
+        msg.contains("must not include a port"),
+        "expected host validation error, got: {msg:?}"
+    );
+}
+
+#[sqlx::test]
+async fn dg26_11_test_gateway_setup_sse_rejects_port_zero(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (client, network_id) = setup_test_no_ca(&pool).await;
+
+    let response = client
+        .get(format!(
+            "/api/v1/network/{network_id}/gateways/setup\
+             ?ip_or_domain=127.0.0.1&grpc_port=0&common_name=test"
+        ))
+        .send()
+        .await;
+
+    assert_eq!(response.status(), StatusCode::OK);
+
+    let body = response.text().await;
+    let events = parse_sse_data_events(&body);
+
+    let error_event = events
+        .iter()
+        .find(|e| e.get("error") == Some(&Value::Bool(true)))
+        .expect("expected an error SSE event");
+
+    let msg = error_event
+        .get("message")
+        .and_then(Value::as_str)
+        .unwrap_or("");
+    assert!(
+        msg.contains("grpc_port must not be 0"),
+        "expected port-zero validation error, got: {msg:?}"
+    );
+}
+
+#[sqlx::test]
+async fn dg26_11_test_adopt_gateway_rest_rejects_host_with_path(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (client, network_id) = setup_test_no_ca(&pool).await;
+
+    let response = client
+        .post(format!("/api/v1/network/{network_id}/gateways/adopt"))
+        .json(&serde_json::json!({
+            "name": "TestGateway",
+            "ip_or_domain": "46.101.217.165:4444/testpath?a=b#",
+            "grpc_port": 50061
+        }))
+        .send()
+        .await;
+
+    assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+    let body: Value = response.json().await;
+    assert!(
+        body.get("msg")
+            .and_then(Value::as_str)
+            .is_some_and(|msg| msg.contains("must not include a port")),
+        "expected host validation error, got: {body}"
+    );
+}
+
+#[sqlx::test]
+async fn dg26_11_test_adopt_gateway_rest_rejects_port_zero(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (client, network_id) = setup_test_no_ca(&pool).await;
+
+    let response = client
+        .post(format!("/api/v1/network/{network_id}/gateways/adopt"))
+        .json(&serde_json::json!({
+            "name": "TestGateway",
+            "ip_or_domain": "127.0.0.1",
+            "grpc_port": 0
+        }))
+        .send()
+        .await;
+
+    assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+    let body: Value = response.json().await;
+    assert!(
+        body.get("msg")
+            .and_then(Value::as_str)
+            .is_some_and(|msg| msg.contains("grpc_port must not be 0")),
+        "expected port-zero validation error, got: {body}"
+    );
+}
+
+#[sqlx::test]
+async fn dg26_11_test_proxy_setup_sse_rejects_host_with_path(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (mut client, _) = make_test_client(pool).await;
+    client.login_user("admin", "pass123").await;
+
+    let response = client
+        .get(
+            "/api/v1/proxy/setup/stream\
+             ?ip_or_domain=46.101.217.165%3A4444%2Ftestpath%3Fa%3Db%23\
+             &grpc_port=50061&common_name=test",
+        )
+        .send()
+        .await;
+
+    assert_eq!(response.status(), StatusCode::OK);
+
+    let body = response.text().await;
+    let events = parse_sse_data_events(&body);
+
+    let error_event = events
+        .iter()
+        .find(|e| e.get("error") == Some(&Value::Bool(true)))
+        .expect("expected an error SSE event");
+
+    let msg = error_event
+        .get("message")
+        .and_then(Value::as_str)
+        .unwrap_or("");
+    assert!(
+        msg.contains("must not include a port"),
+        "expected host validation error, got: {msg:?}"
+    );
+}
+
+#[sqlx::test]
+async fn dg26_11_test_proxy_setup_sse_rejects_port_zero(
+    _: PgPoolOptions,
+    options: PgConnectOptions,
+) {
+    let pool = setup_pool(options).await;
+    let (mut client, _) = make_test_client(pool).await;
+    client.login_user("admin", "pass123").await;
+
+    let response = client
+        .get(
+            "/api/v1/proxy/setup/stream\
+             ?ip_or_domain=127.0.0.1&grpc_port=0&common_name=test",
+        )
+        .send()
+        .await;
+
+    assert_eq!(response.status(), StatusCode::OK);
+
+    let body = response.text().await;
+    let events = parse_sse_data_events(&body);
+
+    let error_event = events
+        .iter()
+        .find(|e| e.get("error") == Some(&Value::Bool(true)))
+        .expect("expected an error SSE event");
+
+    let msg = error_event
+        .get("message")
+        .and_then(Value::as_str)
+        .unwrap_or("");
+    assert!(
+        msg.contains("grpc_port must not be 0"),
+        "expected port-zero validation error, got: {msg:?}"
+    );
+}