userguide: explain rule types and categorization - v9 #12209

jufajardini · 2024-12-03T12:45:09Z

Previous PR: #12184

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

Please check the built version: https://suri-rtd-test.readthedocs.io/en/doc-sigtypes-et-properties-v9/rules/rule-types.html

Describe changes:

from last PR: remove collapsible sections and rebase
Move to its own section under Suricata Rules
Add engine analysis report examples for each rule type
Another try at explaining userguide: explain rule types and categorization - v7 #12114 (comment), I think this is more correct, but a strict read of this table, especially for stream and packet-stream rule types is very welcome
Newer version of the flowchart for SignatureSetType().
Add the drawio file to version control - makes maintaining easier.
Add explanation about different outcomes of using Flowvars and other similar keywords
Add a brief explanation about the existence of rules that require real packets (more work to be done here with https://redmine.openinfosecfoundation.org/issues/7424), trying to tackle detect: don't run pkt sigs on ffr pkts #12095 (comment) -- what do you think, @catenacyber ?
add a table/ list with human-readable rule types

Many of the examples and conclusions documented here were derived from tests and checks as seen on OISF/suricata-verify#2153

Add documentation about the rule types introduced by 2696fda. Add doc tags around code definitions that are referenced in the docs. Task #https://redmine.openinfosecfoundation.org/issues/7031

codecov · 2024-12-03T13:27:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.18%. Comparing base (e9173f3) to head (7c79e48).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12209      +/-   ##
==========================================
+ Coverage   83.17%   83.18%   +0.01%     
==========================================
  Files         912      912              
  Lines      257111   257111              
==========================================
+ Hits       213856   213885      +29     
+ Misses      43255    43226      -29

Flag	Coverage Δ
fuzzcorpus	`61.01% <ø> (-0.01%)`	⬇️
livemode	`19.41% <ø> (-0.01%)`	⬇️
pcap	`44.40% <ø> (+0.01%)`	⬆️
suricata-verify	`62.77% <ø> (-0.01%)`	⬇️
unittests	`59.17% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

jufajardini · 2024-12-03T14:25:50Z

doc/userguide/rules/rule-types.rst

+    * - IP Only
+      - Flow
+      - Once per direction
+      - On IP addresses on the flow
+      - Source/ Destination field of a rule
+    * - IP Only (contains negated address)(*)
+      - Flow
+      - Once per direction
+      - On IP addresses on the flow
+      - Source/ Destination field of a rule, containing negated address


Action is flow if flow exists. Packets that are not part of a flow, .... will always be inspected per packet.

jufajardini · 2024-12-03T14:27:30Z

doc/userguide/rules/rule-types.rst

+      - Source/ Destination field of a rule
+    * - IP Only (contains negated address)(*)
+      - Flow
+      - Once per direction


All packets.

Test this with rule profiling to confirm.

Maybe we can have a SV test with like-ip-only

alert rule: several alerts

drop/pass: should be applied to the flow

jufajardini · 2024-12-03T14:33:43Z

doc/userguide/rules/rule-types.rst

+      - Packet
+      - Per-packet basis
+      - Packet-level info (e.g.: header info)
+      - ``itype``, ``tcp.hdr``, ``tcp.seq``, ``ttl`` etc.


tcp-pkt - add as keyword example here

jufajardini · 2024-12-03T14:36:27Z

doc/userguide/rules/rule-types.rst

+      - Flow, if stateful (**)
+      - Per stream chunk, if stateful, per-packet if not.
+      - Against the reassembled stream. If stream unavailable, match per-packet.
+        (stream payload or packet payload)


move to Inspected, be stream payload AND packet payload

exposes the stream and/or payload data.

jufajardini · 2024-12-03T14:40:05Z

doc/userguide/rules/rule-types.rst

+    * - Type
+      - Action Scope
+      - Inspected
+      - Matches


Data exposed?

jufajardini · 2024-12-03T14:40:20Z

doc/userguide/rules/rule-types.rst

+
+    * - Type
+      - Action Scope
+      - Inspected


Inspection hook?

jufajardini · 2024-12-03T14:42:09Z

doc/userguide/rules/rule-types.rst

+      - Keyword Examples (non-exhaustive)
+    * - Decoder Events Only
+      - Packet
+      - Per-packet basis


Per-broken/ invalid packet

jufajardini · 2024-12-03T14:42:17Z

doc/userguide/rules/rule-types.rst

+    * - Decoder Events Only
+      - Packet
+      - Per-packet basis
+      - Packets that are broken on an IP level


Decoding events

jufajardini · 2024-12-03T14:45:29Z

doc/userguide/rules/rule-types.rst

+      - ``content`` with ``startswith`` or ``depth``
+    * - Stream
+      - Flow, if stateful (**)
+      - Per stream chunk, if stateful, per-packet if not.


Stream chunks, or just packets (if not accepted by stream engine)

jufajardini · 2024-12-03T14:45:46Z

doc/userguide/rules/rule-types.rst

+    * - Stream
+      - Flow, if stateful (**)
+      - Per stream chunk, if stateful, per-packet if not.
+      - Against the reassembled stream. If stream unavailable, match per-packet.


exposes stream reassembled payload or packet payload data

jufajardini · 2024-12-03T14:48:08Z

doc/userguide/rules/rule-types.rst

+    - Application Layer (`app_layer`)
+    - Protocol Detection Only (`pd_only`)
+    - Decoder Events Only (`de_only`)


the ` is wrong.

jufajardini · 2024-12-03T14:51:12Z

doc/userguide/rules/rule-types.rst

+The rule examples provided further cover some such cases, but the table below
+lists those keywords with more details:
+


check detect-flowbits.c:632. some cases will make a packet rule become a stateful / app_tx signature.

jufajardini · 2024-12-03T15:01:25Z

@njlavigne There will be a new version of this coming out soon to take into account a review done with Victor, but do feel free to add feedback, please, if you see anything weird or missing :)

