Skip to content

Detect generic uint 4112 v1#7117

Closed
catenacyber wants to merge 11 commits intoOISF:masterfrom
catenacyber:detect-generic-uint-4112-v1
Closed

Detect generic uint 4112 v1#7117
catenacyber wants to merge 11 commits intoOISF:masterfrom
catenacyber:detect-generic-uint-4112-v1

Conversation

@catenacyber
Copy link
Contributor

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

Describe changes:

  • Makes use of generic DetectUint structure for dsize and dcerpc

Still TODO:

  • remove the C version to use only rust version
  • more keywords in C use specific versions

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
ie <0 is impossible
@catenacyber catenacyber requested review from a team, jasonish and victorjulien as code owners March 6, 2022 12:54
}

#[derive(Debug)]
pub struct DetectU32Data {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasonish how do you recommend doing this ?
We will need the same structure and code for u8, u16, u32 and u64

Copy link
Member

Choose a reason for hiding this comment

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

I guess one could use an enum?

enum UintValue {
    U8(u8),
    U16(u6),
    U32(u32),
}

struct DetectUintData {
    pub val: UintValue,
    pub valrange: UintValue,
}

it will at least reduce non-common code that is only size specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this, val could be u8 and valrange u16 ...
I will look into it a bit more, see how other crates handled it...

Copy link
Contributor Author

@catenacyber catenacyber Mar 10, 2022

Choose a reason for hiding this comment

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

I see that num-integer (used by suricata) uses macros for such as case cf https://github.com/rust-num/num-integer/blob/master/src/roots.rs#L165

@jasonish is not there a derive thing like suricata_derive::AppLayerEvent ?

Or maybe use crate https://docs.rs/num/0.4.0/num/integer/trait.Integer.html ?

Copy link
Member

Choose a reason for hiding this comment

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

I see that num-integer (used by suricata) uses macros for such as case cf https://github.com/rust-num/num-integer/blob/master/src/roots.rs#L165

@jasonish is not there a derive thing like suricata_derive::AppLayerEvent ?

No, we'd have to write that.

Or maybe use crate https://docs.rs/num/0.4.0/num/integer/trait.Integer.html ?

If these fit your needs, then I think they are good to use. We already have them in our deps. I don't think I've used them myself though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Managing something with rust generics... let's talk in next version of PR ;-)

@catenacyber catenacyber marked this pull request as draft March 6, 2022 12:56
@catenacyber
Copy link
Contributor Author

cc @inashivb as well.
Replaced by #7121

@catenacyber catenacyber closed this Mar 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants