Skip to content

Commit 4bd9e9f

Browse files
committed
fix(policy): reject TLD wildcard patterns in network policy endpoints
TLD wildcards like *.com were accepted by policy validation but silently failed at the proxy layer — 0 bytes returned, no denial logged. Now rejected at submission time with a clear error message. Closes #787
1 parent ddb85b1 commit 4bd9e9f

File tree

3 files changed

+164
-8
lines changed

3 files changed

+164
-8
lines changed

crates/openshell-policy/src/lib.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,8 @@ pub enum PolicyViolation {
493493
FieldTooLong { path: String, length: usize },
494494
/// Too many filesystem paths in the policy.
495495
TooManyPaths { count: usize },
496+
/// A network endpoint uses a TLD wildcard (e.g. `*.com`).
497+
TldWildcard { policy_name: String, host: String },
496498
}
497499

498500
impl fmt::Display for PolicyViolation {
@@ -522,6 +524,13 @@ impl fmt::Display for PolicyViolation {
522524
"too many filesystem paths ({count} > {MAX_FILESYSTEM_PATHS})"
523525
)
524526
}
527+
Self::TldWildcard { policy_name, host } => {
528+
write!(
529+
f,
530+
"network policy '{policy_name}': TLD wildcard '{host}' is not allowed; \
531+
use subdomain wildcards like '*.example.com' instead"
532+
)
533+
}
525534
}
526535
}
527536
}
@@ -539,6 +548,7 @@ impl fmt::Display for PolicyViolation {
539548
/// - Read-write paths must not be overly broad (just `/`)
540549
/// - Individual path lengths must not exceed [`MAX_PATH_LENGTH`]
541550
/// - Total path count must not exceed [`MAX_FILESYSTEM_PATHS`]
551+
/// - Network endpoint hosts must not use TLD wildcards (e.g. `*.com`)
542552
pub fn validate_sandbox_policy(
543553
policy: &SandboxPolicy,
544554
) -> std::result::Result<(), Vec<PolicyViolation>> {
@@ -608,6 +618,26 @@ pub fn validate_sandbox_policy(
608618
}
609619
}
610620

621+
// Check network policy endpoint hosts for TLD wildcards.
622+
for (key, rule) in &policy.network_policies {
623+
let name = if rule.name.is_empty() {
624+
key.clone()
625+
} else {
626+
rule.name.clone()
627+
};
628+
for ep in &rule.endpoints {
629+
if ep.host.contains('*') && (ep.host.starts_with("*.") || ep.host.starts_with("**.")) {
630+
let label_count = ep.host.split('.').count();
631+
if label_count <= 2 {
632+
violations.push(PolicyViolation::TldWildcard {
633+
policy_name: name.clone(),
634+
host: ep.host.clone(),
635+
});
636+
}
637+
}
638+
}
639+
}
640+
611641
if violations.is_empty() {
612642
Ok(())
613643
} else {
@@ -1058,6 +1088,88 @@ network_policies:
10581088
);
10591089
}
10601090

