-
Notifications
You must be signed in to change notification settings - Fork 281
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
Detected request method (GET) sometimes does not match actual method (POST) #152
Comments
👍 same issue here, we can reproduce the bug everytime when we attach particular body (file to the POST request) I will come back here once i will get the file. |
@kjakub Very interesting. Can you share more details? Maybe we can come up with a test case. That will make it more likely for the problem to be solved fast. |
@theseion sorry, false alarm that we are NOT able reproduce everytime. Seems random too :-( |
Bummer. Thanks for checking. |
Hi @theseion, Can you tell us the exact version of your libModSecurity? So that we can try to replicate the issue here as well |
We built libmodsecurity with commit owasp-modsecurity/ModSecurity@6d5198b. |
@zimmerle @theseion just yesterday, we registered one more occurrence of this case
Our nginx is running on latest modsecurity everytime we build - which was 2019/03/11 (March 11, 2019) - latest build which was deployed regarding to cicd pipeline. We are using slightly modified docker image which i publish here https://github.com/kjakub/docker-nginx-modsecurity-v3-waf and the core of build is here https://github.com/kjakub/docker-nginx-modsecurity-v3-waf/blob/master/waf/build.sh |
Thank you for all the details guys, sorry for the trouble and the time to answer. I will have a look at it ASAP. That kind of issue is annoying. |
Let me know if I can help in any way. |
For us the behaviour is consistent within one api endpoint. CRS version: v3.1.0 |
I was experiencing similar issues, so i tailed the debug log as follows
turns out, that the POST requests that were appearing in the audit log as a GET were actually being processed twice with the first post request not being sent to the audit log
Not exactly sure what this means, but hopefully this info is helpful |
Update: Seems like every request that gets blocked gets processed as the original method type and then again as a GET, including GET itself which gets processed twice The nginx logs show only one request. Not sure if my experience is related to this issue or just a bad config. If anyone has any suggestions or other tests they think I should run, please let me know. Thanks |
Hi, Can you confirm that it is still an issue within the most recent version of the library+connector? |
I'll have to build it and see what happens. I'll get back to you ASAP. |
Thank you @theseion! |
Hi @theseion, any news? |
Not yet, sorry. Because the requests are so rare I'll have to make a deployment first and then watch the log for a bit. That will take me at least 3 to 4 days to be sure. |
oh I see. thank you! |
It has taken me a bit longer to set up the updated version. I'm now monitoring the logs. I'll get back to you ASAP. |
I still see those bad matches. I found a whole batch from today and the metrics may be interesting: 12 requests within a 5 minute window from three unique IP's and two unique Web server: NGiNX 1.14.2 |
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days |
Still relevant. |
@victorhora Please reopen this issue. |
There is one nginx expected behaviour that may be related to at least some of the reports in this thread. If your config includes error page redirects something like this:
Then, when such a redirect occurs, it is expected behaviour that the original POST method will be converted to a GET ... but the non-zero Content-Length header is retained, along with the original request's body content. See https://trac.nginx.org/nginx/ticket/876 for more information. Near the bottom of that article are suggestions on how to drop the body for such redirects, as well as zero-ing the Content-Length value (I have not actually tried these). Might this explain what's being reported here? Possibly. In particular, the reports from Aug. 6 appear to fit this pattern, with ModSecurity detections always resulting in a second request reported (with non-GET/HEAD requests resulting in the 2nd request as a GET). This could potentially also explain what the OP has reported. Suppose an occasional POST request results in a 50x within nginx itself or the connector. Then ModSecurity would never analyze the original POST request, but would instead see a GET request with a body and a nonzero Content-Length. What happens in such cases may be less obvious than one might think from the logs. In my experiments, the second request (e.g. a GET request that has been redirected to 403.html) still shows up in the logs with the original url (e.g. /testpost.php rather than /403.html). There are a few ways that you can confirm if this is what is happening for you. Perhaps the simplest is to experimentally turn off ModSecurity within the error-redirect location blocks. |
You assessment sounds very accurate as in fact i am using custom error redirects. |
@martinhsv Wow! Nice find! I think you're on to something. I went through the logs to check the request I had reported above and there was indeed a connection timeout in the proxy server. We also do use |
Based on the feedback last week and the lack of any new discussion since, I'm assuming that we think we have a satisfactory explanation for these reports. Unless I hear objections in the meantime, I'll go ahead and close this at eod tomorrow. |
Fine by me. I'll probably implement one of the suggestions from the bug ticket. |
I'd like to share my solution for working around the request method conversion. It's a bit more complicated than I would have liked but I had to work around some limitations (e.g. that status codes can't be variables). Quick note on the setup: we use NGiNX as reverse proxy with multiple backend web servers. We don't interfere with errors returned from the downstream servers except for 5xx errors (note that there appears to be a bug in NGiNX so that my idea fails to convert 502 to 503):
This is the error configuration we use:
|
@martinhsv
Marcello |
Hello @cello86 , I'm not sure I understand what you are asserting here and for what purpose. The reason this issue was closed was because a POST being transformed into a GET due to an error redirect is expected behaviour in nginx. |
Hi @martinhsv
Could it be mod_security matching rule? On modsec_audit.log we noticed that all warning were created by the GET method:
If we disable the SecRequestBodyAccess parameter the POST action passes without issues. Marcello |
It looks like the same issue. But I can only reiterate what @martinhsv has already written: it's NGINX, not ModSecurity. What you need is an NGiNX configuration that catches error responses before they are handled internally, like I have done in my workaround above. The only other option you have is to disable ModSecurity / CRS stuff, which you definitely shouldn't.
|
@theseion we have already implemented the suggestion:
|
It has been a long time since I wrote the workaround, so I have to do some guessing here. What I notice is that your location above does not use |
still confused |
Moved here from SpiderLabs/owasp-modsecurity-crs#1304.
Type of Issue
Incorrect blocking (false positive)
Description
I find sporadic instances of requests blocked by the protocol enforcement rule that checks whether GET or HEAD requests have a Content-Length header > 0. When I try to match the requests to the NGiNX access log I see POST requests (as expected). I can reproduce the requests but haven't found a way to reproduce the blocking. Note that this occurs with different URL's and different body data as well.
The frequency of these events is around 1 to 2 a day in around 20000 POST requests.
audit log sample:
Matching request from NGiNX access log:
Your Environment
Confirmation
[x] I have removed any personal data (email addresses, IP addresses,
passwords, domain names) from any logs posted.
The text was updated successfully, but these errors were encountered: