[Adversarial review] StreamEndpoint peer attribution and pre-connect lifecycle gaps

[al8n]

Defensive adversarial review: StreamEndpoint peer attribution and pre-connect lifecycle

Scope and methodology

Reviewed exactly commit aeef956bc0bdf0ed21ba386b329dab6aae64b47e, limited to the standard memberlist-proto StreamEndpoint, its plain/TLS StreamBridge lifecycle, transport actions, correlation, cleanup, leave, and timeout behavior.

I traced concrete transitions from public entry points through bridge promotion/reap and inner Endpoint events. Reactor/Compio source was inspected only where necessary to establish how the standard coordinator receives an accepted socket address. QUIC and membership-algorithm findings were excluded.

Duplicate screening covered #157, #158, #162, and #163. Known TLS timing/stale-Connect, output-acknowledgement, close-timeout, driver-death, leave, and QUIC findings are not repeated.

This review was performed and accepted by GPT-5.6 Sol (gpt-5.6-sol) at literal max reasoning effort.

Verdict

No Critical or High defect was validated.

Severity	Concrete defects	Architectural findings
Medium	1	0
Low	0	1

STR-001 — Medium: inbound TCP/TLS events report the client's ephemeral socket as the member address

Evidence

The standard accept path obtains the kernel-reported peer socket and passes it directly to accept_connection (reachability cross-check).

accept_connection(from, ...) stores that value both as the exchange peer and as PendingMint::Inbound(from) (coordinator accept path), even though ExchangeMeta.peer is documented as the membership address (metadata contract).

After label/TLS settlement, the value is passed unchanged to Endpoint::accept_stream (promotion), which installs it as Stream.peer (stream mint). The stream copies it into inbound push/pull and user-data events (dispatch), and the endpoint publishes it as RemoteStateReceived.peer or UserPacket.from (push/pull publication, user-data publication). UserPacket.from is documented as the source address (event contract).

The existing in-memory integration harness masks the defect by explicitly making the acceptor see the dialer's advertised address rather than a real client socket source (harness assumption, injection).

Minimal trigger

1. Member A advertises 10.0.0.1:7946.
2. A opens an ordinary outbound TCP connection to B from kernel-selected endpoint 10.0.0.1:49152.
3. B calls accept_connection(10.0.0.1:49152, now).
4. The label or TLS handshake settles and the inbound Stream is minted with that value.
5. A sends reliable user data or push/pull state.
6. B publishes UserPacket.from or RemoteStateReceived.peer as 10.0.0.1:49152, not A's member address 10.0.0.1:7946.

No later path rewrites the value after protocol data becomes available.

Violated invariant

Reliable events must use the same logical address domain as membership entries and outbound reliable dials, or expose logical member identity and transport source separately. A transient client-side connection endpoint must not populate a field described as the membership peer/source address.

Impact and reachability

This happens on ordinary inbound TCP and TLS connections; no malicious timing is needed. Applications cannot reliably use the reported address to find the membership entry or reply because the ephemeral port normally stops listening when the one-exchange connection closes. Per-member accounting and policy see a different apparent peer for each connection.

Push/pull membership merging itself was not shown to be corrupted, and this is not an authentication-bypass claim.

This is distinct from #163 QEP-002: that issue requires QUIC migration and makes a once-valid path stale. STR-001 begins with an address that was never the advertised member address and affects the first message on routine TCP/TLS connections.

Remediation direction

Separate the accepted transport source from the logical member identity/advertised address. Stream framing should carry a stable sender identity or advertised address before dispatching the first message and bind it to membership state and, where applicable, the authenticated TLS client identity. Application events should use the validated logical peer; retain the raw transport source separately for diagnostics and network policy.

Missing regression

Use a real listener and client socket:

1. A advertises listener A:7946 but connects from kernel-selected A:E.
2. Complete UserData and push/pull over plain TCP and TLS.
3. Assert UserPacket.from and RemoteStateReceived.peer identify A:7946.
4. If exposed, separately assert transport source A:E.
5. Repeat from a second ephemeral port and require the same logical attribution.

