-
Notifications
You must be signed in to change notification settings - Fork 300
Put the ModSecurity interception logic onto separated threads #289
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
base: master
Are you sure you want to change the base?
Conversation
|
Hi @wfjsw, there is a new CI workflow test in this repository. Could you pick up the modifications to enable run those tests? Thanks! |
|
I no longer have a ModSecurity install on my machine so I'm unable to investigate further :( Re performance issue: From the CPU loads it still seems heavy. I'd say there is real computing constraint in WAF. (Or it might be a problem caused by PCRE2. Who knows) |
|
Hi @airween, |
|
Hi @SonNgo2211,
Thanks for this link. I'm not a LiteSpeed expert (I haven used that at all yet), but I think the key is this: As I understand that the solution is based on a web server (LiteSpeed) function. So there is no similar function in Nginx, I'm afraid there are no plans to use this mechanism in Nginx. |
|
I noticed that Nginx supports asynchronous thread pools for offloading blocking main working cycle (see: https://www.nginx.com/blog/thread-pools-boost-performance-9x/). As far as I know, ModSecurity/ModSecurity-Nginx connector run synchronously within the Nginx worker’s event loop. This means heavy or complex rules can still block the worker, and the Nginx thread pools cannot currently be used to offload WAF evaluation away from the main event loop. This PR discussing separated threads for ModSecurity (similar solution of LiteSpeed). Is there any technical constraint or design decision that prevents such an approach in connector Modsecurity-Nginx? I’d love to learn more. Thanks @airween! |
Sorry for my initial confusion! I would still be interested to learn if there are any future plans/discussions about supporting asynchronous/multi-threaded WAF assessments in the ModSecurity-nginx connector. I will continue to monitor the issue owasp-modsecurity/ModSecurity#3215 |
Thanks again for bringing up this topic! Actually there is no any plan/discussion about async/mt threaded developing, but this does not mean we don't want to discuss it. Currently I'm working on the libmodsecurity3's API changes (it's necessary to change the current API), then I have to align the connector for it. Probably the connector version will be 1.1.0 then. After that, we can start to think about this topic. |
|
Thanks a lot @airween! |
This PR is created to describe my patch to fix #227 and it is by no means a complete patch ready for merge.
The patch contains several unrelated changes, namely:
ddtongx_log_errorto accomodate my own debugging needngx_http_modsecurity_pcre_malloc_initandngx_http_modsecurity_pcre_malloc_done. They are not used in my configuration where PCRE2 is used, and it looks suspicious for SEGVs so I commented them out as a precaution.NGX_OKin logging handler? Changed toNGX_DECLINED.Not yet implemented:
NGX_THREADSguard for Nginx setup without threading support.Currently it passes all test suites and performs well in production.
Benchmarking is welcomed.