cisagov/icsnpp-modbus

Zeek Modbus Extension Scripts - CISA ICSNPP

icsnpp-modbus's Issues

Request to add Modbus Transaction ID

💡 Summary

Merge requests and responses into a single line (most effort) or just add Modbus Transaction ID to outputs (less effort)

Motivation and context

Amazing script guys! Anyone that can wrangle zeek scripts is at another level! I love the fact that you are parsing both requests and responses, however it is hard to tell which requests belong to which responses when they are not sequential, which for me is most of the time.

Implementation notes

In my perfect world, I would love to see both requests and responses merged into a single line, which would provide more context and cut the size of the modbus_detailed.log file to about 50% of its current size for the same traffic. This would be a bit more coding effort to pull off, but I think provide the most value.

Your current output includes:

• ts
• uid
• id.orig_h
• id.orig_p
• id.resp_h
• id.resp_p
• unit_id
• func
• network_direction
• address
• quantity
• values

The problem currently is:

1. There is no way to associate a request with its response, short of assuming they are sequential, which they often are not, especially when modbus systems use multiple modbus payloads per TCP packet.
2. Most of those fields are repeated for both request and response, however the address field only appears in requests, and in the case of reads, the values only appear in the response, making it hard to map the two without transaction IDs.

My prefered solution would be to merge requests and responses into a single line with a table format like

• ts (timestamp of original request, unless implementation requires this to be response)
• uid
• id.orig_h
• id.orig_p
• id.resp_h
• id.resp_p
• response_ts (new field showing timestamp of req or resp depending on use in ts field)
• unit_id
• transaction_id (new field)
• func
• network_direction (remove this field as its no longer needed)
• address
• quantity
• values

However if that is too difficult to do in zeek, or if there is some other reason both requests and responses are needed separately, then simply adding the transaction_id field would be sufficient to address the problems.

Dropped connection/logs when invalid WRITE_COIL data sent

🐛 Summary

Ignore ANALYZER_VIOLATION errors to continue parsing of Modbus traffic.

To reproduce

Steps to reproduce the behavior:

Modbus traffic that has an invalid WRITE_COIL value such as 0x0001 (only two expected values are 0x0000 and 0xFF00).

Expected behavior

Modbus logs should continue to log data from the connection, but currently after receiving multiple bad values, the entire connection itself is no longer logged which can lead to good/correct Modbus traffic not being logged.

Fix

Because this project/script only extends functionality of Zeek's default Modbus parser and does not control the parser itself, we are unable to modify any code within the Modbus parsing/PAC files themselves. However, adding the following line to main.zeek will ignore these ANALYZER_VIOLATIONS and cause the parser to continue to parse connections as intended even when invalid WRITE_COIL values are sent:

redef DPD::ignore_violations += { Analyzer::ANALYZER_MODBUS };

look at new modbus analyzer features in v6.1.0 and take advantage of them if applicable

the zeek v6.1.0 release has some new modbus stuff:

The ModBus analyzer's function support was expanded, with new handling of the
Encapsulation Interface Transport (function 28) And Diagnostics (function 8)
functions. This adds new modbus_encap_interface_transport_{request,response}
and modbus_diagnostics_{request,response} events.

The ModBus file record read and write events now provide the full data from
the request and response messages as part of the event data.

The full PDU length was added to the ModBusHeader record type passed with
all of the ModBus events.

We ought to look at these changes and see if any of them could and/or should be reflected in the various modbus logs being generated by this plugin.