app-layer-expectation: clean expectation and add limits

When a flow timeout, we can have still existing expectations that
are linked to this flow. Given that there is a delay between the
real ending of the flow and its destruction by Suricata, the
expectation should be already honored so we can assume the risk
to clean the expectations that have been triggered by the
to-be-deleted flow.

This patch introduces a limitation in term of number of
expectations attached to one IPPair. This is done using
a circle list so we have a FIFO approach on expectation
handling.

Circleq list code is copied from BSD code like was pre existing code
in queue.h.

(cherry picked from commit a4cea196c0ea9aa5f2b34a43140056d689003c13)
(cherry picked from commit 230dbafa22dc015552ec73a3b0eb70e209ed2190)

Commits squashed to avoid circular dependeny by Victor Julien.

diff --git a/src/app-layer-expectation.c b/src/app-layer-expectation.c
index dec089bafc01f45f615dd13de25b0a456b7525d1..91cb067c2a0d7a65e2a942fd49836d911d410945 100644 (file)
--- a/src/app-layer-expectation.c
+++ b/src/app-layer-expectation.c
@@ -61,6 +61,7 @@
 #include "app-layer-expectation.h"
 
 #include "util-print.h"
+#include "queue.h"
 
 static int g_expectation_id = -1;
 static int g_expectation_data_id = -1;
@@ -68,6 +69,7 @@ static int g_expectation_data_id = -1;
 SC_ATOMIC_DECLARE(uint32_t, expectation_count);
 
 #define EXPECTATION_TIMEOUT 30