STR-A001 — Low architecture: pre-Connect failures have no typed terminal result

The public lifecycle uses two ID domains. A start_* call returns a StreamId; only a successful Connect introduces an ExchangeId and carries both (correlation contract). Terminal ExchangeCompleted is keyed only by the later ExchangeId (event contract).

service_dials can reject an expired intent, TLS SNI, or record-layer construction before allocating that ID; each path calls dial_failed and continues without a Connect or terminal event (failure paths). The inner endpoint deliberately leaves push/pull and user-message dial failures silent and delegates reporting to the driver (inner contract).

Minimal trace: start_user_message returns Ok(StreamId); the TLS SNI provider returns None; dial_context rejects it; no bridge, ExchangeId, action, or event identifies the failed operation.

Compio and Reactor compensate by inferring failure from the absence of a matching Connect, so this is not classified as a standard-driver correctness defect. It nevertheless makes the public lifecycle non-total and forces every direct integrator to reproduce negative inference correctly, including partial batches.

Return a synchronous typed failure or emit exactly one explicit pre-connect failure keyed by StreamId, kind, and reason. Alternatively allocate a stable terminal operation ID before fallible setup without emitting transport teardown when no socket exists.

Missing regression: missing/invalid SNI, expired intent, and constructor failure must each yield exactly one correlated rejection or synchronous error, no Connect, no transmit, and no retained metadata. Mixed batches need one explicit outcome per returned start ID.

Rejected and narrowed candidates

Candidate	Conclusion
Late or duplicate callbacks	Reap removes the bridge synchronously; later callbacks miss the map. No reachable ID reuse exists before u64 exhaustion.
ID wrap	Collision requires approximately 2^64 exchanges in one endpoint lifetime; no realistic trigger was established.
Stale Connect before Abort	Already reported in #158.
Stale Shutdown before later Abort	Requires feeding new input before completing the documented fixed-point output drain; retained as an API footgun, not a defect.
Stale transmit after failure	Failed reap purges coordinator-owned chunks and clears record-layer output. Post-dequeue ownership is already #157/#162.
Duplicate/missing completion after bridge allocation	Reap removes metadata before publishing once; late callbacks cannot publish again.
Clean success overwritten by later failure	Public data/error handlers immediately pump and reap terminal bridges, leaving no QUIC-style same-turn seam.
New promotion race	Pre-promotion plaintext/EOF is buffered and replayed. The same-time TLS deadline race remains #158.
Duplicate inbound connections	Expected connection-per-exchange behavior; there is no canonical connection map to clobber.
Inbound admission	The configured cap is enforced before allocation. Default unlimited admission is explicit configuration.
Leave cleanup	Pending/unsent exchanges are canceled and late callbacks are inert. Cross-layer completion/output semantics remain #162.

Validation

• git rev-parse HEAD: pinned commit confirmed.
• gh issue view 157/158/162/163 --repo al8n/memberlist: duplicate classification completed.
• With tcp,tls-rustls-ring,lz4,aes-gcm, focused groups passed:
  • streams::tests::tcp::: 16 passed
  • tcp::tests::: 57 passed
  • tls::tests::: 16 passed
  • streams::tests::tls::: 3 passed
  • total: 92 passed, 0 failed
• Named controls passed:
  • tcp::tests::end_to_end_reliable_user_message_delivers_on_acceptor
  • tls::tests::dial_context_failure_does_not_leak_pending_outbound_kinds
• Full built libtest result: 979 passed, 4 environmental failures. The failures were TLS configuration tests unable to create temporary files in the review process's read-only sandbox, not protocol assertions.
• cargo test -p memberlist-proto --features tcp,tls-rustls-ring --tests does not compile: streams/tests.rs unconditionally imports handshaking_pair, while the helper is gated behind compression + encryption. This is a feature-matrix test-gating defect; adding transform features allows the relevant tests to run.

Confidence

Confidence is high for STR-001: it is a direct value flow from the kernel peer address to public events, and the existing harness documents the assumption that masks it.

Confidence is high in the existence and medium in the severity of STR-A001: the missing terminal surface is explicit, but current standard wrappers compensate.