owasp-modsecurity / ModSecurity-nginx

ModSecurity v3 Nginx Connector
Apache License 2.0
1.48k stars 274 forks source link

lazy loading rules set #277

Closed liudongmiao closed 1 year ago

liudongmiao commented 2 years ago

This patch would lazy loading ModSecurity rules in nginx worker process. As the ModSecurity rules are loaded in worker process, then avoid memory leak in master process.

https://github.com/SpiderLabs/ModSecurity/issues/2381 https://github.com/SpiderLabs/ModSecurity/issues/2502 https://github.com/SpiderLabs/ModSecurity/issues/2552 https://github.com/SpiderLabs/ModSecurity/issues/2580 https://github.com/SpiderLabs/ModSecurity/issues/2636 https://github.com/SpiderLabs/ModSecurity/issues/2710

iammattmartin commented 1 year ago

@liudongmiao I've tried this, whilst it did stop nginx crashing/needing a reset, it completely broke scanning web requests correctly.

ModSecurity: Warning. Matched "Operator `Rx' with parameter `^\d+$' against variable `REQUEST_HEADERS:content-length' (Value: `345' ) [file "/etc/nginx/modsec/rules/owasp/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "127"] [id "920160"] [rev ""] [msg "Content-Length HTTP header is not numeric"] [data "345"] [severity "2"] [ver "OWASP_CRS/3.3.2"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-protocol"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/210/272"] [hostname "95.215.174.211"] [uri "/contact-us/"] [unique_id "166187881671.297424"] [ref "v362,3"]
ModSecurity: Warning. Matched "Operator `Rx' with parameter `^[\w/.+-]+(?:\s?;\s?(?:action|boundary|charset|type|start(?:-info)?)\s?=\s?['\"\w.()+,/:=?<>@-]+)*$' against variable `REQUEST_HEADERS:content-type' (Value: `application/x-www-form-urlencoded; charset=UTF-8' ) [file "/etc/nginx/modsec/rules/owasp/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "915"] [id "920470"] [rev ""] [msg "Illegal Content-Type header"] [data "application/x-www-form-urlencoded; charset=utf-8"] [severity "2"] [ver "OWASP_CRS/3.3.2"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-protocol"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/255/153"] [tag "PCI/12.1"] [hostname "95.215.174.211"] [uri "/contact-us/"] [unique_id "166187881671.297424"] [ref "v264,48t:lowercase"]
ModSecurity: Warning. Matched "Operator `Rx' with parameter `^(?i:(?:[a-z]{3,10}\s+(?:\w{3,7}?://[\w\-\./]*(?::\d+)?)?/[^?#]*(?:\?[^#\s]*)?(?:#[\S]*)?|connect (?:\d{1,3}\.){3}\d{1,3}\.?(?::\d+)?|options \*)\s+[\w\./]+|get /[^?#]*(?:\?[^#\s]*)?(?:#[\S]*)?)$' against variable `REQUEST_LINE' (Value: `GET /contact-us/ HTTP/2.0' ) [file "/etc/nginx/modsec/rules/owasp/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "47"] [id "920100"] [rev ""] [msg "Invalid HTTP Request Line"] [data "GET /contact-us/ HTTP/2.0"] [severity "4"] [ver "OWASP_CRS/3.3.2"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-protocol"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/210/272"] [hostname "95.215.174.211"] [uri "/contact-us/"] [unique_id "166187881671.297424"] [ref "v0,25"]
ModSecurity: Access denied with code 403 (phase 2). Matched "Operator `Ge' with parameter `5' against variable `TX:ANOMALY_SCORE' (Value: `13' ) [file "/etc/nginx/modsec/rules/owasp/REQUEST-949-BLOCKING-EVALUATION.conf"] [line "80"] [id "949110"] [rev ""] [msg "Inbound Anomaly Score Exceeded (Total Score: 13)"] [data ""] [severity "2"] [ver "OWASP_CRS/3.3.2"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-generic"] [hostname "95.215.174.211"] [uri "/contact-us/"] [unique_id "166187881671.297424"] [ref ""]

The above were three valid requests. Content-Type was correct, Length was correct and the request was. Going back to the stock/master code immediately stopped these being generated.

liudongmiao commented 1 year ago

@iammattmartin It's nothing with this PR.

  1. what's your pcre version in nginx and in modsecurity? (if you don't link pcre in modsecurity statically and change prefix.)
iammattmartin commented 1 year ago

I believe PCRE(1).

configure arguments: --with-cc-opt='-g -O2 -fdebug-prefix-map=/build/nginx-7KvRN5/nginx-1.18.0=. -fstack-protector-strong -Wformat -Werror=format-security -fPIC -Wdate-time -D_FORTIFY_SOURCE=2' --with-ld-opt='-Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now -fPIC' --prefix=/usr/share/nginx --conf-path=/etc/nginx/nginx.conf --http-log-path=/var/log/nginx/access.log --error-log-path=/var/log/nginx/error.log --lock-path=/var/lock/nginx.lock --pid-path=/run/nginx.pid --modules-path=/usr/lib/nginx/modules --http-client-body-temp-path=/var/lib/nginx/body --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-proxy-temp-path=/var/lib/nginx/proxy --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-debug --with-compat --with-pcre-jit --with-http_ssl_module --with-http_stub_status_module --with-http_realip_module --with-http_auth_request_module --with-http_v2_module --with-http_dav_module --with-http_slice_module --with-threads --with-http_addition_module --with-http_geoip_module=dynamic --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module=dynamic --with-http_sub_module --with-http_xslt_module=dynamic --with-stream=dynamic --with-stream_ssl_module --with-stream_ssl_preread_module --with-mail=dynamic --with-mail_ssl_module --add-dynamic-module=/build/nginx-7KvRN5/nginx-1.18.0/debian/modules/http-auth-pam --add-dynamic-module=/build/nginx-7KvRN5/nginx-1.18.0/debian/modules/http-dav-ext --add-dynamic-module=/build/nginx-7KvRN5/nginx-1.18.0/debian/modules/http-echo --add-dynamic-module=/build/nginx-7KvRN5/nginx-1.18.0/debian/modules/http-upstream-fair --add-dynamic-module=/build/nginx-7KvRN5/nginx-1.18.0/debian/modules/http-subs-filter --add-dynamic-module=/build/nginx-7KvRN5/nginx-1.18.0/debian/modules/http-geoip2

I'm sure I tried a newer version of nginx when building originally and had to revert to this version due to that as pagespeed didn't want to work with PCRE2 (from memory).

liudongmiao commented 1 year ago

@iammattmartin This PR does nothing with actual rules. You can compile again, with a patch -R.

And, for the memory issue (nginx -s reload), it should have been fixed in https://github.com/SpiderLabs/ModSecurity/pull/2728, you can avoid this patch.

liudongmiao commented 1 year ago

@iammattmartin BTW, we have use it on production for at least 3 months.

iammattmartin commented 1 year ago

Thanks for the reference to https://github.com/SpiderLabs/ModSecurity/pull/2728, we've put this back to the master release and tried that. Rules seem to be obeyed correctly and we'll monitor for any crashing.

guelfey commented 1 year ago

I can confirm that at least for our setup (nginx 1.19, PCRE 1 8.45, ModSecurity 3.0.5) this actually breaks Regexp-based rule evaluation since PCRE fails to allocate memory, and ModSecurity treats this as silently assuming that no regexps ever match. I believe this is due to the calls to ngx_http_modsecurity_pcre_malloc_init missing before calling msc_rules_add etc., so PCREs memory allocation pool is not initialized at that time. Furthermore this seems to break configs where there are multiple modsecurity_rules_file directive since they now overwrite each other and only the last one is actually loaded later.

liudongmiao commented 1 year ago

@guelfey Thanks.

  1. Yes, I forget to call ngx_http_modsecurity_pcre_malloc_init and ngx_http_modsecurity_pcre_malloc_done before load msc rules. Just fix.

In our production, we prevent modsecurity to use pcre from nginx by using a different prefix. Our production still use centos 6, so we compile modsecurity to link pcre statically with a different prefix.

  1. Yes, it doesn't support multiple modsecurity_rules_file. In our production, there are about 50+ virtual server, and we use the same rules. So, this solution can save memory, as all the virtual server shares the same modsecurity instance.

And, as https://github.com/SpiderLabs/ModSecurity/pull/2728 actually fix the memory problem, I would close this PR.