From 1dd31d8be809b8dd7244305534ab79731765e1ce Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 00:46:53 +0100 Subject: [PATCH 01/18] docs: plan the full-handshake carrier, and amend design.md's 4% conclusion twiddle emits a resumption hello on every connection: VerifyTicketAuth and CoverProfile.validateClientHello both hard-require pre_shared_key. Measured resumption share in real browsing is 4.1% (636 connections) and 2.7% (485), so that is a ~25x free reduction in a censor's candidate set. design.md already argued this does not matter, on the grounds that 4.1% is an INNER number -- the browser reaching 254 destination origins -- while the outer connection to our egress is a layer we own where ~100% resumption is attainable. The layer distinction is right and the conclusion is too strong: a censor classifies TCP flows, not layers, so our outer connection sits in the same observed population as every inner one. Working that through lands somewhere narrower than "match 4%", and much cheaper. The anomaly is not resumption -- a client with a long relationship to one host and a stack of its tickets is an ordinary pattern. The anomaly is exclusivity: reaching a host for the first time already holding a ticket, and never once completing a full handshake with it. So the requirement is that the resumption have an observable predecessor, which makes first contact full and needs no ratio to tune. Records what #1 already built (ServerHelloFullLen, FullRemainder and its jitter, Adopt's structural validation for the full variant, coverprobe's ProbeBoth/SampleFull), the measured per-cover full profile, and the part that is genuinely unsolved: anti-replay without a ticket. The merged gate keys on clientID and Issued, both read out of the ticket's authenticated plaintext, so a PSK-less hello needs its own construction rather than a reuse. Co-Authored-By: Claude Opus 5 --- docs/design.md | 6 + docs/full-handshake-carrier.md | 199 +++++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 docs/full-handshake-carrier.md diff --git a/docs/design.md b/docs/design.md index 405f567..49c2da3 100644 --- a/docs/design.md +++ b/docs/design.md @@ -223,6 +223,12 @@ traffic — prior art worth reading before implementing the ticket path here. It theatrical opening as a resumption hello: that decision does not depend on the 4.1%, and the operational half of it is already proven in production. +> **Amended.** The layer distinction above holds, but the conclusion drawn from it was too strong: the +> censor does not see layers, so our outer connection sits in the same observed population as every inner +> one. `docs/full-handshake-carrier.md` works the argument through and lands somewhere narrower than "match +> 4%" — the anomaly is *exclusivity*, a client that reaches a host already holding a ticket and never once +> completes a full handshake with it. Read that document alongside this section. + **But do not carry the TLS version over.** http-proxy uses TLS 1.2, and for this transport 1.3 is strictly better: Xue's classifier is *more* precise against 1.2 (`Wb=5`, more consecutive elements must match, lower FPR) and the paper says explicitly that it is "in censors' interest to focus on TLS 1.2." TLS 1.2 also puts diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md new file mode 100644 index 0000000..7da8b6c --- /dev/null +++ b/docs/full-handshake-carrier.md @@ -0,0 +1,199 @@ +# The full-handshake carrier + +**Status:** designed, not built. Everything below the "What already exists" line is groundwork that +landed in #1; the carrier itself is unstarted. + +## The problem, measured + +twiddle emits a **resumption** hello on every connection. `VerifyTicketAuth` requires +`pre_shared_key` (`auth.go`), and so does `CoverProfile.validateClientHello` (`cover.go`). There is no +other authentication path. + +Real browsing is almost never resumption: + +| capture | connections | full | resumed | share | +|---|---|---|---|---| +| `harvest/testdata/resumption-ratio-session.log` — 16 pages, 6 revisits | 636 | 610 | 26 | **4.1%** | +| `harvest/testdata/resumption-ratio-cold-perprocess.log` | 485 | 472 | 13 | **2.7%** | + +`docs/design.md` records the mechanism: a page load opens each origin's connections in a *parallel +burst*, so every connection in the burst starts before any ticket has arrived and none can resume. +`static01.nytimes.com` shows 17 connections and 0 resumptions. 254 distinct origins from 16 page +loads, most contacted once. + +So we sit permanently in a ~4% bucket. A censor filtering on "resumption hello" shrinks its candidate +set ~25× for free. + +### Against `docs/design.md`, which argues this does not matter + +Read `docs/design.md` §"Outer resumption is a different thing, and it is ours" before this section. It +makes a real argument: 4.1% is an **inner** number — the browser reaching 254 destination origins — while +the **outer** connection to our egress is a layer we own, where ~100% resumption is attainable, and +`getlantern/http-proxy` has run outer resumption at scale for years. It concludes the resumption-hello +decision "does not depend on the 4.1%." + +The layer distinction is correct. The conclusion does not follow, for one reason: **the censor does not +see layers.** It sees TCP flows carrying ClientHellos. Our outer connection is not exempt from that +population — it is one more member of it. The relevant question is not "can we attain 100% resumption on +a layer we control" (we can) but "what fraction of the flows the censor observes are resumption hellos" +(~4%), and ours is 100% of them. + +So amend, do not overturn: + +- design.md is right that attainability is not the issue and that the outer ticket path is ours. Keep it. +- design.md is right that http-proxy is prior art for the operational half. Keep that too. +- What it misses is that **the anomaly is not resumption — it is exclusivity.** A client with a long + relationship to one host and a stack of its tickets *is* a real pattern (a mail server, a CDN, a sync + endpoint). What no real client does is reach a host for the first time already holding a ticket, and + never once complete a full handshake with it. + +That reframing sets the bar, and it is much lower than 4%: we do not need to match the wild distribution. +We need first contact to be a full handshake, and resumption afterwards to be what it is everywhere else — +the continuation of an observed relationship. See "Mix policy" below; that is why it needs no dice roll. + +**The sharper form of the problem.** You cannot resume a session that was never established. An +observer with flow history sees a `pre_shared_key` hello to an IP it never saw that client complete a +full handshake with — structurally impossible in real TLS. Combined with the SNI/IP inconsistency +(lantern-cloud#3292), that is a cheap two-term pre-filter that needs no DPI. + +Softening it, honestly: tickets legitimately survive days, client roaming and server IP rotation, and +CDNs share tickets across IPs, so a censor with finite history gets false positives. It is a strong +signal, not a proof — and note it is exactly the signal the first-contact-full policy erases, which is +the argument for building this at all. + +## What already exists + +Landed in #1, all of it prerequisite: + +- `ServerHelloFullLen = 1215` beside `ServerHelloResumedLen = 1221` (`serverhello.go`). The 6-byte + delta is `pre_shared_key`. +- `CoverProfile.FullRemainder []int` and `FullRemainderJitter []int`, deliberately **empty in the + table** — see below. `FullOpeningBurst()` and `CanEmitFullHandshake()` derive from them. +- `CoverProfile.Adopt` is variant-aware. The resumed variant is validated against its measured + constant; the full variant has no constant to compare to, so it is validated structurally (exact + ServerHello length, bounded record count, plausible certificate-sized burst). +- `harvest/coverprobe.ProbeBoth` measures both openings from one pair of connections, and + `SampleFull` measures the jitter. It lives under `harvest/` because it needs `crypto/tls` and + nothing shipped may import one — enforced by `TestShippedPackagesImportNoTLSLibrary`. + +### The measured full-handshake server profile + +`harvest/testdata/postflight-full-vs-resumed.log`: + +| | ServerHello | ccs | remainder | burst | +|---|---|---|---|---| +| cloudflare | 1215 | 6 | `[3848]` | 5069 | +| google | 1215 | 6 | `[3921]` | 5142 | +| microsoft | 1215 | 6 | `[32, 8273, 286, 74]` | 9886 | + +Three consequences: + +1. **The remainder is the certificate**, so a faithful full handshake costs **5–10 KB** of opening + overhead against ~1.3 KB resumed. Price this deliberately. +2. **It cannot be a table constant.** It moves run to run — cloudflare 3846/3847/3848, google + 3920/3921 — because the DER-encoded ECDSA signature in CertificateVerify varies in length, while + microsoft's fixed-length RSA signature holds 8273 exactly. It also changes on every certificate + rotation. `FullRemainder` must come from `coverprobe`, and an emitter must **jitter within the + sampled range**, or it is the only host on the network whose certificate flight never varies. + Sampled jitter is a **floor**: 5 samples reported 1 for cloudflare, but it has been seen at 3846, + 3847 *and* 3848. +3. microsoft splits into EncryptedExtensions/Certificate/CertificateVerify/Finished; cloudflare and + google coalesce all four. `ServerRemainder []int` already models this — the client must read one + record per entry (a fixed single read is the bug #1 hit). + +## The hard part: anti-replay without a ticket + +This is the piece that needs a decision, not code. Everything else is mechanical. + +The replay gate landed in #1 keys on `clientID` and `Issued`, **both of which come out of the +ticket's authenticated plaintext**. A PSK-less hello has neither. The carrier therefore needs its own +anti-replay story, and it must satisfy the lesson that gate was rebuilt around: *eviction is only +sound when the thing evicted is already rejected by validation.* + +### Proposed construction + +The README already names the carrier: `ClientHello.random` carries the MAC, `key_share` supplies the +ephemeral. 32 bytes of `random` to work with. Mirror the ticket's own shape: + +``` +random = AEAD(k_server, nonce, plaintext = clientID ‖ timestamp, aad = key_share ‖ cover_sni) + ~ 8B nonce + 8B plaintext + 16B tag = 32B +``` + +Why this shape: + +- **Server-key encrypted, like the ticket.** The egress decrypts without knowing which client first, + so there is no O(clients) trial-verification and no per-client lookup. +- **No linkable identifier on the wire.** A plaintext `clientID` would let a censor correlate every + connection from one client. Ciphertext under a fresh nonce is uniform to an observer, which is also + what `random` must look like. +- **`key_share` as AAD binds the ephemeral for free**, preventing substitution without spending bytes. +- **The freshness window can be SHORT.** There is no ticket lifetime to respect, so a ±30–60 s window + is enough. That makes the replay set O(connections within the window) instead of O(connections + within 24 h) — and, critically, the window *is* the whole horizon, so the set cannot forget + anything still valid. That is the property the ticket-keyed gate had to be redesigned to get. + +Anti-replay is then: verify the AEAD, check `|now − timestamp| < window`, and dedup on the +`key_share` (32 uniformly random bytes, fresh per connection, a better key than `random`) within the +window. + +### Open questions + +1. **Byte budget.** 8B nonce + 8B plaintext + 16B tag = 32B exactly, leaving ~4B for clientID and ~4B + for the timestamp. Is a 4-byte clientID enough for the population? Is a 4-byte second-resolution + timestamp enough range? Alternative: shrink the tag to 12B and take 4 more bytes of plaintext — + 96-bit authentication may be acceptable here since forging only yields a connection attempt, but + that is a judgement call. +2. **Clock skew.** The window depends on the client's clock. Mobile clocks are usually fine; decide + the tolerance and what happens to a client with a badly wrong clock (it fails to the cover path, + which is safe but invisible to the user). +3. **Where credential rotation lives.** It currently rides a NewSessionTicket-shaped record — but + cloudflare and google send **no** unprompted post-handshake records at all, so on those covers + that record has no counterpart. A full-handshake connection has no ticket to rotate anyway; decide + whether the full path rotates at all, or only issues on first contact. +4. **Mix policy — and it is not a ratio.** Per the reframing above, the target is not 4%. It is that a + censor watching this client and this egress has *seen the full handshake that the resumption + continues*. That makes the first connection to an egress full (there is no ticket yet anyway) and + later ones resumed, with no dice roll. + The real question is the **re-full cadence**, because the censor's flow history is finite and the + client's context changes: a reconnection a week later, from a different network, after the egress IP + rotated, has no observable predecessor even though we hold a valid ticket. Some trigger — new local + address, new egress IP, elapsed time — has to force a fresh full handshake. Note the resulting ratio + is likely *far above* 4% (a long-lived muxed tunnel opens few outer connections, so one full per + handful of resumed), and that is the correct outcome, not a miss: what a censor can check is whether + the predecessor exists, not whether we hit a population average. + +## Implementation sketch + +1. `SetRandomAuth` / `VerifyRandomAuth` in `auth.go`, mirroring `SetTicketAuth` / `VerifyTicketAuth`. + **Ordering matters:** the random binds `key_share`, so it must be written *after* `SetKeyShare` — + the same "authenticate over the final byte layout" constraint the binder has. The pipeline becomes + `SetSNI → Rerandomize → Shuffle → SetKeyShare → SetRandomAuth`. + Note `Rerandomize` currently overwrites `h.Random`, so the auth step must follow it. +2. Branch `validateClientHello` (`cover.go`) and `Server` (`handshake.go`) on whether + `pre_shared_key` is present, instead of requiring it. +3. A second replay gate keyed on `key_share` within the freshness window. Reuse the horizon-soundness + argument from `replay.go`; do not reuse its client-keyed structure, which depends on ticket fields. +4. Server emission: `ServerHelloFullLen`, then CCS, then one record per `FullRemainder` entry, + jittered within `FullRemainderJitter`. Client: one read per entry. +5. Gate on `CanEmitFullHandshake()` — an egress with no probed full profile must not offer the + carrier. Emitting a guessed certificate flight is worse than only offering the resumed path. +6. Extend `cover_test.go`'s oracle. Note it cannot pin `FullRemainder` to a literal (it is probed and + jitters); pin the *structure* instead — ServerHello length, record count, plausible range. + +## Traps worth knowing before starting + +Each of these cost real time in #1: + +- **A test whose oracle is the thing under test proves nothing.** `TestOpeningRecordSequenceMatchesCover` + compared the emitter against the profile that drove it, so collapsing microsoft's `[32 74]` to + `[106]` kept it green. `cover_test.go` now pins against literals transcribed from the logs. The full + variant needs the structural equivalent. +- **Mutation-test every guarantee.** Two regression tests in #1 passed with the fix removed. Break the + thing deliberately and confirm the test fails, or the test is decoration. +- **A flight-style probe measures the wrong handshake.** Emitting a hello and never completing it + yields the *full* profile (1215, multi-KB) — which is now what we want here, but it cannot reach the + resumed shape. Do not conflate the two probes. +- **Go's `crypto/tls` is a valid reference for the SERVER and the wrong one for the CLIENT.** Its + client flights are 64/64/80 against Chrome's measured 149/145/164. Anything client-side needs a + Chrome capture via `cmd/records` or `cmd/capture`. From b4f9c2fdd0b5e67d9a03187e24563fc5ffe44ad8 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 00:50:37 +0100 Subject: [PATCH 02/18] docs: correct the carrier -- the client cannot encrypt under the ticket key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous draft proposed random = AEAD(k_server, clientID ‖ timestamp). That cannot work: TicketKey never leaves the egress, so a client has no way to produce a ciphertext under it. In the resumption path the client encrypts nothing -- it presents a ciphertext the SERVER minted. The direction was backwards. Correcting it shrinks the problem rather than growing it. Provisioned clients always hold a Credential, which is why every hello is a resumption hello to begin with, so the question was never "authenticate with no credential". It is only "where does the ticket go, if not in pre_shared_key". And with the ticket still on the wire, clientID and Issued still come out of TicketKey.Open, so the ReplayCache merged in #1 applies unchanged and this document's former "hard part" does not exist. Proposes the GREASE ECH payload as the carrier. arrival-chrome152.log measured Chrome 152's ECH extension at 186/218/250/282 bytes, redrawn per connection -- a 144/176/208/240-byte payload of random bytes that rerandECHGrease already overwrites every connection. A ticket is AEAD ciphertext, so it is the same object, and it is the one field in the hello where uniform bytes of exactly that length are what belongs. Ticket length becomes free (only the payload length is observable), so fix it at 144 so every bucket stays reachable and pad to whatever bucket was drawn. The binder's job moves to random, which is 32 uniform bytes and takes a 32-byte HMAC exactly. Nothing new is provisioned. Records the objection this creates -- ech.md keeps a non-ECH pool as a deliberate escape hatch, and this couples authentication to it -- and why the answer is CanEmitFullHandshake falling back to the resumption path, which is where we already are. Keeps the ECDH-to-server-static construction as the documented fallback, and makes the Chrome real-ECH-with-secure-DNS measurement the first open question, because it is the one result that can invalidate the carrier. Co-Authored-By: Claude Opus 5 --- docs/full-handshake-carrier.md | 190 +++++++++++++++++++++------------ 1 file changed, 122 insertions(+), 68 deletions(-) diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md index 7da8b6c..06972d4 100644 --- a/docs/full-handshake-carrier.md +++ b/docs/full-handshake-carrier.md @@ -101,85 +101,139 @@ Three consequences: google coalesce all four. `ServerRemainder []int` already models this — the client must read one record per entry (a fixed single read is the bug #1 hit). -## The hard part: anti-replay without a ticket +## The carrier: where does the ticket go? -This is the piece that needs a decision, not code. Everything else is mechanical. +### First, a correction -The replay gate landed in #1 keys on `clientID` and `Issued`, **both of which come out of the -ticket's authenticated plaintext**. A PSK-less hello has neither. The carrier therefore needs its own -anti-replay story, and it must satisfy the lesson that gate was rebuilt around: *eviction is only -sound when the thing evicted is already rejected by validation.* +An earlier draft of this document proposed putting `AEAD(k_server, clientID ‖ timestamp)` in +`ClientHello.random`. **That is impossible.** `TicketKey` never leaves the egress (`auth.go`), so a client +cannot encrypt under it. In the resumption path the client does not encrypt anything — it presents a +ciphertext *the server minted for it*. The direction was backwards. -### Proposed construction +Correcting it shrinks the problem. Provisioned clients always hold a `Credential{Ticket, PSK}`; that is why +every hello is a resumption hello in the first place. So the question was never "authenticate with no +credential." It is: -The README already names the carrier: `ClientHello.random` carries the MAC, `key_share` supplies the -ephemeral. 32 bytes of `random` to work with. Mirror the ticket's own shape: +> **Where does the ticket go, if not in `pre_shared_key`?** + +And with the ticket still present, `clientID` and `Issued` come out of `TicketKey.Open` exactly as they do +today — so **the merged `ReplayCache` applies unchanged**, and this document's former "hard part" does not +exist. + +### Leading candidate: the GREASE ECH payload + +`harvest/testdata/arrival-chrome152.log` measured Chrome 152's ECH extension at **186/218/250/282 bytes** +across 7 hellos, redrawn per connection — a payload of 144/176/208/240 after the 42-byte header +(`config_type ‖ kdf ‖ aead ‖ config_id ‖ enc[32]` plus the two length prefixes). `echGREASELengths` in +`twiddle.go` already models exactly this, and `rerandECHGrease` already overwrites the payload with fresh +random bytes every connection. + +That payload is **the one field in the hello where 144–240 uniform bytes are precisely what belongs.** A +ticket is AEAD ciphertext. It is the same object. + +``` +full-handshake hello: + no pre_shared_key <- looks like a full handshake, because it is one + ECH payload = ticket ‖ random padding <- padded to the drawn Chrome bucket + random = HMAC(binderKey(psk), hello with random zeroed) + key_share = real ephemeral <- unchanged +``` + +Why each piece: + +- **Ticket length becomes free.** In the resumption path `TicketLen` is a hard fidelity parameter because + the ticket sets the emitted hello size (`auth.go`: cloudflare 176 → 1711 B). Inside the ECH payload it is + invisible; only the *payload* length is observable, and that is drawn from Chrome's buckets. So fix the + full-path ticket at **144 bytes** — it fits the smallest bucket, so every bucket stays reachable — and pad + with random bytes to whatever length `rerandECHGrease` drew. Length variation stays exactly Chrome's. +- **`random` takes over the binder's job.** The binder lives in `pre_shared_key` and dies with it. A + 32-byte HMAC keyed from the psk fits `random` exactly, and 32 uniform bytes is what `random` is. Same + "authenticate over the final byte layout" discipline: compute it last, over the marshalled hello with + `random` zeroed. +- **Nothing new is provisioned.** No new key material, no lantern-cloud or lantern-box change. The client + already holds the credential; the server already holds the ticket key. + +### The objection, which is real + +`docs/ech.md` concludes: *"ship ECH, and keep the ability to stop shipping it without shipping anything"* — +because the pool is data, a device tap from a browser that does not send ECH silently produces a non-ECH +pool, and that is the designed escape hatch if China ever blocks `0xfe0d`. + +Putting authentication in the ECH payload **couples the full-handshake path to a hedge built to be +dropped.** If the hedge fires, the carrier vanishes. + +The answer is that this is degradation, not breakage, and there is already a gate for it: +`CanEmitFullHandshake()` must additionally require an ECH extension with a large enough payload. A pool +without ECH falls back to the resumption path — which is exactly where we are today, so the floor is the +status quo. Say this out loud in the code, because a future reader will otherwise re-derive the objection +and assume it was missed. + +### Fallback candidate: ECDH to a server static key + +If the ECH coupling proves unacceptable, the REALITY-style construction works: ``` -random = AEAD(k_server, nonce, plaintext = clientID ‖ timestamp, aad = key_share ‖ cover_sni) - ~ 8B nonce + 8B plaintext + 16B tag = 32B +k_open = HKDF(ECDH(client_eph_priv, server_static_pub)) +random = AEAD(k_open, nonce = KDF(client_eph_pub), clientID ‖ timestamp ‖ psk_proof) ``` -Why this shape: - -- **Server-key encrypted, like the ticket.** The egress decrypts without knowing which client first, - so there is no O(clients) trial-verification and no per-client lookup. -- **No linkable identifier on the wire.** A plaintext `clientID` would let a censor correlate every - connection from one client. Ciphertext under a fresh nonce is uniform to an observer, which is also - what `random` must look like. -- **`key_share` as AAD binds the ephemeral for free**, preventing substitution without spending bytes. -- **The freshness window can be SHORT.** There is no ticket lifetime to respect, so a ±30–60 s window - is enough. That makes the replay set O(connections within the window) instead of O(connections - within 24 h) — and, critically, the window *is* the whole horizon, so the set cannot forget - anything still valid. That is the property the ticket-keyed gate had to be redesigned to get. - -Anti-replay is then: verify the AEAD, check `|now − timestamp| < window`, and dedup on the -`key_share` (32 uniformly random bytes, fresh per connection, a better key than `random`) within the -window. - -### Open questions - -1. **Byte budget.** 8B nonce + 8B plaintext + 16B tag = 32B exactly, leaving ~4B for clientID and ~4B - for the timestamp. Is a 4-byte clientID enough for the population? Is a 4-byte second-resolution - timestamp enough range? Alternative: shrink the tag to 12B and take 4 more bytes of plaintext — - 96-bit authentication may be acceptable here since forging only yields a connection attempt, but - that is a judgement call. -2. **Clock skew.** The window depends on the client's clock. Mobile clocks are usually fine; decide - the tolerance and what happens to a client with a badly wrong clock (it fails to the cover path, - which is safe but invisible to the user). -3. **Where credential rotation lives.** It currently rides a NewSessionTicket-shaped record — but - cloudflare and google send **no** unprompted post-handshake records at all, so on those covers - that record has no counterpart. A full-handshake connection has no ticket to rotate anyway; decide - whether the full path rotates at all, or only issues on first contact. -4. **Mix policy — and it is not a ratio.** Per the reframing above, the target is not 4%. It is that a - censor watching this client and this egress has *seen the full handshake that the resumption - continues*. That makes the first connection to an egress full (there is no ticket yet anyway) and - later ones resumed, with no dice roll. - The real question is the **re-full cadence**, because the censor's flow history is finite and the - client's context changes: a reconnection a week later, from a different network, after the egress IP - rotated, has no observable predecessor even though we hold a valid ticket. Some trigger — new local - address, new egress IP, elapsed time — has to force a fresh full handshake. Note the resulting ratio - is likely *far above* 4% (a long-lived muxed tunnel opens few outer connections, so one full per - handful of resumed), and that is the correct outcome, not a miss: what a censor can check is whether - the predecessor exists, not whether we hit a population average. +The server does **one** X25519 against its static private key to recover the opener key — no per-client +trial, no O(clients) scan. `docs/uniform-ephemeral.md` warns that "the client never performs a DH," but that +warning is about placing a raw curve point in a *ciphertext-shaped* field. Here the curve point goes in +`key_share`, where `auth.go` already says "a curve point is precisely what belongs and carries no anomaly at +all" — and the client already does exactly this DH today. + +Costs, and why it is second choice: + +- A new long-term server keypair, provisioned to every client — a lantern-cloud (`pcfg`) and lantern-box + change, i.e. cross-repo work the ECH carrier does not need. +- No ticket on the wire, so `clientID`/`Issued` no longer come from `TicketKey.Open` and the replay gate + **does** need the separate short-window construction this document previously described. Keep that + sketch in the git history for this case. +- Forward secrecy is unchanged for traffic (session keys still come from the ephemeral-ephemeral ECDH plus + psk), but a later compromise of the static key retroactively reveals the `clientID` in past openings. + The ECH carrier has no equivalent exposure. + +## Open questions + +1. **Does a real Chrome in-region send GREASE ECH to our cover hosts, or real ECH?** The carrier assumes + GREASE. `docs/ech.md` argues in-region it is GREASE — China prevents real ECH indirectly by censoring + encrypted DNS resolvers, so no ECHConfig is fetched — and `arrival-chrome152.log` measured GREASE 7/7 to + a bare IP with no DNS. But that capture *could not* have produced real ECH. **Measure the DNS-enabled + case** before building: a Chrome with secure DNS on, against `www.cloudflare.com`, will fetch the HTTPS + RR and send real ECH, whose payload length is set by the encrypted inner hello rather than by + `echGREASELengths`. If in-region clients would send real ECH, the payload-length model is wrong for them. + This is the one measurement that can invalidate the carrier, so run it first. +2. **Re-full cadence.** See "Mix policy" reasoning above: the censor's flow history is finite and the + client's context changes, so some trigger — new local address, new egress IP, elapsed time — has to + force a fresh full handshake rather than resuming forever off one observed predecessor. +3. **Where credential rotation lives.** It currently rides a NewSessionTicket-shaped record, but cloudflare + and google send **no** unprompted post-handshake records at all, so on those covers that record has no + counterpart. Unchanged by this work, but it lands in the same code. +4. **Full-path ticket length.** 144 bytes is proposed so every ECH bucket stays reachable. Confirm + `MinTicketLen` (76) leaves enough padding entropy, and decide whether the server should accept only 144 + or any length that decrypts. ## Implementation sketch -1. `SetRandomAuth` / `VerifyRandomAuth` in `auth.go`, mirroring `SetTicketAuth` / `VerifyTicketAuth`. - **Ordering matters:** the random binds `key_share`, so it must be written *after* `SetKeyShare` — - the same "authenticate over the final byte layout" constraint the binder has. The pipeline becomes - `SetSNI → Rerandomize → Shuffle → SetKeyShare → SetRandomAuth`. - Note `Rerandomize` currently overwrites `h.Random`, so the auth step must follow it. -2. Branch `validateClientHello` (`cover.go`) and `Server` (`handshake.go`) on whether - `pre_shared_key` is present, instead of requiring it. -3. A second replay gate keyed on `key_share` within the freshness window. Reuse the horizon-soundness - argument from `replay.go`; do not reuse its client-keyed structure, which depends on ticket fields. -4. Server emission: `ServerHelloFullLen`, then CCS, then one record per `FullRemainder` entry, - jittered within `FullRemainderJitter`. Client: one read per entry. -5. Gate on `CanEmitFullHandshake()` — an egress with no probed full profile must not offer the - carrier. Emitting a guessed certificate flight is worse than only offering the resumed path. -6. Extend `cover_test.go`'s oracle. Note it cannot pin `FullRemainder` to a literal (it is probed and - jitters); pin the *structure* instead — ServerHello length, record count, plausible range. +1. `SetECHTicketAuth` / `VerifyECHTicketAuth` in `auth.go`, mirroring `SetTicketAuth` / `VerifyTicketAuth`: + write the ticket into the ECH payload padded to the drawn bucket, then HMAC the marshalled hello with + `random` zeroed and write the result into `random`. + **Ordering matters,** the same rule the binder follows: the MAC covers the final byte layout, so it is + computed last. The pipeline becomes `SetSNI → Rerandomize → Shuffle → SetKeyShare → SetECHTicketAuth`. + Note `Rerandomize` overwrites `h.Random` (`twiddle.go:82`) *and* redraws the ECH payload + (`rerandECHGrease`), so both writes must follow it. +2. Branch `validateClientHello` (`cover.go:155`) and `Server` (`handshake.go`) on whether `pre_shared_key` + is present, instead of requiring it. The full path reads the ticket from ECH and verifies the `random` + MAC; everything downstream — `TicketKey.Open`, `ReplayCache.Consume`, `DeriveSession` — is unchanged. +3. Extend `CanEmitFullHandshake()` to also require an ECH extension whose payload can hold the ticket. A + pool without ECH falls back to resumption; say why in the comment (see "The objection" above). +4. Server emission: `ServerHelloFullLen`, then CCS, then one record per `FullRemainder` entry, jittered + within `FullRemainderJitter`. Client: one read per entry. +5. Extend `cover_test.go`'s oracle. It cannot pin `FullRemainder` to a literal (probed, and it jitters); + pin the *structure* — ServerHello length, record count, plausible range. +6. Mix policy: first contact to an egress is full, later connections resume, plus the re-full trigger from + open question 2. ## Traps worth knowing before starting From 5cba3b6510fa952573297db5813c8065f451e72b Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 00:52:12 +0100 Subject: [PATCH 03/18] measure: the cover identities publish no ECHConfig, so GREASE ECH holds The ECH-payload carrier assumes Chrome sends GREASE ECH, whose payload length is drawn from the four buckets arrival-chrome152.log measured. A Chrome that obtains an ECHConfig sends REAL ECH instead, whose payload length is set by the encrypted inner hello rather than by echGREASELengths, which would make the carrier's length model wrong. arrival-chrome152.log could not settle it: those hellos went to a bare IP with no DNS, so real ECH was impossible there by construction. This was the one open question that could invalidate the carrier, so it goes first. Queried the HTTPS RR for each cover across 1.1.1.1, 8.8.8.8 and 9.9.9.9, with crypto.cloudflare.com as a positive control to prove the method detects ech= when it is present. None of www.cloudflare.com, www.google.com or www.microsoft.com publishes an ECHConfig; the control does, on all three resolvers. So a Chrome with secure DNS fully working still cannot fetch one for any cover identity, and sends GREASE. This is a stronger result than the one in ech.md, which reaches the same conclusion for in-region clients by a contingent route -- China censors encrypted DNS resolvers, so no config gets fetched. Here the config does not exist to fetch, so the carrier holds for an unrestricted client too and does not depend on the censorship it exists to survive. Records the caveat that www.cloudflare.com itself does not enable ECH, only Cloudflare's demo host does, and that Cloudflare has enabled and rolled back ECH for customer zones before -- so this is monitorable rather than permanent. The log carries the one-line check to re-run. Co-Authored-By: Claude Opus 5 --- docs/full-handshake-carrier.md | 43 ++++++++---- harvest/testdata/ech-config-published.log | 84 +++++++++++++++++++++++ 2 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 harvest/testdata/ech-config-published.log diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md index 06972d4..d377328 100644 --- a/docs/full-handshake-carrier.md +++ b/docs/full-handshake-carrier.md @@ -153,6 +153,33 @@ Why each piece: - **Nothing new is provisioned.** No new key material, no lantern-cloud or lantern-box change. The client already holds the credential; the server already holds the ticket key. +### Measured: the covers publish no ECHConfig, so GREASE holds + +The carrier's length model only holds while Chrome sends *GREASE* ECH. A Chrome that obtains an ECHConfig +sends **real** ECH, whose payload length is set by the encrypted inner hello rather than by +`echGREASELengths` — which would make the model wrong. `arrival-chrome152.log` could not settle this: it +captured hellos to a bare IP with no DNS, so real ECH was impossible there by construction. + +Settled now — `harvest/testdata/ech-config-published.log`. Querying the HTTPS RR across three independent +resolvers, with `crypto.cloudflare.com` as a positive control that proves the method detects `ech=`: + +| host | ECHConfig published | +|---|---| +| `www.cloudflare.com` | no | +| `www.google.com` | no | +| `www.microsoft.com` | no | +| `crypto.cloudflare.com` *(control)* | **yes** | + +None of the three cover identities publishes one, so a Chrome with secure DNS fully working still cannot +fetch one for them and sends GREASE. **This is a stronger result than `docs/ech.md`'s**, which reaches the +same conclusion for in-region clients via a contingent route — China censors encrypted DNS resolvers, so no +config is fetched. Here the config does not exist to fetch, so the carrier holds for an unrestricted client +too and does not depend on the censorship it is meant to survive. + +Note `www.cloudflare.com` itself does not enable ECH; only the demo host does. Cloudflare has enabled and +rolled back ECH for customer zones before, so this is a **monitorable** condition, not a permanent one — the +log carries the one-line check. + ### The objection, which is real `docs/ech.md` concludes: *"ship ECH, and keep the ability to stop shipping it without shipping anything"* — @@ -196,21 +223,13 @@ Costs, and why it is second choice: ## Open questions -1. **Does a real Chrome in-region send GREASE ECH to our cover hosts, or real ECH?** The carrier assumes - GREASE. `docs/ech.md` argues in-region it is GREASE — China prevents real ECH indirectly by censoring - encrypted DNS resolvers, so no ECHConfig is fetched — and `arrival-chrome152.log` measured GREASE 7/7 to - a bare IP with no DNS. But that capture *could not* have produced real ECH. **Measure the DNS-enabled - case** before building: a Chrome with secure DNS on, against `www.cloudflare.com`, will fetch the HTTPS - RR and send real ECH, whose payload length is set by the encrypted inner hello rather than by - `echGREASELengths`. If in-region clients would send real ECH, the payload-length model is wrong for them. - This is the one measurement that can invalidate the carrier, so run it first. -2. **Re-full cadence.** See "Mix policy" reasoning above: the censor's flow history is finite and the +1. **Re-full cadence.** See "Mix policy" reasoning above: the censor's flow history is finite and the client's context changes, so some trigger — new local address, new egress IP, elapsed time — has to force a fresh full handshake rather than resuming forever off one observed predecessor. -3. **Where credential rotation lives.** It currently rides a NewSessionTicket-shaped record, but cloudflare +2. **Where credential rotation lives.** It currently rides a NewSessionTicket-shaped record, but cloudflare and google send **no** unprompted post-handshake records at all, so on those covers that record has no counterpart. Unchanged by this work, but it lands in the same code. -4. **Full-path ticket length.** 144 bytes is proposed so every ECH bucket stays reachable. Confirm +3. **Full-path ticket length.** 144 bytes is proposed so every ECH bucket stays reachable. Confirm `MinTicketLen` (76) leaves enough padding entropy, and decide whether the server should accept only 144 or any length that decrypts. @@ -233,7 +252,7 @@ Costs, and why it is second choice: 5. Extend `cover_test.go`'s oracle. It cannot pin `FullRemainder` to a literal (probed, and it jitters); pin the *structure* — ServerHello length, record count, plausible range. 6. Mix policy: first contact to an egress is full, later connections resume, plus the re-full trigger from - open question 2. + open question 1. ## Traps worth knowing before starting diff --git a/harvest/testdata/ech-config-published.log b/harvest/testdata/ech-config-published.log new file mode 100644 index 0000000..dcc8441 --- /dev/null +++ b/harvest/testdata/ech-config-published.log @@ -0,0 +1,84 @@ +Do the cover identities publish an ECHConfig? (No. None of them.) + + tool dig +short HTTPS @ + date 2026-09-03 + method query the HTTPS RR (TYPE65) for each cover host across three + independent public resolvers and look for the ech= SvcParam. + crypto.cloudflare.com is the POSITIVE CONTROL: it is Cloudflare's + ECH demo host and is known to publish one, so a run that does not + flag it has a broken method rather than a negative result. + +Why this was measured: docs/full-handshake-carrier.md proposes carrying the +ticket in the GREASE ECH payload, whose length is drawn from the four buckets +arrival-chrome152.log measured (extension 186/218/250/282, payload +144/176/208/240). That model only holds while Chrome sends GREASE ECH. A Chrome +that obtains an ECHConfig sends REAL ECH instead, whose payload length is set by +the encrypted inner hello rather than by echGREASELengths -- which would make +the carrier's length model wrong. arrival-chrome152.log could not settle this: +it captured hellos to a bare IP literal with no DNS at all, so real ECH was +impossible by construction there. + +RESULTS + + resolver 1.1.1.1 + www.cloudflare.com no ech= param + www.google.com no ech= param + www.microsoft.com no ech= param + crypto.cloudflare.com ECH PUBLISHED <- positive control + + resolver 8.8.8.8 + www.cloudflare.com no ech= param + www.google.com no ech= param + www.microsoft.com no ech= param + crypto.cloudflare.com ECH PUBLISHED <- positive control + + resolver 9.9.9.9 + www.cloudflare.com no ech= param + www.google.com no ech= param + www.microsoft.com no ech= param + crypto.cloudflare.com ECH PUBLISHED <- positive control + +Raw, for the two that matter most: + + www.cloudflare.com 1 . alpn="h3,h2" ipv4hint=104.16.123.96,104.16.124.96 + ipv6hint=2606:4700::6810:7b60,2606:4700::6810:7c60 + crypto.cloudflare.com 1 . alpn="h2" ipv4hint=162.159.135.79,162.159.136.79 + ech=AEX+DQBBwAAgACB9PwKag54xhjMV7Qdb++j+bLnTDMGC5H9P + cW/dD8tkaAAEAAEAAQASY2xvdWRmbGFyZS1lY2guY29tAAA= + ipv6hint=2606:4700:7::a29f:874f,2606:4700:7::a29f:884f + + www.google.com 1 . alpn="h2,h3" + www.microsoft.com CNAME chain to e13678.dscb.akamaiedge.net, no ech= + +FINDINGS + +1. None of the three cover identities publishes an ECHConfig. A Chrome with + secure DNS fully working still cannot fetch one for them, so it sends GREASE + ECH. The carrier's length model holds. + +2. This is a STRONGER result than the argument in docs/ech.md, which reaches + the same conclusion for in-region clients by a different route -- China + censors encrypted DNS resolvers, so no ECHConfig is fetched. That reasoning + is contingent on the censor. This one is not: the config does not exist to + fetch, so GREASE holds for an unrestricted client too, and the carrier does + not depend on the censorship it is meant to survive. + +3. Note that www.cloudflare.com itself does not enable ECH -- only the demo + host does. Cloudflare has enabled and then rolled back ECH for customer + zones before, so this is a MONITORABLE condition, not a permanent one. + +CAVEAT + +Not measured here: what a real Chrome emits with secure DNS on. This measures +only that the input real ECH requires is absent. That is sufficient to answer +the question that was blocking -- no ECHConfig, no real ECH -- but if +Cloudflare re-enables ECH on customer zones, re-run this and then capture an +actual Chrome hello before trusting echGREASELengths for a cloudflare cover. + +MONITOR + + dig +short HTTPS www.cloudflare.com | grep -q 'ech=' && echo "ECH now published" + +Run it against each cover identity. A hit means the GREASE payload-length model +no longer describes what a real Chrome sends to that host, and the carrier's +fidelity claim needs re-measuring for that cover. From 27af1d8b92bfa5de5696be10b30bfb7a4c949dd6 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 00:58:09 +0100 Subject: [PATCH 04/18] Carry the ticket in the ECH payload, so the opening can be a full handshake Every opening this package emits is a resumption hello, because the ticket travels in pre_shared_key and there is no other authentication path. Measured resumption share in real browsing is 4.1% over 636 connections and 2.7% over 485, so filtering on pre_shared_key shrinks a censor's candidate set about 25-fold at no cost. This adds the authenticator that lets an opening carry no pre_shared_key at all. The carrier is GREASE ECH's payload. arrival-chrome152.log measured Chrome 152's ECH extension at 186/218/250/282 bytes, redrawn per connection -- a 144/176/208/240-byte payload of random bytes that rerandECHGrease already rewrites on every emission. A ticket is AEAD ciphertext, so it is the same object, and this is the one field in the hello where uniform bytes of exactly that length are what belongs. The binder's job moves to random, which is 32 uniform bytes and takes a 32-byte HMAC exactly. Ticket length is free on this path, which it is not on the resumption path: there the ticket sets the emitted hello size and must match the impersonated identity. Inside the payload only the payload length is observable, so FullTicketLen is fixed at 144, the smallest bucket, and the rest is random padding. A larger ticket would silently delete buckets from the emitted length distribution -- at 176 the 144 bucket becomes unreachable, and a microsoft-sized 256 fits none of them. That is also why IssueFull exists rather than reusing the credential's ticket: the two paths size tickets for incompatible reasons, so a client carries both, sharing one clientID and psk. Sharing the psk is what keeps them one client rather than two identities, so rotation and the replay gate are unaffected -- and because the ticket survives on the wire, TicketKey.Open still yields clientID and issued, so ReplayCache applies to this path unchanged. Unlike the binder, which mirrors RFC 8446's Truncate() and covers only a prefix, this MAC covers the whole marshalled hello. There is no truncation rule to honour, so the stronger construction is also the simpler one, and SNI, key_share and the ECH padding are all bound. Every guarantee here was mutation-tested rather than assumed: withholding the MAC, truncating it to a 200-byte prefix, dropping the pre_shared_key exclusivity check, minting a fresh psk in IssueFull, dropping the payload-size check, and making the padding refill a no-op each break the test that claims them. The truncation mutation is the load-bearing one -- it breaks exactly the key_share and ECH-padding subtests, which is what distinguishes whole-hello coverage from binder-style prefix coverage. Comments record the objection this creates: ech.md keeps a non-ECH pool as a deliberate escape hatch if China ever blocks 0xfe0d, and this couples authentication to that hedge. A pool without a usable ECH payload simply cannot offer the path and falls back to resumption, which is where we already are. No wiring yet -- Credential, the handshake and the cover profile are untouched, so nothing emits this opening. That is the next commit. Co-Authored-By: Claude Opus 5 --- auth.go | 43 ++++-- echcarrier.go | 220 ++++++++++++++++++++++++++++ echcarrier_test.go | 354 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 608 insertions(+), 9 deletions(-) create mode 100644 echcarrier.go create mode 100644 echcarrier_test.go diff --git a/auth.go b/auth.go index afe6aa6..f1f26eb 100644 --- a/auth.go +++ b/auth.go @@ -96,6 +96,38 @@ func (k *TicketKey) Issue(clientID uint64, ticketLen int) (*Credential, error) { } func (k *TicketKey) issueAt(clientID uint64, ticketLen int, now time.Time) (*Credential, error) { + cred := &Credential{} + if _, err := rand.Read(cred.PSK[:]); err != nil { + return nil, err + } + ticket, err := k.seal(clientID, cred.PSK, ticketLen, now) + if err != nil { + return nil, err + } + cred.Ticket = ticket + return cred, nil +} + +// IssueFull mints the full-handshake companion to an existing credential: the +// same clientID and psk, sealed at FullTicketLen so it fits inside the ECH +// payload. +// +// A client needs both tickets because the two paths size them for different +// reasons. On the resumption path the length is a fidelity parameter -- the +// ticket sets the emitted hello size, so it must match the identity being +// impersonated. Inside the ECH payload it must instead fit Chrome's smallest +// bucket. Those two constraints do not meet: a microsoft-sized 256-byte ticket +// fits no ECH bucket at all. Sharing the psk is what keeps them one credential +// rather than two identities, so rotation and the replay gate see a single +// client either way. +func (k *TicketKey) IssueFull(clientID uint64, psk [32]byte) ([]byte, error) { + return k.seal(clientID, psk, FullTicketLen, time.Now()) +} + +// seal builds one ticket. The plaintext is padded to fill ticketLen so every +// ticket a server issues at a given length is that length, as a real server's +// would be. +func (k *TicketKey) seal(clientID uint64, psk [32]byte, ticketLen int, now time.Time) ([]byte, error) { if ticketLen < MinTicketLen { return nil, fmt.Errorf("twiddle: ticket length %d below minimum %d", ticketLen, MinTicketLen) } @@ -103,14 +135,10 @@ func (k *TicketKey) issueAt(clientID uint64, ticketLen int, now time.Time) (*Cre if err != nil { return nil, err } - cred := &Credential{} - if _, err := rand.Read(cred.PSK[:]); err != nil { - return nil, err - } plain := make([]byte, ticketLen-ticketNonceLen-ticketTagLen) binary.BigEndian.PutUint64(plain[0:8], clientID) - copy(plain[8:40], cred.PSK[:]) + copy(plain[8:40], psk[:]) binary.BigEndian.PutUint64(plain[40:48], uint64(now.Unix())) if _, err := rand.Read(plain[ticketFixed:]); err != nil { return nil, err @@ -120,8 +148,7 @@ func (k *TicketKey) issueAt(clientID uint64, ticketLen int, now time.Time) (*Cre if _, err := rand.Read(nonce); err != nil { return nil, err } - cred.Ticket = aead.Seal(nonce, nonce, plain, nil) - return cred, nil + return aead.Seal(nonce, nonce, plain, nil), nil } // Open recovers a ticket's contents. Only the holder of the ticket key can do @@ -341,5 +368,3 @@ func parsePSK(d []byte) (ticket []byte, age [4]byte, binder []byte, err error) { } return ticket, age, d[p+1 : p+1+bl], nil } - - diff --git a/echcarrier.go b/echcarrier.go new file mode 100644 index 0000000..16f60da --- /dev/null +++ b/echcarrier.go @@ -0,0 +1,220 @@ +package twiddle + +import ( + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "encoding/binary" + "errors" + "fmt" + "time" +) + +// The full-handshake carrier. +// +// Every opening this package emits is a RESUMPTION hello, because the ticket +// travels in pre_shared_key and there is no other authentication path. Real +// browsing is almost never resumption -- measured at 4.1% over 636 connections +// and 2.7% over 485 (harvest/testdata/resumption-ratio-*.log) -- so a censor +// filtering on the presence of pre_shared_key shrinks its candidate set about +// 25-fold for free. Worse, a resumption hello to an address the client was +// never seen completing a full handshake with is structurally impossible in +// real TLS. See docs/full-handshake-carrier.md. +// +// The fix is not to authenticate without a credential: provisioned clients +// always hold one, which is precisely why every hello is a resumption hello. +// It is to carry the ticket somewhere other than pre_shared_key. Here: +// +// ECH payload <- the ticket, padded to the drawn length with random bytes +// random <- HMAC over the whole hello, keyed from the psk +// key_share <- a real ephemeral, exactly as on the resumption path +// +// GREASE ECH's payload is the one field in a Chrome hello where 144 to 240 +// uniform bytes are precisely what belongs. Chrome fills it with random bytes +// and redraws both the contents and the length every connection +// (echGREASELengths, measured in harvest/testdata/arrival-chrome152.log). A +// ticket is AEAD ciphertext, so it is the same object, and rerandECHGrease +// already rewrites that field on every emission. +// +// Ticket length is therefore FREE on this path, which it is not on the +// resumption path: there the ticket sets the emitted hello size and must match +// the identity being impersonated (see DefaultTicketLen). Inside the ECH +// payload only the PAYLOAD length is observable, and that is drawn from +// Chrome's own buckets. FullTicketLen is fixed at the smallest bucket so every +// bucket stays reachable and the length distribution is unchanged. +// +// Because the ticket survives, TicketKey.Open still yields clientID and issued, +// so ReplayCache applies to this path unchanged. +// +// One consequence to keep in view. docs/ech.md keeps a non-ECH hello pool as a +// deliberate escape hatch: the pool is data, so if China ever blocks 0xfe0d we +// can stop emitting ECH without shipping a build. This carrier couples +// authentication to that hedge. The coupling is survivable rather than fatal -- +// a pool without a large enough ECH payload simply cannot offer this path and +// falls back to resumption, which is where we already are -- but it is real, +// and CanEmitFullHandshake is where it is enforced. + +// FullTicketLen is the ticket length for the full-handshake carrier. +// +// It is the smallest value in echGREASELengths, so a ticket fits EVERY bucket +// Chrome draws from and the emitted payload-length distribution stays exactly +// Chrome's. A larger ticket would silently delete buckets from that +// distribution -- at 176 the 144 bucket becomes unreachable, and a +// microsoft-sized 256 fits none of them at all. +const FullTicketLen = 144 + +// fullMACKey derives the key for the random-field MAC. It is domain-separated +// from binderKey so a value lifted from one path cannot be replayed into the +// other, even though both are keyed from the same psk. +func fullMACKey(psk []byte) []byte { + m := hmac.New(sha256.New, psk) + m.Write([]byte("twiddle/full-mac/v1")) + return m.Sum(nil) +} + +// echPayload returns the outer ECH extension's payload as a slice ALIASING the +// extension data, so writes to it land in the hello. +// +// The enc length is read rather than assumed. Chrome's is 32 bytes today, which +// is where the measured 42-byte header comes from, but a hello whose enc is a +// different size is still well formed and must not be silently misparsed. +func echPayload(e *Extension) ([]byte, error) { + d := e.Data + if len(d) < 1 { + return nil, errMalformed + } + if d[0] != 0x00 { + return nil, errors.New("twiddle: ECH extension is not the outer form") + } + p := 1 + 2 + 2 + 1 // config_type, kdf, aead, config_id + if len(d) < p+2 { + return nil, errMalformed + } + p += 2 + int(binary.BigEndian.Uint16(d[p:p+2])) // enc + if len(d) < p+2 { + return nil, errMalformed + } + n := int(binary.BigEndian.Uint16(d[p : p+2])) + p += 2 + if len(d) < p+n { + return nil, errMalformed + } + return d[p : p+n], nil +} + +// ECHPayloadLen reports the outer ECH payload size, or an error if the hello +// carries no usable one. It is what decides whether a pool can offer the +// full-handshake path at all. +func (h *ClientHello) ECHPayloadLen() (int, error) { + e := h.Find(ExtECH) + if e == nil { + return 0, errors.New("twiddle: hello has no ECH extension") + } + pay, err := echPayload(e) + if err != nil { + return 0, err + } + return len(pay), nil +} + +// SetECHTicketAuth installs a full-handshake authenticator: the ticket goes in +// the ECH payload, and the MAC over the finished hello goes in random. +// +// Call it last, for the same reason SetTicketAuth is called last -- the MAC +// covers the final byte layout, so anything that rewrites the hello afterwards +// invalidates it. Note that Rerandomize overwrites BOTH fields this uses, the +// random directly and the ECH payload through rerandECHGrease, so this must +// follow it and not merely follow SetKeyShare. +// +// Unlike the binder, which mirrors RFC 8446's Truncate() and therefore covers +// only a prefix, this MAC covers the whole hello. There is no truncation rule +// to honour here because the field is not a TLS binder, so the stronger +// construction is also the simpler one: SNI, key_share and the ECH padding are +// all bound. +func (h *ClientHello) SetECHTicketAuth(ticket []byte, psk []byte) error { + if len(ticket) != FullTicketLen { + return fmt.Errorf("twiddle: full-handshake ticket is %d bytes, want %d", len(ticket), FullTicketLen) + } + if h.Find(ExtPreSharedKey) != nil { + return errors.New("twiddle: full-handshake opening still carries pre_shared_key") + } + e := h.Find(ExtECH) + if e == nil { + return errors.New("twiddle: hello has no ECH extension to carry the ticket") + } + pay, err := echPayload(e) + if err != nil { + return err + } + if len(pay) < FullTicketLen { + return fmt.Errorf("twiddle: ECH payload is %d bytes, too small for a %d-byte ticket", len(pay), FullTicketLen) + } + copy(pay, ticket) + // The remainder is padding to whatever length was drawn. rerandECHGrease + // has already filled the whole payload with fresh random bytes, but fill it + // again rather than depend on having been called after it: a caller that + // skipped Rerandomize would otherwise emit a harvested browser's payload + // tail verbatim on every connection. + if _, err := rand.Read(pay[FullTicketLen:]); err != nil { + return err + } + + h.Random = [32]byte{} + m := hmac.New(sha256.New, fullMACKey(psk)) + m.Write(h.Marshal()) + copy(h.Random[:], m.Sum(nil)) + return nil +} + +// VerifyECHTicketAuth authenticates a full-handshake opening. maxAge bounds +// ticket lifetime; pass 0 to skip the check. +// +// The AuthResult is the same shape the resumption path returns, because the +// ticket is the same object -- which is what lets ReplayCache and +// DeriveSession stay untouched by this path. +func VerifyECHTicketAuth(h *ClientHello, k *TicketKey, maxAge time.Duration) (*AuthResult, error) { + return verifyECHAt(h, k, maxAge, time.Now()) +} + +func verifyECHAt(h *ClientHello, k *TicketKey, maxAge time.Duration, now time.Time) (*AuthResult, error) { + if h.Find(ExtPreSharedKey) != nil { + return nil, errors.New("twiddle: hello carries pre_shared_key; it belongs to the resumption path") + } + e := h.Find(ExtECH) + if e == nil { + return nil, errors.New("twiddle: hello has no ECH extension") + } + pay, err := echPayload(e) + if err != nil { + return nil, err + } + if len(pay) < FullTicketLen { + return nil, fmt.Errorf("twiddle: ECH payload is %d bytes, too small to carry a ticket", len(pay)) + } + clientID, psk, issued, err := k.Open(pay[:FullTicketLen]) + if err != nil { + return nil, err + } + if maxAge > 0 && now.Sub(issued) > maxAge { + return nil, fmt.Errorf("twiddle: ticket is %v old, limit %v", now.Sub(issued).Truncate(time.Second), maxAge) + } + + // Recompute over the hello with random cleared, which is the layout the + // MAC was taken over. Extensions are shared with h rather than copied + // because nothing here mutates them; only Random differs, and it is an + // array, so the struct copy already separates it. + mac := h.Random + probe := *h + probe.Random = [32]byte{} + m := hmac.New(sha256.New, fullMACKey(psk[:])) + m.Write(probe.Marshal()) + if !hmac.Equal(m.Sum(nil), mac[:]) { + return nil, errors.New("twiddle: full-handshake MAC does not verify") + } + + eph, err := h.KeyShare() + if err != nil { + return nil, err + } + return &AuthResult{ClientID: clientID, PSK: psk, Issued: issued, ClientEphemeral: eph}, nil +} diff --git a/echcarrier_test.go b/echcarrier_test.go new file mode 100644 index 0000000..93f3879 --- /dev/null +++ b/echcarrier_test.go @@ -0,0 +1,354 @@ +package twiddle + +import ( + "bytes" + "testing" + "time" +) + +// helloWithECHPayload returns a parsed pool hello whose ECH payload is exactly +// want bytes, so a test can pick the bucket it needs rather than hope. +func helloWithECHPayload(t *testing.T, want int) *ClientHello { + t.Helper() + for _, rec := range DefaultPool() { + h, err := ParseClientHello(rec) + if err != nil { + t.Fatal(err) + } + if n, err := h.ECHPayloadLen(); err == nil && n == want { + return h + } + } + t.Fatalf("no pool hello carries a %d-byte ECH payload", want) + return nil +} + +// fullCred mints a credential and its full-handshake companion ticket. +func fullCred(t *testing.T, k *TicketKey, clientID uint64) (*Credential, []byte) { + t.Helper() + cred, err := k.Issue(clientID, DefaultTicketLen) + if err != nil { + t.Fatal(err) + } + full, err := k.IssueFull(clientID, cred.PSK) + if err != nil { + t.Fatal(err) + } + return cred, full +} + +func TestECHCarrierRoundTrip(t *testing.T) { + k := ticketKey(t) + cred, full := fullCred(t, k, 42) + h := helloWithECHPayload(t, 240) + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + t.Fatal(err) + } + + res, err := VerifyECHTicketAuth(h, k, time.Hour) + if err != nil { + t.Fatalf("a well-formed full-handshake opening was rejected: %v", err) + } + if res.ClientID != 42 { + t.Errorf("clientID %d, want 42", res.ClientID) + } + if res.PSK != cred.PSK { + t.Error("recovered psk differs from the credential's; DeriveSession would disagree") + } + if res.ClientEphemeral == nil { + t.Error("no client ephemeral recovered") + } +} + +// IssueFull must mint a companion, not a second identity: the replay gate keys +// on clientID and the tunnel keys on psk, so a full ticket carrying either a +// different id or a different psk would silently split one client in two. +func TestIssueFullSharesTheCredentialIdentity(t *testing.T) { + k := ticketKey(t) + cred, full := fullCred(t, k, 7) + if len(full) != FullTicketLen { + t.Fatalf("full ticket is %d bytes, want %d", len(full), FullTicketLen) + } + if bytes.Equal(full, cred.Ticket) { + t.Fatal("the two tickets are byte-identical; this test proves nothing") + } + id, psk, _, err := k.Open(full) + if err != nil { + t.Fatal(err) + } + if id != 7 { + t.Errorf("clientID %d, want 7", id) + } + if psk != cred.PSK { + t.Error("the full ticket carries a different psk from the credential") + } +} + +// The point of this construction over the binder: the binder mirrors RFC 8446's +// Truncate() and covers only a prefix, whereas this MAC covers the whole hello. +// Each mutation below is a field an active adversary would rewrite, and each +// must break verification. Without the MAC actually spanning the marshalled +// hello, several of these would pass. +func TestECHCarrierMACCoversTheWholeHello(t *testing.T) { + k := ticketKey(t) + + mutations := []struct { + name string + bend func(t *testing.T, h *ClientHello) + }{ + {"SNI", func(t *testing.T, h *ClientHello) { + if err := h.SetSNI("www.example.org"); err != nil { + t.Fatal(err) + } + }}, + {"key_share", func(t *testing.T, h *ClientHello) { + e := h.Find(ExtKeyShare) + if e == nil { + t.Fatal("no key_share to bend") + } + e.Data[len(e.Data)-1] ^= 0x01 + }}, + {"ECH padding after the ticket", func(t *testing.T, h *ClientHello) { + pay, err := echPayload(h.Find(ExtECH)) + if err != nil { + t.Fatal(err) + } + if len(pay) <= FullTicketLen { + t.Fatalf("payload %d has no padding to bend", len(pay)) + } + pay[len(pay)-1] ^= 0x01 + }}, + {"ECH enc", func(t *testing.T, h *ClientHello) { + h.Find(ExtECH).Data[10] ^= 0x01 + }}, + {"cipher suites", func(t *testing.T, h *ClientHello) { + h.CipherSuites[len(h.CipherSuites)-1] ^= 0x0001 + }}, + {"session id", func(t *testing.T, h *ClientHello) { + if len(h.SessionID) == 0 { + t.Fatal("no session id to bend") + } + h.SessionID[0] ^= 0x01 + }}, + {"random itself", func(t *testing.T, h *ClientHello) { + h.Random[0] ^= 0x01 + }}, + } + + for _, m := range mutations { + t.Run(m.name, func(t *testing.T) { + cred, full := fullCred(t, k, 3) + h := helloWithECHPayload(t, 240) + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + t.Fatal(err) + } + if _, err := VerifyECHTicketAuth(h, k, time.Hour); err != nil { + t.Fatalf("baseline opening did not verify: %v", err) + } + + m.bend(t, h) + if _, err := VerifyECHTicketAuth(h, k, time.Hour); err == nil { + t.Errorf("verification still succeeded after bending %s; the MAC does not cover it", m.name) + } + }) + } +} + +func TestECHCarrierRejectsAForeignPSK(t *testing.T) { + k := ticketKey(t) + cred, full := fullCred(t, k, 5) + h := helloWithECHPayload(t, 240) + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + // A censor who captured a ticket but not the psk it pairs with. + var wrong [32]byte + copy(wrong[:], cred.PSK[:]) + wrong[0] ^= 0x01 + if err := h.SetECHTicketAuth(full, wrong[:]); err != nil { + t.Fatal(err) + } + if _, err := VerifyECHTicketAuth(h, k, time.Hour); err == nil { + t.Error("an opening MACed under the wrong psk was accepted") + } +} + +// The two paths are mutually exclusive by construction. A hello carrying both +// carriers is not a client we issued, and accepting one would give an adversary +// a choice of which authenticator to satisfy. +func TestECHCarrierAndResumptionAreExclusive(t *testing.T) { + k := ticketKey(t) + cred, full := fullCred(t, k, 9) + + h := helloWithECHPayload(t, 240) + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + if err := h.SetTicketAuth(cred, 32); err != nil { + t.Fatal(err) + } + if err := h.SetECHTicketAuth(full, cred.PSK[:]); err == nil { + t.Error("SetECHTicketAuth accepted a hello that still carries pre_shared_key") + } + if _, err := VerifyECHTicketAuth(h, k, time.Hour); err == nil { + t.Error("VerifyECHTicketAuth accepted a resumption hello") + } +} + +func TestECHCarrierRejectsAnExpiredTicket(t *testing.T) { + k := ticketKey(t) + cred, err := k.Issue(11, DefaultTicketLen) + if err != nil { + t.Fatal(err) + } + old, err := k.seal(11, cred.PSK, FullTicketLen, time.Now().Add(-48*time.Hour)) + if err != nil { + t.Fatal(err) + } + h := helloWithECHPayload(t, 240) + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + if err := h.SetECHTicketAuth(old, cred.PSK[:]); err != nil { + t.Fatal(err) + } + if _, err := VerifyECHTicketAuth(h, k, 24*time.Hour); err == nil { + t.Error("a ticket older than maxAge was accepted") + } + if _, err := VerifyECHTicketAuth(h, k, 0); err != nil { + t.Errorf("maxAge 0 should skip the age check: %v", err) + } +} + +// A pool whose hellos carry no ECH, or too small an ECH, cannot offer this path +// at all. That has to fail loudly at emission rather than produce an opening +// that no server can authenticate. +func TestECHCarrierRefusesAPayloadTooSmallToCarryATicket(t *testing.T) { + k := ticketKey(t) + cred, full := fullCred(t, k, 13) + + t.Run("no ECH extension", func(t *testing.T) { + h := helloWithECHPayload(t, 240) + for i := range h.Extensions { + if h.Extensions[i].Type == ExtECH { + h.Extensions = append(h.Extensions[:i], h.Extensions[i+1:]...) + break + } + } + if err := h.SetECHTicketAuth(full, cred.PSK[:]); err == nil { + t.Error("a hello with no ECH extension was accepted as a carrier") + } + }) + + t.Run("payload below FullTicketLen", func(t *testing.T) { + h := helloWithECHPayload(t, 240) + e := h.Find(ExtECH) + // Shrink the payload to one byte under the ticket size. + short := FullTicketLen - 1 + e.Data = append(e.Data[:len(e.Data)-240-2], byte(short>>8), byte(short)) + e.Data = append(e.Data, make([]byte, short)...) + if n, err := h.ECHPayloadLen(); err != nil || n != short { + t.Fatalf("payload is %d (%v), want %d", n, err, short) + } + err := h.SetECHTicketAuth(full, cred.PSK[:]) + if err == nil { + t.Fatal("a payload too small for the ticket was accepted") + } + if !contains(err.Error(), "too small") { + t.Errorf("unhelpful error: %v", err) + } + }) +} + +// The padding after the ticket is refilled on every call rather than inherited. +// rerandECHGrease normally supplies it, but a caller that reached this function +// without Rerandomize would otherwise emit one harvested browser's payload tail +// verbatim on every connection -- a per-device constant sitting in a field that +// is supposed to be fresh random bytes each time. +func TestECHCarrierRefreshesThePaddingItself(t *testing.T) { + k := ticketKey(t) + cred, full := fullCred(t, k, 19) + + tail := func() []byte { + h := helloWithECHPayload(t, 240) + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + // Deliberately NO Rerandomize: the padding must not come from the pool. + if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + t.Fatal(err) + } + pay, err := echPayload(h.Find(ExtECH)) + if err != nil { + t.Fatal(err) + } + return append([]byte(nil), pay[FullTicketLen:]...) + } + + a, b := tail(), tail() + if bytes.Equal(a, b) { + t.Error("two emissions from the same pool hello produced identical ECH padding") + } +} + +// What a censor actually sees. The emitted opening must carry no +// pre_shared_key -- that is the whole point -- and its ECH payload length must +// still be one Chrome draws, because a length outside the buckets is a +// distinguisher that costs one comparison. +func TestECHCarrierEmitsAFullHandshakeShape(t *testing.T) { + k := ticketKey(t) + + seen := map[int]bool{} + for i := 0; i < 200; i++ { + cred, full := fullCred(t, k, 17) + h, err := ParseClientHello(DefaultPool()[i%len(DefaultPool())]) + if err != nil { + t.Fatal(err) + } + if err := h.Rerandomize(); err != nil { + t.Fatal(err) + } + if _, err := h.SetKeyShare(); err != nil { + t.Fatal(err) + } + if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + t.Fatal(err) + } + + if h.Find(ExtPreSharedKey) != nil { + t.Fatal("the emitted opening carries pre_shared_key; it still reads as a resumption") + } + n, err := h.ECHPayloadLen() + if err != nil { + t.Fatal(err) + } + ok := false + for _, want := range echGREASELengths { + if n == want { + ok = true + } + } + if !ok { + t.Fatalf("ECH payload is %d bytes, which is not one of Chrome's buckets %v", n, echGREASELengths) + } + seen[n] = true + + // It must still authenticate after the round trip through Marshal. + reparsed, err := ParseClientHello(h.Marshal()) + if err != nil { + t.Fatal(err) + } + if _, err := VerifyECHTicketAuth(reparsed, k, time.Hour); err != nil { + t.Fatalf("the opening did not survive marshal/parse: %v", err) + } + } + if len(seen) < 2 { + t.Errorf("only saw payload lengths %v over 200 emissions; the length is not varying", seen) + } +} From a4062ea4eb5a5027a794154de6dc8952a362add5 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 01:56:43 +0100 Subject: [PATCH 05/18] Give every credential a full-handshake companion ticket, and rotate both A client needs two tickets because the paths size them for incompatible reasons: on the resumption path the length is a fidelity parameter that must match the impersonated identity, while inside the ECH payload it must fit Chrome's smallest bucket. A microsoft-sized 256-byte ticket fits no ECH bucket at all. Issue therefore mints both over one clientID and psk. Both are sealed at the SAME instant, which is load-bearing rather than tidy. ReplayCache refuses a ticket older than the client's newest, so two tickets of one credential bearing different issue times would make whichever path the client used second look like a stale capture and fail. That failure is invisible to a unit test of either path alone -- each works, and only using both breaks -- so TestBothTicketsOfOneCredentialAreSpendable exercises the pair through the gate. IssueFullFor, which upgrades a credential provisioned before the carrier, takes the TICKET rather than the fields for the same reason: clientID, psk and issue time can then only come from the ticket being companioned, so it is not possible to seal a companion the gate will later refuse. Rotation carries both, in two records rather than one. Not a stylistic choice: a single record would have to hold both tickets and the psk, which for a microsoft cover is 2+256+2+144+32 = 436 bytes against the 349 that fit inside sessionTicketWire, and writeSized would refuse it. Two records is also the more faithful shape, since microsoft was measured sending two unprompted NewSessionTickets after a full handshake, and it leaves the first record's layout byte-for-byte what it was. Rotating only the resumption ticket would let the companion age past MaxAge while the client kept working, silently collapsing it back to resumption-only, so the end-to-end test now asserts both. CredentialFromWire keeps its signature and is documented as resumption-only; CredentialFromWireFull is the additive form for provisioning that can supply both. A nil companion is legal and degrades to today's behaviour rather than failing. Mutation-tested: sealing the companion one second apart, having IssueFullFor reach for time.Now instead of the ticket's issue time, and dropping rotation's second record each break the test that claims them. Co-Authored-By: Claude Opus 5 --- auth.go | 54 +++++++++++++++++++++---------- cmd/twiddlecred/main.go | 4 +++ echcarrier_test.go | 7 ++-- handshake.go | 37 ++++++++++++++++++++- handshake_test.go | 9 ++++++ pool.go | 19 +++++++++++ replay_test.go | 71 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 179 insertions(+), 22 deletions(-) diff --git a/auth.go b/auth.go index f1f26eb..8ca4796 100644 --- a/auth.go +++ b/auth.go @@ -69,7 +69,15 @@ type TicketKey [32]byte // carries the next, exactly as NewSessionTicket does. type Credential struct { Ticket []byte - PSK [32]byte + // FullTicket is the same clientID and psk sealed at FullTicketLen, for the + // full-handshake carrier, which cannot use Ticket: the two paths size + // tickets for incompatible reasons. See IssueFullFor and echcarrier.go. + // + // Nil is legal and means resumption-only -- a credential provisioned before + // the carrier existed. Twiddle refuses the full path rather than emitting + // an opening no server can authenticate. + FullTicket []byte + PSK [32]byte } func NewTicketKey() (*TicketKey, error) { @@ -100,28 +108,40 @@ func (k *TicketKey) issueAt(clientID uint64, ticketLen int, now time.Time) (*Cre if _, err := rand.Read(cred.PSK[:]); err != nil { return nil, err } - ticket, err := k.seal(clientID, cred.PSK, ticketLen, now) - if err != nil { + var err error + if cred.Ticket, err = k.seal(clientID, cred.PSK, ticketLen, now); err != nil { + return nil, err + } + // Sealed at the SAME instant, deliberately. ReplayCache refuses a ticket + // older than the client's newest, so two tickets of one credential bearing + // different issue times would make whichever path the client used second + // look like a stale capture and fail. + if cred.FullTicket, err = k.seal(clientID, cred.PSK, FullTicketLen, now); err != nil { return nil, err } - cred.Ticket = ticket return cred, nil } -// IssueFull mints the full-handshake companion to an existing credential: the -// same clientID and psk, sealed at FullTicketLen so it fits inside the ECH -// payload. +// IssueFullFor mints the full-handshake companion for an EXISTING ticket, +// which is how a credential provisioned before the carrier is upgraded. +// +// A client needs both tickets because the two paths size them for +// incompatible reasons. On the resumption path the length is a fidelity +// parameter -- the ticket sets the emitted hello size, so it must match the +// identity being impersonated. Inside the ECH payload it must instead fit +// Chrome's smallest bucket. Those constraints do not meet: a microsoft-sized +// 256-byte ticket fits no ECH bucket at all. // -// A client needs both tickets because the two paths size them for different -// reasons. On the resumption path the length is a fidelity parameter -- the -// ticket sets the emitted hello size, so it must match the identity being -// impersonated. Inside the ECH payload it must instead fit Chrome's smallest -// bucket. Those two constraints do not meet: a microsoft-sized 256-byte ticket -// fits no ECH bucket at all. Sharing the psk is what keeps them one credential -// rather than two identities, so rotation and the replay gate see a single -// client either way. -func (k *TicketKey) IssueFull(clientID uint64, psk [32]byte) ([]byte, error) { - return k.seal(clientID, psk, FullTicketLen, time.Now()) +// It takes the ticket rather than the fields so the clientID, psk AND issue +// time can only come from the ticket being companioned. Passing those +// separately would make it possible to seal a companion with a different +// issue time, which ReplayCache would then read as a stale capture. +func (k *TicketKey) IssueFullFor(ticket []byte) ([]byte, error) { + clientID, psk, issued, err := k.Open(ticket) + if err != nil { + return nil, err + } + return k.seal(clientID, psk, FullTicketLen, issued) } // seal builds one ticket. The plaintext is padded to fill ticketLen so every diff --git a/cmd/twiddlecred/main.go b/cmd/twiddlecred/main.go index 4d13a39..2081e28 100644 --- a/cmd/twiddlecred/main.go +++ b/cmd/twiddlecred/main.go @@ -44,5 +44,9 @@ func main() { } fmt.Printf("ticket_key=%s\n", hex.EncodeToString(k[:])) fmt.Printf("ticket=%s\n", base64.StdEncoding.EncodeToString(cred.Ticket)) + // The full-handshake companion. Provisioning that omits it leaves the + // client resumption-only, which is the distinguisher the carrier exists to + // remove -- see docs/full-handshake-carrier.md. + fmt.Printf("full_ticket=%s\n", base64.StdEncoding.EncodeToString(cred.FullTicket)) fmt.Printf("psk=%s\n", hex.EncodeToString(cred.PSK[:])) } diff --git a/echcarrier_test.go b/echcarrier_test.go index 93f3879..feba280 100644 --- a/echcarrier_test.go +++ b/echcarrier_test.go @@ -30,11 +30,10 @@ func fullCred(t *testing.T, k *TicketKey, clientID uint64) (*Credential, []byte) if err != nil { t.Fatal(err) } - full, err := k.IssueFull(clientID, cred.PSK) - if err != nil { - t.Fatal(err) + if len(cred.FullTicket) != FullTicketLen { + t.Fatalf("Issue produced a %d-byte full ticket, want %d", len(cred.FullTicket), FullTicketLen) } - return cred, full + return cred, cred.FullTicket } func TestECHCarrierRoundTrip(t *testing.T) { diff --git a/handshake.go b/handshake.go index d5b4548..61797b9 100644 --- a/handshake.go +++ b/handshake.go @@ -235,14 +235,33 @@ func Server(raw net.Conn, cfg ServerConfig) (*Conn, error) { // both Finisheds, so it is outside the Wb=3 opening window the size bug was // about. Real later bursts also carry application data; matching that volume // is a later shaping concern. +// +// The size is a KNOWN-WRONG constant for all three covers -- microsoft was +// measured issuing 303-byte tickets and cloudflare and google issue none +// unprompted at all. Tracked in docs/full-handshake-carrier.md; not made worse +// here. const sessionTicketWire = 370 +// Rotation is TWO records, one per ticket, rather than one carrying both. +// +// Not a stylistic choice: a single record would have to hold both tickets and +// the psk, which for a microsoft cover is 2+256+2+144+32 = 436 bytes against +// the 349 that fit inside sessionTicketWire, and writeSized would refuse it. +// Two records is also the more faithful shape -- microsoft was measured +// sending two unprompted NewSessionTickets after a full handshake -- and it +// leaves the first record's layout byte-for-byte what it was. func writeTickets(c *Conn, next *Credential) error { body := make([]byte, 0, 2+len(next.Ticket)+32) body = appendU16(body, uint16(len(next.Ticket))) body = append(body, next.Ticket...) body = append(body, next.PSK[:]...) - return c.writeSized(contentHandshake, body, sessionTicketWire) + if err := c.writeSized(contentHandshake, body, sessionTicketWire); err != nil { + return err + } + full := make([]byte, 0, 2+len(next.FullTicket)) + full = appendU16(full, uint16(len(next.FullTicket))) + full = append(full, next.FullTicket...) + return c.writeSized(contentHandshake, full, sessionTicketWire) } func readTickets(c *Conn) (*Credential, error) { @@ -267,6 +286,22 @@ func readTickets(c *Conn) (*Credential, error) { } cred := &Credential{Ticket: append([]byte(nil), body[2:2+tl]...)} copy(cred.PSK[:], body[2+tl:2+tl+32]) + + typ, body, err = c.consumeRecord() + if err != nil { + return nil, err + } + if typ != contentHandshake { + return nil, errMalformed + } + if len(body) < 2 { + return nil, errMalformed + } + fl := int(binary.BigEndian.Uint16(body[0:2])) + if fl != FullTicketLen || 2+fl > len(body) { + return nil, errMalformed + } + cred.FullTicket = append([]byte(nil), body[2:2+fl]...) return cred, nil } diff --git a/handshake_test.go b/handshake_test.go index 29d1acc..9437e37 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -83,6 +83,15 @@ func TestEndToEndOverSocket(t *testing.T) { if _, _, _, err := k.Open(next.Ticket); err != nil { t.Fatalf("rotated ticket does not open: %v", err) } + // Rotation must carry BOTH tickets. Rotating only the resumption ticket + // would let the full-handshake companion age out of MaxAge while the + // client kept working, silently collapsing it back to resumption-only. + if len(next.FullTicket) != FullTicketLen { + t.Fatalf("rotated credential carries a %d-byte full ticket, want %d", len(next.FullTicket), FullTicketLen) + } + if _, _, _, err := k.Open(next.FullTicket); err != nil { + t.Fatalf("rotated full ticket does not open: %v", err) + } payload := make([]byte, 60000) rand.Read(payload) diff --git a/pool.go b/pool.go index 9baad24..bc2c95b 100644 --- a/pool.go +++ b/pool.go @@ -66,11 +66,30 @@ func ParsePool(s string) ([][]byte, error) { } // CredentialFromWire rebuilds a client credential from its provisioned form. +// +// The result is RESUMPTION-ONLY: it carries no full-handshake companion, so +// every opening it authenticates carries pre_shared_key. Provisioning that can +// supply both should call CredentialFromWireFull instead -- see +// docs/full-handshake-carrier.md for why emitting only resumption hellos is a +// distinguisher. func CredentialFromWire(ticket []byte, psk []byte) (*Credential, error) { + return CredentialFromWireFull(ticket, nil, psk) +} + +// CredentialFromWireFull rebuilds a credential that can open either handshake +// shape. fullTicket is the FullTicketLen companion sealed over the same +// clientID, psk and issue time; nil degrades to resumption-only. +func CredentialFromWireFull(ticket, fullTicket, psk []byte) (*Credential, error) { if len(psk) != 32 { return nil, fmt.Errorf("twiddle: psk is %d bytes, want 32", len(psk)) } + if fullTicket != nil && len(fullTicket) != FullTicketLen { + return nil, fmt.Errorf("twiddle: full ticket is %d bytes, want %d", len(fullTicket), FullTicketLen) + } c := &Credential{Ticket: append([]byte(nil), ticket...)} + if fullTicket != nil { + c.FullTicket = append([]byte(nil), fullTicket...) + } copy(c.PSK[:], psk) return c, nil } diff --git a/replay_test.go b/replay_test.go index 1e4daf2..4999ef2 100644 --- a/replay_test.go +++ b/replay_test.go @@ -188,3 +188,74 @@ func contains(s, sub string) bool { } return false } + +// A credential's two tickets must both be spendable. +// +// The gate refuses a ticket older than the client's newest, so if Issue sealed +// the companion even a second apart from the resumption ticket, whichever path +// the client used SECOND would be read as a stale capture and refused. That +// failure would be invisible in unit tests of either path alone: each works, +// and only using both breaks. +func TestBothTicketsOfOneCredentialAreSpendable(t *testing.T) { + k := ticketKey(t) + c := NewReplayCache(0, 0) + + cred, err := k.Issue(21, DefaultTicketLen) + if err != nil { + t.Fatal(err) + } + id, _, issued, err := k.Open(cred.Ticket) + if err != nil { + t.Fatal(err) + } + fid, _, fullIssued, err := k.Open(cred.FullTicket) + if err != nil { + t.Fatal(err) + } + if id != fid { + t.Fatalf("the two tickets carry different clientIDs (%d, %d); they are two clients, not one", id, fid) + } + + if !c.Consume(id, issued, cred.Ticket) { + t.Fatal("the resumption ticket was refused") + } + if !c.Consume(fid, fullIssued, cred.FullTicket) { + t.Error("the full-handshake companion was refused after the resumption ticket; their issue times disagree") + } + // Each is still single-use. + if c.Consume(fid, fullIssued, cred.FullTicket) { + t.Error("a replay of the full ticket was accepted") + } +} + +// IssueFullFor upgrades a resumption-only credential, and must take every +// field from the ticket it companions -- including the issue time, or it +// recreates the bug above. +func TestIssueFullForMatchesTheTicketItCompanions(t *testing.T) { + k := ticketKey(t) + old, err := k.issueAt(31, DefaultTicketLen, time.Now().Add(-3*time.Hour)) + if err != nil { + t.Fatal(err) + } + full, err := k.IssueFullFor(old.Ticket) + if err != nil { + t.Fatal(err) + } + id, psk, issued, err := k.Open(full) + if err != nil { + t.Fatal(err) + } + wantID, wantPSK, wantIssued, err := k.Open(old.Ticket) + if err != nil { + t.Fatal(err) + } + if id != wantID { + t.Errorf("clientID %d, want %d", id, wantID) + } + if psk != wantPSK { + t.Error("companion carries a different psk") + } + if !issued.Equal(wantIssued) { + t.Errorf("companion issued %v, want %v -- the replay gate would refuse whichever is used second", issued, wantIssued) + } +} From afeef38005bf2ada297768861016e3415ec2cd6c Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 09:34:08 +0100 Subject: [PATCH 06/18] Emit and accept the full-handshake opening end to end Wires the carrier from the previous commits into the two handshake paths. A client with FullHandshake set emits no pre_shared_key, and a server dispatches on the presence of that extension -- which is the same signal that selects the authenticator, so the validator and the verifier cannot disagree about which shape they are looking at. The pieces: SynthesizeServerHello gains FullHandshake, which omits pre_shared_key. That extension is exactly 6 bytes here -- type, length, selected_identity -- which is the entire difference between the two measured ServerHello lengths, 1221 resumed and 1215 full. The test now asserts that arithmetic rather than leaving it as a coincidence between two constants. validateClientHello splits. The shared half checks SNI and cipher; the full half reads the ticket out of the ECH payload. Deliberately NOT checked on the full path: whether the payload length is one of Chrome's buckets. That is a property of what we emit, asserted on the emission side, and a server refusing anything else would break the first device-tapped pool from a Chrome whose buckets differ from the ones we measured. Strictness there buys nothing against a censor, who sees the client's hello and not our validation of it. DrawFullRemainder draws each remainder record from [baseline, baseline+jitter] instead of sending the sampled sequence verbatim, because a certificate flight that is byte-identical on every connection is a distinguisher no real server produces. Adopt now carries RemainderJitter, and refuses a result whose jitter does not line up with its remainder: adopting a baseline without its range is exactly what produces the never-varying flight. Twiddle strips pre_shared_key on the full path rather than trusting the template. Harvested hellos routinely carry one -- they are captured from real browsing, which resumes, and the hellos in harvest/testdata do -- so this is the common case, not a defensive edge. The emitted hello is then shorter than the hello it came from by exactly that extension, which is precisely the difference between a real Chrome resumption hello and a real Chrome full one. Both ends gate on a measured profile, and both gates are tested. A client refuses to open a shape its cover was never probed for, and a server refuses to answer one, because emitting a guessed certificate flight is worse than only offering the resumed path. Measured end to end: SH 1215, ccs 6, remainder [32 8273 286 74] -- the microsoft full profile from postflight-full-vs-resumed.log, with the record COUNT right, which is what an observer actually counts. Nine mutations were tried and all nine break a test: ignoring the jitter, dropping it in Adopt, answering with the resumed ServerHello, sending the resumed remainder, reading the resumed record count client-side, verifying the resumption authenticator against a full hello, leaving the harvested pre_shared_key in place, and removing either gate. Co-Authored-By: Claude Opus 5 --- cover.go | 105 ++++++++++- cover_test.go | 87 +++++++++ handshake.go | 62 ++++++- handshake_test.go | 297 +++++++++++++++++++++++++++++++ harvest/coverprobe/coverprobe.go | 4 + hello.go | 14 ++ serverhello.go | 13 +- twiddle.go | 36 +++- 8 files changed, 599 insertions(+), 19 deletions(-) diff --git a/cover.go b/cover.go index 91fd616..ec78d55 100644 --- a/cover.go +++ b/cover.go @@ -1,8 +1,10 @@ package twiddle import ( + "crypto/rand" "errors" "fmt" + "math/big" "slices" "strings" "time" @@ -138,20 +140,65 @@ func (p CoverProfile) Valid() error { return nil } +// validateClientHello checks a hello against the cover identity and returns the +// ticket the replay gate must spend. +// +// It dispatches on the presence of pre_shared_key, which is the same signal +// that selects the authenticator: a hello carrying one is a resumption and its +// ticket is in there, a hello without one is a full handshake and its ticket is +// in the ECH payload. The two are never both valid, so there is no ambiguity to +// resolve and no order to get wrong. func (p CoverProfile) validateClientHello(h *ClientHello) ([]byte, error) { + if err := p.validateCoverIdentity(h); err != nil { + return nil, err + } + if h.Find(ExtPreSharedKey) == nil { + return p.validateFullClientHello(h) + } + return p.validateResumedClientHello(h) +} + +// validateCoverIdentity checks what both handshake shapes must satisfy. +func (p CoverProfile) validateCoverIdentity(h *ClientHello) error { if !strings.EqualFold(h.SNI(), p.Host) { - return nil, fmt.Errorf("twiddle: ClientHello SNI %q does not match cover %q", h.SNI(), p.Host) + return fmt.Errorf("twiddle: ClientHello SNI %q does not match cover %q", h.SNI(), p.Host) } - offersCipher := false for _, suite := range h.CipherSuites { if suite == p.CipherSuite { - offersCipher = true - break + return nil } } - if !offersCipher { - return nil, fmt.Errorf("twiddle: ClientHello does not offer cover cipher %#04x", p.CipherSuite) + return fmt.Errorf("twiddle: ClientHello does not offer cover cipher %#04x", p.CipherSuite) +} + +// validateFullClientHello checks a full-handshake opening and returns the +// ticket carried in the ECH payload. +// +// Deliberately NOT checked here: whether the payload length is one of Chrome's +// buckets. That is a property of what we EMIT, asserted on the emission side, +// and a server refusing anything else would break the first device-tapped pool +// from a Chrome whose buckets differ from the ones we measured. Strictness here +// buys nothing against a censor, who sees the client's hello and not our +// validation of it. +func (p CoverProfile) validateFullClientHello(h *ClientHello) ([]byte, error) { + if !p.CanEmitFullHandshake() { + return nil, fmt.Errorf("twiddle: cover %s has no measured full-handshake profile to answer with", p.Host) + } + e := h.Find(ExtECH) + if e == nil { + return nil, errors.New("twiddle: full-handshake ClientHello carries no ECH extension") + } + pay, err := echPayload(e) + if err != nil { + return nil, err + } + if len(pay) < FullTicketLen { + return nil, fmt.Errorf("twiddle: ECH payload is %d bytes, too small to carry a ticket", len(pay)) } + return pay[:FullTicketLen], nil +} + +func (p CoverProfile) validateResumedClientHello(h *ClientHello) ([]byte, error) { e := h.Find(ExtPreSharedKey) if e == nil { return nil, errors.New("twiddle: ClientHello carries no pre_shared_key") @@ -183,6 +230,41 @@ func (p CoverProfile) FullOpeningBurst() int { return total } +// DrawFullRemainder returns one emission's full-handshake remainder sequence, +// each record jittered within the range coverprobe sampled for it. +// +// Emitting FullRemainder verbatim would make this the only host on the network +// whose certificate flight is byte-identical on every connection, which is a +// distinguisher that costs a censor one comparison. The baseline is the +// smallest length observed and the draw is uniform over +// [baseline, baseline+jitter]. +// +// A sampled jitter is a FLOOR, not the true range: five samples reported 1 for +// cloudflare, which has since been seen at 3846, 3847 and 3848. Widening it +// from more samples is safe; narrowing it is not. +func (p CoverProfile) DrawFullRemainder() ([]int, error) { + if len(p.FullRemainder) == 0 { + return nil, fmt.Errorf("twiddle: cover %s has no measured full-handshake remainder", p.Host) + } + if len(p.FullRemainderJitter) != 0 && len(p.FullRemainderJitter) != len(p.FullRemainder) { + return nil, fmt.Errorf("twiddle: cover %s has %d remainder records but %d jitter ranges", + p.Host, len(p.FullRemainder), len(p.FullRemainderJitter)) + } + out := make([]int, len(p.FullRemainder)) + for i, base := range p.FullRemainder { + out[i] = base + if len(p.FullRemainderJitter) == 0 || p.FullRemainderJitter[i] <= 0 { + continue + } + n, err := rand.Int(rand.Reader, big.NewInt(int64(p.FullRemainderJitter[i])+1)) + if err != nil { + return nil, err + } + out[i] = base + int(n.Int64()) + } + return out, nil +} + // CanEmitFullHandshake reports whether this profile has been given a measured // full-handshake shape. Without one, an egress must not offer that carrier: // emitting a guessed certificate flight is worse than only offering the @@ -251,7 +333,11 @@ type ProbeResult struct { Remainder []int // OpeningBurst is ServerHello + ChangeCipherSpec + every remainder record. OpeningBurst int - Elapsed time.Duration + // RemainderJitter is the per-position range, in bytes, observed across + // samples. Same length as Remainder when set; only SampleFull fills it, + // because a single probe cannot see a range. + RemainderJitter []int + Elapsed time.Duration } const ( @@ -315,8 +401,13 @@ func (p CoverProfile) Adopt(res ProbeResult) (CoverProfile, error) { return p, fmt.Errorf("twiddle: full probe of %s returned a %d B opening burst, outside the plausible %d..%d for a certificate flight", res.Host, res.OpeningBurst, minFullBurst, maxFullBurst) } + if n := len(res.RemainderJitter); n != 0 && n != len(res.Remainder) { + return p, fmt.Errorf("twiddle: full probe of %s returned %d remainder records but %d jitter ranges", + res.Host, len(res.Remainder), n) + } out := p out.FullRemainder = append([]int(nil), res.Remainder...) + out.FullRemainderJitter = append([]int(nil), res.RemainderJitter...) return out, nil } diff --git a/cover_test.go b/cover_test.go index 0529b9a..68df3c6 100644 --- a/cover_test.go +++ b/cover_test.go @@ -127,3 +127,90 @@ func TestPerCoverHelpersAreCaseInsensitive(t *testing.T) { t.Errorf("TicketLenForCover(\"GitHub.com\")=%d, want the recorded 32", got) } } + +// DrawFullRemainder must actually move. An emitter that sent FullRemainder +// verbatim would be the only host on the network whose certificate flight is +// byte-identical on every connection -- and every test that merely checks the +// sequence is "plausible" would still pass, which is why this asserts variation +// rather than membership. +func TestDrawFullRemainderVariesWithinTheSampledRange(t *testing.T) { + p := CoverProfile{ + Host: "example.test", + FullRemainder: []int{3846, 100, 8273}, + FullRemainderJitter: []int{2, 0, 1}, + } + seen := make([]map[int]bool, len(p.FullRemainder)) + for i := range seen { + seen[i] = map[int]bool{} + } + for i := 0; i < 400; i++ { + got, err := p.DrawFullRemainder() + if err != nil { + t.Fatal(err) + } + if len(got) != len(p.FullRemainder) { + t.Fatalf("drew %d records, want %d", len(got), len(p.FullRemainder)) + } + for j, n := range got { + lo := p.FullRemainder[j] + hi := lo + p.FullRemainderJitter[j] + if n < lo || n > hi { + t.Fatalf("record %d drew %d, outside the sampled [%d, %d]", j, n, lo, hi) + } + seen[j][n] = true + } + } + // Position 0 has jitter 2 and position 2 has jitter 1, so both must have + // produced more than one value. Position 1 has jitter 0 and must not. + if len(seen[0]) != 3 { + t.Errorf("record 0 produced %d distinct lengths over 400 draws, want all 3 of [3846, 3848]: %v", len(seen[0]), seen[0]) + } + if len(seen[1]) != 1 { + t.Errorf("record 1 has zero jitter but produced %v", seen[1]) + } + if len(seen[2]) != 2 { + t.Errorf("record 2 produced %d distinct lengths over 400 draws, want 2: %v", len(seen[2]), seen[2]) + } +} + +// A baseline adopted without its range is what produces the never-varying +// flight above, so Adopt must carry the jitter and must refuse a result whose +// jitter does not line up with its remainder. +func TestAdoptCarriesTheFullRemainderJitter(t *testing.T) { + base, err := CoverFor("www.microsoft.com") + if err != nil { + t.Fatal(err) + } + remainder := []int{32, 8273, 286, 74} + burst := ServerHelloFullLen + len(ChangeCipherSpec()) + for _, n := range remainder { + burst += n + } + res := ProbeResult{ + Host: base.Host, Full: true, ServerHello: ServerHelloFullLen, + Remainder: remainder, RemainderJitter: []int{0, 1, 0, 0}, OpeningBurst: burst, + } + + got, err := base.Adopt(res) + if err != nil { + t.Fatal(err) + } + if !slices.Equal(got.FullRemainderJitter, res.RemainderJitter) { + t.Errorf("adopted jitter %v, want %v", got.FullRemainderJitter, res.RemainderJitter) + } + + // A jitter of the wrong length cannot be applied position-by-position, and + // silently ignoring it would drop the variation without saying so. + bad := res + bad.RemainderJitter = []int{0, 1} + if _, err := base.Adopt(bad); err == nil { + t.Error("a jitter shorter than the remainder was adopted") + } + if _, err := (CoverProfile{ + Host: base.Host, + FullRemainder: remainder, + FullRemainderJitter: []int{0, 1}, + }).DrawFullRemainder(); err == nil { + t.Error("DrawFullRemainder accepted a mismatched jitter") + } +} diff --git a/handshake.go b/handshake.go index 61797b9..a70b336 100644 --- a/handshake.go +++ b/handshake.go @@ -28,7 +28,17 @@ type ClientConfig struct { // Credential is the ticket and psk to present. Replaced after each // connection with the one the server issues as a post-handshake ticket. Credential *Credential - Shaper Shaper + // FullHandshake opens with a FULL-handshake shape instead of a resumption: + // no pre_shared_key, the ticket in the ECH payload, and a server flight + // carrying a certificate-sized remainder. + // + // It exists because emitting only resumption hellos is itself a + // distinguisher -- measured at 4.1% of real browsing -- and because a + // resumption to an address the client was never seen completing a full + // handshake with is structurally impossible in real TLS. First contact with + // an egress should use this; see docs/full-handshake-carrier.md. + FullHandshake bool + Shaper Shaper } // ServerConfig is what an egress needs to accept one. @@ -62,15 +72,30 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { if len(cfg.Credential.Ticket) != cfg.Cover.TicketLen { return nil, nil, fmt.Errorf("twiddle: credential ticket length %d does not match cover %d", len(cfg.Credential.Ticket), cfg.Cover.TicketLen) } + // Refused here rather than on the wire. A client that opened a full + // handshake against a cover with no measured full profile would get a + // guessed certificate flight back, which is worse than not offering the + // shape at all. + if cfg.FullHandshake && !cfg.Cover.CanEmitFullHandshake() { + return nil, nil, fmt.Errorf("twiddle: cover %s has no measured full-handshake profile", cfg.Cover.Host) + } + // The remainder record COUNT is what the client reads, so the two shapes + // are read differently and picking the wrong sequence misaligns every + // later read. + remainder := cfg.Cover.ResumedRemainder + if cfg.FullHandshake { + remainder = cfg.Cover.FullRemainder + } pick, err := rand.Int(rand.Reader, bigLen(len(cfg.Pool))) if err != nil { return nil, nil, err } wire, eph, err := Twiddle(cfg.Pool[pick.Int64()], Options{ - CoverSNI: cfg.Cover.Host, - Credential: cfg.Credential, - BinderLen: cfg.Cover.BinderLen, + CoverSNI: cfg.Cover.Host, + Credential: cfg.Credential, + BinderLen: cfg.Cover.BinderLen, + FullHandshake: cfg.FullHandshake, }) if err != nil { return nil, nil, err @@ -109,7 +134,7 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { // remainder 32/74 where cloudflare and google coalesce it into one 64. // Reading a fixed one record left microsoft's second record in the stream // and every later read misaligned. - for range cfg.Cover.ResumedRemainder { + for range remainder { if _, _, err := conn.consumeRecord(); err != nil { return nil, nil, err } @@ -166,7 +191,17 @@ func Server(raw net.Conn, cfg ServerConfig) (*Conn, error) { if err != nil { return nil, ErrNotOurs } - res, err := VerifyTicketAuth(h, cfg.TicketKey, maxAge) + // The same signal that selected the validator selects the authenticator, + // and the two must not disagree: a hello with pre_shared_key is a + // resumption and its ticket came out of that extension, a hello without one + // is a full handshake and its ticket came out of the ECH payload. + full := h.Find(ExtPreSharedKey) == nil + var res *AuthResult + if full { + res, err = VerifyECHTicketAuth(h, cfg.TicketKey, maxAge) + } else { + res, err = VerifyTicketAuth(h, cfg.TicketKey, maxAge) + } if err != nil { return nil, ErrNotOurs } @@ -183,6 +218,7 @@ func Server(raw net.Conn, cfg ServerConfig) (*Conn, error) { CipherSuite: cfg.Cover.CipherSuite, ServerEphemeral: priv.PublicKey(), PSKFirst: cfg.Cover.PSKFirst, + FullHandshake: full, }) if err != nil { return nil, err @@ -208,8 +244,18 @@ func Server(raw net.Conn, cfg ServerConfig) (*Conn, error) { } // One write per record the cover actually sends: microsoft splits the - // remainder 32/74 where cloudflare and google coalesce it into one 64. - for _, n := range cfg.Cover.ResumedRemainder { + // resumed remainder 32/74 where cloudflare and google coalesce it into one + // 64. The full remainder is the certificate flight -- one to two orders of + // magnitude larger -- and is drawn fresh each connection, because a + // certificate flight that is byte-identical every time is a distinguisher + // no real server produces. + remainder := cfg.Cover.ResumedRemainder + if full { + if remainder, err = cfg.Cover.DrawFullRemainder(); err != nil { + return nil, err + } + } + for _, n := range remainder { if err := conn.writeSized(contentHandshake, nil, n); err != nil { return nil, err } diff --git a/handshake_test.go b/handshake_test.go index 9437e37..7358b80 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -234,6 +234,34 @@ func TestSynthesizedServerHelloMatchesMeasuredLength(t *testing.T) { h, _ := ParseClientHello(wire) eph, _ := h.KeyShare() + // The full variant omits pre_shared_key, and that omission is the ENTIRE + // difference between the two measured lengths -- 6 bytes: type, length and + // selected_identity. Both are point targets from + // harvest/testdata/postflight-full-vs-resumed.log, not ranges. + if ServerHelloResumedLen-ServerHelloFullLen != 6 { + t.Errorf("the measured lengths differ by %d, not the 6 bytes pre_shared_key occupies", + ServerHelloResumedLen-ServerHelloFullLen) + } + for _, full := range []bool{false, true} { + for _, pskFirst := range []bool{false, true} { + sh, err := SynthesizeServerHello(ServerHelloParams{ + SessionIDEcho: h.SessionID, ServerEphemeral: eph, + PSKFirst: pskFirst, FullHandshake: full, + }) + if err != nil { + t.Fatal(err) + } + want := ServerHelloResumedLen + if full { + want = ServerHelloFullLen + } + if len(sh) != want { + t.Errorf("full=%v PSKFirst=%v: ServerHello is %d bytes, measured %d", + full, pskFirst, len(sh), want) + } + } + } + for _, pskFirst := range []bool{false, true} { sh, err := SynthesizeServerHello(ServerHelloParams{ SessionIDEcho: h.SessionID, ServerEphemeral: eph, PSKFirst: pskFirst, @@ -408,3 +436,272 @@ func TestReadTicketsRejectsNonHandshakeRecords(t *testing.T) { t.Error("readTickets accepted an application_data record as a rotated credential") } } + +// fullCover returns a microsoft profile with a full-handshake shape adopted. +// The table ships none on purpose -- the certificate flight cannot be a +// constant -- so a test that needs one must supply it the way an egress does, +// through Adopt, which also exercises the adoption path. +func fullCover(t *testing.T) CoverProfile { + t.Helper() + base := mustCover(t, "www.microsoft.com") + // harvest/testdata/postflight-full-vs-resumed.log + remainder := []int{32, 8273, 286, 74} + burst := ServerHelloFullLen + len(ChangeCipherSpec()) + for _, n := range remainder { + burst += n + } + p, err := base.Adopt(ProbeResult{ + Host: base.Host, Full: true, + ServerHello: ServerHelloFullLen, + Remainder: remainder, + RemainderJitter: []int{0, 1, 0, 0}, + OpeningBurst: burst, + }) + if err != nil { + t.Fatal(err) + } + if !p.CanEmitFullHandshake() { + t.Fatal("adopted profile still cannot emit a full handshake") + } + return p +} + +// The full-handshake path end to end: no pre_shared_key anywhere, a 1215-byte +// ServerHello, a certificate-sized remainder, and bytes through the tunnel. +func TestEndToEndFullHandshake(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + cred, err := k.Issue(77, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + + type result struct { + conn *Conn + err error + } + srvCh := make(chan result, 1) + go func() { + c, err := ln.Accept() + if err != nil { + srvCh <- result{nil, err} + return + } + sc, err := Server(c, ServerConfig{ + TicketKey: k, Cover: cover, + MaxAge: time.Hour, Replay: NewReplayCache(16, time.Hour), + }) + srvCh <- result{sc, err} + }() + + raw, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + cc, next, err := Client(raw, ClientConfig{ + Pool: pool(t), Cover: cover, Credential: cred, FullHandshake: true, + }) + if err != nil { + t.Fatalf("client: %v", err) + } + r := <-srvCh + if r.err != nil { + t.Fatalf("server: %v", r.err) + } + if next == nil || len(next.FullTicket) != FullTicketLen { + t.Fatal("the full-handshake flight did not rotate both tickets") + } + + payload := make([]byte, 40000) + rand.Read(payload) + go func() { + r.conn.Write(payload) + r.conn.Close() + }() + got, err := io.ReadAll(cc) + if err != nil && err != io.EOF { + t.Fatalf("read: %v", err) + } + if !bytes.Equal(got, payload) { + t.Fatalf("payload mismatch: got %d bytes, want %d", len(got), len(payload)) + } + t.Logf("opened a full handshake and carried %d bytes", len(got)) +} + +// What the censor counts. The server's answer to a full handshake must be a +// 1215-byte ServerHello, a ChangeCipherSpec, and one record per FullRemainder +// entry -- microsoft's four, not a single coalesced blob. A fixed record count +// here was the bug the resumed path already hit from the other side. +func TestServerAnswersAFullHandshakeWithTheMeasuredShape(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + cred, err := k.Issue(78, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + go func() { + c, err := ln.Accept() + if err != nil { + return + } + defer c.Close() + c.SetDeadline(time.Now().Add(5 * time.Second)) + Server(c, ServerConfig{ + TicketKey: k, Cover: cover, + MaxAge: time.Hour, Replay: NewReplayCache(16, time.Hour), + }) + }() + + raw, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer raw.Close() + raw.SetDeadline(time.Now().Add(5 * time.Second)) + + wire, _, err := Twiddle(pool(t)[0], Options{ + CoverSNI: cover.Host, Credential: cred, FullHandshake: true, + }) + if err != nil { + t.Fatal(err) + } + // The opening itself must read as a full handshake. + h, err := ParseClientHello(wire) + if err != nil { + t.Fatal(err) + } + if h.Find(ExtPreSharedKey) != nil { + t.Fatal("the emitted opening carries pre_shared_key; it still reads as a resumption") + } + if _, err := raw.Write(wire); err != nil { + t.Fatal(err) + } + + sh, err := readRecord(raw) + if err != nil { + t.Fatalf("ServerHello: %v", err) + } + if len(sh) != ServerHelloFullLen { + t.Errorf("ServerHello is %d bytes, want the full-handshake %d", len(sh), ServerHelloFullLen) + } + if _, err := readRecord(raw); err != nil { // ChangeCipherSpec + t.Fatalf("ChangeCipherSpec: %v", err) + } + + var got []int + for range cover.FullRemainder { + rec, err := readRecord(raw) + if err != nil { + t.Fatalf("remainder record %d of %d: %v", len(got)+1, len(cover.FullRemainder), err) + } + got = append(got, len(rec)) + } + if len(got) != len(cover.FullRemainder) { + t.Fatalf("read %d remainder records, want %d", len(got), len(cover.FullRemainder)) + } + for i, n := range got { + lo := cover.FullRemainder[i] + hi := lo + cover.FullRemainderJitter[i] + if n < lo || n > hi { + t.Errorf("remainder record %d is %d bytes, outside the sampled [%d, %d]", i, n, lo, hi) + } + } + // A fifth record would mean the server coalesced or split differently than + // the identity it claims. + raw.SetReadDeadline(time.Now().Add(250 * time.Millisecond)) + if extra, err := readRecord(raw); err == nil { + t.Errorf("server sent an unexpected %d-byte record after the remainder", len(extra)) + } + t.Logf("full opening: SH %d, ccs %d, remainder %v", len(sh), len(ChangeCipherSpec()), got) +} + +// The two ways a client can ask for a shape it cannot produce. +func TestFullHandshakeRefusesWhatItCannotBack(t *testing.T) { + k := ticketKey(t) + + t.Run("cover has no measured full profile", func(t *testing.T) { + cover := mustCover(t, "www.microsoft.com") // table default: no FullRemainder + cred, _ := k.Issue(79, cover.TicketLen) + _, _, err := Client(nil, ClientConfig{ + Pool: pool(t), Cover: cover, Credential: cred, FullHandshake: true, + }) + if err == nil { + t.Fatal("a full handshake was attempted against a cover with no measured profile") + } + if !contains(err.Error(), "full-handshake profile") { + t.Errorf("unhelpful error: %v", err) + } + }) + + t.Run("server has no measured full profile", func(t *testing.T) { + // The client gate is not the only one that matters: a server whose cover + // was never probed must refuse rather than answer with a guessed + // certificate flight, which would be a distinguisher of its own. + emit := fullCover(t) + serve := mustCover(t, "www.microsoft.com") // table default + cred, _ := k.Issue(81, emit.TicketLen) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + errCh := make(chan error, 1) + go func() { + c, err := ln.Accept() + if err != nil { + errCh <- err + return + } + defer c.Close() + c.SetDeadline(time.Now().Add(3 * time.Second)) + _, err = Server(c, ServerConfig{ + TicketKey: k, Cover: serve, + MaxAge: time.Hour, Replay: NewReplayCache(16, time.Hour), + }) + errCh <- err + }() + raw, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer raw.Close() + wire, _, err := Twiddle(pool(t)[0], Options{ + CoverSNI: emit.Host, Credential: cred, FullHandshake: true, + }) + if err != nil { + t.Fatal(err) + } + if _, err := raw.Write(wire); err != nil { + t.Fatal(err) + } + if err := <-errCh; err != ErrNotOurs { + t.Fatalf("got %v, want ErrNotOurs -- the server answered a shape it has not measured", err) + } + }) + + t.Run("credential has no companion ticket", func(t *testing.T) { + cover := fullCover(t) + cred, _ := k.Issue(80, cover.TicketLen) + cred.FullTicket = nil // as CredentialFromWire would leave it + _, _, err := Twiddle(pool(t)[0], Options{ + CoverSNI: cover.Host, Credential: cred, FullHandshake: true, + }) + if err == nil { + t.Fatal("a full handshake was emitted from a resumption-only credential") + } + if !contains(err.Error(), "full ticket") { + t.Errorf("unhelpful error: %v", err) + } + }) +} diff --git a/harvest/coverprobe/coverprobe.go b/harvest/coverprobe/coverprobe.go index 5f51a54..ac203f6 100644 --- a/harvest/coverprobe/coverprobe.go +++ b/harvest/coverprobe/coverprobe.go @@ -313,6 +313,10 @@ func SampleFull(ctx context.Context, dial Dialer, host string, n int) (tw.ProbeR jitter[i] = hi[i] - lo[i] } base.Remainder = lo + // Carried inside the result as well as returned, so CoverProfile.Adopt gets + // the baseline and its range together. Adopting a baseline without the + // range is what produces an emitter whose certificate flight never varies. + base.RemainderJitter = jitter base.OpeningBurst = tw.ServerHelloFullLen + len(tw.ChangeCipherSpec()) for _, v := range lo { base.OpeningBurst += v diff --git a/hello.go b/hello.go index 11da5e0..2342eb2 100644 --- a/hello.go +++ b/hello.go @@ -223,3 +223,17 @@ func (h *ClientHello) SetSNI(name string) error { e.Data = append(d, name...) return nil } + +// dropExtension removes every instance of an extension type, and reports +// whether anything was removed. +func (h *ClientHello) dropExtension(t uint16) bool { + out := h.Extensions[:0] + for _, e := range h.Extensions { + if e.Type != t { + out = append(out, e) + } + } + removed := len(out) != len(h.Extensions) + h.Extensions = out + return removed +} diff --git a/serverhello.go b/serverhello.go index 9d41e36..0cb1523 100644 --- a/serverhello.go +++ b/serverhello.go @@ -58,6 +58,11 @@ type ServerHelloParams struct { // PSKFirst places pre_shared_key before the other extensions, as google and // cloudflare do. Should be stable for a given cover identity. PSKFirst bool + // FullHandshake omits pre_shared_key entirely, which is what a server + // answering a full handshake does. That extension is exactly 6 bytes here + // -- type, length, selected_identity -- which is the whole difference + // between the two measured ServerHello lengths. + FullHandshake bool } // ServerHelloResumedLen is what every measured server produced for a resumed @@ -93,9 +98,11 @@ func SynthesizeServerHello(p ServerHelloParams) ([]byte, error) { copy(share[mlkem768CiphertextLen:], p.ServerEphemeral.Bytes()) var psk []byte - psk = appendU16(psk, ExtPreSharedKey) - psk = appendU16(psk, 2) - psk = appendU16(psk, p.SelectedIdentity) + if !p.FullHandshake { + psk = appendU16(psk, ExtPreSharedKey) + psk = appendU16(psk, 2) + psk = appendU16(psk, p.SelectedIdentity) + } var rest []byte rest = appendU16(rest, 0x002b) // supported_versions diff --git a/twiddle.go b/twiddle.go index 3702943..2c587a2 100644 --- a/twiddle.go +++ b/twiddle.go @@ -392,8 +392,13 @@ type Options struct { // a server's ticket format does not vary connection to connection. TicketLen int // BinderLen must equal the hash length of the cipher suite the synthesised - // ServerHello selects: 32 for SHA-256, 48 for SHA-384. + // ServerHello selects: 32 for SHA-256, 48 for SHA-384. Ignored when + // FullHandshake is set, which has no binder. BinderLen int + // FullHandshake emits a FULL-handshake opening: no pre_shared_key, the + // ticket in the ECH payload and the MAC in random. See echcarrier.go and + // docs/full-handshake-carrier.md. + FullHandshake bool } // Twiddle rewrites a harvested ClientHello for emission and returns the wire @@ -409,6 +414,13 @@ type Options struct { // the transcript and silently invalidates the binder. Real TLS has the same // constraint: a client picks its extension order first and computes the binder // last. +// +// The full-handshake variant substitutes SetECHTicketAuth for the final step +// and is bound by the same rule for the same reason. It is bound MORE tightly, +// in fact: its MAC covers the whole hello rather than a truncation of it, and +// Rerandomize overwrites both fields it uses -- random directly, and the ECH +// payload through rerandECHGrease -- so it must follow Rerandomize and not +// merely SetKeyShare. func Twiddle(harvested []byte, opt Options) (wire []byte, eph *ecdh.PrivateKey, err error) { h, err := ParseClientHello(harvested) if err != nil { @@ -429,6 +441,28 @@ func Twiddle(harvested []byte, opt Options) (wire []byte, eph *ecdh.PrivateKey, if err != nil { return nil, nil, err } + if opt.FullHandshake { + // Harvested hellos routinely carry pre_shared_key -- they are captured + // from real browsing, which resumes -- so the template must be stripped + // rather than trusted. LoadPool's Sanitize already drops them, but a + // caller reading a raw capture does not go through it, and the resumed + // path is equally forgiving: setPSK removes any existing extension + // before appending its own. + // + // The emitted hello is then shorter than the hello it came from by + // exactly the pre_shared_key extension, which is precisely the + // difference between a real Chrome resumption hello and a real Chrome + // full one. + h.dropExtension(ExtPreSharedKey) + if len(opt.Credential.FullTicket) != FullTicketLen { + return nil, nil, fmt.Errorf("twiddle: credential carries a %d-byte full ticket, want %d; it cannot open a full handshake", + len(opt.Credential.FullTicket), FullTicketLen) + } + if err := h.SetECHTicketAuth(opt.Credential.FullTicket, opt.Credential.PSK[:]); err != nil { + return nil, nil, err + } + return h.Marshal(), eph, nil + } if err := h.SetTicketAuth(opt.Credential, opt.BinderLen); err != nil { return nil, nil, err } From 1c484dc7c1a07da61ff1277a0f864de8c4056d84 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 09:35:55 +0100 Subject: [PATCH 07/18] Draw only from pool hellos that can carry a full-handshake ticket Client picked uniformly from the whole pool, so on the full path a hello whose ECH payload is too small -- or absent -- failed the connection. A pool is not uniform: a device tap copies whatever the browser emitted, so hellos with a 240-byte payload sit alongside ones with 144, ones with less, and ones with no ECH at all. The failure therefore depended on the draw, which would present as a flaky connection rather than a configuration that cannot support the shape. FullHandshakeCarriers filters the pool, Client draws from the filtered set, and an empty set fails once with a clear message instead of intermittently. It is exported because a caller deciding whether to offer the full handshake at all needs the same answer -- and because docs/ech.md keeps a non-ECH pool as a deliberate escape hatch, so "this pool cannot carry it" is an expected state, not an error condition. The regression test runs twelve connections against a pool holding one carrier among six hellos; with the unrestricted draw restored it fails, as does the no-carrier case, which is what says the filter is load-bearing rather than decorative. Co-Authored-By: Claude Opus 5 --- echcarrier.go | 26 ++++++++++++++++ echcarrier_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++ handshake.go | 14 +++++++-- handshake_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 2 deletions(-) diff --git a/echcarrier.go b/echcarrier.go index 16f60da..ebed2ce 100644 --- a/echcarrier.go +++ b/echcarrier.go @@ -117,6 +117,32 @@ func (h *ClientHello) ECHPayloadLen() (int, error) { return len(pay), nil } +// FullHandshakeCarriers filters a pool to the hellos whose ECH payload can hold +// a full-handshake ticket. +// +// It exists because a pool is not uniform. Device taps copy whatever the +// browser emitted, so a pool can mix hellos with a 240-byte ECH payload, a +// 144-byte one, and none at all -- and a client drawing uniformly from that +// pool would fail on some connections and succeed on others, which is a far +// worse failure than not offering the path. Callers deciding whether to offer +// the full handshake at all should check this is non-empty. +// +// Unparseable records are skipped rather than reported: the pool loader has +// already rejected those, and a caller reaching here wants the usable subset. +func FullHandshakeCarriers(pool [][]byte) [][]byte { + var out [][]byte + for _, rec := range pool { + h, err := ParseClientHello(rec) + if err != nil { + continue + } + if n, err := h.ECHPayloadLen(); err == nil && n >= FullTicketLen { + out = append(out, rec) + } + } + return out +} + // SetECHTicketAuth installs a full-handshake authenticator: the ticket goes in // the ECH payload, and the MAC over the finished hello goes in random. // diff --git a/echcarrier_test.go b/echcarrier_test.go index feba280..66f69fe 100644 --- a/echcarrier_test.go +++ b/echcarrier_test.go @@ -351,3 +351,79 @@ func TestECHCarrierEmitsAFullHandshakeShape(t *testing.T) { t.Errorf("only saw payload lengths %v over 200 emissions; the length is not varying", seen) } } + +// shrinkECH rewrites a hello's ECH payload to n bytes, standing in for a pool +// hello from a browser whose ECH is too small to carry a ticket. +func shrinkECH(t *testing.T, rec []byte, n int) []byte { + t.Helper() + h, err := ParseClientHello(rec) + if err != nil { + t.Fatal(err) + } + e := h.Find(ExtECH) + if e == nil { + t.Fatal("hello has no ECH to shrink") + } + pay, err := echPayload(e) + if err != nil { + t.Fatal(err) + } + head := len(e.Data) - len(pay) - 2 + d := append([]byte(nil), e.Data[:head]...) + d = append(d, byte(n>>8), byte(n)) + d = append(d, make([]byte, n)...) + e.Data = d + if got, err := h.ECHPayloadLen(); err != nil || got != n { + t.Fatalf("shrink produced %d (%v), want %d", got, err, n) + } + return h.Marshal() +} + +// stripECH removes the ECH extension entirely, standing in for the non-ECH +// pool docs/ech.md keeps as an escape hatch. +func stripECH(t *testing.T, rec []byte) []byte { + t.Helper() + h, err := ParseClientHello(rec) + if err != nil { + t.Fatal(err) + } + if !h.dropExtension(ExtECH) { + t.Fatal("hello had no ECH to strip") + } + return h.Marshal() +} + +// A pool is not uniform: a device tap copies whatever the browser emitted, so +// hellos that can carry a ticket sit alongside hellos that cannot. Drawing +// uniformly from the whole pool would fail on SOME connections and succeed on +// others -- an intermittent failure far worse than not offering the path. +func TestFullHandshakeCarriersFiltersThePool(t *testing.T) { + base := DefaultPool()[0] // 240-byte payload + good := helloWithECHPayload(t, 144) + + mixed := [][]byte{ + stripECH(t, base), + shrinkECH(t, base, FullTicketLen-1), + base, + shrinkECH(t, base, 16), + good.Marshal(), + []byte("not a hello at all"), + } + got := FullHandshakeCarriers(mixed) + if len(got) != 2 { + t.Fatalf("kept %d of 6 hellos, want the 2 with a large enough ECH payload", len(got)) + } + for _, rec := range got { + h, err := ParseClientHello(rec) + if err != nil { + t.Fatal(err) + } + n, err := h.ECHPayloadLen() + if err != nil || n < FullTicketLen { + t.Errorf("kept a hello with payload %d (%v)", n, err) + } + } + if len(FullHandshakeCarriers([][]byte{stripECH(t, base)})) != 0 { + t.Error("a pool with no usable ECH was reported as able to carry the full handshake") + } +} diff --git a/handshake.go b/handshake.go index a70b336..c56c3b6 100644 --- a/handshake.go +++ b/handshake.go @@ -86,12 +86,22 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { if cfg.FullHandshake { remainder = cfg.Cover.FullRemainder } - pick, err := rand.Int(rand.Reader, bigLen(len(cfg.Pool))) + // The full path can only use hellos whose ECH payload holds a ticket, and a + // pool is not uniform -- a device tap copies whatever the browser emitted. + // Drawing from the whole pool would fail on some connections and succeed on + // others, depending on the draw. + candidates := cfg.Pool + if cfg.FullHandshake { + if candidates = FullHandshakeCarriers(cfg.Pool); len(candidates) == 0 { + return nil, nil, errors.New("twiddle: no hello in the pool has an ECH payload large enough to carry a full-handshake ticket") + } + } + pick, err := rand.Int(rand.Reader, bigLen(len(candidates))) if err != nil { return nil, nil, err } - wire, eph, err := Twiddle(cfg.Pool[pick.Int64()], Options{ + wire, eph, err := Twiddle(candidates[pick.Int64()], Options{ CoverSNI: cfg.Cover.Host, Credential: cfg.Credential, BinderLen: cfg.Cover.BinderLen, diff --git a/handshake_test.go b/handshake_test.go index 7358b80..e9214ed 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -705,3 +705,75 @@ func TestFullHandshakeRefusesWhatItCannotBack(t *testing.T) { } }) } + +// The regression the pool filter exists for. With one carrier among many +// hellos that cannot carry a ticket, a client drawing uniformly succeeds only +// about a sixth of the time; the failure depends on the draw, so it would +// present as a flaky connection rather than a broken configuration. +func TestClientAlwaysPicksACarrierFromAMixedPool(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + base := DefaultPool()[0] + mixed := [][]byte{ + stripECH(t, base), + shrinkECH(t, base, FullTicketLen-1), + shrinkECH(t, base, 16), + shrinkECH(t, base, 100), + stripECH(t, base), + base, // the only carrier + } + + for i := 0; i < 12; i++ { + cred, err := k.Issue(uint64(200+i), cover.TicketLen) + if err != nil { + t.Fatal(err) + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + go func() { + c, err := ln.Accept() + if err != nil { + return + } + defer c.Close() + c.SetDeadline(time.Now().Add(5 * time.Second)) + Server(c, ServerConfig{ + TicketKey: k, Cover: cover, + MaxAge: time.Hour, Replay: NewReplayCache(16, time.Hour), + }) + }() + raw, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + ln.Close() + t.Fatal(err) + } + _, _, err = Client(raw, ClientConfig{ + Pool: mixed, Cover: cover, Credential: cred, FullHandshake: true, + }) + raw.Close() + ln.Close() + if err != nil { + t.Fatalf("attempt %d of 12 failed: %v -- the pool draw is not restricted to carriers", i+1, err) + } + } +} + +// And a pool with no carrier at all must fail clearly, not on the draw. +func TestFullHandshakeWithNoCarrierInThePoolFailsClearly(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + cred, _ := k.Issue(210, cover.TicketLen) + none := [][]byte{stripECH(t, DefaultPool()[0]), shrinkECH(t, DefaultPool()[0], 32)} + + _, _, err := Client(nil, ClientConfig{ + Pool: none, Cover: cover, Credential: cred, FullHandshake: true, + }) + if err == nil { + t.Fatal("a full handshake was attempted from a pool with no carrier") + } + if !contains(err.Error(), "ECH payload") { + t.Errorf("unhelpful error: %v", err) + } +} From 6719631d41d881404e2c114c5ed1951848ebd1a0 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 09:36:51 +0100 Subject: [PATCH 08/18] docs: record what the carrier work built, and what is left The document was written as a plan and is now a record of a plan that was carried out, so the status, the implementation sketch and the open questions were all describing work that has since landed. Replaces the sketch with a table of what was built and where, folds the open questions into a "What remains" list, and renames the groundwork section so it does not read as a rival to the new one. Keeps the three decisions that look like omissions until the reason is stated: the server not bucket-checking the ECH payload length, Twiddle stripping the harvested pre_shared_key rather than demanding a clean template, and both tickets of a credential being sealed at one instant. What remains is now explicit: the re-full cadence, which is the only real twiddle-side question left; provisioning the companion ticket through lantern-cloud #3291 and lantern-box; a probed full profile per egress, which needs the same startup-probe plumbing the resumed profile does; and sessionTicketWire, which was already wrong and is not made worse. Co-Authored-By: Claude Opus 5 --- docs/full-handshake-carrier.md | 98 +++++++++++++++++++++------------- 1 file changed, 61 insertions(+), 37 deletions(-) diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md index d377328..ffab447 100644 --- a/docs/full-handshake-carrier.md +++ b/docs/full-handshake-carrier.md @@ -1,7 +1,8 @@ # The full-handshake carrier -**Status:** designed, not built. Everything below the "What already exists" line is groundwork that -landed in #1; the carrier itself is unstarted. +**Status:** built and green in twiddle; not yet provisioned or enabled downstream. The carrier, both +handshake paths, credential rotation and the pool filter all landed on `fisk/full-handshake-carrier`. +What is left is the mix policy and the cross-repo provisioning — see "What remains". ## The problem, measured @@ -49,7 +50,7 @@ So amend, do not overturn: That reframing sets the bar, and it is much lower than 4%: we do not need to match the wild distribution. We need first contact to be a full handshake, and resumption afterwards to be what it is everywhere else — -the continuation of an observed relationship. See "Mix policy" below; that is why it needs no dice roll. +the continuation of an observed relationship. See "What remains"; that is why it needs no dice roll. **The sharper form of the problem.** You cannot resume a session that was never established. An observer with flow history sees a `pre_shared_key` hello to an IP it never saw that client complete a @@ -61,9 +62,9 @@ CDNs share tickets across IPs, so a censor with finite history gets false positi signal, not a proof — and note it is exactly the signal the first-contact-full policy erases, which is the argument for building this at all. -## What already exists +## Groundwork from #1 -Landed in #1, all of it prerequisite: +Prerequisite, and all of it already on main before this work: - `ServerHelloFullLen = 1215` beside `ServerHelloResumedLen = 1221` (`serverhello.go`). The 6-byte delta is `pre_shared_key`. @@ -221,38 +222,61 @@ Costs, and why it is second choice: psk), but a later compromise of the static key retroactively reveals the `clientID` in past openings. The ECH carrier has no equivalent exposure. -## Open questions - -1. **Re-full cadence.** See "Mix policy" reasoning above: the censor's flow history is finite and the - client's context changes, so some trigger — new local address, new egress IP, elapsed time — has to - force a fresh full handshake rather than resuming forever off one observed predecessor. -2. **Where credential rotation lives.** It currently rides a NewSessionTicket-shaped record, but cloudflare - and google send **no** unprompted post-handshake records at all, so on those covers that record has no - counterpart. Unchanged by this work, but it lands in the same code. -3. **Full-path ticket length.** 144 bytes is proposed so every ECH bucket stays reachable. Confirm - `MinTicketLen` (76) leaves enough padding entropy, and decide whether the server should accept only 144 - or any length that decrypts. - -## Implementation sketch - -1. `SetECHTicketAuth` / `VerifyECHTicketAuth` in `auth.go`, mirroring `SetTicketAuth` / `VerifyTicketAuth`: - write the ticket into the ECH payload padded to the drawn bucket, then HMAC the marshalled hello with - `random` zeroed and write the result into `random`. - **Ordering matters,** the same rule the binder follows: the MAC covers the final byte layout, so it is - computed last. The pipeline becomes `SetSNI → Rerandomize → Shuffle → SetKeyShare → SetECHTicketAuth`. - Note `Rerandomize` overwrites `h.Random` (`twiddle.go:82`) *and* redraws the ECH payload - (`rerandECHGrease`), so both writes must follow it. -2. Branch `validateClientHello` (`cover.go:155`) and `Server` (`handshake.go`) on whether `pre_shared_key` - is present, instead of requiring it. The full path reads the ticket from ECH and verifies the `random` - MAC; everything downstream — `TicketKey.Open`, `ReplayCache.Consume`, `DeriveSession` — is unchanged. -3. Extend `CanEmitFullHandshake()` to also require an ECH extension whose payload can hold the ticket. A - pool without ECH falls back to resumption; say why in the comment (see "The objection" above). -4. Server emission: `ServerHelloFullLen`, then CCS, then one record per `FullRemainder` entry, jittered - within `FullRemainderJitter`. Client: one read per entry. -5. Extend `cover_test.go`'s oracle. It cannot pin `FullRemainder` to a literal (probed, and it jitters); - pin the *structure* — ServerHello length, record count, plausible range. -6. Mix policy: first contact to an egress is full, later connections resume, plus the re-full trigger from - open question 1. +## What was built + +All of it mutation-tested — every guarantee below has a deliberate break that fails a test. + +| Piece | Where | +|---|---| +| `SetECHTicketAuth` / `VerifyECHTicketAuth`, `FullTicketLen = 144`, `echPayload`, `ECHPayloadLen` | `echcarrier.go` | +| `Credential.FullTicket`, `IssueFullFor`, both tickets sealed at one instant | `auth.go` | +| `CredentialFromWireFull` (additive; `CredentialFromWire` stays resumption-only) | `pool.go` | +| `ServerHelloParams.FullHandshake` — omits `pre_shared_key`, the exact 6-byte delta | `serverhello.go` | +| `validateClientHello` split; `validateFullClientHello`; `DrawFullRemainder`; `Adopt` carries jitter | `cover.go` | +| `Options.FullHandshake`, `pre_shared_key` stripped from the template | `twiddle.go` | +| `ClientConfig.FullHandshake`, server dispatch on PSK presence, jittered emission, two-record rotation | `handshake.go` | +| `FullHandshakeCarriers` — the pool is not uniform, so the draw must be restricted | `echcarrier.go` | +| `ProbeResult.RemainderJitter`, filled by `SampleFull` | `cover.go`, `harvest/coverprobe` | + +Measured end to end against the microsoft profile: **ServerHello 1215, ccs 6, remainder +`[32 8273 286 74]`** — the shape from `postflight-full-vs-resumed.log`, with the record *count* right, +which is what an observer counts. + +Three decisions worth knowing, because each looks like an omission until you see why: + +- **The server does not check that the ECH payload length is one of Chrome's buckets.** That is a property + of what we *emit*, asserted on the emission side. A server refusing anything else would break the first + device-tapped pool from a Chrome whose buckets differ from the ones we measured, and strictness there + buys nothing against a censor, who sees the client's hello and not our validation of it. +- **`Twiddle` strips `pre_shared_key` rather than requiring a clean template.** Harvested hellos routinely + carry one — they come from real browsing, which resumes, and the hellos in `harvest/testdata` do. The + emitted hello is then shorter than its source by exactly that extension, which is precisely the + difference between a real Chrome resumption hello and a real Chrome full one. +- **Both tickets of a credential are sealed at the same instant.** `ReplayCache` refuses a ticket older + than the client's newest, so a one-second skew between them would make whichever path the client used + *second* look like a stale capture. That failure is invisible to a test of either path alone. + +## What remains + +1. **Mix policy — the last twiddle-side question.** `ClientConfig.FullHandshake` is per-connection, so the + mechanism is in place and the policy is the caller's. What still needs deciding is the **re-full + cadence**: the censor's flow history is finite and the client's context changes, so a reconnection a + week later, from a different network, after the egress IP rotated, has no observable predecessor even + though the ticket is valid. Some trigger — new local address, new egress IP, elapsed time — has to force + a fresh full handshake. Note the resulting ratio will sit *far above* 4% (a long-lived muxed tunnel + opens few outer connections, so one full per handful of resumed), and that is the correct outcome: what + a censor can check is whether the predecessor exists, not whether we hit a population average. +2. **Provisioning the companion ticket.** lantern-cloud's `GenerateTwiddle` (PR #3291, draft) must emit + `full_ticket` alongside `ticket` and `psk`, and lantern-box must pass it to `CredentialFromWireFull`. + Until then a provisioned client is resumption-only, which degrades to today's behaviour rather than + failing. `cmd/twiddlecred` already prints it. +3. **A probed full profile per egress.** `CanEmitFullHandshake()` gates both ends on `FullRemainder`, which + the table ships empty on purpose, so nothing offers the path until `coverprobe.SampleFull` has run + against the live upstream and `Adopt` has taken the result. The startup-probe plumbing is the same work + the resumed profile needs. +4. **`sessionTicketWire = 370` is still wrong for all three covers** (microsoft was measured at 303, + cloudflare and google issue none unprompted). Pre-existing; rotation now sends two records, which is + closer to microsoft's measured pair, but the size itself is untouched. ## Traps worth knowing before starting From 38c32c0fb6abac1419a4a77f7f44452853dfb72c Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 09:52:56 +0100 Subject: [PATCH 09/18] Remember contacted egresses, and open the first connection to each in full This is the mix policy the carrier was missing. The rule is not "match the 4% resumption share measured in real browsing" -- it is narrower and much cheaper to satisfy: a censor watching this client and this egress must already have seen the FULL HANDSHAKE that a resumption continues. A client with a long relationship to one host and a stack of its tickets is an ordinary pattern. What no real client does is reach a host for the first time already holding a ticket, and never once complete a full handshake with it. So ContactMemory remembers address pairs: full on first contact with an egress, resumed afterwards, full again past a horizon beyond which the censor can no longer be assumed to remember. Six hours by default -- the true retention is unknowable, so it errs short, and being wrong in this direction costs one extra full handshake per egress per six hours. Every uncertainty resolves toward full, deliberately. A forgotten entry, an evicted one, a restarted process, a changed local address, an expired horizon -- each produces an extra full handshake, which costs 5-10 KB and looks MORE normal rather than less, since 95%+ of real connections are full handshakes. The opposite mistake is the distinguisher this exists to remove. That asymmetry is what makes the simple entry bound sound, and it is the REVERSE of ReplayCache's situation, where evicting a live entry reopens the window the gate exists to close. The key includes the local address, because a censor's history is tied to a vantage point: a client that moves networks is watched by somebody who never saw the earlier handshake. Behind NAT that is a weak proxy -- two networks can both hand out 192.168.1.5 -- so Reset is the reliable signal for callers that can detect a network change, and the local address catches the cheap cases on its own. Ports are dropped: they change every connection and cannot be part of a relationship correlated by address pair. It lives in ClientConfig rather than the caller so neither half can be forgotten. Failing to consult it emits resumptions; failing to record a completed handshake emits extra full ones. Only the first direction hurts, but both are avoided by keeping the decision here. A Contacts-driven choice DEGRADES to resumption when the cover has no probed profile or the pool cannot carry the ticket, where an explicit FullHandshake still fails: the caller asked for the right shape, not for the connection to be refused, and refusing would make enabling Contacts depend on every cover having been probed first. The degradation is not recorded, so it retries rather than latching. Conn.FullHandshake exposes which shape was used, on both ends. Without it a memory that silently degraded on every connection, because no cover was ever probed, would look exactly like one that was working. Ten mutations were tried. Eight fail a test outright: an inert policy, a key ignoring the local address, a key keeping the port, recording on attempt rather than completion, removing the entry bound, recording the degradation, a constant Conn.FullHandshake, and eviction that stops dropping stale entries. The remaining two are the horizon check and the horizon eviction, which enforce the same bound in two places -- neither is load-bearing alone. That redundancy is now stated in the code rather than left to be discovered, and eviction has its own test so the state bound is covered even though the answer is not uniquely attributable. Co-Authored-By: Claude Opus 5 --- conn.go | 15 ++++ contacts.go | 191 +++++++++++++++++++++++++++++++++++++++++ contacts_test.go | 172 +++++++++++++++++++++++++++++++++++++ handshake.go | 56 ++++++++++-- handshake_test.go | 214 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 639 insertions(+), 9 deletions(-) create mode 100644 contacts.go create mode 100644 contacts_test.go diff --git a/conn.go b/conn.go index 0816eb1..8b51e57 100644 --- a/conn.go +++ b/conn.go @@ -133,8 +133,23 @@ type Conn struct { recvSeq uint64 pending []byte rerr error + + // fullHandshake records which opening shape this connection used. Set once + // by Client or Server before the connection is handed out, and read-only + // after, so it needs no lock. + fullHandshake bool } +// FullHandshake reports whether this connection opened with a full handshake +// rather than a resumption. +// +// Exposed for measurement. The point of the full-handshake carrier is to stop +// emitting 100% resumptions (see docs/full-handshake-carrier.md), and the only +// way to know the deployed mix is to count it -- a ContactMemory that silently +// degraded on every connection, because no cover was ever probed, would +// otherwise look exactly like one that was working. +func (c *Conn) FullHandshake() bool { return c.fullHandshake } + // NewConn wraps raw. isClient selects which direction's keys are used to send. func NewConn(raw net.Conn, s *Session, isClient bool, sh Shaper) (*Conn, error) { sendKeys, recvKeys := s.Client, s.Server diff --git a/contacts.go b/contacts.go new file mode 100644 index 0000000..f143c60 --- /dev/null +++ b/contacts.go @@ -0,0 +1,191 @@ +package twiddle + +import ( + "net" + "sync" + "time" +) + +// ContactMemory decides, per connection, whether the opening should be a full +// handshake or a resumption. +// +// The rule it implements is not "match the 4% resumption share measured in real +// browsing" -- see docs/full-handshake-carrier.md. It is narrower and much +// cheaper to satisfy: a censor watching this client and this egress must have +// already seen the FULL HANDSHAKE that a resumption continues. A client with a +// long relationship to one host and a stack of its tickets is an ordinary +// pattern. What no real client does is reach a host for the first time already +// holding a ticket, and never once complete a full handshake with it. +// +// So: full on first contact with an egress, resumed afterwards, and full again +// once the censor can no longer be assumed to remember. +// +// EVERY UNCERTAINTY RESOLVES TOWARD FULL. A forgotten entry, an evicted one, a +// restarted process, a changed local address, an expired horizon -- each +// produces an extra full handshake, which costs 5-10 KB and looks MORE normal +// rather than less, because 95%+ of real connections are full handshakes. The +// opposite mistake, a resumption with no observable predecessor, is the +// distinguisher this exists to remove. That asymmetry is why the eviction below +// is sound, and it is the reverse of ReplayCache's situation, where evicting a +// live entry reopens the window the gate exists to close. +type ContactMemory struct { + mu sync.Mutex + horizon time.Duration + max int + seen map[contactKey]time.Time +} + +// contactKey pairs the egress address with the local one. +// +// The local address is included because a censor's history is tied to a vantage +// point: a client that moves from one network to another is being watched by +// somebody who never saw the earlier full handshake, so the relationship has to +// be re-established. It is a WEAK proxy -- behind NAT it is a private address, +// and two different networks can both hand out 192.168.1.5, in which case the +// move goes unnoticed. Callers that can detect a network change should call +// Reset, which is the reliable signal; this catches the cheap cases on its own. +type contactKey struct { + local string + remote string +} + +const ( + // DefaultContactHorizon is how long a full handshake is assumed to still be + // in a censor's flow history. + // + // The true value is unknowable, so this errs short. Flow-record retention + // is commonly days, so six hours sits well inside it, and the cost of being + // wrong in this direction is one extra full handshake per egress per six + // hours -- tens of kilobytes a day. Erring long risks emitting exactly the + // resumption-without-predecessor this is meant to prevent. + DefaultContactHorizon = 6 * time.Hour + + // defaultContactMax bounds the map. A client contacts tens of egresses, not + // thousands, so this is a backstop against a leak rather than a working + // limit. + defaultContactMax = 1024 +) + +// NewContactMemory returns a memory with the given horizon and entry bound. +// Zero or negative selects the defaults. +func NewContactMemory(horizon time.Duration, max int) *ContactMemory { + if horizon <= 0 { + horizon = DefaultContactHorizon + } + if max <= 0 { + max = defaultContactMax + } + return &ContactMemory{ + horizon: horizon, + max: max, + seen: make(map[contactKey]time.Time), + } +} + +// Horizon reports how long a recorded full handshake is trusted for. +func (m *ContactMemory) Horizon() time.Duration { + if m == nil { + return 0 + } + return m.horizon +} + +// Reset forgets every contact, so the next connection to each egress opens with +// a full handshake. +// +// Callers should call this when the platform reports a network change -- a new +// interface, a new default route, a VPN coming up or down. That is the reliable +// version of what contactKey's local address approximates: after such a change +// the observer is potentially a different one, with no history of anything this +// client did before. +func (m *ContactMemory) Reset() { + if m == nil { + return + } + m.mu.Lock() + defer m.mu.Unlock() + m.seen = make(map[contactKey]time.Time) +} + +// Tracked reports how many contacts are remembered. +func (m *ContactMemory) Tracked() int { + if m == nil { + return 0 + } + m.mu.Lock() + defer m.mu.Unlock() + return len(m.seen) +} + +func addrKey(a net.Addr) string { + if a == nil { + return "" + } + // Ports are deliberately dropped. A censor correlating a resumption with + // the full handshake it continues does so by address pair; the source port + // changes on every connection and cannot be part of the relationship. + if host, _, err := net.SplitHostPort(a.String()); err == nil { + return host + } + return a.String() +} + +// needsFull reports whether this connection should open with a full handshake. +// +// Two concurrent connections to the same new egress will both be told yes, and +// both will do a full handshake. That is not a race worth closing: it is what a +// browser does on every page load, where a parallel burst opens several +// connections to one origin before any of them has a ticket to resume with. +func (m *ContactMemory) needsFull(local, remote net.Addr, now time.Time) bool { + if m == nil { + return false + } + m.mu.Lock() + defer m.mu.Unlock() + m.evictLocked(now) + last, ok := m.seen[contactKey{addrKey(local), addrKey(remote)}] + // The horizon is enforced TWICE, here and in evictLocked, and neither is + // load-bearing alone -- a mutation removing either one keeps every test + // green. That is deliberate: eviction bounds the state and this comparison + // bounds the answer, so making eviction periodic or lazy later cannot + // silently extend how long a relationship is trusted for. + return !ok || now.Sub(last) >= m.horizon +} + +// record notes a COMPLETED full handshake. Only completed ones count: a +// handshake that failed established no relationship for a later resumption to +// continue. +func (m *ContactMemory) record(local, remote net.Addr, now time.Time) { + if m == nil { + return + } + m.mu.Lock() + defer m.mu.Unlock() + m.seen[contactKey{addrKey(local), addrKey(remote)}] = now + m.evictLocked(now) +} + +// evictLocked drops what is past the horizon, then enforces the entry bound by +// dropping the oldest. +// +// Dropping a live entry is safe here, unlike in ReplayCache, because the only +// consequence is one extra full handshake -- the safe direction. That is what +// lets this use a simple bound where the replay gate needed a redesign. +func (m *ContactMemory) evictLocked(now time.Time) { + for k, t := range m.seen { + if now.Sub(t) >= m.horizon { + delete(m.seen, k) + } + } + for len(m.seen) > m.max { + var oldestKey contactKey + var oldest time.Time + first := true + for k, t := range m.seen { + if first || t.Before(oldest) { + oldestKey, oldest, first = k, t, false + } + } + delete(m.seen, oldestKey) + } +} diff --git a/contacts_test.go b/contacts_test.go new file mode 100644 index 0000000..1678ee5 --- /dev/null +++ b/contacts_test.go @@ -0,0 +1,172 @@ +package twiddle + +import ( + "net" + "testing" + "time" +) + +// addr is a stand-in net.Addr so the unit tests can drive addresses directly. +type addr string + +func (a addr) Network() string { return "tcp" } +func (a addr) String() string { return string(a) } + +// The rule, in one test: full on first contact, resumed afterwards. +func TestContactMemoryIsFullOnFirstContactAndResumedAfter(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + now := time.Now() + local, remote := addr("10.0.0.2:51000"), addr("203.0.113.9:443") + + if !m.needsFull(local, remote, now) { + t.Fatal("first contact with an egress did not ask for a full handshake") + } + // Asking is not recording: until the handshake completes, the relationship + // does not exist and the answer must not change. + if !m.needsFull(local, remote, now) { + t.Error("the answer changed before any handshake was recorded") + } + + m.record(local, remote, now) + if m.needsFull(local, remote, now.Add(time.Minute)) { + t.Error("a second connection to a recorded egress asked for another full handshake") + } +} + +// The source port changes on every connection and cannot be part of a +// relationship a censor correlates by address pair. +func TestContactMemoryIgnoresPorts(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + now := time.Now() + m.record(addr("10.0.0.2:51000"), addr("203.0.113.9:443"), now) + + if m.needsFull(addr("10.0.0.2:52222"), addr("203.0.113.9:443"), now) { + t.Error("a new source port was treated as a new contact") + } +} + +// Past the horizon the censor can no longer be assumed to remember, so the +// relationship has to be re-established. +func TestContactMemoryReFullsPastTheHorizon(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + base := time.Now() + local, remote := addr("10.0.0.2:51000"), addr("203.0.113.9:443") + m.record(local, remote, base) + + if m.needsFull(local, remote, base.Add(59*time.Minute)) { + t.Error("re-fulled inside the horizon") + } + if !m.needsFull(local, remote, base.Add(time.Hour)) { + t.Error("did not re-full at the horizon") + } + if !m.needsFull(local, remote, base.Add(3*time.Hour)) { + t.Error("did not re-full past the horizon") + } +} + +// The horizon is enforced in two places -- the answer and the eviction -- and +// the test above cannot tell them apart, because removing either one leaves it +// green. This one covers eviction specifically: state past the horizon has to +// be dropped, or a long-running client accumulates one entry per egress it ever +// contacted and the bound becomes the only thing holding the map down. +func TestContactMemoryDropsStatePastTheHorizon(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + base := time.Now() + for i := 1; i <= 20; i++ { + m.record(addr("10.0.0.2:1"), addr(net.JoinHostPort( + net.IPv4(203, 0, 113, byte(i)).String(), "443")), base) + } + if m.Tracked() != 20 { + t.Fatalf("tracking %d contacts, want 20", m.Tracked()) + } + + // Any later call runs eviction, and every entry is now stale. + m.needsFull(addr("10.0.0.2:1"), addr("198.51.100.1:443"), base.Add(2*time.Hour)) + if got := m.Tracked(); got != 0 { + t.Errorf("tracking %d contacts after the horizon passed, want 0", got) + } +} + +// A different egress, and a different local address, are both new contacts. The +// second is the roaming case: a censor at the new vantage point never saw the +// earlier full handshake. +func TestContactMemoryKeysOnBothEnds(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + now := time.Now() + m.record(addr("10.0.0.2:51000"), addr("203.0.113.9:443"), now) + + if !m.needsFull(addr("10.0.0.2:51000"), addr("198.51.100.7:443"), now) { + t.Error("a different egress was treated as already contacted") + } + if !m.needsFull(addr("192.168.5.4:51000"), addr("203.0.113.9:443"), now) { + t.Error("a new local address was treated as already contacted; roaming would emit a bare resumption") + } +} + +// Reset is the reliable version of the local-address heuristic, for callers +// that can see a network change the local address does not reveal -- the same +// private address handed out by two different networks. +func TestContactMemoryResetForcesFullAgain(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + now := time.Now() + local, remote := addr("192.168.1.5:51000"), addr("203.0.113.9:443") + m.record(local, remote, now) + if m.needsFull(local, remote, now) { + t.Fatal("recorded contact still asked for a full handshake") + } + + m.Reset() + if m.Tracked() != 0 { + t.Errorf("Reset left %d contacts", m.Tracked()) + } + if !m.needsFull(local, remote, now) { + t.Error("after Reset the same address pair did not ask for a full handshake") + } +} + +// The bound exists so the map cannot leak, and evicting a LIVE entry is sound +// here precisely because the consequence is one extra full handshake. That is +// the reverse of ReplayCache, where evicting a live entry reopens the window +// the gate exists to close. +func TestContactMemoryEvictionFailsTowardFull(t *testing.T) { + const max = 32 + m := NewContactMemory(time.Hour, max) + base := time.Now() + + first := addr("203.0.113.1:443") + m.record(addr("10.0.0.2:1"), first, base) + + for i := 0; i < max*4; i++ { + m.record(addr("10.0.0.2:1"), addr(net.JoinHostPort( + net.IPv4(198, 51, 100, byte(i%250+1)).String(), "443")), base.Add(time.Duration(i+1)*time.Second)) + } + if got := m.Tracked(); got > max { + t.Errorf("tracking %d contacts, above the %d bound", got, max) + } + // The oldest entry is gone, and its absence asks for a full handshake -- + // the safe direction, not a reopened hole. + if !m.needsFull(addr("10.0.0.2:1"), first, base.Add(time.Minute)) { + t.Error("an evicted contact was still treated as already contacted") + } +} + +// A nil memory is the documented default and must behave as today: never ask +// for a full handshake, and never panic on record. +func TestNilContactMemoryIsInert(t *testing.T) { + var m *ContactMemory + if m.needsFull(addr("a:1"), addr("b:2"), time.Now()) { + t.Error("a nil memory asked for a full handshake") + } + m.record(addr("a:1"), addr("b:2"), time.Now()) // must not panic + m.Reset() + if m.Tracked() != 0 || m.Horizon() != 0 { + t.Error("a nil memory reported state") + } +} + +func TestContactMemoryDefaults(t *testing.T) { + m := NewContactMemory(0, 0) + if m.Horizon() != DefaultContactHorizon { + t.Errorf("horizon %v, want the default %v", m.Horizon(), DefaultContactHorizon) + } +} diff --git a/handshake.go b/handshake.go index c56c3b6..9bacc0e 100644 --- a/handshake.go +++ b/handshake.go @@ -28,17 +28,34 @@ type ClientConfig struct { // Credential is the ticket and psk to present. Replaced after each // connection with the one the server issues as a post-handshake ticket. Credential *Credential - // FullHandshake opens with a FULL-handshake shape instead of a resumption: - // no pre_shared_key, the ticket in the ECH payload, and a server flight - // carrying a certificate-sized remainder. + // FullHandshake FORCES a full-handshake opening: no pre_shared_key, the + // ticket in the ECH payload, and a server flight carrying a + // certificate-sized remainder. // // It exists because emitting only resumption hellos is itself a // distinguisher -- measured at 4.1% of real browsing -- and because a // resumption to an address the client was never seen completing a full - // handshake with is structurally impossible in real TLS. First contact with - // an egress should use this; see docs/full-handshake-carrier.md. + // handshake with is structurally impossible in real TLS. See + // docs/full-handshake-carrier.md. + // + // Prefer Contacts, which decides per connection. Setting this is a hard + // request: a cover with no measured full profile fails rather than + // degrading, because a caller who asked for the shape explicitly wants to + // know it is unavailable. FullHandshake bool - Shaper Shaper + // Contacts, when set, chooses the shape per connection: full on first + // contact with an egress, resumed afterwards, full again once the censor + // can no longer be assumed to remember. See ContactMemory. + // + // It lives here rather than in the caller so the decision cannot be + // forgotten, and so recording a completed handshake cannot be missed -- + // both of which fail toward emitting resumptions, the direction that hurts. + // + // Nil means today's behaviour: resumption unless FullHandshake is set. That + // is the only workable default, since the cover table ships no full profile + // until one has been probed. + Contacts *ContactMemory + Shaper Shaper } // ServerConfig is what an egress needs to accept one. @@ -79,11 +96,24 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { if cfg.FullHandshake && !cfg.Cover.CanEmitFullHandshake() { return nil, nil, fmt.Errorf("twiddle: cover %s has no measured full-handshake profile", cfg.Cover.Host) } + + // An explicit request is honoured; otherwise the contact memory decides. + // A Contacts-driven choice DEGRADES to resumption when the cover or the + // pool cannot back a full handshake, where an explicit one fails: the + // caller asked for the right shape, not for the connection to be refused, + // and refusing would make enabling Contacts depend on every cover having + // been probed first. The degradation is not recorded, so it keeps trying + // rather than latching. + full := cfg.FullHandshake + if !full && cfg.Contacts.needsFull(raw.LocalAddr(), raw.RemoteAddr(), time.Now()) { + full = cfg.Cover.CanEmitFullHandshake() && len(FullHandshakeCarriers(cfg.Pool)) > 0 + } + // The remainder record COUNT is what the client reads, so the two shapes // are read differently and picking the wrong sequence misaligns every // later read. remainder := cfg.Cover.ResumedRemainder - if cfg.FullHandshake { + if full { remainder = cfg.Cover.FullRemainder } // The full path can only use hellos whose ECH payload holds a ticket, and a @@ -91,7 +121,7 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { // Drawing from the whole pool would fail on some connections and succeed on // others, depending on the draw. candidates := cfg.Pool - if cfg.FullHandshake { + if full { if candidates = FullHandshakeCarriers(cfg.Pool); len(candidates) == 0 { return nil, nil, errors.New("twiddle: no hello in the pool has an ECH payload large enough to carry a full-handshake ticket") } @@ -105,7 +135,7 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { CoverSNI: cfg.Cover.Host, Credential: cfg.Credential, BinderLen: cfg.Cover.BinderLen, - FullHandshake: cfg.FullHandshake, + FullHandshake: full, }) if err != nil { return nil, nil, err @@ -138,6 +168,7 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { if err != nil { return nil, nil, err } + conn.fullHandshake = full // Server EncryptedExtensions+Finished stand-in. One read per record the // cover sends, because the count varies by identity: microsoft splits the @@ -163,6 +194,12 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { if err != nil { return nil, nil, err } + // Recorded only now, with the opening complete. A full handshake that + // failed established no relationship for a later resumption to continue, so + // recording it would be the one direction that hurts. + if full { + cfg.Contacts.record(raw.LocalAddr(), raw.RemoteAddr(), time.Now()) + } return conn, next, nil } @@ -252,6 +289,7 @@ func Server(raw net.Conn, cfg ServerConfig) (*Conn, error) { if err != nil { return nil, err } + conn.fullHandshake = full // One write per record the cover actually sends: microsoft splits the // resumed remainder 32/74 where cloudflare and google coalesce it into one diff --git a/handshake_test.go b/handshake_test.go index e9214ed..a8cc7b6 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -777,3 +777,217 @@ func TestFullHandshakeWithNoCarrierInThePoolFailsClearly(t *testing.T) { t.Errorf("unhelpful error: %v", err) } } + +// dialOnce runs one full client/server exchange and reports the shape each end +// believes it used. Both are returned because a disagreement is the failure +// worth catching: the two ends would then be reading different record counts. +func dialOnce(t *testing.T, k *TicketKey, cover CoverProfile, cfg ClientConfig, replay *ReplayCache) (clientFull, serverFull bool, err error) { + t.Helper() + ln, lerr := net.Listen("tcp", "127.0.0.1:0") + if lerr != nil { + t.Fatal(lerr) + } + defer ln.Close() + + type sres struct { + full bool + err error + } + srvCh := make(chan sres, 1) + go func() { + c, aerr := ln.Accept() + if aerr != nil { + srvCh <- sres{false, aerr} + return + } + c.SetDeadline(time.Now().Add(5 * time.Second)) + sc, serr := Server(c, ServerConfig{ + TicketKey: k, Cover: cover, MaxAge: time.Hour, Replay: replay, + }) + if serr != nil { + srvCh <- sres{false, serr} + return + } + srvCh <- sres{sc.FullHandshake(), nil} + }() + + raw, derr := net.Dial("tcp", ln.Addr().String()) + if derr != nil { + t.Fatal(derr) + } + defer raw.Close() + raw.SetDeadline(time.Now().Add(5 * time.Second)) + cc, _, cerr := Client(raw, cfg) + r := <-srvCh + if cerr != nil { + return false, false, cerr + } + if r.err != nil { + return false, false, r.err + } + return cc.FullHandshake(), r.full, nil +} + +// The mix policy end to end: the first connection to an egress is a full +// handshake, and the next one resumes. That is the whole point -- a resumption +// with no observable predecessor is the distinguisher, and one full handshake +// per egress removes it. +func TestContactsMakeFirstContactFullAndTheNextResumed(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + replay := NewReplayCache(64, time.Hour) + mem := NewContactMemory(time.Hour, 0) + + cred, err := k.Issue(300, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + cfg := ClientConfig{Pool: pool(t), Cover: cover, Credential: cred, Contacts: mem} + + cf, sf, err := dialOnce(t, k, cover, cfg, replay) + if err != nil { + t.Fatalf("first connection: %v", err) + } + if !cf || !sf { + t.Fatalf("first contact was client-full=%v server-full=%v, want both true", cf, sf) + } + if mem.Tracked() != 1 { + t.Fatalf("the completed full handshake was not recorded (%d contacts)", mem.Tracked()) + } + + // A fresh credential, as rotation would supply, so the second connection + // fails for shape reasons rather than a spent ticket. + cfg.Credential, err = k.Issue(300, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + cf, sf, err = dialOnce(t, k, cover, cfg, replay) + if err != nil { + t.Fatalf("second connection: %v", err) + } + if cf || sf { + t.Errorf("the second connection was client-full=%v server-full=%v, want both false", cf, sf) + } + + // And past the horizon it re-fulls, because the censor can no longer be + // assumed to remember. + mem.Reset() + cfg.Credential, err = k.Issue(300, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + cf, _, err = dialOnce(t, k, cover, cfg, replay) + if err != nil { + t.Fatalf("third connection: %v", err) + } + if !cf { + t.Error("after the relationship was forgotten the client did not re-full") + } +} + +// A Contacts-driven choice degrades rather than failing when the cover has no +// measured full profile, because refusing the connection would make enabling +// Contacts depend on every cover having been probed first. The degradation must +// not be recorded, or it would latch: one silent resumption would look like a +// satisfied relationship forever. +func TestContactsDegradeWhenTheCoverCannotBackAFullHandshake(t *testing.T) { + k := ticketKey(t) + cover := mustCover(t, "www.microsoft.com") // table default: no full profile + mem := NewContactMemory(time.Hour, 0) + cred, err := k.Issue(310, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + + cf, sf, err := dialOnce(t, k, cover, + ClientConfig{Pool: pool(t), Cover: cover, Credential: cred, Contacts: mem}, + NewReplayCache(64, time.Hour)) + if err != nil { + t.Fatalf("the connection was refused instead of degrading: %v", err) + } + if cf || sf { + t.Errorf("client-full=%v server-full=%v against a cover with no full profile", cf, sf) + } + if mem.Tracked() != 0 { + t.Error("a degraded connection was recorded as a completed full handshake; it would never retry") + } +} + +// The same degradation for a pool that cannot carry the ticket -- the non-ECH +// pool docs/ech.md keeps as an escape hatch. Connectivity must survive it. +func TestContactsDegradeWhenThePoolCannotCarryTheTicket(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + mem := NewContactMemory(time.Hour, 0) + cred, err := k.Issue(320, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + none := [][]byte{stripECH(t, DefaultPool()[0]), stripECH(t, DefaultPool()[1])} + + cf, _, err := dialOnce(t, k, cover, + ClientConfig{Pool: none, Cover: cover, Credential: cred, Contacts: mem}, + NewReplayCache(64, time.Hour)) + if err != nil { + t.Fatalf("a pool with no carrier refused the connection instead of degrading: %v", err) + } + if cf { + t.Error("claimed a full handshake from a pool that cannot carry the ticket") + } + if mem.Tracked() != 0 { + t.Error("a degraded connection was recorded") + } +} + +// A full handshake that FAILED established no relationship, so it must not be +// recorded. Recording on attempt rather than completion is the subtle version +// of the bug this whole mechanism exists to prevent: the next connection would +// resume against an egress the censor never saw a completed handshake with, +// which is exactly the structurally impossible shape. +func TestContactsRecordOnlyCompletedHandshakes(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + mem := NewContactMemory(time.Hour, 0) + cred, err := k.Issue(330, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + + // A server that reads the opening and then hangs up, so the client's full + // handshake reaches the wire but never completes. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + go func() { + c, aerr := ln.Accept() + if aerr != nil { + return + } + c.SetDeadline(time.Now().Add(3 * time.Second)) + readRecord(c) // consume the ClientHello, answer nothing + c.Close() + }() + + raw, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer raw.Close() + raw.SetDeadline(time.Now().Add(3 * time.Second)) + local, remote := raw.LocalAddr(), raw.RemoteAddr() + + if _, _, err := Client(raw, ClientConfig{ + Pool: pool(t), Cover: cover, Credential: cred, Contacts: mem, + }); err == nil { + t.Fatal("the client reported success against a server that answered nothing") + } + + if mem.Tracked() != 0 { + t.Errorf("a failed full handshake was recorded (%d contacts)", mem.Tracked()) + } + if !mem.needsFull(local, remote, time.Now()) { + t.Error("after a FAILED full handshake the next connection would resume, with no completed predecessor for a censor to have seen") + } +} From 86c0c9dadeb5b989922e53e57e1b334c76ac2ed5 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 09:53:32 +0100 Subject: [PATCH 10/18] docs: record the mix policy and the two obligations it puts on callers The re-full cadence was the last open twiddle-side question and is now built, so it moves out of "what remains" and into a section of its own. Documents the one number in it -- a six-hour horizon, which is a guess with a direction: unknowable retention, days of flow records in practice, and the cost of being wrong this way is tens of kilobytes a day while the cost of erring long is the shape the carrier exists to remove. Names the two obligations, both of which are wire-up rather than API and both of which fail quietly if skipped: calling Reset on a network change, because the local address is a weak proxy behind NAT and two networks can hand out the same private address, and counting Conn.FullHandshake, because a memory that degraded on every connection looks identical to one that is working. Also states why the state is deliberately in-memory: a restart re-fulls every egress, which is the safe direction, and persisting it would trade a few kilobytes for a file that emits the wrong shape when stale. Co-Authored-By: Claude Opus 5 --- docs/full-handshake-carrier.md | 48 ++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md index ffab447..e69d712 100644 --- a/docs/full-handshake-carrier.md +++ b/docs/full-handshake-carrier.md @@ -236,6 +236,8 @@ All of it mutation-tested — every guarantee below has a deliberate break that | `Options.FullHandshake`, `pre_shared_key` stripped from the template | `twiddle.go` | | `ClientConfig.FullHandshake`, server dispatch on PSK presence, jittered emission, two-record rotation | `handshake.go` | | `FullHandshakeCarriers` — the pool is not uniform, so the draw must be restricted | `echcarrier.go` | +| `ContactMemory` — the mix policy: full on first contact, resumed after, re-full past the horizon | `contacts.go` | +| `Conn.FullHandshake()` — which shape a connection used, so the deployed mix can be counted | `conn.go` | | `ProbeResult.RemainderJitter`, filled by `SampleFull` | `cover.go`, `harvest/coverprobe` | Measured end to end against the microsoft profile: **ServerHello 1215, ccs 6, remainder @@ -252,32 +254,56 @@ Three decisions worth knowing, because each looks like an omission until you see carry one — they come from real browsing, which resumes, and the hellos in `harvest/testdata` do. The emitted hello is then shorter than its source by exactly that extension, which is precisely the difference between a real Chrome resumption hello and a real Chrome full one. +- **Every uncertainty in `ContactMemory` resolves toward a full handshake.** A forgotten entry, an evicted + one, a restarted process, a changed local address, an expired horizon — each produces an *extra* full + handshake, which costs 5–10 KB and looks *more* normal rather than less, since 95%+ of real connections + are full handshakes. The opposite mistake is the distinguisher. That asymmetry is what makes a simple + entry bound sound here, and it is the **reverse** of `ReplayCache`, where evicting a live entry reopens + the window the gate exists to close. - **Both tickets of a credential are sealed at the same instant.** `ReplayCache` refuses a ticket older than the client's newest, so a one-second skew between them would make whichever path the client used *second* look like a stale capture. That failure is invisible to a test of either path alone. ## What remains -1. **Mix policy — the last twiddle-side question.** `ClientConfig.FullHandshake` is per-connection, so the - mechanism is in place and the policy is the caller's. What still needs deciding is the **re-full - cadence**: the censor's flow history is finite and the client's context changes, so a reconnection a - week later, from a different network, after the egress IP rotated, has no observable predecessor even - though the ticket is valid. Some trigger — new local address, new egress IP, elapsed time — has to force - a fresh full handshake. Note the resulting ratio will sit *far above* 4% (a long-lived muxed tunnel - opens few outer connections, so one full per handful of resumed), and that is the correct outcome: what - a censor can check is whether the predecessor exists, not whether we hit a population average. -2. **Provisioning the companion ticket.** lantern-cloud's `GenerateTwiddle` (PR #3291, draft) must emit +1. **Provisioning the companion ticket.** lantern-cloud's `GenerateTwiddle` (PR #3291, draft) must emit `full_ticket` alongside `ticket` and `psk`, and lantern-box must pass it to `CredentialFromWireFull`. Until then a provisioned client is resumption-only, which degrades to today's behaviour rather than failing. `cmd/twiddlecred` already prints it. -3. **A probed full profile per egress.** `CanEmitFullHandshake()` gates both ends on `FullRemainder`, which +2. **A probed full profile per egress.** `CanEmitFullHandshake()` gates both ends on `FullRemainder`, which the table ships empty on purpose, so nothing offers the path until `coverprobe.SampleFull` has run against the live upstream and `Adopt` has taken the result. The startup-probe plumbing is the same work the resumed profile needs. -4. **`sessionTicketWire = 370` is still wrong for all three covers** (microsoft was measured at 303, +3. **`sessionTicketWire = 370` is still wrong for all three covers** (microsoft was measured at 303, cloudflare and google issue none unprompted). Pre-existing; rotation now sends two records, which is closer to microsoft's measured pair, but the size itself is untouched. +### The mix policy, and the one number in it + +`ContactMemory` keys on the (local address, egress address) pair and asks a single question: has this +client completed a full handshake to this egress recently enough that a censor still remembers it? + +- **First contact → full.** There is no ticket-less alternative to explain, and no ratio to tune. +- **Afterwards → resumed**, which is what a real client with a live ticket does. +- **Past the horizon → full again**, because a censor's flow history is finite. + +`DefaultContactHorizon` is **6 hours**, and it is a guess with a direction. The true retention is +unknowable; flow-record retention is commonly days, so six hours sits well inside it, and the cost of +being wrong this way is one extra full handshake per egress per six hours — tens of kilobytes a day. +Erring long risks emitting exactly the resumption-without-predecessor the carrier exists to remove. + +Two caller obligations, both operational rather than API: + +1. **Call `Reset()` on a network change.** The local address is a weak proxy: behind NAT two different + networks can both hand out `192.168.1.5`, and the move goes unnoticed. radiance and lantern-box already + detect network changes for VPN reconnection, so this is a wire-up, not new machinery. +2. **Count `Conn.FullHandshake()`.** A memory that degraded on every connection, because no cover was ever + probed, looks identical to one that is working. The deployed mix is the only evidence it works. + +State is in-memory only, so a restart re-fulls every egress. That is the safe direction and is left alone +deliberately — persisting it would trade a few kilobytes for a file that, if stale, emits the exact shape +we are avoiding. + ## Traps worth knowing before starting Each of these cost real time in #1: From 9c9971327b5d841e31dd6e4b5e2c1675721b9d7e Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 12:33:34 +0100 Subject: [PATCH 11/18] Replay the hellos we emit at the real covers, and require a ServerHello back This is the only test that can fail for a reason no offline test can see, and the failure mode is not hypothetical. rerandKeyShare's comment states it: "a censor can capture one of our hellos and replay it to the SNI we claim -- a genuine Chrome hello draws a ServerHello, ours would draw an alert." An earlier version filled key shares with random bytes, real servers answered illegal_parameter or decode_error, and every offline test passed throughout. A censor replaying one of our hellos is running exactly this check, so we should run it first. TestEmittedHellosAreAcceptedByTheRealCovers emits both handshake shapes from every distinct hello shape and requires a handshake record with a ServerHello back, decoding the alert description when one comes instead. Confirmed to work by reintroducing the historical bug: cloudflare answered decode_error and the test named it. Variety is by SHAPE, not by record. The two sources are different browser builds and both matter -- pool/chrome.hex carries BoringSSL's server_padding and runs 1725-1827 bytes, the harvest/testdata captures are Chrome 152, carry 0xca34, and run 1919-2015 bytes -- but they hold 72 records and 17 distinct shapes. Fingerprint is the repo's own notion of sameness, so deduplicating on it keeps the coverage and drops 400 pointless connections. TestRealCoverServerHelloStillMatchesTheConstant keeps ServerHelloFullLen honest against the live internet, and all three covers answer 1215 today. It can only check the full length: our ticket is ours, so a real server ignores the pre_shared_key and completes a full handshake, which means both variants draw the full shape. Checking the resumed constant live would need a real prior session, which is what harvest/cmd/postflight is for. Three things had to be fixed to make live testing usable at all, all of them real rather than cosmetic: Cloudflare declines to resume about 40% of the time -- a second connection lands on an edge server that cannot decrypt its sibling's ticket. Probe and ProbeBoth treated that as fatal, which made three existing live tests flaky enough to be unusable in CI. ErrNoResume is now a sentinel, and it is the one probe failure that leaves a usable result behind: ProbeBoth reads the full opening before it attempts the resumed one. So SampleFull accepts such a sample outright, since the full profile is all it measures, and the two consistency tests skip rather than fail. TestAtLeastOneCoverStillResumes is the floor that stops every cover skipping silently. Connections are paced and the exhaustive sweep is opt-in. Running the full sweep back to back locally -- ~120 connections to three hosts in a minute -- started failing tests that pass in isolation, because the hosts throttle. A throttled run reads as a code regression, so the default is a handful of shapes and TWIDDLE_LIVE_FULL_SWEEP covers everything. And the live probes immediately earned their keep: full-remainder-drift.log records that google's certificate flight fell from 3921 to 2619 bytes in one day -- a rotation, not jitter -- while cloudflare revealed the 3846/3847/3848 range that five samples had reported as a jitter of 1. The design doc argued FullRemainder "cannot be a constant" from mechanism; this is the evidence, and it settles the cadence question too: a probed profile's useful life is days. Co-Authored-By: Claude Opus 5 --- docs/full-handshake-carrier.md | 6 + harvest/coverprobe/coverprobe.go | 30 +- harvest/coverprobe/coverprobe_test.go | 44 +++ harvest/testdata/full-remainder-drift.log | 46 +++ live_test.go | 357 ++++++++++++++++++++++ 5 files changed, 481 insertions(+), 2 deletions(-) create mode 100644 harvest/testdata/full-remainder-drift.log create mode 100644 live_test.go diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md index e69d712..232cfec 100644 --- a/docs/full-handshake-carrier.md +++ b/docs/full-handshake-carrier.md @@ -87,6 +87,12 @@ Prerequisite, and all of it already on main before this work: | google | 1215 | 6 | `[3921]` | 5142 | | microsoft | 1215 | 6 | `[32, 8273, 286, 74]` | 9886 | +**Since confirmed the hard way.** Re-probing a day later (`harvest/testdata/full-remainder-drift.log`) +found google's flight had fallen from `[3921]` to `[2619]` — 1302 bytes, a certificate rotation rather +than signature jitter — while microsoft held exactly and cloudflare moved 2, revealing the +3846/3847/3848 range that five samples had reported as a jitter of 1. So the "cannot be a constant" +argument below is no longer just from mechanism, and the useful lifetime of a probed profile is days. + Three consequences: 1. **The remainder is the certificate**, so a faithful full handshake costs **5–10 KB** of opening diff --git a/harvest/coverprobe/coverprobe.go b/harvest/coverprobe/coverprobe.go index ac203f6..910741c 100644 --- a/harvest/coverprobe/coverprobe.go +++ b/harvest/coverprobe/coverprobe.go @@ -35,6 +35,7 @@ import ( "context" "crypto/tls" "encoding/binary" + "errors" "fmt" "net" "sync" @@ -100,7 +101,7 @@ func ProbeBoth(ctx context.Context, dial Dialer, host string) (full, resumed tw. return full, resumed, fmt.Errorf("coverprobe %s: resumed handshake: %w", host, err) } if !tap.resumed { - return full, resumed, fmt.Errorf("coverprobe %s: upstream did not resume, so this is not the opening we imitate", host) + return full, resumed, fmt.Errorf("coverprobe %s: %w, so this is not the opening we imitate", host, ErrNoResume) } if err := readOpening(&resumed, tap, tw.ServerHelloResumedLen, host); err != nil { return full, resumed, err @@ -278,6 +279,22 @@ func (r *recorder) serverRecords() []record { // The baseline is the smallest length seen at each position and the jitter is // the observed range. A run where the record COUNT changes between samples is // rejected: that is a different server answering, not the same one jittering. +// ErrNoResume reports that an upstream declined to resume the session it had +// just issued a ticket for. +// +// It is a sentinel because it is the one probe failure that leaves a USABLE +// result behind: ProbeBoth completes and reads the full opening before it +// attempts the resumed one, so a result carrying this error has a valid full +// profile and only an unperformed resumed check. +// +// The condition is common rather than rare. Cloudflare declines most of the +// time -- a second connection lands on a different edge server from the one +// that issued the ticket, so the ticket does not decrypt and the server falls +// back to a full handshake. Nothing is wrong with the cover or with us when +// that happens, which is why SampleFull treats it as success and why retrying +// it is pointless. +var ErrNoResume = errors.New("upstream did not resume") + func SampleFull(ctx context.Context, dial Dialer, host string, n int) (tw.ProbeResult, []int, error) { if n < 2 { return tw.ProbeResult{}, nil, fmt.Errorf("coverprobe %s: SampleFull needs at least 2 samples to see a range", host) @@ -285,8 +302,17 @@ func SampleFull(ctx context.Context, dial Dialer, host string, n int) (tw.ProbeR var base tw.ProbeResult var lo, hi []int for i := 0; i < n; i++ { + // What this function measures is the FULL profile. The resumed leg is + // ProbeBoth's own consistency check and is not sampled here, so a + // sample whose ONLY failure was that the upstream declined to resume + // still carries everything being measured, and is accepted. + // + // This is not leniency for its own sake: requiring the resumed leg made + // this unusable against cloudflare, which declines most of the time, so + // the check was holding the measurement hostage to a behaviour it was + // not measuring. Every other error is still fatal. full, _, err := ProbeBoth(ctx, dial, host) - if err != nil { + if err != nil && !errors.Is(err, ErrNoResume) { return base, nil, fmt.Errorf("coverprobe %s: sample %d: %w", host, i+1, err) } if i == 0 { diff --git a/harvest/coverprobe/coverprobe_test.go b/harvest/coverprobe/coverprobe_test.go index 94b55c1..d586ed7 100644 --- a/harvest/coverprobe/coverprobe_test.go +++ b/harvest/coverprobe/coverprobe_test.go @@ -2,6 +2,7 @@ package coverprobe import ( "context" + "errors" "net" "os" "slices" @@ -36,6 +37,16 @@ func TestProbeReproducesTheMeasuredProfile(t *testing.T) { } res, err := Probe(ctx, dial, host) + // Probe returns only the resumed half, so it inherits ProbeBoth's + // dependence on the upstream actually resuming -- which cloudflare + // declines roughly 40% of the time, because a second connection + // lands on an edge server that cannot decrypt its sibling's ticket. + // Skipped rather than failed: nothing about the cover or about us is + // wrong. TestAtLeastOneCoverStillResumes is the floor that stops + // every cover skipping silently. + if errors.Is(err, ErrNoResume) { + t.Skipf("%s declined to resume on this attempt; nothing to compare", host) + } if err != nil { t.Fatalf("probe: %v", err) } @@ -79,6 +90,16 @@ func TestProbeBothAgainstLiveUpstreams(t *testing.T) { } full, resumed, err := ProbeBoth(ctx, dial, host) + // A cover that declined to resume on this attempt has told us + // nothing is wrong -- it landed on an edge server that could not + // decrypt its own sibling's ticket. Measured at a 40% failure rate + // against cloudflare, so failing here made this test unusable in + // CI. Skipped rather than tolerated, so it cannot pass while + // verifying nothing, and the counter below is what stops EVERY + // cover skipping silently. + if errors.Is(err, ErrNoResume) { + t.Skipf("%s declined to resume on this attempt; nothing to compare", host) + } if err != nil { t.Fatalf("probe: %v", err) } @@ -126,6 +147,29 @@ func TestProbeBothAgainstLiveUpstreams(t *testing.T) { // that establishes it, and it is the one an emitter needs: sending a single // observation verbatim would make us the only host whose certificate flight is // byte-identical on every connection. +// The floor on the skips above: if no cover anywhere +// produced a resumed observation, the test verified nothing and says so. +func TestAtLeastOneCoverStillResumes(t *testing.T) { + if os.Getenv("TWIDDLE_LIVE_PROBE") == "" { + t.Skip("set TWIDDLE_LIVE_PROBE=1 to probe the real covers") + } + for _, host := range tw.MeasuredCovers() { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + dial := func(ctx context.Context) (net.Conn, error) { + var d net.Dialer + return d.DialContext(ctx, "tcp", net.JoinHostPort(host, "443")) + } + _, resumed, err := ProbeBoth(ctx, dial, host) + cancel() + if err == nil { + t.Logf("%s resumed: ServerHello %d, remainder %v", + host, resumed.ServerHello, resumed.Remainder) + return + } + } + t.Error("no measured cover resumed on any attempt; the resumed profile this transport imitates can no longer be observed anywhere") +} + func TestSampleFullObservesTheJitter(t *testing.T) { if os.Getenv("TWIDDLE_LIVE_PROBE") == "" { t.Skip("set TWIDDLE_LIVE_PROBE=1 to probe the real covers") diff --git a/harvest/testdata/full-remainder-drift.log b/harvest/testdata/full-remainder-drift.log new file mode 100644 index 0000000..71210ae --- /dev/null +++ b/harvest/testdata/full-remainder-drift.log @@ -0,0 +1,46 @@ +The full-handshake remainder drifts, and by more than jitter. + + tool coverprobe SampleFull, 5 samples per host, via the live CI tests + date 2026-09-04 + baseline harvest/testdata/postflight-full-vs-resumed.log, 2026-09-03 + +Why this was recorded: docs/full-handshake-carrier.md asserts that +CoverProfile.FullRemainder "cannot be a constant" and must come from a probe +against the live upstream. That was an argument from mechanism -- a DER-encoded +ECDSA signature varies in length, and certificates rotate. Wiring the live +probes into CI produced the evidence for it within a day. + + 2026-09-03 2026-09-04 delta + cloudflare [3848] [3846] jitter [2] -2, within jitter + google [3921] [2619] jitter [1] -1302 + microsoft [32 8273 286 74] [32 8273 286 74] jitter [0 0 0 0] 0 + +FINDINGS + +1. google's certificate flight fell by 1302 bytes in one day. That is not + signature jitter; it is a different certificate chain, most likely a + rotation to a shorter one. Any table constant for this cover would now be + 1302 bytes wrong, and an egress emitting it would be the only host claiming + to be google whose certificate flight is a chain google no longer serves. + +2. cloudflare moved 2 bytes and reported jitter 2, so this run saw the + 3846/3847/3848 range that postflight-full-vs-resumed.log recorded as a + jitter of 1 from 5 samples. That is the documented consequence of a sampled + jitter being a FLOOR rather than a range: more samples widen it, and + narrowing it on the strength of one run would be wrong. + +3. microsoft held exactly, all four records, jitter zero. Its RSA signature is + fixed-length, and its chain did not rotate in this window. Consistent with + the earlier measurement. + +CONSEQUENCE + +Nothing in the shipped code breaks, because FullRemainder is empty in the table +by design and every emitter is gated on CanEmitFullHandshake. What this does +settle is the cadence question the design left open: a probed full profile is +not a one-time startup measurement that can be cached indefinitely. google's +drift says the useful lifetime of one is days, not weeks. + +It also means the numbers quoted in docs and logs are snapshots. They are +correct as measurements and wrong as expectations, which is why the live tests +assert ServerHelloFullLen -- a protocol constant -- and NOT the remainder. diff --git a/live_test.go b/live_test.go new file mode 100644 index 0000000..ab9cbc7 --- /dev/null +++ b/live_test.go @@ -0,0 +1,357 @@ +package twiddle + +import ( + "encoding/binary" + "errors" + "fmt" + "io" + "net" + "os" + "sort" + "strings" + "testing" + "time" +) + +// The acceptance test that matters most, and the only one that can fail for a +// reason no local test can see. +// +// rerandKeyShare's comment states the threat: "a censor can capture one of our +// hellos and replay it to the SNI we claim -- a genuine Chrome hello draws a +// ServerHello, ours would draw an alert." That is not hypothetical. An earlier +// version filled key shares with random bytes and real servers answered with +// illegal_parameter or decode_error, because random bytes are not a valid +// ML-KEM-768 encapsulation key. Every local test passed throughout. +// +// So these tests replay what we actually emit at the real cover hosts and +// require a ServerHello back. Nothing about our own record layer, ticket key or +// replay gate participates: the real server cannot authenticate us and is not +// asked to. What is under test is whether the bytes we put on the wire are +// bytes a real server accepts -- which is exactly what a censor replaying them +// would be testing. +// +// Gated because it needs the network. CI sets TWIDDLE_LIVE_PROBE=1. + +func liveProbeEnabled(t *testing.T) { + t.Helper() + if os.Getenv("TWIDDLE_LIVE_PROBE") == "" { + t.Skip("set TWIDDLE_LIVE_PROBE=1 to replay emitted hellos at the real covers") + } +} + +// pace spaces out connections to the real covers. +// +// Measured the hard way: running the exhaustive sweep back to back locally -- +// roughly 120 connections to three hosts inside a minute -- started producing +// failures in tests that pass in isolation. The hosts throttle, and a throttled +// CI run reads as a code regression. A short gap costs seconds and removes the +// whole class of false failure. +func pace() { time.Sleep(150 * time.Millisecond) } + +// exhaustiveSweep reports whether to replay EVERY distinct hello shape. +// +// Off by default, so a pull request gets cheap live signal from a handful of +// connections. The daily scheduled run sets it and covers everything. The split +// exists because the exhaustive sweep is ~120 connections to three real hosts, +// which is fine once a day and rude on every push. +func exhaustiveSweep() bool { return os.Getenv("TWIDDLE_LIVE_FULL_SWEEP") != "" } + +// sampleShapes trims a shape list for the default run, keeping BOTH sources +// represented -- the two are different browser builds, so a sample from one +// would leave the other unexercised. +func sampleShapes(names []string) []string { + if exhaustiveSweep() { + return names + } + const perSource = 2 + var embedded, captured []string + for _, n := range names { + if strings.HasPrefix(n, "embedded-") { + embedded = append(embedded, n) + } else { + captured = append(captured, n) + } + } + if len(embedded) > perSource { + embedded = embedded[:perSource] + } + if len(captured) > perSource { + captured = captured[:perSource] + } + return append(embedded, captured...) +} + +// alertName decodes the descriptions a rejected hello actually draws, so a +// failure says what was wrong rather than just that something was. +func alertName(desc byte) string { + switch desc { + case 40: + return "handshake_failure" + case 42: + return "bad_certificate" + case 47: + return "illegal_parameter" + case 50: + return "decode_error" + case 51: + return "decrypt_error" + case 70: + return "protocol_version" + case 71: + return "insufficient_security" + case 80: + return "internal_error" + case 109: + return "missing_extension" + case 110: + return "unsupported_extension" + case 112: + return "unrecognized_name" + case 116: + return "certificate_required" + case 120: + return "no_application_protocol" + default: + return fmt.Sprintf("alert(%d)", desc) + } +} + +// errTransient marks a failure to reach the host at all, as opposed to a host +// that answered and rejected us. Only the former is worth retrying: an alert is +// a verdict, and retrying it would turn a real regression into a slow one. +var errTransient = errors.New("transient network failure") + +// replay writes one emitted hello to host:443 and returns the first record the +// server sends back. +func replay(host string, hello []byte) (recType byte, body []byte, err error) { + d := net.Dialer{Timeout: 10 * time.Second} + c, err := d.Dial("tcp", net.JoinHostPort(host, "443")) + if err != nil { + return 0, nil, fmt.Errorf("%w: dial: %v", errTransient, err) + } + defer c.Close() + if err := c.SetDeadline(time.Now().Add(15 * time.Second)); err != nil { + return 0, nil, fmt.Errorf("%w: deadline: %v", errTransient, err) + } + if _, err := c.Write(hello); err != nil { + return 0, nil, fmt.Errorf("%w: write: %v", errTransient, err) + } + var hdr [recordHeaderLen]byte + if _, err := io.ReadFull(c, hdr[:]); err != nil { + // A server that hangs up without a record has rejected us, but at TCP + // level rather than TLS level, and that is indistinguishable here from + // a network fault. Treated as transient so a flaky runner does not fail + // the build; a genuine rejection reproduces on every retry and still + // fails. + return 0, nil, fmt.Errorf("%w: read header: %v", errTransient, err) + } + n := int(binary.BigEndian.Uint16(hdr[3:5])) + if n > maxCiphertext { + return hdr[0], nil, fmt.Errorf("record length %d out of range", n) + } + body = make([]byte, n) + if _, err := io.ReadFull(c, body); err != nil { + return hdr[0], nil, fmt.Errorf("%w: read body: %v", errTransient, err) + } + return hdr[0], body, nil +} + +// replayWithRetry retries only transient failures. +func replayWithRetry(t *testing.T, host string, hello []byte) (byte, []byte, error) { + t.Helper() + var lastErr error + for attempt := 1; attempt <= 3; attempt++ { + typ, body, err := replay(host, hello) + if err == nil || !errors.Is(err, errTransient) { + return typ, body, err + } + lastErr = err + time.Sleep(time.Duration(attempt) * time.Second) + } + return 0, nil, lastErr +} + +// variedHellos returns one hello per distinct SHAPE, drawn from both sources. +// +// Both sources matter because they are different browser builds that exercise +// different code paths: pool/chrome.hex carries BoringSSL's server_padding +// (0x12e0) and runs 1725-1827 bytes, while the harvest/testdata captures are +// Chrome 152, carry 0xca34 instead, and run 1919-2015 bytes. A test using only +// one would not notice a change that broke the other. +// +// But variety means variety of shapes, not of records. The raw sources hold 72 +// hellos and only a handful of distinct shapes, so replaying all of them would +// mean hundreds of connections to real hosts to learn what a few dozen say. +// Fingerprint is the repo's own notion of "same shape" -- it normalises the +// per-connection GREASE draws and keys on the structure a server actually +// reacts to -- so deduplicating on it keeps the coverage and drops the +// repetition. +// +// Keys are sorted so the subtest names, and the order the hosts are hit in, are +// stable run to run. +func variedHellos(t *testing.T) []string { + t.Helper() + byFingerprint := map[string]string{} + names := map[string][]byte{} + + add := func(name string, rec []byte) { + h, err := ParseClientHello(rec) + if err != nil { + return + } + f := h.Fingerprint() + if _, dup := byFingerprint[f]; dup { + return + } + byFingerprint[f] = name + names[name] = rec + } + for i, rec := range DefaultPool() { + add(fmt.Sprintf("embedded-%d", i), rec) + } + // Sorted, because realHellos returns a map and an arbitrary survivor per + // fingerprint would make the selection differ between runs. + var captured []string + raw := realHellos(t) + for name := range raw { + captured = append(captured, name) + } + sort.Strings(captured) + for _, name := range captured { + add("captured-"+name, raw[name]) + } + + var out []string + for name := range names { + out = append(out, name) + } + sort.Strings(out) + liveHellos = names + return out +} + +// liveHellos holds the records variedHellos selected, keyed by the names it +// returned. +var liveHellos map[string][]byte + +// Every hello we emit, in both handshake shapes, must draw a ServerHello from +// the real cover host rather than an alert. +func TestEmittedHellosAreAcceptedByTheRealCovers(t *testing.T) { + liveProbeEnabled(t) + k := ticketKey(t) + names := sampleShapes(variedHellos(t)) + t.Logf("replaying %d hello shapes at %d covers, both variants, exhaustive=%v", + len(names), len(MeasuredCovers()), exhaustiveSweep()) + + for _, host := range MeasuredCovers() { + cover, err := CoverFor(host) + if err != nil { + t.Fatal(err) + } + cred, err := k.Issue(1, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + + t.Run(host, func(t *testing.T) { + for _, name := range names { + rec := liveHellos[name] + for _, variant := range []struct { + label string + full bool + }{{"resumed", false}, {"full", true}} { + // A hello whose ECH payload cannot hold the ticket has no + // full variant, which FullHandshakeCarriers is what decides + // in production too. + if variant.full && len(FullHandshakeCarriers([][]byte{rec})) == 0 { + continue + } + t.Run(name+"/"+variant.label, func(t *testing.T) { + wire, _, err := Twiddle(rec, Options{ + CoverSNI: host, + Credential: cred, + BinderLen: cover.BinderLen, + FullHandshake: variant.full, + }) + if err != nil { + t.Fatalf("emitting: %v", err) + } + + pace() + typ, body, err := replayWithRetry(t, host, wire) + if err != nil { + if errors.Is(err, errTransient) { + t.Skipf("could not reach %s after 3 attempts: %v", host, err) + } + t.Fatalf("replaying a %d-byte hello: %v", len(wire), err) + } + + switch typ { + case contentAlert: + desc := byte(0) + if len(body) >= 2 { + desc = body[1] + } + t.Fatalf("%s REJECTED our %d-byte hello with %s -- a real Chrome hello draws a ServerHello, so this is a live distinguisher", + host, len(wire), alertName(desc)) + case contentHandshake: + if len(body) == 0 || body[0] != 0x02 { + t.Fatalf("%s answered with handshake type %#02x, not a ServerHello", host, body[0]) + } + default: + t.Fatalf("%s answered with record type %#02x, neither a handshake nor an alert", host, typ) + } + }) + } + } + }) + } +} + +// The ServerHello we synthesise is asserted against a constant, and this is +// what keeps that constant honest against the live internet. +// +// It can only check the FULL length. Our ticket is ours, so a real server does +// not recognise it, ignores the pre_shared_key and completes a full handshake +// -- which means both variants draw ServerHelloFullLen here. Checking +// ServerHelloResumedLen live would need a real prior session with the cover, +// which is what harvest/cmd/postflight is for. +func TestRealCoverServerHelloStillMatchesTheConstant(t *testing.T) { + liveProbeEnabled(t) + k := ticketKey(t) + + for _, host := range MeasuredCovers() { + cover, err := CoverFor(host) + if err != nil { + t.Fatal(err) + } + cred, err := k.Issue(1, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + t.Run(host, func(t *testing.T) { + wire, _, err := Twiddle(DefaultPool()[0], Options{ + CoverSNI: host, Credential: cred, BinderLen: cover.BinderLen, + }) + if err != nil { + t.Fatal(err) + } + typ, body, err := replayWithRetry(t, host, wire) + if err != nil { + if errors.Is(err, errTransient) { + t.Skipf("could not reach %s: %v", host, err) + } + t.Fatal(err) + } + if typ != contentHandshake { + t.Fatalf("%s did not answer with a handshake record: type %#02x", host, typ) + } + got := recordHeaderLen + len(body) + t.Logf("%s ServerHello: %d bytes", host, got) + if got != ServerHelloFullLen { + t.Errorf("%s now sends a %d-byte ServerHello, but we synthesise %d; the constant is stale and our opening is a different length from the identity it claims", + host, got, ServerHelloFullLen) + } + }) + } +} From 5230e81beb1c732a8dccbb76f6b467edc9fc9d84 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 12:37:42 +0100 Subject: [PATCH 12/18] Add CI: an offline job, and a live job that replays at the real covers This repo had no .github directory, so nothing was verified anywhere but a laptop. Both PRs merged so far were merged on local runs alone. Two jobs, separate on purpose. The offline job is gofmt, vet, build, test and test -race; a flaky runner or blocked egress cannot make it red, so red means the code. It includes TestShippedPackagesImportNoTLSLibrary, which is really a build gate wearing a test's clothes -- it walks the import graph to keep a TLS stack out of every shipped package, which is this transport's central design property. The live job replays what we actually emit at the real cover hosts. It is the only check that can catch a hello real servers reject, and that is a live distinguisher rather than a bug: a censor replaying our hello to the SNI we claim gets a ServerHello from real Chrome and an alert from us. It also runs the coverprobe live tests, which had never run anywhere automatically. The live job runs on a daily schedule as well as on pushes, because it can start failing without anybody touching this repo -- a cover can rotate its certificate, change its ServerHello, or stop accepting a shape we send. The first day of live probing already found google's certificate flight moving 1302 bytes. A schedule turns that from a surprise at deploy time into a notification. The schedule also sets TWIDDLE_LIVE_FULL_SWEEP so the exhaustive sweep runs once a day, while pushes replay a handful of shapes. That split is not thrift: ~120 connections to three real hosts on every push gets throttled, and a throttled run reads as a code regression. Also gofmt's three files under harvest/cmd that predate this work. They are untouched otherwise; the gate cannot pass over them, and exempting them would make the gate advisory. Co-Authored-By: Claude Opus 5 --- .github/workflows/go.yaml | 92 +++++++++++++++++++++++++++ harvest/cmd/resume/main.go | 108 +++++++++++++++++++++++--------- harvest/cmd/resumeratio/main.go | 10 +-- harvest/cmd/sweep/main.go | 41 ++++++++---- 4 files changed, 207 insertions(+), 44 deletions(-) create mode 100644 .github/workflows/go.yaml diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml new file mode 100644 index 0000000..591bd4c --- /dev/null +++ b/.github/workflows/go.yaml @@ -0,0 +1,92 @@ +name: Go + +on: + push: + branches: ["main"] + pull_request: + workflow_dispatch: + # The live job checks our emitted hellos against real servers, so it can + # start failing without anybody touching this repo: a cover can rotate its + # certificate, change its ServerHello, or stop accepting a shape we send. + # A daily run is what turns that from a surprise at deploy time into a + # notification. + schedule: + - cron: "17 6 * * *" + +permissions: + contents: read + +jobs: + # Everything that needs no network. Kept separate from the live job so a + # flaky runner or a blocked egress cannot be mistaken for a code regression. + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: "go.mod" + + - name: gofmt + run: | + unformatted=$(gofmt -l .) + if [ -n "$unformatted" ]; then + echo "these files are not gofmt'd:" + echo "$unformatted" + gofmt -d $unformatted + exit 1 + fi + + - name: Vet + run: go vet ./... + + - name: Build + run: go build ./... + + # The suite includes TestShippedPackagesImportNoTLSLibrary, which is the + # guard on this transport's central design property: no shipped package + # may import a TLS stack. It is a test rather than a lint because it has + # to walk the import graph, but it is really a build gate. + - name: Test + run: go test -count=1 ./... + + - name: Test with race detector + run: go test -count=1 -race ./... + + # Replays what we actually emit at the real cover hosts and requires a + # ServerHello back. + # + # This is the only test that can fail for a reason no local test can see, and + # the failure mode is not theoretical: an earlier version of freshKeyShare + # filled key shares with random bytes, real servers answered decode_error, + # and every offline test passed throughout. A censor replaying one of our + # hellos to the SNI we claim is running exactly this check. + # + # It needs the network, so the tests are gated on TWIDDLE_LIVE_PROBE and skip + # by default. They also skip rather than fail when a host is unreachable after + # three attempts, so a network fault does not read as a rejection -- a real + # rejection reproduces on every attempt and still fails. + live: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: "go.mod" + + # On a push or pull request this replays a handful of shapes -- enough to + # catch a hello real servers reject, which is the failure that matters. + # The scheduled run sets TWIDDLE_LIVE_FULL_SWEEP and covers every distinct + # shape. The split is not thrift: the exhaustive sweep is ~120 connections + # to three real hosts, and running it repeatedly gets throttled, which + # then reads as a code regression. + - name: Replay emitted hellos at the real covers + env: + TWIDDLE_LIVE_PROBE: "1" + TWIDDLE_LIVE_FULL_SWEEP: ${{ github.event_name == 'schedule' && '1' || '' }} + run: | + go test -count=1 -v -timeout 25m -run 'Live|RealCover|AcceptedByTheReal|SampleFull|Probe|AtLeastOneCover' ./... diff --git a/harvest/cmd/resume/main.go b/harvest/cmd/resume/main.go index 465fc88..e0fd84a 100644 --- a/harvest/cmd/resume/main.go +++ b/harvest/cmd/resume/main.go @@ -30,20 +30,33 @@ func (t *tap) Write(b []byte) (int, error) { func extSizes(rec []byte) (total int, exts map[uint16]int, order []uint16) { exts = map[uint16]int{} - if len(rec) < 6 { return } + if len(rec) < 6 { + return + } b := rec[5:] total = len(rec) p := 4 + 2 + 32 - if p >= len(b) { return } + if p >= len(b) { + return + } p += 1 + int(b[p]) - if p+2 > len(b) { return } - cl := int(binary.BigEndian.Uint16(b[p : p+2])); p += 2 + cl - if p >= len(b) { return } + if p+2 > len(b) { + return + } + cl := int(binary.BigEndian.Uint16(b[p : p+2])) + p += 2 + cl + if p >= len(b) { + return + } p += 1 + int(b[p]) - if p+2 > len(b) { return } - end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])); p += 2 + if p+2 > len(b) { + return + } + end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])) + p += 2 for p+4 <= end && p+4 <= len(b) { - id := binary.BigEndian.Uint16(b[p : p+2]); ln := int(binary.BigEndian.Uint16(b[p+2 : p+4])) + id := binary.BigEndian.Uint16(b[p : p+2]) + ln := int(binary.BigEndian.Uint16(b[p+2 : p+4])) exts[id] = ln order = append(order, id) p += 4 + ln @@ -53,20 +66,34 @@ func extSizes(rec []byte) (total int, exts map[uint16]int, order []uint16) { // extData returns the raw extension_data for one extension id func extData(rec []byte, want uint16) []byte { - if len(rec) < 6 { return nil } + if len(rec) < 6 { + return nil + } b := rec[5:] p := 4 + 2 + 32 - if p >= len(b) { return nil } + if p >= len(b) { + return nil + } p += 1 + int(b[p]) - if p+2 > len(b) { return nil } + if p+2 > len(b) { + return nil + } p += 2 + int(binary.BigEndian.Uint16(b[p:p+2])) - if p >= len(b) { return nil } + if p >= len(b) { + return nil + } p += 1 + int(b[p]) - if p+2 > len(b) { return nil } - end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])); p += 2 + if p+2 > len(b) { + return nil + } + end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])) + p += 2 for p+4 <= end && p+4 <= len(b) { - id := binary.BigEndian.Uint16(b[p:p+2]); ln := int(binary.BigEndian.Uint16(b[p+2:p+4])) - if id == want && p+4+ln <= len(b) { return b[p+4 : p+4+ln] } + id := binary.BigEndian.Uint16(b[p : p+2]) + ln := int(binary.BigEndian.Uint16(b[p+2 : p+4])) + if id == want && p+4+ln <= len(b) { + return b[p+4 : p+4+ln] + } p += 4 + ln } return nil @@ -76,18 +103,26 @@ func extData(rec []byte, want uint16) []byte { // obfuscated_ticket_age) followed by binders. Both are opaque to any observer // without the resumption secret. func dumpPSK(d []byte) { - if len(d) < 2 { return } + if len(d) < 2 { + return + } idsEnd := 2 + int(binary.BigEndian.Uint16(d[0:2])) p := 2 n := 0 for p+2 <= idsEnd && p+2 <= len(d) { - tl := int(binary.BigEndian.Uint16(d[p : p+2])); p += 2 - if p+tl+4 > len(d) { break } + tl := int(binary.BigEndian.Uint16(d[p : p+2])) + p += 2 + if p+tl+4 > len(d) { + break + } age := binary.BigEndian.Uint32(d[p+tl : p+tl+4]) fmt.Printf(" identity[%d]: ticket %d B, obfuscated_ticket_age 0x%08x\n", n, tl, age) - p += tl + 4; n++ + p += tl + 4 + n++ + } + if idsEnd+2 > len(d) { + return } - if idsEnd+2 > len(d) { return } bEnd := idsEnd + 2 + int(binary.BigEndian.Uint16(d[idsEnd:idsEnd+2])) p = idsEnd + 2 for m := 0; p < bEnd && p < len(d); m++ { @@ -105,7 +140,10 @@ func run(host string) { for i := 0; i < 2; i++ { raw, err := net.DialTimeout("tcp", host+":443", 6*time.Second) - if err != nil { fmt.Println(" dial:", err); return } + if err != nil { + fmt.Println(" dial:", err) + return + } tp := &tap{Conn: raw} c := tls.Client(tp, &tls.Config{ ServerName: host, @@ -113,7 +151,11 @@ func run(host string) { MinVersion: tls.VersionTLS13, }) c.SetDeadline(time.Now().Add(8 * time.Second)) - if err := c.Handshake(); err != nil { fmt.Println(" handshake:", err); raw.Close(); return } + if err := c.Handshake(); err != nil { + fmt.Println(" handshake:", err) + raw.Close() + return + } st := c.ConnectionState() if i == 0 { full = tp.first @@ -125,11 +167,15 @@ func run(host string) { resumed = tp.first didResume = st.DidResume } - c.Close(); raw.Close() + c.Close() + raw.Close() time.Sleep(300 * time.Millisecond) } - if full == nil || resumed == nil { fmt.Println(" incomplete"); return } + if full == nil || resumed == nil { + fmt.Println(" incomplete") + return + } ft, fe, _ := extSizes(full) rt, re, ro := extSizes(resumed) fmt.Printf(" full handshake hello : %4d bytes, %d extensions\n", ft, len(fe)) @@ -137,9 +183,13 @@ func run(host string) { fmt.Printf(" delta : %+d bytes\n", rt-ft) if psk, ok := re[0x0029]; ok { fmt.Printf(" pre_shared_key (0x0029) = %d bytes", psk) - if len(ro) > 0 && ro[len(ro)-1] == 0x0029 { fmt.Printf(" [LAST extension, as required]") } + if len(ro) > 0 && ro[len(ro)-1] == 0x0029 { + fmt.Printf(" [LAST extension, as required]") + } fmt.Println() - if d := extData(resumed, 0x0029); d != nil { dumpPSK(d) } + if d := extData(resumed, 0x0029); d != nil { + dumpPSK(d) + } } else { fmt.Println(" no pre_shared_key in second hello — server did not issue a usable ticket") } @@ -153,7 +203,9 @@ func run(host string) { func main() { hosts := os.Args[1:] - if len(hosts) == 0 { hosts = []string{"www.google.com", "www.cloudflare.com", "www.microsoft.com", "github.com"} } + if len(hosts) == 0 { + hosts = []string{"www.google.com", "www.cloudflare.com", "www.microsoft.com", "github.com"} + } for _, h := range hosts { fmt.Printf("\n=== %s\n", h) run(h) diff --git a/harvest/cmd/resumeratio/main.go b/harvest/cmd/resumeratio/main.go index 763d665..5a447f0 100644 --- a/harvest/cmd/resumeratio/main.go +++ b/harvest/cmd/resumeratio/main.go @@ -26,11 +26,11 @@ import ( ) type stats struct { - mu sync.Mutex - full int - resumed int - notTLS int - perHost map[string][2]int // host -> [full, resumed] + mu sync.Mutex + full int + resumed int + notTLS int + perHost map[string][2]int // host -> [full, resumed] } func (s *stats) record(host string, isTLS, psk bool) { diff --git a/harvest/cmd/sweep/main.go b/harvest/cmd/sweep/main.go index d85838b..b1f398e 100644 --- a/harvest/cmd/sweep/main.go +++ b/harvest/cmd/sweep/main.go @@ -16,26 +16,45 @@ func main() { seen := 0 for seen < 8 { c, err := ln.Accept() - if err != nil { continue } + if err != nil { + continue + } h := make([]byte, 5) - if _, e := io.ReadFull(c, h); e != nil { c.Close(); continue } + if _, e := io.ReadFull(c, h); e != nil { + c.Close() + continue + } b := make([]byte, int(binary.BigEndian.Uint16(h[3:5]))) - io.ReadFull(c, b); c.Close() - if h[0] != 0x16 { continue } + io.ReadFull(c, b) + c.Close() + if h[0] != 0x16 { + continue + } p := 4 + 2 + 32 p += 1 + int(b[p]) - cl := int(binary.BigEndian.Uint16(b[p:p+2])); p += 2 + cl + cl := int(binary.BigEndian.Uint16(b[p : p+2])) + p += 2 + cl p += 1 + int(b[p]) - if p+2 > len(b) { continue } - end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])); p += 2 + if p+2 > len(b) { + continue + } + end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])) + p += 2 ech, sni, total := -1, "", len(b)+5 for p+4 <= end && p+4 <= len(b) { - id := binary.BigEndian.Uint16(b[p:p+2]); ln2 := int(binary.BigEndian.Uint16(b[p+2:p+4])) - if id == 0xfe0d { ech = ln2 } - if id == 0 && ln2 > 5 { sni = string(b[p+9 : p+4+ln2]) } + id := binary.BigEndian.Uint16(b[p : p+2]) + ln2 := int(binary.BigEndian.Uint16(b[p+2 : p+4])) + if id == 0xfe0d { + ech = ln2 + } + if id == 0 && ln2 > 5 { + sni = string(b[p+9 : p+4+ln2]) + } p += 4 + ln2 } - if sni == "" { continue } + if sni == "" { + continue + } seen++ fmt.Printf(" #%d sni=%-42q hello=%4d ECH(0xfe0d)=%d\n", seen, sni, total, ech) } From 455a46e05a8f3ce53b1080c41c0200b5308f57a5 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 12:41:54 +0100 Subject: [PATCH 13/18] Correct the drift finding: the remainder varies by vantage point, not by time The first CI run refutes what the previous commit concluded. Seeing only the laptop's 3921 -> 2619 move, full-remainder-drift.log called google's change a certificate rotation over time. The GitHub runner served 3921 on the same day, so 3921 is still being served -- just not to the laptop. The variable is WHERE the probe runs, not when. That is a stronger result than the one it replaces, and it changes an operational conclusion rather than a detail. FullRemainder is not merely perishable, it is specific to the probing vantage point: a profile measured anywhere else -- in CI, or shipped in a config -- can be over a kilobyte wrong for a given egress even though it was correct where it was taken. The design already required each egress to probe the cover it impersonates on startup; this says inheriting a profile is not a degraded option but a wrong one. cloudflare and microsoft agree across both vantage points, so this is not a general property of CDNs but something specific to what a given cover does -- which is itself the argument for measuring each cover instead of reasoning about covers. The mechanism behind google's behaviour is not established from two samples and the log says so; the consequence does not depend on it. Co-Authored-By: Claude Opus 5 --- docs/full-handshake-carrier.md | 12 ++-- harvest/testdata/full-remainder-drift.log | 77 ++++++++++++++--------- 2 files changed, 55 insertions(+), 34 deletions(-) diff --git a/docs/full-handshake-carrier.md b/docs/full-handshake-carrier.md index 232cfec..8850c89 100644 --- a/docs/full-handshake-carrier.md +++ b/docs/full-handshake-carrier.md @@ -87,11 +87,13 @@ Prerequisite, and all of it already on main before this work: | google | 1215 | 6 | `[3921]` | 5142 | | microsoft | 1215 | 6 | `[32, 8273, 286, 74]` | 9886 | -**Since confirmed the hard way.** Re-probing a day later (`harvest/testdata/full-remainder-drift.log`) -found google's flight had fallen from `[3921]` to `[2619]` — 1302 bytes, a certificate rotation rather -than signature jitter — while microsoft held exactly and cloudflare moved 2, revealing the -3846/3847/3848 range that five samples had reported as a jitter of 1. So the "cannot be a constant" -argument below is no longer just from mechanism, and the useful lifetime of a probed profile is days. +**Since confirmed the hard way, and more sharply than expected** +(`harvest/testdata/full-remainder-drift.log`). Probing from two vantage points on the same day, google +served `[2619]` to a laptop and `[3921]` to a GitHub runner — 1302 bytes apart — while cloudflare and +microsoft agreed at both. So the remainder is not merely perishable, it is **specific to the probing +vantage point**: an egress must probe from *itself*, and a profile measured anywhere else can be over a +kilobyte wrong even where it was correct when taken. That makes the per-egress probing requirement +load-bearing rather than tidy — inheriting a profile is not a degraded option, it is a wrong one. Three consequences: diff --git a/harvest/testdata/full-remainder-drift.log b/harvest/testdata/full-remainder-drift.log index 71210ae..96cb8c8 100644 --- a/harvest/testdata/full-remainder-drift.log +++ b/harvest/testdata/full-remainder-drift.log @@ -1,46 +1,65 @@ -The full-handshake remainder drifts, and by more than jitter. +The full-handshake remainder varies by VANTAGE POINT, not just over time. - tool coverprobe SampleFull, 5 samples per host, via the live CI tests + tool coverprobe SampleFull, 5 samples per host date 2026-09-04 - baseline harvest/testdata/postflight-full-vs-resumed.log, 2026-09-03 + where two vantage points on the same day -- a laptop on a US residential + connection, and a GitHub Actions ubuntu-latest runner + baseline harvest/testdata/postflight-full-vs-resumed.log, 2026-09-03, laptop Why this was recorded: docs/full-handshake-carrier.md asserts that CoverProfile.FullRemainder "cannot be a constant" and must come from a probe against the live upstream. That was an argument from mechanism -- a DER-encoded ECDSA signature varies in length, and certificates rotate. Wiring the live -probes into CI produced the evidence for it within a day. +probes into CI produced the evidence, and a stronger form of it than expected. - 2026-09-03 2026-09-04 delta - cloudflare [3848] [3846] jitter [2] -2, within jitter - google [3921] [2619] jitter [1] -1302 - microsoft [32 8273 286 74] [32 8273 286 74] jitter [0 0 0 0] 0 + 09-03 laptop 09-04 laptop 09-04 CI runner + cloudflare [3848] [3846] jitter [2] [3846] jitter [2] + google [3921] [2619] jitter [1] [3921] jitter [0] + microsoft [32 8273 286 74] same, jitter 0 same, jitter 0 FINDINGS -1. google's certificate flight fell by 1302 bytes in one day. That is not - signature jitter; it is a different certificate chain, most likely a - rotation to a shorter one. Any table constant for this cover would now be - 1302 bytes wrong, and an egress emitting it would be the only host claiming - to be google whose certificate flight is a chain google no longer serves. +1. google served a 2619-byte certificate flight to the laptop and a 3921-byte + one to the CI runner ON THE SAME DAY -- a 1302-byte difference. Two vantage + points, two different chains. -2. cloudflare moved 2 bytes and reported jitter 2, so this run saw the - 3846/3847/3848 range that postflight-full-vs-resumed.log recorded as a - jitter of 1 from 5 samples. That is the documented consequence of a sampled - jitter being a FLOOR rather than a range: more samples widen it, and - narrowing it on the strength of one run would be wrong. + A FIRST READING OF THIS WAS WRONG and is corrected here. Seeing only the + laptop's 3921 -> 2619 move, this log originally called it a certificate + rotation over time. The CI run refutes that: 3921 is still being served, + just not to the laptop. The variable is WHERE the probe runs, not when. -3. microsoft held exactly, all four records, jitter zero. Its RSA signature is - fixed-length, and its chain did not rotate in this window. Consistent with - the earlier measurement. + The mechanism is not established from two samples. Plausible causes are a + different google edge with a different chain, geographic or ASN-based + certificate selection, or a difference in what the two clients offered. + Worth pinning down, but the operational consequence does not depend on which. + +2. cloudflare and microsoft agree across both vantage points. So this is not a + general property of CDNs, it is specific to what a given cover does -- which + is itself the argument for measuring each cover rather than reasoning about + covers. + +3. cloudflare moved 2 bytes from the 09-03 measurement and reported jitter 2 at + both vantage points, so both runs saw the 3846/3847/3848 range that five + samples had reported as a jitter of 1. That is the documented consequence of + a sampled jitter being a FLOOR rather than a range: more samples widen it, + and narrowing it on the strength of one run would be wrong. CONSEQUENCE +This is the sharper version of "FullRemainder cannot be a table constant." It +is not merely perishable -- it is SPECIFIC TO THE PROBING VANTAGE POINT. An +egress must probe from itself. A profile measured anywhere else, including one +measured in CI or shipped in a config, can be over a kilobyte wrong for that +egress even when it was correct where it was taken. + +The design already required per-egress probing on startup. This says that +requirement is load-bearing rather than tidy: inheriting a profile is not a +degraded option, it is a wrong one. + Nothing in the shipped code breaks, because FullRemainder is empty in the table -by design and every emitter is gated on CanEmitFullHandshake. What this does -settle is the cadence question the design left open: a probed full profile is -not a one-time startup measurement that can be cached indefinitely. google's -drift says the useful lifetime of one is days, not weeks. - -It also means the numbers quoted in docs and logs are snapshots. They are -correct as measurements and wrong as expectations, which is why the live tests -assert ServerHelloFullLen -- a protocol constant -- and NOT the remainder. +by design and every emitter is gated on CanEmitFullHandshake. + +It also means the numbers in the docs and logs are snapshots of one vantage +point. They are correct as measurements and wrong as expectations, which is why +the live CI tests assert ServerHelloFullLen -- a protocol constant, 1215 from +all three covers at both vantage points -- and NOT the remainder. From b58fe9d9be16c07bf806d692e43a0cf02edc7085 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 17:24:59 +0100 Subject: [PATCH 14/18] Close the Reset race, and stop sweep panicking on a short record Both from the CodeRabbit review on #3, and both real. A ContactMemory decision and its recording straddle the ENTIRE handshake, so a Reset can land between them and the recording that follows would undo it. The entry reappears for an address pair the new observer has no history of, and the next connection resumes with no predecessor it can see -- the exact distinguisher this mechanism exists to remove. The local address usually changes with the network and saves us, but not always: two networks can both hand out 192.168.1.5, which is precisely the case Reset exists to cover, so the race turns a documented-weak heuristic into a silently broken one. needsFull now returns the generation its decision was made under, Reset increments it, and record drops a write whose generation does not match. Dropping it costs one extra full handshake, which is the direction everything in this file errs in. Two regression tests: the interleaving directly, and Reset racing record across goroutines under -race, where either order must leave nothing behind. Removing the guard fails both; removing Reset's increment fails the first. harvest/cmd/sweep listens on a socket and hand-rolled a ClientHello parse that indexed b[38] before checking any length, with io.ReadFull's error discarded -- so anything that could reach the port could panic it. Verified rather than assumed: the old binary dies with "index out of range [0] with length 1" on a one-byte handshake body, the new one survives that and every other short record tried. Fixed by deleting the parser rather than adding bounds checks to it. tw.ParseClientHello validates every fixed-width and length-delimited field, is what the rest of the repo is tested against, and is less code than the checks that were missing. The tool is under harvest/, so importing the library costs nothing that matters. Co-Authored-By: Claude Opus 5 --- contacts.go | 44 +++++++++++-- contacts_test.go | 132 +++++++++++++++++++++++++++++++------- handshake.go | 16 +++-- handshake_test.go | 2 +- harvest/cmd/sweep/main.go | 41 ++++++------ 5 files changed, 183 insertions(+), 52 deletions(-) diff --git a/contacts.go b/contacts.go index f143c60..e034a69 100644 --- a/contacts.go +++ b/contacts.go @@ -33,6 +33,9 @@ type ContactMemory struct { horizon time.Duration max int seen map[contactKey]time.Time + // gen increments on every Reset, so a handshake that was decided before a + // reset cannot write its result after one. See record. + gen uint64 } // contactKey pairs the egress address with the local one. @@ -105,6 +108,18 @@ func (m *ContactMemory) Reset() { m.mu.Lock() defer m.mu.Unlock() m.seen = make(map[contactKey]time.Time) + m.gen++ +} + +// generation reports the current reset generation. Test-facing: production +// callers get it from needsFull, paired with the decision it belongs to. +func (m *ContactMemory) generation() uint64 { + if m == nil { + return 0 + } + m.mu.Lock() + defer m.mu.Unlock() + return m.gen } // Tracked reports how many contacts are remembered. @@ -136,9 +151,12 @@ func addrKey(a net.Addr) string { // both will do a full handshake. That is not a race worth closing: it is what a // browser does on every page load, where a parallel burst opens several // connections to one origin before any of them has a ticket to resume with. -func (m *ContactMemory) needsFull(local, remote net.Addr, now time.Time) bool { +// It also returns the generation the decision was made under, which record +// requires back. A decision and its recording straddle the whole handshake, so +// a Reset can land between them; the generation is what makes that observable. +func (m *ContactMemory) needsFull(local, remote net.Addr, now time.Time) (bool, uint64) { if m == nil { - return false + return false, 0 } m.mu.Lock() defer m.mu.Unlock() @@ -149,18 +167,36 @@ func (m *ContactMemory) needsFull(local, remote net.Addr, now time.Time) bool { // green. That is deliberate: eviction bounds the state and this comparison // bounds the answer, so making eviction periodic or lazy later cannot // silently extend how long a relationship is trusted for. - return !ok || now.Sub(last) >= m.horizon + return !ok || now.Sub(last) >= m.horizon, m.gen } // record notes a COMPLETED full handshake. Only completed ones count: a // handshake that failed established no relationship for a later resumption to // continue. -func (m *ContactMemory) record(local, remote net.Addr, now time.Time) { +// +// gen must be the value needsFull returned when this handshake's shape was +// chosen. A mismatch means Reset ran while the handshake was in flight, and the +// write is DROPPED. +// +// That interval is the whole handshake, so the window is not small. Without the +// guard, a network change detected mid-handshake would be undone by the +// recording that followed it: the entry would reappear for an address pair the +// new observer has no history of, and the next connection would resume with no +// predecessor it can see. The local address usually changes with the network +// and saves us, but not always -- two networks can both hand out 192.168.1.5, +// which is exactly the case Reset exists to cover. +// +// Dropping the write costs one extra full handshake on the next connection, +// which is the direction everything here errs in. +func (m *ContactMemory) record(local, remote net.Addr, now time.Time, gen uint64) { if m == nil { return } m.mu.Lock() defer m.mu.Unlock() + if gen != m.gen { + return + } m.seen[contactKey{addrKey(local), addrKey(remote)}] = now m.evictLocked(now) } diff --git a/contacts_test.go b/contacts_test.go index 1678ee5..97cff49 100644 --- a/contacts_test.go +++ b/contacts_test.go @@ -2,6 +2,7 @@ package twiddle import ( "net" + "sync" "testing" "time" ) @@ -12,23 +13,31 @@ type addr string func (a addr) Network() string { return "tcp" } func (a addr) String() string { return string(a) } +// mustNeedFull drops the generation, for the tests that only assert the +// decision. The generation itself is covered by +// TestContactMemoryIgnoresARecordFromBeforeAReset. +func mustNeedFull(m *ContactMemory, local, remote net.Addr, now time.Time) bool { + full, _ := m.needsFull(local, remote, now) + return full +} + // The rule, in one test: full on first contact, resumed afterwards. func TestContactMemoryIsFullOnFirstContactAndResumedAfter(t *testing.T) { m := NewContactMemory(time.Hour, 0) now := time.Now() local, remote := addr("10.0.0.2:51000"), addr("203.0.113.9:443") - if !m.needsFull(local, remote, now) { + if !mustNeedFull(m, local, remote, now) { t.Fatal("first contact with an egress did not ask for a full handshake") } // Asking is not recording: until the handshake completes, the relationship // does not exist and the answer must not change. - if !m.needsFull(local, remote, now) { + if !mustNeedFull(m, local, remote, now) { t.Error("the answer changed before any handshake was recorded") } - m.record(local, remote, now) - if m.needsFull(local, remote, now.Add(time.Minute)) { + m.record(local, remote, now, m.generation()) + if mustNeedFull(m, local, remote, now.Add(time.Minute)) { t.Error("a second connection to a recorded egress asked for another full handshake") } } @@ -38,9 +47,9 @@ func TestContactMemoryIsFullOnFirstContactAndResumedAfter(t *testing.T) { func TestContactMemoryIgnoresPorts(t *testing.T) { m := NewContactMemory(time.Hour, 0) now := time.Now() - m.record(addr("10.0.0.2:51000"), addr("203.0.113.9:443"), now) + m.record(addr("10.0.0.2:51000"), addr("203.0.113.9:443"), now, m.generation()) - if m.needsFull(addr("10.0.0.2:52222"), addr("203.0.113.9:443"), now) { + if mustNeedFull(m, addr("10.0.0.2:52222"), addr("203.0.113.9:443"), now) { t.Error("a new source port was treated as a new contact") } } @@ -51,15 +60,15 @@ func TestContactMemoryReFullsPastTheHorizon(t *testing.T) { m := NewContactMemory(time.Hour, 0) base := time.Now() local, remote := addr("10.0.0.2:51000"), addr("203.0.113.9:443") - m.record(local, remote, base) + m.record(local, remote, base, m.generation()) - if m.needsFull(local, remote, base.Add(59*time.Minute)) { + if mustNeedFull(m, local, remote, base.Add(59*time.Minute)) { t.Error("re-fulled inside the horizon") } - if !m.needsFull(local, remote, base.Add(time.Hour)) { + if !mustNeedFull(m, local, remote, base.Add(time.Hour)) { t.Error("did not re-full at the horizon") } - if !m.needsFull(local, remote, base.Add(3*time.Hour)) { + if !mustNeedFull(m, local, remote, base.Add(3*time.Hour)) { t.Error("did not re-full past the horizon") } } @@ -74,14 +83,14 @@ func TestContactMemoryDropsStatePastTheHorizon(t *testing.T) { base := time.Now() for i := 1; i <= 20; i++ { m.record(addr("10.0.0.2:1"), addr(net.JoinHostPort( - net.IPv4(203, 0, 113, byte(i)).String(), "443")), base) + net.IPv4(203, 0, 113, byte(i)).String(), "443")), base, m.generation()) } if m.Tracked() != 20 { t.Fatalf("tracking %d contacts, want 20", m.Tracked()) } // Any later call runs eviction, and every entry is now stale. - m.needsFull(addr("10.0.0.2:1"), addr("198.51.100.1:443"), base.Add(2*time.Hour)) + mustNeedFull(m, addr("10.0.0.2:1"), addr("198.51.100.1:443"), base.Add(2*time.Hour)) if got := m.Tracked(); got != 0 { t.Errorf("tracking %d contacts after the horizon passed, want 0", got) } @@ -93,12 +102,12 @@ func TestContactMemoryDropsStatePastTheHorizon(t *testing.T) { func TestContactMemoryKeysOnBothEnds(t *testing.T) { m := NewContactMemory(time.Hour, 0) now := time.Now() - m.record(addr("10.0.0.2:51000"), addr("203.0.113.9:443"), now) + m.record(addr("10.0.0.2:51000"), addr("203.0.113.9:443"), now, m.generation()) - if !m.needsFull(addr("10.0.0.2:51000"), addr("198.51.100.7:443"), now) { + if !mustNeedFull(m, addr("10.0.0.2:51000"), addr("198.51.100.7:443"), now) { t.Error("a different egress was treated as already contacted") } - if !m.needsFull(addr("192.168.5.4:51000"), addr("203.0.113.9:443"), now) { + if !mustNeedFull(m, addr("192.168.5.4:51000"), addr("203.0.113.9:443"), now) { t.Error("a new local address was treated as already contacted; roaming would emit a bare resumption") } } @@ -110,8 +119,8 @@ func TestContactMemoryResetForcesFullAgain(t *testing.T) { m := NewContactMemory(time.Hour, 0) now := time.Now() local, remote := addr("192.168.1.5:51000"), addr("203.0.113.9:443") - m.record(local, remote, now) - if m.needsFull(local, remote, now) { + m.record(local, remote, now, m.generation()) + if mustNeedFull(m, local, remote, now) { t.Fatal("recorded contact still asked for a full handshake") } @@ -119,7 +128,7 @@ func TestContactMemoryResetForcesFullAgain(t *testing.T) { if m.Tracked() != 0 { t.Errorf("Reset left %d contacts", m.Tracked()) } - if !m.needsFull(local, remote, now) { + if !mustNeedFull(m, local, remote, now) { t.Error("after Reset the same address pair did not ask for a full handshake") } } @@ -134,18 +143,19 @@ func TestContactMemoryEvictionFailsTowardFull(t *testing.T) { base := time.Now() first := addr("203.0.113.1:443") - m.record(addr("10.0.0.2:1"), first, base) + m.record(addr("10.0.0.2:1"), first, base, m.generation()) for i := 0; i < max*4; i++ { m.record(addr("10.0.0.2:1"), addr(net.JoinHostPort( - net.IPv4(198, 51, 100, byte(i%250+1)).String(), "443")), base.Add(time.Duration(i+1)*time.Second)) + net.IPv4(198, 51, 100, byte(i%250+1)).String(), "443")), + base.Add(time.Duration(i+1)*time.Second), m.generation()) } if got := m.Tracked(); got > max { t.Errorf("tracking %d contacts, above the %d bound", got, max) } // The oldest entry is gone, and its absence asks for a full handshake -- // the safe direction, not a reopened hole. - if !m.needsFull(addr("10.0.0.2:1"), first, base.Add(time.Minute)) { + if !mustNeedFull(m, addr("10.0.0.2:1"), first, base.Add(time.Minute)) { t.Error("an evicted contact was still treated as already contacted") } } @@ -154,10 +164,10 @@ func TestContactMemoryEvictionFailsTowardFull(t *testing.T) { // for a full handshake, and never panic on record. func TestNilContactMemoryIsInert(t *testing.T) { var m *ContactMemory - if m.needsFull(addr("a:1"), addr("b:2"), time.Now()) { + if mustNeedFull(m, addr("a:1"), addr("b:2"), time.Now()) { t.Error("a nil memory asked for a full handshake") } - m.record(addr("a:1"), addr("b:2"), time.Now()) // must not panic + m.record(addr("a:1"), addr("b:2"), time.Now(), m.generation()) // must not panic m.Reset() if m.Tracked() != 0 || m.Horizon() != 0 { t.Error("a nil memory reported state") @@ -170,3 +180,79 @@ func TestContactMemoryDefaults(t *testing.T) { t.Errorf("horizon %v, want the default %v", m.Horizon(), DefaultContactHorizon) } } + +// The race the generation guard exists for. +// +// A decision and its recording straddle the entire handshake, so a Reset can +// land between them. Without the guard the recording that follows would undo +// the reset: the entry reappears for an address pair the NEW observer has no +// history of, and the next connection resumes with no predecessor it can see. +// +// The local address usually changes with the network and saves us, but not +// always -- two networks can both hand out 192.168.1.5, which is exactly the +// case Reset exists to cover. So this test holds the address pair fixed. +func TestContactMemoryIgnoresARecordFromBeforeAReset(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + now := time.Now() + local, remote := addr("192.168.1.5:51000"), addr("203.0.113.9:443") + + // The shape is decided, and the generation captured with it. + full, gen := m.needsFull(local, remote, now) + if !full { + t.Fatal("first contact did not ask for a full handshake") + } + + // The network changes while the handshake is in flight. + m.Reset() + + // The handshake completes and tries to record what it decided. + m.record(local, remote, now, gen) + + if m.Tracked() != 0 { + t.Errorf("a handshake decided before the reset wrote %d contacts after it", m.Tracked()) + } + if !mustNeedFull(m, local, remote, now) { + t.Error("the next connection would resume, against an observer with no record of the handshake that preceded it") + } + + // And the guard is not a permanent block: a handshake decided AFTER the + // reset records normally. + full, gen = m.needsFull(local, remote, now) + if !full { + t.Fatal("post-reset contact did not ask for a full handshake") + } + m.record(local, remote, now, gen) + if m.Tracked() != 1 { + t.Errorf("tracking %d contacts after a valid record, want 1", m.Tracked()) + } + if mustNeedFull(m, local, remote, now.Add(time.Minute)) { + t.Error("a handshake recorded after the reset was not honoured") + } +} + +// The same interleaving through Client, concurrently, under -race: a Reset +// landing during the handshake I/O must not leave the pair recorded. +func TestContactMemoryResetDuringHandshakeIsNotUndone(t *testing.T) { + m := NewContactMemory(time.Hour, 0) + now := time.Now() + local, remote := addr("192.168.1.5:51000"), addr("203.0.113.9:443") + + full, gen := m.needsFull(local, remote, now) + if !full { + t.Fatal("first contact did not ask for a full handshake") + } + + // Reset and record racing, as they would across two goroutines. + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); m.Reset() }() + go func() { defer wg.Done(); m.record(local, remote, now, gen) }() + wg.Wait() + + // Either order is acceptable ONLY if the outcome is safe. If Reset ran + // first the generation stops the write; if record ran first the reset + // clears it. Both leave nothing behind, which is the point. + if m.Tracked() != 0 { + t.Errorf("tracking %d contacts after a reset raced the recording, want 0", m.Tracked()) + } +} diff --git a/handshake.go b/handshake.go index 9bacc0e..df8ecc8 100644 --- a/handshake.go +++ b/handshake.go @@ -105,8 +105,16 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { // been probed first. The degradation is not recorded, so it keeps trying // rather than latching. full := cfg.FullHandshake - if !full && cfg.Contacts.needsFull(raw.LocalAddr(), raw.RemoteAddr(), time.Now()) { - full = cfg.Cover.CanEmitFullHandshake() && len(FullHandshakeCarriers(cfg.Pool)) > 0 + // contactGen is the generation the shape was decided under. record refuses a + // write from a different one, so a Reset during the handshake cannot be + // undone by the recording that follows it. + var contactGen uint64 + if cfg.Contacts != nil { + wantFull, gen := cfg.Contacts.needsFull(raw.LocalAddr(), raw.RemoteAddr(), time.Now()) + contactGen = gen + if !full && wantFull { + full = cfg.Cover.CanEmitFullHandshake() && len(FullHandshakeCarriers(cfg.Pool)) > 0 + } } // The remainder record COUNT is what the client reads, so the two shapes @@ -197,8 +205,8 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { // Recorded only now, with the opening complete. A full handshake that // failed established no relationship for a later resumption to continue, so // recording it would be the one direction that hurts. - if full { - cfg.Contacts.record(raw.LocalAddr(), raw.RemoteAddr(), time.Now()) + if full && cfg.Contacts != nil { + cfg.Contacts.record(raw.LocalAddr(), raw.RemoteAddr(), time.Now(), contactGen) } return conn, next, nil } diff --git a/handshake_test.go b/handshake_test.go index a8cc7b6..932cd20 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -987,7 +987,7 @@ func TestContactsRecordOnlyCompletedHandshakes(t *testing.T) { if mem.Tracked() != 0 { t.Errorf("a failed full handshake was recorded (%d contacts)", mem.Tracked()) } - if !mem.needsFull(local, remote, time.Now()) { + if !mustNeedFull(mem, local, remote, time.Now()) { t.Error("after a FAILED full handshake the next connection would resume, with no completed predecessor for a censor to have seen") } } diff --git a/harvest/cmd/sweep/main.go b/harvest/cmd/sweep/main.go index b1f398e..7ed5010 100644 --- a/harvest/cmd/sweep/main.go +++ b/harvest/cmd/sweep/main.go @@ -7,6 +7,8 @@ import ( "fmt" "io" "net" + + tw "github.com/getlantern/twiddle" ) func main() { @@ -25,32 +27,31 @@ func main() { continue } b := make([]byte, int(binary.BigEndian.Uint16(h[3:5]))) - io.ReadFull(c, b) + // The error was ignored here, and the hand-rolled parse below indexed + // b[38] before checking any length -- so a peer sending a short + // handshake record panicked this tool. It listens on a socket, so + // "a peer" is anything that can reach it. + if _, e := io.ReadFull(c, b); e != nil { + c.Close() + continue + } c.Close() if h[0] != 0x16 { continue } - p := 4 + 2 + 32 - p += 1 + int(b[p]) - cl := int(binary.BigEndian.Uint16(b[p : p+2])) - p += 2 + cl - p += 1 + int(b[p]) - if p+2 > len(b) { + + // The library's parser rather than a second hand-rolled one. It + // validates every fixed-width and length-delimited field, it is what + // the rest of the repo is tested against, and it is less code than the + // bounds checks the previous version was missing. + hello, e := tw.ParseClientHello(append(h, b...)) + if e != nil { continue } - end := p + 2 + int(binary.BigEndian.Uint16(b[p:p+2])) - p += 2 - ech, sni, total := -1, "", len(b)+5 - for p+4 <= end && p+4 <= len(b) { - id := binary.BigEndian.Uint16(b[p : p+2]) - ln2 := int(binary.BigEndian.Uint16(b[p+2 : p+4])) - if id == 0xfe0d { - ech = ln2 - } - if id == 0 && ln2 > 5 { - sni = string(b[p+9 : p+4+ln2]) - } - p += 4 + ln2 + sni, total := hello.SNI(), len(h)+len(b) + ech := -1 + if e := hello.Find(tw.ExtECH); e != nil { + ech = len(e.Data) } if sni == "" { continue From 566dbef9ceae2ce48d25b054daeaacfe9a1b6123 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 17:35:18 +0100 Subject: [PATCH 15/18] Degrade when the credential has no companion, and type the psk Both from the Copilot review on #3. The contact memory checked the cover and the pool before flipping to a full handshake, but not the CREDENTIAL -- and the credential is the one that will be missing in practice. CredentialFromWire leaves the companion ticket nil, so every client provisioned before lantern-cloud emits full_ticket is resumption-only. Contacts would flip full to true and Twiddle would then refuse the connection, so enabling Contacts ahead of provisioning would have broken every connection instead of quietly resuming. That contradicts the documented degradation and the two existing degrade tests missed it, because both exercised a cover or pool that could not back the shape while holding the credential valid. TestContactsDegradeWhenTheCredentialHasNoCompanion builds its credential through CredentialFromWire so it is the real case, and it fails with the condition removed. SetECHTicketAuth took psk as a slice, which HMAC accepts at any length. A nil or short psk would emit a well-formed hello that simply never authenticates, surfacing as a MAC failure on a server that is not the one holding the bug. Taking [32]byte makes that a compile error instead, which is how Credential, TicketKey and SetTicketAuth already carry key material -- stronger than the length check that was suggested, and it removes the failure mode rather than reporting it. Co-Authored-By: Claude Opus 5 --- echcarrier.go | 11 +++++++++-- echcarrier_test.go | 18 +++++++++--------- handshake.go | 12 +++++++++++- handshake_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ twiddle.go | 2 +- 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/echcarrier.go b/echcarrier.go index ebed2ce..ee4b61c 100644 --- a/echcarrier.go +++ b/echcarrier.go @@ -152,12 +152,19 @@ func FullHandshakeCarriers(pool [][]byte) [][]byte { // random directly and the ECH payload through rerandECHGrease, so this must // follow it and not merely follow SetKeyShare. // +// psk is [32]byte rather than a slice on purpose. A slice would let a caller +// pass a nil or short psk, which HMAC accepts silently -- the emitted hello +// would then be well formed and simply never authenticate, surfacing as a MAC +// failure on a server that is not the one holding the bug. The array makes that +// a compile error instead, which is how Credential, TicketKey and SetTicketAuth +// already carry key material. +// // Unlike the binder, which mirrors RFC 8446's Truncate() and therefore covers // only a prefix, this MAC covers the whole hello. There is no truncation rule // to honour here because the field is not a TLS binder, so the stronger // construction is also the simpler one: SNI, key_share and the ECH padding are // all bound. -func (h *ClientHello) SetECHTicketAuth(ticket []byte, psk []byte) error { +func (h *ClientHello) SetECHTicketAuth(ticket []byte, psk [32]byte) error { if len(ticket) != FullTicketLen { return fmt.Errorf("twiddle: full-handshake ticket is %d bytes, want %d", len(ticket), FullTicketLen) } @@ -186,7 +193,7 @@ func (h *ClientHello) SetECHTicketAuth(ticket []byte, psk []byte) error { } h.Random = [32]byte{} - m := hmac.New(sha256.New, fullMACKey(psk)) + m := hmac.New(sha256.New, fullMACKey(psk[:])) m.Write(h.Marshal()) copy(h.Random[:], m.Sum(nil)) return nil diff --git a/echcarrier_test.go b/echcarrier_test.go index 66f69fe..b9d9888 100644 --- a/echcarrier_test.go +++ b/echcarrier_test.go @@ -43,7 +43,7 @@ func TestECHCarrierRoundTrip(t *testing.T) { if _, err := h.SetKeyShare(); err != nil { t.Fatal(err) } - if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + if err := h.SetECHTicketAuth(full, cred.PSK); err != nil { t.Fatal(err) } @@ -144,7 +144,7 @@ func TestECHCarrierMACCoversTheWholeHello(t *testing.T) { if _, err := h.SetKeyShare(); err != nil { t.Fatal(err) } - if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + if err := h.SetECHTicketAuth(full, cred.PSK); err != nil { t.Fatal(err) } if _, err := VerifyECHTicketAuth(h, k, time.Hour); err != nil { @@ -170,7 +170,7 @@ func TestECHCarrierRejectsAForeignPSK(t *testing.T) { var wrong [32]byte copy(wrong[:], cred.PSK[:]) wrong[0] ^= 0x01 - if err := h.SetECHTicketAuth(full, wrong[:]); err != nil { + if err := h.SetECHTicketAuth(full, wrong); err != nil { t.Fatal(err) } if _, err := VerifyECHTicketAuth(h, k, time.Hour); err == nil { @@ -192,7 +192,7 @@ func TestECHCarrierAndResumptionAreExclusive(t *testing.T) { if err := h.SetTicketAuth(cred, 32); err != nil { t.Fatal(err) } - if err := h.SetECHTicketAuth(full, cred.PSK[:]); err == nil { + if err := h.SetECHTicketAuth(full, cred.PSK); err == nil { t.Error("SetECHTicketAuth accepted a hello that still carries pre_shared_key") } if _, err := VerifyECHTicketAuth(h, k, time.Hour); err == nil { @@ -214,7 +214,7 @@ func TestECHCarrierRejectsAnExpiredTicket(t *testing.T) { if _, err := h.SetKeyShare(); err != nil { t.Fatal(err) } - if err := h.SetECHTicketAuth(old, cred.PSK[:]); err != nil { + if err := h.SetECHTicketAuth(old, cred.PSK); err != nil { t.Fatal(err) } if _, err := VerifyECHTicketAuth(h, k, 24*time.Hour); err == nil { @@ -240,7 +240,7 @@ func TestECHCarrierRefusesAPayloadTooSmallToCarryATicket(t *testing.T) { break } } - if err := h.SetECHTicketAuth(full, cred.PSK[:]); err == nil { + if err := h.SetECHTicketAuth(full, cred.PSK); err == nil { t.Error("a hello with no ECH extension was accepted as a carrier") } }) @@ -255,7 +255,7 @@ func TestECHCarrierRefusesAPayloadTooSmallToCarryATicket(t *testing.T) { if n, err := h.ECHPayloadLen(); err != nil || n != short { t.Fatalf("payload is %d (%v), want %d", n, err, short) } - err := h.SetECHTicketAuth(full, cred.PSK[:]) + err := h.SetECHTicketAuth(full, cred.PSK) if err == nil { t.Fatal("a payload too small for the ticket was accepted") } @@ -280,7 +280,7 @@ func TestECHCarrierRefreshesThePaddingItself(t *testing.T) { t.Fatal(err) } // Deliberately NO Rerandomize: the padding must not come from the pool. - if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + if err := h.SetECHTicketAuth(full, cred.PSK); err != nil { t.Fatal(err) } pay, err := echPayload(h.Find(ExtECH)) @@ -316,7 +316,7 @@ func TestECHCarrierEmitsAFullHandshakeShape(t *testing.T) { if _, err := h.SetKeyShare(); err != nil { t.Fatal(err) } - if err := h.SetECHTicketAuth(full, cred.PSK[:]); err != nil { + if err := h.SetECHTicketAuth(full, cred.PSK); err != nil { t.Fatal(err) } diff --git a/handshake.go b/handshake.go index df8ecc8..b1b32a0 100644 --- a/handshake.go +++ b/handshake.go @@ -113,7 +113,17 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { wantFull, gen := cfg.Contacts.needsFull(raw.LocalAddr(), raw.RemoteAddr(), time.Now()) contactGen = gen if !full && wantFull { - full = cfg.Cover.CanEmitFullHandshake() && len(FullHandshakeCarriers(cfg.Pool)) > 0 + // All three have to be able to back the shape, and the CREDENTIAL + // is the one that will be missing in practice: CredentialFromWire + // leaves the companion nil, so every client provisioned before + // lantern-cloud emits full_ticket is resumption-only. Omitting this + // check made Contacts flip full to true and then fail in Twiddle, + // refusing the connection instead of degrading -- which would have + // broken every connection the moment Contacts was enabled ahead of + // provisioning. + full = cfg.Cover.CanEmitFullHandshake() && + len(FullHandshakeCarriers(cfg.Pool)) > 0 && + len(cfg.Credential.FullTicket) == FullTicketLen } } diff --git a/handshake_test.go b/handshake_test.go index 932cd20..bf246a0 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -991,3 +991,43 @@ func TestContactsRecordOnlyCompletedHandshakes(t *testing.T) { t.Error("after a FAILED full handshake the next connection would resume, with no completed predecessor for a censor to have seen") } } + +// The degradation case that will actually happen in production, and the one the +// other two degrade tests missed. +// +// CredentialFromWire leaves the companion ticket nil, so every client +// provisioned before lantern-cloud emits full_ticket is resumption-only. If the +// contact memory can flip to a full handshake without checking the credential, +// Twiddle then refuses the connection -- so enabling Contacts ahead of +// provisioning would break every connection rather than quietly resuming. +func TestContactsDegradeWhenTheCredentialHasNoCompanion(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + mem := NewContactMemory(time.Hour, 0) + + issued, err := k.Issue(340, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + // Exactly what CredentialFromWire produces. + cred, err := CredentialFromWire(issued.Ticket, issued.PSK[:]) + if err != nil { + t.Fatal(err) + } + if cred.FullTicket != nil { + t.Fatal("CredentialFromWire produced a companion ticket; this test proves nothing") + } + + cf, sf, err := dialOnce(t, k, cover, + ClientConfig{Pool: pool(t), Cover: cover, Credential: cred, Contacts: mem}, + NewReplayCache(64, time.Hour)) + if err != nil { + t.Fatalf("a resumption-only credential was refused instead of degrading: %v", err) + } + if cf || sf { + t.Errorf("client-full=%v server-full=%v from a credential with no companion ticket", cf, sf) + } + if mem.Tracked() != 0 { + t.Error("a degraded connection was recorded") + } +} diff --git a/twiddle.go b/twiddle.go index 2c587a2..53c39ea 100644 --- a/twiddle.go +++ b/twiddle.go @@ -458,7 +458,7 @@ func Twiddle(harvested []byte, opt Options) (wire []byte, eph *ecdh.PrivateKey, return nil, nil, fmt.Errorf("twiddle: credential carries a %d-byte full ticket, want %d; it cannot open a full handshake", len(opt.Credential.FullTicket), FullTicketLen) } - if err := h.SetECHTicketAuth(opt.Credential.FullTicket, opt.Credential.PSK[:]); err != nil { + if err := h.SetECHTicketAuth(opt.Credential.FullTicket, opt.Credential.PSK); err != nil { return nil, nil, err } return h.Marshal(), eph, nil From 17cf6ff6ccb12446e97d0daf06b1abd5a9669a7e Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 17:53:18 +0100 Subject: [PATCH 16/18] Report a nil connection instead of panicking on it From the Copilot review on #3. The Contacts decision reads raw.LocalAddr(), and that put a raw dereference inside Client's config-validation region -- so with Contacts set, Client(nil, cfg) panicked where it previously returned a config error. Confirmed by removing the guard: the test panics with "invalid memory address or nil pointer dereference". Client(nil, cfg) is not an accident, it is how seven call sites across three test files exercise config validation without a socket, so the implicit contract was that config errors are reported before any I/O. The guard is therefore placed AFTER every config check rather than at the top, which keeps a config error reported as a config error and turns only the genuine nil-conn case into a clear message. The test asserts both halves, because a fix that merely stopped the panic could just as easily have started reporting "nil connection" for a mismatched credential. Moving the explicit-request pool check up alongside the profile check is what makes that ordering work, and reads better anyway: both validations of an explicit FullHandshake now sit together, above the point where raw is first needed. Server has the same latent property and is left alone: nothing here added a dereference to its validation region, and an up-front check there would break the config test that depends on the same pattern. Co-Authored-By: Claude Opus 5 --- handshake.go | 23 ++++++++++++++++++++--- handshake_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/handshake.go b/handshake.go index b1b32a0..832fca3 100644 --- a/handshake.go +++ b/handshake.go @@ -89,12 +89,27 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { if len(cfg.Credential.Ticket) != cfg.Cover.TicketLen { return nil, nil, fmt.Errorf("twiddle: credential ticket length %d does not match cover %d", len(cfg.Credential.Ticket), cfg.Cover.TicketLen) } - // Refused here rather than on the wire. A client that opened a full + // An EXPLICIT request is validated here rather than on the wire, and it + // fails where a Contacts-driven choice degrades. A client that opened a full // handshake against a cover with no measured full profile would get a // guessed certificate flight back, which is worse than not offering the // shape at all. - if cfg.FullHandshake && !cfg.Cover.CanEmitFullHandshake() { - return nil, nil, fmt.Errorf("twiddle: cover %s has no measured full-handshake profile", cfg.Cover.Host) + if cfg.FullHandshake { + if !cfg.Cover.CanEmitFullHandshake() { + return nil, nil, fmt.Errorf("twiddle: cover %s has no measured full-handshake profile", cfg.Cover.Host) + } + if len(FullHandshakeCarriers(cfg.Pool)) == 0 { + return nil, nil, errors.New("twiddle: no hello in the pool has an ECH payload large enough to carry a full-handshake ticket") + } + } + + // raw is dereferenced from here on, so a nil one becomes an error rather + // than a panic. Deliberately AFTER every config check: Client(nil, cfg) is + // how several tests exercise config validation without a socket, and that + // ordering keeps a config error reported as a config error. It matters + // because the Contacts decision below reads raw.LocalAddr(). + if raw == nil { + return nil, nil, errors.New("twiddle: nil connection") } // An explicit request is honoured; otherwise the contact memory decides. @@ -140,6 +155,8 @@ func Client(raw net.Conn, cfg ClientConfig) (*Conn, *Credential, error) { // others, depending on the draw. candidates := cfg.Pool if full { + // Non-empty either way by now: an explicit request was checked above, + // and a Contacts-driven one only set full when carriers exist. if candidates = FullHandshakeCarriers(cfg.Pool); len(candidates) == 0 { return nil, nil, errors.New("twiddle: no hello in the pool has an ECH payload large enough to carry a full-handshake ticket") } diff --git a/handshake_test.go b/handshake_test.go index bf246a0..ec76985 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -1031,3 +1031,51 @@ func TestContactsDegradeWhenTheCredentialHasNoCompanion(t *testing.T) { t.Error("a degraded connection was recorded") } } + +// Client(nil, cfg) is how several tests exercise config validation without a +// socket, so raw is not dereferenced until every config check has run. The +// Contacts decision reads raw.LocalAddr(), which put a dereference inside that +// region -- with Contacts set, a nil conn panicked instead of erroring. +// +// Both halves of the contract are asserted, because a fix that only stopped the +// panic could easily have reported "nil connection" for a config error too. +func TestClientReportsANilConnectionRatherThanPanicking(t *testing.T) { + k := ticketKey(t) + cover := fullCover(t) + cred, err := k.Issue(350, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + + t.Run("with Contacts set, a nil conn errors", func(t *testing.T) { + _, _, err := Client(nil, ClientConfig{ + Pool: pool(t), Cover: cover, Credential: cred, + Contacts: NewContactMemory(time.Hour, 0), + }) + if err == nil { + t.Fatal("a nil connection was accepted") + } + if !contains(err.Error(), "nil connection") { + t.Errorf("unhelpful error: %v", err) + } + }) + + t.Run("a config error still wins over the nil conn", func(t *testing.T) { + // Same nil conn, but the credential does not match the cover. The + // config error is the useful one and must be what comes back. + bad, err := k.Issue(351, cover.TicketLen+1) + if err != nil { + t.Fatal(err) + } + _, _, err = Client(nil, ClientConfig{ + Pool: pool(t), Cover: cover, Credential: bad, + Contacts: NewContactMemory(time.Hour, 0), + }) + if err == nil { + t.Fatal("a mismatched credential was accepted") + } + if !contains(err.Error(), "ticket length") { + t.Errorf("got %q, want the config error rather than the nil-conn one", err) + } + }) +} From e38e3af533a65ee45746e09c5501d67b77497442 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 19:05:29 +0100 Subject: [PATCH 17/18] Make the rotation length checks exact, and correct what they defend against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prompted by a question on #3 about the readTickets push-back: can application data really be mistaken for a credential? Measuring it produced a worse answer than the comment claimed, and a better fix. First, the comment was wrong about the threat. It implied an adversarial risk. The inner content type lives inside the AEAD plaintext (writeRecord seals payload ‖ typ ‖ padding), so choosing it requires the session keys, and anyone holding those owns the connection already. Both checks are assertions about our OWN endpoints, not defences against a third party. Ordering is structurally guaranteed today -- writeTickets completes before Server hands the conn to its caller, so no application can write ahead of it -- and the checks are the tripwire if that stops being true. Second, the length check was carrying no weight. writeTickets emits exactly u16 ‖ ticket ‖ psk, and the padding writeSized adds is stripped back off by decryptRecord, so a legitimate body is exactly 2+tl+32 bytes. The check was `2+tl+32 > len(body)`, which accepts anything whose first two bytes encode a small number. Over 200k samples an HTTP/2-frame-shaped body -- which is what a tunnel actually carries -- was accepted 100% of the time, because a frame's first two bytes are the top 16 bits of a 24-bit length and are therefore tiny. Uniform random bodies were accepted 3% of the time. So the answer to the question is yes, easily, and for the most likely payload shape it was not "can" but "always would". Exact equality takes both to ~0. The companion record's check is now exact as well. Its hole was narrower -- fl != FullTicketLen already pinned the declared length -- but narrower is not closed. Both are tested against the real readTickets rather than a reconstruction of its predicate. The first attempt compared two closures written in the test file, which is the oracle-is-the-thing-under-test trap: the loose mutation passed it. The tests also write a well-formed companion record after the bad first one, because otherwise a loosened check blocks forever waiting for a record that never arrives, and a hang is not a failure. Mutation-tested in both directions: loosening either check fails a test, and an off-by-one in the exact check fails rotation for every cover. Co-Authored-By: Claude Opus 5 --- handshake.go | 29 ++++++-- handshake_test.go | 178 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 7 deletions(-) diff --git a/handshake.go b/handshake.go index 832fca3..50827ec 100644 --- a/handshake.go +++ b/handshake.go @@ -398,11 +398,26 @@ func readTickets(c *Conn) (*Credential, error) { if err != nil { return nil, err } - // writeTickets emits contentHandshake and nothing else, so accepting - // application_data here only widens what can be mistaken for a credential. - // After the opening the tunnel carries app-data records; if the ordering - // ever shifts, a lenient check would parse the first of them as a rotated - // ticket instead of failing loudly. + // Both checks below are assertions about OUR OWN endpoints, not defences + // against an adversary. The inner content type lives inside the AEAD + // plaintext (see writeRecord), so forging a contentHandshake record needs + // the session keys, and anyone holding those owns the connection already. + // + // What they defend against is self-inflicted confusion. Ordering is + // structurally guaranteed today -- writeTickets completes before Server + // hands the conn to its caller, so no application can write ahead of it -- + // and these checks are the tripwire if that ever stops being true. + // + // The length check is EXACT because a loose one turned out to carry no + // weight. writeTickets emits precisely u16 ‖ ticket ‖ psk, and the padding + // writeSized adds is stripped back off by decryptRecord, so a legitimate + // body is exactly 2+tl+32 bytes. Against `2+tl+32 > len(body)`, measured + // over 200k samples: an HTTP/2-frame-shaped body -- which is what a tunnel + // actually carries -- was accepted 100% of the time, because a frame's + // first two bytes are the top 16 bits of a 24-bit length and are therefore + // tiny. Uniform random bodies were accepted 3% of the time. Exact equality + // takes both to ~0, so a stray app-data record fails here rather than + // yielding a credential with a psk read out of someone's payload. if typ != contentHandshake { return nil, errMalformed } @@ -410,7 +425,7 @@ func readTickets(c *Conn) (*Credential, error) { return nil, errMalformed } tl := int(binary.BigEndian.Uint16(body[0:2])) - if 2+tl+32 > len(body) { + if len(body) != 2+tl+32 { return nil, errMalformed } cred := &Credential{Ticket: append([]byte(nil), body[2:2+tl]...)} @@ -427,7 +442,7 @@ func readTickets(c *Conn) (*Credential, error) { return nil, errMalformed } fl := int(binary.BigEndian.Uint16(body[0:2])) - if fl != FullTicketLen || 2+fl > len(body) { + if fl != FullTicketLen || len(body) != 2+fl { return nil, errMalformed } cred.FullTicket = append([]byte(nil), body[2:2+fl]...) diff --git a/handshake_test.go b/handshake_test.go index ec76985..a9d897b 100644 --- a/handshake_test.go +++ b/handshake_test.go @@ -1079,3 +1079,181 @@ func TestClientReportsANilConnectionRatherThanPanicking(t *testing.T) { } }) } + +// The rotation record's length check must be EXACT, and this drives the real +// readTickets rather than a reconstruction of its predicate. +// +// The body writeTickets emits is precisely u16 ‖ ticket ‖ psk, and the padding +// writeSized adds is stripped back off by decryptRecord, so a legitimate body +// is exactly 2+tl+32 bytes. The check used to be `2+tl+32 > len(body)`, which +// sounds conservative and is not: it accepts anything whose first two bytes +// encode a small number, which is exactly what a tunnel's own payload looks +// like. An HTTP/2 frame's first two bytes are the TOP 16 bits of a 24-bit +// length, so for any frame under 64 KiB they are tiny, and over 200k samples +// the loose check accepted 100% of them. +// +// Note what this is and is not. The inner content type lives inside the AEAD +// plaintext, so an adversary cannot choose it without the session keys. Both +// this and the content-type check are assertions about our OWN endpoints -- a +// tripwire on the ordering invariant writeTickets relies on -- not defences +// against a third party. +func TestReadTicketsRejectsALooseButInexactBody(t *testing.T) { + cover := mustCover(t, "www.microsoft.com") + newSess := func() *Session { + sess, err := DeriveSession(make([]byte, 32), make([]byte, 32), cover.CipherSuite) + if err != nil { + t.Fatal(err) + } + return sess + } + server, client := net.Pipe() + defer server.Close() + defer client.Close() + w, err := NewConn(server, newSess(), false, nil) + if err != nil { + t.Fatal(err) + } + r, err := NewConn(client, newSess(), true, nil) + if err != nil { + t.Fatal(err) + } + + // An HTTP/2-frame-shaped body, sent as a HANDSHAKE record so the content + // type check cannot be what rejects it. 24-bit length, type, flags, + // 32-bit stream id -- the first two bytes are therefore 0x00 0x00. + const size = 300 + body := make([]byte, size) + rand.Read(body) + n := size - 9 + body[0], body[1], body[2] = byte(n>>16), byte(n>>8), byte(n) + + // It satisfies the OLD loose predicate, which is what makes this a + // regression test rather than a tautology. + if got := 2 + int(binary.BigEndian.Uint16(body[0:2])) + 32; got > len(body) { + t.Fatalf("body does not satisfy the loose check (%d > %d); the test proves nothing", got, len(body)) + } + + // A well-formed COMPANION record follows, so a loosened check produces a + // bogus credential and no error rather than blocking on a record that never + // arrives. A hang is not a test failure, so the mutation has to be able to + // reach a verdict. + companion := make([]byte, 2+FullTicketLen) + companion[0], companion[1] = byte(FullTicketLen>>8), byte(FullTicketLen) + rand.Read(companion[2:]) + + go func() { + _ = w.writeSized(contentHandshake, body, sessionTicketWire) + _ = w.writeSized(contentHandshake, companion, sessionTicketWire) + }() + + got, err := readTickets(r) + if err == nil { + t.Errorf("readTickets accepted an HTTP/2-shaped body as a rotated credential (ticket %d bytes, psk from the payload); the length check is loose again", + len(got.Ticket)) + } +} + +// The companion record's length check must be exact too. +// +// Its hole is narrower than the first record's, because `fl != FullTicketLen` +// already pins the declared length to 144 -- so a loose body check only admits +// a body of 146 bytes or more that happens to start 0x00 0x90. Narrower is not +// closed, and an untested guarantee is not one. +func TestReadTicketsRejectsAnOversizedCompanionBody(t *testing.T) { + cover := mustCover(t, "www.microsoft.com") + newSess := func() *Session { + sess, err := DeriveSession(make([]byte, 32), make([]byte, 32), cover.CipherSuite) + if err != nil { + t.Fatal(err) + } + return sess + } + server, client := net.Pipe() + defer server.Close() + defer client.Close() + w, err := NewConn(server, newSess(), false, nil) + if err != nil { + t.Fatal(err) + } + r, err := NewConn(client, newSess(), true, nil) + if err != nil { + t.Fatal(err) + } + + // A valid first record, so only the companion is under test. + const tl = 176 + first := make([]byte, 2+tl+32) + first[0], first[1] = byte(tl>>8), byte(tl) + rand.Read(first[2:]) + + // A companion declaring the right length but carrying more than that: + // passes `2+fl > len(body)`, fails exact equality. + companion := make([]byte, 2+FullTicketLen+64) + companion[0], companion[1] = byte(FullTicketLen>>8), byte(FullTicketLen) + rand.Read(companion[2:]) + if 2+FullTicketLen > len(companion) { + t.Fatal("companion does not satisfy the loose check; the test proves nothing") + } + + go func() { + _ = w.writeSized(contentHandshake, first, sessionTicketWire) + _ = w.writeSized(contentHandshake, companion, sessionTicketWire) + }() + + if _, err := readTickets(r); err == nil { + t.Error("readTickets accepted a companion record carrying more than it declared; the companion length check is loose again") + } +} + +// End to end, so the exact check is proved against what writeTickets and +// decryptRecord actually produce rather than against a reconstruction of it. +// Padding is the thing that would break exact equality, and only a real +// round trip exercises it. +func TestRotationSurvivesTheExactLengthCheckForEveryCover(t *testing.T) { + k := ticketKey(t) + for _, host := range MeasuredCovers() { + t.Run(host, func(t *testing.T) { + cover := mustCover(t, host) + cred, err := k.Issue(400, cover.TicketLen) + if err != nil { + t.Fatal(err) + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + go func() { + c, aerr := ln.Accept() + if aerr != nil { + return + } + defer c.Close() + c.SetDeadline(time.Now().Add(5 * time.Second)) + Server(c, ServerConfig{ + TicketKey: k, Cover: cover, + MaxAge: time.Hour, Replay: NewReplayCache(16, time.Hour), + }) + }() + raw, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer raw.Close() + raw.SetDeadline(time.Now().Add(5 * time.Second)) + + _, next, err := Client(raw, ClientConfig{ + Pool: pool(t), Cover: cover, Credential: cred, + }) + if err != nil { + t.Fatalf("rotation failed the exact length check for a %d-byte ticket: %v", cover.TicketLen, err) + } + if len(next.Ticket) != cover.TicketLen { + t.Errorf("rotated ticket is %d bytes, want %d", len(next.Ticket), cover.TicketLen) + } + if len(next.FullTicket) != FullTicketLen { + t.Errorf("rotated companion is %d bytes, want %d", len(next.FullTicket), FullTicketLen) + } + }) + } +} From 8ae154a2c350a86237606ce7d8d8fb88c094e517 Mon Sep 17 00:00:00 2001 From: Adam Fisk Date: Fri, 4 Sep 2026 19:24:24 +0100 Subject: [PATCH 18/18] Record that mixed-version deployment is not supported, deliberately The rolling-upgrade compatibility path proposed in review for readTickets was declined. Writing the reasoning where a future reader meets the question, because "why is this check strict?" is exactly the kind of thing that gets re-litigated from first principles by someone who cannot see the decision. The substance: a compatibility path here would cover one field of many and imply a guarantee the rest of the protocol does not offer. Both ends must already agree exactly on TicketLen, BinderLen, CipherSuite, PSKFirst and the ResumedRemainder sequence, and all of it arrives together in one provisioned CoverProfile. If mixed-version deployment is ever needed, the answer is an explicit version in that config -- which already reaches both ends -- deciding the record count up front, rather than inferring it from a record type at runtime and re-opening what the content-type check closes. Co-Authored-By: Claude Opus 5 --- handshake.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/handshake.go b/handshake.go index 50827ec..6a09b23 100644 --- a/handshake.go +++ b/handshake.go @@ -408,6 +408,21 @@ func readTickets(c *Conn) (*Credential, error) { // hands the conn to its caller, so no application can write ahead of it -- // and these checks are the tripwire if that ever stops being true. // + // Asked and declined during review: should a non-handshake second record be + // treated as "no companion ticket" so a one-record server still works, for + // a rolling upgrade? No. This transport supports NO mixed-version + // deployment, deliberately. Both ends must already agree exactly on + // TicketLen, BinderLen, CipherSuite, PSKFirst and the ResumedRemainder + // SEQUENCE -- the client reads one record per entry, so a mismatch there + // misaligns every later read -- and all of it arrives together in one + // provisioned CoverProfile. A compatibility path here would cover one field + // of many and imply a guarantee the rest of the protocol does not offer. + // + // If mixed-version deployment is ever needed, the answer is an explicit + // version in the provisioned config, which already reaches both ends, + // deciding the record count up front. Not a record-type inference at + // runtime, which would re-open exactly what the type check above closes. + // // The length check is EXACT because a loose one turned out to carry no // weight. writeTickets emits precisely u16 ‖ ticket ‖ psk, and the padding // writeSized adds is stripped back off by decryptRecord, so a legitimate