-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect log alert tx one 7199 v6 #2157
Closed
catenacyber
wants to merge
4
commits into
OISF:master
from
catenacyber:detect-log-alert-tx-one-7199-v6
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
fe5dd99
tests: add test for bug-7199
jufajardini c60c3c8
output: use detect.guess-applayer-tx for http-ish content test
catenacyber 7610aeb
pgsql: use detect.guess-applayer-tx for content test
catenacyber 18aba67
tls: add check for catch-all rule logging app-layer metadata
catenacyber File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# Test | ||
|
||
Showcase change of behavior from Suricata-7.0.5 to Suricata-7.0.6. | ||
Before, a non-stream rule that matched traffic associated with an app-layer | ||
transaction would result in app-layer metadata being logged with the alert, if | ||
metadata was enabled. Starting with 7.0.6, this will only be achieved if the | ||
rule is an app-layer/stream one. | ||
|
||
### Pcap | ||
|
||
Packet capture resulting of a curl to suricata.io. | ||
|
||
### Ticket | ||
|
||
https://redmine.openinfosecfoundation.org/issues/7199 |
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
%YAML 1.1 | ||
--- | ||
|
||
outputs: | ||
- eve-log: | ||
enabled: yes | ||
filetype: regular | ||
filename: eve.json | ||
types: | ||
- alert: | ||
enabled: true | ||
tagged-packets: true | ||
metadata: true | ||
http-body: true | ||
- http: | ||
extended: true | ||
tagged-packets: true | ||
- tls: | ||
extended: true | ||
|
||
detect: | ||
guess-applayer-tx: yes |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
reject ip any any -> any any (msg: "Reject by AntreaNetworkPolicy:default/ingress-allow-http-request-to-api-v2"; flow: to_server, established; sid: 1;) | ||
pass http any any -> any any (msg: "Allow http by AntreaNetworkPolicy:default/ingress-allow-http-request-to-api-v2"; http.uri; content:"/api/v2/"; startswith; http.method; content:"GET"; http.host; content:"foo.bar.com"; startswith; endswith; sid: 2;) | ||
alert http any any -> any any (msg: "Alert by AntreaNetworkPolicy:default/ingress-allow-http-request-to-api-v2"; http.uri; content:!"/api/v2/"; sid: 3;) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
requires: | ||
features: | ||
- LIBNET1.1 | ||
|
||
args: | ||
- -k none | ||
- --set stream.midstream=true | ||
- --simulate-ips | ||
|
||
checks: | ||
- filter: | ||
count: 4 | ||
match: | ||
event_type: alert | ||
alert.signature_id: 1 | ||
- filter: | ||
min-version: 8 | ||
count: 1 | ||
match: | ||
event_type: alert | ||
alert.signature_id: 1 | ||
has-key: http | ||
- filter: | ||
count: 0 | ||
match: | ||
event_type: alert | ||
alert.signature_id: 2 | ||
- filter: | ||
count: 1 | ||
match: | ||
event_type: alert | ||
alert.signature_id: 3 | ||
has-key: http |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,3 +15,6 @@ app-layer: | |
protocols: | ||
pgsql: | ||
enabled: yes | ||
|
||
detect: | ||
guess-applayer-tx: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -171,11 +171,4 @@ checks: | |
[email protected].....\[email protected]...'[email protected]...)[email protected]...,[email protected]...'[email protected]...)[email protected][email protected]...\r\ | ||
SELECT 8.Z....I" | ||
pcap_cnt: 87 | ||
pgsql.request.simple_query: SELECT * FROM new_table; | ||
pgsql.response.command_completed: SELECT 8 | ||
pgsql.response.data_rows: 8 | ||
pgsql.response.data_size: 236 | ||
pgsql.response.field_count: 2 | ||
pgsql.tx_id: 26 | ||
stream: 1 | ||
tx_id: 25 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,3 +16,6 @@ app-layer: | |
protocols: | ||
pgsql: | ||
enabled: yes | ||
|
||
detect: | ||
guess-applayer-tx: true | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should there also be a test.yaml update with this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, why should there be ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to include that the tx was guessed? (I guess?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think it is important... Is it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the point of the change if there is no test update with it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TL;DR Otherwise this test pgsql-bug-6983-ips fails with the Suricata PR.
I think you are referring to Suricata change here : https://github.com/OISF/suricata/pull/12197/files#diff-ee484ae4b77e59eb8b6b50f628c84ab626bd6178a6d43e1219dbc9619be7e027L816
Over simplifying it : instead of using
(alert_flags & PACKET_ALERT_FLAG_STREAM_MATCH)
we now usede_ctx->force_applayer
pgsql-bug-6983-ips resorted to
(alert_flags & PACKET_ALERT_FLAG_STREAM_MATCH)
to have pgsql metadata in alerts.With the Suricata PR, we now need
de_ctx->force_applayer
meaningdetect.guess-applayer-tx: true
in yamlThat comes from how I understood your comment OISF/suricata#12166 (comment)
Beyond pgsql-bug-6983-ips test, this SV PR has test bug-7199 to show the new benefits of the new behavior.