Detect generic uint 4112 v3 by catenacyber · Pull Request #7150 · OISF/suricata

catenacyber · 2022-03-18T14:50:58Z

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

Describe changes:

Makes use of generic DetectUint structure for dsize and dcerpc

Still TODO:

more keywords in C use specific versions
Commit history likely to rework...

Replaces #7121 with remove the C version to use only rust version

as is done for u8 and u32

Ie inequality test for integer Also adds prefilter functions for u16

In case of greater/lesser or equal

Despite what the comment said 1<>2 is not a valid range as it is empty and cannot have any match. Maybe we should even consider 1<>3 an invalid range as it should rather be written as =2

from http2 to a generic file so that it can be reused by dcerpc and others

aka DetectUintModeNe

ie <0 is impossible

catenacyber · 2022-03-18T14:51:27Z

+576 −1,110

Begins to look good 😊

codecov · 2022-03-18T15:10:38Z

Codecov Report

Merging #7150 (912faf6) into master (3a490fb) will increase coverage by 0.07%.
The diff coverage is 87.03%.

@@            Coverage Diff             @@
##           master    #7150      +/-   ##
==========================================
+ Coverage   78.06%   78.14%   +0.07%     
==========================================
  Files         628      628              
  Lines      185266   184899     -367     
==========================================
- Hits       144635   144488     -147     
+ Misses      40631    40411     -220

Flag	Coverage Δ
fuzzcorpus	`60.21% <85.36%> (+0.22%)`	⬆️
suricata-verify	`54.64% <51.68%> (+0.04%)`	⬆️
unittests	`63.14% <60.37%> (+0.01%)`	⬆️

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

catenacyber · 2022-03-18T15:42:16Z

I do not know why sip-method S-V test failed only once on MacOS (not failing locally)

jasonish · 2022-03-18T15:54:08Z

rust/src/detect.rs

+#[repr(C)]
+pub struct DetectUintData<T> {
+    pub arg1: T,
+    pub arg2: T,
+    pub mode: DetectUintMode,
+}


Nice, I didn't know this could work with cbindgen.

I first tried to use some kind of typedef alias like pub type DetectUintDataU32 DetectUintData<u32>, but making cbindgen export types caused other problems...

jasonish · 2022-03-18T16:11:07Z

rust/src/detect.rs

