-
Notifications
You must be signed in to change notification settings - Fork 309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Parser for ModuleIdentificationRequest of s7 Protocol #423
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nitpicks, looks great overall for code quality + documentation.
To be perfectly honest, I've been unable to convince myself that I understand the Index W#16#XY
format that Siemens is using and haven't found any guides online. Like for example, Index W#16#0006 and W#16#0007: remaining numbers of the version ID
, does this refer to bits 6 and 7? The use of the word "index" makes me think it's referring to a specific bit in the word, but that seems very arbitrary for a 16 bit value. I'm unsure what else it could mean, but also don't feel confident in it being bits 6 and 7.
If you found some better explanations in your digging, lmk, otherwise IMO we should wait for a real Siemens device to test against to confirm understanding.
Co-authored-by: Phillip Stephens <phillip@cs.stanford.edu>
Co-authored-by: Phillip Stephens <phillip@cs.stanford.edu>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for your work on this
I was just reminded to this. I wrote an s7 Parser some years ago and did some larger measurements. I also used the "Indexes 6 and 7", but also found an additional one, "129", which was so common, that I added it.
(Sorry for the dodgy quality, it was my first go code written in a nighty hurry...) Some documentation I wrote back in the day: The source of the Siemens data was: I can also recommend getting an account at https://support.industry.siemens.com/cs/start?lc=de-DE, to get the manuals. This code was stable enough to conduct Interner-wide Measurements and the resulting IDs and numbers were checked against expected Siements behaviour (such as "legal and existing" patch levels and device IDs) and verififed as far as possible to detect honeypots. |
@Valentinbist Thank you for sharing all these details! These are really helpful and we'll patch our parser accordingly soon. Btw, did the |
Rewrote response parser with information provided in the issue. Resolves #212.
Tested against some real S7 modules and appears to work, absolutely makes more sense than the original code. However, would be great if we can confirm on a device we control.
Also,
ModuleId
field intype S7Log struct
formerly reflects "Order number of the module". This behavior is kept in current version for forward compatibility but not sure if we should rename it to sth else.