detect: allow rule which need both directions to match

doc/userguide/rules/intro.rst

@@ -226,10 +226,15 @@ Direction
 
 The directional arrow indicates which way the signature will be evaluated.
 In most signatures an arrow to the right (``->``) is used. This means that only
-packets with the same direction can match. However, it is also possible to
-have a rule match both directions (``<>``)::
+packets with the same direction can match.
+It is also possible to have a double arrow (``=>``) which means that the
+directionality for adresses and ports is used,
+but such a rule can match a bidirectional transaction, using keywords
+matching in each direction.
+However, it is also possible to have a rule match both directions (``<>``)::
 
   source -> destination
+  source => destination
   source <> destination  (both directions)
 
 The following example illustrates direction. In this example there is a client
@@ -250,6 +255,25 @@ as the direction specifies that we do not want to evaluate the response packet.
 
    There is no 'reverse' style direction, i.e. there is no ``<-``.
 
+Here is an example of a bidirectional rule:
+
+.. container:: example-rule
+
+    alert http any any :example-rule-emphasis:`=>` 5.6.7.8 80 (msg:"matching both uri and status"; sid: 1; http.uri; content: "/download"; http.stat_code; content: "200";)
+
+It will match on flows to 5.6.7.8 and port 80.
+And it will match on a full transaction, using both the uri from the request,
+and the stat_code from the response.
+As such, it will match only when Suricata got both request and response.
+
+Bidirectional rules have some limitations :
+- They are only meant to work on transactions with first a request to the server,
+and then a response to the client, and not the other way around.
+- They cannot have ``fast_pattern`` or ``prefilter`` on a keyword which is on
+the direction to client.
+- They will not work with ambiguous keywords, which work for both directions.
+- They will refuse to load if a single directional rule is enough.
+
 Rule options
 ------------
 The rest of the rule consists of options. These are enclosed by parenthesis

src/app-layer-htp.c

@@ -1354,6 +1354,8 @@ static void FlagDetectStateNewFile(HtpTxUserData *tx, int dir)
             SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW set");
             tx->tx_data.de_state->dir_state[1].flags |= DETECT_ENGINE_STATE_FLAG_FILE_NEW;
         }
+        tx->tx_data.de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR].flags |=
+                DETECT_ENGINE_STATE_FLAG_FILE_NEW;
     }
 }
 

src/app-layer-smtp.c

@@ -454,6 +454,8 @@ static void FlagDetectStateNewFile(SMTPTransaction *tx)
     if (tx && tx->tx_data.de_state) {
         SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW set");
         tx->tx_data.de_state->dir_state[0].flags |= DETECT_ENGINE_STATE_FLAG_FILE_NEW;
+        tx->tx_data.de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR].flags |=
+                DETECT_ENGINE_STATE_FLAG_FILE_NEW;
     } else if (tx == NULL) {
         SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW NOT set, no TX");
     } else if (tx->tx_data.de_state == NULL) {

src/detect-engine-mpm.c

@@ -1066,6 +1066,19 @@ static SigMatch *GetMpmForList(const Signature *s, SigMatch *list, SigMatch *mpm
 
 int g_skip_prefilter = 0;
 
+bool DetectBufferToClient(DetectEngineCtx *de_ctx, int buf_id, AppProto alproto)
+{
+    const DetectEngineAppInspectionEngine *app = de_ctx->app_inspect_engines;
+    for (; app != NULL; app = app->next) {
+        if (app->sm_list == buf_id && (AppProtoEquals(alproto, app->alproto) || alproto == 0)) {
+            if (app->dir == 1) {
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
 void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
 {
     if (g_skip_prefilter)
@@ -1168,6 +1181,11 @@ void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
              tmp != NULL && priority == tmp->priority;
              tmp = tmp->next)
         {
+            if (s->flags & SIG_FLAG_BOTHDIR) {
+                if (DetectBufferToClient(de_ctx, tmp->list_id, s->alproto)) {
+                    continue;
+                }
+            }
             SCLogDebug("tmp->list_id %d tmp->priority %d", tmp->list_id, tmp->priority);
             if (tmp->list_id >= nlists)
                 continue;

src/detect-engine-mpm.h

@@ -135,5 +135,7 @@ struct MpmListIdDataArgs {
 
 void EngineAnalysisAddAllRulePatterns(DetectEngineCtx *de_ctx, const Signature *s);
 
+bool DetectBufferToClient(DetectEngineCtx *de_ctx, int buf_id, AppProto alproto);
+
 #endif /* __DETECT_ENGINE_MPM_H__ */
 

src/detect-engine-prefilter.h

@@ -33,6 +33,8 @@ typedef struct DetectTransaction_ {
     const uint64_t tx_id;
     struct AppLayerTxData *tx_data_ptr;
     DetectEngineStateDirection *de_state;
+    // state for bidirectional signatures
+    DetectEngineStateDirection *de_state_bidir;
     const uint64_t detect_flags; /* detect flags get/set from/to applayer */
     uint64_t prefilter_flags; /* prefilter flags for direction, to be updated by prefilter code */
     const uint64_t

src/detect-engine-state.c

@@ -92,9 +92,9 @@ static DeStateStore *DeStateStoreAlloc(void)
 }
 
 #ifdef DEBUG_VALIDATION
-static int DeStateSearchState(DetectEngineState *state, uint8_t direction, SigIntId num)
+static int DeStateSearchState(DetectEngineState *state, uint8_t dsi, SigIntId num)
 {
-    DetectEngineStateDirection *dir_state = &state->dir_state[direction & STREAM_TOSERVER ? 0 : 1];
+    DetectEngineStateDirection *dir_state = &state->dir_state[dsi];
     DeStateStore *tx_store = dir_state->head;
     SigIntId store_cnt;
     SigIntId state_cnt = 0;
@@ -123,11 +123,15 @@ static void DeStateSignatureAppend(DetectEngineState *state,
 {
     SCEnter();
 
-    DetectEngineStateDirection *dir_state =
-            &state->dir_state[(direction & STREAM_TOSERVER) ? 0 : 1];
+    uint8_t dsi = (direction & STREAM_TOSERVER) ? 0 : 1;
+    if (s->flags & SIG_FLAG_BOTHDIR) {
+        dsi = DETECT_ENGINE_STATE_DIRECTION_BOTHDIR;
+    }
+
+    DetectEngineStateDirection *dir_state = &state->dir_state[dsi];
 
 #ifdef DEBUG_VALIDATION
-    BUG_ON(DeStateSearchState(state, direction, s->num));
+    BUG_ON(DeStateSearchState(state, dsi, s->num));
 #endif
     DeStateStore *store = dir_state->tail;
     if (store == NULL) {
@@ -175,7 +179,7 @@ void DetectEngineStateFree(DetectEngineState *state)
     DeStateStore *store_next;
     int i = 0;
 
-    for (i = 0; i < 2; i++) {
+    for (i = 0; i < DETECT_ENGINE_STATE_DIRECTIONS; i++) {
         store = state->dir_state[i].head;
         while (store != NULL) {
             store_next = store->next;
@@ -246,17 +250,13 @@ void DetectRunStoreStateTx(
 static inline void ResetTxState(DetectEngineState *s)
 {
     if (s) {
-        s->dir_state[0].cnt = 0;
-        s->dir_state[0].filestore_cnt = 0;
-        s->dir_state[0].flags = 0;
-        /* reset 'cur' back to the list head */
-        s->dir_state[0].cur = s->dir_state[0].head;
-
-        s->dir_state[1].cnt = 0;
-        s->dir_state[1].filestore_cnt = 0;
-        s->dir_state[1].flags = 0;
-        /* reset 'cur' back to the list head */
-        s->dir_state[1].cur = s->dir_state[1].head;
+        for (int i = 0; i < DETECT_ENGINE_STATE_DIRECTIONS; i++) {
+            s->dir_state[i].cnt = 0;
+            s->dir_state[i].filestore_cnt = 0;
+            s->dir_state[i].flags = 0;
+            /* reset 'cur' back to the list head */
+            s->dir_state[i].cur = s->dir_state[0].head;
+        }
     }
 }
 

src/detect-engine-state.h

@@ -90,8 +90,17 @@ typedef struct DetectEngineStateDirection_ {
     /* coccinelle: DetectEngineStateDirection:flags:DETECT_ENGINE_STATE_FLAG_ */
 } DetectEngineStateDirection;
 
+#define DETECT_ENGINE_STATE_DIRECTIONS 3
+
+enum {
+    DETECT_ENGINE_STATE_DIRECTION_TOSERVER = 0,
+    DETECT_ENGINE_STATE_DIRECTION_TOCLIENT = 1,
+    DETECT_ENGINE_STATE_DIRECTION_BOTHDIR = 2,
+};
+
 typedef struct DetectEngineState_ {
-    DetectEngineStateDirection dir_state[2];
+    // to server, to client, and bidirectional
+    DetectEngineStateDirection dir_state[DETECT_ENGINE_STATE_DIRECTIONS];
 } DetectEngineState;
 
 /**

src/detect-fast-pattern.c

@@ -238,6 +238,15 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, const c
         pm = pm2;
     }
 
+    if (s->flags & SIG_FLAG_BOTHDIR && s->init_data->curbuf != NULL) {
+        if (DetectBufferToClient(de_ctx, s->init_data->curbuf->id, s->alproto)) {
+            SCLogError("fast_pattern cannot be used on to_client keyword for "
+                       "bidirectional rule %u",
+                    s->id);
+            goto error;
+        }
+    }
+
     cd = (DetectContentData *)pm->ctx;
     if ((cd->flags & DETECT_CONTENT_NEGATED) &&
         ((cd->flags & DETECT_CONTENT_DISTANCE) ||

src/detect-flow.c

@@ -391,8 +391,18 @@ int DetectFlowSetup (DetectEngineCtx *de_ctx, Signature *s, const char *flowstr)
     bool appendsm = true;
     /* set the signature direction flags */
     if (fd->flags & DETECT_FLOW_FLAG_TOSERVER) {
+        if (s->flags & SIG_FLAG_BOTHDIR) {
+            SCLogError(
+                    "rule %u means to use both directions, cannot specify a flow direction", s->id);
+            goto error;
+        }
         s->flags |= SIG_FLAG_TOSERVER;
     } else if (fd->flags & DETECT_FLOW_FLAG_TOCLIENT) {
+        if (s->flags & SIG_FLAG_BOTHDIR) {
+            SCLogError(
+                    "rule %u means to use both directions, cannot specify a flow direction", s->id);
+            goto error;
+        }
         s->flags |= SIG_FLAG_TOCLIENT;
     } else {
         s->flags |= SIG_FLAG_TOSERVER;

src/detect-parse.c

@@ -1394,6 +1394,8 @@ static int SigParseBasics(DetectEngineCtx *de_ctx, Signature *s, const char *sig
 
     if (strcmp(parser->direction, "<>") == 0) {
         s->init_data->init_flags |= SIG_FLAG_INIT_BIDIREC;
+    } else if (strcmp(parser->direction, "=>") == 0) {
+        s->flags |= SIG_FLAG_BOTHDIR;
     } else if (strcmp(parser->direction, "->") != 0) {
         SCLogError("\"%s\" is not a valid direction modifier, "
                    "\"->\" and \"<>\" are supported.",
@@ -2012,8 +2014,21 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s)
         SCLogDebug("%s/%d: %d/%d", DetectEngineBufferTypeGetNameById(de_ctx, x), x, bufdir[x].ts,
                 bufdir[x].tc);
     }
-    if (ts_excl && tc_excl) {
-        SCLogError("rule %u mixes keywords with conflicting directions", s->id);
+    if (s->flags & SIG_FLAG_BOTHDIR) {
+        if (!ts_excl || !tc_excl) {
+            SCLogError("rule %u should use both directions, but does not", s->id);
+            SCReturnInt(0);
+        }
+        if (dir_amb) {
+            SCLogError("rule %u means to use both directions, cannot have keywords ambiguous about "
+                       "directions",
+                    s->id);
+            SCReturnInt(0);
+        }
+    } else if (ts_excl && tc_excl) {
+        SCLogError("rule %u mixes keywords with conflicting directions, a bidirection rule with => "
+                   "should be used",
+                s->id);
         SCReturnInt(0);
     } else if (ts_excl) {
         SCLogDebug("%u: implied rule direction is toserver", s->id);

src/detect-prefilter.c

@@ -29,6 +29,7 @@
 #include "detect.h"
 #include "detect-parse.h"
 #include "detect-content.h"
+#include "detect-engine-mpm.h"
 #include "detect-prefilter.h"
 #include "util-debug.h"
 
@@ -75,6 +76,15 @@ static int DetectPrefilterSetup (DetectEngineCtx *de_ctx, Signature *s, const ch
     /* if the sig match is content, prefilter should act like
      * 'fast_pattern' w/o options. */
     if (sm->type == DETECT_CONTENT) {
+        if (s->flags & SIG_FLAG_BOTHDIR && s->init_data->curbuf != NULL) {
+            if (DetectBufferToClient(de_ctx, s->init_data->curbuf->id, s->alproto)) {
+                SCLogError("prefilter cannot be used on to_client keyword for "
+                           "bidirectional rule %u",
+                        s->id);
+                SCReturnInt(-1);
+            }
+        }
+
         DetectContentData *cd = (DetectContentData *)sm->ctx;
         if ((cd->flags & DETECT_CONTENT_NEGATED) &&
                 ((cd->flags & DETECT_CONTENT_DISTANCE) ||