Make ModSecurity IIS work with SecStreamInBodyInspection option disabled to prevent memory leak #104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vkrivopalov
requested review from
allanbomsft,
alonzop,
appgwwaf,
asridharan,
azhao155 and
meetug
vkrivopalov
assigned vkrivopalov and unassigned asridharan, alonzop, azhao155, allanbomsft, meetug and appgwwaf
vkrivopalov
force-pushed
the
vlkrivop/disable_stream_in_body_inspection
branch
from
28297cc to
c58266f
Compare
allanbomsft
previously approved these changes
Mar 13, 2019
If we set EOS bucket to the brigade and assign this brigade to APR request, it causes an error code returned from modsecProcessRequest(). This does not affect detection-only mode because we dismiss the code but needs to be taken into account for prevention mode. Signed-off-by: Vladimir Krivopalov <vlkrivop@microsoft.com>
Use the same helper that has been previously only called for detection-only mode when prevention mode is enabled. Signed-off-by: Vladimir Krivopalov <vlkrivop@microsoft.com>
Signed-off-by: Vladimir Krivopalov <vlkrivop@microsoft.com>
vkrivopalov
force-pushed
the
vlkrivop/disable_stream_in_body_inspection
branch
from
c58266f to
87bd3ad
Compare
allanbomsft
approved these changes
Mar 14, 2019
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
There is a long-time known memory leak that occurs to requests blocked by ModSecurity when
SecStreamInBodyInspection. The primary reason for keeping this option is that ModSecurity for IIS needs it turned on - see owasp-modsecurity#538 for details.This is bad, however, because this flag causes a known memory leak: owasp-modsecurity#1316
We have earlier disabled this option for ModSecurity for Nginx but couldn't do so for IIS for it would break processing.
Now we can leverage the code implemented for processing requests in background for detection-only mode and not depend on
SecStreamInBodyInspectionbeing set any longer.NB: there are some other memory leaks coming from other places even when body processing is disabled. They leak much less memory so are not addressed by this fix but should certainly be chased later.
Testing
I've been using requests with 128Kb body carrying randomly generated payload:
Prevention mode, SecStreamInBodyInspection On
Before any requests: 5.3 Mb
After first request (config loaded): 22.6 Mb
After 100 more requests (101 overall): 36.6 Mb
After another 100 requests (201 overall): 49.1 Mb
Prevention mode, SecStreamInBodyInspection Off
Before any requests: 5.2 Mb
After first request (config loaded): 22.3 Mb
After 100 more requests (101 overall): 23.2 Mb
After another 100 requests (201 overall): 23.6 Mb