[Adversarial review] TLS EOF can authorize truncated exchanges; pre-label close_notify is non-terminal

[al8n]

[Adversarial review] TLS EOF can authorize truncated exchanges; pre-label close_notify is non-terminal

Review metadata

• Reviewed commit: aeef956bc0bdf0ed21ba386b329dab6aae64b47e
• Repository state: HEAD matched the requested commit; worktree was clean before and after review.
• Model: gpt-5.6-sol
• Reasoning effort: max
• Mode: bounded, defensive, read-only; no files changed, issues filed, subagents used, or other Codex process invoked.
• Scope: the requested TCP/TLS record files, StreamTransport, Labeled/Passthrough, record-boundary portions of streams/bridge.rs, focused tests, Cargo.lock, and only the locked rustls 0.23.40 behavior needed to validate candidates.
• Excluded as requested: drivers and scheduling; generic endpoint/stream lifecycle except immediate record contracts; membership, QUIC, DNS, facade/config parity; certificate authorization/shared-CA isolation; [Adversarial review] Wire decoder length arithmetic and fail-closed skipping #167 wire-decoder findings; and duplicates from [Adversarial review] Correctness, lifecycle, and architecture findings across proto/compio/reactor #157–[Adversarial review] Embedded/Embassy/smoltcp reliable socket lifecycle composition #166, including close_timeout, peer attribution, cross-driver output/terminalization, embedded socket teardown, and broad endpoint findings.

Executive summary

Two findings survived adversarial falsification:

Severity	Count
Critical	0
High	0
Medium	1
Low	1

The principal defect is that the shared bridge treats unauthenticated TCP EOF as equivalent to authenticated TLS close_notify. Once a complete reliable unit has decoded, an EOF without close_notify is therefore reported and committed as a clean exchange rather than TLS truncation.

Separately, a labeled TLS peer that completes TLS and sends close_notify before its label leaves the bridge in an irrecoverable Handshaking state until its deadline, even though rustls will accept no more data.

Fragmentation/coalescing, bounded TLS intake, label gating, whole-unit outbound admission, close ordering, and failure-side output suppression otherwise held under the scoped tests. Neither finding overlaps the reviewed #157–#167 reports.

State and ownership model

Layer	Inbound ownership	Outbound ownership	State/termination
StreamTransport	Caller retains the slice; Done claims all bytes consumed, Pending(n) transfers exactly the prefix	Implementation appends complete transport bytes to caller’s Vec	Failed is terminal only because the bridge terminalizes it
Passthrough	Copies every input byte into inbound, then drains it verbatim	outbound: Vec<u8>; bounded only by the bridge’s one-unit cadence	No handshake or in-band close
Labeled<R>	LabelGate owns partial label bytes; post_label_tail owns plaintext coalesced after the label	A maximum 255-byte label frame is staged into R before application plaintext	Gate validating → validated/rejected; inner handshake is composed with the gate
TlsRecords	rustls owns ciphertext deframing and up to 16 KiB received plaintext; peer_closed is sticky	pending owns whole application plaintext up to max(max_frame + max_frame/16 + 16 KiB, 64 KiB); rustls owns handshake/application/alert ciphertext behind a 2× backstop	TLS handshake → established; peer_closed records close_notify; close_requested defers local close until pending drains
StreamBridge	pending_inbound owns unconsumed ciphertext; recv_accum owns a partial length-delimited reliable unit; pending_eof carries pre-promotion FIN	One encoded reliable unit is admitted to the record layer; fin_sent owns the local close obligation	Handshaking → Established(Active) → {SendClosed, RecvClosed} → BothClosed; failures should be terminal

F1 — TCP EOF is accepted as clean TLS completion

Severity: Medium (P2)
Confidence: High

Evidence

• Empty TLS intake skips read_tls and merely calls process_new_packets: tls/records/mod.rs#L255-L295.
• TLS reader errors—including rustls UnexpectedEof—are collapsed into “no plaintext”: tls/records/mod.rs#L123-L184.
• A pre-promotion EOF is accepted whenever the composed record handshake has just settled, without requiring TLS closure: streams/bridge.rs#L1022-L1036.
• Established intake explicitly defines fin_seen = eof || records.peer_has_closed(): streams/bridge.rs#L1153-L1196.
• Reaching RecvClosed authorizes queued payload events: streams/bridge.rs#L1353-L1379.
• The locked dependency is rustls 0.23.40: Cargo.lock#L3609-L3612. Its Reader distinguishes TCP EOF without close_notify as UnexpectedEof, but only after EOF has been reported through read_tls.

Violated invariants

