From 27f1cacfde63ab4c809f35646b9e139dc740d596 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 1 Aug 2023 14:00:54 +0200 Subject: [PATCH 1/5] threads: improve spawn failure error reporting --- src/tm-threads.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tm-threads.c b/src/tm-threads.c index 7bc1172a42d6..63bf3be19db4 100644 --- a/src/tm-threads.c +++ b/src/tm-threads.c @@ -1671,7 +1671,8 @@ TmEcode TmThreadSpawn(ThreadVars *tv) int rc = pthread_create(&tv->t, &attr, tv->tm_func, (void *)tv); if (rc) { - FatalError("Unable to create thread with pthread_create() is %" PRId32, rc); + FatalError("Unable to create thread with pthread_create(): retval %d: %s", rc, + strerror(errno)); } #if DEBUG && HAVE_PTHREAD_GETATTR_NP From a32488ab81aae443e4f948b596dcb4ded7c7e3bc Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 31 Jul 2023 21:52:18 +0200 Subject: [PATCH 2/5] pcap/file: normalize file timestamps Normalize the timestamps that are too far in the past to epoch. Bug: #6240. --- src/source-pcap-file-helper.c | 2 +- src/util-time.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/source-pcap-file-helper.c b/src/source-pcap-file-helper.c index 8853080e9175..936b65fb3d9f 100644 --- a/src/source-pcap-file-helper.c +++ b/src/source-pcap-file-helper.c @@ -77,7 +77,7 @@ void PcapFileCallbackLoop(char *user, struct pcap_pkthdr *h, u_char *pkt) PACKET_PROFILING_TMM_START(p, TMM_RECEIVEPCAPFILE); PKT_SET_SRC(p, PKT_SRC_WIRE); - p->ts = SCTIME_FROM_TIMEVAL(&h->ts); + p->ts = SCTIME_FROM_TIMEVAL_UNTRUSTED(&h->ts); SCLogDebug("p->ts.tv_sec %" PRIuMAX "", (uintmax_t)SCTIME_SECS(p->ts)); p->datalink = ptv->datalink; p->pcap_cnt = ++pcap_g.cnt; diff --git a/src/util-time.h b/src/util-time.h index 5be13ebdbca8..9bbd8798dd17 100644 --- a/src/util-time.h +++ b/src/util-time.h @@ -73,6 +73,13 @@ typedef struct { { \ .secs = (tv)->tv_sec, .usecs = (tv)->tv_usec \ } +/** \brief variant to deal with potentially bad timestamps, like from pcap files */ +#define SCTIME_FROM_TIMEVAL_UNTRUSTED(tv) \ + (SCTime_t) \ + { \ + .secs = ((tv)->tv_sec > 0) ? (tv)->tv_sec : 0, \ + .usecs = ((tv)->tv_usec > 0) ? (tv)->tv_usec : 0 \ + } #define SCTIME_FROM_TIMESPEC(ts) \ (SCTime_t) \ { \ From 3209dea2dc9f4d1dcac1c20cfd6eeb14dd3a3a05 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 31 Jul 2023 21:54:45 +0200 Subject: [PATCH 3/5] ftp: reenable debug check; improve debug log --- src/app-layer-ftp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c index c22c39c134b2..3db448279073 100644 --- a/src/app-layer-ftp.c +++ b/src/app-layer-ftp.c @@ -1067,8 +1067,7 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, } else { if (ftpdata_state->state == FTPDATA_STATE_FINISHED) { SCLogDebug("state is already finished"); - // TODO put back the assert after deciding on the bug... - // DEBUG_VALIDATE_BUG_ON(input_len); // data after state finished is a bug. + DEBUG_VALIDATE_BUG_ON(input_len); // data after state finished is a bug. SCReturnStruct(APP_LAYER_OK); } if ((direction & ftpdata_state->direction) == 0) { @@ -1099,7 +1098,7 @@ static AppLayerResult FTPDataParse(Flow *f, FtpDataState *ftpdata_state, if (eof) { ret = FileCloseFile(ftpdata_state->files, &sbcfg, NULL, 0, flags); ftpdata_state->state = FTPDATA_STATE_FINISHED; - SCLogDebug("closed because of eof"); + SCLogDebug("closed because of eof: state now FTPDATA_STATE_FINISHED"); } out: if (ret < 0) { From e4834ee7501950502b08c113e5a3f9aa0d249f3a Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 1 Aug 2023 07:48:04 +0200 Subject: [PATCH 4/5] stream: add stream.rst_with_data event for RST with data --- src/decode-events.c | 4 ++++ src/decode-events.h | 1 + src/stream-tcp.c | 15 +++++++++++++++ 3 files changed, 20 insertions(+) diff --git a/src/decode-events.c b/src/decode-events.c index 207de286f6ef..83f8b22efd93 100644 --- a/src/decode-events.c +++ b/src/decode-events.c @@ -814,6 +814,10 @@ const struct DecodeEvents_ DEvents[] = { "stream.rst_invalid_ack", STREAM_RST_INVALID_ACK, }, + { + "stream.rst_with_data", + STREAM_RST_WITH_DATA, + }, { "stream.pkt_retransmission", STREAM_PKT_RETRANSMISSION, diff --git a/src/decode-events.h b/src/decode-events.h index 451482403cb6..5547866fa59b 100644 --- a/src/decode-events.h +++ b/src/decode-events.h @@ -280,6 +280,7 @@ enum { STREAM_PKT_INVALID_ACK, STREAM_PKT_BROKEN_ACK, STREAM_RST_INVALID_ACK, + STREAM_RST_WITH_DATA, STREAM_PKT_RETRANSMISSION, STREAM_PKT_SPURIOUS_RETRANSMISSION, STREAM_PKT_BAD_WINDOW_UPDATE, diff --git a/src/stream-tcp.c b/src/stream-tcp.c index 99dcd299530c..d76a0593a0d2 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -5865,6 +5865,21 @@ static int StreamTcpValidateRst(TcpSession *ssn, Packet *p) } } + /* RST with data, it's complicated: + + 4.2.2.12 RST Segment: RFC-793 Section 3.4 + + A TCP SHOULD allow a received RST segment to include data. + + DISCUSSION + It has been suggested that a RST segment could contain + ASCII text that encoded and explained the cause of the + RST. No standard has yet been established for such + data. + */ + if (p->payload_len) + StreamTcpSetEvent(p, STREAM_RST_WITH_DATA); + /* Set up the os_policy to be used in validating the RST packets based on target system */ if (PKT_IS_TOSERVER(p)) { From ee739c5197264ec54c3dee27edeb055f18e19b79 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 1 Aug 2023 08:44:53 +0200 Subject: [PATCH 5/5] stream: special handling for RST data Data on RST packets is not invalid, but also shouldn't be used in reassembly. RFC 1122: 4.2.2.12 RST Segment: RFC-793 Section 3.4 A TCP SHOULD allow a received RST segment to include data. DISCUSSION It has been suggested that a RST segment could contain ASCII text that encoded and explained the cause of the RST. No standard has yet been established for such data. RST data will be presented to the detection engine per packet, but will not be part of stream reassembly. Bug: #6244. --- src/stream-tcp-reassemble.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/stream-tcp-reassemble.c b/src/stream-tcp-reassemble.c index 90bac3c64949..135b22485c04 100644 --- a/src/stream-tcp-reassemble.c +++ b/src/stream-tcp-reassemble.c @@ -2009,7 +2009,8 @@ int StreamTcpReassembleHandleSegment(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ } } /* if this segment contains data, insert it */ - if (p->payload_len > 0 && !(stream->flags & STREAMTCP_STREAM_FLAG_NOREASSEMBLY)) { + if (p->payload_len > 0 && !(stream->flags & STREAMTCP_STREAM_FLAG_NOREASSEMBLY) && + (p->tcph->th_flags & TH_RST) == 0) { SCLogDebug("calling StreamTcpReassembleHandleSegmentHandleData"); if (StreamTcpReassembleHandleSegmentHandleData(tv, ra_ctx, ssn, stream, p) != 0) { @@ -2024,10 +2025,9 @@ int StreamTcpReassembleHandleSegment(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ p->flags |= PKT_STREAM_ADD; } else { SCLogDebug("ssn %p / stream %p: not calling StreamTcpReassembleHandleSegmentHandleData:" - " p->payload_len %u, STREAMTCP_STREAM_FLAG_NOREASSEMBLY %s", + " p->payload_len %u, STREAMTCP_STREAM_FLAG_NOREASSEMBLY %s", ssn, stream, p->payload_len, (stream->flags & STREAMTCP_STREAM_FLAG_NOREASSEMBLY) ? "true" : "false"); - } /* if the STREAMTCP_STREAM_FLAG_DEPTH_REACHED is set, but not the