Conversation
Ticket: 7199 Uses a config parameter detect.guess-applayer-tx to enable this behavior (off by default) This feature is requested for use cases with signatures not using app-layer keywords but still targetting application layer transactions, such as pass/drop rule combination, or lua usage. This overrides the previous behavior of checking if the signature has a content match, by checking if there is only one live transaction, in addition to the config parameter being set. (cherry picked from commit f2c3776)
The `guess-applayer-tx` work also removed the stream match condition for adding app-layer metadata to alerts. This is a behavior change that is not desired at this point, so this commit reverts that part of the changes. We keep the exising logging of app-layer metadata if the match was in the stream.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main-7.0.x #12267 +/- ##
==============================================
- Coverage 83.19% 83.18% -0.02%
==============================================
Files 922 922
Lines 260888 260898 +10
==============================================
- Hits 217048 217026 -22
- Misses 43840 43872 +32
Flags with carried forward coverage won't be shown. Click here to find out more. |
catenacyber
left a comment
There was a problem hiding this comment.
Original PR ok for me, plus added doc commit ok for me
| picked up.** This is to reduce the chances of logging unrelated data, and may | ||
| lead to alerts being logged without metadata, in some cases. | ||
| The alert event will have ``tx_guessed: true`` to recognize | ||
| such alerts. |
There was a problem hiding this comment.
Do you want to add there that we also log some transaction metadata for rule with stream matches ?
And that the first transaction is logged, like one TCP packet with 3 DNS requests to suricata.io oisf.net and suricon.net and rule alert tcp any any -> any any (content: "suricon"; sid: 1) will log the request to suricata.io
There was a problem hiding this comment.
Seems all too much of a low level implementation detail to discuss in configuration docs. But these types of issues are making me think we need a know issues and shortcomings chapter.
| # try to tie an app-layer transaction for rules without app-layer keywords | ||
| # if there is only one live transaction for the flow | ||
| # allows to log app-layer metadata in alert | ||
| # but the transaction may not be the relevant one. |
There was a problem hiding this comment.
Tried to add a bit more here (from https://github.com/OISF/suricata/pull/12260/files):
| # try to tie an app-layer transaction for rules without app-layer keywords | |
| # if there is only one live transaction for the flow | |
| # allows to log app-layer metadata in alert | |
| # but the transaction may not be the relevant one. | |
| # Try to guess an app-layer transaction for rules without app-layer keywords, | |
| # ONLY IF there is just one live transaction for the flow. | |
| # This allows logging app-layer metadata in alert - the transaction may not | |
| # be the relevant one for the alert. |
There was a problem hiding this comment.
Tried to add a bit more here (from https://github.com/OISF/suricata/pull/12260/files):
On suggestion...
ONLY IF there is a single live transaction for the flow,
IMO, can be cleaned up in post-release.
|
Should pass with: SV_BRANCH=OISF/suricata-verify#2179 |
jasonish
left a comment
There was a problem hiding this comment.
Stage with #12264 and OISF/suricata-verify#2179
|
replaced by #12268 |
#12263, with 5d22aa4