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]