diff --git a/crates/defguard_common/src/utils.rs b/crates/defguard_common/src/utils.rs index b8e795de7..74d7d29c9 100644 --- a/crates/defguard_common/src/utils.rs +++ b/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}"); + } } diff --git a/crates/defguard_core/src/handlers/component_setup.rs b/crates/defguard_core/src/handlers/component_setup.rs index 26990300e..b5aee72f1 100644 --- a/crates/defguard_core/src/handlers/component_setup.rs +++ b/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) diff --git a/crates/defguard_core/tests/integration/api/component_setup.rs b/crates/defguard_core/tests/integration/api/component_setup.rs index 472b3dd4e..33f7d3232 100644 --- a/crates/defguard_core/tests/integration/api/component_setup.rs +++ b/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:?}" + ); +}