• Invariant 3: TLS truncation is mistaken for clean completion.
• Invariant 6: an unauthenticated transport FIN can authorize buffered application side effects.

Minimal reachable trace

1. Complete TLS authentication and label validation; promote an inbound bridge.
2. Receive authenticated ciphertext containing exactly one complete reliable unit. It decodes successfully and its endpoint event remains queued pending close proof.
3. The peer omits close_notify and closes the TCP write half. The bridge receives handle_transport_data(&[]).
4. TlsRecords does not tell rustls about EOF and returns Done; peer_has_closed() remains false.
5. The bridge nevertheless computes fin_seen = true || false, calls Stream::handle_data(&[]), and transitions to RecvClosed.
6. drain_payload_only commits the queued event. Subsequent local close can produce BothClosed and a clean lifecycle notification.

The same result is reachable when the final TLS/application bytes arrive before promotion and the following TCP EOF is stored in pending_eof.

Impact

A peer closing TLS without close_notify, or an on-path actor capable of truncating the TCP stream, can make a complete TLS plaintext prefix appear cleanly finalized. Already-decoded membership/user events may be committed and the exchange reported as closed rather than errored. Partial reliable units still fail, so the impact is confined to truncation at a complete-unit boundary; no plaintext forgery was established.

Root cause

The bridge’s transport-neutral close expression conflates two different proofs:

• raw TCP’s out-of-band FIN, which is its normal close signal;
• TLS’s authenticated close_notify, for which bare TCP EOF is truncation.

The trait exposes is_secure() and peer_has_closed(), but has no explicit policy describing whether transport EOF is a valid clean close.

Remediation

• Add an explicit transport capability such as requires_authenticated_close() or a close outcome returned by intake.
• For TLS, treat eof && !peer_has_closed() as ConnectionLost/Decode, both pre- and post-promotion. Accept later TCP EOF only if close_notify was already latched.
• Preserve TCP behavior, where EOF remains the clean close anchor.
• Stop collapsing UnexpectedEof with WouldBlock if EOF is routed through rustls in a future API.

Regression tests

1. Complete TLS handshake and label validation, feed one complete reliable unit without close_notify, then feed TCP EOF; assert Failed, no committed payload event, and StreamErrored.
2. Repeat with the complete unit buffered pre-promotion followed by EOF.
3. Positive control: complete unit + close_notify + TCP EOF remains clean.
4. Confirm the equivalent plain-TCP complete-unit + FIN path remains clean.

F2 — close_notify before label completion leaves an impossible handshake live

Severity: Low (P3)
Confidence: High

Evidence

• TLS latches peer_closed after processing close_notify: tls/records/mod.rs#L255-L295 and #L426-L428.
• Labeled<R> returns the inner intake unchanged and remains handshaking while either the gate or inner is handshaking: streams/labeled/mod.rs#L479-L507.
• Pre-stream intake only checks whether the composed handshake settled; it never checks peer_has_closed: streams/bridge.rs#L389-L433.
• The only remaining terminalization is the handshake deadline: streams/bridge.rs#L712-L725.

Violated invariants

• Invariant 2: a close transition is not immediately terminal.
• Invariant 3: authenticated TLS closure is not classified as incomplete label failure.
• Invariant 5: a state that can no longer make progress remains allocated until timeout.

Minimal reachable trace

1. Construct an acceptor Labeled<TlsRecords> with a required label.
2. Complete the TLS handshake without sending application plaintext. The inner TLS handshake is complete, but the label gate keeps Labeled handshaking.
3. Send an authenticated close_notify and keep the TCP connection open.
4. rustls latches the close. consume_inbound_label finds no plaintext, so the gate remains validating and Labeled::handle_transport_data returns Done.
5. intake_handshaking observes “Done but still handshaking” and returns successfully.
6. rustls will not read further TLS bytes after close_notify; nevertheless the bridge remains live until its handshake deadline.

Impact

An already TLS-authenticated peer can retain an accept-side bridge and its connection until the configured handshake deadline after explicitly closing TLS. This delays teardown and failure reporting. The resource lifetime is bounded, no Stream is minted, and no local label or application plaintext is exposed; a silent peer could already consume the same timeout, supporting Low severity.

Root cause

The in-band close latch is consulted only by established intake. Composing TLS handshaking with label validation hides the fact that the inner record stream has terminated before the outer gate can succeed.

Remediation

After every pre-stream intake, if records.peer_has_closed() is true while records.is_handshaking() remains true, fail the bridge immediately without promotion or outbound label emission. A stronger design would let Intake carry an explicit Closed { consumed } outcome.

Regression tests