+#define EXPECTATION_MAX_LEVEL 10
 
 typedef struct Expectation_ {
     struct timeval ts;
@@ -75,8 +77,10 @@ typedef struct Expectation_ {
     Port dp;
     AppProto alproto;
     int direction;
+    /* use pointer to Flow as identifier of the Flow the expectation is linked to */
+    void *orig_f;
     void *data;
-    struct Expectation_ *next;
+    CIRCLEQ_ENTRY(Expectation_) entries;
 } Expectation;
 
 typedef struct ExpectationData_ {
@@ -85,6 +89,11 @@ typedef struct ExpectationData_ {
     void (*DFree)(void *);
 } ExpectationData;
 
+typedef struct ExpectationList_ {
+    CIRCLEQ_HEAD(EList, Expectation_) list;
+    uint8_t length;
+} ExpectationList;
+
 static void ExpectationDataFree(void *e)
 {
     SCLogDebug("Free expectation data");
@@ -96,23 +105,37 @@ static void ExpectationDataFree(void *e)
     }
 }
 
-static void ExpectationListFree(void *e)
+/**
+ * Free expectation
+ */
+static void AppLayerFreeExpectation(Expectation *exp)
 {
-    Expectation *exp = (Expectation *)e;
-    Expectation *lexp;
-    while (exp) {
-        lexp = exp->next;
-        if (exp->data) {
-            ExpectationData *expdata = (ExpectationData *) exp->data;
-            if (expdata->DFree) {
-                expdata->DFree(exp->data);
-            } else {
-                SCFree(exp->data);
-            }
+    if (exp->data) {
+        ExpectationData *expdata = (ExpectationData *)exp->data;
+        if (expdata->DFree) {
+            expdata->DFree(exp->data);
+        } else {
+            SCFree(exp->data);
+        }
+    }
+    SCFree(exp);
+}
+
+static void ExpectationListFree(void *el)
+{
+    ExpectationList *exp_list = (ExpectationList *)el;
+    Expectation *exp, *pexp;
+    if (exp_list == NULL)
+        return;
+
+    if (exp_list->length > 0) {
+        CIRCLEQ_FOREACH_SAFE(exp, &exp_list->list, entries, pexp) {
+            CIRCLEQ_REMOVE(&exp_list->list, exp, entries);
+            exp_list->length--;
+            AppLayerFreeExpectation(exp);
         }
-        SCFree(exp);
-        exp = lexp;
     }
+    SCFree(exp_list);
 }
 
 uint64_t ExpectationGetCounter(void)
@@ -144,7 +167,7 @@ static inline int GetFlowAddresses(Flow *f, Address *ip_src, Address *ip_dst)
     return 0;
 }
 
-static Expectation *AppLayerExpectationLookup(Flow *f, IPPair **ipp)
+static ExpectationList *AppLayerExpectationLookup(Flow *f, IPPair **ipp)
 {
     Address ip_src, ip_dst;
     if (GetFlowAddresses(f, &ip_src, &ip_dst) == -1)
@@ -157,11 +180,30 @@ static Expectation *AppLayerExpectationLookup(Flow *f, IPPair **ipp)
     return IPPairGetStorageById(*ipp, g_expectation_id);
 }
 
+
+static ExpectationList *AppLayerExpectationRemove(IPPair *ipp,
+                                                  ExpectationList *exp_list,
+                                                  Expectation *exp)
+{
+    CIRCLEQ_REMOVE(&exp_list->list, exp, entries);
+    AppLayerFreeExpectation(exp);
+    SC_ATOMIC_SUB(expectation_count, 1);
+    exp_list->length--;
+    if (exp_list->length == 0) {
+        IPPairSetStorageById(ipp, g_expectation_id, NULL);
+        ExpectationListFree(exp_list);
+        exp_list = NULL;
+    }
+    return exp_list;
+}
+
 /**
  * Create an entry in expectation list
  *
  * Create a expectation from an existing Flow. Currently, only Flow between
- * the two original IP addresses are supported.
+ * the two original IP addresses are supported. In case of success, the
+ * ownership of the data pointer is taken. In case of error, the pointer
+ * to data has to be freed by the caller.
  *
  * \param f a pointer to the original Flow
  * \param direction the direction of the data in the expectation flow
@@ -176,7 +218,7 @@ static Expectation *AppLayerExpectationLookup(Flow *f, IPPair **ipp)
 int AppLayerExpectationCreate(Flow *f, int direction, Port src, Port dst,
                               AppProto alproto, void *data)
 {
-    Expectation *iexp = NULL;
+    ExpectationList *exp_list = NULL;
     IPPair *ipp;
     Address ip_src, ip_dst;
 
@@ -188,6 +230,7 @@ int AppLayerExpectationCreate(Flow *f, int direction, Port src, Port dst,
     exp->dp = dst;
     exp->alproto = alproto;
     exp->ts = f->lastts;
+    exp->orig_f = (void *)f;
     exp->data = data;
     exp->direction = direction;
 
@@ -197,11 +240,34 @@ int AppLayerExpectationCreate(Flow *f, int direction, Port src, Port dst,
     if (ipp == NULL)
         goto error;
 
-    iexp = IPPairGetStorageById(ipp, g_expectation_id);
-    exp->next = iexp;
-    IPPairSetStorageById(ipp, g_expectation_id, exp);
+    exp_list = IPPairGetStorageById(ipp, g_expectation_id);
+    if (exp_list) {
+        CIRCLEQ_INSERT_HEAD(&exp_list->list, exp, entries);
+        /* In case there is already EXPECTATION_MAX_LEVEL expectations waiting to be fullfill,
+         * we remove the older expectation to limit the total number of expectations */
+        if (exp_list->length >= EXPECTATION_MAX_LEVEL) {
+            Expectation *last_exp = CIRCLEQ_LAST(&exp_list->list);
+            CIRCLEQ_REMOVE(&exp_list->list, last_exp, entries);
+            AppLayerFreeExpectation(last_exp);
+            /* We keep the same amount of expectation so we fully release
+             * the IP pair */
+            f->flags |= FLOW_HAS_EXPECTATION;
+            IPPairRelease(ipp);
+            return 0;
+        }
+    } else {
+        exp_list = SCCalloc(1, sizeof(*exp_list));
+        if (exp_list == NULL)
+            goto error;
+        exp_list->length = 0;
+        CIRCLEQ_INIT(&exp_list->list);
+        CIRCLEQ_INSERT_HEAD(&exp_list->list, exp, entries);
+        IPPairSetStorageById(ipp, g_expectation_id, exp_list);
+    }
 
+    exp_list->length += 1;
     SC_ATOMIC_ADD(expectation_count, 1);
+    f->flags |= FLOW_HAS_EXPECTATION;
     /* As we are creating the expectation, we release lock on IPPair without
      * setting the ref count to 0. This way the IPPair will be kept till
      * cleanup */
@@ -223,42 +289,6 @@ int AppLayerExpectationGetDataId(void)
     return g_expectation_data_id;
 }
 
