From: Victor Julien Date: Mon, 6 Mar 2017 15:41:05 +0000 (+0100) Subject: detect: enforce isdataat:!1,relative earlier X-Git-Tag: suricata-4.0.0-beta1~109 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=842dfbc3f86cee39d19ae394d39f87be2e1eea94;p=thirdparty%2Fsuricata.git 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 2182b1bacd..81bd2a1fa3 100644 --- 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 bea5157e53..7c69c236e1 100644 --- 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 4d5df7f738..56cdbb3f62 100644 --- 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 fe3eddbdb7..a855a21ea3 100644 --- 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);