diff --git a/CHANGES b/CHANGES index 3930de8d..0cd6e49d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,8 +1,14 @@ -14 Dec 2007 - 2.5.0-rc1 + +17 Dec 2007 - 2.5.0-rc1 ----------------------- Changes since 2.5.0-dev2: + * Changed how allow works. Used on its own it now allows phases 1-4. Used + with parameter "phase" (e.g. SecAction allow:phase) it only affects + the current phase. Used with parameter "request" it allows phases + 1-2. + * Fixed issue where only the first phase 5 rule would run when the request was intercepted in an earlier phase. diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index f5a7fe41..f3bcc277 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -595,7 +595,12 @@ static const char *add_rule(cmd_parms *cmd, directory_config *dcfg, const char * rule->actionset, 1); /* Must NOT specify a disruptive action in logging phase. */ - if ((rule->actionset != NULL) && (rule->actionset->phase == PHASE_LOGGING) && (rule->actionset->intercept_action != ACTION_ALLOW) && (rule->actionset->intercept_action != ACTION_NONE)) { + if ((rule->actionset != NULL) + && (rule->actionset->phase == PHASE_LOGGING) + && (rule->actionset->intercept_action != ACTION_ALLOW) + && (rule->actionset->intercept_action != ACTION_ALLOW_REQUEST) + && (rule->actionset->intercept_action != ACTION_NONE) + ) { return apr_psprintf(cmd->pool, "ModSecurity: Disruptive actions " "cannot be specified in the logging phase."); } diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 1553c962..add7ac78 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -198,6 +198,21 @@ int perform_interception(modsec_rec *msr) { status = DECLINED; message = apr_psprintf(msr->mp, "Access allowed%s.", phase_text); msr->was_intercepted = 0; + msr->allow_scope = ACTION_ALLOW; + break; + + case ACTION_ALLOW_PHASE : + status = DECLINED; + message = apr_psprintf(msr->mp, "Access to phase allowed%s.", phase_text); + msr->was_intercepted = 0; + msr->allow_scope = ACTION_ALLOW_PHASE; + break; + + case ACTION_ALLOW_REQUEST : + status = DECLINED; + message = apr_psprintf(msr->mp, "Access to request allowed%s.", phase_text); + msr->was_intercepted = 0; + msr->allow_scope = ACTION_ALLOW_REQUEST; break; default : diff --git a/apache2/modsecurity.c b/apache2/modsecurity.c index cc543618..e3b694a5 100644 --- a/apache2/modsecurity.c +++ b/apache2/modsecurity.c @@ -359,7 +359,12 @@ static apr_status_t modsecurity_process_phase_request_headers(modsec_rec *msr) { * */ static apr_status_t modsecurity_process_phase_request_body(modsec_rec *msr) { - msr_log(msr, 4, "Starting phase REQUEST_BODY."); + if ((msr->allow_scope == ACTION_ALLOW_REQUEST)||(msr->allow_scope == ACTION_ALLOW)) { + msr_log(msr, 4, "Skipping phase REQUEST_BODY (allow used)."); + return 0; + } else { + msr_log(msr, 4, "Starting phase REQUEST_BODY."); + } if (msr->txcfg->ruleset != NULL) { return msre_ruleset_process_phase(msr->txcfg->ruleset, msr); @@ -372,7 +377,12 @@ static apr_status_t modsecurity_process_phase_request_body(modsec_rec *msr) { * */ static apr_status_t modsecurity_process_phase_response_headers(modsec_rec *msr) { - msr_log(msr, 4, "Starting phase RESPONSE_HEADERS."); + if (msr->allow_scope == ACTION_ALLOW) { + msr_log(msr, 4, "Skipping phase RESPONSE_HEADERS (allow used)."); + return 0; + } else { + msr_log(msr, 4, "Starting phase RESPONSE_HEADERS."); + } if (msr->txcfg->ruleset != NULL) { return msre_ruleset_process_phase(msr->txcfg->ruleset, msr); @@ -385,7 +395,12 @@ static apr_status_t modsecurity_process_phase_response_headers(modsec_rec *msr) * */ static apr_status_t modsecurity_process_phase_response_body(modsec_rec *msr) { - msr_log(msr, 4, "Starting phase RESPONSE_BODY."); + if (msr->allow_scope == ACTION_ALLOW) { + msr_log(msr, 4, "Skipping phase RESPONSE_BODY (allow used)."); + return 0; + } else { + msr_log(msr, 4, "Starting phase RESPONSE_BODY."); + } if (msr->txcfg->ruleset != NULL) { return msre_ruleset_process_phase(msr->txcfg->ruleset, msr); diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index a9c8a79f..7ec5d9a4 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -172,6 +172,8 @@ extern DSOLOCAL const command_rec module_directives[]; #define ACTION_PROXY 3 #define ACTION_DROP 4 #define ACTION_ALLOW 5 +#define ACTION_ALLOW_REQUEST 6 +#define ACTION_ALLOW_PHASE 7 #define MODSEC_DISABLED 0 #define MODSEC_DETECTION_ONLY 1 @@ -366,6 +368,14 @@ struct modsec_rec { /* removed rules */ apr_array_header_t *removed_rules; + + /* When "allow" is executed the variable below is + * updated to contain the scope of the allow action. Set + * at 0 by default, it will have ACTION_ALLOW if we are + * to allow phases 1-4 and ACTION_ALLOW_REQUEST if we + * are to allow phases 1-2 only. + */ + unsigned int allow_scope; }; struct directory_config { diff --git a/apache2/re_actions.c b/apache2/re_actions.c index 873d126d..bd8e2707 100644 --- a/apache2/re_actions.c +++ b/apache2/re_actions.c @@ -430,7 +430,8 @@ static char *msre_action_skipAfter_validate(msre_engine *engine, msre_action *ac static apr_status_t msre_action_skipAfter_init(msre_engine *engine, msre_actionset *actionset, msre_action *action) { - // TODO: Need to keep track of skipAfter IDs so we can insert placeholders after we get to the real rule with that ID. + // TODO: Need to keep track of skipAfter IDs so we can insert placeholders after + // we get to the real rule with that ID. actionset->skip_after = action->param; return 1; } @@ -441,9 +442,34 @@ static apr_status_t msre_action_allow_init(msre_engine *engine, msre_actionset * msre_action *action) { actionset->intercept_action = ACTION_ALLOW; + + if (action->param != NULL) { + if (strcasecmp(action->param, "phase") == 0) { + actionset->intercept_action = ACTION_ALLOW_PHASE; + } else + if (strcasecmp(action->param, "request") == 0) { + actionset->intercept_action = ACTION_ALLOW_REQUEST; + } + } + return 1; } +static char *msre_action_allow_validate(msre_engine *engine, msre_action *action) { + if (action->param != NULL) { + if (strcasecmp(action->param, "phase") == 0) { + return NULL; + } else + if (strcasecmp(action->param, "request") == 0) { + return NULL; + } else { + return apr_psprintf("Invalid parameter for allow: %s", action->param); + } + } + + return NULL; +} + /* phase */ static char *msre_action_phase_validate(msre_engine *engine, msre_action *action) { @@ -1699,10 +1725,10 @@ void msre_engine_register_default_actions(msre_engine *engine) { msre_engine_action_register(engine, "allow", ACTION_DISRUPTIVE, - 0, 0, + 0, 1, NO_PLUS_MINUS, ACTION_CARDINALITY_ONE, - NULL, + msre_action_allow_validate, msre_action_allow_init, NULL ); diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml index 5dadecf8..37026a1d 100644 --- a/doc/modsecurity2-apache-reference.xml +++ b/doc/modsecurity2-apache-reference.xml @@ -3,7 +3,7 @@ ModSecurity Reference Manual - Version 2.5.0-rc1/ (December 14, 2007) + Version 2.5.0-rc1/ (December 17, 2007) 2004-2007 @@ -1090,7 +1090,7 @@ SecAuditLogStorageDir logs/audit Dependencies/Notes: Rules following a SecDefaultAction directive will inherit this setting unless a specific action is specified for an indivdual rule or until another - SecDefaultAction is specified. Take special note that in the logging + SecDefaultAction is specified. Take special note that in the logging disruptive actions are not allowed, but this can inadvertantly be inherited using a disruptive action in SecDefaultAction. @@ -1963,28 +1963,26 @@ SecAction setsid:%{REQUEST_COOKIES.PHPSESSID} - Request headers + Request headers (REQUEST_HEADERS) - Request body + Request body (REQUEST_BODY) - Response headers + Response headers (RESPONSE_HEADERS) - Response body + Response body (RESPONSE_BODY) - Logging + Logging (LOGGING) - ModSecurity Processing Phases Diagram - Below is a diagram of the standard Apache Request Cycle. In the diagram, the 5 ModSecurity processing phases are shown. @@ -1999,14 +1997,22 @@ SecAction setsid:%{REQUEST_COOKIES.PHPSESSID} SecDefaultAction "log,pass,phase:2" SecRule REQUEST_HEADERS:Host "!^$" "deny,phase:1" - Note on Rule and Phases + + Keep in mind that rules are executed according to phases, so even + if two rules are adjacent in a configuration file, but are set to + execute in different phases, they would not happen one after the other. + The order of rules in the configuration file is important only within + the rules of each phase. This is especially important when using the + skip and skipAfter actions. + - Keep in mind that rules are executed according to phases, so even if - two rules are adjacent in a configuration file, but are set to execute in - different phases, they would not happen one after the other. The order of - rules in the configuration file is important only within the rules of each - phase. This is especially important when using the skip - and skipAfter actions. + + The LOGGING phase is special. It is executed at + the end of each transaction no matter what happened in the previous + phases. This means it will be processed even if the request was + intercepted of the allow action was used to pass the + transaction through. +
Phase Request Headers @@ -2092,9 +2098,9 @@ SecRule REQUEST_HEADERS:Host "!^$" "deny,phase:1" + available during phase:3 or phase:4. Note that you must be careful not + to inherit a disruptive action into a rule in this phase as this is a + configuration error in ModSecurity 2.5.0 and later versions.
@@ -3552,8 +3558,8 @@ SecRule XML:/xq:employees/employee/name/text() Fred \
<literal>allow</literal> - Description: Stops processing on a successful - match and allows transaction to proceed. + Description: Stops rule processing on a + successful match and allows the transaction to proceed. Action Group: Disruptive @@ -3561,12 +3567,51 @@ SecRule XML:/xq:employees/employee/name/text() Fred \ SecRule REMOTE_ADDR "^192\.168\.1\.100$" nolog,phase:1,allow - Note + Prior to ModSecurity 2.5 the allow action would + only affect the current phase. An allow in phase 1 + would skip processing the remaining rules in phase 1 but the rules from + phase 2 would execute. Starting with v2.5 allow was + enhanced to allow for fine-grained control of what is done. The + following rules now apply: - The allow action only applies to the current processing phase. If - your intent is to explicitly allow a request, then you should use the - "ctl" action to turn the ruleEngine off - - ctl:ruleEngine=Off. + + + If used one its own, like in the example above, + allow will affect the entire transaction, + stopping processing of the current phase but also skipping over all + other phases apart from the logging phase. (The logging phase is + special; it is designed to always execute.) + + + + If used with parameter "phase", allow will + cause the engine to stop processing the current phase. Other phases + will continue as normal. + + + + If used with parameter "request", allow + will cause the engine to stop processing the current phase. The next + phase to be processed will be phase + RESPONSE_HEADERS. + + + + Examples: + + # Do not process request but process response. +SecAction phase:1,allow:request + +# Do not process transaction (request and response). +SecAction phase:1,allow + + + If you want to allow a response through, put a rule in phase + RESPONSE_HEADERS and simply use + allow on its own: + + # Allow response through. +SecAction phase:3,allow
@@ -5164,4 +5209,4 @@ SecRule REQUEST_METHOD "!@within %{tx.allowed_methods}" t:l
- + \ No newline at end of file