Fix phase 2 and 3 audit logging in case of internal redirect

[mlevogiannis]

Pull request #241 attempted to fix the problem where the log handler was not called in case of internal redirects (e.g. when using error_page). The problem was that the log handler was registered at Nginx log phase, however processing never reached that Nginx phase in case of an internal redirect (the Nginx context of the original request was cleared and a new one was created for the internal redirect). The pull request tackled the issue by introducing the concept of “early logging”, where the process intervention function would manually call the log handler.

However, this early logging mechanism was only enabled for ModSecurity’s phase 1 (request headers). As a result, the original issue was fixed only for that phase but not for ModSecurity’s phases 2 (request body) and 3 (response headers). It did not apply to ModSecurity’s phase 4 (response body), as at this phase it not possible to perform a redirection due to Nginx limitations (see this comment for more information).

This pull request unconditionally enables early logging in the process intervention function for all ModSecurity phases, by removing the respective control flag (what it does is similar to pull request #90, on which pull request #241 is based on). The log handler should always be called in both the process intervention function and the Nginx log phase. The former is the only place in which the log handler is guaranteed to be called in case of an intervention (as demonstrated by this issue) and the latter is required so that it is called when an intervention is not triggered. The "logged" context flag ensures that logging takes place only once.

Finally, the custom error page test case has been updated to include rules of all ModSecurity phases. The tests for ModSecurity phases 2 and 3 fail with the current upstream code and succeed after the proposed fix is applied.

[kolotouros]

Any update on this?

[martinhsv]

@kolotouros : No

[liudongmiao]

@kolotouros This will duplicate log entry, and no extra response header, nor response body.

For error_page, current implementations:

1. request, no response, no log
2. second request, got response, log with response headers and body

In this patch:

1. request, no response, log without headers
2. second request, got response, log without headers

[mlevogiannis]

This pull request is more of a fix to the existing workaround ("early logging") than a proper solution to the issue caused by internal redirects.

Pull request #273 is in the correct direction, it recovers the original request's context which can then be used to log the transaction properly. However in its current state it does not properly mitigate the issue described in this pull request. Specifically, if ModSecurity is not enabled in the internal redirect's location conf, the log handler will not run at all. This is demonstrated by the updated test case included in this pull request.

If pull request #273 is merged (along with the extra changes discussed in the respective thread), this pull request can be closed.

[SWGAKamui]

Is there any news please? It seems that those commits helps my need. Thanks to the author for your contribution. :)