detect: enforce isdataat:!1,relative earlier

The expression 'isdataat:!1,relative' is used to make sure a match
is at the end of a buffer quite often. This patch optimizes this case
for 'content' followed by the expression. It enforces it by setting
and 'ends with' flag on the content and then taking that flag into
account while doing the pattern match.

diff --git a/src/detect-content.h b/src/detect-content.h
index 2182b1bacd00ced507bb481257dacbbcfa6392dc..81bd2a1fa316d00846a8f10908ec5a6c6cb01289 100644 (file)
--- a/src/detect-content.h
+++ b/src/detect-content.h
@@ -39,7 +39,7 @@
 /** content is negated */
 #define DETECT_CONTENT_NEGATED           BIT_U32(9)
 
-// bit 10 unused
+#define DETECT_CONTENT_ENDS_WITH         BIT_U32(10)
 
 /* BE - byte extract */
 #define DETECT_CONTENT_OFFSET_BE         BIT_U32(11)

diff --git a/src/detect-engine-content-inspection.c b/src/detect-engine-content-inspection.c
index bea5157e53fae38035bcf17ec1340d5ef00f84d6..7c69c236e146f707bc8461f6411c2b118d1ab657 100644 (file)
--- a/src/detect-engine-content-inspection.c
+++ b/src/detect-engine-content-inspection.c
@@ -273,13 +273,16 @@ int DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCtx
 #ifdef DEBUG
             BUG_ON(sbuffer_len > buffer_len);
 #endif
-
-            /* \todo Add another optimization here.  If cd->content_len is
-             * greater than sbuffer_len found is anyways NULL */
-
-            /* do the actual search */
-            found = SpmScan(cd->spm_ctx, det_ctx->spm_thread_ctx, sbuffer,
-                            sbuffer_len);
+            if (cd->flags & DETECT_CONTENT_ENDS_WITH && depth < buffer_len) {
+                SCLogDebug("depth < buffer_len while DETECT_CONTENT_ENDS_WITH is set. Can't possibly match.");
+                found = NULL;
+            } else if (cd->content_len > sbuffer_len) {
+                found = NULL;
+            } else {
+                /* do the actual search */
+                found = SpmScan(cd->spm_ctx, det_ctx->spm_thread_ctx, sbuffer,
+                        sbuffer_len);
+            }
 
             /* next we evaluate the result in combination with the
              * negation flag. */
@@ -306,48 +309,49 @@ int DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCtx
                 SCLogDebug("content %"PRIu32" matched at offset %"PRIu32"", cd->id, match_offset);
                 det_ctx->buffer_offset = match_offset;
 
-                /* Match branch, add replace to the list if needed */
-                if (cd->flags & DETECT_CONTENT_REPLACE) {
-                    if (inspection_mode == DETECT_ENGINE_CONTENT_INSPECTION_MODE_PAYLOAD) {
-                        /* we will need to replace content if match is confirmed */
-                        det_ctx->replist = DetectReplaceAddToList(det_ctx->replist, found, cd);
-                    } else {
-                        SCLogWarning(SC_ERR_INVALID_VALUE, "Can't modify payload without packet");
+                if ((cd->flags & DETECT_CONTENT_ENDS_WITH) == 0 || match_offset == buffer_len) {
+                    /* Match branch, add replace to the list if needed */
+                    if (cd->flags & DETECT_CONTENT_REPLACE) {
+                        if (inspection_mode == DETECT_ENGINE_CONTENT_INSPECTION_MODE_PAYLOAD) {
+                            /* we will need to replace content if match is confirmed */
+                            det_ctx->replist = DetectReplaceAddToList(det_ctx->replist, found, cd);
+                        } else {
+                            SCLogWarning(SC_ERR_INVALID_VALUE, "Can't modify payload without packet");
+                        }
                     }
-                }
 
-                /* if this is the last match we're done */
-                if (smd->is_last) {
-                    goto match;
-                }
+                    /* if this is the last match we're done */
+                    if (smd->is_last) {
+                        goto match;
+                    }
 
-                SCLogDebug("content %"PRIu32, cd->id);
-                KEYWORD_PROFILING_END(det_ctx, smd->type, 1);
+                    SCLogDebug("content %"PRIu32, cd->id);
+                    KEYWORD_PROFILING_END(det_ctx, smd->type, 1);
 
-                /* see if the next buffer keywords match. If not, we will
-                 * search for another occurence of this content and see
-                 * if the others match then until we run out of matches */
-                int r = DetectEngineContentInspection(de_ctx, det_ctx, s, smd+1,
-                        f, buffer, buffer_len, stream_start_offset, inspection_mode, data);
-                if (r == 1) {
-                    SCReturnInt(1);
-                }
-                SCLogDebug("no match for 'next sm'");
-
-                if (det_ctx->discontinue_matching) {
-                    SCLogDebug("'next sm' said to discontinue this right now");
-                    goto no_match;
-                }
+                    /* see if the next buffer keywords match. If not, we will
+                     * search for another occurence of this content and see
+                     * if the others match then until we run out of matches */
+                    int r = DetectEngineContentInspection(de_ctx, det_ctx, s, smd+1,
+                            f, buffer, buffer_len, stream_start_offset, inspection_mode, data);
+                    if (r == 1) {
+                        SCReturnInt(1);
+                    }
+                    SCLogDebug("no match for 'next sm'");
 
-                /* no match and no reason to look for another instance */
-                if ((cd->flags & DETECT_CONTENT_WITHIN_NEXT) == 0) {
-                    SCLogDebug("'next sm' does not depend on me, so we can give up");
-                    det_ctx->discontinue_matching = 1;
-                    goto no_match;
-                }
+                    if (det_ctx->discontinue_matching) {
+                        SCLogDebug("'next sm' said to discontinue this right now");
+                        goto no_match;
+                    }
 
-                SCLogDebug("'next sm' depends on me %p, lets see what we can do (flags %u)", cd, cd->flags);
+                    /* no match and no reason to look for another instance */
+                    if ((cd->flags & DETECT_CONTENT_WITHIN_NEXT) == 0) {
+                        SCLogDebug("'next sm' does not depend on me, so we can give up");
+                        det_ctx->discontinue_matching = 1;
+                        goto no_match;
+                    }
 
+                    SCLogDebug("'next sm' depends on me %p, lets see what we can do (flags %u)", cd, cd->flags);
+                }
                 /* set the previous match offset to the start of this match + 1 */
                 prev_offset = (match_offset - (cd->content_len - 1));
                 SCLogDebug("trying to see if there is another match after prev_offset %"PRIu32, prev_offset);

diff --git a/src/detect-isdataat.c b/src/detect-isdataat.c
index 4d5df7f73811bab6540f3121816c4771fd89bf17..56cdbb3f62e686c56c08e570f2746eb8deaf2fcd 100644 (file)
--- a/src/detect-isdataat.c
+++ b/src/detect-isdataat.c
@@ -242,6 +242,17 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, char *isdataatst
         SCFree(offset);
     }
 
+    /* 'ends with' scenario */
+    if (prev_pm != NULL && prev_pm->type == DETECT_CONTENT &&
+        idad->dataat == 1 &&
+        (idad->flags & (ISDATAAT_RELATIVE|ISDATAAT_NEGATED)) == (ISDATAAT_RELATIVE|ISDATAAT_NEGATED))
+    {
+        DetectContentData *cd = (DetectContentData *)prev_pm->ctx;
+        cd->flags |= DETECT_CONTENT_ENDS_WITH;
+        ret = 0;
+        goto end;
+    }
+
     sm = SigMatchAlloc();
     if (sm == NULL)
         goto end;

diff --git a/src/tests/detect-engine-content-inspection.c b/src/tests/detect-engine-content-inspection.c
index fe3eddbdb774bd594b82b2379d43e0d8baf77b83..a855a21ea3b7d9d8c379cee132e9d73d65f1feb3 100644 (file)
--- a/src/tests/detect-engine-content-inspection.c
+++ b/src/tests/detect-engine-content-inspection.c
@@ -139,12 +139,17 @@ static int DetectEngineContentInspectionTest06(void) {
     TEST_RUN("ababc", 5, "content:\"a\"; content:\"b\"; content:\"d\";", false, 3);
 
     // 6 steps: (1) a, (2) 1st b, (3) c not found, (4) 2nd b, (5) c found, isdataat
-    TEST_RUN("ababc", 5, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:!1,relative;", true, 6);
+    TEST_RUN("ababc", 5, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:!1,relative;", true, 5);
     TEST_RUN("ababc", 5, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:1,relative;", false, 6);
 
-    TEST_RUN("ababcabc", 8, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:!1,relative;", true, 9);
+    TEST_RUN("ababcabc", 8, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:!1,relative;", true, 7);
     TEST_RUN("ababcabc", 8, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:1,relative;", true, 6);
 
+    TEST_RUN("abcXYZ", 6, "content:\"abc\"; content:\"XYZ\"; distance:0; within:3; isdataat:!1,relative;", true, 2);
+    TEST_RUN("abcXYZ", 6, "content:\"XYZ\"; distance:3; within:3; isdataat:!1,relative;", true, 1);
+    TEST_RUN("abcXYZ", 6, "content:\"cXY\"; distance:2; within:3; isdataat:!1,relative;", false, 1);
+
+    TEST_RUN("xxxxxxxxxxxxxxxxxyYYYYYYYYYYYYYYYY", 34, "content:\"yYYYYYYYYYYYYYYYY\"; distance:9; within:29; isdataat:!1,relative;", true, 1);
     TEST_FOOTER;
 }
 
@@ -157,13 +162,13 @@ static int DetectEngineContentInspectionTest07(void) {
 
     TEST_RUN("abcabcabcabcabcabcabcabcabcabcx", 31, "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; content:\"d\"; distance:0; ", false, 4);
     TEST_RUN("abcabcabcabcabcabcabcabcabcabcx", 31, "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; pcre:\"/^d/R\"; ", false, 13);
-    TEST_RUN("abcabcabcabcabcabcabcabcabcabcx", 31, "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; isdataat:!1,relative; ", false, 13); // TODO should be 4?
+    TEST_RUN("abcabcabcabcabcabcabcabcabcabcx", 31, "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; isdataat:!1,relative; ", false, 3);
     TEST_RUN("abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdx", 41,
             "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; content:\"d\"; distance:0; content:\"e\"; distance:0; ", false, 5);
     TEST_RUN("abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdx", 41,
             "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; content:\"d\"; distance:0; pcre:\"/^e/R\"; ", false, 14); // TODO should be 5?
     TEST_RUN("abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdx", 41,
-            "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; content:\"d\"; distance:0; isdataat:!1,relative; ", false, 14); // TODO should be 5?
+            "content:\"a\"; content:\"b\"; distance:0; content:\"c\"; distance:0; content:\"d\"; distance:0; isdataat:!1,relative; ", false, 4);
 
     TEST_RUN("abcabcabcabcabcabcabcabcabcabcd", 31, "content:\"a\"; content:\"b\"; within:1; distance:0; content:\"c\"; distance:0; within:1; pcre:\"/d/\";", true, 4);
     TEST_RUN("abcabcabcabcabcabcabcabcabcabcd", 31, "content:\"a\"; content:\"b\"; within:1; distance:0; content:\"c\"; distance:0; within:1; pcre:\"/d/R\";", true, 4);
@@ -195,7 +200,7 @@ static int DetectEngineContentInspectionTest09(void) {
 
     TEST_RUN("abc03abcxyz", 11, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3;", true, 3);
     TEST_RUN("abc03abc03abcxyz", 16, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3;", true, 5);
-    TEST_RUN("abc03abc03abcxyz", 16, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3; isdataat:!1,relative;", true, 6);
+    TEST_RUN("abc03abc03abcxyz", 16, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3; isdataat:!1,relative;", true, 5);
     TEST_RUN("abc03abc03abcxyz", 16, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3; pcre:\"/klm$/R\";", false, 7);
     TEST_RUN("abc03abc03abcxyzklm", 19, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3; pcre:\"/klm$/R\";", true, 6);
     TEST_RUN("abc03abc03abcxyzklx", 19, "content:\"abc\"; byte_jump:2,0,relative,string,dec; content:\"xyz\"; within:3; pcre:\"/^klm$/R\";", false, 7);