Skip to content

fix(transport)!: make inconsistent handshake state unrepresentable - #60

Open
sephynox wants to merge 1 commit into
fix/audit-phase-3from
fix/audit-phase4-1
Open

sephynox wants to merge 1 commit into
fix/audit-phase-3from
fix/audit-phase4-1

Conversation

@sephynox

@sephynox sephynox commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

A transport could hold a completed handshake with no session keys, or keys with no completed handshake, because the phase was recomputed from two independently settable fields on every read. The handshake state machine separately accepted any transition legal in either ECIES or CMS, relying on the orchestrators to never attempt the other protocol's moves.

Related Issues

None

Changes Made

  • Session keys live inside the encrypted phase, so neither can exist alone
  • Completing a handshake installs its keys in one transition
  • The circuit breaker resets position and keys together
  • A CMS handshake now refuses ECIES-only transitions, and the reverse

Testing

  • make test-all: passes

Breaking Changes

  • TcpHandshakeState is removed. Match SessionPhase instead.
  • EncryptedProtocolState replaces four state setters with session_phase and set_session_phase.
  • ClientStateMachine and ServerStateMachine take a flow parameter, Cms or Ecies.
  • Listener impls now require P: CryptoProvider + Send + Sync + 'static, which the trait already demanded.

Note

Medium Risk
Refactors handshake completion, session keys, and protocol transitions across TCP transport and CMS/ECIES orchestrators—security-critical paths where regressions would affect encryption gating or handshake validity.

Overview
This PR tightens transport and handshake state so invalid combinations cannot be held in memory, and CMS vs ECIES flows are separated at the type level.

Session lifecycle: TcpHandshakeState and separate session_keys are replaced by a stored SessionPhase (Cleartext, Provisioned, Handshaking { initiated_at }, Encrypted(SessionKeys)). Keys are installed only via complete_handshake, resets use reset_session, and handshake start uses begin_handshake / HandshakeInstant for deadlines. I/O and messaging now match on SessionPhase instead of handshake enums.

Protocol state machines: ClientStateMachine / ServerStateMachine are generic over Cms or Ecies with per-flow transition tables, so cross-protocol jumps (e.g. CMS skipping Finished) return InvalidState instead of relying on orchestrator ordering alone.

Orchestrators: CMS/ECIES clients and servers drop HandshakeInvariant and the related HandshakeError invariant variants; ordering is enforced by flow-specific transitions and transport phase helpers instead.

Breaking API: EncryptedProtocolState drops set_handshake_state / set_session_keys / unset_session_keys in favor of session_phase / set_session_phase and the helper transitions above; listener impls require P: CryptoProvider + Send + Sync + 'static.

Reviewed by Cursor Bugbot for commit e3ae60e. Bugbot is set up for automated code reviews on this repo. Configure here.

@sephynox sephynox self-assigned this Sep 9, 2026
@sephynox
sephynox added this pull request to stack #55 September 9, 2026 03:22
@sephynox sephynox added the bug Something isn't working label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant