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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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