Skip to content

Commit 194fb10

Browse files
committed
feat(cli): allow-CIDR over DNS-resolved IPs
Close the symmetry gap with deny-CIDR. mode=Allowlist with only allow-CIDR rules (no matching host rule) now works for named-host URIs: the HTTP layer defers to the DNS resolver, which drops resolved IPs not covered by any allow-CIDR. Host-anchored allow rules still short-circuit — if the hostname itself matches, all resolved IPs are kept regardless of CIDR rules. decide_uri(): for named hosts with no direct match but at least one allow-CIDR rule, return Allow and let the resolver filter. IP-literal URIs still get fully checked at the HTTP layer (DNS never runs). PolicyDnsResolver: carries allow rules + mode; per-IP keep rule is "no deny-CIDR match AND (host-match OR allow-CIDR match)" in Allowlist mode with allow-CIDR rules; plain deny-CIDR filter in all other modes. Adds two unit tests in http_client.rs (DNS resolver enforcement + host-match bypass) and two in http_policy.rs (HTTP-layer deferral). Updates the module doc to reflect the closed gaps.
1 parent 3877f0f commit 194fb10

2 files changed

Lines changed: 217 additions & 27 deletions

File tree

act-cli/src/runtime/http_client.rs

Lines changed: 150 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,50 +19,95 @@ use wasmtime_wasi_http::p2::bindings::http::types::ErrorCode as P2ErrorCode;
1919
use wasmtime_wasi_http::p2::body::HyperIncomingBody;
2020
use wasmtime_wasi_http::p3::bindings::http::types::ErrorCode as P3ErrorCode;
2121

22-
use crate::config::HttpConfig;
22+
use crate::config::{HttpConfig, PolicyMode};
2323
use crate::runtime::network::{self, NetworkRule};
2424

25-
/// reqwest DNS resolver that filters resolved addresses against deny-CIDR
26-
/// rules. Drops every resolved `SocketAddr` whose IP matches any deny CIDR
27-
/// (respecting `except_ports`). If no addresses survive, returns an empty
28-
/// iterator — reqwest surfaces this as a DNS error, which our
29-
/// `reqwest_to_p2_error` / `reqwest_to_p3_error` maps to the appropriate
30-
/// wasi:http error variant.
25+
/// reqwest DNS resolver that filters resolved addresses against both deny
26+
/// and allow CIDR rules.
27+
///
28+
/// Logic per resolved `SocketAddr`:
29+
/// 1. Drop if any deny-CIDR matches (respecting `except_ports`).
30+
/// 2. In `Allowlist` mode, if any allow rule carries a `cidr`, the IP must
31+
/// be covered by either a host-anchored allow (meaning the hostname
32+
/// itself was allowed, so every resolved IP is OK) or an allow-CIDR.
33+
/// This closes the prior asymmetry where `allow = [{ cidr = "..." }]`
34+
/// required an IP-literal URI.
35+
/// 3. `Open` / `Deny` modes: no allow-side filter here (`Deny` never
36+
/// reaches the resolver; `Open` still honors deny-CIDR as a safety
37+
/// net).
38+
///
39+
/// If no addresses survive, returns an empty iterator — reqwest surfaces
40+
/// this as a DNS error, which our `reqwest_to_p2_error` /
41+
/// `reqwest_to_p3_error` maps to `ErrorCode::DnsError`.
3142
struct PolicyDnsResolver {
43+
allow_nets: Arc<Vec<NetworkRule>>,
3244
deny_nets: Arc<Vec<NetworkRule>>,
45+
mode: PolicyMode,
3346
}
3447

3548
impl PolicyDnsResolver {
3649
fn new(cfg: &HttpConfig) -> Self {
50+
let allow_nets = cfg.allow.iter().map(|r| r.net.clone()).collect();
3751
let deny_nets = cfg.deny.iter().map(|r| r.net.clone()).collect();
3852
Self {
53+
allow_nets: Arc::new(allow_nets),
3954
deny_nets: Arc::new(deny_nets),
55+
mode: cfg.mode,
4056
}
4157
}
4258
}
4359

4460
impl Resolve for PolicyDnsResolver {
4561
fn resolve(&self, name: Name) -> Resolving {
62+
let allow = self.allow_nets.clone();
4663
let deny = self.deny_nets.clone();
64+
let mode = self.mode;
4765
Box::pin(async move {
4866
let host = name.as_str().to_string();
4967
let addrs = tokio::net::lookup_host(format!("{host}:0"))
5068
.await
5169
.map_err(|e| Box::new(e) as Box<dyn std::error::Error + Send + Sync>)?;
5270
let all: Vec<SocketAddr> = addrs.collect();
5371
let total = all.len();
72+
73+
// If the hostname itself matches any host-anchored allow rule,
74+
// we don't need to require per-IP CIDR matches — the guest is
75+
// already allowed to talk to this host. Compute once.
76+
let host_allowed = allow.iter().any(|r| {
77+
r.host
78+
.as_deref()
79+
.is_some_and(|pat| network::host_matches(pat, &host))
80+
});
81+
let require_allow_cidr = mode == PolicyMode::Allowlist
82+
&& !host_allowed
83+
&& allow.iter().any(|r| r.cidr.is_some());
84+
5485
let filtered: Vec<SocketAddr> = all
5586
.into_iter()
56-
.filter(|addr| !network::any_deny_cidr_matches(&deny, addr.ip(), addr.port()))
87+
.filter(|addr| {
88+
if network::any_deny_cidr_matches(&deny, addr.ip(), addr.port()) {
89+
return false;
90+
}
91+
if require_allow_cidr {
92+
return allow.iter().any(|r| {
93+
r.cidr
94+
.as_deref()
95+
.is_some_and(|c| network::cidr_contains(c, addr.ip()))
96+
});
97+
}
98+
true
99+
})
57100
.collect();
58101
tracing::debug!(
59102
%host,
60103
resolved = total,
61104
kept = filtered.len(),
105+
require_allow_cidr,
106+
host_allowed,
62107
"http policy dns resolve",
63108
);
64109
if filtered.is_empty() {
65-
return Err("all resolved addresses matched a deny CIDR".into());
110+
return Err("all resolved addresses filtered by policy CIDR rules".into());
66111
}
67112
let iter: Addrs = Box::new(filtered.into_iter());
68113
Ok(iter)
@@ -712,4 +757,100 @@ mod tests {
712757
"expected DnsError or ConnectionRefused, got {err:?}"
713758
);
714759
}
760+
761+
#[tokio::test(flavor = "current_thread")]
762+
async fn dns_resolver_requires_allow_cidr_match_for_hostnames() {
763+
// mode=Allowlist with only an allow-CIDR rule. Any URI whose
764+
// resolved IPs land outside that CIDR must fail at DNS level.
765+
use crate::config::{HttpConfig, HttpRule, PolicyMode};
766+
use crate::runtime::network::NetworkRule;
767+
768+
let cfg = HttpConfig {
769+
mode: PolicyMode::Allowlist,
770+
// Only permit internal RFC1918 space — example.com is public.
771+
allow: vec![HttpRule {
772+
net: NetworkRule {
773+
cidr: Some("10.0.0.0/8".into()),
774+
..Default::default()
775+
},
776+
..Default::default()
777+
}],
778+
deny: vec![],
779+
..Default::default()
780+
};
781+
let client = ActHttpClient::new(cfg).expect("client builds");
782+
let body: UnsyncBoxBody<bytes::Bytes, P2ErrorCode> = Empty::<bytes::Bytes>::new()
783+
.map_err(|_| unreachable!())
784+
.boxed_unsync();
785+
let hyper_req = hyper::Request::builder()
786+
.method(Method::GET)
787+
.uri("https://example.com/")
788+
.body(body)
789+
.unwrap();
790+
let config = wasmtime_wasi_http::p2::types::OutgoingRequestConfig {
791+
use_tls: true,
792+
connect_timeout: std::time::Duration::from_secs(5),
793+
first_byte_timeout: std::time::Duration::from_secs(5),
794+
between_bytes_timeout: std::time::Duration::from_secs(5),
795+
};
796+
let err = client
797+
.send_p2(hyper_req, config)
798+
.await
799+
.expect_err("example.com IPs not in 10/8, must fail at DNS");
800+
assert!(
801+
matches!(err, P2ErrorCode::DnsError(_)),
802+
"expected DnsError, got {err:?}"
803+
);
804+
}
805+
806+
#[tokio::test(flavor = "current_thread")]
807+
async fn dns_resolver_host_match_bypasses_allow_cidr() {
808+
// mode=Allowlist with BOTH a host-allow AND an allow-CIDR. A
809+
// request to the allowed host should succeed even if its IPs
810+
// don't fall in the CIDR — the host match approves all IPs.
811+
use crate::config::{HttpConfig, HttpRule, PolicyMode};
812+
use crate::runtime::network::NetworkRule;
813+
814+
let cfg = HttpConfig {
815+
mode: PolicyMode::Allowlist,
816+
allow: vec![
817+
HttpRule {
818+
net: NetworkRule {
819+
host: Some("example.com".into()),
820+
..Default::default()
821+
},
822+
..Default::default()
823+
},
824+
HttpRule {
825+
net: NetworkRule {
826+
cidr: Some("10.0.0.0/8".into()),
827+
..Default::default()
828+
},
829+
..Default::default()
830+
},
831+
],
832+
deny: vec![],
833+
..Default::default()
834+
};
835+
let client = ActHttpClient::new(cfg).expect("client builds");
836+
let body: UnsyncBoxBody<bytes::Bytes, P2ErrorCode> = Empty::<bytes::Bytes>::new()
837+
.map_err(|_| unreachable!())
838+
.boxed_unsync();
839+
let hyper_req = hyper::Request::builder()
840+
.method(Method::GET)
841+
.uri("https://example.com/")
842+
.body(body)
843+
.unwrap();
844+
let config = wasmtime_wasi_http::p2::types::OutgoingRequestConfig {
845+
use_tls: true,
846+
connect_timeout: std::time::Duration::from_secs(10),
847+
first_byte_timeout: std::time::Duration::from_secs(10),
848+
between_bytes_timeout: std::time::Duration::from_secs(10),
849+
};
850+
let incoming = client
851+
.send_p2(hyper_req, config)
852+
.await
853+
.expect("example.com allowed via host rule");
854+
assert_eq!(incoming.resp.status().as_u16(), 200);
855+
}
715856
}

act-cli/src/runtime/http_policy.rs

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,16 @@
1010
//! - Host matching: literal host, exact match or `*.suffix` wildcard.
1111
//! - Scheme / methods / ports matching.
1212
//! - IP literals in URI: matched against `cidr` entries at HTTP-layer.
13-
//! - **DNS-resolved IPs against deny CIDRs**: checked in a pre-flight
14-
//! `lookup_host` before the default handler runs. Catches
15-
//! SSRF-by-name ("fetch internal.example.com" where it resolves to a
16-
//! 10.x host). This is a second DNS lookup per request (the default
17-
//! handler does its own `TcpStream::connect(authority)` which resolves
18-
//! again); the OS resolver cache usually makes the second lookup a
19-
//! no-op. There's a narrow TOCTOU window where a racing resolver
20-
//! could return a different IP between our check and the handler's
21-
//! connect — full DNS-rebinding defence requires forking the default
22-
//! handler to accept a pre-resolved `SocketAddr`, which is deferred.
23-
//! - Allow-CIDR rules against DNS-resolved IPs: NOT IMPLEMENTED. Allow
24-
//! CIDRs still require an IP literal in the URI. Deny CIDRs are the
25-
//! security-critical direction.
26-
//! - Redirect re-decision: deferred (wasi-http follows redirects inside
27-
//! `default_send_request`; we'd need to disable that and resubmit).
13+
//! - **DNS-resolved IPs against both allow and deny CIDRs**: enforced in
14+
//! the reqwest `PolicyDnsResolver` hook (`runtime::http_client`). The
15+
//! resolver runs once per request, filters denied IPs, and in
16+
//! `Allowlist` mode additionally requires allow-CIDR coverage when the
17+
//! hostname doesn't match any host-anchored allow rule. Named-host URIs
18+
//! with only allow-CIDR rules defer their verdict from the HTTP layer
19+
//! to the resolver. The single resolve pins the addresses for the
20+
//! subsequent connect, closing the DNS-rebinding window.
21+
//! - Redirect re-decision: each hop re-evaluated via `reqwest::redirect`
22+
//! hook (see `http_client::build_redirect_policy`).
2823
2924
use std::sync::Arc;
3025

@@ -57,6 +52,12 @@ impl PolicyHttpHooks {
5752
/// Decide an HTTP request against the config. Scheme / method checks are
5853
/// HTTP-layer; the host / port / CIDR parts are delegated to
5954
/// `runtime::network::decide`.
55+
///
56+
/// For name-host URIs (non-IP-literal), when no allow rule matches but
57+
/// there's at least one allow rule with a `cidr` field, return `Allow`
58+
/// and defer the per-IP check to `PolicyDnsResolver`. This is how
59+
/// `allow = [{ cidr = "..." }]` rules take effect against hostnames —
60+
/// the HTTP layer doesn't know the resolved IPs yet.
6061
fn decide_uri(&self, method: Option<&str>, uri: &Uri) -> Decision {
6162
match self.config.mode {
6263
PolicyMode::Deny => return Decision::Deny,
@@ -65,6 +66,7 @@ impl PolicyHttpHooks {
6566
}
6667

6768
let host = uri.host().unwrap_or("");
69+
let host_is_ip_literal = host.parse::<std::net::IpAddr>().is_ok();
6870
let scheme = uri.scheme_str();
6971
let port = uri
7072
.port_u16()
@@ -85,10 +87,14 @@ impl PolicyHttpHooks {
8587
.iter()
8688
.any(|r| http_rule_matches(r, scheme, method, &check))
8789
{
88-
Decision::Allow
89-
} else {
90-
Decision::Deny
90+
return Decision::Allow;
9191
}
92+
if !host_is_ip_literal && self.config.allow.iter().any(|r| r.net.cidr.is_some()) {
93+
// Defer to DNS resolver: it will drop resolved IPs that don't
94+
// match any allow-CIDR (see `PolicyDnsResolver`).
95+
return Decision::Allow;
96+
}
97+
Decision::Deny
9298
}
9399
}
94100

@@ -384,4 +390,47 @@ mod tests {
384390
Decision::Deny
385391
);
386392
}
393+
394+
#[test]
395+
fn allow_cidr_defers_named_host_to_dns() {
396+
// mode=Allowlist with only an allow-CIDR rule. Hostname URIs must
397+
// reach the DNS resolver (return Allow at HTTP layer); the
398+
// resolver will drop IPs not in the CIDR. IP-literal URIs still
399+
// get fully checked at HTTP layer.
400+
let h = hooks(HttpConfig {
401+
mode: PolicyMode::Allowlist,
402+
allow: vec![rule(None, None, None, Some("10.0.0.0/8"), None)],
403+
..Default::default()
404+
});
405+
406+
// Name host: defer to DNS → Allow at HTTP layer.
407+
assert_eq!(
408+
h.decide_uri(Some("GET"), &uri("https://internal.corp/")),
409+
Decision::Allow
410+
);
411+
// IP literal in allowed CIDR: Allow.
412+
assert_eq!(
413+
h.decide_uri(Some("GET"), &uri("http://10.1.2.3/")),
414+
Decision::Allow
415+
);
416+
// IP literal outside CIDR: Deny (no DNS deferral for literals).
417+
assert_eq!(
418+
h.decide_uri(Some("GET"), &uri("http://8.8.8.8/")),
419+
Decision::Deny
420+
);
421+
}
422+
423+
#[test]
424+
fn allow_cidr_does_not_defer_without_cidr_rule() {
425+
// No allow-CIDR rules → HTTP layer decides purely on host.
426+
let h = hooks(HttpConfig {
427+
mode: PolicyMode::Allowlist,
428+
allow: vec![rule(Some("example.com"), None, None, None, None)],
429+
..Default::default()
430+
});
431+
assert_eq!(
432+
h.decide_uri(Some("GET"), &uri("https://other.com/")),
433+
Decision::Deny
434+
);
435+
}
387436
}

0 commit comments

Comments
 (0)