From: Victor Julien <victor@inliniac.net>
Date: Tue, 16 Oct 2012 11:27:58 +0000 (+0200)
Subject: stream: ignore ack value if ack flag is not set. Add stream.pkt_broken_ack event... 
X-Git-Tag: suricata-1.4beta3~85
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9094eb47839e75ea8208cdb0d732c2d50937463c;p=thirdparty%2Fsuricata.git

stream: ignore ack value if ack flag is not set. Add stream.pkt_broken_ack event for when ack value is not 0 and ack flag not set.
---

diff --git a/rules/stream-events.rules b/rules/stream-events.rules
index 447885d8b4..3e57b6a935 100644
--- a/rules/stream-events.rules
+++ b/rules/stream-events.rules
@@ -50,6 +50,9 @@ alert tcp any any -> any any (msg:"SURICATA STREAM TIMEWAIT ACK with wrong seq";
 alert tcp any any -> any any (msg:"SURICATA STREAM TIMEWAIT invalid ack"; stream-event:timewait_invalid_ack; sid:2210043; rev:1;)
 alert tcp any any -> any any (msg:"SURICATA STREAM Packet with invalid timestamp"; stream-event:pkt_invalid_timestamp; sid:2210044; rev:1;)
 alert tcp any any -> any any (msg:"SURICATA STREAM Packet with invalid ack"; stream-event:pkt_invalid_ack; sid:2210045; rev:1;)
+# Broken TCP: ack field non 0, but ACK flag not set. http://ask.wireshark.org/questions/3183/acknowledgment-number-broken-tcp-the-acknowledge-field-is-nonzero-while-the-ack-flag-is-not-set
+# Often result of broken load balancers, firewalls and such.
+#alert tcp any any -> any any (msg:"SURICATA STREAM Packet with broken ack"; stream-event:pkt_broken_ack; sid:2210051; rev:1;)
 alert tcp any any -> any any (msg:"SURICATA STREAM SHUTDOWN RST invalid ack"; stream-event:rst_invalid_ack; sid:2210046; rev:1;)
 # SYN (re)send during shutdown (closing, closewait, finwait1, finwait2, lastack, timewait states)
 #alert tcp any any -> any any (msg:"SURICATA STREAM SYN resend"; stream-event:shutdown_syn_resend; sid:2210049; rev:1;)
@@ -57,5 +60,5 @@ alert tcp any any -> any any (msg:"SURICATA STREAM SHUTDOWN RST invalid ack"; st
 # Sequence gap: missing data in the reassembly engine. Usually due to packet loss. Will be very noisy on a overloaded link / sensor.
 #alert tcp any any -> any any (msg:"SURICATA STREAM reassembly sequence GAP -- missing packet(s)"; stream-event:reassembly_seq_gap; sid:2210048; rev:1;)
 alert tcp any any -> any any (msg:"SURICATA STREAM reassembly overlap with different data"; stream-event:reassembly_overlap_different_data; sid:2210050; rev:1;)
-# next sid 2210051
+# next sid 2210052
 
diff --git a/src/decode-events.h b/src/decode-events.h
index 2a1bb69f5c..3e08f2c2f6 100644
--- a/src/decode-events.h
+++ b/src/decode-events.h
@@ -176,6 +176,7 @@ enum {
     STREAM_SHUTDOWN_SYN_RESEND,
     STREAM_PKT_INVALID_TIMESTAMP,
     STREAM_PKT_INVALID_ACK,
+    STREAM_PKT_BROKEN_ACK,
     STREAM_RST_INVALID_ACK,
 
     STREAM_REASSEMBLY_SEGMENT_BEFORE_BASE_SEQ,
diff --git a/src/detect-engine-event.h b/src/detect-engine-event.h
index b16d213825..52653e3581 100644
--- a/src/detect-engine-event.h
+++ b/src/detect-engine-event.h
@@ -166,6 +166,7 @@ struct DetectEngineEvents_ {
     { "stream.timewait_invalid_ack", STREAM_TIMEWAIT_INVALID_ACK, },
     { "stream.pkt_invalid_timestamp", STREAM_PKT_INVALID_TIMESTAMP, },
     { "stream.pkt_invalid_ack", STREAM_PKT_INVALID_ACK, },
+    { "stream.pkt_broken_ack", STREAM_PKT_BROKEN_ACK, },
     { "stream.rst_invalid_ack", STREAM_RST_INVALID_ACK, },
     { "stream.shutdown_syn_resend", STREAM_SHUTDOWN_SYN_RESEND, },
     { "stream.reassembly_segment_before_base_seq", STREAM_REASSEMBLY_SEGMENT_BEFORE_BASE_SEQ, },
diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index 45e78eba3c..8b881a7394 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -3591,6 +3591,11 @@ static int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,
         SCPerfCounterIncr(stt->counter_tcp_rst, tv->sc_perf_pca);
     }
 
+    /* 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);
+    }
+
     /* If we are on IPS mode, and got a drop action triggered from
      * the IP only module, or from a reassembled msg and/or from an
      * applayer detection, then drop the rest of the packets of the
@@ -4012,7 +4017,8 @@ static int StreamTcpValidateRst(TcpSession *ssn, Packet *p)
 
         os_policy = ssn->server.os_policy;
 
-        if (TCP_GET_ACK(p) && StreamTcpValidateAck(ssn, &ssn->server, p) == -1) {
+        if (p->tcph->th_flags & TH_ACK &&
+                TCP_GET_ACK(p) && StreamTcpValidateAck(ssn, &ssn->server, p) == -1) {
             SCLogDebug("ssn %p: rejecting because of invalid ack value", ssn);
             StreamTcpSetEvent(p, STREAM_RST_INVALID_ACK);
             SCReturnInt(0);
@@ -4024,7 +4030,8 @@ static int StreamTcpValidateRst(TcpSession *ssn, Packet *p)
 
         os_policy = ssn->client.os_policy;
 
-        if (TCP_GET_ACK(p) && StreamTcpValidateAck(ssn, &ssn->client, p) == -1) {
+        if (p->tcph->th_flags & TH_ACK &&
+                TCP_GET_ACK(p) && StreamTcpValidateAck(ssn, &ssn->client, p) == -1) {
             SCLogDebug("ssn %p: rejecting because of invalid ack value", ssn);
             StreamTcpSetEvent(p, STREAM_RST_INVALID_ACK);
             SCReturnInt(0);