Skip to content

enip: convert to rust#9824

Closed
catenacyber wants to merge 1 commit intoOISF:masterfrom
catenacyber:enip-rust-3958-v1
Closed

enip: convert to rust#9824
catenacyber wants to merge 1 commit intoOISF:masterfrom
catenacyber:enip-rust-3958-v1

Conversation

@catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/3958

Describe changes:

  • convert enip parser to rust

Alon the way, also

  • enip_command keyword accepts now string enumeration as values.
  • transactions are now bidirectional
  • there is a enip logger
  • gap support is improved with probing for resync
  • SEQUENCE_ADDR_ITEM value is fixed to 0x8002 instead of 0xB002

Draft as is this is not complete :

  • need to fix S-V number of alerts for enip-keywords (off by one)
  • add more S-V test for enip_command with string , for logger, complete json schema
  • need better response/request association for bidirectional transactions
  • get cip attributes by parsing deeper based on cip service, and reuse that for cip_service keyword
  • parse more, log more, add more keywords (enip.status)...
  • set event on parsing error
  • use frames ?
  • take a look into https://redmine.openinfosecfoundation.org/issues/6304

Ticket: 3958

- enip_command keyword accepts now string enumeration as values.
- transactions are now bidirectional
- there is a logger
- gap support is improved with probing for resync
- SEQUENCE_ADDR_ITEM value is fixed to 0x8002 instead of 0xB002
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 16594

match seg.segment_type {
ENIP_CIP_PATH_CLASS_8BIT | ENIP_CIP_PATH_CLASS_16BIT => {
if seg.value == class {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working locally, will try CI with MSRV next PR ;-)

@catenacyber
Copy link
Contributor Author

Replaced by #9844

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants