Skip to content

SecStreamInBodyInspection=On causes slow uploads #1366

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

Closed
tomsommer opened this issue Mar 26, 2017 · 14 comments
Closed

SecStreamInBodyInspection=On causes slow uploads #1366

tomsommer opened this issue Mar 26, 2017 · 14 comments
Assignees

Comments

@tomsommer
Copy link
Contributor

tomsommer commented Mar 26, 2017

This post is still very very relevant: https://gryzli.info/2015/11/07/waf-modsecurity-slow-upload/
SecStreamInBodyInspection=On makes uploads extemely slow.

If this is unavoidable, then at the very least you should put a big fat warning on the usage of SecStreamInBodyInspection=on

Since StreamInBodyInspection cannot be controlled in ctl it's a big problem.

@bostrt
Copy link

bostrt commented May 27, 2017

@tomsommer Do you think adding something like below in docs would suffice?

Note: This directive may significantly impact file upload times due to added overhead. 

https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual#secstreaminbodyinspection

@tomsommer
Copy link
Contributor Author

Well, if this is working as intended, then yes.

@bostrt
Copy link

bostrt commented May 28, 2017

@tomsommer I tried running some tests myself but wasn't able to notice any performance impact. I do not doubt that there is a performance impact by enabling SecStreamInBodyInspection, my test just probably isn't decent.

Do you have a simple test you can share in here that others can see/test with?

(Would be great if the poster of https://gryzli.info/2015/11/07/waf-modsecurity-slow-upload/ would share their tests showing the 20x slow-down and high CPU usage)

Regarding the docs change, maybe we'll hold off on that until we can validate any slowdown.

@tomsommer
Copy link
Contributor Author

It's been a while so I cannot find a testcase, however I can surely confirm this issue. I see the feature is removed in v3, for good reason I guess.

@tomsommer
Copy link
Contributor Author

Maybe #1316 is related

@victorhora
Copy link
Contributor

Yes SecStreamInBodyInspection directive is no longer supported on libModSecurity (aka v3). By default v3 will try to use stream whenever it is possible. I've added a note on the docs to reflect this a couple of weeks ago.

I agree with @bostrt it would be great if the poster of that blog could share his test cases with us. But anyways I think this might be a won't fix for 2.9.x as it's already fixed by libModSecurity. I'll tag @zimmerle here for him to confirm what's the case for this one.

@zimmerle
Copy link
Contributor

zimmerle commented Jun 3, 2017

Indeed, it shouldn't be a concern for v3.x. As of v2.x I believe that it is only really workable for Apache. Depends on your server resources and request type (size) keep things in memory may be computational expensive, even if it is about a chunk to perform a stream inspection. Hence, I don't think the slowness should be considerate as a bug, as it may be normal given an environment.

@zimmerle zimmerle closed this as completed Jun 3, 2017
@bostrt
Copy link

bostrt commented Jun 3, 2017

@zimmerle do you think this thread warrants an update to the reference manual?

@zimmerle
Copy link
Contributor

zimmerle commented Jun 6, 2017

Hi @bostrt,

That may be a good idea. Do you want to perform the changes?

@bostrt
Copy link

bostrt commented Jun 6, 2017

@zimmerle Sure, I'll drop in a change today.

@zimmerle
Copy link
Contributor

zimmerle commented Jun 6, 2017

@bostrt thanks ;)

@bostrt
Copy link

bostrt commented Jun 6, 2017

@zimmerle Apparently, I cannot fork the wiki to submit a pull request. I think GitHub is lacking the feature :
Here's the best I can do: bostrt/ModSecurity-wiki@d22a1e9

@victorhora
Copy link
Contributor

That's new for me as well @bostrt. :/

Well as I was working in the manual today I just added your statement and you can see the history here: https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual/_history

Thanks! :)

@allanbomsft
Copy link

Is the problem here perhaps this line in msc_reqbody.c?:
stream_input_body = (char *)realloc(msr->stream_input_data, msr->stream_input_length + 1);
Looks like we are reallocating (malloc+memcpy) thousands of times when doing multi-megabyte file uploads. It seems to only grow by a few kilobytes at a time.

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

No branches or pull requests

5 participants