Iprep feature 6857/v3#11264
Conversation
No need to init ptrs to NULL after SCCalloc.
Implement special "isset" and "isnotset" modes. "isset" matches if an IP address is part of an iprep category with any value. It is internally implemented as ">=,0", which should always be true if there is a value to evaluate, as valid reputation values are 0-127. "isnotset" matches if an IP address is not part of an iprep category. Internally it is implemented outside the uint support. Ticket: OISF#6857.
Bring in line with new Rust code naming for FFI functions.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11264 +/- ##
==========================================
+ Coverage 79.68% 82.99% +3.30%
==========================================
Files 942 942
Lines 249186 249487 +301
==========================================
+ Hits 198566 207059 +8493
+ Misses 50620 42428 -8192
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: ERROR: QA failed on SURI_TLPR1_suri_time.
Pipeline 20984 |
|
|
||
| drop ip $HOME_NET any -> any any (:example-rule-options:`iprep:src,trusted-hosts,isnotset;` sid:1;) | ||
|
|
||
| In this example traffic for a host w/o a trust score would be blocked. |
There was a problem hiding this comment.
Shouldn't we use the preferred "without" form, since these are the official docs?
catenacyber
left a comment
catenacyber
left a comment
There was a problem hiding this comment.
Thanks for the work :-)
Good enough even if I have a few nits
- CI : 🟢
- Code : good for me
- Commits segmentation : ok for me
- Commit messages : good enough for me
- Git ID set : looks fine for me
- CLA : you already contributed :-p
- Doc update : ok for me
- Redmine ticket : ok
- Rustfmt : 🟠 some changes done like
- - } else {
+ } else {
- Tests : SV ok
- Dependencies added: none
| ``category``: the category short name | ||
|
|
||
| operator: <, >, = | ||
| ``operator``: <, <=, >, >=, = |
There was a problem hiding this comment.
There are more
You could add a link to doc/userguide/rules/integer-keywords.rst
| } | ||
|
|
||
| /// value matching is done use `DetectUintData` logic. | ||
| /// isset matching is done using special `DetectUintData` value ">= 0" |
There was a problem hiding this comment.
Ok, but why not add a bool outside the DetectUintData ?
| if args == 4 || args == 3 { | ||
| let cmd = if let Ok(cmd) = DetectIPRepDataCmd::from_str(values[0].trim()) { | ||
| cmd | ||
| } else { |
There was a problem hiding this comment.
5 lines instead of let (i, cmd) = map_res(alpha0, DetectIPRepDataCmd::from_str)(i)?;
There was a problem hiding this comment.
(meaning I preferred it without separated_list1 and we could still support what you want with alt)
| }; | ||
| let du8 = DetectUintData::<u8> { | ||
| arg1, | ||
| arg2: 0, |
There was a problem hiding this comment.
Should we support ranges ?
| } | ||
|
|
||
| if args == 4 { | ||
| let mode = match detect_parse_uint_mode(values[2].trim()) { |
There was a problem hiding this comment.
if let instead of match
| exit(EXIT_FAILURE); | ||
| SRepCIDRTree *cidr_ctx = de_ctx->srepCIDR_ctx; | ||
|
|
||
| for (i = 0; i < SREP_MAX_CATS; i++) { |
There was a problem hiding this comment.
Nice, I wonder if we can remove more of these...
|
Merged in #11309, thanks! |
4 participants
SV_BRANCH=OISF/suricata-verify#1896
#11091, rebased, cleaned up and with docs added.
https://redmine.openinfosecfoundation.org/issues/6857