Printable/v16 #10592
Printable/v16 #10592
Conversation
Needed a workaround cast for RBTREE use.
Modeled after the same option in eve/alert. Defaults to 4k.
This avoids looping over partly duplicate segments that cause output data corruption by logging parts of the stream data multiple times. For data with GAPs now add a indicator '[4 bytes missing]' similar to how Wireshark does it. Bug: OISF#6553.
Don't init buffer to 0 size but use the desired default of 4k.
In preparation of stream logging changes.
Log using stream callback API, meaning that data will also be logged if there are GAPs. Also implement GAP indicators: '[123 bytes missing]'.
For better readability and type checking.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10592 +/- ##
===========================================
+ Coverage 64.07% 82.72% +18.64%
===========================================
Files 851 924 +73
Lines 135327 247486 +112159
===========================================
+ Hits 86716 204737 +118021
+ Misses 48611 42749 -5862
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 19077 |
| jb_set_string(jb, "payload_printable", (char *)printable_buf); | ||
| } | ||
| } else if (p->payload_len) { | ||
| if (stream && p->flow != NULL) { |
There was a problem hiding this comment.
does it work when Suricata captures the flow midway and the alert wants to dump the payload on the first packet?
There was a problem hiding this comment.
yes there will be a flow then
| ssn, stream, FrameJsonStreamDataCallback, &cbd, frame->offset, &unused, false); | ||
| /* if we have all data, but didn't log until the end of the frame, we have a gap at the | ||
| * end of the frame | ||
| * TODO what about not logging due to buffer full? */ |
There was a problem hiding this comment.
is todo intended to be left out?
| @@ -1,4 +1,4 @@ | |||
| /* Copyright (C) 2007-2012 Open Information Security Foundation | |||
| /* Copyright (C) 2007-2023 Open Information Security Foundation | |||
| @@ -1,4 +1,4 @@ | |||
| /* Copyright (C) 2007-2012 Open Information Security Foundation | |||
| /* Copyright (C) 2007-2023 Open Information Security Foundation | |||
|
I'll fix the years up if there is a reason to redo the PR. |
| uint8_t printable_buf[cbd.payload->offset + 1]; | ||
| uint32_t offset = 0; | ||
| PrintStringsToBuffer(printable_buf, &offset, sizeof(printable_buf), cbd.payload->buffer, | ||
| cbd.payload->offset); | ||
| jb_set_string(jb, "payload_printable", (char *)printable_buf); |
There was a problem hiding this comment.
Do you think it would make sense to have a variant of jb_set_string that takes care of all this?
There was a problem hiding this comment.
Yeah, that seems like a good idea. We could avoid this stack local copy, and perhaps it can be refactored to not even use the membuffer too? Would like to check that post merge though
|
Merged in #10654, thanks! |
SV_BRANCH=OISF/suricata-verify#1689
https://redmine.openinfosecfoundation.org/issues/6553
replaces #10261: