From 6ebe7b7cd3c28982a15dc4651e88e2e5d37f8f55 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 10 Dec 2010 17:00:18 +0100 Subject: [PATCH] Change the way the request body limit is enforced. --- src/app-layer-htp.c | 182 ++++++++++++++++++++++++++------------- src/app-layer-htp.h | 10 ++- src/detect-engine-hcbd.c | 15 ++-- src/detect-engine.c | 12 --- suricata.yaml | 18 ++-- 5 files changed, 147 insertions(+), 90 deletions(-) diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 57c6c525ba..7a3836ff95 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -61,11 +61,13 @@ #include "conf.h" /** Need a linked list in order to keep track of these */ -typedef struct HTPCfgRec_ HTPCfgRec; -struct HTPCfgRec_ { - htp_cfg_t *cfg; - HTPCfgRec *next; -}; +typedef struct HTPCfgRec_ { + htp_cfg_t *cfg; + struct HTPCfgRec_ *next; + + /** max size of the client body we inspect */ + uint32_t request_body_limit; +} HTPCfgRec; /** Fast lookup tree (radix) for the various HTP configurations */ static SCRadixTree *cfgtree; @@ -352,8 +354,12 @@ static int HTPHandleRequestData(Flow *f, void *htp_state, htp = htp_cfg_rec->cfg; SCLogDebug("LIBHTP using config: %p", htp); } + + hstate->request_body_limit = htp_cfg_rec->request_body_limit; } else { SCLogDebug("Using default HTP config: %p", htp); + + hstate->request_body_limit = cfglist.request_body_limit; } if (NULL == htp) { @@ -537,27 +543,28 @@ static int HTPHandleResponseData(Flow *f, void *htp_state, * \param body pointer to the HtpBody holding the list * \param data pointer to the data of the chunk * \param len length of the chunk pointed by data - * \retval none + * \retval 0 ok + * \retval -1 error */ -void HtpBodyAppendChunk(SCHtpTxUserData *htud, HtpBody *body, uint8_t *data, uint32_t len) +int HtpBodyAppendChunk(SCHtpTxUserData *htud, HtpBody *body, uint8_t *data, uint32_t len) { SCEnter(); HtpBodyChunk *bd = NULL; - if (len == 0 || data == NULL) - goto error; + if (len == 0 || data == NULL) { + SCReturnInt(0); + } if (body->nchunks == 0) { /* New chunk */ bd = (HtpBodyChunk *)SCMalloc(sizeof(HtpBodyChunk)); if (bd == NULL) - return; + goto error; bd->len = len; bd->data = SCMalloc(len); if (bd->data == NULL) { - SCLogError(SC_ERR_MEM_ALLOC, "malloc failed: %s", strerror(errno)); goto error; } @@ -568,51 +575,28 @@ void HtpBodyAppendChunk(SCHtpTxUserData *htud, HtpBody *body, uint8_t *data, uin bd->next = NULL; bd->id = body->nchunks; } else { - /* New or old, we have to check it.. */ - if (body->last->data == data) { - /* Weird, but sometimes htp lib calls the callback - * more than once for the same chunk, with more - * len, so updating the len */ - if (body->last->len > len) - htud->content_len_so_far -= (body->last->len - len); - else - htud->content_len_so_far += (len - body->last->len); - - body->last->len = len; - bd = body->last; - - bd->data = SCRealloc(bd->data, len); - if (bd->data == NULL) { - SCLogError(SC_ERR_MEM_ALLOC, "realloc failed: %s", strerror(errno)); - goto error; - } - - memcpy(bd->data, data, len); - } else { - bd = (HtpBodyChunk *)SCMalloc(sizeof(HtpBodyChunk)); - if (bd == NULL) - return; - - bd->len = len; - bd->data = SCMalloc(len); - if (bd->data == NULL) { - SCLogError(SC_ERR_MEM_ALLOC, "malloc failed: %s", strerror(errno)); - goto error; - } + bd = (HtpBodyChunk *)SCMalloc(sizeof(HtpBodyChunk)); + if (bd == NULL) + goto error; - memcpy(bd->data, data, len); - htud->content_len_so_far += len; - body->last->next = bd; - body->last = bd; - body->nchunks++; - bd->next = NULL; - bd->id = body->nchunks; + bd->len = len; + bd->data = SCMalloc(len); + if (bd->data == NULL) { + goto error; } + + memcpy(bd->data, data, len); + htud->content_len_so_far += len; + body->last->next = bd; + body->last = bd; + body->nchunks++; + bd->next = NULL; + bd->id = body->nchunks; } SCLogDebug("Body %p; Chunk id: %"PRIu32", data %p, len %"PRIu32"", body, bd->id, bd->data, (uint32_t)bd->len); - SCReturn; + SCReturnInt(0); error: if (bd != NULL) { @@ -621,7 +605,7 @@ error: } SCFree(bd->data); } - SCReturn; + SCReturnInt(-1); } /** @@ -698,7 +682,6 @@ int HTPCallbackRequestBodyData(htp_tx_data_t *d) if (htud == NULL) { htud = SCMalloc(sizeof(SCHtpTxUserData)); if (htud == NULL) { - SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory"); SCReturnInt(HOOK_OK); } memset(htud, 0, sizeof(SCHtpTxUserData)); @@ -715,14 +698,39 @@ int HTPCallbackRequestBodyData(htp_tx_data_t *d) htud->body.operation = HTP_BODY_REQUEST; - HtpBodyAppendChunk(htud, &htud->body, (uint8_t*)d->data, (uint32_t)d->len); - htud->body.pcre_flags = HTP_PCRE_NONE; - if (SCLogDebugEnabled()) { - HtpBodyPrint(&htud->body); - } + SCLogDebug("htud->content_len_so_far %u", htud->content_len_so_far); + SCLogDebug("hstate->request_body_limit %u", hstate->request_body_limit); + + /* within limits, add the body chunk to the state. */ + if (htud->content_len_so_far < hstate->request_body_limit) + { + uint32_t len = (uint32_t)d->len; + + if ((htud->content_len_so_far + len) > hstate->request_body_limit) { + len = hstate->request_body_limit - htud->content_len_so_far; + BUG_ON(len > (uint32_t)d->len); + } - /* set the new chunk flag */ - hstate->flags |= HTP_FLAG_NEW_BODY_SET; + SCLogDebug("len %u", len); + + int r = HtpBodyAppendChunk(htud, &htud->body, (uint8_t*)d->data, len); + if (r < 0) { + htud->flags |= HTP_BODY_COMPLETE; + } else if (htud->content_len_so_far >= hstate->request_body_limit) { + htud->flags |= HTP_BODY_COMPLETE; + } else if (htud->content_len_so_far == htud->content_len) { + htud->flags |= HTP_BODY_COMPLETE; + } + + htud->body.pcre_flags = HTP_PCRE_NONE; + + /* set the new chunk flag */ + hstate->flags |= HTP_FLAG_NEW_BODY_SET; + + //if (SCLogDebugEnabled()) { + // HtpBodyPrint(&htud->body); + //} + } SCReturnInt(HOOK_OK); } @@ -857,6 +865,7 @@ static void HTPConfigure(void) SCLogDebug("LIBHTP default config: %p", cfglist.cfg); + cfglist.request_body_limit = HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT; htp_config_register_request(cfglist.cfg, HTPCallbackRequest); htp_config_register_response(cfglist.cfg, HTPCallbackResponse); htp_config_set_generate_request_uri_normalized(cfglist.cfg, 1); @@ -897,11 +906,35 @@ static void HTPConfigure(void) continue; } + } + } else if (strcasecmp("client-request-body-buffer-limit", p->name) == 0) { + /* limit */ + TAILQ_FOREACH(pval, &p->head, next) { + SCLogDebug("LIBHTP default: %s=%s", + p->name, pval->val); + + int limit = atoi(pval->val); + + if (limit >= 0) { + SCLogDebug("LIBHTP default: %s=%s (%d)", + p->name, pval->val, limit); + + cfglist.request_body_limit = (uint32_t)limit; + } + else { + SCLogWarning(SC_ERR_UNKNOWN_VALUE, + "LIBHTP malformed client-request-body-buffer-limit " + "\"%s\", using default %u", pval->val, + HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT); + cfglist.request_body_limit = HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT; + continue; + } + } } else { SCLogWarning(SC_ERR_UNKNOWN_VALUE, - "LIBHTP Ignoring unknown default config: %s", - p->name); + "LIBHTP Ignoring unknown default config: %s", + p->name); } } } @@ -953,6 +986,7 @@ static void HTPConfigure(void) } } + htprec->request_body_limit = HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT; htp_config_register_request(htp, HTPCallbackRequest); htp_config_register_response(htp, HTPCallbackResponse); htp_config_set_generate_request_uri_normalized(htp, 1); @@ -1021,6 +1055,30 @@ static void HTPConfigure(void) continue; } + } + } else if (strcasecmp("client-request-body-buffer-limit", p->name) == 0) { + /* limit */ + TAILQ_FOREACH(pval, &p->head, next) { + SCLogDebug("LIBHTP default: %s=%s", + p->name, pval->val); + + int limit = atoi(pval->val); + + if (limit >= 0) { + SCLogDebug("LIBHTP default: %s=%s (%d)", + p->name, pval->val, limit); + + htprec->request_body_limit = (uint32_t)limit; + } + else { + SCLogWarning(SC_ERR_UNKNOWN_VALUE, + "LIBHTP malformed client-request-body-buffer-limit " + "\"%s\", using default %u", pval->val, + HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT); + htprec->request_body_limit = HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT; + continue; + } + } } else { SCLogWarning(SC_ERR_UNKNOWN_VALUE, @@ -1038,6 +1096,7 @@ static void HtpConfigCreateBackup(void) { cfglist_backup.cfg = cfglist.cfg; cfglist_backup.next = cfglist.next; + cfglist_backup.request_body_limit = cfglist.request_body_limit; return; } @@ -1046,6 +1105,7 @@ static void HtpConfigRestoreBackup(void) { cfglist.cfg = cfglist_backup.cfg; cfglist.next = cfglist_backup.next; + cfglist.request_body_limit = cfglist_backup.request_body_limit; return; } diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index b91ec654c7..5bb0714c25 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -31,6 +31,9 @@ #include +/* default request body limit */ +#define HTP_CONFIG_DEFAULT_REQUEST_BODY_LIMIT 4096U + #define HTP_FLAG_STATE_OPEN 0x01 /**< Flag to indicate that HTTP connection is open */ #define HTP_FLAG_STATE_CLOSED 0x02 /**< Flag to indicate that HTTP @@ -82,6 +85,9 @@ typedef struct HtpBody_ { any pcre (so we can free() without waiting) */ } HtpBody; +#define HTP_BODY_COMPLETE 0x01 /* body is complete or limit is reached, + either way, this is it. */ + /** Now the Body Chunks will be stored per transaction, at * the tx user data */ typedef struct SCHtpTxUserData_ { @@ -91,17 +97,17 @@ typedef struct SCHtpTxUserData_ { uint32_t content_len; /* Holds the length of the htp request body seen so far */ uint32_t content_len_so_far; + uint8_t flags; } SCHtpTxUserData; typedef struct HtpState_ { htp_connp_t *connp; /**< Connection parser structure for each connection */ -// size_t new_in_tx_index; /**< Index to indicate that after this we have -// new requests to log */ uint8_t flags; uint16_t transaction_cnt; uint16_t transaction_done; + uint32_t request_body_limit; } HtpState; void RegisterHTPParsers(void); diff --git a/src/detect-engine-hcbd.c b/src/detect-engine-hcbd.c index 2fb23f9b49..caa9b9b8c6 100644 --- a/src/detect-engine-hcbd.c +++ b/src/detect-engine-hcbd.c @@ -393,25 +393,22 @@ static void DetectEngineBufferHttpClientBodies(DetectEngineCtx *de_ctx, if ((htud->content_len_so_far > 0) && tx->progress != TX_PROGRESS_REQ_BODY) { /* final length of the body */ - htud->content_len = htud->content_len_so_far; + htud->flags |= HTP_BODY_COMPLETE; } } - if (htud->content_len != htud->content_len_so_far) { + /* inspect the body if the transfer is complete or we have hit + * our body size limit */ + if (!(htud->flags & HTP_BODY_COMPLETE)) { SCLogDebug("we still haven't seen the entire request body. " - "Let's defer body inspection till we see the " - "entire body."); + "Let's defer body inspection till we see the " + "entire body."); continue; } uint8_t *chunks_buffer = NULL; int32_t chunks_buffer_len = 0; while (cur != NULL) { - /* \todo Currently we limit the body length we inspect. We - * should change to handling chunks statefully */ - if (chunks_buffer_len > de_ctx->hcbd_buffer_limit) - break; - chunks_buffer_len += cur->len; if ( (chunks_buffer = SCRealloc(chunks_buffer, chunks_buffer_len)) == NULL) { goto end; diff --git a/src/detect-engine.c b/src/detect-engine.c index ebcc6f5e8d..4993288bf6 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -189,7 +189,6 @@ static uint8_t DetectEngineCtxLoadConf(DetectEngineCtx *de_ctx) { const char *max_uniq_toserver_dp_groups_str = NULL; char *sgh_mpm_context = NULL; - char *hcbd_buffer_limit = NULL; ConfNode *de_ctx_custom = ConfGetNode("detect-engine"); ConfNode *opt = NULL; @@ -200,9 +199,6 @@ static uint8_t DetectEngineCtxLoadConf(DetectEngineCtx *de_ctx) { de_ctx_profile = opt->head.tqh_first->val; } else if (strcmp(opt->val, "sgh-mpm-context") == 0) { sgh_mpm_context = opt->head.tqh_first->val; - } else if (strcmp(opt->val, - "http-client-request-body-buffer-limit") == 0) { - hcbd_buffer_limit = opt->head.tqh_first->val; } } } @@ -244,14 +240,6 @@ static uint8_t DetectEngineCtxLoadConf(DetectEngineCtx *de_ctx) { } } - if (hcbd_buffer_limit == NULL) { - de_ctx->hcbd_buffer_limit = ENGINE_HCBD_BUFFER_LIMIT; - } else { - de_ctx->hcbd_buffer_limit = atoi(hcbd_buffer_limit); - if (de_ctx->hcbd_buffer_limit <= 0) - de_ctx->hcbd_buffer_limit = ENGINE_HCBD_BUFFER_LIMIT; - } - opt = NULL; switch (profile) { case ENGINE_PROFILE_LOW: diff --git a/suricata.yaml b/suricata.yaml index 9538723080..ac89627623 100644 --- a/suricata.yaml +++ b/suricata.yaml @@ -126,7 +126,6 @@ detect-engine: toserver_dp_groups: 25 - sgh-mpm-context: auto - inspection-recursion-limit: 3000 - - http-client-request-body-buffer-limit: 20000 # Suricata is multi-threaded. Here the threading can be influenced. threading: @@ -518,12 +517,16 @@ asn1_max_frames: 256 # Configure libhtp. # # -# default-config: Used when no server-config matches -# personality: List of personalities used by default +# default-config: Used when no server-config matches +# personality: List of personalities used by default +# request-body-limit: Limit reassembly of request body for inspection +# by http_client_body & pcre /P option. # -# server-config: List of server configurations to use if address matches -# address: List of ip addresses or networks for this block -# personalitiy: List of personalities used by this block +# server-config: List of server configurations to use if address matches +# address: List of ip addresses or networks for this block +# personalitiy: List of personalities used by this block +# request-body-limit: Limit reassembly of request body for inspection +# by http_client_body & pcre /P option. # # Currently Available Personalities: # Minimal @@ -542,18 +545,21 @@ libhtp: default-config: personality: IDS + request-body-limit: 3072 server-config: - apache: address: [192.168.1.0/24, 127.0.0.0/8, "::1"] personality: Apache_2_2 + request-body-limit: 4096 - iis7: address: - 192.168.0.0/24 - 192.168.10.0/24 personality: IIS_7_0 + request-body-limit: 4096 # rule profiling settings. Only effective if Suricata has been built with the # the --enable-profiling configure flag.