jufajardini · 2024-12-03T18:00:18Z

doc/userguide/rules/rule-types.rst

+aspects aforementioned.
+
+.. list-table:: Suricata Rule Types
+    :widths: 10, 17, 18, 30, 25


Review width for last 2 columns, especially.

the rtd theme isn't properly rendering list-table widths properly - it's ignoring this, basically.

The best workaround I've found was using nested paragraphs, as it seems that the width is being based on the column header, and this seems to be working.

jasonish · 2024-12-03T21:28:27Z

Given the embedded code, etc. I wonder if this is a little too deep to from the rule chapter. Is it better suited at the end with some "rule internals" or part of a Suricata internals guide? At first I was wondering devguide, but I do realize there is some internals that should be documented with a power user audience, and not necessarily a developer audience.

suricata-qa · 2024-12-03T22:25:03Z

Information: QA ran without warnings.

Pipeline 23656

jufajardini · 2024-12-04T12:31:14Z

Given the embedded code, etc. I wonder if this is a little too deep to from the rule chapter. Is it better suited at the end with some "rule internals" or part of a Suricata internals guide? At first I was wondering devguide, but I do realize there is some internals that should be documented with a power user audience, and not necessarily a developer audience.

We have Making sense out of alerts, right after the two Rules chapters. Maybe we could have this part about the rule types before or after that one? I understand it has more advanced stuff, but I think semantically it also makes sense that it remains close to the other chapters. imho, the code excerpts that we show aren't mandatory to understand the contents - more to add hooks for those who like to dig...

jasonish · 2024-12-04T15:35:50Z

Given the embedded code, etc. I wonder if this is a little too deep to from the rule chapter. Is it better suited at the end with some "rule internals" or part of a Suricata internals guide? At first I was wondering devguide, but I do realize there is some internals that should be documented with a power user audience, and not necessarily a developer audience.

We have Making sense out of alerts, right after the two Rules chapters. Maybe we could have this part about the rule types before or after that one? I understand it has more advanced stuff, but I think semantically it also makes sense that it remains close to the other chapters. imho, the code excerpts that we show aren't mandatory to understand the contents - more to add hooks for those who like to dig...

Maybe move it to the end of the rules chapter? My thought are:

Making sense of alerts is more about reading the alert. Could be a different audience than rule writers. I think the large majority of our users are not rule writers, but read the alerts.
Reading documentation in order. If this new info it must have reading material before writing your first rule, keep it where it is, otherwise if its for the smaller few that really need to know whats going on, move it down

To be fair, advanced rule writing could be its own book I'm sure!

jufajardini · 2024-12-04T19:12:31Z

doc/userguide/rules/rule-types.rst

+      - ``to_server``, ``to_client``
+      - no type change
+    * - ``flow``
+      - ``established``, ``not_established``
+      - to `packet`


ip_only and like_ip_only behave differently here.

Are there cases where a "packet" type rule changes to a stream-type rule when a content keyword is added? What type is a rule that contains both content and flow:to_server?

njlavigne · 2024-12-05T01:15:23Z

doc/userguide/rules/rule-types.rst

+    Although both are related to matching on application layer protocols, since
+    Suricata 7, a Protocol Detection rule (that uses the ``app-layer-protocol``
+    keyword) is not internally classified the same as a simple rule matching on
+    the application layer protocol on the `protocol` field.


These two rule types are similar but work differently - could you provide any guidance as to when you might prefer to use one over the other? What are they intended to be used for?

njlavigne · 2024-12-05T01:15:46Z

doc/userguide/rules/rule-types.rst

+.. note::
+    (*) IP Only signatures with negated addresses are `like` IP-only signatures, but
+    currently handled differently due to limitations of the algorithm processing
+    IP Only rules.


What is the significance of this to me as a rule writer? Are these two types functionally different, or is the only difference just an internal optimization?

