Skip to content

Make S7CommPlus session setup and SessionKey activation atomic #831

Description

@gijzelaerr

Problem

The sync client installs self._session_key and enables integrity counters before the PLC accepts SetupSession. Both sync and async clients can then set connected = True even when session setup returns false.

This produces an invalid public state: transport TCP/COTP may be connected, but the S7CommPlus session is unusable or locally authenticated with a key the PLC rejected.

Scope

  • Keep generated key material pending until SetupSession succeeds.
  • Commit the key and integrity state only after validating the matching successful response.
  • Treat mandatory session-setup failure as a connection failure.
  • Ensure cleanup clears pending and active authentication state.
  • Define internal transport-connected versus protocol-session-ready states without changing unrelated public APIs.

Acceptance criteria

  • connected is never true after mandatory SetupSession rejection or malformed response.
  • Rejected authentication does not leave _session_key or integrity tracking active.
  • Successful legacy and TLS setup still transition to connected exactly once.
  • Disconnect is safe from every intermediate state.
  • Tests cover success, PLC error, malformed response and socket failure during setup.

Out of scope

  • HMAC verification itself
  • Key renewal
  • Async legacy-auth parity

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions