From 24f071cabbc2b62102b7a7a186a69eb457379c77 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 5 Jan 2011 20:53:51 +0100 Subject: [PATCH] Make sure http_cookie inspects all HTTP transactions. Clean up error messages. Get rid of unused code and dead comments. --- src/detect-engine-hcd.c | 4 +- src/detect-http-cookie.c | 121 ++++----------------------------------- src/detect-http-method.c | 93 +++++------------------------- src/detect.c | 5 -- src/util-error.c | 6 ++ src/util-error.h | 6 ++ 6 files changed, 38 insertions(+), 197 deletions(-) diff --git a/src/detect-engine-hcd.c b/src/detect-engine-hcd.c index b0e484cf98..0fcfab3796 100644 --- a/src/detect-engine-hcd.c +++ b/src/detect-engine-hcd.c @@ -325,7 +325,7 @@ int DetectEngineRunHttpCookieMpm(DetectEngineThreadCtx *det_ctx, Flow *f, "Cookie"); if (h == NULL) { SCLogDebug("HTTP cookie header not present in this request"); - goto end; + continue; } cnt += HttpCookiePatternSearch(det_ctx, @@ -387,7 +387,7 @@ int DetectEngineInspectHttpCookie(DetectEngineCtx *de_ctx, "Cookie"); if (h == NULL) { SCLogDebug("HTTP cookie header not present in this request"); - goto end; + continue; } r = DoInspectHttpCookie(de_ctx, det_ctx, s, s->sm_lists[DETECT_SM_LIST_HCDMATCH], diff --git a/src/detect-http-cookie.c b/src/detect-http-cookie.c index f7e8de28d3..58246ae469 100644 --- a/src/detect-http-cookie.c +++ b/src/detect-http-cookie.c @@ -53,9 +53,6 @@ #include "detect-http-cookie.h" #include "stream-tcp.h" -int DetectHttpCookieMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx, - Flow *f, uint8_t flags, void *state, Signature *s, - SigMatch *m); static int DetectHttpCookieSetup (DetectEngineCtx *, Signature *, char *); void DetectHttpCookieRegisterTests(void); void DetectHttpCookieFree(void *); @@ -67,7 +64,6 @@ void DetectHttpCookieRegister (void) { sigmatch_table[DETECT_AL_HTTP_COOKIE].name = "http_cookie"; sigmatch_table[DETECT_AL_HTTP_COOKIE].Match = NULL; sigmatch_table[DETECT_AL_HTTP_COOKIE].AppLayerMatch = NULL; - //sigmatch_table[DETECT_AL_HTTP_COOKIE].AppLayerMatch = DetectHttpCookieMatch; sigmatch_table[DETECT_AL_HTTP_COOKIE].alproto = ALPROTO_HTTP; sigmatch_table[DETECT_AL_HTTP_COOKIE].Setup = DetectHttpCookieSetup; sigmatch_table[DETECT_AL_HTTP_COOKIE].Free = DetectHttpCookieFree; @@ -76,100 +72,6 @@ void DetectHttpCookieRegister (void) { sigmatch_table[DETECT_AL_HTTP_COOKIE].flags |= SIGMATCH_PAYLOAD; } -/** - * \brief match the specified content in the signature with the received http - * cookie header in the http request. - * - * \param t pointer to thread vars - * \param det_ctx pointer to the pattern matcher thread - * \param f pointer to the current flow - * \param flags flags to indicate the direction of the received packet - * \param state pointer the app layer state, which will cast into HtpState - * \param s pointer to the current signature - * \param sm pointer to the sigmatch - * - * \retval 0 no match - * \retval 1 match - */ -int DetectHttpCookieMatch (ThreadVars *t, DetectEngineThreadCtx *det_ctx, - Flow *f, uint8_t flags, void *state, Signature *s, - SigMatch *sm) -{ - SCEnter(); - - int ret = 0; - size_t idx; - - SCMutexLock(&f->m); - SCLogDebug("got lock %p", &f->m); - - DetectContentData *co = (DetectContentData *)sm->ctx; - - HtpState *htp_state = (HtpState *)state; - if (htp_state == NULL) { - SCLogDebug("no HTTP layer state has been received, so no match"); - goto end; - } - - if (!(htp_state->flags & HTP_FLAG_STATE_OPEN)) { - SCLogDebug("HTP state not yet properly setup, so no match"); - goto end; - } - - SCLogDebug("htp_state %p, flow %p", htp_state, f); - SCLogDebug("htp_state->connp %p", htp_state->connp); - SCLogDebug("htp_state->connp->conn %p", htp_state->connp->conn); - - if (htp_state->connp == NULL || htp_state->connp->conn == NULL) { - SCLogDebug("HTTP connection structure is NULL"); - goto end; - } - - htp_tx_t *tx = NULL; - - for (idx = 0;//htp_state->new_in_tx_index; - idx < list_size(htp_state->connp->conn->transactions); idx++) - { - tx = list_get(htp_state->connp->conn->transactions, idx); - if (tx == NULL) - continue; - - htp_header_t *h = NULL; - h = (htp_header_t *) table_getc(tx->request_headers, "Cookie"); - if (h == NULL) { - SCLogDebug("no HTTP Cookie header in the received request"); - goto end; - } - - SCLogDebug("we have a cookie header"); - - /* call the case insensitive version if nocase has been specified in the sig */ - if (co->flags & DETECT_CONTENT_NOCASE) { - if (SpmNocaseSearch((uint8_t *) bstr_ptr(h->value), bstr_size(h->value), - co->content, co->content_len) != NULL) { - SCLogDebug("match has been found in received request and given http_" - "cookie rule"); - ret = 1; - } - } else { - if (SpmSearch((uint8_t *) bstr_ptr(h->value), bstr_size(h->value), - co->content, co->content_len) != NULL) { - SCLogDebug("match has been found in received request and given http_" - "cookie rule"); - ret = 1; - } - } - } - - SCMutexUnlock(&f->m); - return ret ^ ((co->flags & DETECT_CONTENT_NEGATED) ? 1 : 0); - -end: - SCMutexUnlock(&f->m); - SCLogDebug("released lock %p", &f->m); - SCReturnInt(ret); -} - /** * \brief this function clears the memory of http_cookie modifier keyword * @@ -208,15 +110,15 @@ static int DetectHttpCookieSetup (DetectEngineCtx *de_ctx, Signature *s, char *s } if (s->sm_lists_tail[DETECT_SM_LIST_PMATCH] == NULL) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_cookie found inside the " - "rule, without any preceding content keywords"); + SCLogError(SC_ERR_HTTP_COOKIE_NEEDS_PRECEEDING_CONTENT, "http_cookie " + "found inside the rule, without any preceding content keywords"); return -1; } sm = DetectContentGetLastPattern(s->sm_lists_tail[DETECT_SM_LIST_PMATCH]); if (sm == NULL) { - SCLogWarning(SC_ERR_INVALID_SIGNATURE, "http_cookie found inside " - "the rule, without a content context. Please use a " + SCLogWarning(SC_ERR_HTTP_COOKIE_NEEDS_PRECEEDING_CONTENT, "http_cookie " + "found inside the rule, without a content context. Please use a " "content keyword before using http_cookie"); return -1; } @@ -225,15 +127,14 @@ static int DetectHttpCookieSetup (DetectEngineCtx *de_ctx, Signature *s, char *s /* http_cookie should not be used with the rawbytes rule */ if (cd->flags & DETECT_CONTENT_RAWBYTES) { - - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_cookie rule can not " - "be used with the rawbytes rule keyword"); + SCLogError(SC_ERR_HTTP_COOKIE_INCOMPATIBLE_WITH_RAWBYTES, "http_cookie " + "rule can not be used with the rawbytes rule keyword"); return -1; } if (s->alproto != ALPROTO_UNKNOWN && s->alproto != ALPROTO_HTTP) { - SCLogError(SC_ERR_CONFLICTING_RULE_KEYWORDS, "rule contains a non http " - "alproto set"); + SCLogError(SC_ERR_CONFLICTING_RULE_KEYWORDS, "rule contains keywords" + "that conflict with http_cookie"); goto error; } @@ -259,9 +160,9 @@ static int DetectHttpCookieSetup (DetectEngineCtx *de_ctx, Signature *s, char *s DETECT_PCRE, s->sm_lists_tail[DETECT_SM_LIST_HCDMATCH]); if (pm == NULL) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_cookie seen with a " - "distance or within without a previous http_cookie " - "content. Invalidating signature."); + SCLogError(SC_ERR_HTTP_COOKIE_RELATIVE_MISSING, "http_cookie with " + "a distance or within requires preceeding http_cookie " + "content, but none was found"); goto error; } if (pm->type == DETECT_PCRE) { diff --git a/src/detect-http-method.c b/src/detect-http-method.c index 6cc1c27d9a..a306628d51 100644 --- a/src/detect-http-method.c +++ b/src/detect-http-method.c @@ -53,8 +53,6 @@ #include "stream-tcp.h" -int DetectHttpMethodMatch(ThreadVars *, DetectEngineThreadCtx *, - Flow *, uint8_t, void *, Signature *, SigMatch *); static int DetectHttpMethodSetup(DetectEngineCtx *, Signature *, char *); void DetectHttpMethodRegisterTests(void); void DetectHttpMethodFree(void *); @@ -65,7 +63,6 @@ void DetectHttpMethodFree(void *); void DetectHttpMethodRegister(void) { sigmatch_table[DETECT_AL_HTTP_METHOD].name = "http_method"; sigmatch_table[DETECT_AL_HTTP_METHOD].Match = NULL; - //sigmatch_table[DETECT_AL_HTTP_METHOD].AppLayerMatch = DetectHttpMethodMatch; sigmatch_table[DETECT_AL_HTTP_METHOD].AppLayerMatch = NULL; sigmatch_table[DETECT_AL_HTTP_METHOD].alproto = ALPROTO_HTTP; sigmatch_table[DETECT_AL_HTTP_METHOD].Setup = DetectHttpMethodSetup; @@ -76,71 +73,6 @@ void DetectHttpMethodRegister(void) { SCLogDebug("registering http_method rule option"); } -/** - * \brief match the specified version on a tls session - * - * \param t pointer to thread vars - * \param det_ctx pointer to the pattern matcher thread - * \param f pointer to the current flow - * \param flags flags to indicate the direction of the received packet - * \param state pointer the app layer state, which will cast into HtpState - * \param sm pointer to the sigmatch - * - * \retval 0 no match - * \retval 1 match - */ -int DetectHttpMethodMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx, - Flow *f, uint8_t flags, void *state, - Signature *s, SigMatch *sm) -{ - SCEnter(); - - size_t idx; - DetectContentData *data = (DetectContentData *)sm->ctx; - HtpState *hs = (HtpState *)state; - htp_tx_t *tx = NULL; - int ret = 0; - - if (hs == NULL || hs->connp == NULL || hs->connp->conn == NULL) { - SCLogDebug("No HTP state."); - SCReturnInt(0); - } - - SCMutexLock(&f->m); - for (idx = 0;//hs->new_in_tx_index; - idx < list_size(hs->connp->conn->transactions); idx++) - { - tx = list_get(hs->connp->conn->transactions, idx); - if (tx == NULL || tx->request_method == NULL) - continue; - - const uint8_t *meth_str = (const uint8_t *)bstr_ptr(tx->request_method); - if (meth_str != NULL) { - /* - printf("Method buffer: ");PrintRawUriFp(stdout, meth_str, bstr_size(tx->request_method));printf("\n"); - printf("Pattern: ");PrintRawUriFp(stdout, data->content, data->content_len);printf("\n"); - */ - - if (data->flags & DETECT_CONTENT_NOCASE) { - SCLogDebug("no case inspection"); - ret = (SpmNocaseSearch((uint8_t *)meth_str, bstr_size(tx->request_method), - data->content, data->content_len) != NULL); - } else { - SCLogDebug("case sensitive inspection"); - ret = (SpmSearch((uint8_t *)meth_str, bstr_size(tx->request_method), - data->content, data->content_len) != NULL); - } - if (ret == 1) { - SCLogDebug("matched HTTP method."); - break; - } - } - } - - SCMutexUnlock(&f->m); - SCReturnInt(ret ^ ((data->flags & DETECT_CONTENT_NEGATED) ? 1 : 0)); -} - /** * \brief This function is used to add the parsed "http_method" option * into the current signature. @@ -163,29 +95,29 @@ static int DetectHttpMethodSetup(DetectEngineCtx *de_ctx, Signature *s, char *st } if (s->sm_lists_tail[DETECT_SM_LIST_PMATCH] == NULL) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method modifier used " - "before any signature match"); + SCLogError(SC_ERR_HTTP_METHOD_NEEDS_PRECEEDING_CONTENT, "http_method " + "modifies preceeding \"content\", but none was found"); SCReturnInt(-1); } SigMatch *sm = DetectContentGetLastPattern(s->sm_lists_tail[DETECT_SM_LIST_PMATCH]); if (sm == NULL) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method modifies \"content\", " - "but none was found"); + SCLogError(SC_ERR_HTTP_METHOD_NEEDS_PRECEEDING_CONTENT, "http_method " + "modifies preceeding \"content\", but none was found"); SCReturnInt(-1); } cd = (DetectContentData *)sm->ctx; if (cd->flags & DETECT_CONTENT_RAWBYTES) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method cannot be used " - "with \"rawbytes\""); + SCLogError(SC_ERR_HTTP_METHOD_INCOMPATIBLE_WITH_RAWBYTES, "http_method " + "cannot be used with \"rawbytes\""); SCReturnInt(-1); } if (s->alproto != ALPROTO_UNKNOWN && s->alproto != ALPROTO_HTTP) { - SCLogError(SC_ERR_CONFLICTING_RULE_KEYWORDS, "rule contains a non http " - "alproto set"); + SCLogError(SC_ERR_CONFLICTING_RULE_KEYWORDS, "rule contains keywords" + "that conflict with http_method"); goto error; } @@ -211,9 +143,9 @@ static int DetectHttpMethodSetup(DetectEngineCtx *de_ctx, Signature *s, char *st DETECT_PCRE, s->sm_lists_tail[DETECT_SM_LIST_HMDMATCH]); if (pm == NULL) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method seen with a " - "distance or within without a previous http_method " - "content. Invalidating signature."); + SCLogError(SC_ERR_HTTP_METHOD_RELATIVE_MISSING, "http_method with " + "a distance or within requires preceeding http_method " + "content, but none was found"); goto error; } if (pm->type == DETECT_PCRE) { @@ -252,7 +184,8 @@ error: void DetectHttpMethodFree(void *ptr) { DetectContentData *data = (DetectContentData *)ptr; - if (data->content != NULL) SCFree(data->content); + if (data->content != NULL) + SCFree(data->content); SCFree(data); } diff --git a/src/detect.c b/src/detect.c index 959fc7659f..e109837c54 100644 --- a/src/detect.c +++ b/src/detect.c @@ -8049,11 +8049,7 @@ int SigTest40NoPayloadInspection02(void) { goto end; } -// sigmatch_table[DETECT_CONTENT].flags |= SIGMATCH_PAYLOAD; -// de_ctx->sig_list->sm_lists[DETECT_SM_LIST_PMATCH]->type = DETECT_CONTENT; - SigGroupBuild(de_ctx); - //PatternMatchPrepare(mpm_ctx,MPM_B2G); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); if (!(de_ctx->sig_list->init_flags & SIG_FLAG_PAYLOAD)) @@ -8072,7 +8068,6 @@ int SigTest40NoPayloadInspection02(void) { SigGroupCleanup(de_ctx); SigCleanSignatures(de_ctx); DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); - //PatternMatchDestroy(mpm_ctx); DetectEngineCtxFree(de_ctx); end: SCFree(p); diff --git a/src/util-error.c b/src/util-error.c index 67f870adba..d24d6ea742 100644 --- a/src/util-error.c +++ b/src/util-error.c @@ -194,6 +194,12 @@ const char * SCErrorToString(SCError err) CASE_CODE (SC_ERR_REFERENCE_CONFIG); CASE_CODE (SC_WARN_PCAP_MULTI_DEV_EXPERIMENTAL); CASE_CODE (SC_ERR_PCAP_MULTI_DEV_NO_SUPPORT); + CASE_CODE (SC_ERR_HTTP_METHOD_NEEDS_PRECEEDING_CONTENT); + CASE_CODE (SC_ERR_HTTP_METHOD_INCOMPATIBLE_WITH_RAWBYTES); + CASE_CODE (SC_ERR_HTTP_METHOD_RELATIVE_MISSING); + CASE_CODE (SC_ERR_HTTP_COOKIE_NEEDS_PRECEEDING_CONTENT); + CASE_CODE (SC_ERR_HTTP_COOKIE_INCOMPATIBLE_WITH_RAWBYTES); + CASE_CODE (SC_ERR_HTTP_COOKIE_RELATIVE_MISSING); default: return "UNKNOWN_ERROR"; diff --git a/src/util-error.h b/src/util-error.h index 4cca5d3395..9752f10c22 100644 --- a/src/util-error.h +++ b/src/util-error.h @@ -205,6 +205,12 @@ typedef enum { SC_ERR_REFERENCE_CONFIG, SC_WARN_PCAP_MULTI_DEV_EXPERIMENTAL, SC_ERR_PCAP_MULTI_DEV_NO_SUPPORT, + SC_ERR_HTTP_METHOD_NEEDS_PRECEEDING_CONTENT, + SC_ERR_HTTP_METHOD_INCOMPATIBLE_WITH_RAWBYTES, + SC_ERR_HTTP_METHOD_RELATIVE_MISSING, + SC_ERR_HTTP_COOKIE_NEEDS_PRECEEDING_CONTENT, + SC_ERR_HTTP_COOKIE_INCOMPATIBLE_WITH_RAWBYTES, + SC_ERR_HTTP_COOKIE_RELATIVE_MISSING, } SCError; const char *SCErrorToString(SCError);