1091+
#[test]
1092+
fn validate_rejects_tld_wildcard() {
1093+
let mut policy = restrictive_default_policy();
1094+
policy.network_policies.insert(
1095+
"bad".into(),
1096+
NetworkPolicyRule {
1097+
name: "bad-rule".into(),
1098+
endpoints: vec![NetworkEndpoint {
1099+
host: "*.com".into(),
1100+
port: 443,
1101+
..Default::default()
1102+
}],
1103+
..Default::default()
1104+
},
1105+
);
1106+
let violations = validate_sandbox_policy(&policy).unwrap_err();
1107+
assert!(
1108+
violations
1109+
.iter()
1110+
.any(|v| matches!(v, PolicyViolation::TldWildcard { .. }))
1111+
);
1112+
}
1113+
1114+
#[test]
1115+
fn validate_rejects_double_star_tld_wildcard() {
1116+
let mut policy = restrictive_default_policy();
1117+
policy.network_policies.insert(
1118+
"bad".into(),
1119+
NetworkPolicyRule {
1120+
name: "bad-rule".into(),
1121+
endpoints: vec![NetworkEndpoint {
1122+
host: "**.org".into(),
1123+
port: 443,
1124+
..Default::default()
1125+
}],
1126+
..Default::default()
1127+
},
1128+
);
1129+
let violations = validate_sandbox_policy(&policy).unwrap_err();
1130+
assert!(
1131+
violations
1132+
.iter()
1133+
.any(|v| matches!(v, PolicyViolation::TldWildcard { .. }))
1134+
);
1135+
}
1136+
1137+
#[test]
1138+
fn validate_accepts_subdomain_wildcard() {
1139+
let mut policy = restrictive_default_policy();
1140+
policy.network_policies.insert(
1141+
"ok".into(),
1142+
NetworkPolicyRule {
1143+
name: "ok-rule".into(),
1144+
endpoints: vec![NetworkEndpoint {
1145+
host: "*.example.com".into(),
1146+
port: 443,
1147+
..Default::default()
1148+
}],
1149+
..Default::default()
1150+
},
1151+
);
1152+
assert!(validate_sandbox_policy(&policy).is_ok());
1153+
}
1154+
1155+
#[test]
1156+
fn validate_accepts_explicit_domain() {
1157+
let mut policy = restrictive_default_policy();
1158+
policy.network_policies.insert(
1159+
"ok".into(),
1160+
NetworkPolicyRule {
1161+
name: "ok-rule".into(),
1162+
endpoints: vec![NetworkEndpoint {
1163+
host: "example.com".into(),
1164+
port: 443,
1165+
..Default::default()
1166+
}],
1167+
..Default::default()
1168+
},
1169+
);
1170+
assert!(validate_sandbox_policy(&policy).is_ok());
1171+
}
1172+
10611173
#[test]
10621174
fn normalize_path_collapses_separators() {
10631175
assert_eq!(normalize_path("/usr//lib"), "/usr/lib");

crates/openshell-sandbox/src/l7/mod.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -255,11 +255,14 @@ pub fn validate_l7_policies(data_json: &serde_json::Value) -> (Vec<String>, Vec<
255255
"{loc}: host wildcard must start with '*.' or '**.' (e.g., '*.example.com'), got '{host}'"
256256
));
257257
} else {
258-
// Warn on very broad wildcards like *.com (2 labels)
258+
// Reject TLD wildcards like *.com (2 labels) — they are
259+
// accepted by the policy engine but silently fail at the
260+
// proxy layer (see #787).
259261
let label_count = host.split('.').count();
260262
if label_count <= 2 {
261-
warnings.push(format!(
262-
"{loc}: host wildcard '{host}' is very broad (covers all subdomains of a TLD)"
263+
errors.push(format!(
264+
"{loc}: TLD wildcard '{host}' is not allowed; \
265+
use subdomain wildcards like '*.example.com' instead"
263266
));
264267
}
265268
}
@@ -849,7 +852,7 @@ mod tests {
849852
}
850853

851854
#[test]
852-
fn validate_wildcard_host_broad_warning() {
855+
fn validate_wildcard_host_tld_rejected() {
853856
let data = serde_json::json!({
854857
"network_policies": {
855858
"test": {
@@ -861,11 +864,30 @@ mod tests {
861864
}
862865
}
863866
});
864-
let (errors, warnings) = validate_l7_policies(&data);
865-
assert!(errors.is_empty(), "*.com should not error: {errors:?}");
867+
let (errors, _warnings) = validate_l7_policies(&data);
868+
assert!(
869+
errors.iter().any(|e| e.contains("TLD wildcard")),
870+
"*.com should be rejected as TLD wildcard, got errors: {errors:?}"
871+
);
872+
}
873+
874+
#[test]
875+
fn validate_wildcard_host_double_star_tld_rejected() {
876+
let data = serde_json::json!({
877+
"network_policies": {
878+
"test": {
879+
"endpoints": [{
880+
"host": "**.org",
881+
"port": 443
882+
}],
883+
"binaries": []
884+
}
885+
}
886+
});
887+
let (errors, _warnings) = validate_l7_policies(&data);
866888
assert!(
867-
warnings.iter().any(|w| w.contains("very broad")),
868-
"*.com should warn about breadth, got warnings: {warnings:?}"
889+
errors.iter().any(|e| e.contains("TLD wildcard")),
890+
"**.org should be rejected as TLD wildcard, got errors: {errors:?}"
869891
);
870892
}
871893

crates/openshell-server/src/grpc/validation.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,28 @@ mod tests {
795795
assert!(validate_policy_safety(&policy).is_ok());
796796
}
797797

798+
#[test]
799+
fn validate_policy_safety_rejects_tld_wildcard() {
800+
use openshell_core::proto::{NetworkEndpoint, NetworkPolicyRule};
801+
802+
let mut policy = openshell_policy::restrictive_default_policy();
803+
policy.network_policies.insert(
804+
"bad".into(),
805+
NetworkPolicyRule {
806+
name: "bad-rule".into(),
807+
endpoints: vec![NetworkEndpoint {
808+
host: "*.com".into(),
809+
port: 443,
810+
..Default::default()
811+
}],
812+
..Default::default()
813+
},
814+
);
815+
let err = validate_policy_safety(&policy).unwrap_err();
816+
assert_eq!(err.code(), Code::InvalidArgument);
817+
assert!(err.message().contains("TLD wildcard"));
818+
}
819+
798820
// ---- Static field validation ----
799821

800822
#[test]

0 commit comments

Comments
 (0)