Fix error_page internal redirect false positive logging

[jeremyjpj0916]

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:

ce1d438

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.

[jeremyjpj0916]

@zimmerle @martinhsv @defanator @victorhora Any thoughts here?

Should be good to #ShipIt

[jeremyjpj0916]

still pending? oof.

[zimmerle]

@defanator Can you have a look at this?

[github-actions]

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

[jeremyjpj0916]

Github bot being bad again 💀.

[jeremyjpj0916]

Can say I have ran this in my fork of code for a long time 10/10 does indeed work 😆 .

[defanator]

@jeremyjpj0916 copying here as well - it is believed that 4f26b48 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.

[jeremyjpj0916]

What was wrong with the simple original change from my PR to ignore processings on error_page redirects since ModSecurity will have the wrong context post an error_page redirect?

From my OP this is still relevant:

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.

cc @airween

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud