detect: allow rule which need both directions to match

Ticket: 5665

This is done with `alert ip any any => any any`
The => operator means that we will need both directions

doc/userguide/rules/intro.rst

@@ -228,11 +228,14 @@ 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.
+There is also the double arrow (``=>``), which respects the directionality as ``->``,
+but allows matching on bidirectional transactions, used with keywords matching each direction.
+Finally, it is also possible to have a rule match either directions (``<>``)::
 
   source -> destination
-  source <> destination  (both directions)
+  source => destination
+  source <> destination  (either directions)
 
 The following example illustrates direction. In this example there is a client
 with IP address 1.2.3.4 using port 1024. A server with IP address 5.6.7.8,
@@ -248,10 +251,54 @@ Now, let's say we have a rule with the following header::
 Only the traffic from the client to the server will be matched by this rule,
 as the direction specifies that we do not want to evaluate the response packet.
 
+Now,if we have a rule with the following header::
+
+    alert tcp 1.2.3.4 any <> 5.6.7.8 80
+
+Suricata will duplicate it and use the same rule with headers in both directions :
+
+    alert tcp 1.2.3.4 any -> 5.6.7.8 80
+    alert tcp 5.6.7.8 80 -> 1.2.3.4 any
+
 .. warning::
 
    There is no 'reverse' style direction, i.e. there is no ``<-``.
 
+Bidirectional rules
+~~~~~~~~~~~~~~~~~~~
+
+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 can use direction-ambiguous keywords, by first using
+``bidir.toclient`` or ``bidir.toserver`` keywords.
+
+.. container:: example-rule
+
+    alert http any any => 5.6.7.8 80 (msg:"matching json to server and xml to client"; sid: 1;  :example-rule-emphasis:`bidir.toserver;` http.content_type; content: "json";  :example-rule-emphasis:`bidir.toclient;` http.content_type; content: "xml";)
+
+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 (not tested).
+* They cannot have ``fast_pattern`` or ``prefilter`` the direction to client
+  if they also have a streaming buffer on the direction to server, see example below.
+* They will refuse to load if a single directional rule is enough.
+
+This rule cannot have the ``fast_pattern`` to client, as ``file.data`` is a streaming buffer.
+
+.. container:: example-rule
+
+    alert http any any => any any (bidir.toserver; file.data; content: "123";  http.stat_code; content: "500"; fast_patten;)
+
 Rule options
 ------------
 The rest of the rule consists of options. These are enclosed by parenthesis

src/app-layer-htp.c

@@ -1172,6 +1172,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

@@ -495,6 +495,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