1. Required-label TLS acceptor: complete TLS only, send close_notify, withhold TCP FIN; assert immediate terminal failure, no Stream, no outbound label, and no ciphertext.
2. Repeat after a partial fragmented label.
3. Positive control: a complete matching label and reliable unit coalesced with close_notify still promotes, drains, and closes normally.

Architecture and hardening observations

• Intake::Done promises full consumption, but rustls returns Ok(0) after receiving close_notify; a subsequent nonempty slice can therefore be reported Done without being read. Model intake as { consumed, status }, with explicit Open, Backpressured, Closed, and Failed statuses.
• Labeled::flush_outbound_label_into_inner ignores write_plaintext’s whole-unit admission result and marks the prefix emitted. This is safe for the two current inners because labels are at most 255 bytes and TLS has a 64 KiB floor, but it violates the generic StreamTransport contract. Propagate failure or make infallible prefix admission a documented bound.
• TlsRecords maps every read_tls error to backpressure. Locked rustls behavior plus per-chunk process_new_packets rejected the concrete malformed-record candidates before this became unsafe, so no bug was retained. Matching only the backpressure error kind would make the contract resilient to dependency changes.
• Passthrough’s Vec buffers have no intrinsic cap; boundedness depends on the bridge’s current one-unit-per-pump cadence. A local one-unit limit would make ownership explicit rather than architectural.

Validation

All checks were non-mutating.

• git rev-parse HEAD → aeef956bc0bdf0ed21ba386b329dab6aae64b47e
• git status --porcelain=v1 --untracked-files=all → empty before and after
• cargo test -p memberlist-proto --features 'tcp,tls-rustls-ring,lz4,aes-gcm' tls::records::tests:: -- --test-threads=1 → 11 passed
• Same command with streams::labeled::tests:: → 23 passed
• Same command with tls::bridge:: → 12 passed
• Same command with tcp::bridge:: → 26 passed
• Exact tls::tests::failed_established_tls_exchange_no_close_notify_before_abort → 1 passed
• Total focused tests: 73 passed, 0 failed.
• Tests emitted non-fatal macOS object-version linker warnings; execution succeeded.
• Locked rustls 0.23.40 source was checked for read_tls, Reader::UnexpectedEof, process_new_packets, close latching, and buffer-limit behavior.
• Issue titles [Adversarial review] Correctness, lifecycle, and architecture findings across proto/compio/reactor #157–[Adversarial review] Wire decoder length arithmetic and fail-closed skipping #167 and targeted matching excerpts from [Adversarial review] Correctness, lifecycle, and architecture findings across proto/compio/reactor #157, Adversarial review: standard Endpoint, StreamEndpoint, and QuicEndpoint #158, [Adversarial review] Cross-driver contracts and dependency-aware remediation #162, and [Adversarial review] Wire decoder length arithmetic and fail-closed skipping #167 were checked. Neither finding is present; F1 is record-layer-specific and not [Adversarial review] Cross-driver contracts and dependency-aware remediation #162’s cross-driver terminalization report.

Rejected or narrowed candidates

• Coalesced ciphertext above rustls’s 16 KiB plaintext limit does not duplicate or lose bytes: consumed offsets, interleaved drains, retained tails, and large-frame tests held.
• Fragmented labels and reliable units preserve order; the gate owns partial headers and post_label_tail precedes later inner plaintext.
• Outbound TLS admission is whole-unit: overflow rejects without partial staging; transmit re-feeds short rustls writes until pending empties.
• close_notify does not overtake admitted application bytes: it remains deferred until staged plaintext drains.
• No application plaintext was found to surface before TLS handshake completion or label validation.
• Failed raw exchanges clear labels/application bytes; failed TLS reaps suppress rustls-owned close ciphertext. The focused regression passed, so the known cross-driver output finding was not restated.
• Capacity headroom covers the current maximum label, reliable-unit length prefix, TLS framing, and non-inflating compression path. No silent whole-unit drop survived.
• Runtime compression/encryption update concerns did not yield a scoped incompatible-epoch byte mix and were not broadened into endpoint-policy review.
• The rustls deframer-error-as-backpressure hypothesis was narrowed to hardening: under locked 0.23.40, malformed record sizes reach process_new_packets failure in the reviewed feed cadence.

Limitations

This was a source and focused-test review, not live-socket fault injection. No regression tests were added, so the two failing traces are established from exact control flow and locked rustls semantics rather than newly executed negative tests. Only the ring/lz4/AES-GCM focused feature combination was run; alternate crypto providers share the reviewed rustls connection state machine but were not separately exercised. All requested driver, scheduling, endpoint-wide, certificate-policy, QUIC, DNS, and wire-decoder areas remained excluded.