-/**
- *
- * Remove expectation and return next one
- *
- * \param ipp an IPPair
- * \param pexp pointer to previous Expectation
- * \param exp pointer to Expectation to remove
- * \param lexp pointer to head of Expectation ist
- * \return expectation
- */
-static Expectation * RemoveExpectationAndGetNext(IPPair *ipp,
-                                Expectation *pexp, Expectation *exp,
-                                Expectation *lexp)
-{
-    /* we remove the object so we get ref count down by 1 to remove reference
-     * hold by the expectation
-     */
-    (void) IPPairDecrUsecnt(ipp);
-    SC_ATOMIC_SUB(expectation_count, 1);
-    if (pexp == NULL) {
-        IPPairSetStorageById(ipp, g_expectation_id, lexp);
-    } else {
-        pexp->next = lexp;
-    }
-    if (exp->data) {
-        ExpectationData *expdata = (ExpectationData *)exp->data;
-        if (expdata->DFree) {
-            expdata->DFree(exp->data);
-        } else {
-            SCFree(exp->data);
-        }
-    }
-    SCFree(exp);
-    return lexp;
-}
-
 /**
  * Function doing a lookup in expectation list and updating Flow if needed.
  *
@@ -274,7 +304,7 @@ AppProto AppLayerExpectationHandle(Flow *f, int direction)
     AppProto alproto = ALPROTO_UNKNOWN;
     IPPair *ipp = NULL;
     Expectation *lexp = NULL;
-    Expectation *pexp = NULL;
+    Expectation *exp = NULL;
 
     int x = SC_ATOMIC_GET(expectation_count);
     if (x == 0) {
@@ -282,16 +312,14 @@ AppProto AppLayerExpectationHandle(Flow *f, int direction)
     }
 
     /* Call will take reference of the ip pair in 'ipp' */
-    Expectation *exp = AppLayerExpectationLookup(f, &ipp);
-    if (exp == NULL)
+    ExpectationList *exp_list = AppLayerExpectationLookup(f, &ipp);
+    if (exp_list == NULL)
         goto out;
 
     time_t ctime = f->lastts.tv_sec;
 
-    pexp = NULL;
-    while (exp) {
-        lexp = exp->next;
-        if ( (exp->direction & direction) &&
+    CIRCLEQ_FOREACH_SAFE(exp, &exp_list->list, entries, lexp) {
+        if ((exp->direction & direction) &&
              ((exp->sp == 0) || (exp->sp == f->sp)) &&
              ((exp->dp == 0) || (exp->dp == f->dp))) {
             alproto = exp->alproto;
@@ -308,16 +336,18 @@ AppProto AppLayerExpectationHandle(Flow *f, int direction)
                 }
             }
             exp->data = NULL;
-            exp = RemoveExpectationAndGetNext(ipp, pexp, exp, lexp);
+            exp_list = AppLayerExpectationRemove(ipp, exp_list, exp);
+            if (exp_list == NULL)
+                goto out;
             continue;
         }
         /* Cleaning remove old entries */
-        if (exp && (ctime > exp->ts.tv_sec + EXPECTATION_TIMEOUT)) {
-            exp = RemoveExpectationAndGetNext(ipp, pexp, exp, lexp);
+        if (ctime > exp->ts.tv_sec + EXPECTATION_TIMEOUT) {
+            exp_list = AppLayerExpectationRemove(ipp, exp_list, exp);
+            if (exp_list == NULL)
+                goto out;
             continue;
         }
-        pexp = exp;
-        exp = lexp;
     }
 
 out:
@@ -326,6 +356,37 @@ out:
     return alproto;
 }
 
