Project

General

Profile

Actions

Bug #5464

closed

eve: if alert and drop rules match for a packet, "alert.action" is ambigious

Added by Victor Julien over 1 year ago. Updated 8 months ago.

Status:
Closed
Priority:
High
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

The alert record produced for the alert rule will say "allowed". The alert record produced for the drop rule will say "blocked". There is no indication which of these "won" in the alert record itself.

I think the "allowed" for alert is a bit misleading. Alert is passive so it sets no action. So this could perhaps be changed into something more appropriate, like "action: alert".

Additionally it might be a good idea to list the action that was applied to the packet in the record separately, as the authoritative field to indicate what the decision of suricata on this packet was.


Related issues 3 (2 open1 closed)

Related to Suricata - Task #6084: output/alert: enable logging `PASS` alertsAssignedJuliana Fajardini ReichowActions
Related to Suricata - Feature #6210: outputs: add verdict event typeNewJuliana Fajardini ReichowActions
Related to Suricata - Bug #5794: eve: if alert and drop rules match for a packet, "alert.action" is ambigious (6.0.x backport)ClosedJuliana Fajardini ReichowActions
Actions #1

Updated by Victor Julien over 1 year ago

  • Status changed from New to Assigned
  • Assignee changed from OISF Dev to Juliana Fajardini Reichow
  • Target version changed from TBD to 7.0.0-rc1
  • Label Needs backport to 6.0 added

If a drop rule and a alert rule both match, the alert record for the alert rule will say allowed, even if the packet will be dropped because of the drop rule. Perhaps it should say action "alerted".

Actions #2

Updated by Victor Julien over 1 year ago

Could also be addressed by adding a new (final) "verdict" field.

Actions #3

Updated by Juliana Fajardini Reichow over 1 year ago

Following the discussion, I tend to prefer the verdict field, too.
Would this be part of the "packet" event?

Actions #4

Updated by Juliana Fajardini Reichow over 1 year ago

  • Status changed from Assigned to In Review
Actions #5

Updated by Shivani Bhardwaj about 1 year ago

  • Subtask #5794 added
Actions #6

Updated by Shivani Bhardwaj about 1 year ago

  • Label deleted (Needs backport to 6.0)
Actions #7

Updated by Juliana Fajardini Reichow about 1 year ago

To address this, I'm working on (possibly) adding a new field to events such as `drop` and
`alert`, which indicates what was the final verdict by Suri for the packet that
triggered the `drop`/`alert`. It is currently called `verdict`, and is
structured as (json resumed):

{
  "pcap_cnt": 10,
  "event_type": "drop",
  "direction": "to_server",
  "drop": {
  ...
    "reason": "flow drop" 
  },
  "verdict": "drop" 
}

and

{
  "pcap_cnt": 4,
  "event_type": "alert",
  "proto": "TCP",
  "pkt_src": "wire/pcap",
  "alert": {
    ...
  },
  "verdict": "reject" 
}

Currently, the possible values are "accept", "drop" or "reject".

We'd like to get feedback on this new output field: field values, field name etc. That's because once something is introduced to our output, it's much harder to get rid of it, so we wanna be sure(r).

Actions #8

Updated by Jamie Lavigne about 1 year ago

Would it make sense to use a value of "pass" instead of "accept" in order to be consistent with the terminology of the rule actions? Both "drop" and "reject" are consistent with those.

It's unfortunate that the field name "action" in the output is already taken because that would be consistent with what the rules format calls that value: https://suricata.readthedocs.io/en/suricata-6.0.0/rules/intro.html#action

Actions #9

Updated by Victor Julien about 1 year ago

I feel pass would actually be confusing, as it has a different meaning than issuing a IPS verdict. Pass in detection means the packet is processed but not inspected against rules. Accept/drop/reject are more per packet verdicts indicating what happened to that packet wrt the IPS policy engine. For example, iptables uses ACCEPT/DROP/REJECT as well.

Actions #10

Updated by Victor Julien about 1 year ago

  • Target version changed from 7.0.0-rc1 to 7.0.0-rc2
Actions #11

Updated by Juliana Fajardini Reichow 11 months ago

To add: Have verdict as an optional new eve field but also as part of the alert event.

Actions #12

Updated by Victor Julien 10 months ago

  • Target version changed from 7.0.0-rc2 to 7.0.0
Actions #13

Updated by Juliana Fajardini Reichow 9 months ago

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

Updated by Juliana Fajardini Reichow 9 months ago

Jamie Lavigne wrote in #note-8:

Would it make sense to use a value of "pass" instead of "accept" in order to be consistent with the terminology of the rule actions? Both "drop" and "reject" are consistent with those.

It's unfortunate that the field name "action" in the output is already taken because that would be consistent with what the rules format calls that value: https://suricata.readthedocs.io/en/suricata-6.0.0/rules/intro.html#action

Took longer than expected, but I believe we have something very close to merge state, now, in case you want to have a look at how we're approaching this. :)
https://github.com/OISF/suricata/pull/9162

Actions #15

Updated by Juliana Fajardini Reichow 9 months ago

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

Updated by Victor Julien 9 months ago

  • Related to Task #6084: output/alert: enable logging `PASS` alerts added
Actions #17

Updated by Juliana Fajardini Reichow 9 months ago

  • Related to Feature #6210: outputs: add verdict event type added
Actions #18

Updated by Victor Julien 9 months ago

  • Status changed from In Review to Resolved
Actions #19

Updated by Victor Julien 8 months ago

  • Subtask deleted (#5794)
Actions #20

Updated by Victor Julien 8 months ago

  • Related to Bug #5794: eve: if alert and drop rules match for a packet, "alert.action" is ambigious (6.0.x backport) added
Actions #21

Updated by Victor Julien 8 months ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF