]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
stream: detect and filter out bad window updates 1094/head
authorVictor Julien <victor@inliniac.net>
Wed, 16 Jul 2014 22:23:50 +0000 (00:23 +0200)
committerVictor Julien <victor@inliniac.net>
Thu, 7 Aug 2014 13:31:35 +0000 (15:31 +0200)
Reported in bug 1238 is an issue where stream reassembly can be
disrupted.

A packet that was in-window, but otherwise unexpected set the
window to a really low value, causing the next *expected* packet
to be considered out of window. This lead to missing data in the
stream reassembly.

The packet was unexpected in various ways:
- it would ack unseen traffic
- it's sequence number would not match the expected next_seq
- set a really low window, while not being a proper window update

Detection however, it greatly hampered by the fact that in case of
packet loss, quite similar packets come in. Alerting in this case
is unwanted. Ignoring/skipping packets in this case as well.

The logic used in this patch is as follows. If:

- the packet is not a window update AND
- packet seq > next_seq AND
- packet acq > next_seq (packet acks unseen data) AND
- packet shrinks window more than it's own data size
THEN set event and skip the packet in the stream engine.

So in case of a segment with no data, any window shrinking is rejected.

Bug #1238.

rules/stream-events.rules
src/decode-events.h
src/detect-engine-event.h
src/stream-tcp.c

index 0c03b9243745d31500f3587169837f8bf0fdc0a4..fe4c6cb00e54ea4221b831dc5c4437a9ae57f40e 100644 (file)
@@ -60,6 +60,8 @@ 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; classtype:protocol-command-decode; sid:2210048; rev:2;)
 alert tcp any any -> any any (msg:"SURICATA STREAM reassembly overlap with different data"; stream-event:reassembly_overlap_different_data; classtype:protocol-command-decode; sid:2210050; rev:2;)
+# Bad Window Update: see bug 1238 for an explanation
+alert tcp any any -> any any (msg:"SURICATA STREAM bad window update"; stream-event:pkt_bad_window_update; classtype:protocol-command-decode; sid:2210056; rev:1;)
 
 # retransmission detection
 #
@@ -79,5 +81,5 @@ alert tcp any any -> any any (msg:"SURICATA STREAM Packet is retransmission"; st
 # rule to alert if a stream has excessive retransmissions
 alert tcp any any -> any any (msg:"SURICATA STREAM excessive retransmissions"; flowbits:isnotset,tcp.retransmission.alerted; flowint:tcp.retransmission.count,>=,10; flowbits:set,tcp.retransmission.alerted; classtype:protocol-command-decode; sid:2210054; rev:1;)
 
-# next sid 2210056
+# next sid 2210057
 
index ceb8c640138e1541eca084980dd25419f2d2da99..3080764621ef7a7aad65d13d06cdf3add0e52c73 100644 (file)
@@ -198,6 +198,7 @@ enum {
     STREAM_PKT_BROKEN_ACK,
     STREAM_RST_INVALID_ACK,
     STREAM_PKT_RETRANSMISSION,
+    STREAM_PKT_BAD_WINDOW_UPDATE,
 
     STREAM_REASSEMBLY_SEGMENT_BEFORE_BASE_SEQ,
     STREAM_REASSEMBLY_NO_SEGMENT,
index 5b42f6e7439961285f68a6c3d4cd9ae345f64ff5..fb6089b92ac13a629a943344b604e89d41b638bc 100644 (file)
@@ -211,6 +211,7 @@ struct DetectEngineEvents_ {
     { "stream.reassembly_no_segment", STREAM_REASSEMBLY_NO_SEGMENT, },
     { "stream.reassembly_seq_gap", STREAM_REASSEMBLY_SEQ_GAP, },
     { "stream.reassembly_overlap_different_data", STREAM_REASSEMBLY_OVERLAP_DIFFERENT_DATA, },
+    { "stream.pkt_bad_window_update", STREAM_PKT_BAD_WINDOW_UPDATE, },
 
     /* SCTP EVENTS */
     { "sctp.pkt_too_small", SCTP_PKT_TOO_SMALL, },
index 607377c1a13648a78561433b492e8dab790a87fb..d76496a926674104f7697c9f865cfb574245ff87 100644 (file)
@@ -4210,6 +4210,75 @@ static int StreamTcpPacketIsWindowUpdate(TcpSession *ssn, Packet *p)
     return 0;
 }
 
+/**
+ *  Try to detect packets doing bad window updates
+ *
+ *  See bug 1238.
+ *
+ *  Find packets that are unexpected, and shrink the window to the point
+ *  where the packets we do expect are rejected for being out of window.
+ *
+ *  The logic we use here is:
+ *  - packet seq > next_seq
+ *  - packet acq > next_seq (packet acks unseen data)
+ *  - packet shrinks window more than it's own data size
+ *    (in case of no data, any shrinking is rejected)
+ *
+ *  Packets coming in after packet loss can look quite a bit like this.
+ */
+static int StreamTcpPacketIsBadWindowUpdate(TcpSession *ssn, Packet *p)
+{
+    TcpStream *stream = NULL, *ostream = NULL;
+    uint32_t seq;
+    uint32_t ack;
+    uint32_t pkt_win;
+
+    if (p->flags & PKT_PSEUDO_STREAM_END)
+        return 0;
+
+    if (ssn->state < TCP_ESTABLISHED)
+        return 0;
+
+    if ((p->tcph->th_flags & (TH_SYN|TH_FIN|TH_RST)) != 0)
+        return 0;
+
+    if (PKT_IS_TOSERVER(p)) {
+        stream = &ssn->client;
+        ostream = &ssn->server;
+    } else {
+        stream = &ssn->server;
+        ostream = &ssn->client;
+    }
+
+    seq = TCP_GET_SEQ(p);
+    ack = TCP_GET_ACK(p);
+
+    pkt_win = TCP_GET_WINDOW(p) << ostream->wscale;
+
+    if (pkt_win < ostream->window) {
+        uint32_t diff = ostream->window - pkt_win;
+        if (diff > p->payload_len &&
+                SEQ_GT(ack, ostream->next_seq) &&
+                SEQ_GT(seq, stream->next_seq))
+        {
+            SCLogDebug("%"PRIu64", pkt_win %u, stream win %u, diff %u, dsize %u",
+                p->pcap_cnt, pkt_win, ostream->window, diff, p->payload_len);
+            SCLogDebug("%"PRIu64", pkt_win %u, stream win %u",
+                p->pcap_cnt, pkt_win, ostream->window);
+            SCLogDebug("%"PRIu64", seq %u ack %u ostream->next_seq %u stream->last_ack %u, diff %u (%u)",
+                p->pcap_cnt, seq, ack, ostream->next_seq, stream->last_ack,
+                ostream->next_seq - ostream->last_ack, stream->next_seq - stream->last_ack);
+
+            StreamTcpSetEvent(p, STREAM_PKT_BAD_WINDOW_UPDATE);
+            return 1;
+        }
+
+    }
+    SCLogDebug("seq %u (%u), ack %u (%u)", seq, stream->next_seq, ack, ostream->last_ack);
+    return 0;
+}
+
+
 /* flow is and stays locked */
 int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,
                      PacketQueue *pq)
@@ -4274,7 +4343,6 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,
         if (ssn->flags & STREAMTCP_FLAG_MIDSTREAM_SYNACK)
             StreamTcpPacketSwitchDir(ssn, p);
 
-        StreamTcpPacketIsWindowUpdate(ssn, p);
         if (StreamTcpPacketIsKeepAlive(ssn, p) == 1) {
             goto skip;
         }
@@ -4284,6 +4352,12 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,
         }
         StreamTcpClearKeepAliveFlag(ssn, p);
 
+        /* if packet is not a valid window update, check if it is perhaps
+         * a bad window update that we should ignore (and alert on) */
+        if (StreamTcpPacketIsWindowUpdate(ssn, p) == 0)
+            if (StreamTcpPacketIsBadWindowUpdate(ssn,p))
+                goto skip;
+
         switch (ssn->state) {
             case TCP_SYN_SENT:
                 if(StreamTcpPacketStateSynSent(tv, p, stt, ssn, &stt->pseudo_queue)) {