Hi, Can you please send the original patch? v3 never made it to the list, and the patch in this email is quoted. Thanks. On Mon, 2011-04-18 at 20:26 +0200, Gergely Nagy wrote:
(forwarding the original mail, as it looks like it never made to the list)
Serge Hallyn <serge.hallyn@canonical.com> writes:
(A-ha - great idea from Andrew - we *can* figure out whether the kernel knows about CAP_SYSLOG, using the bounding set API)
If cap_syslog exists, the kernel will complain (once) that we only have cap_sys_admin. Additionally, using cap_syslog instead of cap_sys_admin significantly lowers the unneeded privs we are using.
Changelog: v2: At startup, detect whether libcap knows about CAP_SYSLOG. (Thanks to Gergely Nagy for pointing out that case) v3: Andrew Morgan pointed out a nice way to detect whether the kernel has CAP_SYSLOG. Thanks, Andrew!
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com> --- lib/gprocess.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++- lib/gprocess.h | 2 +- modules/affile/affile.c | 4 +++ syslog-ng/main.c | 10 ++------ 4 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/lib/gprocess.c b/lib/gprocess.c index d9a2dfb..f9bda7b 100644 --- a/lib/gprocess.c +++ b/lib/gprocess.c @@ -98,6 +98,7 @@ static gint startup_result_pipe[2] = { -1, -1 }; static gint init_result_pipe[2] = { -1, -1 }; static GProcessKind process_kind = G_PK_STARTUP; static gboolean stderr_present = TRUE; +static int have_capsyslog = FALSE;
/* global variables */ static struct @@ -210,6 +211,14 @@ g_process_cap_modify(int capability, int onoff) if (!process_opts.caps) return TRUE;
+#ifdef CAP_SYSLOG + /* + * if libcap or kernel doesn't support cap_syslog, then resort to + * cap_sys_admin + */ + if (capability == CAP_SYSLOG && !have_capsyslog) + capability = CAP_SYS_ADMIN; +#endif caps = cap_get_proc(); if (!caps) return FALSE; @@ -416,7 +425,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 +888,43 @@ 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. + */ +void +g_process_setup_caps(void) +{ + int ret; + 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"; +#ifdef CAP_SYSLOG + cap_t caps; + + /* Check whether cap_syslog exists. If not, get cap_sys_admin, else get + * cap_syslog */ + caps = cap_from_text("cap_syslog=p"); + if (caps) { + cap_free(caps); + /* libcap knows it, does the kernel? */ + ret = prctl(PR_CAPBSET_READ, CAP_SYSLOG); + if (ret != -1) { + /* kernel knows cap_syslog! Use it */ + 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=ep"; + have_capsyslog = TRUE; + } + } +#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..358a083 100644 --- a/modules/affile/affile.c +++ b/modules/affile/affile.c @@ -59,7 +59,11 @@ 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); +#else g_process_cap_modify(CAP_SYS_ADMIN, TRUE); +#endif } 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);
-- Bazsi