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

Synchronous process of the request destroys performance #227

Open
yaa110 opened this issue Oct 7, 2020 · 13 comments · May be fixed by #289
Open

Synchronous process of the request destroys performance #227

yaa110 opened this issue Oct 7, 2020 · 13 comments · May be fixed by #289
Assignees

Comments

@yaa110
Copy link

yaa110 commented Oct 7, 2020

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

zimmerle commented Oct 7, 2020

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?

@zimmerle zimmerle self-assigned this Oct 7, 2020
@yaa110
Copy link
Author

yaa110 commented Oct 8, 2020

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

zimmerle commented Oct 8, 2020

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

@yaa110 ?

@yaa110
Copy link
Author

yaa110 commented Oct 14, 2020

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

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

yaa110 commented Oct 15, 2020

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

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

abregar commented Dec 13, 2021

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

wfjsw commented Aug 22, 2022

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

travisbell commented Oct 27, 2022

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

wfjsw commented Oct 27, 2022

Don't forget to apply owasp-modsecurity/ModSecurity#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
Copy link

jeremyjpj0916 commented Jun 17, 2024

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.

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 a pull request may close this issue.

6 participants