Project

General

Profile

Actions

Feature #5972

closed

Feature #5489: research: multi version rules; or version dependent rules

rules: "requires" keyword representing the minimum version of suricata to support the rule

Added by Brandon Murphy over 1 year ago. Updated 9 months ago.

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

Description

reference: https://redmine.openinfosecfoundation.org/issues/4067#note-35

The concept here is that a rule can have a keyword which contains an option used to indicate the minimum version of Suricata required to support the rule's logic. This feature will allow rule writers to indicate that a rule requires a specific version due to either features, bug fixes, etc. and would allow the rule to be published within an existing ruleset but only loaded by engines which are supported.

Ideally, the engine will self detect the engine's version, and when loading a rule which requires a newer version, will throw a warning and not load the rule (not an error)

This feedback to users is valuable and can help to increase adoption of newer versions of Suricata.


Subtasks 2 (0 open2 closed)

Feature #6524: rules: "requires" keyword representing the minimum version of suricata to support the rule (7.0.x backport)ClosedJason IshActions
Feature #6637: requires: add skipped rules to statsClosedJason IshActions

Related issues 3 (1 open2 closed)

Related to Suricata - Task #4067: http2: overload existing http keywords to support http/2ClosedPhilippe AntoineActions
Related to Suricata - Task #6443: Suricon 2023 brainstormAssignedVictor JulienActions
Related to Suricata - Bug #6656: detect/requires: assertion failed !(ret == -4)ClosedPhilippe AntoineActions
Actions #1

Updated by Philippe Antoine over 1 year ago

  • Status changed from New to In Review
  • Target version changed from TBD to 7.0.0
Actions #2

Updated by Philippe Antoine over 1 year ago

  • Related to Task #4067: http2: overload existing http keywords to support http/2 added
Actions #3

Updated by Philippe Antoine over 1 year ago

Use case : a rule can use alert http1 when Suricata version is above 6

Actions #4

Updated by Philippe Antoine over 1 year ago

  • Assignee changed from Philippe Antoine to OISF Dev
Actions #5

Updated by Juliana Fajardini Reichow over 1 year ago

  • Tracker changed from Feature to Task
  • Subject changed from create a rule keyword representing the minimum version of suricata to support the rule to research: rule keyword representing the minimum version of suricata to support the rule
  • Target version changed from 7.0.0 to 8.0.0-beta1
Actions #6

Updated by Victor Julien over 1 year ago

  • Status changed from In Review to New

As mentioned in the PR, I would like to see a bit bigger discussion on ruleset versioning.

Actions #7

Updated by Philippe Antoine 12 months ago

Questions :
- Can multiple rules have the same sid but different "Suricata versions" ?
- How would 3rd party tooling know which exact rule was used (the one for suri version 6 or version 7) ?

Remarks:
- Besides suricata version, the same kind of thing should be done for features (like availability of geoip, file magic...)

Proposals:
- add a new version keyword

alert http any any -> any any (http.uri; content: "abc"; requires: features:geoip, version:6.0.0-7.0.0 ;)

- use a key value in metadata keyword
alert http any any -> any any (http.uru; content: "abc"; metadata: features geoip, version 6.0.0-7.0.0 ;)

- jijna style
alert http any any -> any any (msg:"TEST"; {{ if suricata.version >= 7 }}new stuff;{{ else }}old stuff;{{endif}} sid:1; rev:1;)@ (can be also suricata style @alert http any any -> any any (version:6.0.0; http.uri; content: "abc"; version:7.0.0; http.uri; content: "def"; version:*; http.otheri; content: "abc";) )

- use special comments
#version >7
alert http any any -> any any (http.uri; content: "abc";)

- use new keywords in file parsing
if SURICATA_VERSION > 7
alert http any any -> any any (http.uri; content: "abc";)
endif

Actions #8

Updated by Philippe Antoine 12 months ago

  • Related to Task #6443: Suricon 2023 brainstorm added
Actions #9

Updated by Philippe Antoine 12 months ago

Preferred version for now seems

alert http any any -> any any (http.uri; content: "abc"; requires: features:geoid, version:6.0.0-7.0.0 ;)

Actions #10

Updated by Jason Ish 12 months ago

On the requires format, we cannot re-use ':' within.. So

requires: features geoip,foo version >=7.0.4
requires: version 6.0.0-7.0.0

One idea would be to take a version specification format like used in Rust crates, so we can use a library like https://crates.io/crates/versions.

Actions #11

Updated by Victor Julien 12 months ago

  • Parent task set to #5489
Actions #12

Updated by Victor Julien 12 months ago

  • Tracker changed from Task to Feature
  • Subject changed from research: rule keyword representing the minimum version of suricata to support the rule to rules: "requires" keyword representing the minimum version of suricata to support the rule
Actions #13

Updated by Victor Julien 12 months ago

alert tls any any -> any any (ja3; content:"...."; geoip:src,NL; \
    requires: feature ja3, feature geoip, version >=6.0.0, version <10.0.0; sid:1; rev:1;)
Actions #14

Updated by Victor Julien 12 months ago

  • Status changed from New to Assigned
  • Assignee changed from OISF Dev to Jason Ish
  • Priority changed from Normal to High
  • Target version changed from 8.0.0-beta1 to 7.0.3

We discussed at Suricon that a risk could be that gui's / post-processing tools might not like multiple identical sids in a rule file. However we realized this can already happen accidentally w/o error if one of the rules has a higher rev than others. So we've decided we feel it's important enough to add to 7.0.x. Targeting 7.0.3 now, might slip to 7.0.4 depending on other timelines.

Request to @Brandon Murphy : can you help test this during development?

Actions #15

Updated by Brandon Murphy 12 months ago

Victor Julien wrote in #note-14:

Request to @Brandon Murphy : can you help test this during development?

Sure.

I will say it is VERY unlikely ET will ever intentionally produce a rules file with duplicate sids.

Given the new keyword will not be present in 7.0.0, it will also go against ET practice to use it starting at a minor release. To ensure compatibility with all 7.x versions, we're unlikely to use it until 8.0.0 .

To document current behavior, when an unknown keyword is found, a non-fatal error is produced. Suricata skips over the invalid rule, but continues to load the other rules

Notice: suricata: This is Suricata version 7.0.1 RELEASE running in USER mode
Info: cpu: CPUs/cores online: 4
Info: suricata: Setting engine mode to IDS mode by default

<snip boring stuff>

Error: detect-parse: unknown rule keyword 'requires'.
Error: detect: error parsing signature "alert http any any -> any any (msg:"TEST"; flow:established,to_server; http.method; content:"POST"; requires: feature ja3; classtype:attempted-admin; sid:1; rev:1;)" from file /tmp/499d08574bf7c6c5_Nov-10-2023_00-50-23/dalton-custom.rules at line 1
Info: detect: 2 rule files processed. 67630 rules successfully loaded, 1 rules failed
Info: threshold-config: Threshold config parsed: 0 rule(s) found
Info: detect: 67633 signatures processed. 1310 are IP-only rules, 10927 are inspecting packet payload, 55360 inspect application layer, 0 are decoder event only
Info: pcap: Starting file run for /tmp/499d08574bf7c6c5_Nov-10-2023_00-50-23/pcaps/proxy-231103-180549_fixed.pcap
Notice: threads: Threads created -> RX: 1 W: 4 FM: 1 FR: 1   Engine started.
Info: pcap: pcap file /tmp/499d08574bf7c6c5_Nov-10-2023_00-50-23/pcaps/proxy-231103-180549_fixed.pcap end of file reached (pcap err code 0)
Notice: suricata: Signal Received.  Stopping engine.
Info: suricata: time elapsed 0.243s
Notice: pcap: read 1 file, 550 packets, 241653 bytes

<snip boring stuff>

Info: counters: Alerts: 13
Actions #16

Updated by Victor Julien 12 months ago

The errors are still errors. Most (serious) deployments will first test with -T and those errors will fail then. Same for --init-errors-fatal.

Actions #17

Updated by Victor Julien 12 months ago

Another thought from post-processing yesterdays session. Would it make sense to add support for protocols to check if a protocol is enabled?

requires: protocol dnp3;

This would allow the use of dnp3 in the rule w/o error if the protocol is disabled. Currently we do present an error if protocols/keywords are used for a disabled protocol.

Actions #18

Updated by Victor Julien 12 months ago

  • Target version changed from 7.0.3 to 8.0.0-beta1
  • Label Needs backport to 7.0 added
Actions #19

Updated by OISF Ticketbot 12 months ago

  • Subtask #6524 added
Actions #20

Updated by OISF Ticketbot 12 months ago

  • Label deleted (Needs backport to 7.0)
Actions #21

Updated by Jason Ish 12 months ago

I'm looking to only support the following version comparison operators:
- >
- >=
- <
- <=

I don't think ^ and ~ per semver make sense for us. And = probably has not much use, or could lead to confusion.

Actions #22

Updated by Victor Julien 11 months ago

I agree.

Actions #23

Updated by Jason Ish 11 months ago

  • Status changed from Assigned to In Review

Draft PR for some initial review: https://github.com/OISF/suricata/pull/9915

Actions #24

Updated by Jason Ish 11 months ago

  • Subtask #6637 added
Actions #25

Updated by Jason Ish 11 months ago

  • Status changed from In Review to Resolved

Merged to master.

Actions #26

Updated by Jason Ish 9 months ago

  • Related to Bug #6656: detect/requires: assertion failed !(ret == -4) added
Actions #27

Updated by Jason Ish 9 months ago

  • Status changed from Resolved to Closed

Closing. This is in git master (8.0) and 7.0.x now.

Actions

Also available in: Atom PDF