From 9673d256cc9c091f571577ead63cc02a3f4f5e89 Mon Sep 17 00:00:00 2001 From: Gijs Molenaar Date: Fri, 11 Sep 2026 09:16:06 +0200 Subject: [PATCH] fix(s7commplus): enforce async auth boundary --- CHANGES.md | 2 ++ README.rst | 4 ++++ doc/API/client.rst | 22 +++++++++++++++-- s7commplus/async_client.py | 18 ++++++++++++-- tests/test_s7_v2.py | 49 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f664baeb..9a8d4f6f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,8 @@ CHANGES Major release: new `s7commplus` package with S7CommPlus protocol support. +* Make the asynchronous S7CommPlus authentication boundary explicit: support + V2/V3 over TLS and reject legacy V1 SessionKey PLCs during connection setup. * Decode corroborating CPU execution attributes so S7CommPlus `get_cpu_state()` distinguishes RUN from STOP on S7-1500 and returns UNKNOWN for absent or inconsistent state attributes, including S7-1200 responses that omit them. diff --git a/README.rst b/README.rst index 233ceefe..9d5a5cae 100644 --- a/README.rst +++ b/README.rst @@ -95,6 +95,10 @@ disabled. python-snap7 now supports S7CommPlus V1, V2 (with TLS), and V3:: data = client.db_read(1, 0, 4) client.disconnect() +Legacy V1 SessionKey authentication is supported by the synchronous client. +The asynchronous client supports the V2/V3 TLS paths and rejects SessionKey +PLCs during connection setup with an actionable error. + The new ``s7commplus`` package provides S7CommPlus protocol support for S7-1200/1500 PLCs. The ``s7`` package (recommended) and its ``snap7`` alias continue to work unchanged for legacy S7-300/400 PLCs and S7-1200/1500 with diff --git a/doc/API/client.rst b/doc/API/client.rst index 110a553a..185c12e7 100644 --- a/doc/API/client.rst +++ b/doc/API/client.rst @@ -23,7 +23,25 @@ s7commplus.AsyncClient The asynchronous client currently supports the TLS connection and legitimation path. Legacy V1 SessionKey authentication is available only on -the synchronous client. +the synchronous client. ``AsyncClient.connect()`` detects the SessionKey +attributes returned by older PLCs and fails before sending an unsupported +session setup; use ``s7commplus.Client`` for those devices. + +.. list-table:: S7CommPlus authentication compatibility + :header-rows: 1 + + * - Protocol path + - ``Client`` + - ``AsyncClient`` + * - V1 legacy SessionKey (no TLS) + - Supported + - Not supported; ``connect()`` raises + * - V2 over TLS + - Supported + - Supported + * - V3 over TLS + - Supported + - Supported .. code-block:: python @@ -32,7 +50,7 @@ the synchronous client. async def main(): client = AsyncClient() - await client.connect("192.168.1.10") + await client.connect("192.168.1.10", use_tls=True) data = await client.db_read(1, 0, 4) await client.disconnect() diff --git a/s7commplus/async_client.py b/s7commplus/async_client.py index 1a54774f..63720c35 100644 --- a/s7commplus/async_client.py +++ b/s7commplus/async_client.py @@ -39,8 +39,8 @@ encode_object_qualifier, encode_pvalue_blob, encode_typed_value, + parse_create_object_attributes, parse_create_object_session_id, - parse_server_session_version, ) from .connection import ( _MAX_SYSTEM_EVENTS_PER_RESPONSE, @@ -132,6 +132,7 @@ def __init__(self) -> None: # ServerSessionVersion is captured as its raw typed value (flags+datatype+data) # so it can be echoed back verbatim — real S7-1500 PLCs send it as a Struct. self._server_session_version: Optional[bytes] = None + self._legacy_session_key_required: bool = False self._session_setup_ok: bool = False # Effective protection level, read once the session is up self._protection_level: Optional[int] = None @@ -232,6 +233,14 @@ async def connect( if self._tls_active: self._protocol_version = ProtocolVersion.V2 + if self._protocol_version == ProtocolVersion.V1 and self._legacy_session_key_required: + from snap7.error import S7ConnectionError + + raise S7ConnectionError( + "AsyncClient does not support legacy V1 SessionKey authentication; " + "use the synchronous s7commplus.Client for this PLC" + ) + # Step 5: Session setup. A transport and CreateObject response do # not make the public client usable until the PLC accepts setup. if self._server_session_version is None: @@ -504,6 +513,7 @@ async def disconnect(self) -> None: self._outgoing_bio = None self._oms_secret = None self._server_session_version = None + self._legacy_session_key_required = False self._session_setup_ok = False self._protection_level = None @@ -1162,11 +1172,15 @@ async def _create_session(self) -> None: _log_create_object_return_value(return_value, self._tls_active) - self._server_session_version = parse_server_session_version(response[10 + obj_end :]) + attrs = parse_create_object_attributes(response[10 + obj_end :]) + self._server_session_version = attrs.server_session_version + self._legacy_session_key_required = attrs.public_key_fingerprint is not None or attrs.session_challenge is not None if self._server_session_version is not None: logger.info(f"ServerSessionVersion captured: {len(self._server_session_version)} bytes") else: logger.debug("ServerSessionVersion not found in CreateObject response") + if self._legacy_session_key_required: + logger.info("PLC advertised legacy SessionKey authentication attributes") async def _setup_session(self) -> bool: """Echo ServerSessionVersion back to the PLC via SetMultiVariables.""" diff --git a/tests/test_s7_v2.py b/tests/test_s7_v2.py index 9b70719e..92e1b571 100644 --- a/tests/test_s7_v2.py +++ b/tests/test_s7_v2.py @@ -908,6 +908,25 @@ async def test_async_request_shape(self) -> None: assert frame[: len(expected)] == expected assert frame[-4:] == struct.pack(">BBH", 0x72, ProtocolVersion.V1, 0x0000) + @pytest.mark.asyncio + async def test_async_detects_session_key_attributes(self) -> None: + server = S7CommPlusServer( + public_key_fingerprint="01:BD426B091F08731A", + session_challenge=bytes(range(20)), + ) + application_response = server._handle_create_object(seq_num=0, request_data=b"") + response = encode_header(ProtocolVersion.V1, len(application_response)) + application_response + response += struct.pack(">BBH", 0x72, ProtocolVersion.V1, 0x0000) + + client = S7CommPlusAsyncClient() + client._send_cotp_dt = AsyncMock() + client._recv_cotp_dt = AsyncMock(return_value=response) + + await client._create_session() + + assert client._legacy_session_key_required + assert client._server_session_version is not None + class TestInitSSLResponse: @pytest.mark.asyncio @@ -1240,6 +1259,36 @@ async def create_session() -> None: assert not client._transport_connected writer.close.assert_called_once() + @pytest.mark.asyncio + async def test_async_legacy_session_key_fails_before_setup(self, monkeypatch: pytest.MonkeyPatch) -> None: + client = S7CommPlusAsyncClient() + reader = MagicMock() + writer = MagicMock() + writer.wait_closed = AsyncMock() + monkeypatch.setattr("s7commplus.async_client.asyncio.open_connection", AsyncMock(return_value=(reader, writer))) + client._cotp_connect = AsyncMock() + client._init_ssl = AsyncMock() + + async def create_session() -> None: + client._protocol_version = ProtocolVersion.V1 + client._session_id = 7 + client._server_session_version = bytes([0x00, DataType.UDINT, 0x01]) + client._legacy_session_key_required = True + + client._create_session = AsyncMock(side_effect=create_session) + client._setup_session = AsyncMock() + + with pytest.raises(S7ConnectionError, match="synchronous s7commplus.Client"): + await client.connect("127.0.0.1") + + client._setup_session.assert_not_awaited() + assert not client.connected + assert not client.session_setup_ok + assert not client._session_ready + assert not client._transport_connected + assert not client._legacy_session_key_required + writer.close.assert_called_once() + class TestProtocolVersionV2: """Test V2 protocol version constant."""