Summary: | >=sys-apps/{portage-2.1.10.5,2.2.0_alpha44} and >=app-admin/logrotate-3.8.0 fail to rotate summary.log | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Malte Starostik <bugs> |
Component: | [OLD] Core system | Assignee: | Portage team <dev-portage> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | clemente.aguiar, dang, darkside, gentoo, mikel, tobias.pal |
Priority: | Normal | Keywords: | InVCS |
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 373933 | ||
Attachments: | run logrotate as root:portage |
Description
Malte Starostik
2011-08-09 09:16:41 UTC
Created attachment 282657 [details, diff]
run logrotate as root:portage
This still keeps logrotate from complaining about the directory permissions but also makes rotating actually work ;)
*** This bug has been marked as a duplicate of bug 374287 *** Sorry, but this is a different issue that is only caused by the fix for bug 374287. Simple enough solution: make the elog directory group writable by portage. I'm not sure the solution given in the patch is good, since it seems to work around the fix for the CVE. Sure, it kills the warning, but it still allows modifying files as root, which I believe allows the symlink attack. (In reply to comment #4) > Simple enough solution: make the elog directory group writable by portage. I'm > not sure the solution given in the patch is good, since it seems to work around > the fix for the CVE. Sure, it kills the warning, but it still allows modifying > files as root, which I believe allows the symlink attack. That won't cut it: # logrotate -f /etc/logrotate.conf error: error setting owner of /var/log/portage/elog/summary.log-19700101.gz: Die Operation ist nicht erlaubt # ls -ld $(pwd) drwxrws--- 2 portage root 4096 10. Aug 14:43 /var/log/portage/elog # chown portage:portage . # logrotate -f /etc/logrotate.conf error: error setting owner of /var/log/portage/elog/summary.log-19700101.gz: Die Operation ist nicht erlaubt Please note that this is not a warning but a fatal error and with each run of logrotate, an empty logrotate_temp.XXXXXX file is created to stay until manually cleaned up. The problem seems to be that the directory is owned by portage:root, but the log file is not: # ls -l -rw-rw-r-- 1 root root 317 Aug 9 11:44 summary.log (In reply to comment #5) > Please note that this is not a warning but a fatal error and with each run of > logrotate, an empty logrotate_temp.XXXXXX file is created to stay until > manually cleaned up. The problem seems to be that the directory is owned by > portage:root, but the log file is not: > > # ls -l > -rw-rw-r-- 1 root root 317 Aug 9 11:44 summary.log This should fix it: chown root:portage /var/log/portage/elog/summary.log Otherwise, portage will copy the group permission bits from the parent directory the next time that it writes to the file, as shown in the mod_save_summary changes in the following commit: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=4d689ffbe80b8d5038cc0105ace69de7b80e6cb1 (In reply to comment #6) > (In reply to comment #5) > > Please note that this is not a warning but a fatal error and with each run of > > logrotate, an empty logrotate_temp.XXXXXX file is created to stay until > > manually cleaned up. The problem seems to be that the directory is owned by > > portage:root, but the log file is not: > > > > # ls -l > > -rw-rw-r-- 1 root root 317 Aug 9 11:44 summary.log > > This should fix it: > > chown root:portage /var/log/portage/elog/summary.log Can the ebuild do that, please. I don't want to manually update XX machines. Like I mentioned at the end of comment #6, portage should do it automatically the next time that it writes to the file. It works by copying permissions from the /var/log/portage directory, which is fine as long as you have the default permissions set by portage. (In reply to comment #6) > This should fix it: > > chown root:portage /var/log/portage/elog/summary.log Nope. portage:root or portage:portage would do it but I'm not sure whether that'd be okay for portage. With "su portage portage" in /etc/logrotate.d/elog-save-summary, logrotate effectively runs as portage:portage. It processes the file in two passes over subsequent executions. In the first pass, it renames the existing summary.log to summary.log-$DATE. This steps succeeds as user portage has write access to the parent dir. By creating the phony summary.log-19700101, the steps to reproduce above simulate this has already happened. One thing I forgot to mention in the report is that summary.log must exist as well. The second pass then fails: As a result of the second pass, summary.log-$DATE shall be compressed to summary.log-$DATE.gz. For this, * logrotate_temp.XXXXXX is created. Due to logrotate's euid and the directory's sgid bit, this file is owned by portage:root. * now, according to strace, logrotate tries to fchown() that open temp file to root:portage, presumably in order to match the ownership of summary.log-$DATE * That fchown() fails and logrotate misleadingly barfs about setting the owner of summary.log-$DATE.gz when it actually still has the temporary name. The group ownership doesn't really matter here, the fchwon() will fail unless summary.log is owned by *user* portage or logrotate runs as root. (In reply to comment #9) > The second pass then fails: > As a result of the second pass, summary.log-$DATE shall be compressed to > summary.log-$DATE.gz. For this, > * logrotate_temp.XXXXXX is created. Due to logrotate's euid and the > directory's sgid bit, this file is owned by portage:root. It came out portage:portage when I tried it, as expected from the "su portage portage" directive in the config file. > * now, according to strace, logrotate tries to fchown() that open temp file to > root:portage, presumably in order to match the ownership of summary.log-$DATE > * That fchown() fails and logrotate misleadingly barfs about setting the owner > of summary.log-$DATE.gz when it actually still has the temporary name. It failed for me too. I think it's a bug in logrotate, since it shouldn't assume that it can chown to the owner of the file, since that would require superuser privileges. > The group ownership doesn't really matter here, the fchwon() will fail unless > summary.log is owned by *user* portage or logrotate runs as root. Right, the fchown() call is just plain wrong. logrotate shouldn't assume that will work. Suppose the owner was some other random user in the portage group. The chown call would fail then too, without superuser privileges. (In reply to comment #4) > Simple enough solution: make the elog directory group writable by portage. I'm > not sure the solution given in the patch is good, since it seems to work around > the fix for the CVE. Sure, it kills the warning, but it still allows modifying > files as root, which I believe allows the symlink attack. Well, considering that the portage group is already sensitive to attack from various angles (see bug 149062), the symlink attack seems negligible. So, the "su root portage" solution would be fine with me. My only concern is that this will stop working in the future for "security" reasons. (In reply to comment #9) > The second pass then fails: > As a result of the second pass, summary.log-$DATE shall be compressed to > summary.log-$DATE.gz. If anybody is trying to reproduce this, the simple way is to comment out "delaycompress" in /etc/logrotate.d/elog-save-summary before running `logrotate -f /etc/logrotate.conf`. That allows you to trigger the error in a single pass. I'm planning to make portage copy portage:portage ownership bits from /var/log/portage down to /var/log/portage/elog/summary.log. That way it should be compatible with the "su portage portage" directive. (In reply to comment #12) > I'm planning to make portage copy portage:portage ownership bits from > /var/log/portage down to /var/log/portage/elog/summary.log. That way it should > be compatible with the "su portage portage" directive. This is implemented in git: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=e3954bead8d7e37978cac4ae5a5ec7836d3dcd1c (In reply to comment #7) > Can the ebuild do that, please. I don't want to manually update XX machines. We had a problem with the default permissions reported in bug 377177, so I've updated the ebuild to initialize /var/log/portage permissions in pkg_preinst: http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/sys-apps/portage/portage-2.1.10.11.ebuild?view=log#rev1.4 This is fixed in 2.1.10.11 and 2.2.0_alpha51. |