[PATCH RFC] Use cap_syslog when it is available
If cap_syslog exists, the kernel will complain (once per boot) that we only have cap_sys_admin. Ideally, we would detect whether the kernel knows about cap_syslog, and not use cap_sys_admin if it does. However, there is no good way to deduce that. We can only detect whether userspace knows about it, and kernel may not match userspace. I will work to get such polling into the kernel and, when successful, will provide a syslog-ng patch to use that, so that when CAP_SYSLOG is supported by the kernel, we can reduce the amount of privilege we needlessly take. Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- lib/gprocess.c | 35 ++++++++++++++++++++++++++++++++++- lib/gprocess.h | 2 +- modules/affile/affile.c | 3 +++ syslog-ng/main.c | 10 +++------- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/lib/gprocess.c b/lib/gprocess.c index d9a2dfb..0884237 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -416,7 +416,7 @@ g_process_set_working_dir(const gchar *cwd) * capability set. The process will change its capabilities to this value * during startup, provided it has enough permissions to do so. **/ -void +static void g_process_set_caps(const gchar *caps) { if (!process_opts.caps) @@ -879,6 +879,39 @@ g_process_change_dir(void) } /** + * g_process_setup_caps(void) + * + * NOTE: polling /proc/kmsg requires cap_sys_admin, otherwise it'll always + * indicate readability. Enabling/disabling cap_sys_admin on every poll + * invocation seems to be too expensive. So I enable it for now. + * + * If CAP_SYSLOG exists, add that to our list of capabilities alongside + * CAP_SYS_ADMIN. Ideally we would *replace* it. However, we currently + * have no reliable way to check whether the kernel - which may not + * match userspace - supports it. If it doesn't, there is no harm in + * adding CAP_SYSLOG, but we of course still need CAP_SYS_ADMIN for + * /proc/kmsg. + * + * Once the kernel supports polling for the number of capabilities, we + * can drop CAP_SYSLOG after detecting whether CAP_SYSLOG exists. + */ +void +g_process_setup_caps(void) +{ +#ifdef CAP_SYSLOG + gchar * capsstr = "cap_net_bind_service,cap_net_broadcast,cap_net_raw," + "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " + "cap_syslog,cap_sys_admin=ep"; +#else + gchar * capsstr = "cap_net_bind_service,cap_net_broadcast,cap_net_raw," + "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " + "cap_sys_admin=ep"; +#endif + + g_process_set_caps(capsstr); +} + +/** * g_process_send_result: * @ret_num: exit code of the process * diff --git a/lib/gprocess.h b/lib/gprocess.h index cda35b0..4ad0866 100644 --- a/lib/gprocess.h +++ b/lib/gprocess.h @@ -66,7 +66,7 @@ void g_process_set_chroot(const gchar *chroot); void g_process_set_pidfile(const gchar *pidfile); void g_process_set_pidfile_dir(const gchar *pidfile_dir); void g_process_set_working_dir(const gchar *cwd); -void g_process_set_caps(const gchar *caps); +void g_process_setup_caps(void); void g_process_set_argv_space(gint argc, gchar **argv); void g_process_set_use_fdlimit(gboolean use); void g_process_set_check(gint check_period, gboolean (*check_fn)(void)); diff --git a/modules/affile/affile.c b/modules/affile/affile.c index a713bfc..b11b7ac 100644 --- a/modules/affile/affile.c +++ b/modules/affile/affile.c @@ -59,6 +59,9 @@ affile_open_file(gchar *name, gint flags, if (privileged) { g_process_cap_modify(CAP_DAC_READ_SEARCH, TRUE); +#ifdef CAP_SYSLOG + g_process_cap_modify(CAP_SYSLOG, TRUE); +#endif g_process_cap_modify(CAP_SYS_ADMIN, TRUE); } else diff --git a/syslog-ng/main.c b/syslog-ng/main.c index 7e81f2f..5e62c1f 100644 --- a/syslog-ng/main.c +++ b/syslog-ng/main.c @@ -167,13 +167,9 @@ main(int argc, char *argv[]) g_process_set_argv_space(argc, (gchar **) argv); - /* NOTE: polling /proc/kmsg requires cap_sys_admin, otherwise it'll always - * indicate readability. Enabling/disabling cap_sys_admin on every poll - * invocation seems to be too expensive. So I enable it for now. */ - - g_process_set_caps("cap_net_bind_service,cap_net_broadcast,cap_net_raw," - "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " - "cap_sys_admin=ep"); + /* Set up the minimal privilege we'll need */ + g_process_setup_caps(); + ctx = g_option_context_new("syslog-ng"); g_process_add_option_group(ctx); msg_add_option_group(ctx); -- 1.7.4.1
Serge Hallyn <serge.hallyn@ubuntu.com> writes:
+void +g_process_setup_caps(void) +{ +#ifdef CAP_SYSLOG + gchar * capsstr = "cap_net_bind_service,cap_net_broadcast,cap_net_raw," + "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " + "cap_syslog,cap_sys_admin=ep"; +#else + gchar * capsstr = "cap_net_bind_service,cap_net_broadcast,cap_net_raw," + "cap_dac_read_search,cap_dac_override,cap_chown,cap_fowner=p " + "cap_sys_admin=ep"; +#endif + + g_process_set_caps(capsstr); +}
I seem to remember having tried something similar in the past, and deciding against it... as far as I remember, the issue was that if compiled with a libcap that supports CAP_SYSLOG, the binary would still be runnable on a system with an old libcap, which then wouldn't recognise cap_syslog and syslog-ng would refuse to start. I'm not 100% certain, as it was a while ago that I was working on this case, so please correct me if I'm wrong. (Yes, I do understand that this is a non-issue for most people, and it certainly is no problem for distributions) -- |8]
participants (2)
-
Gergely Nagy
-
Serge Hallyn