From 9fbcc2419a58f9fe97f75aa7688cc684b6925e71 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 20 Dec 2017 11:26:36 -0300 Subject: [PATCH 01/30] Adds Andrei Belov in the list of authors --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 595a829..490612f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -1 +1,2 @@ zimmerle = Felipe Zimmerle +defanator = Andrei Belov From c0ae166cc30c01b96147b6bc3d0cda708f5cdfb7 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 20 Dec 2017 14:36:29 -0300 Subject: [PATCH 02/30] Updates the CHANGES files --- CHANGES | 7 ++++--- release.sh | 20 ++++++++++++++++++++ src/ngx_http_modsecurity_common.h | 28 ++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 3 deletions(-) create mode 100755 release.sh diff --git a/CHANGES b/CHANGES index a669e53..a1ba664 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,6 @@ -DD mmm YYYY - 1.0.0 + +v1.0.0 - 2017-Dec-20 -------------------- - * First version of the ModSecurity-nginx - [Felipe Zimmerle] + - First version of ModSecurity-nginx connector + diff --git a/release.sh b/release.sh new file mode 100755 index 0000000..6b6e0a3 --- /dev/null +++ b/release.sh @@ -0,0 +1,20 @@ +#!/bin/bash + +git clean -xfdi +git submodule foreach --recursive git clean -xfdi + +VERSION=`git describe --tags` +DIR_NAME="modsecurity-nginx-$VERSION" +TAR_NAME="modsecurity-nginx-$VERSION.tar.gz" + +MY_DIR=${PWD##*/} + +cd .. +tar --transform "s/^$MY_DIR/$DIR_NAME/" -cvzf $TAR_NAME --exclude .git $MY_DIR + +sha256sum $TAR_NAME > $TAR_NAME.sha256 +gpg --detach-sign -a $TAR_NAME + +cd - +echo $TAR_NAME ": done." + diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index f2a4aa8..7d8ebcd 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -26,6 +26,34 @@ #include #include + +/** + * TAG_NUM: + * + * Alpha - 001 + * Beta - 002 + * Dev - 010 + * Rc1 - 051 + * Rc2 - 052 + * ... - ... + * Release- 100 + * + */ + +#define MODSECURITY_NGINX_MAJOR "1" +#define MODSECURITY_NGINX_MINOR "0" +#define MODSECURITY_NGINX_PATCHLEVEL "0" +#define MODSECURITY_NGINX_TAG "" +#define MODSECURITY_NGINX_TAG_NUM "100" + +#define MODSECURITY_NGINX_VERSION MODSECURITY_NGINX_MAJOR "." \ + MODSECURITY_NGINX_MINOR "." MODSECURITY_NGINX_PATCHLEVEL \ + MODSECURITY_NGINX_TAG + +#define MODSECURITY_NGINX_VERSION_NUM MODSECURITY_NGINX_MAJOR \ + MODSECURITY_NGINX_MINOR MODSECURITY_NGINX_PATCHLEVEL \ + MODSECURITY_NGINX_TAG_NUM + typedef struct { ngx_str_t name; ngx_str_t value; From 4582a3602788344bbbc3f3bb81a50948fb063583 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Wed, 21 Mar 2018 15:20:13 +0300 Subject: [PATCH 03/30] travis-ci: update nginx versions to latest available --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2d71eb5..2431f31 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,8 +16,8 @@ addons: - liblmdb-dev env: - - VER_NGINX=1.13.4 - - VER_NGINX=1.12.1 + - VER_NGINX=1.13.10 + - VER_NGINX=1.12.2 before_script: - cd .. From 2966dd6bc3b7fa2ec71df8d14badf6bc1fe7f200 Mon Sep 17 00:00:00 2001 From: Airis777 Date: Tue, 19 Dec 2017 10:21:20 +0400 Subject: [PATCH 04/30] Fixed memory leak --- src/ngx_http_modsecurity_module.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 6d6ee7a..0c47fb4 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -651,9 +651,11 @@ ngx_http_modsecurity_config_cleanup(void *data) old_pool = ngx_http_modsecurity_pcre_malloc_init(NULL); msc_rules_cleanup(t->rules_set); + msc_cleanup(t->modsec); ngx_http_modsecurity_pcre_malloc_done(old_pool); t->rules_set = NULL; + t->modsec = NULL; } From c66e52ccbfcb84d218939f113f98dac50f6679a7 Mon Sep 17 00:00:00 2001 From: Airis777 Date: Mon, 25 Dec 2017 10:45:48 +0400 Subject: [PATCH 05/30] modsec initialized with NULL --- src/ngx_http_modsecurity_module.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 0c47fb4..a773fb4 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -545,6 +545,7 @@ static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf) conf->enable = NGX_CONF_UNSET; conf->sanity_checks_enabled = NGX_CONF_UNSET; conf->rules_set = msc_create_rules_set(); + conf->modsec = NULL; cln = ngx_pool_cleanup_add(cf->pool, 0); if (cln == NULL) { From 7e328a3aec49319180aa84deb8002d36d652f801 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 21 Mar 2018 21:59:09 -0300 Subject: [PATCH 06/30] CHANGES: Adds info about #80 --- CHANGES | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES b/CHANGES index a1ba664..4bfb756 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +v1.0.x - YYYY-MMM-DD (To be released) +------------------------------------- + + - Fixed memory leak on config cleanup. + [Issue #80 - @AirisX, @defanator] + v1.0.0 - 2017-Dec-20 -------------------- From d77c4c1e28176b442231b4f77a704f9e1c2e43c6 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Mon, 25 Dec 2017 12:31:30 +0300 Subject: [PATCH 07/30] Emit connector version in error log While here, fixed msc_set_connector_info() to use consistent version string. This fixes #85. --- src/ngx_http_modsecurity_common.h | 3 +++ src/ngx_http_modsecurity_module.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 7d8ebcd..1a21f78 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -54,6 +54,9 @@ MODSECURITY_NGINX_MINOR MODSECURITY_NGINX_PATCHLEVEL \ MODSECURITY_NGINX_TAG_NUM +#define MODSECURITY_NGINX_WHOAMI "ModSecurity-nginx v" \ + MODSECURITY_NGINX_VERSION + typedef struct { ngx_str_t name; ngx_str_t value; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index a773fb4..7890437 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -496,6 +496,8 @@ ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf) { ngx_http_modsecurity_conf_t *conf; + ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, MODSECURITY_NGINX_WHOAMI); + /* ngx_pcalloc already sets all of this scructure to zeros. */ conf = ngx_http_modsecurity_create_conf(cf); @@ -515,7 +517,7 @@ ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf) } /* Provide our connector information to LibModSecurity */ - msc_set_connector_info(conf->modsec, "ModSecurity-nginx v0.1.1-beta"); + msc_set_connector_info(conf->modsec, MODSECURITY_NGINX_WHOAMI); msc_set_log_cb(conf->modsec, ngx_http_modsecurity_log); return conf; From 995f631767c24de8fabf828b8f44d27b316d1395 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Mon, 26 Mar 2018 16:24:58 -0300 Subject: [PATCH 08/30] CHANGES: Adds info about #88 --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 4bfb756..278bb96 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v1.0.x - YYYY-MMM-DD (To be released) ------------------------------------- + - Emit connector version in error log + [Isseu #88 - @defanator] - Fixed memory leak on config cleanup. [Issue #80 - @AirisX, @defanator] From dfb341c5900f8de001d9b65a82e488a040a9d574 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 4 Apr 2018 14:03:58 -0300 Subject: [PATCH 09/30] Fix typo in the CHANGES file --- CHANGES | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 278bb96..6202806 100644 --- a/CHANGES +++ b/CHANGES @@ -2,7 +2,7 @@ v1.0.x - YYYY-MMM-DD (To be released) ------------------------------------- - Emit connector version in error log - [Isseu #88 - @defanator] + [Issue #88 - @defanator] - Fixed memory leak on config cleanup. [Issue #80 - @AirisX, @defanator] From bcfe69a58db229dbb5e96f9f771a7420b5bbd849 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Wed, 4 Apr 2018 17:20:49 +0300 Subject: [PATCH 10/30] Fix memory leak in intervention processing intervention.log is being allocated via strdup() here: https://github.com/SpiderLabs/ModSecurity/blob/v3/master/src/transaction.cc#L1362 and should be freed by connector. --- src/ngx_http_modsecurity_module.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 7890437..12577fe 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -132,6 +132,7 @@ ngx_inline char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p) ngx_inline int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r) { + char *log = NULL; ModSecurityIntervention intervention; intervention.status = 200; intervention.url = NULL; @@ -145,11 +146,16 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re return 0; } + log = intervention.log; if (intervention.log == NULL) { - intervention.log = "(no log message was specified)"; + log = "(no log message was specified)"; } - ngx_log_error(NGX_LOG_WARN, (ngx_log_t *)r->connection->log, 0, "%s", intervention.log); + ngx_log_error(NGX_LOG_WARN, (ngx_log_t *)r->connection->log, 0, "%s", log); + + if (intervention.log != NULL) { + free(intervention.log); + } if (intervention.url != NULL) { From 6d5f75968124925ea8861f8fc941c000c50c3dc5 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 4 Apr 2018 16:40:10 -0300 Subject: [PATCH 11/30] CHANGES: Adds info about #100 --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 6202806..276a0a0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v1.0.x - YYYY-MMM-DD (To be released) ------------------------------------- + - Fix memory leak in intervention processing + [Issue #100 - @defanator] - Emit connector version in error log [Issue #88 - @defanator] - Fixed memory leak on config cleanup. From 68117847a54a985c5fd27cb885905b908594a805 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 5 Apr 2018 10:38:35 +0300 Subject: [PATCH 12/30] README: libmodsecurity is now GA, removing outdated notice --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 4ba5e5f..6da71c5 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,6 @@ The ModSecurity-nginx connector is the connection point between Nginx and libmod The ModSecurity-nginx connector takes the form of an Nginx module. The module simply serves as a layer of communication between Nginx and ModSecurity. Notice that this project depends on libmodsecurity rather than ModSecurity (version 2.9 or less). -libmodsecurity has not reached a stable release candidate, thus, use this project with caution. ### What is the difference between this project and the old ModSecurity add-on for nginx? From 8085a2f527c55e58dabfbdb8312309a3cc2dc9ba Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 5 Apr 2018 10:41:26 +0300 Subject: [PATCH 13/30] README: use common capitalization style for "nginx" as a software --- README.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 6da71c5..141c23d 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,9 @@ -The ModSecurity-nginx connector is the connection point between Nginx and libmodsecurity (ModSecurity v3). Said another way, this project provides a communication channel between Nginx and libmodsecurity. This connector is required to use LibModSecurity with Nginx. +The ModSecurity-nginx connector is the connection point between nginx and libmodsecurity (ModSecurity v3). Said another way, this project provides a communication channel between nginx and libmodsecurity. This connector is required to use LibModSecurity with nginx. -The ModSecurity-nginx connector takes the form of an Nginx module. The module simply serves as a layer of communication between Nginx and ModSecurity. +The ModSecurity-nginx connector takes the form of an nginx module. The module simply serves as a layer of communication between nginx and ModSecurity. Notice that this project depends on libmodsecurity rather than ModSecurity (version 2.9 or less). @@ -25,7 +25,7 @@ Apache. As a result, This current version has less dependencies, fewer bugs, and Before compile this software make sure that you have libmodsecurity installed. You can download it from the [ModSecurity git repository](https://github.com/SpiderLabs/ModSecurity). For information pertaining to the compilation and installation of libmodsecurity please consult the documentation provided along with it. -With libmodsecurity installed, you can proceed with the installation of the ModSecurity-nginx connector, which follow the Nginx 3rd party module installation procedure: +With libmodsecurity installed, you can proceed with the installation of the ModSecurity-nginx connector, which follow the nginx 3rd party module installation procedure: ``` ./configure --add-module=/path/your/modsecurity-for-nginx @@ -36,17 +36,17 @@ http://wiki.nginx.org/3rdPartyModules # Usage -ModSecurity for Nginx extends your Nginx configuration directives. It adds four +ModSecurity for nginx extends your nginx configuration directives. It adds four new directives and they are: modsecurity [On|Off] - This directive turns on or off ModSecurity functionality. Note that -this configuration directive is no longer related to the SecRule state. Instead, it now serves solely as an Nginx flag to enable or disable the module. +this configuration directive is no longer related to the SecRule state. Instead, it now serves solely as an nginx flag to enable or disable the module. modsecurity_rules_file [] - This directive indicates the location of the modsecurity configuration file. modsecurity_rules_remote [server-key] [] - This directive is used to indicate from where (on the internet) a modsecurity configuration file will be downloaded. It also specifies the key that will be used to authenticate to that server. -modsecurity_rules [] - This directive allows for the direct inclusion of a ModSecurity rule into the Nginx configuration. +modsecurity_rules [] - This directive allows for the direct inclusion of a ModSecurity rule into the nginx configuration. ### Usage example: injecting rules within nginx configuration @@ -128,7 +128,7 @@ You may also take a look at recent bug reports and open issues to get an idea of ### Testing your patch Along with the manual testing, we strongly recommend that you to use the nginx test -utility to make sure that you patch does not adversely affect the behavior or performance of Nginx. +utility to make sure that you patch does not adversely affect the behavior or performance of nginx. The nginx tests are available on: http://hg.nginx.org/nginx-tests/ @@ -141,14 +141,14 @@ $ cd /path/to/nginx/test/repository $ TEST_NGINX_BINARY=/path/to/your/nginx prove . ``` -If you are facing problems getting your added functionality to pass all the Nginx tests, feel free to contact us or the nginx mailing list at: http://nginx.org/en/support.html +If you are facing problems getting your added functionality to pass all the nginx tests, feel free to contact us or the nginx mailing list at: http://nginx.org/en/support.html ### Debugging We respect the nginx debugging schema. By using the configuration option “--with-debug” during the nginx configuration you will also be enabling the connector's debug messages. Core dumps and crashes are expected to be debugged -in the same fashion that is used to debug Nginx. For further information, +in the same fashion that is used to debug nginx. For further information, please check the nginx debugging information: http://wiki.nginx.org/Debugging From 3c86d57f04b3262c4637603d3dfeda846bb55ad6 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 5 Apr 2018 10:55:09 +0300 Subject: [PATCH 14/30] README: minor fixes --- README.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 141c23d..ece8c6a 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,6 @@ [![](https://raw.githubusercontent.com/ZenHubIO/support/master/zenhub-badge.png)](https://zenhub.com) - The ModSecurity-nginx connector is the connection point between nginx and libmodsecurity (ModSecurity v3). Said another way, this project provides a communication channel between nginx and libmodsecurity. This connector is required to use LibModSecurity with nginx. The ModSecurity-nginx connector takes the form of an nginx module. The module simply serves as a layer of communication between nginx and ModSecurity. @@ -17,7 +16,7 @@ Notice that this project depends on libmodsecurity rather than ModSecurity (vers The old version uses ModSecurity standalone, which is a wrapper for Apache internals to link ModSecurity to nginx. This current version is closer to nginx, consuming the new libmodsecurity which is no longer dependent on -Apache. As a result, This current version has less dependencies, fewer bugs, and is faster. In addition, Some new functionality is also provided - such as the possibility of use of global rules configuration with per directory/location customizations (e.g. SecRuleRemoveById). +Apache. As a result, this current version has less dependencies, fewer bugs, and is faster. In addition, some new functionality is also provided - such as the possibility of use of global rules configuration with per directory/location customizations (e.g. SecRuleRemoveById). # Compilation @@ -28,12 +27,13 @@ You can download it from the [ModSecurity git repository](https://github.com/Spi With libmodsecurity installed, you can proceed with the installation of the ModSecurity-nginx connector, which follow the nginx 3rd party module installation procedure: ``` -./configure --add-module=/path/your/modsecurity-for-nginx +./configure --add-module=/path/to/ModSecurity-nginx ``` Further information about nginx 3rd party add-ons support are available here: http://wiki.nginx.org/3rdPartyModules + # Usage ModSecurity for nginx extends your nginx configuration directives. It adds four @@ -48,7 +48,6 @@ modsecurity_rules_remote [server-key] [] - This directive is used modsecurity_rules [] - This directive allows for the direct inclusion of a ModSecurity rule into the nginx configuration. - ### Usage example: injecting rules within nginx configuration ``` ... @@ -95,12 +94,14 @@ location / { ... ``` + # Contributing As an open source project we invite (and encourage) anyone from the community to contribute to our project. This may take the form of: new functionality, bug fixes, bug reports, beginners user support, and anything else that you are willing to help with. Thank you. + ## Providing Patches We prefer to have your patch within the GitHub infrastructure to facilitate our @@ -146,7 +147,7 @@ If you are facing problems getting your added functionality to pass all the ngin ### Debugging We respect the nginx debugging schema. By using the configuration option -“--with-debug” during the nginx configuration you will also be enabling the +"--with-debug" during the nginx configuration you will also be enabling the connector's debug messages. Core dumps and crashes are expected to be debugged in the same fashion that is used to debug nginx. For further information, please check the nginx debugging information: http://wiki.nginx.org/Debugging @@ -168,11 +169,13 @@ version of your libmodsecurity and the version of the nginx connector you are ru Please do not publicly report any security issue. Instead, contact us at: security@modsecurity.org to report the issue. Once the problem is fixed we will provide you with credit for the discovery. + ## Feature Request We would love to discuss any ideas that you may have for a new feature. Please keep in mind this is a community driven project so be sure to contact the community via the mailing list to get feedback first. Alternatively, feel free to open GitHub issues requesting for new features. Before opening a new issue, please check if there is an existing feature request for the desired functionality. + ## Packing Having our packages in distros on time is something we highly desire. Let us know if From 81252653a1f0f1f92ba9e3d1080c4f0900922668 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 5 Apr 2018 11:26:28 +0300 Subject: [PATCH 15/30] README: revisited configuration directives section --- README.md | 120 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index ece8c6a..67c4121 100644 --- a/README.md +++ b/README.md @@ -36,62 +36,94 @@ http://wiki.nginx.org/3rdPartyModules # Usage -ModSecurity for nginx extends your nginx configuration directives. It adds four -new directives and they are: +ModSecurity for nginx extends your nginx configuration directives. +It adds four new directives and they are: -modsecurity [On|Off] - This directive turns on or off ModSecurity functionality. Note that -this configuration directive is no longer related to the SecRule state. Instead, it now serves solely as an nginx flag to enable or disable the module. +#### modsecurity +---------------- -modsecurity_rules_file [] - This directive indicates the location of the modsecurity configuration file. +**syntax:** *modsecurity on | off* -modsecurity_rules_remote [server-key] [] - This directive is used to indicate from where (on the internet) a modsecurity configuration file will be downloaded. It also specifies the key that will be used to authenticate to that server. +**context:** *http, server, location* -modsecurity_rules [] - This directive allows for the direct inclusion of a ModSecurity rule into the nginx configuration. +**default:** *off* -### Usage example: injecting rules within nginx configuration -``` -... -modsecurity on; -location / { - modsecurity_rules ' - SecRuleEngine On - SecDebugLog /tmp/modsec_debug.log - SecDebugLogLevel 9 - SecRule ARGS "@contains test" "id:1,phase:2,t:trim,block" - '; +Turns on or off ModSecurity functionality. +Note that this configuration directive is no longer related to the SecRule state. +Instead, it now serves solely as an nginx flag to enable or disable the module. + +#### modsecurity_rules_file +--------------------------- + +**syntax:** *modsecurity_rules_file <path to rules file>* + +**context:** *http, server, location* + +**default:** *no* + +Specifies the location of the modsecurity configuration file, e.g.: + +```nginx +server { + modsecurity on; + location / { + root /var/www/html; + modsecurity_rules_file /etc/my_modsecurity_rules.conf; + } } -... ``` -### Usage example: loading rules from a file and injecting specific configurations per directory/alias -``` -... -modsecurity on; -location / { - root /var/www/html; - modsecurity_rules_file /etc/my_modsecurity_rules.conf; -} -location /ops { - root /var/www/html/opts; - modsecurity_rules ' - SecRuleEngine On - SecDebugLog /tmp/modsec_debug.log - SecDebugLogLevel 9 - SecRuleRemoveById 10 - '; +#### modsecurity_rules_remote +----------------------------- + +**syntax:** *modsecurity_rules_remote <key> <URL to rules>* + +**context:** *http, server, location* + +**default:** *no* + +Specifies from where (on the internet) a modsecurity configuration file will be downloaded. +It also specifies the key that will be used to authenticate to that server: + +```nginx +server { + modsecurity on; + location / { + root /var/www/html; + modsecurity_rules_remote my-server-key https://my-own-server/rules/download; + } } -... ``` -### Usage example: loading rules from a remote server -``` -... -modsecurity on; -location / { - root /var/www/html; - modsecurity_rules_remote my-server-key https://my-own-server/rules/download; +#### modsecurity_rules +---------------------- + +**syntax:** *modsecurity_rules <modsecurity rule>* + +**context:** *http, server, location* + +**default:** *no* + +Allows for the direct inclusion of a ModSecurity rule into the nginx configuration. +The following example is loading rules from a file and injecting specific configurations per directory/alias: + +```nginx +server { + modsecurity on; + location / { + root /var/www/html; + modsecurity_rules_file /etc/my_modsecurity_rules.conf; + } + location /ops { + root /var/www/html/opts; + modsecurity_rules ' + SecRuleEngine On + SecDebugLog /tmp/modsec_debug.log + SecDebugLogLevel 9 + SecRuleRemoveById 10 + '; + } } -... ``` From c7c0676372793a84f3d37c3267d22c1da78b2391 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 5 Apr 2018 11:34:11 +0300 Subject: [PATCH 16/30] README: improve formatting --- README.md | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 67c4121..4b8719d 100644 --- a/README.md +++ b/README.md @@ -39,9 +39,8 @@ http://wiki.nginx.org/3rdPartyModules ModSecurity for nginx extends your nginx configuration directives. It adds four new directives and they are: -#### modsecurity ----------------- - +modsecurity +----------- **syntax:** *modsecurity on | off* **context:** *http, server, location* @@ -52,9 +51,8 @@ Turns on or off ModSecurity functionality. Note that this configuration directive is no longer related to the SecRule state. Instead, it now serves solely as an nginx flag to enable or disable the module. -#### modsecurity_rules_file ---------------------------- - +modsecurity_rules_file +---------------------- **syntax:** *modsecurity_rules_file <path to rules file>* **context:** *http, server, location* @@ -73,9 +71,8 @@ server { } ``` -#### modsecurity_rules_remote ------------------------------ - +modsecurity_rules_remote +------------------------ **syntax:** *modsecurity_rules_remote <key> <URL to rules>* **context:** *http, server, location* @@ -95,9 +92,8 @@ server { } ``` -#### modsecurity_rules ----------------------- - +modsecurity_rules +----------------- **syntax:** *modsecurity_rules <modsecurity rule>* **context:** *http, server, location* From 08c3fbab854483ad87d8be25be78bbae344c64ef Mon Sep 17 00:00:00 2001 From: dennus Date: Mon, 23 Apr 2018 14:09:58 +0400 Subject: [PATCH 17/30] fix log output --- src/ngx_http_modsecurity_body_filter.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 802fe1e..c55c255 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -192,19 +192,15 @@ if (in == NULL) { } if (is_request_processed) { - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH PROCESSING"); old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_response_body(ctx->modsec_transaction); ngx_http_modsecurity_pcre_malloc_done(old_pool); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH PROCESSING RET = %d", ret); if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){ - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH DO HEADER FILTERS = %d", ret); ctx->header_pt(r); } else { - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH DO FINALIZE = %d", ret); ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module , ret); @@ -218,7 +214,6 @@ if (in == NULL) { ctx->header_pt(r); return ngx_http_next_body_filter(r, ctx->temp_chain); } else { - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS WAITING FOR NEXT CHUNK"); return NGX_AGAIN; } } From 106572e43a9abee0f91e4396ce6b6ade44e713ab Mon Sep 17 00:00:00 2001 From: Airis777 Date: Fri, 22 Dec 2017 11:22:25 +0400 Subject: [PATCH 18/30] The pool pointer is now available for ngx_http_modsecurity_config_cleanup --- src/ngx_http_modsecurity_common.h | 2 ++ src/ngx_http_modsecurity_module.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 1a21f78..7108347 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -93,6 +93,8 @@ typedef struct { ngx_flag_t sanity_checks_enabled; Rules *rules_set; + + void *pool; } ngx_http_modsecurity_conf_t; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 12577fe..0c491cc 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -554,6 +554,7 @@ static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf) conf->sanity_checks_enabled = NGX_CONF_UNSET; conf->rules_set = msc_create_rules_set(); conf->modsec = NULL; + conf->pool = cf->pool; cln = ngx_pool_cleanup_add(cf->pool, 0); if (cln == NULL) { @@ -658,7 +659,7 @@ ngx_http_modsecurity_config_cleanup(void *data) dd("deleting a loc conf -- RuleSet is: \"%p\"", t->rules_set); - old_pool = ngx_http_modsecurity_pcre_malloc_init(NULL); + old_pool = ngx_http_modsecurity_pcre_malloc_init(t->pool); msc_rules_cleanup(t->rules_set); msc_cleanup(t->modsec); ngx_http_modsecurity_pcre_malloc_done(old_pool); From c9e38d8507da94fa68d79dd1cdebc2717a4b458f Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Thu, 3 May 2018 16:45:57 -0300 Subject: [PATCH 19/30] CHANGES: Adds info about #87 --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 276a0a0..2b60259 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,8 @@ v1.0.x - YYYY-MMM-DD (To be released) ------------------------------------- + - Pool pointer is now handled in ngx_http_modsecurity_config_cleanup + [Issue #87 - @AirisX, @defanator, @zimmerle] - Fix memory leak in intervention processing [Issue #100 - @defanator] - Emit connector version in error log From 269fde27d117c89133990913e0824b52959cb26f Mon Sep 17 00:00:00 2001 From: Sergei Turchanov Date: Wed, 18 Apr 2018 23:27:33 +1000 Subject: [PATCH 20/30] Fix incorrect handling of request/response body Fix incorrect handling of request/response body data chain of ngx_buf_t buffers. The documentation [http://nginx.org/en/docs/dev/development_guide.html#buffer] clearly states that .pos, .last must be used to reference actual data contained by the buffer. Whereas .start, .end denote the boundaries of the memory block allocated for the buffer (in case of dynamically allocated data) or just NULL (when .pos, .last reference a static memory location - one can see that kind of usage in ngx_http_gzip_filter_module.c:ngx_http_gzip_filter_gzheader()). To back up my words I invite to examine ngx_http_charset_filter_module.c:ngx_http_charset_recode() as an example of iteration over data contained in data buffer. Without this fix ngx_http_modsecurity_body_filter feeds random bytes from memory pointed by .start, .end range to msc_append_response_body. In my case is was 8KB of data instead of 10 bytes when referenced by (.pos, .last). That is this vulnerability may disclose sensitive data like passwords or whatever from nginx heap. The fix for ngx_http_modsecurity_pre_access_handler is to use .pos not .start to reference data as they may differ in general case. --- src/ngx_http_modsecurity_body_filter.c | 4 ++-- src/ngx_http_modsecurity_pre_access.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index f8b3c71..edf44f6 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -150,9 +150,9 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) for (chain = in; chain != NULL; chain = chain->next) { - u_char *data = chain->buf->start; + u_char *data = chain->buf->pos; - msc_append_response_body(ctx->modsec_transaction, data, chain->buf->end - data); + msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { return ngx_http_filter_finalize_request(r, diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index 70f9feb..6f4cbcb 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -163,10 +163,10 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) while (chain && !already_inspected) { - u_char *data = chain->buf->start; + u_char *data = chain->buf->pos; msc_append_request_body(ctx->modsec_transaction, data, - chain->buf->last - chain->buf->pos); + chain->buf->last - data); if (chain->buf->last_buf) { break; From 29409ab0673c888a0c417fa86345be4e7e20fc59 Mon Sep 17 00:00:00 2001 From: Sergei Turchanov Date: Thu, 19 Apr 2018 21:20:23 +1000 Subject: [PATCH 21/30] Fixed processing of response body chunks in ngx_http_modsecurity_body_filter. A body filter function (ngx_http_modsecurity_body_filter in our case) can be called by Nginx several times during request processing. And each time with it own unique set of chained buf pointers. For example, suppose a complete response consists of this chain of data: A->B->C->D->E Ngix may (and actually does, as verified by me in gdb) call body filter two times like this: handler(r, in = A->B->C) handler(r, in = D->E), E has last_buf set Current implementation delays feeding chain->buf to msc_append_response_body until it comes upon a chain with buf->last_buf set. So we loose chain containing A->B->C sequence. We must process body bufs as soon as we see them in body handler otherwise we will not see them again. N.B. You have PR #84 pending. It goes further and fixes the problem when a blocking decision is made after headers were sent. I intentionally retained current (buggy) behavior to make my patch less intrusive and easier to review. Besides #84 impose an excessive memory usage due to a complete copy of all bufs passed through body filter (we have sometimes 500K and more replies in our applications) - I will elaborate on this in code review for #84. --- src/ngx_http_modsecurity_body_filter.c | 55 ++++++++++++-------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index edf44f6..fd286d0 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -35,7 +35,6 @@ ngx_http_modsecurity_body_filter_init(void) ngx_int_t ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) { - int buffer_fully_loadead = 0; ngx_chain_t *chain = in; ngx_http_modsecurity_ctx_t *ctx = NULL; #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) @@ -135,47 +134,43 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } #endif + int is_request_processed = 0; for (; chain != NULL; chain = chain->next) { -/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */ - if (chain->buf->last_buf) { - buffer_fully_loadead = 1; + u_char *data = chain->buf->pos; + int ret; + + msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); + if (ret > 0) { + return ngx_http_filter_finalize_request(r, + &ngx_http_modsecurity_module, ret); } - } - if (buffer_fully_loadead == 1) - { - int ret; - ngx_pool_t *old_pool; +/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */ + is_request_processed = chain->buf->last_buf; - for (chain = in; chain != NULL; chain = chain->next) - { - u_char *data = chain->buf->pos; + if (is_request_processed) { + ngx_pool_t *old_pool; + + old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); + msc_process_response_body(ctx->modsec_transaction); + ngx_http_modsecurity_pcre_malloc_done(old_pool); - msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data); +/* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's + XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */ ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { - return ngx_http_filter_finalize_request(r, - &ngx_http_modsecurity_module, ret); + return ret; } - } - - old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); - msc_process_response_body(ctx->modsec_transaction); - ngx_http_modsecurity_pcre_malloc_done(old_pool); + else if (ret < 0) { + return ngx_http_filter_finalize_request(r, + &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); -/* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's - XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */ - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); - if (ret > 0) { - return ret; - } - else if (ret < 0) { - return ngx_http_filter_finalize_request(r, - &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); + } } } - else + if (!is_request_processed) { dd("buffer was not fully loaded! ctx: %p", ctx); } From a4f2a5b186d9f8ebaa6a725c450659f0185cb34a Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 15 May 2018 12:16:21 -0300 Subject: [PATCH 22/30] CHANGES: Adds info about #104 & #105 --- CHANGES | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES b/CHANGES index 2b60259..7cc2794 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,12 @@ v1.0.x - YYYY-MMM-DD (To be released) ------------------------------------- + - Fixed processing of response body chunks in + ngx_http_modsecurity_body_filter. + [Issue #105 - @turchanov, @defanator] + - Fix incorrect handling of request/response body data chain of ngx_buf_t + buffers + [Issue #104 - @turchanov, @defanator] - Pool pointer is now handled in ngx_http_modsecurity_config_cleanup [Issue #87 - @AirisX, @defanator, @zimmerle] - Fix memory leak in intervention processing From e4df1aa3c4d4605d7409de2b0b8d66c227df6255 Mon Sep 17 00:00:00 2001 From: Sergei Turchanov Date: Mon, 23 Apr 2018 12:37:46 +1000 Subject: [PATCH 23/30] Fix processing of response body when gzip compression is enabled Changed placement of ngx_http_modsecurity_module in nginx load order of modules to come after ngx_http_gzip_filter_module so that we could read a response body before it gets compressed. Prior to this fix ngx_http_modsecurity_body_filter was called after gzip filter so that msc_append_response_body was fed with compressed body bytes thus effectively making all further response body processing meaningless. --- config | 71 +++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 13 deletions(-) diff --git a/config b/config index ce77a3e..c6e7467 100644 --- a/config +++ b/config @@ -82,6 +82,30 @@ fi ngx_addon_name=ngx_http_modsecurity_module +# We must place ngx_http_modsecurity_module after ngx_http_gzip_filter_module +# in load order list to be able to read response body before it gets compressed +# (for filter modules later initialization means earlier execution). +# +# Nginx implements load ordering only for dynamic modules and only a BEFORE part +# of "ngx_module_order". So we list all of the modules that come after +# ngx_http_gzip_filter_module as a BEFORE dependency for +# ngx_http_modsecurity_module. +# +# For static compilation HTTP_FILTER_MODULES will be patched later. + +modsecurity_dependency="ngx_http_postpone_filter_module \ + ngx_http_ssi_filter_module \ + ngx_http_charset_filter_module \ + ngx_http_xslt_filter_module \ + ngx_http_image_filter_module \ + ngx_http_sub_filter_module \ + ngx_http_addition_filter_module \ + ngx_http_gunzip_filter_module \ + ngx_http_userid_filter_module \ + ngx_http_headers_filter_module \ + ngx_http_copy_filter_module" + + if test -n "$ngx_module_link"; then ngx_module_type=HTTP_FILTER ngx_module_name="$ngx_addon_name" @@ -98,7 +122,12 @@ if test -n "$ngx_module_link"; then ngx_module_libs="$ngx_feature_libs" ngx_module_incs="$ngx_feature_path" - ngx_module_order="ngx_http_chunked_filter_module ngx_http_v2_filter_module $ngx_module_name ngx_http_range_header_filter_module" + ngx_module_order="ngx_http_chunked_filter_module \ + ngx_http_v2_filter_module \ + ngx_http_range_header_filter_module \ + ngx_http_gzip_filter_module \ + $ngx_module_name \ + $modsecurity_dependency"; . auto/module else @@ -128,20 +157,36 @@ fi # # Nginx does not provide reliable way to introduce our module into required -# place in static ($ngx_module_link=ADDON) compilation mode, so we should +# place in static ($ngx_module_link=ADDON) compilation mode, so we must # explicitly update module "ordering rules". # -# Default runtime location of ngx_http_modsecurity_module is right before -# ngx_http_chunked_filter_module, but in case if ngx_http_v2_filter_module is -# compiled in, we should put our module before ngx_http_v2_filter_module in -# order to support SecRules processing for HTTP/2.0 requests. -# if [ "$ngx_module_link" != DYNAMIC ] ; then - pre_module='ngx_http_chunked_filter_module' - if [ "$HTTP_V2" = "YES" ]; then - pre_module='ngx_http_v2_filter_module' + # Reposition modsecurity module to satisfy $modsecurity_dependency + # (this mimics dependency resolution made by ngx_add_module() function + # though less optimal in terms of computational complexity). + modules= + found= + for module in $HTTP_FILTER_MODULES; do + # skip our module name from the original list + if [ "$module" = "$ngx_addon_name" ]; then + continue + fi + if [ -z "${found}" ]; then + for item in $modsecurity_dependency; do + if [ "$module" = "$item" ]; then + modules="${modules} $ngx_addon_name" + found=1 + break + fi + done + fi + modules="${modules} $module" + done + if [ -z "${found}" ]; then + # This must never happen since ngx_http_copy_filter_module must be in HTTP_FILTER_MODULES + # and we stated dependency on it in $modsecurity_dependency + echo "$0: error: cannot reposition modsecurity module in HTTP_FILTER_MODULES list" + exit 1 fi - HTTP_FILTER_MODULES=`echo $HTTP_FILTER_MODULES | \ - sed -E "s/$ngx_addon_name/ /g" | \ - sed -E "s/$pre_module/$pre_module $ngx_addon_name/g"` + HTTP_FILTER_MODULES="${modules}" fi From 37b76e88df4bce8a9846345c27271d7e6ce1acfb Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 15 May 2018 18:33:58 -0300 Subject: [PATCH 24/30] CHANGES: Adds info about #107 --- CHANGES | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 7cc2794..04b2252 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,7 @@ v1.0.x - YYYY-MMM-DD (To be released) ------------------------------------- - + - Fix processing of response body when gzip compression is enabled + [Issue #107 - @turchanov] - Fixed processing of response body chunks in ngx_http_modsecurity_body_filter. [Issue #105 - @turchanov, @defanator] From e59954f6015d162422ef24752b9913045677f0b3 Mon Sep 17 00:00:00 2001 From: dennus Date: Wed, 16 May 2018 18:01:22 +0400 Subject: [PATCH 25/30] Switch ResponseBodyAccess on Phase 4 --- tests/modsecurity-proxy.t | 3 ++- tests/modsecurity.t | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/modsecurity-proxy.t b/tests/modsecurity-proxy.t index ae94f15..50f613c 100644 --- a/tests/modsecurity-proxy.t +++ b/tests/modsecurity-proxy.t @@ -86,7 +86,8 @@ http { modsecurity on; modsecurity_rules ' SecRuleEngine On - SecDefaultAction "phase:4,log,deny,status:403" + SecResponseBodyAccess On + SecDefaultAction "phase:4,log,deny,status:403" SecRule ARGS "@streq redirect301" "id:1,phase:4,status:301,redirect:http://www.modsecurity.org" SecRule ARGS "@streq redirect302" "id:2,phase:4,status:302,redirect:http://www.modsecurity.org" SecRule ARGS "@streq block401" "id:3,phase:4,status:401,block" diff --git a/tests/modsecurity.t b/tests/modsecurity.t index 0dd982f..ca341ad 100644 --- a/tests/modsecurity.t +++ b/tests/modsecurity.t @@ -97,7 +97,8 @@ http { modsecurity on; modsecurity_rules ' SecRuleEngine On - SecDefaultAction "phase:4,log,deny,status:403" + SecResponseBodyAccess On + SecDefaultAction "phase:4,log,deny,status:403" SecRule ARGS "@streq redirect301" "id:1,phase:4,status:301,redirect:http://www.modsecurity.org" SecRule ARGS "@streq redirect302" "id:2,phase:4,status:302,redirect:http://www.modsecurity.org" SecRule ARGS "@streq block401" "id:3,phase:4,status:401,block" From 17cb5b42a563d7f77cb09fdf92ad07ddbd1852f6 Mon Sep 17 00:00:00 2001 From: dennus Date: Tue, 26 Jun 2018 09:30:29 +0400 Subject: [PATCH 26/30] fix return --- src/ngx_http_modsecurity_body_filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 2a8931f..be088f9 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -201,7 +201,7 @@ if (in == NULL) { ctx->header_pt(r); } else { - ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module , ret); } From 42c6a981427d6d110d6c1f39ba557f024dbae79c Mon Sep 17 00:00:00 2001 From: dennus Date: Tue, 26 Jun 2018 12:33:03 +0400 Subject: [PATCH 27/30] case sentivity var name? --- tests/modsecurity-scoring.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/modsecurity-scoring.t b/tests/modsecurity-scoring.t index ddd3b61..33a3612 100644 --- a/tests/modsecurity-scoring.t +++ b/tests/modsecurity-scoring.t @@ -46,7 +46,7 @@ http { SecRuleEngine On SecRule ARGS "@streq badarg1" "id:11,phase:2,setvar:tx.score=1" SecRule ARGS "@streq badarg2" "id:12,phase:2,setvar:tx.score=2" - SecRule TX:SCORE "@ge 2" "id:199,phase:request,deny,log,status:403" + SecRule tx:score "@ge 2" "id:199,phase:request,deny,log,status:403" '; } @@ -56,7 +56,7 @@ http { SecRule ARGS "@streq badarg1" "id:21,phase:2,setvar:tx.score=+1" SecRule ARGS "@streq badarg2" "id:22,phase:2,setvar:tx.score=+1" SecRule ARGS "@streq badarg3" "id:23,phase:2,setvar:tx.score=+1" - SecRule TX:SCORE "@ge 3" "id:299,phase:request,deny,log,status:403" + SecRule tx:score "@ge 3" "id:299,phase:request,deny,log,status:403" '; } } From 5f5a32c4a5d78182ac9e557d08b2da2924bb0a8d Mon Sep 17 00:00:00 2001 From: Dennus Date: Tue, 30 Oct 2018 13:30:55 +0400 Subject: [PATCH 28/30] Update ngx_http_modsecurity_body_filter.c --- src/ngx_http_modsecurity_body_filter.c | 39 ++++++++++++++++---------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index be088f9..f6fa2f2 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -57,6 +57,8 @@ if (in == NULL) { dd("body filter, recovering ctx: %p", ctx); if (ctx == NULL || r->filter_finalize || ctx->response_body_filtered) { + if (ctx && ctx->response_body_filtered) + r->filter_finalize = 1; return ngx_http_next_body_filter(r, in); } @@ -138,8 +140,8 @@ if (in == NULL) { } } #endif - - for (chain = in; chain != NULL; chain = chain->next) { + + for (chain = in; chain != NULL; chain = chain->next) { ngx_buf_t *copy_buf; ngx_chain_t* copy_chain; @@ -154,29 +156,28 @@ if (in == NULL) { return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module, ret); } + if (!chain->buf->last_buf){ copy_chain = ngx_alloc_chain_link(r->pool); if (copy_chain == NULL) { return NGX_ERROR; } - - copy_buf = ngx_calloc_buf(r->pool); + copy_buf = ngx_calloc_buf(r->pool); if (copy_buf == NULL) { return NGX_ERROR; } - copy_buf->start = ngx_pcalloc(r->pool, data_size); - if (copy_buf->start == NULL) { - return NGX_ERROR; - } - ngx_memcpy(copy_buf->start, chain->buf->pos, data_size); - copy_buf->pos = copy_buf->start ; - copy_buf->end = copy_buf->pos + data_size ; - copy_buf->last = copy_buf->pos + ngx_buf_size(chain->buf); + //copy_buf->last=ngx_cpymem(copy_buf->pos, chain->buf->pos, data_size); + copy_buf->pos = chain->buf->pos ; + copy_buf->end = chain->buf->end; + copy_buf->last = chain->buf->last; copy_buf->temporary = (chain->buf->temporary == 1) ? 1 : 0; copy_buf->memory = (chain->buf->memory == 1) ? 1 : 0; copy_chain->buf = copy_buf; - copy_chain->buf->last_buf = 1; + copy_chain->buf->last_buf = chain->buf->last_buf; copy_chain->next = NULL; chain->buf->pos = chain->buf->last; + } + else + copy_chain = chain; if (ctx->temp_chain == NULL) { ctx->temp_chain = copy_chain; } else { @@ -189,23 +190,30 @@ if (in == NULL) { } ctx->current_chain = copy_chain; } + } if (is_request_processed) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH PROCESSING"); old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_response_body(ctx->modsec_transaction); ngx_http_modsecurity_pcre_malloc_done(old_pool); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH PROCESSING RET = %d", ret); if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){ + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH DO HEADER FILTERS = %d", ret); ctx->header_pt(r); } else { - return ngx_http_filter_finalize_request(r, + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH DO FINALIZE = %d", ret); + ctx->response_body_filtered = 1; + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module , ret); } } else if (ret < 0) { + ctx->response_body_filtered = 1; return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); } @@ -214,6 +222,7 @@ if (in == NULL) { ctx->header_pt(r); return ngx_http_next_body_filter(r, ctx->temp_chain); } else { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS WAITING FOR NEXT CHUNK"); return NGX_AGAIN; } -} \ No newline at end of file +} From 9b5bd5335a2a6c75418874e68146696e49c0780c Mon Sep 17 00:00:00 2001 From: Dennus Date: Tue, 30 Oct 2018 13:34:42 +0400 Subject: [PATCH 29/30] bypass buffer data copying --- src/ngx_http_modsecurity_body_filter.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index f6fa2f2..7d5b06c 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -165,7 +165,6 @@ if (in == NULL) { if (copy_buf == NULL) { return NGX_ERROR; } - //copy_buf->last=ngx_cpymem(copy_buf->pos, chain->buf->pos, data_size); copy_buf->pos = chain->buf->pos ; copy_buf->end = chain->buf->end; copy_buf->last = chain->buf->last; @@ -192,21 +191,17 @@ if (in == NULL) { } } - + if (is_request_processed) { - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH PROCESSING"); old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_response_body(ctx->modsec_transaction); ngx_http_modsecurity_pcre_malloc_done(old_pool); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH PROCESSING RET = %d", ret); - if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){ - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH DO HEADER FILTERS = %d", ret); + if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){ ctx->header_pt(r); } - else { - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS FINISH DO FINALIZE = %d", ret); + else { ctx->response_body_filtered = 1; return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module @@ -222,7 +217,6 @@ if (in == NULL) { ctx->header_pt(r); return ngx_http_next_body_filter(r, ctx->temp_chain); } else { - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,"MDS WAITING FOR NEXT CHUNK"); return NGX_AGAIN; } } From e315f5b91044dde08b106a85fc9fe739120b301f Mon Sep 17 00:00:00 2001 From: Dennus Date: Tue, 30 Oct 2018 14:31:26 +0400 Subject: [PATCH 30/30] remove unused variables --- src/ngx_http_modsecurity_body_filter.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 7d5b06c..05fb571 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -146,7 +146,6 @@ if (in == NULL) { ngx_buf_t *copy_buf; ngx_chain_t* copy_chain; is_request_processed = chain->buf->last_buf; - ngx_int_t data_size = chain->buf->last - chain->buf->pos; u_char *data = chain->buf->pos; msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data);