From 34798e9abe548b885f9f71c18b515c3060c5f33e Mon Sep 17 00:00:00 2001 From: brectanus Date: Wed, 3 Sep 2008 20:42:28 +0000 Subject: [PATCH] Allow ability to force request body buffering to memory. Fixes MODSEC-2. --- CHANGES | 8 +++++++- apache2/apache2_config.c | 4 ++++ apache2/modsecurity.c | 6 ++++++ apache2/modsecurity.h | 1 + apache2/msc_reqbody.c | 24 +++++++++++++++++++++++- apache2/msc_test.c | 1 + apache2/re_actions.c | 17 +++++++++++++++++ doc/modsecurity2-apache-reference.xml | 11 ++++++++++- 8 files changed, 69 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 9abf19e3..3458b75b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,7 +1,13 @@ 03 Sep 2008 - trunk ------------------- - * Integrate mlogc source. + * Added ctl:requestBodyBuffering=on|off which, when enabled, will force + the request body to be buffered and allow REQUEST_BODY to be inspected. + Previously the REQUEST_BODY target was only populated if the request body + was a parsable type (application/x-www-form-urlencoded or + multipart/form-data) or was forced to be parsed via ctl:requestBodyProcessor. + + * Integrated mlogc source. * Fixed logging the hostname in the error_log which was logging the request hostname instead of the Apache resolved hostname. diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index 7e54f409..e404dbff 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -45,6 +45,7 @@ void *create_directory_config(apr_pool_t *mp, char *path) { dcfg->is_enabled = NOT_SET; dcfg->reqbody_access = NOT_SET; + dcfg->reqbody_buffering = NOT_SET; dcfg->reqbody_inmemory_limit = NOT_SET; dcfg->reqbody_limit = NOT_SET; dcfg->reqbody_no_files_limit = NOT_SET; @@ -236,6 +237,8 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) { /* IO parameters */ merged->reqbody_access = (child->reqbody_access == NOT_SET ? parent->reqbody_access : child->reqbody_access); + merged->reqbody_buffering = (child->reqbody_buffering == NOT_SET + ? parent->reqbody_buffering : child->reqbody_buffering); merged->reqbody_inmemory_limit = (child->reqbody_inmemory_limit == NOT_SET ? parent->reqbody_inmemory_limit : child->reqbody_inmemory_limit); merged->reqbody_limit = (child->reqbody_limit == NOT_SET @@ -480,6 +483,7 @@ void init_directory_config(directory_config *dcfg) { if (dcfg->is_enabled == NOT_SET) dcfg->is_enabled = 0; if (dcfg->reqbody_access == NOT_SET) dcfg->reqbody_access = 0; + if (dcfg->reqbody_buffering == NOT_SET) dcfg->reqbody_buffering = 0; if (dcfg->reqbody_inmemory_limit == NOT_SET) dcfg->reqbody_inmemory_limit = REQUEST_BODY_DEFAULT_INMEMORY_LIMIT; if (dcfg->reqbody_limit == NOT_SET) dcfg->reqbody_limit = REQUEST_BODY_DEFAULT_LIMIT; diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c index 071ccd23..fa8f9378 100644 --- a/apache2/modsecurity.c +++ b/apache2/modsecurity.c @@ -259,6 +259,12 @@ apr_status_t modsecurity_tx_init(modsec_rec *msr) { } } + /* Check if we are forcing buffering, then use memory only. */ + if (msr->txcfg->reqbody_buffering) { + msr->msc_reqbody_storage = MSC_REQBODY_MEMORY; + msr->msc_reqbody_spilltodisk = 0; + } + /* Initialise arguments */ msr->arguments = apr_table_make(msr->mp, 32); if (msr->arguments == NULL) return -1; diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index a745db69..7cfdd5ea 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -362,6 +362,7 @@ struct directory_config { int is_enabled; int reqbody_access; + int reqbody_buffering; long int reqbody_inmemory_limit; long int reqbody_limit; long int reqbody_no_files_limit; diff --git a/apache2/msc_reqbody.c b/apache2/msc_reqbody.c index 26aed3a3..97449c2e 100644 --- a/apache2/msc_reqbody.c +++ b/apache2/msc_reqbody.c @@ -309,6 +309,9 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, msr->msc_reqbody_processor); return -1; } + } else if (msr->txcfg->reqbody_buffering) { + /* Increase per-request data length counter if forcing buffering. */ + msr->msc_reqbody_no_files_length += length; } /* Check that we are not over the request body no files limit. */ @@ -334,7 +337,7 @@ apr_status_t modsecurity_request_body_store(modsec_rec *msr, /** * */ -static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr, char **error_msg) { +static apr_status_t modsecurity_request_body_end_raw(modsec_rec *msr, char **error_msg) { msc_data_chunk **chunks, *one_chunk; char *d; int i, sofar; @@ -394,6 +397,22 @@ static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr, cha one_chunk->is_permanent = 1; *(const msc_data_chunk **)apr_array_push(msr->msc_reqbody_chunks) = one_chunk; + return 1; +} + +/** + * + */ +static apr_status_t modsecurity_request_body_end_urlencoded(modsec_rec *msr, char **error_msg) { + int invalid_count = 0; + + *error_msg = NULL; + + /* Create the raw buffer */ + if (modsecurity_request_body_end_raw(msr, error_msg) != 1) { + return -1; + } + /* Parse URL-encoded arguments in the request body. */ if (parse_arguments(msr, msr->msc_reqbody_buffer, msr->msc_reqbody_length, @@ -458,6 +477,9 @@ apr_status_t modsecurity_request_body_end(modsec_rec *msr, char **error_msg) { return -1; } } + } else if (msr->txcfg->reqbody_buffering) { + /* No processing if there is no processor and forcing buffering. */ + return modsecurity_request_body_end_raw(msr, error_msg); } /* Note the request body no files length. */ diff --git a/apache2/msc_test.c b/apache2/msc_test.c index e526d650..947fc6d4 100644 --- a/apache2/msc_test.c +++ b/apache2/msc_test.c @@ -458,6 +458,7 @@ static void init_msr() { dcfg = (directory_config *)apr_pcalloc(g_mp, sizeof(directory_config)); dcfg->is_enabled = 0; dcfg->reqbody_access = 0; + dcfg->reqbody_buffering = 0; dcfg->reqbody_inmemory_limit = REQUEST_BODY_DEFAULT_INMEMORY_LIMIT; dcfg->reqbody_limit = REQUEST_BODY_DEFAULT_LIMIT; dcfg->reqbody_no_files_limit = REQUEST_BODY_NO_FILES_DEFAULT_LIMIT; diff --git a/apache2/re_actions.c b/apache2/re_actions.c index d753e50c..c791d2e5 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -713,6 +713,13 @@ static char *msre_action_ctl_validate(msre_engine *engine, msre_action *action) */ return NULL; } else + if (strcasecmp(name, "requestBodyBuffering") == 0) { + if (parse_boolean(value) == -1) { + return apr_psprintf(engine->mp, "Invalid setting for ctl name " + " requestBodyBuffering: %s", value); + } + return NULL; + } else if (strcasecmp(name, "responseBodyAccess") == 0) { if (parse_boolean(value) == -1) { return apr_psprintf(engine->mp, "Invalid setting for ctl name " @@ -831,6 +838,16 @@ static apr_status_t msre_action_ctl_execute(modsec_rec *msr, apr_pool_t *mptmp, return 1; } else + if (strcasecmp(name, "requestBodyBuffering") == 0) { + int pv = parse_boolean(value); + + if (pv == -1) return -1; + msr->txcfg->reqbody_buffering = pv; + msr->usercfg->reqbody_buffering = pv; + msr_log(msr, 4, "Ctl: Set requestBodyAccess to %d.", pv); + + return 1; + } else if (strcasecmp(name, "requestBodyProcessor") == 0) { msr->msc_reqbody_processor = value; msr_log(msr, 4, "Ctl: Set requestBodyProcessor to %s.", value); diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml index 2c242802..dea4eb40 100644 --- a/doc/modsecurity2-apache-reference.xml +++ b/doc/modsecurity2-apache-reference.xml @@ -4406,6 +4406,10 @@ SecRule REQUEST_CONTENT_TYPE ^text/xml nolog,pass,ctl:requestBodyProce requestBodyAccess + + requestBodyBuffering + + requestBodyLimit @@ -4428,7 +4432,8 @@ SecRule REQUEST_CONTENT_TYPE ^text/xml nolog,pass,ctl:requestBodyProce With the exception of - requestBodyProcessor, each configuration option corresponds to + requestBodyProcessor and + requestBodyBuffering, each configuration option corresponds to one configuration directive and the usage is identical. The requestBodyProcessor option allows you to configure the @@ -4450,6 +4455,10 @@ SecRule REQUEST_CONTENT_TYPE ^text/xml nolog,pass,ctl:requestBodyProce should be inspected in the REQUEST_BODY phase and an appropriate action taken. + + The requestBodyBuffering option allows you to configure the + request body to be buffered (in memory) even if it is not parsed. This + allows inspection of REQUEST_BODY even when no parser is used.