Skip to content

Commit c36faac

Browse files
committed
test(sandbox): reject malformed proxy hostnames
1 parent cd70249 commit c36faac

2 files changed

Lines changed: 119 additions & 2 deletions

File tree

crates/openshell-sandbox/src/opa.rs

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ pub struct NetworkInput {
5252
pub cmdline_paths: Vec<PathBuf>,
5353
}
5454

55+
pub(crate) fn network_host_is_safe(host: &str) -> bool {
56+
!host.is_empty()
57+
&& !host.chars().any(|ch| {
58+
ch.is_ascii_control() || ch.is_ascii_whitespace() || matches!(ch, '%' | '/' | '\\')
59+
})
60+
}
61+
5562
/// Sandbox configuration extracted from OPA data at startup.
5663
pub struct SandboxConfig {
5764
pub filesystem: FilesystemPolicy,
@@ -239,6 +246,14 @@ impl OpaEngine {
239246
/// `allow_network` rule, and returns a `PolicyDecision` with the result,
240247
/// deny reason, and matched policy name.
241248
pub fn evaluate_network(&self, input: &NetworkInput) -> Result<PolicyDecision> {
249+
if !network_host_is_safe(&input.host) {
250+
return Ok(PolicyDecision {
251+
allowed: false,
252+
reason: "invalid network host".to_string(),
253+
matched_policy: None,
254+
});
255+
}
256+
242257
let ancestor_strs: Vec<String> = input
243258
.ancestors
244259
.iter()
@@ -309,6 +324,16 @@ impl OpaEngine {
309324
&self,
310325
input: &NetworkInput,
311326
) -> Result<(NetworkAction, u64)> {
327+
let generation = self.current_generation();
328+
if !network_host_is_safe(&input.host) {
329+
return Ok((
330+
NetworkAction::Deny {
331+
reason: "invalid network host".to_string(),
332+
},
333+
generation,
334+
));
335+
}
336+
312337
let ancestor_strs: Vec<String> = input
313338
.ancestors
314339
.iter()
@@ -335,7 +360,6 @@ impl OpaEngine {
335360
.engine
336361
.lock()
337362
.map_err(|_| miette::miette!("OPA engine lock poisoned"))?;
338-
let generation = self.current_generation();
339363

340364
engine
341365
.set_input_json(&input_json.to_string())
@@ -3966,6 +3990,71 @@ network_policies:
39663990
);
39673991
}
39683992

3993+
#[test]
3994+
fn wildcard_host_rejects_malformed_input_hosts() {
3995+
let data = r#"
3996+
network_policies:
3997+
wildcard:
3998+
name: wildcard
3999+
endpoints:
4000+
- { host: "*.example.com", port: 443 }
4001+
binaries:
4002+
- { path: /usr/bin/curl }
4003+
"#;
4004+
let engine = OpaEngine::from_strings(TEST_POLICY, data).unwrap();
4005+
4006+
for host in [
4007+
"api%00.example.com",
4008+
"api%2eexample.com",
4009+
"api.example.com\u{0}",
4010+
"api.example.com\t",
4011+
"api.example.com/path",
4012+
"api.example.com\\path",
4013+
] {
4014+
let input = NetworkInput {
4015+
host: host.into(),
4016+
port: 443,
4017+
binary_path: PathBuf::from("/usr/bin/curl"),
4018+
binary_sha256: "unused".into(),
4019+
ancestors: vec![],
4020+
cmdline_paths: vec![],
4021+
};
4022+
let decision = engine.evaluate_network(&input).unwrap();
4023+
assert!(!decision.allowed, "malformed host {host:?} must deny");
4024+
assert_eq!(decision.reason, "invalid network host");
4025+
assert_eq!(decision.matched_policy, None);
4026+
}
4027+
}
4028+
4029+
#[test]
4030+
fn network_action_rejects_malformed_input_host_before_policy_allow() {
4031+
let data = r#"
4032+
network_policies:
4033+
wildcard:
4034+
name: wildcard
4035+
endpoints:
4036+
- { host: "*.example.com", port: 443 }
4037+
binaries:
4038+
- { path: /usr/bin/curl }
4039+
"#;
4040+
let engine = OpaEngine::from_strings(TEST_POLICY, data).unwrap();
4041+
let input = NetworkInput {
4042+
host: "api%00.example.com".into(),
4043+
port: 443,
4044+
binary_path: PathBuf::from("/usr/bin/curl"),
4045+
binary_sha256: "unused".into(),
4046+
ancestors: vec![],
4047+
cmdline_paths: vec![],
4048+
};
4049+
4050+
assert_eq!(
4051+
engine.evaluate_network_action(&input).unwrap(),
4052+
NetworkAction::Deny {
4053+
reason: "invalid network host".to_string()
4054+
}
4055+
);
4056+
}
4057+
39694058
#[test]
39704059
fn wildcard_host_plus_port() {
39714060
let data = r#"

crates/openshell-sandbox/src/proxy.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use crate::denial_aggregator::DenialEvent;
77
use crate::identity::BinaryIdentityCache;
88
use crate::l7::tls::ProxyTlsState;
9-
use crate::opa::{NetworkAction, OpaEngine, PolicyGenerationGuard};
9+
use crate::opa::{NetworkAction, OpaEngine, PolicyGenerationGuard, network_host_is_safe};
1010
use crate::policy::ProxyPolicy;
1111
use crate::policy_local::{POLICY_LOCAL_HOST, PolicyLocalContext};
1212
use crate::provider_credentials::ProviderCredentialState;
@@ -3618,6 +3618,9 @@ fn parse_target(target: &str) -> Result<(String, u16)> {
36183618
let (host, port_str) = target
36193619
.split_once(':')
36203620
.ok_or_else(|| miette::miette!("CONNECT target missing port: {target}"))?;
3621+
if !network_host_is_safe(host) {
3622+
return Err(miette::miette!("Invalid host in CONNECT target: {target}"));
3623+
}
36213624
let port: u16 = port_str
36223625
.parse()
36233626
.map_err(|_| miette::miette!("Invalid port in CONNECT target: {target}"))?;
@@ -5258,6 +5261,31 @@ network_policies:
52585261
assert!(!msg.contains("/etc/openshell"));
52595262
}
52605263

5264+
#[test]
5265+
fn parse_target_accepts_plain_authority() {
5266+
let (host, port) = parse_target("api.example.com:443").expect("parse CONNECT target");
5267+
assert_eq!(host, "api.example.com");
5268+
assert_eq!(port, 443);
5269+
}
5270+
5271+
#[test]
5272+
fn parse_target_rejects_malformed_hostname_differentials() {
5273+
for target in [
5274+
"api%00.example.com:443",
5275+
"api%2eexample.com:443",
5276+
"api.example.com\u{0}:443",
5277+
"api.example.com\t:443",
5278+
"api.example.com/path:443",
5279+
"api.example.com\\path:443",
5280+
] {
5281+
let err = parse_target(target).expect_err("malformed CONNECT host must be rejected");
5282+
assert!(
5283+
format!("{err}").contains("Invalid host"),
5284+
"unexpected error for {target:?}: {err}"
5285+
);
5286+
}
5287+
}
5288+
52615289
#[test]
52625290
fn sanitize_response_headers_strips_hop_by_hop() {
52635291
let headers = vec![

0 commit comments

Comments
 (0)