From d45c61534214f375d6c698eaed05c0249e37dd89 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 11:10:35 +0100 Subject: [PATCH 01/14] stream: const args for StreamReassembleLog Needed a workaround cast for RBTREE use. (cherry picked from commit a5a6527d26b2c4f2c133ff2a3d7e8eed81fad8cf) --- src/stream-tcp-reassemble.c | 13 ++++++------- src/stream-tcp.h | 7 +++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/stream-tcp-reassemble.c b/src/stream-tcp-reassemble.c index 737b222d53e2..a1f8b8ea683f 100644 --- a/src/stream-tcp-reassemble.c +++ b/src/stream-tcp-reassemble.c @@ -1394,7 +1394,7 @@ int StreamTcpReassembleAppLayer (ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, /** \internal * \brief get stream data from offset * \param offset stream offset */ -static int GetRawBuffer(TcpStream *stream, const uint8_t **data, uint32_t *data_len, +static int GetRawBuffer(const TcpStream *stream, const uint8_t **data, uint32_t *data_len, StreamingBufferBlock **iter, uint64_t offset, uint64_t *data_offset) { const uint8_t *mydata; @@ -1417,7 +1417,7 @@ static int GetRawBuffer(TcpStream *stream, const uint8_t **data, uint32_t *data_ *iter == NULL ? "starting" : "continuing", offset); if (*iter == NULL) { StreamingBufferBlock key = { .offset = offset, .len = 0 }; - *iter = SBB_RB_FIND_INCLUSIVE(&stream->sb.sbb_tree, &key); + *iter = SBB_RB_FIND_INCLUSIVE((struct SBB *)&stream->sb.sbb_tree, &key); SCLogDebug("*iter %p", *iter); } if (*iter == NULL) { @@ -1758,7 +1758,7 @@ static int StreamReassembleRawInline(TcpSession *ssn, const Packet *p, * `respect_inspect_depth` is used to avoid useless inspection of too * much data. */ -static int StreamReassembleRawDo(TcpSession *ssn, TcpStream *stream, +static int StreamReassembleRawDo(const TcpSession *ssn, const TcpStream *stream, StreamReassembleRawFunc Callback, void *cb_data, const uint64_t progress_in, const uint64_t re, uint64_t *progress_out, bool eof, bool respect_inspect_depth) { @@ -1920,10 +1920,9 @@ int StreamReassembleRaw(TcpSession *ssn, const Packet *p, progress_out, (p->flags & PKT_PSEUDO_STREAM_END), respect_inspect_depth); } -int StreamReassembleLog(TcpSession *ssn, TcpStream *stream, - StreamReassembleRawFunc Callback, void *cb_data, - uint64_t progress_in, - uint64_t *progress_out, bool eof) +int StreamReassembleLog(const TcpSession *ssn, const TcpStream *stream, + StreamReassembleRawFunc Callback, void *cb_data, const uint64_t progress_in, + uint64_t *progress_out, const bool eof) { if (stream->flags & (STREAMTCP_STREAM_FLAG_NOREASSEMBLY)) return 0; diff --git a/src/stream-tcp.h b/src/stream-tcp.h index 324671245995..598edc6bd115 100644 --- a/src/stream-tcp.h +++ b/src/stream-tcp.h @@ -131,10 +131,9 @@ typedef int (*StreamReassembleRawFunc)( int StreamReassembleForFrame(TcpSession *ssn, TcpStream *stream, StreamReassembleRawFunc Callback, void *cb_data, const uint64_t offset, const bool eof); -int StreamReassembleLog(TcpSession *ssn, TcpStream *stream, - StreamReassembleRawFunc Callback, void *cb_data, - uint64_t progress_in, - uint64_t *progress_out, bool eof); +int StreamReassembleLog(const TcpSession *ssn, const TcpStream *stream, + StreamReassembleRawFunc Callback, void *cb_data, const uint64_t progress_in, + uint64_t *progress_out, const bool eof); int StreamReassembleRaw(TcpSession *ssn, const Packet *p, StreamReassembleRawFunc Callback, void *cb_data, uint64_t *progress_out, bool respect_inspect_depth); From 5712f7f4b0ef76a41b9b831bb36682b7cc429eb0 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 23 Nov 2023 06:49:41 +0100 Subject: [PATCH 02/14] eve/frame: implement payload-buffer-size option Modeled after the same option in eve/alert. Defaults to 4k. (cherry picked from commit 829bab295b1bdf58c7df00a62b2d083294744b5c) --- src/output-json-frame.c | 14 ++++++++++++++ suricata.yaml.in | 1 + 2 files changed, 15 insertions(+) diff --git a/src/output-json-frame.c b/src/output-json-frame.c index d23ba92ac380..2d7c6c6c6bd1 100644 --- a/src/output-json-frame.c +++ b/src/output-json-frame.c @@ -483,8 +483,22 @@ static OutputInitResult JsonFrameLogInitCtxSub(ConfNode *conf, OutputCtx *parent } memset(json_output_ctx, 0, sizeof(FrameJsonOutputCtx)); + uint32_t payload_buffer_size = 4096; + if (conf != NULL) { + const char *payload_buffer_value = ConfNodeLookupChildValue(conf, "payload-buffer-size"); + if (payload_buffer_value != NULL) { + uint32_t value; + if (ParseSizeStringU32(payload_buffer_value, &value) < 0) { + SCLogError("Error parsing payload-buffer-size \"%s\"", payload_buffer_value); + goto error; + } + payload_buffer_size = value; + } + } + json_output_ctx->file_ctx = ajt->file_ctx; json_output_ctx->eve_ctx = ajt; + json_output_ctx->payload_buffer_size = payload_buffer_size; output_ctx->data = json_output_ctx; output_ctx->DeInit = JsonFrameLogDeInitCtxSub; diff --git a/suricata.yaml.in b/suricata.yaml.in index 458360b1e64f..8bfbaac356c7 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -176,6 +176,7 @@ outputs: - frame: # disabled by default as this is very verbose. enabled: no + # payload-buffer-size: 4kb # max size of frame payload buffer to output in eve-log - anomaly: # Anomaly log records describe unexpected conditions such # as truncated packets, packets with invalid IP/UDP/TCP From 5e4d86e129398141e75454b461ad36fc65e8ddba Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 20 Nov 2023 10:57:38 +0100 Subject: [PATCH 03/14] eve/alert: log payload directly from stream buffer 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: #6553. (cherry picked from commit 43858f70ad26fe17e2399e3a12c4ee6168f68af1) --- src/output-json-alert.c | 100 ++++++++++++++++++++++++---------------- 1 file changed, 60 insertions(+), 40 deletions(-) diff --git a/src/output-json-alert.c b/src/output-json-alert.c index 575ee97649ed..c7bbf7d3d89a 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -35,6 +35,7 @@ #include "tm-threads.h" #include "threadvars.h" #include "util-debug.h" +#include "stream-tcp.h" #include "util-logopenfile.h" #include "util-misc.h" @@ -127,17 +128,6 @@ typedef struct JsonAlertLogThread_ { OutputJsonThreadCtx *ctx; } JsonAlertLogThread; -/* Callback function to pack payload contents from a stream into a buffer - * so we can report them in JSON output. */ -static int AlertJsonDumpStreamSegmentCallback( - const Packet *p, TcpSegment *seg, void *data, const uint8_t *buf, uint32_t buflen) -{ - MemBuffer *payload = (MemBuffer *)data; - MemBufferWriteRaw(payload, buf, buflen); - - return 1; -} - static void AlertJsonTls(const Flow *f, const uint32_t sig_flags, JsonBuilder *js) { SSLState *ssl_state = (SSLState *)FlowGetAppState(f); @@ -725,9 +715,61 @@ void EveAddVerdict(JsonBuilder *jb, const Packet *p) jb_close(jb); } +struct AlertJsonStreamDataCallbackData { + MemBuffer *payload; + uint64_t last_re; +}; + +static int AlertJsonStreamDataCallback( + void *cb_data, const uint8_t *input, const uint32_t input_len, const uint64_t input_offset) +{ + struct AlertJsonStreamDataCallbackData *cbd = cb_data; + if (input_offset > cbd->last_re) { + MemBufferWriteString( + cbd->payload, "[%" PRIu64 " bytes missing]", input_offset - cbd->last_re); + } + + MemBufferWriteRaw(cbd->payload, input, input_len); + cbd->last_re = input_offset + input_len; + return 0; +} + +/** \internal + * \brief try to log stream data into payload/payload_printable + * \retval true stream data logged + * \retval false stream data not logged + */ +static bool AlertJsonStreamData(const AlertJsonOutputCtx *json_output_ctx, JsonAlertLogThread *aft, + Flow *f, const Packet *p, JsonBuilder *jb) +{ + TcpSession *ssn = f->protoctx; + TcpStream *stream = (PKT_IS_TOSERVER(p)) ? &ssn->client : &ssn->server; + + MemBufferReset(aft->payload_buffer); + struct AlertJsonStreamDataCallbackData cbd = { .payload = aft->payload_buffer, + .last_re = STREAM_BASE_OFFSET(stream) }; + uint64_t unused = 0; + StreamReassembleLog(ssn, stream, AlertJsonStreamDataCallback, &cbd, STREAM_BASE_OFFSET(stream), + &unused, false); + if (cbd.payload->offset) { + if (json_output_ctx->flags & LOG_JSON_PAYLOAD_BASE64) { + jb_set_base64(jb, "payload", cbd.payload->buffer, cbd.payload->offset); + } + + if (json_output_ctx->flags & LOG_JSON_PAYLOAD) { + 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); + } + return true; + } + return false; +} + static int AlertJson(ThreadVars *tv, JsonAlertLogThread *aft, const Packet *p) { - MemBuffer *payload = aft->payload_buffer; AlertJsonOutputCtx *json_output_ctx = aft->json_output_ctx; if (p->alerts.cnt == 0 && !(p->flags & PKT_HAS_TAG)) @@ -835,36 +877,14 @@ static int AlertJson(ThreadVars *tv, JsonAlertLogThread *aft, const Packet *p) int stream = (p->proto == IPPROTO_TCP) ? (pa->flags & (PACKET_ALERT_FLAG_STATE_MATCH | PACKET_ALERT_FLAG_STREAM_MATCH) ? 1 : 0) : 0; + DEBUG_VALIDATE_BUG_ON( + p->flow == NULL); // should be impossible, but scan-build got confused /* Is this a stream? If so, pack part of it into the payload field */ - if (stream) { - uint8_t flag; - - MemBufferReset(payload); - - if (p->flowflags & FLOW_PKT_TOSERVER) { - flag = STREAM_DUMP_TOCLIENT; - } else { - flag = STREAM_DUMP_TOSERVER; - } - - StreamSegmentForEach((const Packet *)p, flag, - AlertJsonDumpStreamSegmentCallback, - (void *)payload); - if (payload->offset) { - if (json_output_ctx->flags & LOG_JSON_PAYLOAD_BASE64) { - jb_set_base64(jb, "payload", payload->buffer, payload->offset); - } - - if (json_output_ctx->flags & LOG_JSON_PAYLOAD) { - uint8_t printable_buf[payload->offset + 1]; - uint32_t offset = 0; - PrintStringsToBuffer(printable_buf, &offset, - sizeof(printable_buf), - payload->buffer, payload->offset); - jb_set_string(jb, "payload_printable", (char *)printable_buf); - } - } else if (p->payload_len) { + if (stream && p->flow != NULL) { + const bool stream_data_logged = + AlertJsonStreamData(json_output_ctx, aft, p->flow, p, jb); + if (!stream_data_logged && p->payload_len) { /* Fallback on packet payload */ AlertAddPayload(json_output_ctx, jb, p); } From c57203ddda07a0ade694b318ac8dc3c61e9902bb Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 23 Nov 2023 06:49:12 +0100 Subject: [PATCH 04/14] eve/alert: init membuffer size on missing config Don't init buffer to 0 size but use the desired default of 4k. (cherry picked from commit 462a6d7913c927dba7d1d8313acf137d7d87b071) --- src/output-json-alert.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/output-json-alert.c b/src/output-json-alert.c index c7bbf7d3d89a..1ca42df8ad52 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -1137,14 +1137,13 @@ static void JsonAlertLogSetupMetadata(AlertJsonOutputCtx *json_output_ctx, warn_no_meta = true; } } - - json_output_ctx->payload_buffer_size = payload_buffer_size; } if (flags & LOG_JSON_RULE_METADATA) { DetectEngineSetParseMetadata(); } + json_output_ctx->payload_buffer_size = payload_buffer_size; json_output_ctx->flags |= flags; } From cfcd85edd33372f6c7966f216853a83b03b6944a Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 21 Nov 2023 14:24:12 +0100 Subject: [PATCH 05/14] eve/frames: pass membuffer to API In preparation of stream logging changes. (cherry picked from commit a205583269eaec92fae05026f32fc2cd748c0bb5) --- src/output-json-alert.c | 9 +++++---- src/output-json-frame.c | 6 +++--- src/output-json-frame.h | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/output-json-alert.c b/src/output-json-alert.c index 1ca42df8ad52..3fecc51a2034 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -627,7 +627,8 @@ static void AlertAddFiles(const Packet *p, JsonBuilder *jb, const uint64_t tx_id } } -static void AlertAddFrame(const Packet *p, JsonBuilder *jb, const int64_t frame_id) +static void AlertAddFrame( + const Packet *p, const int64_t frame_id, JsonBuilder *jb, MemBuffer *buffer) { if (p->flow == NULL || (p->proto == IPPROTO_TCP && p->flow->protoctx == NULL)) return; @@ -649,7 +650,7 @@ static void AlertAddFrame(const Packet *p, JsonBuilder *jb, const int64_t frame_ } Frame *frame = FrameGetById(frames, frame_id); if (frame != NULL) { - FrameJsonLogOneFrame(IPPROTO_TCP, frame, p->flow, stream, p, jb); + FrameJsonLogOneFrame(IPPROTO_TCP, frame, p->flow, stream, p, jb, buffer); } } else if (p->proto == IPPROTO_UDP) { if (PKT_IS_TOSERVER(p)) { @@ -659,7 +660,7 @@ static void AlertAddFrame(const Packet *p, JsonBuilder *jb, const int64_t frame_ } Frame *frame = FrameGetById(frames, frame_id); if (frame != NULL) { - FrameJsonLogOneFrame(IPPROTO_UDP, frame, p->flow, NULL, p, jb); + FrameJsonLogOneFrame(IPPROTO_UDP, frame, p->flow, NULL, p, jb, buffer); } } } @@ -897,7 +898,7 @@ static int AlertJson(ThreadVars *tv, JsonAlertLogThread *aft, const Packet *p) } if (pa->flags & PACKET_ALERT_FLAG_FRAME) { - AlertAddFrame(p, jb, pa->frame_id); + AlertAddFrame(p, pa->frame_id, jb, aft->payload_buffer); } /* base64-encoded full packet */ diff --git a/src/output-json-frame.c b/src/output-json-frame.c index 2d7c6c6c6bd1..7e2bc28deae5 100644 --- a/src/output-json-frame.c +++ b/src/output-json-frame.c @@ -224,7 +224,7 @@ static void FrameAddPayloadUDP(JsonBuilder *js, const Packet *p, const Frame *fr * \note ipproto argument is passed to assist static code analyzers */ void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, const Flow *f, - const TcpStream *stream, const Packet *p, JsonBuilder *jb) + const TcpStream *stream, const Packet *p, JsonBuilder *jb, MemBuffer *buffer) { DEBUG_VALIDATE_BUG_ON(ipproto != p->proto); DEBUG_VALIDATE_BUG_ON(ipproto != f->proto); @@ -287,7 +287,7 @@ static int FrameJsonUdp( return TM_ECODE_OK; jb_set_string(jb, "app_proto", AppProtoToString(f->alproto)); - FrameJsonLogOneFrame(IPPROTO_UDP, frame, p->flow, NULL, p, jb); + FrameJsonLogOneFrame(IPPROTO_UDP, frame, p->flow, NULL, p, jb, aft->payload_buffer); OutputJsonBuilderBuffer(jb, aft->ctx); jb_free(jb); frame->flags |= FRAME_FLAG_LOGGED; @@ -359,7 +359,7 @@ static int FrameJson(ThreadVars *tv, JsonFrameLogThread *aft, const Packet *p) return TM_ECODE_OK; jb_set_string(jb, "app_proto", AppProtoToString(p->flow->alproto)); - FrameJsonLogOneFrame(IPPROTO_TCP, frame, p->flow, stream, p, jb); + FrameJsonLogOneFrame(IPPROTO_TCP, frame, p->flow, stream, p, jb, aft->payload_buffer); OutputJsonBuilderBuffer(jb, aft->ctx); jb_free(jb); frame->flags |= FRAME_FLAG_LOGGED; diff --git a/src/output-json-frame.h b/src/output-json-frame.h index 009e7b6426bb..4fc038c2542f 100644 --- a/src/output-json-frame.h +++ b/src/output-json-frame.h @@ -31,7 +31,7 @@ #include "stream-tcp-private.h" void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, const Flow *f, - const TcpStream *stream, const Packet *p, JsonBuilder *jb); + const TcpStream *stream, const Packet *p, JsonBuilder *jb, MemBuffer *); void JsonFrameLogRegister(void); #endif /* __OUTPUT_JSON_FRAME_H__ */ From c9c8b6883f1c5869b5d74628529f7d9cd82062f8 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 21 Nov 2023 16:27:16 +0100 Subject: [PATCH 06/14] eve/frame: improve frame payload logging Log using stream callback API, meaning that data will also be logged if there are GAPs. Also implement GAP indicators: '[123 bytes missing]'. (cherry picked from commit 6e10c660782044472fd0fb2bdc91c4c102c7fb5b) --- src/output-json-frame.c | 113 +++++++++++++++++++++++++--------------- src/output-json-frame.h | 2 +- 2 files changed, 71 insertions(+), 44 deletions(-) diff --git a/src/output-json-frame.c b/src/output-json-frame.c index 7e2bc28deae5..b587010a25c2 100644 --- a/src/output-json-frame.c +++ b/src/output-json-frame.c @@ -125,61 +125,88 @@ static void PayloadAsHex(const uint8_t *data, uint32_t data_len, char *str, size } #endif -static void FrameAddPayloadTCP(JsonBuilder *js, const TcpStream *stream, const Frame *frame) +struct FrameJsonStreamDataCallbackData { + MemBuffer *payload; + const Frame *frame; + uint64_t last_re; /**< used to detect gaps */ +}; + +static int FrameJsonStreamDataCallback( + void *cb_data, const uint8_t *input, const uint32_t input_len, const uint64_t input_offset) { - uint32_t sb_data_len = 0; - const uint8_t *data = NULL; - uint64_t data_offset = 0; + struct FrameJsonStreamDataCallbackData *cbd = cb_data; + const Frame *frame = cbd->frame; - // TODO consider ACK'd + uint32_t write_size = input_len; + int done = 0; - if (frame->offset < STREAM_BASE_OFFSET(stream)) { - if (StreamingBufferGetData(&stream->sb, &data, &sb_data_len, &data_offset) == 0) { - SCLogDebug("NO DATA1"); - return; + if (frame->len >= 0) { + const uint64_t data_re = input_offset + input_len; + const uint64_t frame_re = frame->offset + (uint64_t)frame->len; + + /* data entirely after frame, we're done */ + if (input_offset >= frame_re) { + return 1; } - } else { - data_offset = (uint64_t)frame->offset; - SCLogDebug("data_offset %" PRIu64, data_offset); - if (StreamingBufferGetDataAtOffset( - &stream->sb, &data, &sb_data_len, (uint64_t)data_offset) == 0) { - SCLogDebug("NO DATA1"); - return; + /* make sure to only log data belonging to the frame */ + if (data_re >= frame_re) { + const uint64_t to_write = frame_re - input_offset; + if (to_write < (uint64_t)write_size) { + write_size = (uint32_t)to_write; + } + done = 1; } } - if (data == NULL || sb_data_len == 0) { - SCLogDebug("NO DATA2"); - return; + if (input_offset > cbd->last_re) { + MemBufferWriteString( + cbd->payload, "[%" PRIu64 " bytes missing]", input_offset - cbd->last_re); } - if (frame->len >= 0) { - sb_data_len = MIN(frame->len, (int32_t)sb_data_len); + if (write_size > 0) { + MemBufferWriteRaw(cbd->payload, input, write_size); } - SCLogDebug("frame data_offset %" PRIu64 ", data_len %u frame len %" PRIi64, data_offset, - sb_data_len, frame->len); + cbd->last_re = input_offset + write_size; + return done; +} + +/** \internal + * \brief try to log frame's stream data into payload/payload_printable + */ +static void FrameAddPayloadTCP(Flow *f, const TcpSession *ssn, const TcpStream *stream, + const Frame *frame, JsonBuilder *jb, MemBuffer *buffer) +{ + MemBufferReset(buffer); + /* consider all data, ACK'd and non-ACK'd */ + const uint64_t stream_data_re = StreamDataRightEdge(stream, true); bool complete = false; - if (frame->len > 0) { - const uint64_t frame_re = frame->offset + (uint64_t)frame->len; - const uint64_t data_re = data_offset + sb_data_len; - complete = frame_re <= data_re; + if (frame->len >= 0 && frame->offset + (uint64_t)frame->len <= stream_data_re) { + complete = true; } - jb_set_bool(js, "complete", complete); - uint32_t data_len = MIN(sb_data_len, 256); - jb_set_base64(js, "payload", data, data_len); + struct FrameJsonStreamDataCallbackData cbd = { + .payload = buffer, .frame = frame, .last_re = frame->offset + }; + uint64_t unused = 0; + StreamReassembleLog( + 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? */ + if (complete && frame->len >= 0 && cbd.last_re < frame->offset + (uint64_t)frame->len) { + MemBufferWriteString(cbd.payload, "[%" PRIu64 " bytes missing]", + (frame->offset + (uint64_t)frame->len) - cbd.last_re); + } - uint8_t printable_buf[data_len + 1]; - uint32_t o = 0; - PrintStringsToBuffer(printable_buf, &o, data_len + 1, data, data_len); - printable_buf[data_len] = '\0'; - jb_set_string(js, "payload_printable", (char *)printable_buf); -#if 0 - char pretty_buf[data_len * 4 + 1]; - pretty_buf[0] = '\0'; - PayloadAsHex(data, data_len, pretty_buf, data_len * 4 + 1); - jb_set_string(js, "payload_hex", pretty_buf); -#endif + if (cbd.payload->offset) { + jb_set_base64(jb, "payload", cbd.payload->buffer, cbd.payload->offset); + 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); + jb_set_bool(jb, "complete", complete); + } } static void FrameAddPayloadUDP(JsonBuilder *js, const Packet *p, const Frame *frame) @@ -223,7 +250,7 @@ static void FrameAddPayloadUDP(JsonBuilder *js, const Packet *p, const Frame *fr /** \brief log a single frame * \note ipproto argument is passed to assist static code analyzers */ -void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, const Flow *f, +void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, Flow *f, const TcpStream *stream, const Packet *p, JsonBuilder *jb, MemBuffer *buffer) { DEBUG_VALIDATE_BUG_ON(ipproto != p->proto); @@ -249,7 +276,7 @@ void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, const Flow } else { jb_set_uint(jb, "length", frame->len); } - FrameAddPayloadTCP(jb, stream, frame); + FrameAddPayloadTCP(f, f->protoctx, stream, frame, jb, buffer); } else { jb_set_uint(jb, "length", frame->len); FrameAddPayloadUDP(jb, p, frame); diff --git a/src/output-json-frame.h b/src/output-json-frame.h index 4fc038c2542f..386882eb6e7b 100644 --- a/src/output-json-frame.h +++ b/src/output-json-frame.h @@ -30,7 +30,7 @@ #include "app-layer-frames.h" #include "stream-tcp-private.h" -void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, const Flow *f, +void FrameJsonLogOneFrame(const uint8_t ipproto, const Frame *frame, Flow *f, const TcpStream *stream, const Packet *p, JsonBuilder *jb, MemBuffer *); void JsonFrameLogRegister(void); From 7bab2fb594383d1baeaeab5eb483c38ec82d61d8 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 13:58:12 +0100 Subject: [PATCH 07/14] unix-manager: add \n string to buffer using correct API call (cherry picked from commit ea98df8da25e1bd1505a13d39ae52f5b54dbaa03) --- src/unix-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unix-manager.c b/src/unix-manager.c index 9fb5bd7935bc..df53f3c2f2cf 100644 --- a/src/unix-manager.c +++ b/src/unix-manager.c @@ -300,7 +300,7 @@ static int UnixCommandSendJSONToClient(UnixClient *client, json_t *js) if (MEMBUFFER_OFFSET(client->mbuf) + 1 >= MEMBUFFER_SIZE(client->mbuf)) { MemBufferExpand(&client->mbuf, 1); } - MemBufferWriteRaw(client->mbuf, "\n", 1); + MemBufferWriteString(client->mbuf, "\n"); } if (send(client->fd, (const char *)MEMBUFFER_BUFFER(client->mbuf), From 99ba333aee6ffa07eac267e2789494e7489ef2c0 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 13:58:43 +0100 Subject: [PATCH 08/14] membuffer: turn complex macros into functions For better readability and type checking. (cherry picked from commit 3ef98f2b87e3cd1de022d9e7eb1581730c08bcdb) --- src/output-json.c | 6 ++-- src/util-buffer.c | 69 +++++++++++++++++++++++++++++++++++++++---- src/util-buffer.h | 75 ++++++++++------------------------------------- 3 files changed, 81 insertions(+), 69 deletions(-) diff --git a/src/output-json.c b/src/output-json.c index 6e5ff238d7a5..1d9f8b214692 100644 --- a/src/output-json.c +++ b/src/output-json.c @@ -933,7 +933,7 @@ int OutputJSONMemBufferCallback(const char *str, size_t size, void *data) MemBufferExpand(memb, wrapper->expand_by); } - MemBufferWriteRaw((*memb), str, size); + MemBufferWriteRaw((*memb), (const uint8_t *)str, size); return 0; } @@ -949,7 +949,7 @@ int OutputJSONBuffer(json_t *js, LogFileCtx *file_ctx, MemBuffer **buffer) } if (file_ctx->prefix) { - MemBufferWriteRaw((*buffer), file_ctx->prefix, file_ctx->prefix_len); + MemBufferWriteRaw((*buffer), (const uint8_t *)file_ctx->prefix, file_ctx->prefix_len); } OutputJSONMemBufferWrapper wrapper = { @@ -983,7 +983,7 @@ int OutputJsonBuilderBuffer(JsonBuilder *js, OutputJsonThreadCtx *ctx) MemBufferReset(*buffer); if (file_ctx->prefix) { - MemBufferWriteRaw((*buffer), file_ctx->prefix, file_ctx->prefix_len); + MemBufferWriteRaw((*buffer), (const uint8_t *)file_ctx->prefix, file_ctx->prefix_len); } size_t jslen = jb_len(js); diff --git a/src/util-buffer.c b/src/util-buffer.c index 2dd94f6eac3b..71a92f7b6247 100644 --- a/src/util-buffer.c +++ b/src/util-buffer.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2012 Open Information Security Foundation +/* Copyright (C) 2007-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -40,15 +40,13 @@ MemBuffer *MemBufferCreateNew(uint32_t size) return NULL; } - uint32_t total_size = size + sizeof(MemBuffer); + size_t total_size = size + sizeof(MemBuffer); MemBuffer *buffer = SCMalloc(total_size); if (unlikely(buffer == NULL)) { sc_errno = SC_ENOMEM; return NULL; } - memset(buffer, 0, total_size); - buffer->size = size; buffer->buffer = (uint8_t *)buffer + sizeof(MemBuffer); @@ -69,13 +67,12 @@ int MemBufferExpand(MemBuffer **buffer, uint32_t expand_by) { return -1; } - uint32_t total_size = (*buffer)->size + sizeof(MemBuffer) + expand_by; + size_t total_size = (*buffer)->size + sizeof(MemBuffer) + expand_by; MemBuffer *tbuffer = SCRealloc(*buffer, total_size); if (unlikely(tbuffer == NULL)) { return -1; } - *buffer = tbuffer; (*buffer)->size += expand_by; (*buffer)->buffer = (uint8_t *)tbuffer + sizeof(MemBuffer); @@ -90,3 +87,63 @@ void MemBufferFree(MemBuffer *buffer) return; } + +void MemBufferPrintToFP(MemBuffer *buffer, FILE *fp) +{ + for (uint32_t i = 0; i < buffer->offset; i++) { + if (isprint(buffer->buffer[i])) + fprintf(fp, "%c", buffer->buffer[i]); + else + fprintf(fp, "|%02X|", buffer->buffer[i]); + } +} + +size_t MemBufferPrintToFPAsString(MemBuffer *b, FILE *fp) +{ + return fwrite(MEMBUFFER_BUFFER(b), sizeof(uint8_t), MEMBUFFER_OFFSET(b), fp); +} + +void MemBufferPrintToFPAsHex(MemBuffer *b, FILE *fp) +{ + for (uint32_t i = 0; i < MEMBUFFER_OFFSET(b); i++) { + if (MEMBUFFER_OFFSET(b) % 8 == 0) + fprintf(fp, "\n"); + fprintf(fp, " %02X", b->buffer[i]); + } +} + +void MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_len) +{ + uint32_t write_len; + if (raw_len >= dst->size - dst->offset) { + SCLogDebug("Truncating data write since it exceeded buffer limit of %" PRIu32, dst->size); + write_len = dst->size - dst->offset - 1; + } else { + write_len = raw_len; + } + memcpy(dst->buffer + dst->offset, raw, write_len); + dst->offset += write_len; + dst->buffer[dst->offset] = '\0'; +} + +void MemBufferWriteString(MemBuffer *dst, const char *fmt, ...) +{ + uint32_t available = dst->size - dst->offset; + uint32_t max_string_size = MIN(available, 2048); + va_list ap; + char string[max_string_size]; + va_start(ap, fmt); + int written = vsnprintf(string, sizeof(string), fmt, ap); + va_end(ap); + if (written < 0) { + return; + } else if ((uint32_t)written > max_string_size) { + SCLogDebug("Truncating data write since it exceeded buffer " + "limit of %" PRIu32, + dst->size); + } + size_t string_size = strlen(string); + memcpy(dst->buffer + dst->offset, string, string_size); + dst->offset += string_size; + dst->buffer[dst->offset] = '\0'; +} diff --git a/src/util-buffer.h b/src/util-buffer.h index 3a88c8e96b82..a653ee044483 100644 --- a/src/util-buffer.h +++ b/src/util-buffer.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2012 Open Information Security Foundation +/* Copyright (C) 2007-2023 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -39,10 +39,11 @@ void MemBufferFree(MemBuffer *buffer); * * \param mem_buffer Pointer to the mem buffer instance. */ -#define MemBufferReset(mem_buffer) do { \ - (mem_buffer)->buffer[0] = 0; \ - (mem_buffer)->offset = 0; \ - } while (0) +static inline void MemBufferReset(MemBuffer *b) +{ + b->buffer[0] = 0; + b->offset = 0; +} /** * \brief Get the MemBuffers underlying buffer. @@ -73,43 +74,24 @@ void MemBufferFree(MemBuffer *buffer); * \param buffer Pointer to the src MemBuffer instance to write. * \param fp Pointer to the file instance to write to. */ -#define MemBufferPrintToFP(buffer, fp) do { \ - uint32_t i; \ - \ - for (i = 0; i < (buffer)->offset; i++) { \ - if (isprint(buffer->buffer[i])) \ - fprintf(fp, "%c", (buffer)->buffer[i]); \ - else \ - fprintf(fp, "|%02X|", (buffer)->buffer[i]); \ - } \ - } while (0) +void MemBufferPrintToFP(MemBuffer *buffer, FILE *fp); /** * \brief Write a buffer to the file pointer as a printable char string. * - * \param buffer Pointer to the src MemBuffer instance to write. - * \param fp Pointer to the file instance to write to. + * \param b Pointer to the src MemBuffer instance to write. + * \param fp Pointer to the file instance to write to. + * \retval size_t bytes written by fwrite() */ -#define MemBufferPrintToFPAsString(mem_buffer, fp) ({ \ - fwrite((mem_buffer)->buffer, sizeof(uint8_t), (mem_buffer)->offset, fp); \ -}) +size_t MemBufferPrintToFPAsString(MemBuffer *b, FILE *fp); /** * \brief Write a buffer in hex format. * - * \param buffer Pointer to the src MemBuffer instance to write. + * \param b Pointer to the src MemBuffer instance to write. * \param fp Pointer to the file instance to write to. */ -#define MemBufferPrintToFPAsHex(mem_buffer, fp) do { \ - uint32_t i; \ - \ - for (i = 0; i < (mem_buffer)->offset; i++) { \ - if (((mem_buffer)->offset % 8) == 0) \ - fprintf(fp, "\n"); \ - fprintf(fp, " %02X", (mem_buffer)->buffer[i]); \ - } \ - } while (0) - +void MemBufferPrintToFPAsHex(MemBuffer *b, FILE *fp); /** * \brief Write a raw buffer to the MemBuffer dst. @@ -130,21 +112,7 @@ void MemBufferFree(MemBuffer *buffer); * \param raw_buffer The buffer to write. * \param raw_buffer_len Length of the above buffer. */ -#define MemBufferWriteRaw(dst, raw_buffer, raw_buffer_len) do { \ - uint32_t write_len; \ - \ - if (((raw_buffer_len) >= (dst)->size - (dst)->offset)) { \ - SCLogDebug("Truncating data write since it exceeded buffer limit of " \ - "- %"PRIu32, (dst)->size); \ - write_len = ((dst)->size - (dst)->offset) - 1; \ - } else { \ - write_len = (raw_buffer_len); \ - } \ - \ - memcpy((dst)->buffer + (dst)->offset, (raw_buffer), write_len); \ - (dst)->offset += write_len; \ - dst->buffer[dst->offset] = '\0'; \ - } while (0) +void MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_len); /** * \brief Write a string buffer to the Membuffer dst. @@ -159,19 +127,6 @@ void MemBufferFree(MemBuffer *buffer); * \param format The format string. * \param ... Variable arguments. */ -#define MemBufferWriteString(dst, ...) do { \ - int cw = snprintf((char *)(dst)->buffer + (dst)->offset, \ - (dst)->size - (dst)->offset, \ - __VA_ARGS__); \ - if (cw >= 0) { \ - if ( ((dst)->offset + cw) >= (dst)->size) { \ - SCLogDebug("Truncating data write since it exceeded buffer " \ - "limit of - %"PRIu32"\n", (dst)->size); \ - (dst)->offset = (dst)->size - 1; \ - } else { \ - (dst->offset) += cw; \ - } \ - } \ - } while (0) +void MemBufferWriteString(MemBuffer *dst, const char *fmt, ...); #endif /* __UTIL_BUFFER_H__ */ From f9208e9d7dd646314d665aae834b18a731b7fdb6 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 16:12:47 +0100 Subject: [PATCH 09/14] membuffer: use buffer pointer as flexible array member (cherry picked from commit 9c3669b03fc3903c30ced9088361e74fd4aec04f) An additional change was made to correct an ASAN issue -- the membuffer is reset following allocation in MemBufferCreateNew(). --- src/util-buffer.c | 5 +---- src/util-buffer.h | 6 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/util-buffer.c b/src/util-buffer.c index 71a92f7b6247..5b52a05dc41e 100644 --- a/src/util-buffer.c +++ b/src/util-buffer.c @@ -42,14 +42,12 @@ MemBuffer *MemBufferCreateNew(uint32_t size) size_t total_size = size + sizeof(MemBuffer); - MemBuffer *buffer = SCMalloc(total_size); + MemBuffer *buffer = SCCalloc(1, total_size); if (unlikely(buffer == NULL)) { sc_errno = SC_ENOMEM; return NULL; } buffer->size = size; - buffer->buffer = (uint8_t *)buffer + sizeof(MemBuffer); - return buffer; } @@ -75,7 +73,6 @@ int MemBufferExpand(MemBuffer **buffer, uint32_t expand_by) { } *buffer = tbuffer; (*buffer)->size += expand_by; - (*buffer)->buffer = (uint8_t *)tbuffer + sizeof(MemBuffer); SCLogDebug("expanded buffer by %u, size is now %u", expand_by, (*buffer)->size); return 0; diff --git a/src/util-buffer.h b/src/util-buffer.h index a653ee044483..e58a790d7b26 100644 --- a/src/util-buffer.h +++ b/src/util-buffer.h @@ -25,9 +25,9 @@ #define __UTIL_BUFFER_H__ typedef struct MemBuffer_ { - uint8_t *buffer; - uint32_t size; - uint32_t offset; + uint32_t size; + uint32_t offset; + uint8_t buffer[]; } MemBuffer; MemBuffer *MemBufferCreateNew(uint32_t size); From d1c421e6ef9673d0b08c66772648b9345fb0e8b0 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 16:10:16 +0100 Subject: [PATCH 10/14] membuffer: return bytes written (cherry picked from commit 7d5b537f5cec0e88d4442a81500254b98004f117) --- src/util-buffer.c | 3 ++- src/util-buffer.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util-buffer.c b/src/util-buffer.c index 5b52a05dc41e..05b365825818 100644 --- a/src/util-buffer.c +++ b/src/util-buffer.c @@ -109,7 +109,7 @@ void MemBufferPrintToFPAsHex(MemBuffer *b, FILE *fp) } } -void MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_len) +uint32_t MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_len) { uint32_t write_len; if (raw_len >= dst->size - dst->offset) { @@ -121,6 +121,7 @@ void MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_le memcpy(dst->buffer + dst->offset, raw, write_len); dst->offset += write_len; dst->buffer[dst->offset] = '\0'; + return write_len; } void MemBufferWriteString(MemBuffer *dst, const char *fmt, ...) diff --git a/src/util-buffer.h b/src/util-buffer.h index e58a790d7b26..58bee053979d 100644 --- a/src/util-buffer.h +++ b/src/util-buffer.h @@ -111,8 +111,9 @@ void MemBufferPrintToFPAsHex(MemBuffer *b, FILE *fp); * * \param raw_buffer The buffer to write. * \param raw_buffer_len Length of the above buffer. + * \retval write_len Bytes written. If less than raw_len, the buffer is full. */ -void MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_len); +uint32_t MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t raw_len); /** * \brief Write a string buffer to the Membuffer dst. From 750bb1b4be2e0ba1284ee406cbd2eec0e4460217 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 15:53:23 +0100 Subject: [PATCH 11/14] eve/frame: break out of logging callback if buffer is full (cherry picked from commit 1dea4fea0b3989f6a76d5ea012588f32e20702ac) --- src/output-json-frame.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/output-json-frame.c b/src/output-json-frame.c index b587010a25c2..69e74a7e92dd 100644 --- a/src/output-json-frame.c +++ b/src/output-json-frame.c @@ -163,7 +163,9 @@ static int FrameJsonStreamDataCallback( } if (write_size > 0) { - MemBufferWriteRaw(cbd->payload, input, write_size); + uint32_t written = MemBufferWriteRaw(cbd->payload, input, write_size); + if (written < write_size) + done = 1; } cbd->last_re = input_offset + write_size; return done; From c20e1e9eaa608d1ef726b3ffd627b837788392c1 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 16:02:14 +0100 Subject: [PATCH 12/14] eve/alert: break out of payload logging callback if buffer is full (cherry picked from commit 926c6e3addad81cb696e478c8648abb4d7384fbe) --- src/output-json-alert.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/output-json-alert.c b/src/output-json-alert.c index 3fecc51a2034..e74906b7bea7 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -730,9 +730,12 @@ static int AlertJsonStreamDataCallback( cbd->payload, "[%" PRIu64 " bytes missing]", input_offset - cbd->last_re); } - MemBufferWriteRaw(cbd->payload, input, input_len); + int done = 0; + uint32_t written = MemBufferWriteRaw(cbd->payload, input, input_len); + if (written < input_len) + done = 1; cbd->last_re = input_offset + input_len; - return 0; + return done; } /** \internal From dcbef39b303ad0b6f1812ee67f6678fa1637e167 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 24 Nov 2023 17:06:20 +0100 Subject: [PATCH 13/14] membuffer: annotate printf style function (cherry picked from commit ff8597d50bebe92a9bf25df61d091f530c30791d) --- src/util-buffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util-buffer.h b/src/util-buffer.h index 58bee053979d..a08ac3804286 100644 --- a/src/util-buffer.h +++ b/src/util-buffer.h @@ -128,6 +128,6 @@ uint32_t MemBufferWriteRaw(MemBuffer *dst, const uint8_t *raw, const uint32_t ra * \param format The format string. * \param ... Variable arguments. */ -void MemBufferWriteString(MemBuffer *dst, const char *fmt, ...); +void MemBufferWriteString(MemBuffer *dst, const char *fmt, ...) ATTR_FMT_PRINTF(2, 3); #endif /* __UTIL_BUFFER_H__ */ From 6280ceedadde95e7e32ff06065e7c5a2474b6eb9 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 20 Mar 2024 07:18:44 +0100 Subject: [PATCH 14/14] eve/alert: fix validation check Bug: #6875. (cherry picked from commit 0be3ba802e1433632e48a7160cc6ae9fbe4c239e) --- src/output-json-alert.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/output-json-alert.c b/src/output-json-alert.c index e74906b7bea7..8dade1901bac 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -881,8 +881,8 @@ static int AlertJson(ThreadVars *tv, JsonAlertLogThread *aft, const Packet *p) int stream = (p->proto == IPPROTO_TCP) ? (pa->flags & (PACKET_ALERT_FLAG_STATE_MATCH | PACKET_ALERT_FLAG_STREAM_MATCH) ? 1 : 0) : 0; - DEBUG_VALIDATE_BUG_ON( - p->flow == NULL); // should be impossible, but scan-build got confused + // should be impossible, as stream implies flow + DEBUG_VALIDATE_BUG_ON(stream && p->flow == NULL); /* Is this a stream? If so, pack part of it into the payload field */ if (stream && p->flow != NULL) {