Conversation
|
NOTE: This PR may contain new authors. |
The EnumStringU8 and EnumStringU32 derives both register the suricata attribute, but EnumStringU16 does not, so a u16 enumeration cannot select a string style even though stringenum.rs handles every width the same way. Register the attribute for u16 as well. Ticket: OISF#8131.
Add uint keywords for the Modbus MBAP header values already present in the EVE log: modbus.unit_id, modbus.transaction_id and modbus.protocol_id. They follow the ENIP keyword pattern: uint matching with ranges and negation, inspecting the request in to-server direction and the response in to-client direction. Ticket: OISF#8131.
Add uint keywords for the Modbus function-level values already present in the EVE log: modbus.function (function code), modbus.subfunction (diagnostic subfunction code) and modbus.exception_code (exception code in error responses). They match numerically, with ranges and negation, and also accept the code names that the EVE log prints in function_code, diagnostic.code and exception.code, so a name read from a log can be used as is in a rule. Names are matched case insensitively and can be negated. The catch-all variants of the sawp enumerations are deliberately left out of the names: sawp assigns them to any value it cannot decode, but their discriminants collide with real codes, so accepting them by name would match something other than what the name says. Error responses carry the request function code with its highest bit set, while the logged name is derived from the code without that bit. A name therefore does not match an error response; the numeric form does. Ticket: OISF#8131.
Add uint keywords for the Modbus data-access values already present in the EVE log: modbus.read.address, modbus.read.quantity, modbus.write.address, modbus.write.quantity and modbus.write.value. Read keywords match read request headers. Write keywords also match the echoed values in write responses, following what is logged. Multiple-write responses echo a quantity where single-write responses echo a value, so modbus.write.quantity matches that echo and modbus.write.value only matches single writes. Ticket: OISF#8131.
Document modbus.unit_id, modbus.transaction_id, modbus.protocol_id, modbus.function, modbus.subfunction, modbus.exception_code, modbus.read.address, modbus.read.quantity, modbus.write.address, modbus.write.quantity and modbus.write.value, naming the EVE field each one is taken from and how error responses and write response echoes are matched. List the names accepted by modbus.function, modbus.subfunction and modbus.exception_code. Say that the names logged for codes that have none are not accepted, as they stand for more than one value, and that a name does not match the error response of a request, since the logged name drops the error bit that the matched value carries. Note how the address keywords differ from the 1-based addressing of the modbus keyword in the sections of the two keywords concerned, rather than once for the whole set. Ticket: OISF#8131.
Map the new modbus per-value keywords to the EVE fields they match. Also add the write.quantity, write.and_mask and write.or_mask properties and allow string data in multiple-write requests, matching what the modbus logger already emits, so that alerts embedding such transactions validate against the schema. Ticket: OISF#8131.
akekulip
force-pushed
the
modbus-keywords-8131-v5
branch
from
September 21, 2026 15:24
d4f3687 to
737b45f
Compare
5 tasks
|
NOTE: This PR may contain new authors. |
akekulip
marked this pull request as ready for review
September 21, 2026 15:32
akekulip
requested review from
a team,
jasonish and
jufajardini
as code owners
September 21, 2026 15:32
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
1 participant
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.
Make sure these boxes are checked accordingly before submitting your Pull Request -- thank you.
Contribution style:
https://docs.suricata.io/en/latest/devguide/contributing/contribution-process.html
Our Contribution agreements:
https://suricata.io/about/contribution-agreement/ (note: this is only required once)
Changes (if applicable):
(including schema descriptions)
https://redmine.openinfosecfoundation.org/projects/suricata/issues
Link to ticket: https://redmine.openinfosecfoundation.org/issues/8131
Previous PR: #16108
SV_BRANCH=OISF/suricata-verify#3260
Changes since v4:
well as numbers, as requested in review
jobs failed and every build, suricata-verify and schema job was skipped. v4 had no CI.
git rev-list --reverse origin/main..., a symmetric difference, so it tried to build 93revisions and was cancelled at the 6 hour limit. This branch keeps that walk to 6.
Describe changes:
modbus.transaction_id, modbus.protocol_id, modbus.function, modbus.subfunction,
modbus.exception_code, modbus.read.address, modbus.read.quantity, modbus.write.address,
modbus.write.quantity and modbus.write.value
request matched to-server and response to-client; the legacy modbus keyword is unchanged
logged in function_code, diagnostic.code and exception.code
logger prints the sawp variant names rather than EnumString output. With snake_case the
log would say RdCoils and the rule rd_coils. This differs from dnp3.func, snmp.pdu_type
and ntp.mode, so say if you would rather have snake_case
ExceptionCode::Unknown is 12 and DiagnosticSubfunction::Reserved is 22, all real codes.
Those values still match numerically
An error response to function 8 logs function_raw 136 with function_code Diagnostic, so
a name matches the request but not its error response, where >127 or 136 does. This is
documented. Masking the bit would break modbus.function:>127
EnumStringU32, and modbus.subfunction is a u16. Fixed in the first commit
modbus.write.quantity matches that echo and modbus.write.value only matches single
writes; documented, along with the difference from the 1-based addressing of the legacy
keyword
and write.or_mask properties and allow string data in multiple-write requests, which the
modbus logger already emits