[sstriker]

The egress checked a CONNECT tunnel only by its authority, the host and the port. After the 200, it copied bytes both ways. Inside the tunnel, the harness chose the TLS server name (SNI). So a harness could send CONNECT lh3.googleusercontent.com:443 and then a ClientHello with the name of another tenant on the same front end. An adversarial pass on #20 showed this through the real egress: GCS buckets and App Engine apps through lh3.googleusercontent.com, other Cloudflare customers through chatgpt.com, and raw.githubusercontent.com through api.githubcopilot.com.

The change

A new module, crates/egress/src/hello.rs, parses the ClientHello by hand, with no new dependency. Every length is checked against the bytes that remain, and there is no indexing that can panic. connect_or_not_found in crates/egress/src/lib.rs now does these steps, in this order:

1. It sends the 200. A client sends its ClientHello only after the 200.
2. It reads the first ClientHello and checks it. Nothing is dialled before this check.
3. It dials, and sends only the records of that hello.
4. It reads the upstream's answer, and sends it on to the client as it comes, until the whole ServerHello is in. Client bytes after the hello are held until then.
5. If the answer is a HelloRetryRequest, the client's next bytes must be a second ClientHello that passes the same check. One ChangeCipherSpec and TLS 1.3 early data may come before it. The egress then reads the answer to the second hello, and refuses a second HelloRetryRequest.
6. Only then does it copy both ways.

A ClientHello passes only if its server name is the CONNECT host. Case and a trailing dot do not matter. The egress refuses these:

• bytes that are not a TLS handshake;
• a hello with no server name, except when the CONNECT names an IP address (RFC 6066 clients send no name for an address);
• a server name extension that occurs two times, or a name list with more than one entry;
• Encrypted Client Hello (0xfe0d and the drafts 0xfe00 to 0xfe0c) and the draft encrypted SNI (0xffce), because then the name in the clear is a decoy;
• a hello whose record carries more bytes after it.

Limits:

• A hello is at most 16 KiB plus the record header.
• One deadline of 10 s covers all reads, from the 200 to the answer to the second hello. A client that sends one byte at a time cannot keep the tunnel open.

On a refusal, the egress writes egress: tunnel to <host>: <reason>; closed to its log. It sends a TLS access_denied alert to the client and closes the tunnel.

What this does not close

The module documentation says this too. The egress cannot see inside TLS, so these stay open:

• A front end that routes on the HTTP Host header inside TLS, and not on the server name.
• Paths on a shared host: any bucket on storage.googleapis.com, any repository on raw.githubusercontent.com.
• A TLS 1.2 renegotiation, because its hello is encrypted.

Effect on harnesses

An adversarial pass captured the first ClientHello of each TLS stack on this host through a fake CONNECT proxy. Each one passes: the name is the CONNECT host, and the hello has no ECH extension. The stacks were:

• curl 8.5.0 with OpenSSL 3.0.13;
• Python with OpenSSL 3.0.13;
• Go 1.22.2;
• Node 24.21.0 with OpenSSL 3.5.8;
• codex 0.160.0.

agy uses Go's crypto/tls, which sends ECH only when it has an ECH config. A Chromium-based client sends GREASE ECH by default, so the egress refuses it. No harness here uses that stack. local/README.md says this.

Tests

hello.rs has unit tests for these cases:

• a real rustls hello, another name, upper case, a trailing dot, no name, and an IP address;
• ECH and its drafts, two names, an SSLv2 hello, and bytes that are not TLS;
• every cut of a hello, a hello split over records of 1 to 40 bytes, and a hello split across reads;
• too long, a timeout, an early close, and lengths that disagree;
• a HelloRetryRequest and every cut of it, a normal ServerHello, a TLS 1.2 ServerHello, and the server's ChangeCipherSpec whole, alone and split;
• early data, a full-record second hello, and a full-record ServerHello after a ChangeCipherSpec;
• bytes that are ready after the deadline;
• a fuzz pass: random bytes, and each byte of a valid hello or answer changed. None panics, and a changed name never passes.

lib.rs has tests through the Network stand-in from #13:

• a matching name sends the hello and the bytes after it on unchanged;
• another name, no name, and bytes that are not TLS dial nothing and send nothing upstream, and the client gets the exact alert;
• a tunnel that gets no hello is closed when its wait ends;
• after a HelloRetryRequest, a matching second hello goes upstream, and a second hello with another name goes nowhere;
• a second hello sent in the same write as the first is held, and goes nowhere;
• a second HelloRetryRequest is refused, and a third hello goes nowhere;
• one deadline covers all the hellos and answers.

Failing first

The new integration test, against main with only the tests added:

assertion `left == right` failed: refused, and let through
  left: ["another name: dialled [[198.41.0.4:443]], closed false, echoed true", "no name: dialled [[198.41.0.4:443]], closed false, echoed true", "not TLS: dialled [[198.41.0.4:443]], closed false, echoed true"]
 right: []

Each later commit shows its new assertions failing first, against the commit before it or under a mutation. Examples:

• The second hello to another host, on fa8ae79: the upstream hears nothing after it asked. The second hello reached the upstream.
• The server's ChangeCipherSpec in its own read, on 80fca25: the third hello, which names other-tenant.example, reached the upstream.
• Bytes that are ready after the deadline, on 0d47686: taken after the deadline, with left: ["a hello: Ok(77)", "a hello: Ok(77)", "an answer: Ok(Other)", "an answer: Ok(Other)"].

make check: 680 tests, 680 passed, 6 skipped.

Adversarial passes

Four fresh agents attacked this branch. Each round of findings is fixed in the commit after it.

1. On c0591fe. It found the gap after a HelloRetryRequest: only the first hello was checked. It found four parts of the wiring that no test held: the alert, the close, the wait at the call site, and a record with more bytes. It found the ECH drafts. Fixed in 3184fd1.
2. On 3184fd1. It found no way to give the upstream a second name. Real rustls TLS 1.3 handshakes, with and without a real HelloRetryRequest, and TLS 1.2 handshakes passed through. It found that no test held the held bytes or the single deadline. It also found three gaps: a second HelloRetryRequest, early data before the second hello, and the size bound. Fixed in 80fca25.
3. On 80fca25. It found that the server's ChangeCipherSpec in its own read let a second HelloRetryRequest through. It also found one more untested hold and a test that could panic on a slow runner. Fixed in b7296d9. A second ChangeCipherSpec before the ServerHello is now refused (0d47686).
4. On 0d47686. It found no correctness or security bug. A rustls handshake with a forced HelloRetryRequest passed byte for byte under five ways to split the server's writes. It found that no test held the size bound for an answer, and that the deadline was not strict after a stall. Fixed in 6ac0e3d.

One finding is declined. A refusal by name writes no telemetry row. The CONNECT refusals before this change also write none.

Posted by Claude on behalf of @sstriker

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.