Skip to content

Detect count 5044 v3.3 - #14346

Closed
catenacyber wants to merge 4 commits into
OISF:mainfrom
catenacyber:detect-count-5044-v3.3
Closed

Detect count 5044 v3.3#14346
catenacyber wants to merge 4 commits into
OISF:mainfrom
catenacyber:detect-count-5044-v3.3

Conversation

@catenacyber

Copy link
Copy Markdown
Contributor

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

Describe changes:

  • adds a count option to multi-buffers, behaving like a keyword but syntax is email.received: count <3; instead of email.received; count: <3;
  • adds other modes to multi-buffers like all, all_or_absent, nb`, and precise indexing

SV_BRANCH=OISF/suricata-verify#2634

Draft :

  • Feedback about general design ?

#14279 needed rebase, #14345 good branch rebased

TODOs :

  • update doc if design is agreed
  • add support for all multi-buf keywords
  • add more tests
  • rustfmt removes one line in mod.rs

Also rename parse_uint_count as parse_multi_count

This allows to prepare multi-buffers using this code
As for multi-integers, multi-buffers can now have the following
arguments
- count
- nb
- precise index
- all

Ticket: 5044
@codecov

codecov Bot commented Nov 18, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.33333% with 76 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.18%. Comparing base (626027a) to head (45d9fbb).
⚠️ Report is 474 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14346      +/-   ##
==========================================
- Coverage   84.20%   84.18%   -0.02%     
==========================================
  Files        1012     1013       +1     
  Lines      261769   261981     +212     
==========================================
+ Hits       220415   220552     +137     
- Misses      41354    41429      +75     
Flag Coverage Δ
fuzzcorpus 63.23% <21.79%> (-0.06%) ⬇️
livemode 18.71% <5.55%> (-0.02%) ⬇️
pcap 44.59% <10.68%> (-0.06%) ⬇️
suricata-verify 64.97% <68.10%> (+<0.01%) ⬆️
unittests 59.19% <18.06%> (-0.04%) ⬇️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 28484

@inashivb inashivb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, Philippe. This is great! :)
Q: Why do we need to create lists per mode? Can we not check the optional args corresponding to any multi buffer?

Comment thread src/detect-engine.c
Comment on lines +2217 to +2218
if (prec->oob) {
// match as out of bounds

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not following this. Why match if oob?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an option. It is especially meant with negated content : we may want to match if 3rd buffer does not have "toto" or if there is no third buffer.

Does that answer your question ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. thank you!

Comment thread src/detect-email.c
Comment on lines +212 to +215
if (arg) {
return DetectMultiSetup(de_ctx, s, arg);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and these updates will have to be done for all keywords?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, hence the draft draft to get feedback about general design.

See TODOs in first comment :

  • update doc if design is agreed
  • add support for all multi-buf keywords
  • add more tests
  • rustfmt removes one line in mod.

Comment thread src/detect-engine.c
case DETECT_MULTI_NB:
if (!eof) {
DetectU32Data *du32 = (DetectU32Data *)smd->ctx;
if (du32->mode != DETECT_UINT_GTE && du32->mode != DETECT_UINT_GT) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't we have other modes for number of matches?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I have a rule that matches if there are less than 10 http headers, I might get there in a case I have only 5 http headers, but I may receive more in the future (because !eof).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. Why do we do a hasty inspection of multi buffers with count if they're not complete? Will this not lead to cases like http.header: all; to be evaluated needlessly only to find it can't match yet?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we do a hasty inspection of multi buffers with count if they're not complete?

It works if we want to see at least 5 http headers (DETECT_UINT_GTE and DETECT_UINT_GT)

For all case, we wait for eof, so this is good, right ?

@catenacyber

Copy link
Copy Markdown
Contributor Author

Why do we need to create lists per mode? Can we not check the optional args corresponding to any multi buffer?

I am not sure I understand your questions.
Where do you think we create lists per mode ?

@inashivb

Copy link
Copy Markdown
Member

I am not sure I understand your questions. Where do you think we create lists per mode ?

Sorry for being unclear. We're registering 5 types of sm lists that we append the data to respectively as and when we encounter the keyword.

@catenacyber

Copy link
Copy Markdown
Contributor Author

I am not sure I understand your questions. Where do you think we create lists per mode ?

Sorry for being unclear. We're registering 5 types of sm lists that we append the data to respectively as and when we encounter the keyword.

Yes, I think we can indeed just register one and then have the options in it...
This is just about code style, there is no difference in rules expressivity, right ?

I think this style comes from the fact that we may thus add easily all as its own keyword...

@catenacyber catenacyber added the needs rebase Needs rebase to main label Mar 25, 2026
@catenacyber

Copy link
Copy Markdown
Contributor Author

rebased in #15100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs rebase Needs rebase to main

Development

Successfully merging this pull request may close these issues.

3 participants