deno
e4444d5a - refactor(ext/fetch): check net deny list against connected peer addr (#34418)

Commit
106 days ago
refactor(ext/fetch): check net deny list against connected peer addr (#34418) Follow-up to #34236, addressing https://github.com/denoland/deno/pull/34236#discussion_r3309154779. #34236 introduced a `REQUEST_PORT` `tokio::task_local!` to smuggle the real destination port from `PermissionedHttpConnector` (which sees the `Uri`) into the DNS resolver (which is only handed a `Name` by hyper-util's `HttpConnector`). The DNS resolver then ran the post-resolution net-deny check against every resolved address with that port. This was load-bearing but pretty ugly: it relied on the task-local being read before each `tokio::spawn` inside the resolver and scoped around the inner connector's future. Instead, run both the resolution and the check in `PermissionedHttpConnector` itself, which sees the full `Uri` (and therefore the real port). The connector now owns the `Resolver` and the connector config (local address) rather than a pre-built `HttpConnector`: - For hostnames, it resolves via the `Resolver`, checks every resolved address with `check_net_resolved` before any socket is opened (matching what `Deno.connect` does in `ext/net/ops.rs`), and then hands the vetted addresses to a hyper-util `HttpConnector` through a pre-resolved resolver. The connection goes to exactly the addresses that were checked, so no second DNS query can race with a record change. - For IP-literal hosts, the literal is checked directly and resolution is skipped (hyper-util connects to literals without consulting the resolver). - Resolution errors are wrapped in a `DnsError` type that mirrors hyper-util's `ConnectError` display ("dns error" with the `io::Error` as source), so fetch error messages keep their existing `client error (Connect): dns error: ...` format. This removes the `REQUEST_PORT` task-local, the `permissions` field on `Resolver`, `Resolver::with_permissions`, and the in-resolver `check_resolved_permissions` helper. Callers pass `permissions: Option<PermissionsContainer>` via `CreateHttpClientOptions`, and `ext/fetch` and `ext/websocket` set it there. No behavior change for the existing `tests/specs/permission/deny_net_*` regression tests. ## Follow-up: proxied request destinations A request routed through a proxy only opens a socket to the proxy, so the post-resolution deny check on the connector previously only saw the proxy address. An IP-level `--deny-net` rule on the destination could therefore be bypassed by routing through an allowed proxy. `ProxyConnector` now runs a best-effort net-deny check against the real destination before connecting to the proxy: it resolves the destination host (via the new `CheckDst` trait on `PermissionedHttpConnector`) and checks every resolved address. It is best-effort because a proxy may be able to reach a host this process cannot resolve locally, so a local resolution failure is not fatal; the connection to the proxy itself is still checked separately. Covered by a new `ext/fetch` test that denies a proxied destination whose hostname resolves to a denied IP.
Author
Parents
Loading