Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lazy loading rules set #277

Closed

Conversation

liudongmiao
Copy link

No description provided.

@liudongmiao
Copy link
Author

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.

owasp-modsecurity/ModSecurity#2381
owasp-modsecurity/ModSecurity#2502
owasp-modsecurity/ModSecurity#2552
owasp-modsecurity/ModSecurity#2580
owasp-modsecurity/ModSecurity#2636
owasp-modsecurity/ModSecurity#2710

@iammattmartin
Copy link

@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
Copy link
Author

@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
Copy link

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
Copy link
Author

@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 owasp-modsecurity/ModSecurity#2728, you can avoid this patch.

@liudongmiao
Copy link
Author

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

@iammattmartin
Copy link

Thanks for the reference to owasp-modsecurity/ModSecurity#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
Copy link

guelfey commented Sep 14, 2022

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.

nginx with pcre prevent original pcre_malloc and pcre_free
ngx_http_modsecurity_pcre_malloc_init/done is required before call pcre.
@liudongmiao
Copy link
Author

@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 owasp-modsecurity/ModSecurity#2728 actually fix the memory problem, I would close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants