[Bug 42] New: capabilities, chown, chmod
https://bugzilla.balabit.com/show_bug.cgi?id=42 Summary: capabilities, chown, chmod Product: syslog-ng Version: 3.0.x Platform: PC OS/Version: Linux Status: NEW Severity: normal Priority: unspecified Component: syslog-ng AssignedTo: bazsi@balabit.hu ReportedBy: zbyniu@pld-linux.org Type of the Report: bug Estimated Hours: 0.0 Let's take a look at syslog-ng-3.0.1/src/affile.c lines 60-83 1. CAP_SYS_ADMIN is needed only for /proc/kmsg, it is added w/o check 2. CAP_DAC_READ_SEARCH should be added only if open fail with errno 13 2a. CAP_DAC_OVERRIDE should be added only if open fail with errno 13 and with CAP_DAC_READ_SEARCH set 3. fchown needs CAP_CHOWN unconditionaly 4. fchmod needs CAP_FOWNER if file owner != euid (root here) 5. all caps should be restored summary: - CAP_SYS_ADMIN and CAP_DAC_OVERRIDE are set always even if unnecessary, and permanently - owner, group and perm doesn't work -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 Arkadiusz Miśkiewicz <arekm@maven.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arekm@maven.pl -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 --- Comment #1 from Balazs Scheidler <bazsi@balabit.hu> 2009-04-16 10:16:08 --- thanks for the report, I'll look into fixing this. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED --- Comment #2 from Balazs Scheidler <bazsi@balabit.hu> 2009-04-29 17:39:19 --- (In reply to comment #0)
Let's take a look at syslog-ng-3.0.1/src/affile.c lines 60-83
1. CAP_SYS_ADMIN is needed only for /proc/kmsg, it is added w/o check 2. CAP_DAC_READ_SEARCH should be added only if open fail with errno 13 2a. CAP_DAC_OVERRIDE should be added only if open fail with errno 13 and with CAP_DAC_READ_SEARCH set
well, I wouldn't want to complicate enabling those capabilities too much. Currently those capabilities are only enabled for /proc/kmsg and nothing else. (see the check for AFFILE_PRIVILEGED in affile_sd_new) so the effects are already a quite limited, I wouldn't want to complicate matters by adding errno 13 checks.
3. fchown needs CAP_CHOWN unconditionaly 4. fchmod needs CAP_FOWNER if file owner != euid (root here)
I didn't know those. If these are needed for fchown/fchmod, do I need CAP_DAC_OVERRIDE at all? I was enabling DAC_OVERRIDE to be able to change owner/mode information, but as it seems that is not needed, right? I've tested it and it does not seem to be needed, so I've removed DAC_OVERRIDE.
5. all caps should be restored
this was done: if (privileged) { g_process_cap_restore(saved_caps); } however I've found one possible case when it wasn't disabled, thus I moved the 'save-caps' code a bit earlier.
summary: - CAP_SYS_ADMIN and CAP_DAC_OVERRIDE are set always even if unnecessary, and permanently
no, this is not true.
- owner, group and perm doesn't work
this should be fixed by this patch: commit f96ae94df8abdc92402247682537404613db26b9 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Wed Apr 29 17:38:49 2009 +0200 [affile] fixed capability management around file opens (fixes: pub#42) caps are always saved not just in case of "privileged" operation. instead of using DAC_OVERRIDE use CAP_CHOWN and CAP_FOWNER for changing file ownership. Reported-By: Zbigniew Krzystolik -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 --- Comment #3 from Zbigniew Krzystolik <zbyniu@pld-linux.org> 2009-05-05 15:10:04 --- Created an attachment (id=14) --> (https://bugzilla.balabit.com/attachment.cgi?id=14) Better CAPs support + if (privileged) + g_process_cap_modify(CAP_SYS_ADMIN, TRUE); + saved_caps = g_process_cap_save(); Add CAP_SYS_ADMIN permanently on open /proc/kmsg but only if it is used. If you remove source /proc/kmsg and reload cap will not be dropped. + g_process_cap_modify(CAP_DAC_OVERRIDE, TRUE); CAP_DAC_OVERRIDE before create_containing_directory - if (privileged) - { - g_process_cap_restore(saved_caps); - } + g_process_cap_restore(saved_caps); Restore caps always. I've tested it all please review. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 Zbigniew Krzystolik <zbyniu@pld-linux.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED | --- Comment #4 from Zbigniew Krzystolik <zbyniu@pld-linux.org> 2009-05-05 15:14:55 --- (In reply to comment #2)
(In reply to comment #0)
Let's take a look at syslog-ng-3.0.1/src/affile.c lines 60-83
1. CAP_SYS_ADMIN is needed only for /proc/kmsg, it is added w/o check 2. CAP_DAC_READ_SEARCH should be added only if open fail with errno 13 2a. CAP_DAC_OVERRIDE should be added only if open fail with errno 13 and with CAP_DAC_READ_SEARCH set
well, I wouldn't want to complicate enabling those capabilities too much. Currently those capabilities are only enabled for /proc/kmsg and nothing else. (see the check for AFFILE_PRIVILEGED in affile_sd_new)
Ok, agreed, maybe it's better to keep it simple. But CAP_SYS_ADMIN is enabled always (in src/main.c).
3. fchown needs CAP_CHOWN unconditionaly 4. fchmod needs CAP_FOWNER if file owner != euid (root here)
I didn't know those. If these are needed for fchown/fchmod, do I need CAP_DAC_OVERRIDE at all? I was enabling DAC_OVERRIDE to be able to change owner/mode information, but as it seems that is not needed, right?
Yes, it is needed to write in log file w/o permissions ie owner(bla) group(ble) perm(0660). And for create dirs if parent has no perm too.
5. all caps should be restored
this was done:
if (privileged) { g_process_cap_restore(saved_caps); }
Ah, it simply sholud be restored without this condition.
summary: - CAP_SYS_ADMIN and CAP_DAC_OVERRIDE are set always even if unnecessary, and permanently
no, this is not true.
It is. Run getpcaps `pidof syslog-ng`
this should be fixed by this patch: [...]
Yes, but now have CAP_CHOWN and CAP_FOWNER permanently (run getpcaps). -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 Zbigniew Krzystolik <zbyniu@pld-linux.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |zbyniu@pld-linux.org -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 --- Comment #5 from Balazs Scheidler <bazsi@balabit.hu> 2009-05-07 16:58:21 --- (In reply to comment #4)
(In reply to comment #2)
(In reply to comment #0)
Let's take a look at syslog-ng-3.0.1/src/affile.c lines 60-83
1. CAP_SYS_ADMIN is needed only for /proc/kmsg, it is added w/o check 2. CAP_DAC_READ_SEARCH should be added only if open fail with errno 13 2a. CAP_DAC_OVERRIDE should be added only if open fail with errno 13 and with CAP_DAC_READ_SEARCH set
well, I wouldn't want to complicate enabling those capabilities too much. Currently those capabilities are only enabled for /proc/kmsg and nothing else. (see the check for AFFILE_PRIVILEGED in affile_sd_new)
Ok, agreed, maybe it's better to keep it simple. But CAP_SYS_ADMIN is enabled always (in src/main.c).
Well, the problem is that you need CAP_SYS_ADMIN even for reading from /proc/kmsg not just to open it. And the read() call is so heavily buried in various layers, that I can't get propagate the fact that it is actually reading /proc/kmsg. Sure it must be possible, but I find it odd that I need that cap for reading in the first place. I'd definitely rather fix that in the kernel, but this solution was rejected by kernel gods in the past. I'd recommend using a separate dd process to read /proc/kmsg and feed that into a pipe that syslog-ng is configured to read. This is similar to what ubuntu does for instance. Then you can turn off that capability. The problem is that you need to be in a position of controlling the whole system (e.g. distribution) to do that, I really can't require all syslog-ng users to read /proc/kmsg with the dd process, especially that earlier syslog-ng required all-root priviliges to start with. The current situation is still much better.
3. fchown needs CAP_CHOWN unconditionaly 4. fchmod needs CAP_FOWNER if file owner != euid (root here)
I didn't know those. If these are needed for fchown/fchmod, do I need CAP_DAC_OVERRIDE at all? I was enabling DAC_OVERRIDE to be able to change owner/mode information, but as it seems that is not needed, right?
Yes, it is needed to write in log file w/o permissions ie owner(bla) group(ble) perm(0660). And for create dirs if parent has no perm too.
I didn't want to enable writing to any files on the system. I'd guess that if you are running syslog-ng non-root, you'd create your /var/log directory with the appropriate permissions. (e.g. user "syslog-ng") Changing the permissions is useful even in that case though, therefore I allowed it explicitly. But you can convince me with this one.
5. all caps should be restored
this was done:
if (privileged) { g_process_cap_restore(saved_caps); }
Ah, it simply sholud be restored without this condition.
Yes. And that I did in my patch. ah. where's that hunk? I'm positive that I did this, but probably lost it during my commits, just the same that I did with saving the caps in the beginning of the function.
summary: - CAP_SYS_ADMIN and CAP_DAC_OVERRIDE are set always even if unnecessary, and permanently
no, this is not true.
It is. Run getpcaps `pidof syslog-ng`
without my hunk to restore the caps, this is true, I was talking with that in the context.
this should be fixed by this patch: [...]
Yes, but now have CAP_CHOWN and CAP_FOWNER permanently (run getpcaps).
this is because of the missing hunk, but here it goes this time: commit 691c7a3a49eb3f5d019bfa753b80aabd875af1fd Author: Balazs Scheidler <bazsi@balabit.hu> Date: Thu May 7 16:57:55 2009 +0200 [affile] restore changed caps unconditionally (fixes: pub#42) -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 --- Comment #6 from Balazs Scheidler <bazsi@balabit.hu> 2009-05-07 17:04:58 --- Just to clear things up, as I see there are two problems left: * the always-on nature of CAP_SYS_ADMIN and * whether to grant CAP_DAC_OVERRIDE for file opens you can convince me with the second, but please open a separate ticket for CAP_SYS_ADMIN as that's really out of scope for this bug -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=42 Gergely Nagy <algernon@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |algernon@balabit.hu Resolution| |FIXED Status|REOPENED |RESOLVED --- Comment #10 from Gergely Nagy <algernon@balabit.hu> 2012-12-20 13:15:23 --- I'm closing this issue, as I believe it is not relevant for recent syslog-ng anymore. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (2)
-
bugzilla＠bugzilla.balabit.com
-
okapareeya＠hotmail.com