For some reason, logrotate tries to read "." here: https://fedorahosted.org/logrotate/browser/trunk/config.c#L305 "." = "/root" when run from cron. This is a problem on selinux systems. I get these selinux denials: type=AVC msg=audit(1226489667.211:371): avc: denied { read } for pid=2291 comm="logrotate" name="root" dev=dm-0 ino=2162689 scontext=system_u:system_r:logrotate_t:s0-s0:c0.c1023 tcontext=system_u:object_r:admin_home_t:s0 tclass=dir type=SYSCALL msg=audit(1226489667.211:371): arch=40000003 syscall=5 success=no exit=-13 a0=80525d3 a1=8000 a2=0 a3=8000 items=0 ppid=2289 pid=2291 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=39 comm="logrotate" exe="/usr/sbin/logrotate" subj=system_u:system_r:logrotate_t:s0-s0:c0.c1023 key=(null) The selinux developers can add a policy exception for this, but it seems unnecessary - why should logrotate require access to /root? Could that be removed? This bug report was also posted at https://fedorahosted.org/logrotate/ticket/1, but it's ticket #1, which seems a little... odd. - Mike
Created attachment 323581 [details] Patch, so logrotate can co-exist with selinux I think this one-line patch should fix logrotate + selinux. logrotate checks to see if "." is readable. If not, logrotate used to print an error message and stop execution. The patch keeps the error message, but continues execution, since access to "." is not normally needed (unless the config files are sloppy and do not include full paths - but that would be unusual - and the program will now simply report errors when it tries to access "."). - Mike
But why not report the error only if the config files are sloppy and try to read a file via relative paths? Since there is no error, here why report anything?
I'm not sure I understand all of the use cases of logrotate, so I tried to keep it simple. Are you suggesting that relative path names be prohibited in config files? (Does anybody run logrotate from their home directory? Relative names might be handy in a scenario like that - one config file handles all users. Dunno.) Or do you mean just delete the error messages in config.c at the open and fchdir functions (proceed silently if they don't work)? - Mike
06 if (here < 0) { 307 message(MESS_ERROR, "cannot open current directory: %s\n", 308 strerror(errno)); 309 return 1; 310 } What is the purpose of this code at all. I don't know why you are checking? I would say don't check. Just try to read the path and if this fails report it.
Created attachment 323902 [details] Patch, so logrotate can co-exist with selinux, version2 Yes, stripping out that section seems to work fine on my systems. Can someone review/commit? (Is this the right upstream place?) - Mike
Created attachment 324170 [details] 323902: Patch, so logrotate can co-exist with selinux, version3 Hello Daniel and Michael, this can be tweaked a bit more: it seems the variable "here" does not make sense at all: apart from the useless check, it is used only in "fchdir(here)" which can be changed to "chdir(".")" thus, the occasional "close(here)" can be omitted, too,...
What purpose does the chdir(".") serve anyway? - Mike
(In reply to comment #7) > What purpose does the chdir(".") serve anyway? > > - Mike oh, now I see the purpose: you open the directory, got the filehandle, then chdir somewhere else and after that chdir back to the previous place the question is what can you do, if selinux prevents you to get the filehandle for "here"... question is if there is a better way (selinux-proof) to do this
I think that this situation only arises if the config file(s) point to log files using relative paths. Is that a real scenario? I think any normal config file would always have absolute paths ("/var/log/messages"). Using relative paths seems like a bad idea, since it relies on ".". But I might not understand all use cases for logwatch. I would suggest considering modifying basenames.c:ourDirName to abort with an error if a relative path is specified, instead of using "." But I'm OK with the current patch too... - Mike
OK... I released logrotate-3.7.7-3.fc11 which removes the check and "return 1", but keeps the filehandle "here", so it serves its previous purpose (remembering your position in the directory tree) when it works -i.e. I used your patch instead of mine after all... if there are any other ideas, feel free to reopen the bug