@@ -1071,6 +1071,24 @@ static SigMatch *GetMpmForList(const Signature *s, SigMatch *list, SigMatch *mpm
 
 int g_skip_prefilter = 0;
 
+bool DetectBufferToClient(const DetectEngineCtx *de_ctx, int buf_id, AppProto alproto)
+{
+    bool r = false;
+    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) {
+                // do not return yet in case we have app engines on both sides
+                r = true;
+            } else {
+                // ambiguous keywords have a app-engine to server
+                return false;
+            }
+        }
+    }
+    return r;
+}
+
 void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
 {
     if (g_skip_prefilter)
@@ -1173,6 +1191,12 @@ void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
              tmp != NULL && priority == tmp->priority;
              tmp = tmp->next)
         {
+            if (s->flags & SIG_FLAG_BOTHDIR) {
+                // prefer to choose a fast_pattern to server by default
+                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

@@ -136,4 +136,6 @@ struct MpmListIdDataArgs {
 
 void EngineAnalysisAddAllRulePatterns(DetectEngineCtx *de_ctx, const Signature *s);
 
+bool DetectBufferToClient(const DetectEngineCtx *de_ctx, int buf_id, AppProto alproto);
+
 #endif /* SURICATA_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-register.c

@@ -568,6 +568,7 @@ void SigTableSetup(void)
     DetectOffsetRegister();
     DetectReplaceRegister();
     DetectFlowRegister();
+    DetectBidirRegister();
     DetectFlowAgeRegister();
     DetectFlowPktsToClientRegister();
     DetectFlowPktsToServerRegister();

src/detect-engine-register.h

@@ -302,6 +302,9 @@ enum DetectKeywordId {
 
     DETECT_PREFILTER,
 
+    DETECT_BIDIR_TOCLIENT,
+    DETECT_BIDIR_TOSERVER,
+
     DETECT_TRANSFORM_COMPRESS_WHITESPACE,
     DETECT_TRANSFORM_STRIP_WHITESPACE,
     DETECT_TRANSFORM_STRIP_PSEUDO_HEADERS,

src/detect-engine-state.c

@@ -89,9 +89,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;
@@ -120,11 +120,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) {
@@ -172,7 +176,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;
@@ -239,17 +243,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[i].head;
+        }
     }
 }
 

src/detect-engine-state.h

@@ -89,8 +89,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-engine.c

@@ -761,6 +761,15 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature
             for (const DetectEngineAppInspectionEngine *t = de_ctx->app_inspect_engines; t != NULL;
                     t = t->next) {
                 if (t->sm_list == s->init_data->buffers[x].id) {
+                    if (s->flags & SIG_FLAG_BOTHDIR) {
+                        // ambiguous keywords have app engines in both directions
+                        // so we skip the wrong direction for this buffer
+                        if (s->init_data->buffers[x].only_tc && t->dir == 0) {
+                            continue;
+                        } else if (s->init_data->buffers[x].only_ts && t->dir == 1) {
+                            continue;
+                        }
+                    }
                     AppendAppInspectEngine(
                             de_ctx, t, s, smd, mpm_list, files_id, &last_id, &head_is_mpm);
                 }
@@ -1351,6 +1360,30 @@ bool DetectBufferIsPresent(const Signature *s, const uint32_t buf_id)
     return false;
 }
 
+static bool DetectEngineBufferAmbiguousDir(
+        DetectEngineCtx *de_ctx, const int list, AppProto alproto)
+{
+    bool has_ts = false;
+    bool has_tc = false;
+    const DetectEngineAppInspectionEngine *app = de_ctx->app_inspect_engines;
+    for (; app != NULL; app = app->next) {
+        if (app->sm_list == list && (AppProtoEquals(alproto, app->alproto) || alproto == 0)) {
+            if (app->dir == 0) {
+                if (has_tc) {
+                    return true;
+                }
+                has_ts = true;
+            } else if (app->dir == 1) {
+                if (has_ts) {
+                    return true;
+                }
+                has_tc = true;
+            }
+        }
+    }
+    return false;
+}
+
 int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int list)
 {
     BUG_ON(s->init_data == NULL);
@@ -1389,7 +1422,12 @@ int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int l
 
             } else if (DetectEngineBufferTypeSupportsMultiInstanceGetById(de_ctx, list)) {
                 // fall through
+            } else if (b->only_tc && (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOSERVER)) {
+                // fall through
+            } else if (b->only_ts && (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT)) {
+                // fall through
             } else {
+                // we create a new buffer for the same id but forced different direction
                 SCLogWarning("duplicate instance for %s in '%s'",
                         DetectEngineBufferTypeGetNameById(de_ctx, list), s->sig_str);
                 s->init_data->curbuf = b;
@@ -1413,6 +1451,13 @@ int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int l
     s->init_data->curbuf->tail = NULL;
     s->init_data->curbuf->multi_capable =
             DetectEngineBufferTypeSupportsMultiInstanceGetById(de_ctx, list);
+    if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) {
+        s->init_data->curbuf->only_tc = DetectEngineBufferAmbiguousDir(de_ctx, list, s->alproto);
+    }
+    if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOSERVER) {
+        s->init_data->curbuf->only_ts = DetectEngineBufferAmbiguousDir(de_ctx, list, s->alproto);
+    }
+
     SCLogDebug("new: idx %u list %d set up curbuf %p", s->init_data->buffer_index - 1, list,
             s->init_data->curbuf);
 

src/detect-fast-pattern.c

@@ -237,6 +237,19 @@ 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)) {
+            if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER) {
+                SCLogError("fast_pattern cannot be used on to_client keyword for "
+                           "bidirectional rule with a streaming buffer to server %u",
+                        s->id);
+                goto error;
+            } else {
+                s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT;
+            }
+        }
+    }
+
     cd = (DetectContentData *)pm->ctx;
     if ((cd->flags & DETECT_CONTENT_NEGATED) &&
         ((cd->flags & DETECT_CONTENT_DISTANCE) ||

src/detect-file-data.c

@@ -151,6 +151,9 @@ static int DetectFiledataSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
         return -1;
 
     s->init_data->init_flags |= SIG_FLAG_INIT_FILEDATA;
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
     SetupDetectEngineConfig(de_ctx);
     return 0;
 }

src/detect-file-hash-common.c

@@ -332,6 +332,9 @@ int DetectFileHashSetup(
     }
 
     s->file_flags |= FILE_SIG_NEED_FILE;
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
 
     // Setup the file flags depending on the hashing algorithm
     if (type == DETECT_FILEMD5) {

src/detect-filemagic.c

@@ -211,6 +211,9 @@ static int DetectFilemagicSetup (DetectEngineCtx *de_ctx, Signature *s, const ch
     }
     s->init_data->list = DETECT_SM_LIST_NOTSET;
     s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_MAGIC);
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
 
     if (DetectContentSetup(de_ctx, s, str) < 0) {
         return -1;