SessionKey handshake for V1-initial S7-1200 PLCs - #775
Open
gijzelaerr wants to merge 26 commits into
Open
Conversation
Rebased onto the s7commplus/ package split (#774). All session_auth code now lives under s7commplus/session_auth/ instead of s7/session_auth/. Adds the full session_auth package for V1-initial S7-1200 PLCs that require a SessionKey handshake before data operations: - s7commplus/session_auth/: Public-key-based session authentication (Family 0 key derivation, fingerprint matching, SecurityKeyEncryptedKey blob generation, legitimation challenge solver, HMAC key derivation) - V3 HMAC framing for post-auth data operations - TIA-style CreateObject attributes (required by V1-initial firmware) - PAOM string stripping in ServerSessionVersion echo - Post-auth legitimation flow (challenge read + solved blob write) - Connection.connect(password=) parameter for auth flow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ation - Add type annotations for hashmod parameters in HKDF helpers - Fix frame_version type annotation (int vs ProtocolVersion) - Remove duplicated _skip_typed_value type-dispatch block (dead code after return) - Guard auth_result unpacking with explicit None check for mypy - Add None guards for session_key and challenge in _post_auth_legitimation - Add assert/guard for Optional connection in substreamed client methods Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Analysis of TIA Portal V19 pcap captures (xBiggs, GH-710) revealed the root cause of the ConnectionReset after SessionKey auth: TIA Portal's post-SetupSession sequence: 1. SET_VARIABLE (0x04F2) — write attribute 323 on session 2. GET_VAR_SUBSTREAMED — read from InObj=50, addr 7920 (finalize) 3. Data reads work immediately (no password needed) 4. Legitimation (addr 303/1846) happens later, optionally Our code was skipping steps 1-2 and jumping straight to legitimation (read challenge → write blob → finalize), which caused the PLC to return V254 SYSTEM_EVENT error 0xE9 and RST subsequent requests. Changes: - Add _session_activate() that mirrors TIA Portal's activation: SET_VARIABLE attr 323 + GET_VAR_SUBSTREAMED InObj=50 addr 7920 - Only call _post_auth_legitimation() when a password is provided (previously ran unconditionally, even with empty password) - Reorder: activate first, then legitimation (if password given) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace duplicate _skip_typed_value (73 lines) with codec.skip_typed_value import; fixes DWORD bug (was VLQ instead of fixed 4 bytes) - Initialize _session_auth_public_key/_session_auth_family in __init__ instead of using hasattr() guards - Remove redundant lazy imports (DataType, LegitimationId already at top level) - Make SessionKey and TLS auth mutually exclusive in client.connect() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gijzelaerr
force-pushed
the
fix-tls-layering-v2
branch
2 times, most recently
from
July 16, 2026 18:15
3a5660f to
6a3b019
Compare
Move the 95K+ lines of machine-transpiled monolith transforms, constant tables, and binary data into family0/_generated/ to clearly separate opaque generated code from the human-readable orchestration layer. Add ARCHITECTURE.md documenting: - When SessionKey auth is needed (V1-initial vs TLS) - The full authentication flow (diagram) - Module map: which file does what - How the 180-byte SecurityKeyEncryptedKey blob is built - References to upstream HarpoS7, Black Hat papers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Data2Collection table 1 had 2111 entries (4222 bytes) but its 33 operations require 2112 entries (4224 bytes). The missing final word 0x4C28 caused IndexError during session key renewal when a challenge selected the last lookup block. Backported from HarpoS7 commit 22b9dc0 (bonk-dev/HarpoS7#24). Adds: - The missing 0x4C28 entry in fp_data2.bin - Lookup table size invariant test - Regression test with captured renewal challenge 5B15B469... Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Analysis of all 4 TIA Portal pcaps (GH-710 + GH-728) shows TIA consistently sends 4 requests after SetupSession, not 2: 1. SET_VARIABLE attr 323 = USINT(5) 2. GET_VAR_SUBSTREAMED InObj=50, addr 7920 3. GET_VAR_SUBSTREAMED InObj=session, addr 1842 4. GET_VAR_SUBSTREAMED InObj=50, addr 7920 (again) Previously we only did steps 1-2. Add steps 3-4 and extract a _build_get_var_substreamed helper to reduce boilerplate in both _session_activate and _post_auth_legitimation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move _parse_create_object_response (120 lines, connection.py) into codec.py as parse_create_object_attributes — a reusable function that returns ServerSessionVersion, public key fingerprint, and session challenge in a CreateObjectAttributes dataclass. Both sync connection and async_client can use it. - Keep parse_server_session_version as a thin wrapper for backward compat. - Fix: add session_auth/**/*.bin to pyproject.toml package-data so binary data files (fp_data, transform12 metadata) ship in wheels. Net: -69 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Hi, I'm the creator of HarpoS7. I see that you're porting it to Python which is awesome! However, could you please include the original license text somewhere in your repository?: LICENSE-HarpoS7 |
Owner
Author
|
hey! thanks for making HarpoS7! But i don't fully understand your request. the licenses we put on our software is the same, it's both MIT right? https://github.com/gijzelaerr/python-snap7/blob/master/LICENSE Or are you talking about credits? I think the parts that are ported from HaroS7 should indicate it comes from your source code, but i happily do another iteration and check credit is given where applicable. I'll aso add you to the credits file/section. Also, i'm not 100% convinced yet that the SessionKey handshake for V1 s7commplus should land in this package, since it is very reverse-engineery with tons of binary blobs and magic. I've been trying to polish it, but there is still quite some magic. Maybe my cleanups can be helpful on your side again :) Let me know if i can help you out in another way. |
The session_auth package is a Python port of HarpoS7 by bonk-dev. Include the original MIT license text as requested in #775, and add credits in the changelog and docs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GH-710) The 4-step activation sequence (SET_VARIABLE attr 323 + finalize reads) was modeled after TIA Portal pcaps but is TIA-specific behavior — the HarpoS7 reference implementation skips it entirely and goes straight from SetupSession to data operations with V3+HMAC framing. V1-initial PLCs (FW < 4.5) RST the connection when they receive the SET_VARIABLE request, and the finalize read to addr 7920 in _post_auth_legitimation caused V254 SYSTEM_EVENT error 0xE9. Remove both and align with HarpoS7's proven flow: SetupSession → optional legitimation (read 303, write 1846) → data ops. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…-710) Real S7-1200 testing by @xBiggs showed that the PLC requires the legitimation handshake after SetupSession even when no password is configured. Without it, db_read fails with access-denied error 0xE03388000081FFE0. The legitimation solver already handles an empty password correctly (SHA-1 of ""), so the conditional guard was simply wrong. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…H-710) The GET_VAR_SUBSTREAMED response for the legitimation challenge (addr 303) has the format: UInt64Vlq ReturnValue | byte | PValue(datatype + count_vlq + length_vlq + data) | UInt32Vlq IntegrityId. The code was skipping only the 1-byte return value VLQ, then taking the next 20 bytes as the challenge — but those first 4 bytes (00 10 02 14) are BLOB metadata (unknown byte, datatype tag, count, length). The actual challenge starts 4 bytes later. This fed wrong input to the legitimation solver, producing a wrong DEADBEEF blob that the PLC correctly rejected (ReturnValue 0xE605CC0004C6FEA1 = negative = error). The code ignored this error and logged "completed", then all subsequent data reads failed with V254 SystemEvent error 0xE9. Fix both issues: - Parse the PValue BLOB header properly to extract the challenge at the correct offset (matching thomas-v2 GetVarSubstreamedResponse) - Check the SET_VAR_SUBSTREAMED response return value and raise on negative (= legitimation rejected) Root-caused from @xBiggs's real S7-1200 (6ES7 215-1BG40-0XB0, FW V4.2) debug traces. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
V1-initial PLCs accept GET_VAR_SUBSTREAMED (0x0586) for session-level operations (legitimation challenge/response), but reject it for DB data reads with V254 SystemEvent error 0xE9. This matches the thomas-v2 S7CommPlusDriver which uses GetMultiVariables (0x054C) for all data reads, even over TLS/V2. Change requires_substreamed to return False so data operations route through GetMultiVariables. V3+HMAC framing is controlled separately by session_key presence in send_request, so HMAC signing is preserved. Tested by @xBiggs on S7-1200 (6ES7 215-1BG40-0XB0, FW V4.2) — legitimation now succeeds (positive return value) after the challenge extraction fix in 1c0cae5, but db_read still hit V254/0xE9 because GET_VAR_SUBSTREAMED was used for the data read. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…hange (GH-710) xBiggs's TIA Portal pcap (TIAPortalWatchDB7.pcapng) proves that TIA sends SET_VARIABLE writing USINT(5) to address 323 immediately after the V2 key exchange, BEFORE any data reads or legitimation. Without this step, data reads fail with 0xE9. Previous attempt (432d9c6) bundled this with 3 extra GET_VAR_SUBSTREAMED reads (addr 7920, 1842) that are NOT in TIA's flow at that point. The combined 4-step sequence caused RST and was removed in e73f915. This version sends only the SET_VARIABLE, matching pcap frame 17 exactly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…change (GH-710) xBiggs's test of a4be9de showed the SET_VARIABLE at addr 323 returning V254/0xE9. Byte-by-byte comparison with the TIA Portal pcap revealed two differences: 1. IntegrityId not included — _with_integrity_id was never set to True for V1-initial PLCs because the V2 post-setup code path doesn't run. TIA's frame 17 has IntegrityId=4, ours had none. 2. Transport flags 0x36 instead of 0x34 — TIA uses 0x34 for ALL requests after the key exchange (SET_VARIABLE, GET_VAR_SUBSTREAMED, etc.). Our code only used 0x34 for GET_MULTI_VARIABLES and EXPLORE. Fix: enable IntegrityId tracking immediately when the SecurityKey blob is accepted, and use 0x34 transport flags for all requests when a session key is active. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The IntegrityId insertion and counter increment were gated behind _protocol_version >= V2, which prevented V1 PLCs that complete SessionKey authentication from receiving the IntegrityId in their request frames. The _with_integrity_id flag alone is sufficient since it's only set when IntegrityId is actually required. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
V1-initial PLCs reject SET_VARIABLE addr 323 with error 0xE9 in a V254 SystemEvent — the session activation is TIA-specific behavior that doesn't apply to V1 PLCs (documented in ARCHITECTURE.md but contradicted by the connect flow comments). After SetupSession with the SecurityKeyEncryptedKey blob, V1+SessionKey PLCs are ready for data operations immediately with V3+HMAC framing. The legitimation handshake is only needed when a password is provided. Confirmed by xBiggs's trace: the V254 response to SET_VARIABLE contained ReturnValue=0xE9, followed by PLC RST on the next request.
…-710) V1+SessionKey PLCs accept GET_VAR_SUBSTREAMED for session-level ops (legitimation) but return errors for DB data reads. This matches the thomas-v2 S7CommPlusDriver which uses GET_MULTI_VARIABLES for all data reads regardless of protocol version. V3+HMAC framing is controlled separately by session_key presence in send_request.
…H-710) Real PLCs prepend an IntegrityId VLQ to the response payload when integrity tracking is active. Strip it in send_request so callers see only the application-level data (ReturnValue + items + errors). Also format error codes as hex in _parse_read_response for easier debugging against S7CommPlus protocol documentation.
Byte-for-byte comparison with TIA Portal pcap (frame 17) revealed two issues in the ObjectQualifier that caused the 0xE9 rejection: 1. KEY_QUALIFIER value was VLQ(0) (1 byte) — must be uint32 containing the frame sequence number (4 bytes) 2. OQ terminator byte was redundant — the IntegrityId takes its position when send_request splices it with integrity_tail=3 Session activation (SetMultiVariables addr 323) and post-auth legitimation (GetVarSubstreamed) are restored in the connect flow.
# Conflicts: # pyproject.toml
… RST-happy firmware
browse() does an EXPLORE (list_datablocks) then a symbolic GetMultiVariables
read (_read_typeinfo_rid) then another EXPLORE (type-info container). On
firmware that sends a TCP RST after the first symbolic read per connection
(e.g. S7-1200 FW V4.1 — the behaviour already documented in read_symbolic's
docstring), the final EXPLORE runs on a dead socket and browse() raises
S7ConnectionError("Not connected").
Add a lazy reconnect-and-retry: connect() now stores its arguments, and a new
_with_reconnect() helper retries an operation once on a fresh session if the
PLC dropped the socket. browse() wraps the per-DB reads and the container
EXPLORE with it; _read_typeinfo_rid propagates S7ConnectionError so the retry
can act. Well-behaved firmware never triggers the retry (the first call
succeeds), so there is no behaviour change there.
Also downgrade the misleading "CreateObject returned error ... PLC may require
TLS" warning to debug when TLS is already active: this PLC returns a non-zero
CreateObject value on a fully functional TLS session.
Validated live on an S7-1200 FW V4.1 over TLS: browse() now returns the full
I/Q/M symbol tree with correct types (input_1 BOOL, mtag_byte BYTE %MB100,
mtag_word WORD %MW102, ...). Tests: 107 passed, ruff + format clean.
Refs #793, #775
Merged
4 tasks
…connect Add `rst_after_symbolic_read` flag to S7CommPlusServer that closes the TCP connection after responding to GetMultiVariables — matching the behaviour of S7-1200 FW V4.1 that RSTs after a symbolic read. This lets the reconnect logic from PR #801 be exercised without real hardware.
Contributor
|
Hardware validation — TLS path on real S7-1200 (re #793) I validated the TLS layering on this branch against real TLS-required hardware: a Siemens S7-1200, FW V4.1, with secure PG/PC communication enabled (so TLS is mandatory, not optional). This covers the "validate against real hardware" ask in #793. What works end-to-end (validated live):
One issue found and fixed (already merged here as #801): Minor notes (not blockers): the Net: the TLS transport on this branch is solid on real V4.1 hardware over TLS. 👍 |
Contributor
|
As it stands, this PR breaks compatibility with previously working PLCs from master (after merging master into this branch). Here is the behaviour on master on a variable browse: Here is the behaviour on this branch: StacktraceI have also tried the legitimation without success (different PLC, not working on master nor this branch), it fails earlier: |
Co-Authored-By: Codex <noreply@openai.com>
Owner
Author
|
@bvanelli Thanks for the traces — they exposed two distinct framing problems. Fixed in 2f27f37:
Local verification: 1,759 passed, 82 skipped; full pre-commit suite passed; sdist and wheel builds passed. Could you retest both the two TLS PLCs and the password-protected PLC when convenient? |
Contributor
|
Only partially worked now. The connect now triggers a loop that reconnects multiple times. Even when the browse succeeds, the read immediately fails afterwards. Stack traceRegarding the |
Owner
Author
|
@bvanelli Thanks for the retest — I found a regression in the previous merge resolution. TLS/V2 responses prepend the IntegrityId before the application payload, but 2f27f37 left it in place. That made successful symbolic reads parse as errors; this PLC then reset the socket, causing the repeated reconnects. Fixed in 33a4c9e, with a wire-level regression test. The full local test suite, pre-commit checks, and package build pass. Could you retest the browse followed by at least two consecutive symbolic reads? I left the separate GetVarSubStreamed legitimation failure out of this change pending your binary capture. |
Contributor
|
@gijzelaerr some findings: The legitimation issue is completely unrelated, and I'll submit a PR with contract testing once you merge this one. I'll focus on the backwards compatibility here instead. The breaking compatibility is related to the changes in diff --git a/s7commplus/codec.py b/s7commplus/codec.py
index 17b04a8..237998e 100644
--- a/s7commplus/codec.py
+++ b/s7commplus/codec.py
@@ -324,7 +324,11 @@ def encode_object_qualifier(key_qualifier: int = 0) -> bytes:
result += bytes([0x00, DataType.AID]) + encode_uint32_vlq(0)
# KeyQualifier = UDInt(key_qualifier)
result += encode_uint32_vlq(Ids.KEY_QUALIFIER)
- result += bytes([0x00, DataType.UDINT]) + struct.pack(">I", key_qualifier)
+ # Framing for the V1 (unconfirmed by me)
+ # result += bytes([0x00, DataType.UDINT]) + struct.pack(">I", key_qualifier)
+ # Framing for the V2 (confirmed by me)
+ result += bytes([0x00, DataType.UDINT]) + encode_uint32_vlq(key_qualifier)
+ result += b"\x00" # trailing zero byte, as emitted by S7p.EncodeObjectQualifier
return bytes(result)You can check that the new framing has different sizes, and I believe that that is causing the connection to be reset: >>> struct.pack(">I", 0) # produces a fixed size of 4 bytes
b'\x00\x00\x00\x00'
>>> encode_uint32_vlq(0) # produces variable size
b'\x00'
>>> encode_uint32_vlq(2555)
b'\x93{'The V2 version agrees with the original source code for S7p.EncodeObjectQualifier. The new V1 version needs to match this As a third problem, the change introduce in 33a4c9e is a regression. I don't think is needed at all, as it causes: Reverting it back to Let me know if you need more input (or message me directly). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #761, updated onto the s7commplus/ package split (#774).
Summary
Adds the full
session_authpackage for V1-initial S7-1200 PLCs that require a SessionKey handshake before data operations:s7commplus/session_auth/: public-key-based session authentication (Family 0 key derivation, fingerprint matching, SecurityKeyEncryptedKey blob generation, legitimation challenge solver, HMAC key derivation)Connection.connect(password=)parameter for auth flowTest plan
Known limitations
01:prefix) public keys are included