From 544ac300a990aa65db4cf89504024315a2a80566 Mon Sep 17 00:00:00 2001
From: Shivani Bhardwaj <shivani@oisf.net>
Date: Tue, 6 Jun 2023 16:13:50 +0530
Subject: [PATCH 1/9] ftp: separate truncated line markers

So far, we store one variable in state to hold whether we want to
discard a long line till LF irrespective of direction. This means that a
long command to the client followed by a regular command w LF can be
considered as one long line which is incorrect.

Bug 6055
---
 src/app-layer-ftp.c | 27 +++++++++++++++------------
 src/app-layer-ftp.h |  3 ++-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c
index caee50ba7497..33b46a906f6b 100644
--- a/src/app-layer-ftp.c
+++ b/src/app-layer-ftp.c
@@ -353,14 +353,15 @@ static void FTPTransactionFree(FTPTransaction *tx)
     FTPFree(tx, sizeof(*tx));
 }
 
-static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state)
+static int FTPGetLineForDirection(
+        FtpState *state, FtpLineState *line_state, bool *current_line_truncated)
 {
     void *ptmp;
     if (line_state->current_line_lf_seen == 1) {
         /* we have seen the lf for the previous line.  Clear the parser
          * details to parse new line */
         line_state->current_line_lf_seen = 0;
-        state->current_line_truncated = false;
+        *current_line_truncated = false;
         if (line_state->current_line_db == 1) {
             line_state->current_line_db = 0;
             FTPFree(line_state->db, line_state->db_len);
@@ -387,7 +388,7 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state)
             int32_t input_len = state->input_len;
             if ((uint32_t)input_len > ftp_max_line_len) {
                 input_len = ftp_max_line_len;
-                state->current_line_truncated = true;
+                *current_line_truncated = true;
             }
             line_state->db = FTPMalloc(input_len);
             if (line_state->db == NULL) {
@@ -396,12 +397,12 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state)
             line_state->current_line_db = 1;
             memcpy(line_state->db, state->input, input_len);
             line_state->db_len = input_len;
-        } else if (!state->current_line_truncated) {
+        } else if (!*current_line_truncated) {
             int32_t input_len = state->input_len;
             if (line_state->db_len + input_len > ftp_max_line_len) {
                 input_len = ftp_max_line_len - line_state->db_len;
                 DEBUG_VALIDATE_BUG_ON(input_len < 0);
-                state->current_line_truncated = true;
+                *current_line_truncated = true;
             }
             if (input_len > 0) {
                 ptmp = FTPRealloc(
@@ -427,12 +428,12 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state)
         line_state->current_line_lf_seen = 1;
 
         if (line_state->current_line_db == 1) {
-            if (!state->current_line_truncated) {
+            if (!*current_line_truncated) {
                 int32_t input_len = lf_idx + 1 - state->input;
                 if (line_state->db_len + input_len > ftp_max_line_len) {
                     input_len = ftp_max_line_len - line_state->db_len;
                     DEBUG_VALIDATE_BUG_ON(input_len < 0);
-                    state->current_line_truncated = true;
+                    *current_line_truncated = true;
                 }
                 if (input_len > 0) {
                     ptmp = FTPRealloc(
@@ -465,7 +466,7 @@ static int FTPGetLineForDirection(FtpState *state, FtpLineState *line_state)
             state->current_line = state->input;
             if (lf_idx - state->input > ftp_max_line_len) {
                 state->current_line_len = ftp_max_line_len;
-                state->current_line_truncated = true;
+                *current_line_truncated = true;
             } else {
                 state->current_line_len = lf_idx - state->input;
             }
@@ -497,9 +498,11 @@ static int FTPGetLine(FtpState *state)
 
     /* toserver */
     if (state->direction == 0)
-        return FTPGetLineForDirection(state, &state->line_state[0]);
+        return FTPGetLineForDirection(
+                state, &state->line_state[0], &state->current_line_truncated_ts);
     else
-        return FTPGetLineForDirection(state, &state->line_state[1]);
+        return FTPGetLineForDirection(
+                state, &state->line_state[1], &state->current_line_truncated_tc);
 }
 
 /**
@@ -631,7 +634,7 @@ static AppLayerResult FTPParseRequest(Flow *f, void *ftp_state,
         tx->command_descriptor = cmd_descriptor;
         tx->request_length = CopyCommandLine(&tx->request,
                 state->current_line, state->current_line_len);
-        tx->request_truncated = state->current_line_truncated;
+        tx->request_truncated = state->current_line_truncated_ts;
 
         /* change direction (default to server) so expectation will handle
          * the correct message when expectation will match.
@@ -856,7 +859,7 @@ static AppLayerResult FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserS
             FTPString *response = FTPStringAlloc();
             if (likely(response)) {
                 response->len = CopyCommandLine(&response->str, state->current_line, state->current_line_len);
-                response->truncated = state->current_line_truncated;
+                response->truncated = state->current_line_truncated_tc;
                 TAILQ_INSERT_TAIL(&tx->response_list, response, next);
             }
         }
diff --git a/src/app-layer-ftp.h b/src/app-layer-ftp.h
index af975bd14d50..716cee7e921d 100644
--- a/src/app-layer-ftp.h
+++ b/src/app-layer-ftp.h
@@ -177,7 +177,8 @@ typedef struct FtpState_ {
     /** length of the line in current_line.  Doesn't include the delimiter */
     uint32_t current_line_len;
     uint8_t current_line_delimiter_len;
-    bool current_line_truncated;
+    bool current_line_truncated_ts;
+    bool current_line_truncated_tc;
 
     /* 0 for toserver, 1 for toclient */
     FtpLineState line_state[2];

From 1b9e4fba061e3549d3db7ed6ae00d0a81d29b570 Mon Sep 17 00:00:00 2001
From: Shivani Bhardwaj <shivani@oisf.net>
Date: Tue, 6 Jun 2023 16:18:12 +0530
Subject: [PATCH 2/9] ftp: don't decrement truncated line len

In case LF was found for a long line way outside of the limit, we should
not need to update the delimiter len and current line len because the
line is capped at 4k and the LF was not within these 4k bytes.
---
 src/app-layer-ftp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c
index 33b46a906f6b..fee52a3e61bb 100644
--- a/src/app-layer-ftp.c
+++ b/src/app-layer-ftp.c
@@ -471,12 +471,13 @@ static int FTPGetLineForDirection(
                 state->current_line_len = lf_idx - state->input;
             }
 
-            if (state->input != lf_idx &&
-                *(lf_idx - 1) == 0x0D) {
-                state->current_line_len--;
-                state->current_line_delimiter_len = 2;
-            } else {
-                state->current_line_delimiter_len = 1;
+            if (!*current_line_truncated) {
+                if (state->input != lf_idx && *(lf_idx - 1) == 0x0D) {
+                    state->current_line_len--;
+                    state->current_line_delimiter_len = 2;
+                } else {
+                    state->current_line_delimiter_len = 1;
+                }
             }
         }
 

From 13dbb5d11aef9387fe1acd7413881a5585d2949a Mon Sep 17 00:00:00 2001
From: Jason Ish <jason.ish@oisf.net>
Date: Wed, 7 Jun 2023 09:22:32 -0600
Subject: [PATCH 3/9] windows: add -lntdll to Windows builds

Rust 1.70 has introduced some possible issues between LLVM and gcc
causing link errors that are fixed by explicitly adding -lntdll.

Thanks to https://github.com/extendr/rextendr/pull/285 for the fix.
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 6920047abb6f..83acfbc8f927 100644
--- a/configure.ac
+++ b/configure.ac
@@ -268,7 +268,7 @@
             CFLAGS="${CFLAGS} -DOS_WIN32"
             WINDOWS_PATH="yes"
             AC_DEFINE([HAVE_NON_POSIX_MKDIR], [1], [mkdir is not POSIX compliant: single arg])
-            RUST_LDADD=" -lws2_32 -liphlpapi -lwbemuuid -lOle32 -lOleAut32 -lUuid -luserenv -lshell32 -ladvapi32 -lgcc_eh -lbcrypt"
+            RUST_LDADD=" -lws2_32 -liphlpapi -lwbemuuid -lOle32 -lOleAut32 -lUuid -luserenv -lshell32 -ladvapi32 -lgcc_eh -lbcrypt -lntdll"
             TRY_WPCAP="yes"
             ;;
         *-*-cygwin)

From e275a1e28e759619ba2003ba2946b463dd9d2fd5 Mon Sep 17 00:00:00 2001
From: Victor Julien <vjulien@oisf.net>
Date: Tue, 25 Apr 2023 10:22:11 +0200
Subject: [PATCH 4/9] stream: update no-flow checks

(cherry picked from commit 0360cb654293c333e3be70204705fa7ec328512e)
---
 src/stream-tcp.c | 11 +++++------
 src/stream-tcp.h |  2 --
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index 2a921b31df1a..c17e42a07a87 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -5476,6 +5476,11 @@ int TcpSessionPacketSsnReuse(const Packet *p, const Flow *f, const void *tcp_ssn
 
 TmEcode StreamTcp (ThreadVars *tv, Packet *p, void *data, PacketQueueNoLock *pq)
 {
+    DEBUG_VALIDATE_BUG_ON(p->flow == NULL);
+    if (unlikely(p->flow == NULL)) {
+        return TM_ECODE_OK;
+    }
+
     StreamTcpThread *stt = (StreamTcpThread *)data;
 
     SCLogDebug("p->pcap_cnt %"PRIu64, p->pcap_cnt);
@@ -5487,11 +5492,6 @@ TmEcode StreamTcp (ThreadVars *tv, Packet *p, void *data, PacketQueueNoLock *pq)
         return TM_ECODE_OK;
     }
 
-    if (p->flow == NULL) {
-        StatsIncr(tv, stt->counter_tcp_no_flow);
-        return TM_ECODE_OK;
-    }
-
     HandleThreadId(tv, p, stt);
 
     /* only TCP packets with a flow from here */
@@ -5531,7 +5531,6 @@ TmEcode StreamTcpThreadInit(ThreadVars *tv, void *initdata, void **data)
     stt->counter_tcp_pseudo = StatsRegisterCounter("tcp.pseudo", tv);
     stt->counter_tcp_pseudo_failed = StatsRegisterCounter("tcp.pseudo_failed", tv);
     stt->counter_tcp_invalid_checksum = StatsRegisterCounter("tcp.invalid_checksum", tv);
-    stt->counter_tcp_no_flow = StatsRegisterCounter("tcp.no_flow", tv);
     stt->counter_tcp_syn = StatsRegisterCounter("tcp.syn", tv);
     stt->counter_tcp_synack = StatsRegisterCounter("tcp.synack", tv);
     stt->counter_tcp_rst = StatsRegisterCounter("tcp.rst", tv);
diff --git a/src/stream-tcp.h b/src/stream-tcp.h
index 48b32a4fb6e3..6bea83bb060b 100644
--- a/src/stream-tcp.h
+++ b/src/stream-tcp.h
@@ -91,8 +91,6 @@ typedef struct StreamTcpThread_ {
     uint16_t counter_tcp_pseudo_failed;
     /** packets rejected because their csum is invalid */
     uint16_t counter_tcp_invalid_checksum;
-    /** TCP packets with no associated flow */
-    uint16_t counter_tcp_no_flow;
     /** sessions reused */
     uint16_t counter_tcp_reused_ssn;
     /** syn pkts */

From b46d54178a11ae78c14251670b0d690dd4235345 Mon Sep 17 00:00:00 2001
From: Victor Julien <vjulien@oisf.net>
Date: Tue, 25 Apr 2023 10:09:27 +0200
Subject: [PATCH 5/9] counters: make tcp stats independent of flow, ssn

Counters depended on availability of flow and tcp session, meaning
that 2 memcaps could affect the counters.

Bug: #5017.
(cherry picked from commit 36f6e0515592812259fb327d529740a030dba98e)
---
 src/decode-tcp.c |  9 +++++++++
 src/decode.c     |  5 +++++
 src/decode.h     |  3 +++
 src/stream-tcp.c | 13 -------------
 src/stream-tcp.h |  6 ------
 5 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/src/decode-tcp.c b/src/decode-tcp.c
index 84d7595cffdd..fee390fbb4a9 100644
--- a/src/decode-tcp.c
+++ b/src/decode-tcp.c
@@ -259,6 +259,15 @@ int DecodeTCP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         return TM_ECODE_FAILED;
     }
 
+    /* update counters */
+    if ((p->tcph->th_flags & (TH_SYN | TH_ACK)) == (TH_SYN | TH_ACK)) {
+        StatsIncr(tv, dtv->counter_tcp_synack);
+    } else if (p->tcph->th_flags & (TH_SYN)) {
+        StatsIncr(tv, dtv->counter_tcp_syn);
+    }
+    if (p->tcph->th_flags & (TH_RST)) {
+        StatsIncr(tv, dtv->counter_tcp_rst);
+    }
 #ifdef DEBUG
     SCLogDebug("TCP sp: %" PRIu32 " -> dp: %" PRIu32 " - HLEN: %" PRIu32 " LEN: %" PRIu32 " %s%s%s%s%s%s",
         GET_TCP_SRC_PORT(p), GET_TCP_DST_PORT(p), TCP_GET_HLEN(p), len,
diff --git a/src/decode.c b/src/decode.c
index 243dce422c07..574f91451a0f 100644
--- a/src/decode.c
+++ b/src/decode.c
@@ -537,6 +537,11 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv)
     dtv->counter_null = StatsRegisterCounter("decoder.null", tv);
     dtv->counter_sll = StatsRegisterCounter("decoder.sll", tv);
     dtv->counter_tcp = StatsRegisterCounter("decoder.tcp", tv);
+
+    dtv->counter_tcp_syn = StatsRegisterCounter("tcp.syn", tv);
+    dtv->counter_tcp_synack = StatsRegisterCounter("tcp.synack", tv);
+    dtv->counter_tcp_rst = StatsRegisterCounter("tcp.rst", tv);
+
     dtv->counter_udp = StatsRegisterCounter("decoder.udp", tv);
     dtv->counter_sctp = StatsRegisterCounter("decoder.sctp", tv);
     dtv->counter_icmpv4 = StatsRegisterCounter("decoder.icmpv4", tv);
diff --git a/src/decode.h b/src/decode.h
index ded72485787b..3c160c220ae7 100644
--- a/src/decode.h
+++ b/src/decode.h
@@ -677,6 +677,9 @@ typedef struct DecodeThreadVars_
     uint16_t counter_ipv4;
     uint16_t counter_ipv6;
     uint16_t counter_tcp;
+    uint16_t counter_tcp_syn;
+    uint16_t counter_tcp_synack;
+    uint16_t counter_tcp_rst;
     uint16_t counter_udp;
     uint16_t counter_icmpv4;
     uint16_t counter_icmpv6;
diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index c17e42a07a87..dc6d8f1e20cb 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -5085,16 +5085,6 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,
         }
     }
 
-    /* update counters */
-    if ((p->tcph->th_flags & (TH_SYN|TH_ACK)) == (TH_SYN|TH_ACK)) {
-        StatsIncr(tv, stt->counter_tcp_synack);
-    } else if (p->tcph->th_flags & (TH_SYN)) {
-        StatsIncr(tv, stt->counter_tcp_syn);
-    }
-    if (p->tcph->th_flags & (TH_RST)) {
-        StatsIncr(tv, stt->counter_tcp_rst);
-    }
-
     /* broken TCP http://ask.wireshark.org/questions/3183/acknowledgment-number-broken-tcp-the-acknowledge-field-is-nonzero-while-the-ack-flag-is-not-set */
     if (!(p->tcph->th_flags & TH_ACK) && TCP_GET_ACK(p) != 0) {
         StreamTcpSetEvent(p, STREAM_PKT_BROKEN_ACK);
@@ -5531,9 +5521,6 @@ TmEcode StreamTcpThreadInit(ThreadVars *tv, void *initdata, void **data)
     stt->counter_tcp_pseudo = StatsRegisterCounter("tcp.pseudo", tv);
     stt->counter_tcp_pseudo_failed = StatsRegisterCounter("tcp.pseudo_failed", tv);
     stt->counter_tcp_invalid_checksum = StatsRegisterCounter("tcp.invalid_checksum", tv);
-    stt->counter_tcp_syn = StatsRegisterCounter("tcp.syn", tv);
-    stt->counter_tcp_synack = StatsRegisterCounter("tcp.synack", tv);
-    stt->counter_tcp_rst = StatsRegisterCounter("tcp.rst", tv);
     stt->counter_tcp_midstream_pickups = StatsRegisterCounter("tcp.midstream_pickups", tv);
     stt->counter_tcp_wrong_thread = StatsRegisterCounter("tcp.pkt_on_wrong_thread", tv);
 
diff --git a/src/stream-tcp.h b/src/stream-tcp.h
index 6bea83bb060b..d071d95ec391 100644
--- a/src/stream-tcp.h
+++ b/src/stream-tcp.h
@@ -93,12 +93,6 @@ typedef struct StreamTcpThread_ {
     uint16_t counter_tcp_invalid_checksum;
     /** sessions reused */
     uint16_t counter_tcp_reused_ssn;
-    /** syn pkts */
-    uint16_t counter_tcp_syn;
-    /** syn/ack pkts */
-    uint16_t counter_tcp_synack;
-    /** rst pkts */
-    uint16_t counter_tcp_rst;
     /** midstream pickups */
     uint16_t counter_tcp_midstream_pickups;
     /** wrong thread */

From 66b36f4de167ee9892ec83de35b9d63227199f99 Mon Sep 17 00:00:00 2001
From: Jason Ish <jason.ish@oisf.net>
Date: Wed, 21 Dec 2022 09:01:15 -0600
Subject: [PATCH 6/9] dns: mark test buffers with rustfmt::skip

(cherry picked from commit 39d2524bf6d57658b532c73ceb4def34ed9e2c8a)
---
 rust/src/dns/dns.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs
index ace6d0a39468..ee0098658861 100644
--- a/rust/src/dns/dns.rs
+++ b/rust/src/dns/dns.rs
@@ -1200,6 +1200,7 @@ mod tests {
     fn test_dns_parse_request_tcp_valid() {
         // A UDP DNS request with the DNS payload starting at byte 42.
         // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0x00, 0x15, 0x17, 0x0d, 0x06, 0xf7, 0xd8, 0xcb, /* ........ */
             0x8a, 0xed, 0xa1, 0x46, 0x08, 0x00, 0x45, 0x00, /* ...F..E. */
@@ -1235,6 +1236,7 @@ mod tests {
     fn test_dns_parse_request_tcp_short_payload() {
         // A UDP DNS request with the DNS payload starting at byte 42.
         // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0x00, 0x15, 0x17, 0x0d, 0x06, 0xf7, 0xd8, 0xcb, /* ........ */
             0x8a, 0xed, 0xa1, 0x46, 0x08, 0x00, 0x45, 0x00, /* ...F..E. */
@@ -1271,6 +1273,7 @@ mod tests {
     fn test_dns_parse_response_tcp_valid() {
         // A UDP DNS response with the DNS payload starting at byte 42.
         // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0xd8, 0xcb, 0x8a, 0xed, 0xa1, 0x46, 0x00, 0x15, /* .....F.. */
             0x17, 0x0d, 0x06, 0xf7, 0x08, 0x00, 0x45, 0x00, /* ......E. */
@@ -1314,6 +1317,7 @@ mod tests {
     fn test_dns_parse_response_tcp_short_payload() {
         // A UDP DNS response with the DNS payload starting at byte 42.
         // From pcap: https://github.com/jasonish/suricata-verify/blob/7cc0e1bd0a5249b52e6e87d82d57c0b6aaf75fce/dns-udp-dig-a-www-suricata-ids-org/dig-a-www.suricata-ids.org.pcap
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0xd8, 0xcb, 0x8a, 0xed, 0xa1, 0x46, 0x00, 0x15, /* .....F.. */
             0x17, 0x0d, 0x06, 0xf7, 0x08, 0x00, 0x45, 0x00, /* ......E. */
@@ -1359,6 +1363,7 @@ mod tests {
          * TTL: 86400
          * serial 20130422 refresh 28800 retry 7200 exp 604800 min ttl 86400
          * ns, hostmaster */
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0x00, 0x3c, 0x85, 0x00, 0x00, 0x01, 0x00, 0x00,
             0x00, 0x01, 0x00, 0x00, 0x0b, 0x61, 0x62, 0x63,
@@ -1379,6 +1384,7 @@ mod tests {
     // Port of the C RustDNSUDPParserTest02 unit test.
     #[test]
     fn test_dns_udp_parser_test_02() {
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0x6D,0x08,0x84,0x80,0x00,0x01,0x00,0x08,0x00,0x00,0x00,0x01,0x03,0x57,0x57,0x57,
             0x04,0x54,0x54,0x54,0x54,0x03,0x56,0x56,0x56,0x03,0x63,0x6F,0x6D,0x02,0x79,0x79,
@@ -1398,6 +1404,7 @@ mod tests {
     // Port of the C RustDNSUDPParserTest03 unit test.
     #[test]
     fn test_dns_udp_parser_test_03() {
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0x6F,0xB4,0x84,0x80,0x00,0x01,0x00,0x02,0x00,0x02,0x00,0x03,0x03,0x57,0x57,0x77,
             0x0B,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x56,0x03,0x55,0x55,0x55,
@@ -1419,6 +1426,7 @@ mod tests {
     // Test the TXT records in an answer.
     #[test]
     fn test_dns_udp_parser_test_04() {
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0xc2,0x2f,0x81,0x80,0x00,0x01,0x00,0x01,0x00,0x01,0x00,0x01,0x0a,0x41,0x41,0x41,
             0x41,0x41,0x4f,0x31,0x6b,0x51,0x41,0x05,0x3d,0x61,0x75,0x74,0x68,0x03,0x73,0x72,
@@ -1442,6 +1450,7 @@ mod tests {
     // Test TXT records in answer with a bad length.
     #[test]
     fn test_dns_udp_parser_test_05() {
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0xc2,0x2f,0x81,0x80,0x00,0x01,0x00,0x01,0x00,0x01,0x00,0x01,0x0a,0x41,0x41,0x41,
             0x41,0x41,0x4f,0x31,0x6b,0x51,0x41,0x05,0x3d,0x61,0x75,0x74,0x68,0x03,0x73,0x72,
@@ -1463,6 +1472,7 @@ mod tests {
     // Port of the C RustDNSTCPParserTestMultiRecord unit test.
     #[test]
     fn test_dns_tcp_parser_multi_record() {
+        #[rustfmt::skip]
         let buf: &[u8] = &[
             0x00, 0x1e, 0x00, 0x00, 0x01, 0x00, 0x00, 0x01,
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x30,
@@ -1557,11 +1567,13 @@ mod tests {
     #[test]
     fn test_dns_tcp_parser_split_payload() {
         /* incomplete payload */
+        #[rustfmt::skip]
         let buf1: &[u8] = &[
             0x00, 0x1c, 0x10, 0x32, 0x01, 0x00, 0x00, 0x01,
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00
         ];
         /* complete payload plus the start of a new payload */
+        #[rustfmt::skip]
         let buf2: &[u8] = &[
             0x00, 0x1c, 0x10, 0x32, 0x01, 0x00, 0x00, 0x01,
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
@@ -1574,6 +1586,7 @@ mod tests {
         ];
 
         /* and the complete payload again with no trailing data. */
+        #[rustfmt::skip]
         let buf3: &[u8] = &[
             0x00, 0x1c, 0x10, 0x32, 0x01, 0x00, 0x00, 0x01,
             0x00, 0x00, 0x00, 0x00, 0x00, 0x00,

From dbaf63df5a8f32b946888658bcc91bab0695a9ab Mon Sep 17 00:00:00 2001
From: Jason Ish <jason.ish@oisf.net>
Date: Tue, 20 Dec 2022 19:17:38 -0600
Subject: [PATCH 7/9] dns: parse and alert on invalid opcodes

Accept DNS messages with an invalid opcode that are otherwise
valid. Such DNS message will create a parser event.

This is a change of behavior, previously an invalid opcode would cause
the DNS message to not be detected or parsed as DNS.

Issue: #5444
(cherry picked from commit c98c49d4bad413dbbe4e21a48ebf37260ee5cc8e)
---
 rules/dns-events.rules |  1 +
 rust/src/dns/dns.rs    | 27 ++++++++++++++++++---------
 rust/src/dns/log.rs    |  8 ++++++--
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/rules/dns-events.rules b/rules/dns-events.rules
index 0e34dae139d0..d4c02b5c2f78 100644
--- a/rules/dns-events.rules
+++ b/rules/dns-events.rules
@@ -7,3 +7,4 @@ alert dns any any -> any any (msg:"SURICATA DNS Not a request"; flow:to_server;
 alert dns any any -> any any (msg:"SURICATA DNS Not a response"; flow:to_client; app-layer-event:dns.not_a_response; classtype:protocol-command-decode; sid:2240005; rev:2;)
 # Z flag (reserved) not 0
 alert dns any any -> any any (msg:"SURICATA DNS Z flag set"; app-layer-event:dns.z_flag_set; classtype:protocol-command-decode; sid:2240006; rev:2;)
+alert dns any any -> any any (msg:"SURICATA DNS Invalid opcode"; app-layer-event:dns.invalid_opcode; classtype:protocol-command-decode; sid:2240007; rev:1;)
diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs
index ee0098658861..9403a26620dd 100644
--- a/rust/src/dns/dns.rs
+++ b/rust/src/dns/dns.rs
@@ -133,10 +133,11 @@ static mut ALPROTO_DNS: AppProto = ALPROTO_UNKNOWN;
 
 #[repr(u32)]
 pub enum DNSEvent {
-    MalformedData = 0,
-    NotRequest = 1,
-    NotResponse = 2,
-    ZFlagSet = 3,
+    MalformedData,
+    NotRequest,
+    NotResponse,
+    ZFlagSet,
+    InvalidOpcode,
 }
 
 impl DNSEvent {
@@ -146,6 +147,7 @@ impl DNSEvent {
             DNSEvent::NotRequest => "NOT_A_REQUEST\0",
             DNSEvent::NotResponse => "NOT_A_RESPONSE\0",
             DNSEvent::ZFlagSet => "Z_FLAG_SET\0",
+            DNSEvent::InvalidOpcode => "INVALID_OPCODE\0",
         }
     }
 
@@ -155,6 +157,7 @@ impl DNSEvent {
             1 => Some(DNSEvent::NotRequest),
             2 => Some(DNSEvent::NotResponse),
             4 => Some(DNSEvent::ZFlagSet),
+            5 => Some(DNSEvent::InvalidOpcode),
             _ => None,
         }
     }
@@ -165,6 +168,7 @@ impl DNSEvent {
             "not_a_request" => Some(DNSEvent::NotRequest),
             "not_a_response" => Some(DNSEvent::NotRequest),
             "z_flag_set" => Some(DNSEvent::ZFlagSet),
+            "invalid_opcode" => Some(DNSEvent::InvalidOpcode),
             _ => None
         }
     }
@@ -506,6 +510,7 @@ impl DNSState {
                 }
 
                 let z_flag = request.header.flags & 0x0040 != 0;
+                let opcode = ((request.header.flags >> 11) & 0xf) as u8;
 
                 let mut tx = self.new_tx();
                 tx.request = Some(request);
@@ -517,6 +522,10 @@ impl DNSState {
                     self.set_event(DNSEvent::ZFlagSet);
                 }
 
+                if opcode >= 7 {
+                    self.set_event(DNSEvent::InvalidOpcode);
+                }
+
                 return true;
             }
             Err(nom::Err::Incomplete(_)) => {
@@ -546,6 +555,7 @@ impl DNSState {
                 }
 
                 let z_flag = response.header.flags & 0x0040 != 0;
+                let opcode = ((response.header.flags >> 11) & 0xf) as u8;
 
                 let mut tx = self.new_tx();
                 if let Some(ref mut config) = &mut self.config {
@@ -562,6 +572,10 @@ impl DNSState {
                     self.set_event(DNSEvent::ZFlagSet);
                 }
 
+                if opcode >= 7 {
+                    self.set_event(DNSEvent::InvalidOpcode);
+                }
+
                 return true;
             }
             Err(nom::Err::Incomplete(_)) => {
@@ -692,11 +706,6 @@ impl DNSState {
 const DNS_HEADER_SIZE: usize = 12;
 
 fn probe_header_validity(header: DNSHeader, rlen: usize) -> (bool, bool, bool) {
-    let opcode = ((header.flags >> 11) & 0xf) as u8;
-    if opcode >= 7 {
-        //unassigned opcode
-        return (false, false, false);
-    }
     if 2 * (header.additional_rr as usize
         + header.answer_rr as usize
         + header.authority_rr as usize
diff --git a/rust/src/dns/log.rs b/rust/src/dns/log.rs
index bba983873ed5..27abab245afc 100644
--- a/rust/src/dns/log.rs
+++ b/rust/src/dns/log.rs
@@ -488,10 +488,12 @@ fn dns_log_json_answer(js: &mut JsonBuilder, response: &DNSResponse, flags: u64)
         js.set_bool("z", true)?;
     }
 
-    for query in &response.queries {
+    let opcode = ((header.flags >> 11) & 0xf) as u8;
+    js.set_uint("opcode", opcode as u64)?;
+
+    if let Some(query) = response.queries.first() {
         js.set_string_from_bytes("rrname", &query.name)?;
         js.set_string("rrtype", &dns_rrtype_string(query.rrtype))?;
-        break;
     }
     js.set_string("rcode", &dns_rcode_string(header.flags))?;
 
@@ -602,6 +604,8 @@ fn dns_log_query(tx: &mut DNSTransaction,
                 if request.header.flags & 0x0040 != 0 {
                     jb.set_bool("z", true)?;
                 }
+                let opcode = ((request.header.flags >> 11) & 0xf) as u8;
+                jb.set_uint("opcode", opcode as u64)?;
                 return Ok(true);
             }
         }

From 0b283ef4a6943229cafb7b1b93a36c3d54f94ced Mon Sep 17 00:00:00 2001
From: Jason Ish <jason.ish@oisf.net>
Date: Tue, 20 Dec 2022 19:30:29 -0600
Subject: [PATCH 8/9] dns: validate header on every incoming message

As UDP streams getting probed, a stream that does not appear to be DNS
at first, may have a single packet that does look close enough to DNS
to be picked up as DNS causing every subsequent packet to result in a
parser error.

To mitigate this, probe every incoming DNS message header for validity
before continuing onto the body.  If the header doesn't validate as
DNS, just ignore the packet so no parse error is registered.

(cherry picked from commit 595700ab7e9dc9d12d46cf4d6833a86840decdf9)
---
 rust/src/dns/dns.rs | 77 ++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 29 deletions(-)

diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs
index 9403a26620dd..f2ffc6d704a7 100644
--- a/rust/src/dns/dns.rs
+++ b/rust/src/dns/dns.rs
@@ -500,7 +500,17 @@ impl DNSState {
                                                         event as u8);
     }
 
-    pub fn parse_request(&mut self, input: &[u8]) -> bool {
+    fn validate_header(&self, input: &[u8]) -> bool {
+        parser::dns_parse_header(input)
+            .map(|(_, header)| probe_header_validity(header, input.len()).0)
+            .unwrap_or(false)
+    }
+
+    fn parse_request(&mut self, input: &[u8], is_tcp: bool) -> bool {
+        if !self.validate_header(input) {
+            return !is_tcp;
+        }
+
         match parser::dns_parse_request(input) {
             Ok((_, request)) => {
                 if request.header.flags & 0x8000 != 0 {
@@ -543,7 +553,19 @@ impl DNSState {
         }
     }
 
-    pub fn parse_response(&mut self, input: &[u8]) -> bool {
+    fn parse_request_udp(&mut self, input: &[u8]) -> bool {
+        self.parse_request(input, false)
+    }
+
+    fn parse_response_udp(&mut self, input: &[u8]) -> bool {
+        self.parse_response(input, false)
+    }
+
+    pub fn parse_response(&mut self, input: &[u8], is_tcp: bool) -> bool {
+        if !self.validate_header(input) {
+            return !is_tcp;
+        }
+
         match parser::dns_parse_response(input) {
             Ok((_, response)) => {
 
@@ -620,8 +642,8 @@ impl DNSState {
             SCLogDebug!("[request] Have {} bytes, need {} to parse",
                         cur_i.len(), size + 2);
             if size > 0 && cur_i.len() >= size + 2 {
-                let msg = &cur_i[0..(size + 2)];
-                if self.parse_request(&msg[2..]) {
+                let msg = &cur_i[2..(size + 2)];
+                if self.parse_request(msg, true) {
                     cur_i = &cur_i[(size + 2)..];
                     consumed += size  + 2;
                 } else {
@@ -667,8 +689,8 @@ impl DNSState {
             SCLogDebug!("[response] Have {} bytes, need {} to parse",
                         cur_i.len(), size + 2);
             if size > 0 && cur_i.len() >= size + 2 {
-                let msg = &cur_i[0..(size + 2)];
-                if self.parse_response(&msg[2..]) {
+                let msg = &cur_i[2..(size + 2)];
+                if self.parse_response(msg, true) {
                     cur_i = &cur_i[(size + 2)..];
                     consumed += size + 2;
                 } else {
@@ -706,16 +728,19 @@ impl DNSState {
 const DNS_HEADER_SIZE: usize = 12;
 
 fn probe_header_validity(header: DNSHeader, rlen: usize) -> (bool, bool, bool) {
-    if 2 * (header.additional_rr as usize
-        + header.answer_rr as usize
-        + header.authority_rr as usize
-        + header.questions as usize)
-        + DNS_HEADER_SIZE
-        > rlen
-    {
-        //not enough data for such a DNS record
+    let min_msg_size = 2
+        * (header.additional_rr as usize
+            + header.answer_rr as usize
+            + header.authority_rr as usize
+            + header.questions as usize)
+        + DNS_HEADER_SIZE;
+
+    if min_msg_size > rlen {
+        // Not enough data for records defined in the header, or
+        // impossibly large.
         return (false, false, false);
     }
+
     let is_request = header.flags & 0x8000 == 0;
     return (true, is_request, false);
 }
@@ -812,11 +837,8 @@ pub extern "C" fn rs_dns_parse_request(_flow: *const core::Flow,
                                        -> AppLayerResult {
     let state = cast_pointer!(state, DNSState);
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
-    if state.parse_request(buf) {
-        AppLayerResult::ok()
-    } else {
-        AppLayerResult::err()
-    }
+    state.parse_request_udp(buf);
+    AppLayerResult::ok()
 }
 
 #[no_mangle]
@@ -830,11 +852,8 @@ pub extern "C" fn rs_dns_parse_response(_flow: *const core::Flow,
                                         -> AppLayerResult {
     let state = cast_pointer!(state, DNSState);
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
-    if state.parse_response(buf) {
-        AppLayerResult::ok()
-    } else {
-        AppLayerResult::err()
-    }
+    state.parse_response_udp(buf);
+    AppLayerResult::ok()
 }
 
 /// C binding parse a DNS request. Returns 1 on success, -1 on failure.
@@ -1387,7 +1406,7 @@ mod tests {
             0x80,
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest02 unit test.
@@ -1407,7 +1426,7 @@ mod tests {
             0x10,0x00,0x02,0xC0,0x85,0x00,0x00,0x29,0x05,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest03 unit test.
@@ -1427,7 +1446,7 @@ mod tests {
             0x29,0x05,0x00,0x00,0x00,0x00,0x00,0x00,0x00
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest04 unit test.
@@ -1451,7 +1470,7 @@ mod tests {
             0x6b,0x00,0x01,0x00,0x01,0x00,0x09,0x3a,0x80,0x00,0x04,0x0a,0x1e,0x1c,0x5f
         ];
         let mut state = DNSState::new();
-        assert!(state.parse_response(buf));
+        assert!(state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSUDPParserTest05 unit test.
@@ -1475,7 +1494,7 @@ mod tests {
             0x6b,0x00,0x01,0x00,0x01,0x00,0x09,0x3a,0x80,0x00,0x04,0x0a,0x1e,0x1c,0x5f
         ];
         let mut state = DNSState::new();
-        assert!(!state.parse_response(buf));
+        assert!(!state.parse_response(buf, false));
     }
 
     // Port of the C RustDNSTCPParserTestMultiRecord unit test.

From 2e4aade51df2d06e1c6105c2a5033acb08dbce34 Mon Sep 17 00:00:00 2001
From: Jason Ish <jason.ish@oisf.net>
Date: Wed, 21 Dec 2022 09:35:19 -0600
Subject: [PATCH 9/9] dns: split header and body parsing

As part of extra header validation, split out DNS body parsing to
avoid the overhead of parsing the header twice.

(cherry picked from commit d720ead470bcb5dd5a0c0ae7db302ab170205ee6)
---
 rust/src/dns/dns.rs    | 51 ++++++++++++++++-------------
 rust/src/dns/parser.rs | 73 +++++++++++++++++++++---------------------
 2 files changed, 64 insertions(+), 60 deletions(-)

diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs
index f2ffc6d704a7..460c97def1e1 100644
--- a/rust/src/dns/dns.rs
+++ b/rust/src/dns/dns.rs
@@ -29,7 +29,7 @@ use crate::core::STREAM_TOSERVER;
 use crate::core::{self, AppProto, ALPROTO_UNKNOWN, IPPROTO_UDP, IPPROTO_TCP};
 use crate::dns::parser;
 
-use nom::IResult;
+use nom::{Err, IResult};
 use nom::number::streaming::be_u16;
 
 /// DNS record types.
@@ -500,18 +500,23 @@ impl DNSState {
                                                         event as u8);
     }
 
-    fn validate_header(&self, input: &[u8]) -> bool {
-        parser::dns_parse_header(input)
-            .map(|(_, header)| probe_header_validity(header, input.len()).0)
-            .unwrap_or(false)
+    fn validate_header<'a>(&self, input: &'a [u8]) -> Option<(&'a [u8], DNSHeader)> {
+        if let Ok((body, header)) = parser::dns_parse_header(input) {
+            if probe_header_validity(&header, input.len()).0 {
+                return Some((body, header));
+            }
+        }
+        None
     }
 
     fn parse_request(&mut self, input: &[u8], is_tcp: bool) -> bool {
-        if !self.validate_header(input) {
+        let (body, header) = if let Some((body, header)) = self.validate_header(input) {
+            (body, header)
+        } else {
             return !is_tcp;
-        }
+        };
 
-        match parser::dns_parse_request(input) {
+        match parser::dns_parse_request_body(body, input, header) {
             Ok((_, request)) => {
                 if request.header.flags & 0x8000 != 0 {
                     SCLogDebug!("DNS message is not a request");
@@ -562,11 +567,13 @@ impl DNSState {
     }
 
     pub fn parse_response(&mut self, input: &[u8], is_tcp: bool) -> bool {
-        if !self.validate_header(input) {
+        let (body, header) = if let Some((body, header)) = self.validate_header(input) {
+            (body, header)
+        } else {
             return !is_tcp;
-        }
+        };
 
-        match parser::dns_parse_response(input) {
+        match parser::dns_parse_response_body(body, input, header) {
             Ok((_, response)) => {
 
                 SCLogDebug!("Response header flags: {}", response.header.flags);
@@ -727,7 +734,7 @@ impl DNSState {
 
 const DNS_HEADER_SIZE: usize = 12;
 
-fn probe_header_validity(header: DNSHeader, rlen: usize) -> (bool, bool, bool) {
+fn probe_header_validity(header: &DNSHeader, rlen: usize) -> (bool, bool, bool) {
     let min_msg_size = 2
         * (header.additional_rr as usize
             + header.answer_rr as usize
@@ -756,7 +763,7 @@ fn probe(input: &[u8], dlen: usize) -> (bool, bool, bool) {
     // parse a complete message, so perform header validation only.
     if input.len() < dlen {
         if let Ok((_, header)) = parser::dns_parse_header(input) {
-            return probe_header_validity(header, dlen);
+            return probe_header_validity(&header, dlen);
         } else {
             return (false, false, false);
         }
@@ -764,17 +771,15 @@ fn probe(input: &[u8], dlen: usize) -> (bool, bool, bool) {
 
     match parser::dns_parse_request(input) {
         Ok((_, request)) => {
-            return probe_header_validity(request.header, dlen);
-        },
-        Err(nom::Err::Incomplete(_)) => {
-            match parser::dns_parse_header(input) {
-                Ok((_, header)) => {
-                    return probe_header_validity(header, dlen);
-                }
-                Err(nom::Err::Incomplete(_)) => (false, false, true),
-                Err(_) => (false, false, false),
-            }
+            return probe_header_validity(&request.header, dlen);
         }
+        Err(Err::Incomplete(_)) => match parser::dns_parse_header(input) {
+            Ok((_, header)) => {
+                return probe_header_validity(&header, dlen);
+            }
+            Err(Err::Incomplete(_)) => (false, false, true),
+            Err(_) => (false, false, false),
+        },
         Err(_) => (false, false, false),
     }
 }
diff --git a/rust/src/dns/parser.rs b/rust/src/dns/parser.rs
index d25e0ad23310..dd16c8f49b55 100644
--- a/rust/src/dns/parser.rs
+++ b/rust/src/dns/parser.rs
@@ -20,6 +20,7 @@
 use nom::IResult;
 use nom::combinator::rest;
 use nom::error::ErrorKind;
+use nom::multi::count;
 use nom::number::streaming::{be_u8, be_u16, be_u32};
 use nom;
 use crate::dns::dns::*;
@@ -50,10 +51,8 @@ named!(pub dns_parse_header<DNSHeader>,
 ///
 /// Parameters:
 ///   start: the start of the name
-///   message: the complete message that start is a part of
-pub fn dns_parse_name<'a, 'b>(start: &'b [u8],
-                              message: &'b [u8])
-                              -> IResult<&'b [u8], Vec<u8>> {
+///   message: the complete message that start is a part of with the DNS header
+pub fn dns_parse_name<'b>(start: &'b [u8], message: &'b [u8]) -> IResult<&'b [u8], Vec<u8>> {
     let mut pos = start;
     let mut pivot = start;
     let mut name: Vec<u8> = Vec::with_capacity(32);
@@ -197,26 +196,27 @@ fn dns_parse_answer<'a>(slice: &'a [u8], message: &'a [u8], count: usize)
 
 
 /// Parse a DNS response.
-pub fn dns_parse_response<'a>(slice: &'a [u8])
-                              -> IResult<&[u8], DNSResponse> {
-    do_parse!(
-        slice,
-        header: dns_parse_header
-            >> queries: count!(
-                call!(dns_parse_query, slice), header.questions as usize)
-            >> answers: call!(
-                dns_parse_answer, slice, header.answer_rr as usize)
-            >> authorities: call!(
-                dns_parse_answer, slice, header.authority_rr as usize)
-            >> (
-                DNSResponse{
-                    header: header,
-                    queries: queries,
-                    answers: answers,
-                    authorities: authorities,
-                }
-            )
-    )
+pub fn dns_parse_response(message: &[u8]) -> IResult<&[u8], DNSResponse> {
+    let i = message;
+    let (i, header) = dns_parse_header(i)?;
+    dns_parse_response_body(i, message, header)
+}
+
+pub fn dns_parse_response_body<'a>(
+    i: &'a [u8], message: &'a [u8], header: DNSHeader,
+) -> IResult<&'a [u8], DNSResponse> {
+    let (i, queries) = count(|b| dns_parse_query(b, message), header.questions as usize)(i)?;
+    let (i, answers) = dns_parse_answer(i, message, header.answer_rr as usize)?;
+    let (i, authorities) = dns_parse_answer(i, message, header.authority_rr as usize)?;
+    Ok((
+        i,
+        DNSResponse {
+            header,
+            queries,
+            answers,
+            authorities,
+        },
+    ))
 }
 
 /// Parse a single DNS query.
@@ -344,19 +344,18 @@ pub fn dns_parse_rdata<'a>(input: &'a [u8], message: &'a [u8], rrtype: u16)
 }
 
 /// Parse a DNS request.
-pub fn dns_parse_request<'a>(input: &'a [u8]) -> IResult<&[u8], DNSRequest> {
-    do_parse!(
-        input,
-        header: dns_parse_header >>
-        queries: count!(call!(dns_parse_query, input),
-                        header.questions as usize) >>
-            (
-                DNSRequest{
-                    header: header,
-                    queries: queries,
-                }
-            )
-    )
+pub fn dns_parse_request(input: &[u8]) -> IResult<&[u8], DNSRequest> {
+    let i = input;
+    let (i, header) = dns_parse_header(i)?;
+    dns_parse_request_body(i, input, header)
+}
+
+pub fn dns_parse_request_body<'a>(
+    input: &'a [u8], message: &'a [u8], header: DNSHeader,
+) -> IResult<&'a [u8], DNSRequest> {
+    let i = input;
+    let (i, queries) = count(|b| dns_parse_query(b, message), header.questions as usize)(i)?;
+    Ok((i, DNSRequest { header, queries }))
 }
 
 #[cfg(test)]