detect: allow rule which need both directions to match

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/detect-engine-mpm.c

@@ -1071,6 +1071,26 @@ static SigMatch *GetMpmForList(const Signature *s, SigMatch *list, SigMatch *mpm
 
 int g_skip_prefilter = 0;
 
+// tells if a buffer id is only used to client
+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 == ALPROTO_UNKNOWN)) {
+            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)
@@ -1176,6 +1196,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;
@@ -1211,7 +1237,12 @@ void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
             }
         } else {
             for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
+                if (s->init_data->buffers[x].only_tc) {
+                    // prefer to choose a fast_pattern to server by default
+                    continue;
+                }
                 const int list_id = s->init_data->buffers[x].id;
+
                 if (final_sm_list[i] == list_id) {
                     SCLogDebug("%u: list_id %d: %s", s->id, list_id,
                             DetectEngineBufferTypeGetNameById(de_ctx, list_id));

src/detect-engine-mpm.h

@@ -131,4 +131,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-register.c

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

src/detect-engine-register.h

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

@@ -231,6 +231,11 @@ void DetectRunStoreStateTx(
         SCLogDebug("destate created for %"PRIu64, tx_id);
     }
     DeStateSignatureAppend(tx_data->de_state, s, inspect_flags, flow_flags);
+    if (s->flags & SIG_FLAG_BOTHDIR) {
+        // add also in the other DetectEngineStateDirection
+        DeStateSignatureAppend(tx_data->de_state, s, inspect_flags,
+                flow_flags ^ (STREAM_TOSERVER | STREAM_TOCLIENT));
+    }
     StoreStateTxHandleFiles(sgh, f, tx_data->de_state, flow_flags, tx, tx_id, file_no_match);
 
     SCLogDebug("Stored for TX %"PRIu64, tx_id);

src/detect-engine.c

@@ -764,6 +764,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);
                 }
@@ -1354,6 +1363,32 @@ bool DetectBufferIsPresent(const Signature *s, const uint32_t buf_id)
     return false;
 }
 
+// Tells if a buffer (from its list id) is ambiguous about directions
+// meaning it can match on both to client and to server, like http.connection for example
+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);
@@ -1392,7 +1427,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;
@@ -1416,6 +1456,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,18 @@ 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;
+            }
+            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,16 @@ 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) {
+        // we cannot use a bidirectional rule with a fast pattern to client and this
+        if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
+            SCLogError("fast_pattern cannot be used on to_client keyword for "
+                       "bidirectional rule with a streaming buffer to server %u",
+                    s->id);
+            return -1;
+        }
+        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,16 @@ int DetectFileHashSetup(
     }
 
     s->file_flags |= FILE_SIG_NEED_FILE;
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        // we cannot use a bidirectional rule with a fast pattern to client and this
+        if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
+            SCLogError("fast_pattern cannot be used on to_client keyword for "
+                       "bidirectional rule with a streaming buffer to server %u",
+                    s->id);
+            goto error;
+        }
+        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,16 @@ 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) {
+        // we cannot use a bidirectional rule with a fast pattern to client and this
+        if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
+            SCLogError("fast_pattern cannot be used on to_client keyword for "
+                       "bidirectional rule with a streaming buffer to server %u",
+                    s->id);
+            SCReturnInt(-1);
+        }
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
 
     if (DetectContentSetup(de_ctx, s, str) < 0) {
         return -1;

src/detect-filename.c

@@ -128,6 +128,9 @@ static int DetectFileextSetup(DetectEngineCtx *de_ctx, Signature *s, const char
     }
     s->init_data->list = DETECT_SM_LIST_NOTSET;
     s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
 
     size_t dotstr_len = strlen(str) + 2;
     char *dotstr = SCCalloc(1, dotstr_len);
@@ -175,6 +178,16 @@ static int DetectFilenameSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
     }
     s->init_data->list = DETECT_SM_LIST_NOTSET;
     s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        // we cannot use a bidirectional rule with a fast pattern to client and this
+        if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
+            SCLogError("fast_pattern cannot be used on to_client keyword for "
+                       "bidirectional rule with a streaming buffer to server %u",
+                    s->id);
+            SCReturnInt(-1);
+        }
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
 
     if (DetectContentSetup(de_ctx, s, str) < 0) {
         return -1;
@@ -210,6 +223,9 @@ static int DetectFilenameSetupSticky(DetectEngineCtx *de_ctx, Signature *s, cons
     if (DetectBufferSetActiveList(de_ctx, s, g_file_name_buffer_id) < 0)
         return -1;
     s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
     return 0;
 }
 

src/detect-filesize.c

@@ -133,6 +133,17 @@ static int DetectFilesizeSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
     }
 
     s->file_flags |= (FILE_SIG_NEED_FILE|FILE_SIG_NEED_SIZE);
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
+        // we cannot use a bidirectional rule with a fast pattern to client and this
+        if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
+            SCLogError("fast_pattern cannot be used on to_client keyword for "
+                       "bidirectional rule with a streaming buffer to server %u",
+                    s->id);
+            DetectFilesizeFree(de_ctx, fsd);
+            SCReturnInt(-1);
+        }
+        s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
+    }
     SCReturnInt(0);
 }