-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: absent keyword to test absence of sticky buffer #11459
detect: absent keyword to test absence of sticky buffer #11459
Conversation
Ticket: 2224 It takes an argument to match only if the buffer is absent, or it can still match if the buffer is present, but we test the absence of some content
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11459 +/- ##
==========================================
+ Coverage 82.44% 82.57% +0.13%
==========================================
Files 938 938
Lines 248068 248441 +373
==========================================
+ Hits 204513 205150 +637
+ Misses 43555 43291 -264
Flags with carried forward coverage won't be shown. Click here to find out more. |
Information: QA ran without warnings. Pipeline 21495 |
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.
Thank you, Philippe! Looking very good. 🌟
Some questions inline.
@@ -2214,6 +2218,13 @@ uint8_t DetectEngineInspectMultiBufferGeneric(DetectEngineCtx *de_ctx, | |||
} | |||
local_id++; | |||
} while (1); | |||
if (local_id == 0) { |
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.
Why do we only want to check on the first buffer?
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.
ah bc if the first one doesn't exist, others shouldn't either.
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.
adding a comment
"alert http any any -> any any " | ||
"(msg:\"invalid absent only with negated content\"; http.user_agent; " | ||
"absent; content:!\"one\"; sid:2;)"); | ||
FAIL_IF(s != NULL); |
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.
Why is it designed to fail with any content following absent
? If the http.user_agent
is absent, nothing else can be matched against unless it's a sticky buffer?
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.
Yes absent;
means only match if the buffer is absent, so cannot have additional tests on it.
Otherwise, we can use absent: or_else;
@@ -56,11 +57,97 @@ static DetectParseRegex parse_regex; | |||
int DetectIsdataatSetup (DetectEngineCtx *, Signature *, const char *); | |||
#ifdef UNITTESTS | |||
static void DetectIsdataatRegisterTests(void); | |||
static void DetectAbsentRegisterTests(void); |
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.
Why have you done entire absent
lifecycle in the detect-isdataat.c
file?
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.
because it used to be implemented as isdataat:!0
What do you think ?
It can take an argument "or_else" to match on absent buffer or on what comes next such as negated content, for instance : | ||
|
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.
or_else
: So - match if absent OR if not absent, check something else?
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.
yes
Continued in #11509 |
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/2224
Describe changes:
absent
keyword to match on absent bufferSV_BRANCH=OISF/suricata-verify#1957
#11423 with clean history