-
Notifications
You must be signed in to change notification settings - Fork 1.6k
transform/subslice: Add subslice transform #14297
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14297 +/- ##
==========================================
- Coverage 84.17% 84.16% -0.01%
==========================================
Files 1012 1013 +1
Lines 261868 262074 +206
==========================================
+ Hits 220421 220587 +166
- Misses 41447 41487 +40
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
WARNING:
Pipeline = 28419 |
|
This looks cool. Why a draft ? |
The subslice transform creates a slice of the input buffer.
Specify the subslice desired -- nbytes is optional:
subslice: [offset <,nbytes>]
offset: Specifies the starting offset for the new subslice. When
negative, expresses how far from the end of the input buffer to begin.
nbytes: Specifies the size of the subslice. When negative, specifies the
byte count preceding the offset to include.
When nbytes is not specified, the size of the subslice will be the size
of the input buffer - offset.
Examples:
subslice[1] - The subslice will be a copy of the input
buffer but omit the input buffer's first byte
"This is Suricata" -> "his is Suricata"
subslice[0, 13] - The slice is created from the first 13 bytes
of the input buffer
"This is Suricata" -> "This is Suric"
subslice[10, -5] - The subslice is created starting at offset 10
and continuing to 5 bytes before the end. This is the same as
subslice[5, 5]
"This is Suricata" -> "is Su"
subslice[-3] - The subslice will be the last 3 bytes of the
input buffer.
"This is Suricata" -> "ata"
I don't have the doc ready and wanted to do a CI run in the meantime |
Ok, so you see you need some clippy fixes ;-) |
|
Information: QA ran without warnings. Pipeline = 28436 |
| desc: b"create a subslice from the current buffer\0".as_ptr() as *const libc::c_char, | ||
| url: b"/rules/transforms.html#subslice\0".as_ptr() as *const libc::c_char, | ||
| Setup: Some(subslice_setup), | ||
| flags: SIGMATCH_OPTIONAL_OPT, |
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.
Does it make sense without an option ? Should it not be required_opt ?
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.
good point ... i'll set that field to 0
|
|
||
| // This works because the structure is flat | ||
| // Once variables are really implemented, we should investigate if the structure should own | ||
| // its serialization or just borrow it to a caller |
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.
I am not sure this is stable over rust versions
I think DetectTransformSubsliceData should be made repr(C)
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.
will do.
| input: &[u8], output: &mut [u8], ctx: &DetectTransformSubsliceData, | ||
| ) -> u32 { | ||
| let Some(slice) = subslice_apply(input, ctx) else { | ||
| return 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.
Where are we about handling transforms errors ?
| let mut end = match ctx.end { | ||
| None => len, | ||
| Some(e) if e < 0 => { | ||
| let abs_end = start + e; |
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.
subslice: [offset <,nbytes>]
nbytes: Specifies the size of the subslice. When negative, specifies the byte count preceding the offset to include.
subslice[10, -5] - This is the same as subslice[5, 5]
This looks weird to me
I would not expect negative nbytes to change the offset
I would expect negative nbytes to be take until end of buffer expect last 5 bytes...
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.
understood ... i'll revisit this.
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.
Generally speaking, I guess the python way to subslice feels good.
But I concede Suricata is more about offset+nbytes
|
|
||
| match parts.as_slice() { | ||
| [start] => { | ||
| let start = start.parse::<isize>().ok()?; |
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.
Should we reject start=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? That would mean starting the subslice at the first byte.
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.
Meaning the subspace is the whole slice, so not interesting, right ?
| } | ||
| [start, end] => { | ||
| let start = start.parse::<isize>().ok()?; | ||
| let end = end.parse::<isize>().ok()?; |
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.
Should we reject end=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.
Good point. I'll do that.
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.
In general enforce end > start?
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.
start and end should be better named offset and nbytes if I understand correctly
catenacyber
left a comment
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.
Looks nice, left some questions.
Doc is still missing ;-)
| let start = start.parse::<isize>().ok()?; | ||
| Some(DetectTransformSubsliceData { start, end: None }) | ||
| } | ||
| [start, end] => { |
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.
think it's the first time I see this bracket notation, what does it do?
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.
I understand it is a slice of 2 elements, and so, we can name them
The subslice transform creates a slice of the input buffer.
Specify the subslice desired -- nbytes is optional:
subslice: [offset <,nbytes>]
offset: Specifies the starting offset for the new subslice. When negative, it expresses how far from the end of the input buffer to begin.
nbytes: Specifies the size of the subslice. When negative, specifies the byte count preceding the offset to include.
When nbytes is not specified, the size of the subslice will be the size of the input buffer - offset.
Examples:
subslice[1] - The subslice will be a copy of the input
buffer but omit the input buffer's first byte
"This is Suricata" -> "his is Suricata"
subslice[0, 13] - The slice is created from the first 13 bytes
of the input buffer
"This is Suricata" -> "This is Suric"
subslice[10, -5] - This is the same as subslice[5, 5]
"This is Suricata" -> "is Su"
subslice[-3] - The subslice will be the last 3 bytes of the
input buffer.
"This is Suricata" -> "ata"
Link to ticket: https://redmine.openinfosecfoundation.org/issues/7672
Describe changes:
Provide values to any of the below to override the defaults.
link to the pull request in the respective
_BRANCHvariable.SV_REPO=
SV_BRANCH=OISF/suricata-verify#2749
SU_REPO=
SU_BRANCH=