Repository navigation
feat(bep): accept only box connections and terminate TLS for the module hosts over HTTP/1.1 - #2475
Conversation
The listener checks the peer's uid, the per-boot token, the delivery header version and the box id before a connection reaches the proxy. A host shell that finds the socket is refused with no answer (BEP-026). Adds the control socket for client key registrations and the request shell that terminates a box's TLS (BEP-029, BEP-073, BEP-075).
Covers BEP-029 (off_module audit), BEP-073 (http/1.1 alone in ALPN on the box and upstream legs) and BEP-075 (CONNECT and absolute-form over HTTP/1.1 only, Extended CONNECT and connect-udp refused).
bep serve reads the per-boot token from standard input, then serves box connections and client key registrations. No Keychain key can sign a leaf yet (#2440), so an admitted CONNECT is answered 502 until one can.
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none seen, all four source files are new. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The listener, the ALPN policy and the off_module refusals conform to Gatehouse §6.10 and networking §5.7. The interim that keeps bep serve closed is not honest. relay() forwards requests with no egress check, no redemption and no admit audit record. Gatehouse §6.10 line 823 requires all three. Only NoSigningKey blocks that path, and the stated flip ('when #2440 lands') removes the block. Make the change in C1 to bind the flip to the redemption wiring and its issue. Make the change in C2 to name the owner of the missing store authorities.
Ruled at 43493c0 against gominimal/arch@109541a48c31 by claude-opus-5-5.
Retry transient accept errors on the listener and control socket.
Time out the upstream dial and the box TLS handshake, and end a relayed
flow that is idle or whose upstream closed. Bind sockets in a 0700
staging directory, and note that the proxy does not read the registry.<
8000
/pre>
|
Round 1 of 10 on the runtime's review findings, pushed as
Verify Review threads the coder judged:
The listener refuses a host-shell connection and logs a warning. A box's CONNECT to a GitHub host completes TLS with only http/1.1 in ALPN. The upstream dial and the box's TLS handshake now time out after 30 seconds. A relayed flow ends after 300 seconds idle, or when the upstream closes. Notes from the agent: Revise round 1. All six review threads were valid and are fixed in one commit. Per-box caps on concurrent flows are not added. The reviewer only suggested them, and they need a policy decision. The registry is still not given to Proxy, because the proxy redeems no store handles yet. A comment in main.rs now says this. just clippy-strict was not run. |
|
The runtime will not revise this pull request again on its own: the diff is +2364/-3 (2367 lines), past 3x the plan's estimate of 690 lines (est: 450 code, 240 tests, as #2271 reads now), at round 1 (the last trigger was the runtime's review findings). A person decides now. Remove |
|
/revise Carry the arch-review conditions at 43493c0 (still open on c5ce8e8) and one message fix. Documentation only, apart from C3's string. C1. The interim that keeps C2. At C3. In Leave the round-1 changes as they are. Reply on the three arch threads with what moved. Generated by Claude Code |
Bind the NoSigningKey interim to the redemption wiring in relay() (#2272), not to #2440 alone, on the issuer, the startup warn line and relay(). Name T25 (#2280) as the issue that reads [secret-store-rules] at start. An https:// absolute-form target to a credentialed host now says that the proxy carries https through CONNECT only.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A host-shell connection is refused at the listener, and a box's CONNECT to a GitHub host completes TLS offering HTTP/1.1 alone. This round, the docs and the startup warn line say that bep serve must not reach relay() until T11 (#2272) lands, and that #2440 alone does not lift the interim. An https:// absolute-form request to a GitHub host now gets a 403 that says the proxy carries https through CONNECT only. Notes from the agent: Revise round 2. C1: doc comments on NoSigningKey and relay(), and the startup warn line, now name T11 (#2272) and say that #2440 alone does not lift the interim. C2: the comment above Proxy::new in serve names T25 (#2280) and the off_module refusal for store authorities. C3: Proxy::absolute keeps the upstream_not_tls marker and the 403. The https message is now 'The Box Egress Proxy refused the https:// request to {host}. It carries https to {host} through CONNECT only.' The http message is unchanged. connect_and_absolute_form_accepted_over_http11_only now also checks the https case. The warn line is a log record, so it cites issue numbers. just clippy-strict was not run. |
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 5 unresolved threads — minimal-foundry[bot] crates/bep/src/listener.rs, minimal-arch[bot] crates/bep/src/main.rs:208, minimal-arch[bot] crates/bep/src/proxy.rs, minimal-arch[bot] crates/bep/src/main.rs, minimal-foundry[bot] crates/bep/src/listener.rs:373. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
There was a problem hiding this comment.
arch-review: CONDITIONS · 1 condition · the full ruling
The listener, admission and the HTTP/1.1-only legs conform to Gatehouse §6.10 and networking §5.7. The interims name their issues. One path does not write the audit record that §6.10 line 823 requires. serve_inner refuses Extended CONNECT and connect-udp before it checks admission. So a tunnel request to an authority outside the module sets gets 501 and no off_module record. Make the change in C1. Then the change conforms.
Ruled at bbf29ff against gominimal/arch@109541a48c31 by claude-opus-5-5.
There was a problem hiding this comment.
Babysitter review of T10 at bbf29ff (closes #2271). Not an approval yet.
Spec coverage, read against spec 24 on main. The listener refuses every non-box connection before a byte of answer (BEP-026: peer uid, per-boot token in constant time, header version, non-zero box id, attachment). CONNECT and absolute-form targets outside the module host sets and the store authorities get 403 and an off_module decision record under the box id (BEP-029). Both TLS legs offer http/1.1 alone, and an h2-only client is refused at the handshake (BEP-073). HTTP/1.0, Extended CONNECT, connect-udp in either form and origin-form requests are refused, and the h2 preface is closed (BEP-075, BEP-074). The upstream leg validates against the host roots and is dialed before the 200 and the box handshake (BEP-055 ordering). The delivery constants and byte layout in listener.rs match switch::bep_host on main byte for byte.
Rounds 1 and 2, verified line by line. Round 1 (c5ce8e8) adds transient accept retries, a 30 s handshake timeout on the dial and the box accept, a 300 s idle limit over both directions with a cancellation that ends the blocking read when the upstream closes, and a staging-directory bind; each of the five resolved threads is fixed as its reply says. Round 2 (bbf29ff) carries the first arch ruling's two conditions and the https absolute-form message, docs and one string, with a test.
Still open on this head.
- arch-review CONDITIONS, one condition:
serve_innerrefuses Extended CONNECT andconnect-udpbefore the admission check, so an off-module tunnel request gets 501 with nooff_modulerecord (Gatehouse §6.10 line 823). Agreed. - Review thread 4333766425:
bind_owner_onlyremoves a socket a runningbep servestill listens on, so a secondbep servesilently takes the path. Agreed; probe with a connect and refuseAddrInUsewhen it answers. - Thread 4333707091 is fixed in c5ce8e8 (
listener::accept) and only awaits resolution. - The runtime has capped its own rounds (
needs-human): the diff is 3.4× the plan's estimate. That is with the person.
Residuals to file if no later task takes them. A workspace test that pins bep::listener's delivery constants to switch::bep_host's (neither crate depends on the other). A per-box cap on concurrent flows (thread 4333707116 deferred it).
Generated by Claude Code
|
/revise Carry the arch-review condition at bbf29ff, one open review finding, and one stale thread. C1 (arch). In C2 (thread 4333766425). C3. Thread 4333707091 is fixed in c5ce8e8 ( Generated by Claude Code |
…y-box-connections-and-terminate-tls-fo
An Extended CONNECT or connect-udp request to an authority outside the admitted set now leaves an off_module record, as a plain CONNECT does.
bind_owner_only probes an existing socket with a connect, removes it only when the connect is refused, and returns AddrInUse otherwise.
|
Round 3 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A host-shell connection is refused at the listener, and a box's CONNECT to a GitHub host completes TLS offering HTTP/1.1 alone. An Extended CONNECT or connect-udp request to an authority outside the modules now gets 501 and an off_module record that min box audit shows. A second bep serve at the socket path of a running one fails with AddrInUse and does not take the path. Notes from the agent: C1: serve_inner now gets the authority (the CONNECT target, or the absolute-form authority) before the tunnel refusal. When Admission::admits rejects it, serve_inner writes off_module through audit_refusal, and then answers 501 as before. A tunnel request to an admitted authority still leaves no record. C2: bind_owner_only connects to an existing socket first. It removes the socket only on ConnectionRefused or NotFound, and returns AddrInUse for any other result. The new test second_bind_at_a_live_socket_is_addr_in_use is in listener.rs. C3 asks for a thread resolution. These instructions do not let me resolve threads, so I gave thread 4333707091 no new verdict. Its earlier valid verdict still stands. The probe uses a blocking std connect. A live socket with a full backlog can make the probe wait until the backlog drains. |
|
The runtime will not revise this pull request again on its own: the diff is +2409/-3 (2412 lines), past 3x the plan's estimate of 690 lines (est: 450 code, 240 tests, as #2271 reads now), at round 3 (the last trigger was a /revise note). A person decides now. Remove |
There was a problem hiding this comment.
Babysitter review of T10 at e11fedc (round 3 over bbf29ff). Not an approval yet.
Round 3 carries the second arch ruling's condition and the live-socket finding, both verified line by line: serve_inner now derives the tunnel request's authority before the 501 and writes the off_module record when it is outside the admitted set, with the test asserting one record for each off-module case and none for the admitted ones; bind_owner_only probes an existing socket with a connect and returns AddrInUse unless the connect is refused, with a test that binds twice. Both commits lint. The merge of main carries T37 and T50 and touches no bep file.
Still open on this head: the arch re-rule, the runtime's review, and the stale thread 4333707091, which round 1 fixed and nobody has resolved. The runtime re-applied needs-human on size alone after the round; it was removed under the standing rule the person set.
Generated by Claude Code
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The two new commits close the open points. A tunnel request outside the admitted set now gets an off_module audit record, as Gatehouse §6.10 line 823 and the BEP-075 note require. The listener bind now refuses to remove a live socket, and it fails closed on an unknown probe error. The interims name their issues (#2272, #2440, #2280) and stay on the safe side. All four requirements conform.
Ruled at e11fedc against gominimal/arch@109541a48c31 by claude-opus-5-5.
|
Round 1 of 10 on the runtime's review findings: the coder changed nothing. Revise round 1: the one open thread is invalid, so I changed no code and ran no tests. The spec (BEP-075 rationale and the BEP-055 unwanted clause) binds the audit marker to the host, not the port. An absolute-form request to a credentialed host is refused upstream_not_tls. The check order the finding asks for also changes the main case. http://github.com/ uses port 80, so admits(host, 80) is false for every GitHub host, and that request would be audited off_module with the wrong message. Review threads the coder judged:
|
There was a problem hiding this comment.
Babysitter approval for T10 (closes #2271) at e11fedc.
All twelve gates pass on this head: own review, arch-review PASS (5602075069), the runtime's review concluded with every thread resolved, zero open threads, every lane aggregator green, CodeRabbit and license/cla green, not a draft, no needs-human, no implement in flight. The squash subject is a Conventional Commits feat(bep) subject under the length limit. The size cap was lifted under the person's standing rule.
Enqueuing a squash merge.
Generated by Claude Code
T10 — Accept only box connections and terminate TLS for the module's hosts over HTTP/1.1
slice: S0
kind: task
covers: BEP-026, BEP-029, BEP-073, BEP-075
size: L
est: 450 code, 240 tests
executor: agent
difficulty: unknown ground
model: strongest
diagnostics: a connection refused at the listener is logged with its reason and peer, and a request outside the modules is audited
off_module, whichmin box audit <box>showsobservability: one span per connection with the box id, the authority and the outcome; one warn log line per listener refusal with its reason
layers:
- listen:
- crates/bep/src/listener.rs (new): the acceptor for the switch peer's deliveries: per-boot token, header version and box id; BEP-026, BEP-029 and BEP-075
- crates/bep/src/proxy.rs (new): terminates the box's TLS with a leaf for the admitted name, offering
http/1.1alone in ALPN- crates/bep/src/control.rs (new): the control socket, separate from the listener, taking client key registrations into the handle registry
- crates/bep/src/main.rs:
bep serve- crates/bep/src/lib.rs: module lines
- crates/bep/Cargo.toml: the TLS and async dependencies
- Cargo.lock: the TLS dependencies
test: cargo nextest run -p bep host_shell_connection_is_refused
test: cargo nextest run -p bep request_outside_modules_is_refused_off_module
test: cargo nextest run -p bep proxy_offers_http11_only_on_both_legs
test: cargo nextest run -p bep connect_and_absolute_form_accepted_over_http11_only
produces:
- crates/bep/src/proxy.rs
- crates/bep/src/control.rs
consumes:
- crates/bep/src/ca.rs
- crates/bep/src/http.rs
- crates/bep/src/audit.rs
- crates/bep/src/handle.rs
proof: A host-shell connection is refused at the listener, and a box's CONNECT to a GitHub host completes TLS offering HTTP/1.1 alone.
depends on:
- blocked by #2265
- blocked by #2270
- blocked by #2261
- blocked by #2262
Proof
A process on the host that connects to the proxy's listener socket and speaks HTTP (with a sealed value or not) gets no answer. The same holds for a wrong token, an unknown header version or a zero box id, and one warn line records the reason, peer uid and pid. Only a delivery with this boot's token, header version 1 and an attached box id reaches the proxy. A box's
CONNECT api.github.c 8000 om:443through the listener gets200, and its TLS completes under the interception root withhttp/1.1negotiated, though the box offersh2, http/1.1. The upstream leg is offeredhttp/1.1alone. A client that requiresh2is refused at the handshake. A CONNECT or absolute-form request outside the module host sets and store authorities gets403, and the audit log records itoff_moduleunder the box's id, wheremin box audit <box>reads it.Notes from the agent: 1)
bep servecannot terminate TLS in production yet. No Keychain key implementsca::CaKey(issue #2440), and keys.rs is outside this task's layers.servetherefore uses a leaf issuer that always refuses: the listener, the control socket, the off_module and upstream_not_tls refusals and their audit all run, but an admitted CONNECT gets502('did not get a certificate for '). A startup warn line says so. When #2440 lands, replaceNoSigningKeyin main.rs withMutex<Ca<'static, KeychainKey>>. The proof holds in the tests, which use a software CA key. 2)bep serveusesPeerAttestedfor attachments: the token proves the dial came from the switch peer, and the peer fills the box id from the attachment before it dials. TheAttachmentstrait takes a real table when the proxy gets one. 3)servepasses no store authorities toAdmission, because nothing reads the[secret-store-rules]at start yet. 4) The listener duplicates the switch's delivery-header constants (TOKEN_LEN,DELIVERY_HEADER_VERSION,DELIVERY_HEADER_LEN) and does not depend on theswitchcrate. 5) Not in this …[truncated]Tests
cargo nextest run -p bep host_shell_connection_is_refusedcargo nextest run -p bep request_outside_modules_is_refused_off_modulecargo nextest run -p bep proxy_offers_http11_only_on_both_legscargo nextest run -p bep connect_and_absolute_form_accepted_over_http11_onlyVerify
min task run agent-verifypassed ([tasks.verify]).Closes #2271