diff --git a/crates/native-sidecar-core/src/permissions.rs b/crates/native-sidecar-core/src/permissions.rs index 1ab663a8a2..e0a68e74bd 100644 --- a/crates/native-sidecar-core/src/permissions.rs +++ b/crates/native-sidecar-core/src/permissions.rs @@ -68,26 +68,31 @@ pub fn evaluate_permissions_policy( ), "network" => evaluate_pattern_permission_scope( permissions.network.as_ref(), + domain, capability_operation(capability, domain), resource, ), "child_process" => evaluate_pattern_permission_scope( permissions.child_process.as_ref(), + domain, capability_operation(capability, domain), resource, ), "process" => evaluate_pattern_permission_scope( permissions.process.as_ref(), + domain, capability_operation(capability, domain), resource, ), "env" => evaluate_pattern_permission_scope( permissions.env.as_ref(), + domain, capability_operation(capability, domain), resource, ), "binding" => evaluate_pattern_permission_scope( permissions.binding.as_ref(), + domain, capability_operation(capability, domain), resource, ), @@ -121,7 +126,7 @@ pub fn evaluate_matching_pattern_permission_policy( vm_config::PatternPermissionScope::Rules(rules) => rules .rules .iter() - .filter(|rule| pattern_rule_matches(rule, operation, resource)) + .filter(|rule| pattern_rule_matches(rule, domain, operation, resource)) .map(|rule| rule.mode) .next_back(), } @@ -149,6 +154,7 @@ fn evaluate_fs_permission_scope( fn evaluate_pattern_permission_scope( scope: Option<&vm_config::PatternPermissionScope>, + domain: &str, operation: &str, resource: Option<&str>, ) -> vm_config::PermissionMode { @@ -157,7 +163,7 @@ fn evaluate_pattern_permission_scope( Some(vm_config::PatternPermissionScope::Rules(rules)) => { let mut mode = rules.default.unwrap_or(vm_config::PermissionMode::Deny); for rule in &rules.rules { - if pattern_rule_matches(rule, operation, resource) { + if pattern_rule_matches(rule, domain, operation, resource) { mode = rule.mode; } } @@ -179,11 +185,15 @@ fn fs_rule_matches( fn pattern_rule_matches( rule: &vm_config::PatternPermissionRule, + domain: &str, operation: &str, resource: Option<&str>, ) -> bool { let operations_match = permission_operation_matches(&rule.operations, operation); - let patterns_match = permission_resource_matches(&rule.patterns, resource); + let patterns_match = match domain { + "network" => network_resource_matches(&rule.patterns, resource), + _ => permission_resource_matches(&rule.patterns, resource), + }; operations_match && patterns_match } @@ -201,6 +211,96 @@ fn permission_resource_matches(patterns: &[String], resource: Option<&str>) -> b }) } +/// Match `network` rule patterns against a kernel network resource. +/// +/// The kernel formats network resources as URIs before the policy check +/// (`tcp://host:port` from `format_tcp_resource`, `dns://host` from +/// `format_dns_resource`). Operators write rules in the documented host form +/// (`api.example.com`, `api.example.com:443`, `*.example.com`, `*`), which can +/// never equal a URI and whose single `*` cannot cross the `//`. Without this +/// translation an allowlist denies every host and a blocklist permits every +/// host. +/// +/// A pattern that carries a resource scheme keeps matching the full URI. A +/// scheme-less pattern is matched against the URI's host subject: the bare +/// host and, when the resource carries a port, `host:port`. Hosts are +/// case-insensitive, and the kernel lowercases `dns://` resources but not +/// `tcp://` ones, so both sides are lowercased before globbing; otherwise a +/// rule could apply to the DNS half of a connection and miss the TCP half. +/// +/// Only `tcp://`, `udp://`, and `dns://` resources have a host subject. Unix +/// socket resources (`unix:/path`, `unix:abstract:`, `unix://path`) are +/// matched by `unix:` patterns only, and any other resource shape is matched +/// by nothing but a full-URI pattern, so the unexpected fails closed. +fn network_resource_matches(patterns: &[String], resource: Option<&str>) -> bool { + let Some(resource) = resource else { + return false; + }; + let subject = network_resource_subject(resource); + patterns.iter().any(|pattern| { + if network_pattern_has_scheme(pattern) { + return permission_glob_matches(pattern, resource); + } + let Some(subject) = &subject else { + return false; + }; + let pattern = pattern.to_ascii_lowercase(); + permission_glob_matches(&pattern, &subject.host) + || subject + .host_port + .as_deref() + .is_some_and(|host_port| permission_glob_matches(&pattern, host_port)) + }) +} + +/// Lowercased host subject of a network resource URI: the host alone and, +/// when present, the `host:port` form. +struct NetworkResourceSubject { + host: String, + host_port: Option, +} + +/// Resource schemes the kernel and sidecar emit for `network` checks. A +/// pattern starting with one of these is a full-URI pattern. +const NETWORK_RESOURCE_SCHEMES: [&str; 4] = ["tcp:", "udp:", "dns:", "unix:"]; + +fn network_pattern_has_scheme(pattern: &str) -> bool { + NETWORK_RESOURCE_SCHEMES + .iter() + .any(|scheme| pattern.starts_with(scheme)) +} + +fn network_resource_subject(resource: &str) -> Option { + let rest = ["tcp://", "udp://", "dns://"] + .iter() + .find_map(|scheme| resource.strip_prefix(scheme))?; + if rest.is_empty() || rest.contains('/') { + return None; + } + let rest = rest.to_ascii_lowercase(); + // `host:port` when the suffix after the last `:` is a port number, or the + // `*` wildcard that listener-inspection resources use for "any port". The + // host is kept as the kernel formatted it, so an IPv6 literal such as + // `tcp://::1:443` yields the host `::1`, which is also the pattern form an + // operator writes for it. + match rest.rsplit_once(':') { + Some((host, port)) + if !host.is_empty() + && !port.is_empty() + && (port == "*" || port.bytes().all(|b| b.is_ascii_digit())) => + { + Some(NetworkResourceSubject { + host: host.to_owned(), + host_port: Some(rest.clone()), + }) + } + _ => Some(NetworkResourceSubject { + host: rest.clone(), + host_port: None, + }), + } +} + pub fn validate_permissions_policy( permissions: &vm_config::PermissionsPolicy, ) -> Result<(), SidecarCoreError> { @@ -480,7 +580,7 @@ mod tests { &policy, "network", "network.http", - Some("198.51.100.7:443"), + Some("tcp://198.51.100.7:443"), ), None, ); @@ -489,7 +589,7 @@ mod tests { &policy, "network", "network.http", - Some("203.0.113.9:443"), + Some("tcp://203.0.113.9:443"), ), Some(vm_config::PermissionMode::Allow), ); diff --git a/crates/native-sidecar-core/tests/network_permissions.rs b/crates/native-sidecar-core/tests/network_permissions.rs new file mode 100644 index 0000000000..1ae0241ad9 --- /dev/null +++ b/crates/native-sidecar-core/tests/network_permissions.rs @@ -0,0 +1,423 @@ +//! Network permission rules must honor the documented pattern form. +//! +//! The kernel formats network resources as URIs (`tcp://host:port`, +//! `dns://host`) before the policy check, while the documented rule form is a +//! bare host or `host:port`. A scheme-less pattern must therefore be matched +//! against the host subject of the URI, and a pattern that carries a scheme +//! must keep matching the full URI. Everything else fails closed. + +use agentos_native_sidecar_core::permissions::{ + evaluate_matching_pattern_permission_policy, evaluate_permissions_policy, +}; +use agentos_vm_config::{ + PatternPermissionRule, PatternPermissionRuleSet, PatternPermissionScope, PermissionMode, + PermissionsPolicy, +}; + +fn network_policy( + default: PermissionMode, + rules: Vec<(PermissionMode, &str)>, +) -> PermissionsPolicy { + PermissionsPolicy { + fs: None, + network: Some(PatternPermissionScope::Rules(PatternPermissionRuleSet { + default: Some(default), + rules: rules + .into_iter() + .map(|(mode, pattern)| PatternPermissionRule { + mode, + operations: vec![String::from("*")], + patterns: vec![String::from(pattern)], + }) + .collect(), + })), + child_process: None, + process: None, + env: None, + binding: None, + } +} + +fn http(policy: &PermissionsPolicy, resource: &str) -> PermissionMode { + evaluate_permissions_policy(policy, "network", "network.http", Some(resource)) +} + +fn dns(policy: &PermissionsPolicy, resource: &str) -> PermissionMode { + evaluate_permissions_policy(policy, "network", "network.dns", Some(resource)) +} + +// --- Documented host form under `default: deny` (the allowlist posture) ------ + +#[test] +fn bare_host_allow_rule_matches_tcp_resource_on_any_port() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "api.example.com")], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!( + http(&policy, "tcp://api.example.com:80"), + PermissionMode::Allow + ); + assert_eq!( + http(&policy, "tcp://other.example.com:443"), + PermissionMode::Deny + ); +} + +#[test] +fn host_port_allow_rule_matches_only_that_port() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "api.example.com:443")], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!( + http(&policy, "tcp://api.example.com:80"), + PermissionMode::Deny + ); +} + +#[test] +fn bare_host_allow_rule_matches_dns_resource() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "api.example.com")], + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Allow); + assert_eq!( + dns(&policy, "dns://other.example.com"), + PermissionMode::Deny + ); +} + +#[test] +fn subdomain_glob_matches_host_subject() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "*.example.com")], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Allow); + assert_eq!(http(&policy, "tcp://example.com:443"), PermissionMode::Deny); + assert_eq!( + http(&policy, "tcp://api.example.org:443"), + PermissionMode::Deny + ); +} + +#[test] +fn single_star_allow_rule_matches_every_host() { + let policy = network_policy(PermissionMode::Deny, vec![(PermissionMode::Allow, "*")]); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Allow); +} + +// --- Documented host form under `default: allow` (the blocklist posture) ----- +// +// This is the fail-open direction from the report: a deny rule that never +// matches silently permits every host. + +#[test] +fn bare_host_deny_rule_blocks_that_host_under_default_allow() { + let policy = network_policy( + PermissionMode::Allow, + vec![(PermissionMode::Deny, "example.com")], + ); + assert_eq!(http(&policy, "tcp://example.com:443"), PermissionMode::Deny); + assert_eq!(dns(&policy, "dns://example.com"), PermissionMode::Deny); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); +} + +#[test] +fn single_star_deny_rule_blocks_every_host_under_default_allow() { + let policy = network_policy(PermissionMode::Allow, vec![(PermissionMode::Deny, "*")]); + assert_eq!(http(&policy, "tcp://example.com:443"), PermissionMode::Deny); + assert_eq!(dns(&policy, "dns://example.com"), PermissionMode::Deny); +} + +// --- URI form keeps working exactly as before --------------------------------- + +#[test] +fn uri_patterns_still_match_the_full_resource() { + let policy = network_policy( + PermissionMode::Deny, + vec![ + (PermissionMode::Allow, "tcp://api.example.com:*"), + (PermissionMode::Allow, "dns://api.example.com"), + ], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Allow); + assert_eq!( + http(&policy, "tcp://other.example.com:443"), + PermissionMode::Deny + ); +} + +#[test] +fn uri_pattern_for_one_scheme_does_not_match_another_scheme() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "dns://api.example.com")], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Deny + ); +} + +#[test] +fn double_star_uri_pattern_still_matches_every_host() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "tcp://**")], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Deny); +} + +// --- Fail closed on anything the subject parser cannot understand ------------ + +#[test] +fn bare_pattern_does_not_match_a_resource_without_a_scheme_subject() { + // A resource the network layer never produces must not be matched by a + // host pattern through some accidental substring equivalence. + let policy = network_policy(PermissionMode::Deny, vec![(PermissionMode::Allow, "*")]); + assert_eq!(http(&policy, "unix:/run/agent.sock"), PermissionMode::Deny); + assert_eq!(http(&policy, ""), PermissionMode::Deny); +} + +#[test] +fn ipv6_literal_host_matches_the_kernel_formatted_subject() { + // The kernel formats IPv6 hosts unbracketed (`tcp://::1:8080`); only the + // trailing `:port` is split off, so the host subject is `::1`. + let policy = network_policy(PermissionMode::Deny, vec![(PermissionMode::Allow, "::1")]); + assert_eq!(http(&policy, "tcp://::1:8080"), PermissionMode::Allow); + assert_eq!(http(&policy, "tcp://fe80::1:8080"), PermissionMode::Deny); + let port_policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "::1:8080")], + ); + assert_eq!(http(&port_policy, "tcp://::1:8080"), PermissionMode::Allow); + assert_eq!(http(&port_policy, "tcp://::1:9090"), PermissionMode::Deny); +} + +// --- Last matching rule still wins, and host matching reaches the +// post-resolution evaluator too ------------------------------------------ + +#[test] +fn later_host_rule_overrides_earlier_wildcard_rule() { + let policy = network_policy( + PermissionMode::Deny, + vec![ + (PermissionMode::Allow, "*"), + (PermissionMode::Deny, "internal.example.com"), + ], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!( + http(&policy, "tcp://internal.example.com:443"), + PermissionMode::Deny + ); +} + +#[test] +fn matching_pattern_evaluation_uses_host_subject_for_resolved_addresses() { + let policy = network_policy( + PermissionMode::Allow, + vec![(PermissionMode::Deny, "203.0.113.*")], + ); + assert_eq!( + evaluate_matching_pattern_permission_policy( + &policy, + "network", + "network.http", + Some("tcp://203.0.113.9:443"), + ), + Some(PermissionMode::Deny) + ); + assert_eq!( + evaluate_matching_pattern_permission_policy( + &policy, + "network", + "network.http", + Some("tcp://198.51.100.7:443"), + ), + None + ); +} + +// --- Other pattern scopes are untouched --------------------------------------- + +#[test] +fn child_process_patterns_are_not_subject_parsed() { + let policy = PermissionsPolicy { + fs: None, + network: None, + child_process: Some(PatternPermissionScope::Rules(PatternPermissionRuleSet { + default: Some(PermissionMode::Deny), + rules: vec![PatternPermissionRule { + mode: PermissionMode::Allow, + operations: vec![String::from("spawn")], + patterns: vec![String::from("sh")], + }], + })), + process: None, + env: None, + binding: None, + }; + assert_eq!( + evaluate_permissions_policy(&policy, "child_process", "child_process.spawn", Some("sh")), + PermissionMode::Allow + ); + assert_eq!( + evaluate_permissions_policy( + &policy, + "child_process", + "child_process.spawn", + Some("tcp://sh:1") + ), + PermissionMode::Deny + ); +} + +// --- Host matching is case-insensitive on both sides --------------------------- +// +// DNS resources are lowercased by the kernel before the check while TCP +// resources keep the caller's spelling, so a case-sensitive host comparison +// would let one rule apply to the `dns://` half of a connection and miss the +// `tcp://` half. + +#[test] +fn host_pattern_case_does_not_matter() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "Api.Example.COM")], + ); + assert_eq!( + http(&policy, "tcp://api.example.com:443"), + PermissionMode::Allow + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Allow); +} + +#[test] +fn resource_host_case_does_not_matter() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "api.example.com:443")], + ); + assert_eq!( + http(&policy, "tcp://API.Example.com:443"), + PermissionMode::Allow + ); +} + +#[test] +fn deny_rule_blocks_both_halves_of_a_mixed_case_connection() { + let policy = network_policy( + PermissionMode::Allow, + vec![(PermissionMode::Deny, "API.example.com")], + ); + assert_eq!(dns(&policy, "dns://api.example.com"), PermissionMode::Deny); + assert_eq!( + http(&policy, "tcp://API.example.com:443"), + PermissionMode::Deny + ); +} + +// --- Listener inspection resources use a wildcard port ------------------------ + +#[test] +fn wildcard_port_inspection_resource_exposes_the_host_subject() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "127.0.0.1")], + ); + let listen = |resource: &str| { + evaluate_permissions_policy(&policy, "network", "network.listen", Some(resource)) + }; + assert_eq!(listen("tcp://127.0.0.1:*"), PermissionMode::Allow); + assert_eq!(listen("udp://127.0.0.1:*"), PermissionMode::Allow); + assert_eq!(listen("tcp://10.0.0.5:*"), PermissionMode::Deny); +} + +#[test] +fn host_port_pattern_does_not_match_a_wildcard_port_resource() { + let policy = network_policy( + PermissionMode::Deny, + vec![(PermissionMode::Allow, "127.0.0.1:8080")], + ); + assert_eq!( + evaluate_permissions_policy( + &policy, + "network", + "network.listen", + Some("tcp://127.0.0.1:*") + ), + PermissionMode::Deny + ); +} + +// --- Unix socket resources are not hosts --------------------------------------- +// +// Unix sockets are emitted as `unix:/path`, `unix:abstract:`, or +// `unix://path` depending on the producer. Host patterns never apply to them; +// they are matched by `unix:` patterns only. + +#[test] +fn star_host_pattern_does_not_match_unix_socket_resources() { + let policy = network_policy(PermissionMode::Deny, vec![(PermissionMode::Allow, "*")]); + let listen = |resource: &str| { + evaluate_permissions_policy(&policy, "network", "network.listen", Some(resource)) + }; + assert_eq!(listen("unix:/tmp/app.sock"), PermissionMode::Deny); + assert_eq!(listen("unix://tmp/app.sock"), PermissionMode::Deny); + assert_eq!(listen("unix:abstract:6170"), PermissionMode::Deny); + assert_eq!(listen("unix:autobind"), PermissionMode::Deny); +} + +#[test] +fn unix_patterns_match_unix_socket_resources_literally() { + let policy = network_policy( + PermissionMode::Deny, + vec![ + (PermissionMode::Allow, "unix:/tmp/app.sock"), + (PermissionMode::Allow, "unix://tmp/app.sock"), + (PermissionMode::Allow, "unix:abstract:*"), + ], + ); + let listen = |resource: &str| { + evaluate_permissions_policy(&policy, "network", "network.listen", Some(resource)) + }; + assert_eq!(listen("unix:/tmp/app.sock"), PermissionMode::Allow); + assert_eq!(listen("unix://tmp/app.sock"), PermissionMode::Allow); + assert_eq!(listen("unix:abstract:6170"), PermissionMode::Allow); + assert_eq!(listen("unix:/tmp/other.sock"), PermissionMode::Deny); + assert_eq!(listen("tcp://tmp:80"), PermissionMode::Deny); +} diff --git a/crates/native-sidecar/tests/service.rs b/crates/native-sidecar/tests/service.rs index 9e4900747a..a51f195b90 100644 --- a/crates/native-sidecar/tests/service.rs +++ b/crates/native-sidecar/tests/service.rs @@ -10476,10 +10476,10 @@ console.log(JSON.stringify({ status: "ok", summary })); let denied = bridge.require_resolved_network_access( "vm-resolved", NetworkOperation::Http, - "allowed.test:443", + "tcp://allowed.test:443", &[ - String::from("198.51.100.7:443"), - String::from("203.0.113.9:443"), + String::from("tcp://198.51.100.7:443"), + String::from("tcp://203.0.113.9:443"), ], ); assert!(denied @@ -10497,8 +10497,8 @@ console.log(JSON.stringify({ status: "ok", summary })); .require_resolved_network_access( "vm-resolved", NetworkOperation::Http, - "allowed.test:443", - &[String::from("198.51.100.7:443")], + "tcp://allowed.test:443", + &[String::from("tcp://198.51.100.7:443")], ) .expect("hostname allow remains sufficient without an applicable address rule"); @@ -10512,8 +10512,8 @@ console.log(JSON.stringify({ status: "ok", summary })); .require_resolved_network_access( "vm-resolved", NetworkOperation::Http, - "198.51.100.7:443", - &[String::from("198.51.100.7:443")], + "tcp://198.51.100.7:443", + &[String::from("tcp://198.51.100.7:443")], ) .expect("literal IP authority keeps ordinary IP-only semantics"); assert!( @@ -10521,8 +10521,8 @@ console.log(JSON.stringify({ status: "ok", summary })); .require_resolved_network_access( "vm-resolved", NetworkOperation::Http, - "allowed.test:443", - &[String::from("198.51.100.7:443")], + "tcp://allowed.test:443", + &[String::from("tcp://198.51.100.7:443")], ) .is_err(), "an IP allow must not implicitly authorize a hostname" diff --git a/docs/content/docs/permissions.mdx b/docs/content/docs/permissions.mdx index 9ee78a75fb..54575440c3 100644 --- a/docs/content/docs/permissions.mdx +++ b/docs/content/docs/permissions.mdx @@ -77,7 +77,7 @@ To invert it, flip `default` to `"deny"` and allow just one subtree: ## Allow only specific network hosts -Every non-`fs` scope matches by `patterns` instead of `paths`. For `network`, a pattern is a host (or `host:port`), and the operations are `fetch`, `http`, `dns`, and `listen`. +Every non-`fs` scope matches by `patterns` instead of `paths`. For `network`, a pattern is a host (or `host:port`), and the operations are `fetch`, `http`, `dns`, and `listen`. Host patterns are case-insensitive globs: `*.example.com` matches every subdomain, `api.example.com:443` matches one port, and `*` matches every host. A pattern may also name the full connection URI, `tcp://:` for `fetch`, `http`, and `listen`, or `dns://` for `dns`, when a rule should apply to one scheme only. Unix domain sockets are not hosts: a host pattern never matches them, and they are matched only by `unix:` patterns such as `unix:/tmp/app.sock`.