o_append in addition to o_wronly in Parallel logging v3/master

[met3or]

Hi all,

I've been looking into an ongoing issue with getting v3/master to work smoothly with SELinux and it does for the most part, but the logging element is still having minor issues:

such as:

type=SYSCALL msg=audit(1505122867.651:719700): arch=c000003e syscall=2 success=no exit=-13 a0=7f3bb4787ab8 a1=41 a2=1a4 a3=6 items=0 ppid=72017 pid=72028 auid=4294967295 uid=988 gid=985 euid=988 suid=988 fsuid=988 egid=985 sgid=985 fsgid=985 tty=(none) ses=4294967295 comm="nginx" exe="/usr/sbin/nginx" subj=system_u:system_r:httpd_t:s0 key=(null)  

type=AVC msg=audit(1505122867.651:719700): avc:  denied  { write } for  pid=72028 comm="nginx" path="/var/log/nginx/17486_013.example_0/20170911/20170911-1041/20170911-104107-150512286779.251763" dev="dm-0" ino=467940 scontext=system_u:system_r:httpd_t:s0 tcontext=system_u:object_r:httpd_log_t:s0 tclass=file

Where when using the http_t context and http_log_t object the system is unable to write to the defined modsecurity log directory in parallel mode, it creates the folders and file correctly but does not populate this file with data.

https://github.com/SpiderLabs/ModSecurity/blob/04f7009980d6715749de697710a640fd54a32789/src/audit_log/writer/parallel.cc#L136-L141

I have pin-pointed the issue to this section of code where we're writing to the file rather than appending, which would be allowed in the context type http_t.

For the time being to get this working I have created an SELinux policy which allows this action, but, for ootb functionality which works with existing context policies I believe O_WRONLY should be O_APPEND.

fd = open(fileName.c_str(), O_CREAT | O_WRONLY

I believe should be:

fd = open(fileName.c_str(), O_CREAT | O_APPEND

This change has not yet been tested, and I've not checked the rest of the source code (also my knowledge of cpp is low), but I am curious if others feel this change would be useful.

edit: after a bit more reading, the O_WRONLY will still be needed, but will require the addition of O_APPEND to ensure the write is appended.

nginx do it this way in their code:

#define NGX_FILE_APPEND (O_WRONLY|O_APPEND)
ngx_log_file.fd = ngx_open_file(name, NGX_FILE_APPEND,

As an example,

Regards,

David

[met3or]

Following some tests building with these changes:

from:
https://github.com/SpiderLabs/ModSecurity/blob/04f7009980d6715749de697710a640fd54a32789/src/audit_log/writer/parallel.cc#L145-L149

to:

    std::ofstream myfile;
    std::string a(fileName.c_str());
    myfile.open(a, std::ofstream::out | std::ofstream::app);
    myfile << log;
    myfile.close();

Rather than what had originally been proposed, seems to fix my issue, this compiles fine for me on C7 and works well with the http_t SELinux policy.

Is this something that can be considered for merge?

[zimmerle]

Hi @met3or,

Thanks for look into this. I had not time to play with SELinux vs ModSecurity v3 yet. I consider your findings very important. And yes, that is definitively something that we want to merge! I went to your three and extracted the patches from there, just to save you the trouble of opening a pull request.

Changes are now on our buildbots. Thanks!