ModSecurity-nginx
ModSecurity-nginx copied to clipboard
Fix error_page internal redirect false positive logging
Fixes: #182
Problem here was error_page redirects don't maintain original client request data. Hence ModSecurity has false information when evaluating the clients HTTP request a second time around during the request(HTTP Method Header is always reset to GET, URI is also not the original from client request). It is also inefficient to evaluate the request data a second time around after the internal redirect, hence cutting out those phases the second time around after the redirect makes sense and is a performance improvement.
To figure it out I was reviewing prior commits, specifically:
https://github.com/SpiderLabs/ModSecurity-nginx/commit/ce1d438bc665c28853eb5b429aad515d42027919
Which regarding this commit did indeed ensure the log phase still runs during error_page internal redirect, but just had not considered the implications of re-processing the NGX request phases(phases prior to the proxy_pass directive for instance) again.
Currently running these changes in our dev and stage environments personally. Will be pushing to our prod in the next week or so, will run as a fork until this is merged.
@zimmerle @martinhsv @defanator @victorhora Any thoughts here?
Should be good to #ShipIt
still pending? oof.
@defanator Can you have a look at this?
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
Github bot being bad again 💀.
Can say I have ran this in my fork of code for a long time 10/10 does indeed work 😆 .
@jeremyjpj0916 copying here as well - it is believed that https://github.com/SpiderLabs/ModSecurity-nginx/commit/4f26b48998db5119ca818b0909b6b14e08ebb544 should resolve at least some of observed misbehaviours reported in #182.
If it doesn't, I would appreciate a test case demonstrating the issue with minimal configuration involved.