+void AppLayerExpectationClean(Flow *f)
+{
+    IPPair *ipp = NULL;
+    Expectation *exp = NULL;
+    Expectation *pexp = NULL;
+
+    int x = SC_ATOMIC_GET(expectation_count);
+    if (x == 0) {
+        return;
+    }
+
+    /* Call will take reference of the ip pair in 'ipp' */
+    ExpectationList *exp_list = AppLayerExpectationLookup(f, &ipp);
+    if (exp_list == NULL)
+        goto out;
+
+    CIRCLEQ_FOREACH_SAFE(exp, &exp_list->list, entries, pexp) {
+        /* Cleaning remove old entries */
+        if (exp->orig_f == (void *)f) {
+            exp_list = AppLayerExpectationRemove(ipp, exp_list, exp);
+            if (exp_list == NULL)
+                goto out;
+        }
+    }
+
+out:
+    if (ipp)
+        IPPairRelease(ipp);
+    return;
+}
+
 /**
  * @}
  */

diff --git a/src/app-layer-expectation.h b/src/app-layer-expectation.h
index 70f743a03fc4c6ae598bb7a07faac8c3c82d42cb..c45b85f5194c65b050ce0a8d91ba4f2eeabed54f 100644 (file)
--- a/src/app-layer-expectation.h
+++ b/src/app-layer-expectation.h
@@ -30,6 +30,8 @@ int AppLayerExpectationCreate(Flow *f, int direction, Port src, Port dst,
 AppProto AppLayerExpectationHandle(Flow *f, int direction);
 int AppLayerExpectationGetDataId(void);
 
+void AppLayerExpectationClean(Flow *f);
+
 uint64_t ExpectationGetCounter(void);
 
 #endif /* __APP_LAYER_EXPECTATION__H__ */

diff --git a/src/flow.c b/src/flow.c
index 0d3a187ef4eb315a4f4d6cf15583a9e0307ef798..c7a8617ee071b033eb66cd265dc29a8e21bd15d8 100644 (file)
--- a/src/flow.c
+++ b/src/flow.c
@@ -62,6 +62,7 @@
 #include "stream.h"
 
 #include "app-layer-parser.h"
+#include "app-layer-expectation.h"
 
 #define FLOW_DEFAULT_EMERGENCY_RECOVERY 30
 
@@ -1052,6 +1053,9 @@ int FlowClearMemory(Flow* f, uint8_t proto_map)
 
     FlowFreeStorage(f);
 
+    if (f->flags & FLOW_HAS_EXPECTATION)
+        AppLayerExpectationClean(f);
+
     FLOW_RECYCLE(f);
 
     SCReturnInt(1);

diff --git a/src/flow.h b/src/flow.h
index b2eb217964834aa1a9265764a981e0fb5d00c04f..5cdbdbc64affbd0b4f4c232cc75d13c971f1be46 100644 (file)
--- a/src/flow.h
+++ b/src/flow.h
@@ -104,6 +104,8 @@ typedef struct AppLayerParserState_ AppLayerParserState;
 #define FLOW_WRONG_THREAD               BIT_U32(25)
 /** Protocol detection told us flow is picked up in wrong direction (midstream) */
 #define FLOW_DIR_REVERSED               BIT_U32(26)
+/** Indicate that the flow did trigger an expectation creation */
+#define FLOW_HAS_EXPECTATION            BIT_U32(27)
 
 /* File flags */
 

diff --git a/src/queue.h b/src/queue.h
index 5e2a7b1366f43fa110eb032b3359f9488bcd52ff..d5fb32f11a7bb4e84f732c1939e0a3064f08851c 100644 (file)
--- a/src/queue.h
+++ b/src/queue.h
@@ -550,4 +550,16 @@ struct {                                                           \
        _Q_INVALIDATE((elm)->field.cqe_next);                           \
 } while (0)
 
+#define        CIRCLEQ_FOREACH_SAFE(var, head, field, tvar)                    \
+    for ((var) = CIRCLEQ_FIRST(head);                          \
+        (var) != CIRCLEQ_END(head) &&                          \
+        ((tvar) = CIRCLEQ_NEXT(var, field), 1);                        \
+        (var) = (tvar))
+
+#define        CIRCLEQ_FOREACH_REVERSE_SAFE(var, head, headname, field, tvar)  \
+    for ((var) = CIRCLEQ_LAST(head, headname);                 \
+        (var) != CIRCLEQ_END(head) &&                          \
+        ((tvar) = CIRCLEQ_PREV(var, headname, field), 1);              \
+        (var) = (tvar))
+
 #endif /* !_SYS_QUEUE_H_ */