diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 9cf543bdf..84358ea67 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -493,6 +493,8 @@ pub enum PolicyViolation { FieldTooLong { path: String, length: usize }, /// Too many filesystem paths in the policy. TooManyPaths { count: usize }, + /// A network endpoint uses a TLD wildcard (e.g. `*.com`). + TldWildcard { policy_name: String, host: String }, } impl fmt::Display for PolicyViolation { @@ -522,6 +524,13 @@ impl fmt::Display for PolicyViolation { "too many filesystem paths ({count} > {MAX_FILESYSTEM_PATHS})" ) } + Self::TldWildcard { policy_name, host } => { + write!( + f, + "network policy '{policy_name}': TLD wildcard '{host}' is not allowed; \ + use subdomain wildcards like '*.example.com' instead" + ) + } } } } @@ -539,6 +548,7 @@ impl fmt::Display for PolicyViolation { /// - Read-write paths must not be overly broad (just `/`) /// - Individual path lengths must not exceed [`MAX_PATH_LENGTH`] /// - Total path count must not exceed [`MAX_FILESYSTEM_PATHS`] +/// - Network endpoint hosts must not use TLD wildcards (e.g. `*.com`) pub fn validate_sandbox_policy( policy: &SandboxPolicy, ) -> std::result::Result<(), Vec> { @@ -608,6 +618,26 @@ pub fn validate_sandbox_policy( } } + // Check network policy endpoint hosts for TLD wildcards. + for (key, rule) in &policy.network_policies { + let name = if rule.name.is_empty() { + key.clone() + } else { + rule.name.clone() + }; + for ep in &rule.endpoints { + if ep.host.contains('*') && (ep.host.starts_with("*.") || ep.host.starts_with("**.")) { + let label_count = ep.host.split('.').count(); + if label_count <= 2 { + violations.push(PolicyViolation::TldWildcard { + policy_name: name.clone(), + host: ep.host.clone(), + }); + } + } + } + } + if violations.is_empty() { Ok(()) } else { @@ -1058,6 +1088,88 @@ network_policies: ); } + #[test] + fn validate_rejects_tld_wildcard() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "bad".into(), + NetworkPolicyRule { + name: "bad-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "*.com".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + let violations = validate_sandbox_policy(&policy).unwrap_err(); + assert!( + violations + .iter() + .any(|v| matches!(v, PolicyViolation::TldWildcard { .. })) + ); + } + + #[test] + fn validate_rejects_double_star_tld_wildcard() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "bad".into(), + NetworkPolicyRule { + name: "bad-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "**.org".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + let violations = validate_sandbox_policy(&policy).unwrap_err(); + assert!( + violations + .iter() + .any(|v| matches!(v, PolicyViolation::TldWildcard { .. })) + ); + } + + #[test] + fn validate_accepts_subdomain_wildcard() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "ok".into(), + NetworkPolicyRule { + name: "ok-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "*.example.com".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + assert!(validate_sandbox_policy(&policy).is_ok()); + } + + #[test] + fn validate_accepts_explicit_domain() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "ok".into(), + NetworkPolicyRule { + name: "ok-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "example.com".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + assert!(validate_sandbox_policy(&policy).is_ok()); + } + #[test] fn normalize_path_collapses_separators() { assert_eq!(normalize_path("/usr//lib"), "/usr/lib"); diff --git a/crates/openshell-sandbox/src/l7/mod.rs b/crates/openshell-sandbox/src/l7/mod.rs index ca76dc47a..af403aead 100644 --- a/crates/openshell-sandbox/src/l7/mod.rs +++ b/crates/openshell-sandbox/src/l7/mod.rs @@ -255,11 +255,14 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec, Vec< "{loc}: host wildcard must start with '*.' or '**.' (e.g., '*.example.com'), got '{host}'" )); } else { - // Warn on very broad wildcards like *.com (2 labels) + // Reject TLD wildcards like *.com (2 labels) — they are + // accepted by the policy engine but silently fail at the + // proxy layer (see #787). let label_count = host.split('.').count(); if label_count <= 2 { - warnings.push(format!( - "{loc}: host wildcard '{host}' is very broad (covers all subdomains of a TLD)" + errors.push(format!( + "{loc}: TLD wildcard '{host}' is not allowed; \ + use subdomain wildcards like '*.example.com' instead" )); } } @@ -849,7 +852,7 @@ mod tests { } #[test] - fn validate_wildcard_host_broad_warning() { + fn validate_wildcard_host_tld_rejected() { let data = serde_json::json!({ "network_policies": { "test": { @@ -861,11 +864,30 @@ mod tests { } } }); - let (errors, warnings) = validate_l7_policies(&data); - assert!(errors.is_empty(), "*.com should not error: {errors:?}"); + let (errors, _warnings) = validate_l7_policies(&data); + assert!( + errors.iter().any(|e| e.contains("TLD wildcard")), + "*.com should be rejected as TLD wildcard, got errors: {errors:?}" + ); + } + + #[test] + fn validate_wildcard_host_double_star_tld_rejected() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "**.org", + "port": 443 + }], + "binaries": [] + } + } + }); + let (errors, _warnings) = validate_l7_policies(&data); assert!( - warnings.iter().any(|w| w.contains("very broad")), - "*.com should warn about breadth, got warnings: {warnings:?}" + errors.iter().any(|e| e.contains("TLD wildcard")), + "**.org should be rejected as TLD wildcard, got errors: {errors:?}" ); } diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 79cca7e54..8d53da6a0 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -795,6 +795,28 @@ mod tests { assert!(validate_policy_safety(&policy).is_ok()); } + #[test] + fn validate_policy_safety_rejects_tld_wildcard() { + use openshell_core::proto::{NetworkEndpoint, NetworkPolicyRule}; + + let mut policy = openshell_policy::restrictive_default_policy(); + policy.network_policies.insert( + "bad".into(), + NetworkPolicyRule { + name: "bad-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "*.com".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + let err = validate_policy_safety(&policy).unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("TLD wildcard")); + } + // ---- Static field validation ---- #[test]