Support reopening audit logs #2304
Conversation
|
This is failing cppcheck. Will take a look and fix. |
|
Actually, the warning appears to be unrelated to this patch:
|
|
Sorry, I was incorrect. The previous warning is the only warning I'm seeing on my dev machine. There is a relevant warning from TravisCI running
|
|
I added the above warning to the cppcheck suppressions list because |
|
Regarding thread-safety: I was reading the nginx source this evening and noticed it redirects stderr using a POSIX function, This looks like a good way to address the thread-safety issue with the current SharedFiles::reopen() in this PR. MS also provides an implementation of dup2 for POSIX compliance, and given that the POSIX docs require atomicity, it seems like this should be thread-safe on Windows as well. |
|
What's the current status of this PR? |
|
Is this pull request getting attention? The code in this pull request will support the rotation of the audit log in Nginx. More specifically, the code in this pull request is required before ModSecurity-nginx pull request owasp-modsecurity/ModSecurity-nginx#198 can proceed. |
|
There should be a reopen for debug log. I'd like to use a flush on |
|
Any chance of getting this fixed? Sort of annoying not being able to reliably rotate logs |
|
As @liudongmiao mentioned here, this does not reopen the debug log. It would be good to generalize this PR to reopen logs in general. I don't have time to work on this immediately but hope to get back to it in the next month or so. If anyone else wants to work on this before then, please feel free. |
Any news on this? |
|
Hi @Orgoth, thanks for bringing this up - I try to take a look at this soon. |
|
Just tried to apply the patch but it failed. I inserted the code my self lets see if it works. Thank you for your help airween ^^ Well it didn't work |
This code was written by: @brandonpayton Co-authored-by: @brandonpayton
Signed-off-by: Kasumi <kasumi@kitsune.exposed>
This PR adds support for reopening log files by:
msc_rules_reopen_audit_log(rules, error)function to trigger audit log reopen for a given rules setreopen(error)method toAuditLogreopen(error)method to the audit logWriterinterfacereopen(filename, error)method toSharedFilesSharedFiles::reopen()to be thread-safe, and thread-safety must be ensured by the caller. Even if the SharedFiles general lock could be used, we may not want to lock for all SharedFile writes. Please let me know if you'd like to resolve the issue as part of this PR or a later PR.This is based on work to support ModSecurity audit log rotation within Automattic. It is used in a ModSecurity-nginx patch, and the PR for that is here.
resolves #1968