Rework S7Comm dissector; add S7Comm Plus support #2165
Conversation
| "s7comm", NDPI_PROTOCOL_CATEGORY_NETWORK, | ||
| ndpi_build_default_ports(ports_a, 102, 0, 0, 0, 0) /* TCP */, | ||
| "S7Comm", NDPI_PROTOCOL_CATEGORY_IOT_SCADA, | ||
| ndpi_build_default_ports(ports_a, 0, 0, 0, 0, 0) /* TCP */, |
There was a problem hiding this comment.
TCP port 102 is assigned to the TSAP, so I set it to 0 to avoid false positives when guessing. Yea, S7Comm uses this port, but I left the check inside the dissector.
|
@IvanNardi could you please do a review? Btw, I can share the rest of my industrial/SCADA stuff as well if that would be useful. |
That would be great. I was trying to implement more industrial/IoT protocols. But I do not have lot's of related PCAPs. |
Cool, then I'll go clean up my stuff and create pull requests |
|
@0xA50C1A1, could you rebase, please? |
+1 |
Sure, but not all RRs at once 😃 |
|
Kudos, SonarCloud Quality Gate passed!
|
|
@0xA50C1A1, what do you think of my change? |
Not bad. The number of dissector calls is reduced by 3 on average. |
Please sign (check) the below before submitting the Pull Request:
Describe changes:
Reworked the old S7Comm protocol dissector so it shouldn't give false positives now. Added support for the S7Comm Plus protocol under a new protocol id as it is completely different from classic S7Comm, the only similarity is that both use TPKT/COTP as transport.