+    pub mode: DetectUintMode,
+}
+
+fn detect_parse_uint_start_equal<T: std::str::FromStr + std::cmp::PartialOrd + num::Bounded>(


I wonder if a custom trait would clean up these function definitions.. Something like:

pub trait DetectIntType: std::str::FromStr + std::cmp::PartialOrd + num::PrimInt + num::Bounded {} impl<T> DetectIntType for T where T: std::str::FromStr + std::cmp::PartialOrd + num::PrimInt + num::Bounded {}

then most of these methods can use <T: DetectIntType> instead of the string of traits.

catenacyber · 2022-03-18T16:17:58Z

I think windows is segfaulting because I call C's free on rust's Box::into_raw output...

@jasonish what do you think of it ?

jasonish · 2022-03-18T16:43:13Z

I think windows is segfaulting because I call C's free on rust's Box::into_raw output...

@jasonish what do you think of it ?

You have to free memory that came from Rust back in Rust itself. Pass the pointer back and and call Box::from_raw, Rust will take ownership of it and drop it as expected.

Probably the simplest example of this is jb_clone and jb_free: https://github.com/OISF/suricata/blob/master/rust/src/jsonbuilder.rs#L632, but all our parsers do this as well.

suricata-qa · 2022-03-19T04:35:03Z

Information: QA ran without warnings.

Pipeline 6597

suricata-qa · 2022-03-20T02:00:03Z

Information: QA ran without warnings.

Pipeline 6601

victorjulien · 2022-03-21T10:05:35Z

I do not know why sip-method S-V test failed only once on MacOS (not failing locally)

This test has been flaky before.

victorjulien

Needs some further work, see inline discussion between Jason and Philippe

catenacyber · 2022-03-23T21:00:29Z

Replaced by #7167

catenacyber added 18 commits March 18, 2022 11:30

detect: generic function for u16

1be1a82

as is done for u8 and u32

detect: adds generic DETECT_UINT_NE

9909290

Ie inequality test for integer Also adds prefilter functions for u16

detect: use generic functions for dsize

108e73a

detect: better validation for generic int

6c1268f

In case of greater/lesser or equal

detect: fix unit tests for dsize

db1d563

Despite what the comment said 1<>2 is not a valid range as it is empty and cannot have any match. Maybe we should even consider 1<>3 an invalid range as it should rather be written as =2

detect: move generic int detect function

310d8fe

from http2 to a generic file so that it can be reused by dcerpc and others

detect: generic rust function detect_match for integers

52ee506

detect: use generic instead of DETECT_DCE_IFACE_OP_LT

1069d2f

detect: rust generic integer not equal

1f92f60

aka DetectUintModeNe

detect: fix unit tests for http2 and dcerpc

f050c21

detect: verify integer validity

62d492e

ie <0 is impossible

smb: fix detection for dcerpc iface without version

d54af4e

detect: using rust generics for DetectUintData<T>

2ed4677

detect: move ffi functions from http2 to generic detect

802ba53

detect: move generic uint from C to only rust

ccad2b3

detext: fix tests compilation

6fa4387

dcerpc: fix test case

751a146

detect: force completion on generic uints

9cf77cd

catenacyber requested review from a team, jasonish and victorjulien as code owners March 18, 2022 14:50

catenacyber mentioned this pull request Mar 18, 2022

Detect generic uint 4112 v2 #7121

Closed

jasonish reviewed Mar 18, 2022

View reviewed changes

catenacyber mentioned this pull request Mar 18, 2022

Watch out for modules privacy when proposing traits to associate with an item rust-lang/rust#95080

Open

detect: use rust free functions

912faf6

victorjulien requested changes Mar 21, 2022

View reviewed changes

catenacyber mentioned this pull request Mar 23, 2022

Detect generic uint 4112 v4 #7167

Closed

catenacyber closed this Mar 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect generic uint 4112 v3#7150

Detect generic uint 4112 v3#7150
catenacyber wants to merge 19 commits intoOISF:masterfrom
catenacyber:detect-generic-uint-4112-v3

catenacyber commented Mar 18, 2022

Uh oh!

catenacyber commented Mar 18, 2022

Uh oh!

codecov bot commented Mar 18, 2022 •

edited

Loading

Uh oh!

catenacyber commented Mar 18, 2022

Uh oh!

jasonish Mar 18, 2022

Uh oh!

catenacyber Mar 18, 2022

Uh oh!

jasonish Mar 18, 2022

Uh oh!

catenacyber commented Mar 18, 2022

Uh oh!

jasonish commented Mar 18, 2022

Uh oh!

suricata-qa commented Mar 19, 2022

Uh oh!

suricata-qa commented Mar 20, 2022

Uh oh!

victorjulien commented Mar 21, 2022

Uh oh!

victorjulien left a comment

Uh oh!

catenacyber commented Mar 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Conversation

catenacyber commented Mar 18, 2022

Uh oh!

catenacyber commented Mar 18, 2022

Uh oh!

codecov bot commented Mar 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

catenacyber commented Mar 18, 2022

Uh oh!

jasonish Mar 18, 2022

Choose a reason for hiding this comment

Uh oh!

catenacyber Mar 18, 2022

Choose a reason for hiding this comment

Uh oh!

jasonish Mar 18, 2022

Choose a reason for hiding this comment

Uh oh!

catenacyber commented Mar 18, 2022

Uh oh!

jasonish commented Mar 18, 2022

Uh oh!

suricata-qa commented Mar 19, 2022

Uh oh!

suricata-qa commented Mar 20, 2022

Uh oh!

victorjulien commented Mar 21, 2022

Uh oh!

victorjulien left a comment

Choose a reason for hiding this comment

Uh oh!

catenacyber commented Mar 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

codecov bot commented Mar 18, 2022 •

edited

Loading