Re: [syslog-ng] [PATCH RFC] Use cap_syslog when it is available (v3)
(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);
-- |8]
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
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> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- 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..4a4ce52 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 67a4632..5c56bc5 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); -- 1.7.2.5
Hi, Thanks for the great stuff, it is really appreciated. I've some notes, please let me know if you or Serge would like to do that, or I should. Nothing extraordinary, the mass of the work has already been done, I just prefer some things a bit differently. Thanks. On Sun, 2011-05-01 at 09:46 +0200, Gergely Nagy wrote:
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> Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- 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..4a4ce52 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
could you check whether CAP_SYSLOG exists in the gprocess.h header file, and if it doesn't define it to an extremal value, so that call-sites do not have to be littered with #ifdef CAP_SYSLOG? Something like: #ifndef CAP_SYSLOG #define CAP_SYSLOG -1 #endif Then in g_process_cap_modify() we could to the translation based on whether CAP_SYSLOG is -1 or a sane value.
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); +}
I would prefer to construct the set of permissions needed for syslog-ng to be constructed in the main.c file, e.g. the detection code should be moved into a gboolean g_process_is_cap_supported(cap_t cap) Then in the main.c file (perhaps in a separate function), construct the required set of caps, and set by g_process_set_caps(). (e.g don't make it static and remove the g_process_setup_caps() stuff. Also note that the user can override the capset of syslog-ng using the --caps option.
+ +/** * 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 67a4632..5c56bc5 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 }
This would then become g_process_cap_modify(CAP_SYSLOG), the CAP_SYS_ADMIN is not needed.
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
Balazs Scheidler <bazsi@balabit.hu> writes:
I've some notes, please let me know if you or Serge would like to do that, or I should.
Nothing extraordinary, the mass of the work has already been done, I just prefer some things a bit differently.
I'll be sending a patch in a few moments, but I've done a few things a bit differently than you suggested, so I'd like to explain that in advance.
/** + * 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); +}
I would prefer to construct the set of permissions needed for syslog-ng to be constructed in the main.c file, e.g. the detection code should be moved into a
gboolean g_process_is_cap_supported(cap_t cap)
Then in the main.c file (perhaps in a separate function), construct the required set of caps, and set by g_process_set_caps(). (e.g don't make it static and remove the g_process_setup_caps() stuff.
This is something I've done differently: I introduced g_process_check_cap_syslog(), which checks for CAP_SYSLOG presence, and sets have_capsyslog to TRUE if it is found. This was done so that g_process_cap_modify() wouldn't have to do it itself. But come to think of it, I could've done something like this (in lib/gprocess.c): static gint cap_syslog_override = -1; And in g_process_cap_modify(): if (capability == CAP_SYSLOG) { if (cap_syslog_override != -1) capability = cap_syslog_override; else { if (g_process_is_cap_supported(CAP_SYSLOG)) cap_syslog_override = CAP_SYSLOG; else cap_syslog_override = CAP_SYS_ADMIN; } } Or something similar... This would have the advantage of having a generic g_process_is_cap_supported(), with the downside of the code being longer. And for reasons I can't properly explain, I like the _check_cap_syslog() solution better.
Also note that the user can override the capset of syslog-ng using the --caps option.
With my solution, that doesn't need special handling, I believe. But I haven't tested all scenarios. -- |8]
participants (2)
-
Balazs Scheidler
-
Gergely Nagy