Bug #5464
closedeve: if alert and drop rules match for a packet, "alert.action" is ambigious
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.
Updated by Victor Julien almost 2 years 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".
Updated by Victor Julien almost 2 years ago
Could also be addressed by adding a new (final) "verdict" field.
Updated by Juliana Fajardini Reichow almost 2 years ago
Following the discussion, I tend to prefer the verdict field, too.
Would this be part of the "packet" event?
Updated by Juliana Fajardini Reichow almost 2 years ago
- Status changed from Assigned to In Review
PR for review: https://github.com/OISF/suricata/pull/8318
Updated by Shivani Bhardwaj almost 2 years ago
- Label deleted (
Needs backport to 6.0)
Updated by Juliana Fajardini Reichow almost 2 years 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).
Updated by Jamie Lavigne almost 2 years 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
Updated by Victor Julien almost 2 years 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.
Updated by Victor Julien almost 2 years ago
- Target version changed from 7.0.0-rc1 to 7.0.0-rc2
Updated by Juliana Fajardini Reichow over 1 year ago
To add: Have verdict as an optional new eve field but also as part of the alert event.
Updated by Victor Julien over 1 year ago
- Target version changed from 7.0.0-rc2 to 7.0.0
Updated by Juliana Fajardini Reichow over 1 year ago
- Status changed from In Review to In Progress
Updated by Juliana Fajardini Reichow over 1 year 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
Updated by Juliana Fajardini Reichow over 1 year ago
- Status changed from In Progress to In Review
Updated by Victor Julien over 1 year ago
- Related to Task #6084: output/alert: enable logging `PASS` alerts added
Updated by Juliana Fajardini Reichow over 1 year ago
- Related to Feature #6210: outputs: add verdict event type added
Updated by Victor Julien over 1 year ago
- Status changed from In Review to Resolved
Updated by Victor Julien over 1 year ago
- Related to Bug #5794: eve: if alert and drop rules match for a packet, "alert.action" is ambigious (6.0.x backport) added
Updated by Victor Julien over 1 year ago
- Status changed from Resolved to Closed