flow: handle TCP session reuse in flow engine

Until now, TCP session reuse was handled in the TCP stream engine.
If the state was TCP_CLOSED, a new SYN packet was received and a few
other conditions were met, the flow was 'reset' and reused for the
'new' TCP session.

There are a number of problems with this approach:
- it breaks the normal flow lifecycle wrt timeout, detection, logging
- new TCP sessions could come in on different threads due to mismatches
  in timeouts between suricata and flow balancing hw/nic/drivers
- cleanup code was often causing problems
- it complicated locking because of the possible thread mismatch

This patch implements a different solution, where a new TCP session also
gets a new flow. To do this 2 main things needed to be done:

1. the flow engine needed to be aware of when the TCP reuse case was
   happening
2. the flow engine needs to be able to 'skip' the old flow once it was
   replaced by a new one

To handle (1), a new function TcpSessionPacketSsnReuse() is introduced
to check for the TCP reuse conditions. It's called from 'FlowCompare()'
for TCP packets / TCP flows that are candidates for reuse. FlowCompare
returns FALSE for the 'old' flow in the case of TCP reuse.

This in turn will lead to the flow engine not finding a flow for the TCP
SYN packet, resulting in the creation of a new flow.

To handle (2), FlowCompare flags the 'old' flow. This flag causes future
FlowCompare calls to always return FALSE on it. In other words, the flow
can't be found anymore. It can only be accessed by:

1. existing packets with a reference to it
2. flow timeout handling as this logic gets the flows from walking the
   hash directly
3. flow timeout pseudo packets, as they are set up by (2)

The old flow will time out normally, as governed by the "tcp closed"
flow timeout setting. At timeout, the normal detection, logging and
cleanup code will process it.

The flagging of a flow making it 'unfindable' in the flow hash is a bit
of a hack. The reason for this approach over for example putting the
old flow into a forced timeout queue where it could be timed out, is
that such a queue could easily become a contention point. The TCP
session reuse case can easily be created by an attacker. In case of
multiple packet handlers, this could lead to contention on such a flow
timeout queue.

diff --git a/src/flow-hash.c b/src/flow-hash.c
index a2ce5708c96a375e44d17623e1d48d987853f092..1fea10a67d3963387c6435ffc6ece60f2c4d41ef 100644 (file)
--- a/src/flow-hash.c
+++ b/src/flow-hash.c
@@ -390,10 +390,37 @@ static inline int FlowCompareICMPv4(Flow *f, const Packet *p)
     return 0;
 }
 
+int TcpSessionPacketSsnReuse(const Packet *p, void *tcp_ssn);
+
 static inline int FlowCompare(Flow *f, const Packet *p)
 {
     if (p->proto == IPPROTO_ICMP) {
         return FlowCompareICMPv4(f, p);
+    } else if (p->proto == IPPROTO_TCP) {
+        if (CMP_FLOW(f, p) == 0)
+            return 0;
+
+        /* if this session is 'reused', we don't return it anymore,
+         * so return false on the compare */
+        if (f->flags & FLOW_TCP_REUSED)
+            return 0;
+
+        /* lets see if we need to consider the existing session for
+         * reuse: only considering SYN packets. */
+        int syn = (p->tcph && ((p->tcph->th_flags & TH_SYN) == TH_SYN));
+        int has_protoctx = ((f->protoctx != NULL));
+
+        /* syn on existing state, need to see if we need to 'reuse' */
+        if (unlikely(syn && has_protoctx && FlowGetPacketDirection(f,p) == TOSERVER)) {
+            if (unlikely(TcpSessionPacketSsnReuse(p, f->protoctx) == 1)) {
+                /* okay, we need to setup a new flow for this packet.
+                 * Flag the flow that it's been replaced by a new one */
+                f->flags |= FLOW_TCP_REUSED;
+                SCLogDebug("flow obsolete: TCP reuse will use a new flow");
+                return 0;
+            }
+        }
+        return 1;
     } else {
         return CMP_FLOW(f, p);
     }

diff --git a/src/flow.h b/src/flow.h
index bf38fa5543066235fe207854405ecb6fc277b257..0570adbc4d80ec7284b1189655846df87e33a147 100644 (file)
--- a/src/flow.h
+++ b/src/flow.h
@@ -46,9 +46,8 @@ typedef struct AppLayerParserState_ AppLayerParserState;
 #define FLOW_TO_SRC_SEEN                  0x00000001
 /** At least on packet from the destination address was seen */
 #define FLOW_TO_DST_SEEN                  0x00000002
-
-// vacany 1x
-
+/** Don't return this from the flow hash. It has been replaced. */
+#define FLOW_TCP_REUSED                   0x00000004
 /** no magic on files in this flow */
 #define FLOW_FILE_NO_MAGIC_TS             0x00000008
 #define FLOW_FILE_NO_MAGIC_TC             0x00000010

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index f0952719f2a797543afb3d8b649c571411aa9eae..f6d79eaf96c0155b1df1c4b2f64b41d17cd5a09c 100644 (file)
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -4386,6 +4386,17 @@ static int StreamTcpPacketIsBadWindowUpdate(TcpSession *ssn, Packet *p)
     return 0;
 }
 
+int TcpSessionPacketSsnReuse(const Packet *p, void *tcp_ssn)
+{
+    TcpSession *ssn = tcp_ssn;
+    if (ssn->state == TCP_CLOSED) {
+        if(!(SEQ_EQ(ssn->client.isn, TCP_GET_SEQ(p))))
+        {
+            return 1;
+        }
+    }
+    return 0;
+}
 
 /* flow is and stays locked */
 int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,