From ffc5d968e694c0d1451d91fbec80f4a894fa6290 Mon Sep 17 00:00:00 2001 From: b1v1r Date: Thu, 21 May 2009 06:18:18 +0000 Subject: [PATCH] Merge 2.5.x changes into trunk. --- CHANGES | 4 +- apache2/re.c | 23 ++++++++- apache2/re_actions.c | 2 + apache2/t/regression/rule/00-basics.t | 67 +++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 08943466..952b30ab 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ -22 Apr 2009 - trunk +20 May 2009 - trunk ------------------- + * Correctly resolve chained rule actions in logs. + * Cleanup some code for portability. * AIX does not support hidden visibility with xlc compiler. diff --git a/apache2/re.c b/apache2/re.c index 620967c6..ce219745 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -68,9 +68,12 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions const apr_array_header_t *tarr = NULL; const apr_table_entry_t *telts = NULL; char *actions = NULL; + int chain; int i; - if (actionset == NULL) return apr_pstrdup(pool, ""); + if (actionset == NULL) return NULL; + + chain = ((actionset->rule != NOT_SET_P) && actionset->rule->chain_starter) ? 1 : 0; tarr = apr_table_elts(actionset->actions); telts = (const apr_table_entry_t*)tarr->elts; @@ -79,6 +82,22 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions msre_action *action = (msre_action *)telts[i].val; int use_quotes = 0; + if (chain) { + /* Skip some actions that are not used in a chain. */ + if ( (action->metadata->type == ACTION_DISRUPTIVE) + || (action->metadata->type == ACTION_METADATA) + || (strcmp("log", action->metadata->name) == 0) + || (strcmp("auditlog", action->metadata->name) == 0) + || (strcmp("nolog", action->metadata->name) == 0) + || (strcmp("noauditlog", action->metadata->name) == 0) + || (strcmp("severity", action->metadata->name) == 0) + || (strcmp("tag", action->metadata->name) == 0) + || (strcmp("phase", action->metadata->name) == 0)) + { + continue; + } + } + /* Check if we need any quotes */ if (action->param != NULL) { int j; @@ -102,7 +121,7 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions NULL); } - return (actions == NULL) ? apr_pstrdup(pool, "") : actions; + return actions; } /** diff --git a/apache2/re_actions.c b/apache2/re_actions.c index b15f568c..e1909953 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -2120,6 +2120,7 @@ void msre_engine_register_default_actions(msre_engine *engine) { ); /* phase */ + /* ENH: This should be ACTION_NON_DISRUPTIVE or ACTION_FLOW??? */ msre_engine_action_register(engine, "phase", ACTION_DISRUPTIVE, @@ -2354,6 +2355,7 @@ void msre_engine_register_default_actions(msre_engine *engine) { ); /* tag */ + /* ENH: This should be ACTION_METADATA??? */ msre_engine_action_register(engine, "tag", ACTION_NON_DISRUPTIVE, diff --git a/apache2/t/regression/rule/00-basics.t b/apache2/t/regression/rule/00-basics.t index 396d0ea1..295bbc99 100644 --- a/apache2/t/regression/rule/00-basics.t +++ b/apache2/t/regression/rule/00-basics.t @@ -22,3 +22,70 @@ GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", ), }, + +# SecRule +{ + type => "rule", + comment => "SecRule (no action)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 5 + SecDefaultAction "phase:2,deny,status:403" + SecRule ARGS:test "value" + ), + match_log => { + error => [ qr/ModSecurity: /, 1 ], + debug => [ qr/Rule [0-9a-f]+: SecRule "ARGS:test" "\@rx value" "phase:2,deny,status:403"$/m, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?test=value", + ), +}, +{ + type => "rule", + comment => "SecRule (action)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 5 + SecDefaultAction "phase:2,pass" + SecRule ARGS:test "value" "deny,status:403" + ), + match_log => { + error => [ qr/ModSecurity: /, 1 ], + debug => [ qr/Rule [0-9a-f]+: SecRule "ARGS:test" "\@rx value" "phase:2,deny,status:403"$/m, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?test=value", + ), +}, +{ + type => "rule", + comment => "SecRule (chain)", + conf => qq( + SecRuleEngine On + SecDebugLog $ENV{DEBUG_LOG} + SecDebugLogLevel 5 + SecDefaultAction "phase:2,log,noauditlog,pass,tag:foo" + SecRule ARGS:test "value" "chain,phase:2,deny,status:403" + SecRule &ARGS "\@eq 1" "chain,setenv:tx.foo=bar" + SecRule REQUEST_METHOD "\@streq GET" + ), + match_log => { + error => [ qr/ModSecurity: /, 1 ], + debug => [ qr/Rule [0-9a-f]+: SecRule "ARGS:test" "\@rx value" "phase:2,log,noauditlog,tag:foo,chain,deny,status:403"\r?\n.*Rule [0-9a-f]+: SecRule "&ARGS" "\@eq 1" "chain,setenv:tx.foo=bar"\r?\n.*Rule [0-9a-f]+: SecRule "REQUEST_METHOD" "\@streq GET"\r?\n/s, 1 ], + }, + match_response => { + status => qr/^403$/, + }, + request => new HTTP::Request( + GET => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt?test=value", + ), +},