owasp-modsecurity / ModSecurity-nginx

ModSecurity v3 Nginx Connector
Apache License 2.0
1.49k stars 277 forks source link

Synchronous process of the request destroys performance #227

Open yaa110 opened 3 years ago

yaa110 commented 3 years ago

As mentioned in nginx dev guide using blocking libraries destroys performance. For example after enabling Modsecurity module, the performance of nginx decreases about 70%.

zimmerle commented 3 years ago

Hi @yaa110, what leads you to believe that the performance drop that you are seeing is a consequence of a "blocking library" and/or ModSecurity?

yaa110 commented 3 years ago

Hi @zimmerle I performed a benchmark test with and without ModSecurity module by returning 200 for location / using wrk. The throughput of nginx without ModSecurity was about 40,000 req/s, however, after enabling ModSecurity modules, the value dropped to about 1,500 req/s. I tried to implement a non-blocking version of modsecurity using nginx thread pool (it had some known issues and segfaults but worked acceptable for benchmark test), the throughput changed to 20,000 req/s.

zimmerle commented 3 years ago

Perhaps you may want to provide a picture of the architecture of what you have done so far. That will help us foment a discussion about how the nginx connector architecture is built and understanding for a global perspective of the differences in such approaches. The concept of blocking libraries is comprehensive.

As a WAF that works-oriented by phases, ModSecurity expects to block the requests on phase 1 at phase 1. There is no point in blocking the request at response headers while identifying a malicious content at request headers phase. In that case, the attack will hit the server anyway.

As of the test case, I would not recommend optimizing ModSecurity to inspect static pages; that is not the typical use case scenario for a WAF. The percentages on performance drop make more sense in dynamic content, the content you want to have the WAF in front.

zimmerle commented 3 years ago

@yaa110 ?

yaa110 commented 3 years ago

Sorry for the late reply, the summary of architecture is as follows:

  1. Reading body (if needed) in NGX_HTTP_REWRITE_PHASE (async)
  2. Allocating a new task in NGX_HTTP_PREACCESS_PHASE handler using ngx_thread_task_alloc.
  3. Setting task->handler and task->event.handler. Modsecurity is done inside task->handler.
  4. Posting the task using ngx_thread_task_post.
  5. Dropping or bypassing the request in task->event.handler.

By this approach, nginx workers are not blocked and the throughput increases significantly.

Consider using Modsecurity in a CDN cloud company, in this situation, the performance drop is not acceptable.

zimmerle commented 3 years ago

his approach, nginx workers are not blocked and the throughput increases significantly

With that approach how can we avoid that the requests headers got blocked prior to hit the application?

Consider using Modsecurity in a CDN cloud company, in this situation, the performance drop is not acceptable.

If I read correctly, you have tested ModSecurity against a static page. Why you are interested in have static pages inspected by ModSecurity in a CDN? To test what would be the performance drop in a CDN it will be interesting to have a scenario similar to what a CDN have, don't you think?

yaa110 commented 3 years ago

Consider the following structure:

Untitled Diagram

In this structure, static and dynamic content (if cache is missed) is tested against WAF. Since, the async test is done in pre-access phase, the upstream is not affected by attacks.

zimmerle commented 3 years ago

One of the abilities of the WAF is to adrress the virtual host/folder that you want to inspect. With that ability you can not only enable/disable the WAF per vhost/folder but also load rules according to what you want to protect. The architecture in that picture seems to disable such capability.

abregar commented 2 years ago

Came across this interesting topic, it is a bit dated, but seems still open for discussion.

I have briefly reviewed the code on behavior and must agree with @yaa110 on potential unneeded blocking library usage. I see usage of nginx threads just proper to offload the lengthy operation as is expected by invoking scan inside ModSecurityLib at whatever handler (phase) request might penetrate.

Nginx requests processing are isolated and processing performance is boosted by event-loop not being blocked. Thus introducing the nginx threads would not tamper the original behavior - ergo - potential malicious request might still be blocked 'on phase 1 at phase 1'. Just not blocking the rest of requests processing already accepted in the event-loop.

(Just for community reference https://www.nginx.com/blog/thread-pools-boost-performance-9x/, knowing that both of you @zimmerle and @yaa110 know this very well).

In any case @yaa110 are you willing to share the proof of concept code in PR for the author and community review?

wfjsw commented 1 year ago

I've followed the hint provided by @yaa110 and implemented the thread pool thing here:

https://github.com/wfjsw/ModSecurity-nginx/tree/cb3f264ab77660c1e005bd455066e109a83f2da8

if anyone is interested please help review and benchmark this. On my own side I've put this onto production and it works nicely. I haven't noticed any SEGV caused by this yet.

travisbell commented 1 year ago

@wfjsw Wow, thanks! I'll give this a spin on my local setup and report back.

I had to abandon this module because the non-async design was just brutal for performance. As a comparison point, look at how Litespeed designed their modsec feature and it's many, many factors faster than this module for Nginx (using CRS). Night and day--it's not even close. So if this can get it close, it might have a chance of replacing it. I'll let you know!

wfjsw commented 1 year ago

Don't forget to apply https://github.com/SpiderLabs/ModSecurity/pull/2791 as well if you are using PCRE2.

Currently only phase 1 and 2 benefit from this. I have yet to find a solution for phase 3/4, given the phase 4 is quite heavy.

jeremyjpj0916 commented 1 month ago

Anyone ever get around to performance testing with the changes? Feels like this definitely should have been considered as I would love to be able to start using modsecurity with nginx in a more performant production environment vs small scale use cases.