If you use the idea above for a section per rule type, special notices like this could go into those sections which might help it flow naturally with the other information you are providing per rule type.

njlavigne · 2024-12-05T01:16:33Z

doc/userguide/rules/rule-types.rst

+The ``SignatureSetType()`` overall flow is described below:
+
+.. image:: intro/OverallAlgoHorizontal-20241127.png
+   :width: 600
+   :alt: A flowchart representing the SignatureSetType function.


Overall I think this new page does a good job of outlining what each rule type does when applied to traffic, but for rule writers the other piece of relevant background is "what influences a signature be a particular type" and to me that part still isn't as clear. I think the flow chart is great and helps with this but the branch conditions like "Is PDOnly" and "Is IpOnly" don't have enough supporting detail to understand when they will apply.

I think it would help to include a short description for each rule type stating at a high level what the types are used for, for example

The IPONLY rule type is used for rules that match only on source & destination IPs and port numbers, and not on any other flow or content modifier.

It probably wouldn't fit in the table so it would likely need to go somewhere else, like in a short section introducing each type. Maybe could combine that with the examples so that each type would have a small section that combines both an explainer and the examples for that type.

njlavigne · 2024-12-05T01:17:11Z

doc/userguide/rules/rule-types.rst

+.. list-table:: Suricata Rule Types, and their Engine Analysis Term
+    :header-rows: 1


Have you considered merging the two rule type tables into one table?

I know it's not ideal as is, but I wanted to avoid having even more columns, as some of the cels in the other column are quite busy with text and Read the Docs doesn't offer a lot of flexibility for adjusting column width...

njlavigne · 2024-12-05T01:31:18Z

doc/userguide/rules/rule-types.rst

+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Aside from the scope of action of a signature, certain rule conditions will
+require that it matches against a *real packet* (as opposed to a *pseudo packet*).


Is it worth saying what the difference is here (or link to it, if it's explained elsewhere in the docs)?

njlavigne · 2024-12-05T01:34:56Z

doc/userguide/rules/rule-types.rst

+    * - ``flow``
+      - ``to_server``, ``to_client``
+      - no type change


You can correct me if I'm wrong, but I believe there is at least one example where this does change - I think a rule that would otherwise be "iponly" type does change to something else when a flow:to_server or flow:to_client is added.

njlavigne · 2024-12-05T01:42:06Z

doc/userguide/rules/rule-types.rst

+      - ``to_server``, ``to_client``
+      - no type change
+    * - ``flow``
+      - ``established``, ``not_established``
+      - to `packet`


Are there cases where a "packet" type rule changes to a stream-type rule when a content keyword is added? What type is a rule that contains both content and flow:to_server?

njlavigne · 2024-12-05T01:56:05Z

doc/userguide/rules/rule-types.rst

+    * - Application Layer Protocol Transactions
+      - Flow
+      - Per transaction update
+      - On buffer keywords
+      - Application layer protocol-related, e.g. ``http.host``, ``rfb.secresult``,
+        ``dcerpc.stub_data``, ``frame`` keywords


What exactly is a transaction is a topic that would be very relevant here, for understanding how these rules will be matched against traffic. You have a good explanation of them here...I understand that the developer guide is aimed at a different audience than this page (aimed at rule writers), but they would both benefit from understanding that.

njlavigne · 2024-12-05T02:07:20Z

@njlavigne There will be a new version of this coming out soon to take into account a review done with Victor, but do feel free to add feedback, please, if you see anything weird or missing :)

Thanks for tagging me, please feel free to keep doing it for changes you think I'd be interested in!

userguide: explain rule types and categorization

7c79e48

Add documentation about the rule types introduced by 2696fda. Add doc tags around code definitions that are referenced in the docs. Task #https://redmine.openinfosecfoundation.org/issues/7031

jufajardini added the typo/doc update No code change : only doc or typo fixes label Dec 3, 2024

jufajardini requested a review from victorjulien as a code owner December 3, 2024 12:45

jufajardini mentioned this pull request Dec 3, 2024

userguide: explain rule types and categorization - v8 #12184

Closed

jufajardini commented Dec 3, 2024

View reviewed changes

jufajardini commented Dec 4, 2024

View reviewed changes

njlavigne reviewed Dec 5, 2024

View reviewed changes

		The rule examples provided further cover some such cases, but the table below
		lists those keywords with more details:

		.. list-table:: Suricata Rule Types, and their Engine Analysis Term
		:header-rows: 1

userguide: explain rule types and categorization - v9 #12209

Are you sure you want to change the base?

userguide: explain rule types and categorization - v9 #12209

Conversation

jufajardini commented Dec 3, 2024

codecov bot commented Dec 3, 2024

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jufajardini commented Dec 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonish commented Dec 3, 2024

suricata-qa commented Dec 3, 2024

jufajardini commented Dec 4, 2024

jasonish commented Dec 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njlavigne commented Dec 5, 2024