Project

General

Profile

Actions

Feature #3957

closed

Convert protocol to Rust: Modbus

Added by Shivani Bhardwaj over 3 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Effort:
Difficulty:
Label:

Description

Modbus implementation already exists for Suricata but in order to make it more secure and trustworthy, we'd like to convert it to Rust.

Below is a (very) brief step by step overview of going about the implementation:

  1. Get to know more about the protocol from the official RFC.
  2. Match the RFC defined header structs and handling with C code and understand and note down the flow and handling.
  3. Create basic header structures in Rust. (These do not have to be same as C but should follow the protocol standards (e.g. alignment and data types of certain fields) as per the RFC)
  4. Use test data from C for headers and try to write a parser such that those tests pass as they did in C.
  5. After the header parsers work in Rust, move to UDP implementation of packet handling (start with creating a <Protocol>State struct to store all important info).
  6. Try to replicate UDP handling in C for the protocol and make unit tests pass for one transaction only.
  7. Integrate with C, make sure the tests pass.
  8. Find PCAPs for the application layer protocol over UDP covering all the modes (in case there are diff modes of operation for a protocol e.g. diff versions of protocol)
  9. Write a (few) suricata-verify tests (you could use createst.py to create those for you with the PCAP) with those PCAPS and make sure all the tests pass.

This completes UDP conversion of the protocol and some basic chassis for protocol over TCP (structures and header parsing).

10. Try and replicate the TCP implementation in C of the protocol.
11. Learn more about stream handling and how Suricata does internal reassembly of packets, etc.
12. Repeat steps 6-9 for TCP.

This completes TCP conversion of the protocol as well but all works only for one transaction.

13. Try adding more transactions to tests and make them pass.
14. Make sure suricata-verify tests pass too.

This completes multi-transaction handling of the protocols.

15. Check out TCP stream gap handling for other protocols and try to implement the same for the protocol in Rust.
16. Enable gap handling for the protocol in C and integrate Rust code.
17. Find out or create PCAPs with TCP stream gaps, add tests with them to suricata-verify, make sure they pass.

This almost completes the conversion of the protocol.

18. Enable logging for the protocol, convert json logging from C to Rust.
19. Test that logging results pass with `--enable-debug-validation` and jsonbuilder does not panic anywhere.
20. Write suricata-verify tests for the same, make sure they pass.

This completes the protocol conversion to Rust. Now, fix bugs as when they arrive. :)

Pro tips:
- Take inspiration from other protocols, smaller/popular protocols (e.g. DNS, SIP) are easier to follow.
- Git is your best friend. (e.g. to see how a particular protocol's conversion progressed check all the closed PRs corresponding to that, a lot of useful content might be there)


Related issues 4 (2 open2 closed)

Related to Suricata - Task #3195: tracking: rustify all inputNewOISF DevActions
Related to Suricata - Task #2778: tracking: port app-layer parsers to RustNewOISF DevActions
Related to Suricata - Feature #2096: eve: event_type for MODBUSClosedSimon DugasActions
Has duplicate Suricata - Optimization #2782: Convert Modbus from C to RustRejectedActions
Actions #1

Updated by Victor Julien over 3 years ago

  • Related to Task #3195: tracking: rustify all input added
Actions #2

Updated by Shivani Bhardwaj over 3 years ago

  • Related to Task #2778: tracking: port app-layer parsers to Rust added
Actions #3

Updated by Shivani Bhardwaj over 3 years ago

Actions #4

Updated by Shivani Bhardwaj over 3 years ago

Actions #5

Updated by Victor Julien over 3 years ago

  • Status changed from New to Assigned
  • Assignee set to Juliana Fajardini Reichow
  • Target version set to 7.0.0-beta1
Actions #6

Updated by Victor Julien over 3 years ago

Actions #7

Updated by Victor Julien over 3 years ago

Actions #8

Updated by Juliana Fajardini Reichow over 3 years ago

  • Status changed from Assigned to In Progress
Actions #9

Updated by Simon Dugas over 3 years ago

Looks like there is some good progress on this ticket with https://github.com/OISF/suricata/pull/5675

We have also been working on an implementation and plan to have a PR ready in January. It is currently going through internal code review and qa. It features moving the app-layer, logging and detection to rust.

The modbus parser itself will be a separate rust crate that is part of a library of crates for security-aware protocol parsers.

Our apologies for not updating the ticket when we started development a couple months ago :).

Actions #10

Updated by Victor Julien over 3 years ago

Hi Simon, thanks for the update. Appreciate the work you are doing. At the same time, this is a bit tricky for us as Juliana is working on this as part of her internship with us for Outreachy. We'll have to look at how to handle that as I'd hate there to be duplication of efforts. Are there other things you plan to contrib that are not reflected by tickets yet?

Actions #11

Updated by Simon Dugas over 3 years ago

Hi Julien. Whichever way you decide to move forward is good with us.

There are a bunch of protocols we are looking to implement down the road but the ones we are actively working on are modbus (January 2021), diameter for LTE/4G (March 2021) and TCAP/MAP for SS7 (April 2021). I can't find redmine tickets for these at the moment.

For other protocols like ENIP and DNP3, we can notify you via redmine once they are assigned to a developer on our end to avoid duplication.

Actions #12

Updated by Victor Julien over 3 years ago

Thanks Simon. Could you create tickets for those plans?

Actions #13

Updated by Juliana Fajardini Reichow over 3 years ago

  • Assignee changed from Juliana Fajardini Reichow to Simon Dugas

Changing assignment to Simon Dugas, based on latest comments and agreements.

Actions #14

Updated by Simon Dugas over 3 years ago

Thank you, our pull request is coming soon for this.

The tickets for SS7 and diameter have been created:
Actions #15

Updated by Simon Dugas about 3 years ago

Pull request is open: https://github.com/OISF/suricata/pull/5750
You are also welcome to review our parsing library: https://github.com/CybercentreCanada/sawp

Actions #16

Updated by Philippe Antoine about 3 years ago

  • Status changed from In Progress to In Review
Actions #17

Updated by Simon Dugas almost 3 years ago

Pull request https://github.com/OISF/suricata/pull/6090 has been merged.

We are still finalizing lower priority fixes from the previous code review comments:
1. Fix for write multiple coils check identified in https://github.com/OISF/suricata/pull/5770
2. Adding a more detailed suricata-verify test to follow https://github.com/OISF/suricata-verify/pull/486

Actions #18

Updated by Victor Julien almost 3 years ago

  • Status changed from In Review to Closed
Actions

Also available in: Atom PDF