From 5d8ecac1d5881f45d684e0f4ce18b70f78194425 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Sun, 9 Aug 2026 16:38:44 +0100 Subject: [PATCH 1/2] Stop duplicate writing of symlink config Signed-off-by: Quentin Armitage --- keepalived/core/global_data.c | 1 - 1 file changed, 1 deletion(-) diff --git a/keepalived/core/global_data.c b/keepalived/core/global_data.c index b35427cad0..cc3f29c385 100644 --- a/keepalived/core/global_data.c +++ b/keepalived/core/global_data.c @@ -709,7 +709,6 @@ dump_global_data(FILE *fp, data_t * data) if (data->reload_file) conf_write(fp, " Reload_file = %s", data->reload_file); #endif - conf_write(fp, " keep script symlinks = %s", data->use_symlinks ? "true" : "false"); if (data->config_directory) conf_write(fp, " config save directory = %s", data->config_directory); if (data->data_use_instance) From 541c59b844dd0144d445e6e6d3dd1e5cb77c608c Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Sun, 9 Aug 2026 16:40:32 +0100 Subject: [PATCH 2/2] Add option for setting supplementary groups for scripts Signed-off-by: Quentin Armitage --- doc/man/man5/keepalived.conf.5.in | 61 +++++---- keepalived/bfd/bfd_daemon.c | 1 + keepalived/check/check_daemon.c | 1 + keepalived/check/check_data.c | 11 +- keepalived/check/check_misc.c | 12 +- keepalived/core/Makefile.am | 2 +- keepalived/core/global_data.c | 41 ++++-- keepalived/core/global_parser.c | 20 ++- keepalived/core/main.c | 9 +- {lib => keepalived/core}/notify.c | 188 +++++++++++++++++---------- keepalived/include/global_data.h | 5 + {lib => keepalived/include}/notify.h | 39 +++--- keepalived/vrrp/vrrp.c | 5 +- keepalived/vrrp/vrrp_daemon.c | 1 + keepalived/vrrp/vrrp_data.c | 15 ++- keepalived/vrrp/vrrp_parser.c | 4 +- lib/Makefile.am | 4 +- lib/parser.c | 3 - 18 files changed, 270 insertions(+), 152 deletions(-) rename {lib => keepalived/core}/notify.c (85%) rename {lib => keepalived/include}/notify.h (91%) diff --git a/doc/man/man5/keepalived.conf.5.in b/doc/man/man5/keepalived.conf.5.in index 3bfd4dd689..8a819a1035 100644 --- a/doc/man/man5/keepalived.conf.5.in +++ b/doc/man/man5/keepalived.conf.5.in @@ -81,7 +81,11 @@ if they exit with a non-zero status. By default the scripts will be executed by user keepalived_script if that user exists, or if not by root, but for each script the user/group under which it is -to be executed can be specified. +to be executed can be specified. If the groupname is preceeded by a ':' character, +then the supplementary groups of the user executing the script will be set +for the process; if ':' is specified with no groupname, then the supplementary +groups will be specified as above, and the primary group will be the primary +group of the user executing the script. There are significant security implications if scripts are executed with root privileges, especially if the scripts themselves are modifiable or replaceable @@ -266,6 +270,11 @@ possibly following any cleanup actions needed. # To keep symlinks in pathnames, specify use_syslink_paths. \fBuse_symlink_paths \fR[] + # By default, keepalived does not set the supplementary groups of a + # process running a script. This option changes the default to setting + # the supplementary groups for a script. + \fBset_supplementary_groups + # The startup and shutdown scripts are run once, when keepalived starts # before any child processes are run, and when keepalived stops after # all child processes have terminated, respectively. @@ -280,9 +289,9 @@ possibly following any cleanup actions needed. # The timeouts (in seconds default 10 seconds) are the time allowed for # scripts to run; if the timeout expires the scripts will be killed (this # is to stop keepalived hanging waiting for the scripts to terminate). - \fBstartup_script\fR SCRIPT_NAME [username [groupname]] + \fBstartup_script\fR SCRIPT_NAME [username [:][groupname]] \fBstartup_script_timeout\fR SECONDS # range [1,1000] - \fBshutdown_script\fR SCRIPT_NAME [username [groupname]] + \fBshutdown_script\fR SCRIPT_NAME [username [:][groupname]] \fBshutdown_script_timeout\fR SECONDS # range [1,1000] # Set of email To: notify. To include a display name, the whole email address @@ -727,7 +736,7 @@ possibly following any cleanup actions needed. # If this option is not specified, the user defaults to keepalived_script # if that user exists, otherwise the uid/gid under which keepalived is running. # If groupname is not specified, it defaults to the user's group. - \fBscript_user \fRusername [groupname] + \fBscript_user \fRusername [:][groupname] # Don't run scripts configured to be run as root if any part of the path # is writable by a non-root user. Also, enforce the default script_user is @@ -748,7 +757,7 @@ possibly following any cleanup actions needed. # script to be run by keepalived to process notify events # The FIFO name will be passed to the script as the last parameter - \fBnotify_fifo_script \fRSTRING|QUOTED-STRING [username [groupname]] + \fBnotify_fifo_script \fRSTRING|QUOTED-STRING [username [:][groupname]] # FIFO to write vrrp notify events to. # The string written will be a line of the form: INSTANCE "VI_1" MASTER 100 @@ -759,7 +768,7 @@ possibly following any cleanup actions needed. # script to be run by keepalived to process vrrp notify events # The FIFO name will be passed to the script as the last parameter - \fBvrrp_notify_fifo_script \fRSTRING|QUOTED-STRING [username [groupname]] + \fBvrrp_notify_fifo_script \fRSTRING|QUOTED-STRING [username [:][groupname]] # FIFO to write notify healthchecker events to # The string written will be a line of the form: @@ -770,7 +779,7 @@ possibly following any cleanup actions needed. # script to be run by keepalived to process healthchecher notify events # The FIFO name will be passed to the script as the last parameter - \fBlvs_notify_fifo_script \fRSTRING|QUOTED-STRING [username [groupname]] + \fBlvs_notify_fifo_script \fRSTRING|QUOTED-STRING [username [:][groupname]] # By default, when keepalived reloads the vrrp instance and sync group states # are not written to the relevant FIFOs. Setting this option will cause the @@ -1451,7 +1460,7 @@ The syntax for the vrrp script is: # user/group names to run script under. # group default to group of user - \fBuser \fRUSERNAME [GROUPNAME] + \fBuser \fRusername [:][groupname] # assume script initially is in failed state \fBinit_fail\fR @@ -1549,22 +1558,22 @@ The syntax for vrrp_sync_group is : # global script_user and script_group # to MASTER transition - \fBnotify_master \fR/path/to_master.sh [username [groupname]] + \fBnotify_master \fR/path/to_master.sh [username [:][groupname]] # to BACKUP transition - \fBnotify_backup \fR/path/to_backup.sh [username [groupname]] + \fBnotify_backup \fR/path/to_backup.sh [username [:][groupname]] # FAULT transition - \fBnotify_fault \fR"/path/fault.sh VG_1" [username [groupname]] + \fBnotify_fault \fR"/path/fault.sh VG_1" [username [:][groupname]] # executed when stopping vrrp - \fBnotify_stop \fR| [username [groupname]] + \fBnotify_stop \fR| [username [:][groupname]] # notify_deleted causes DELETED to be sent to notifies rather # than the default FAULT after a vrrp instance is deleted during a # reload. If a script is specified, that script will be executed # as well. - \fBnotify_deleted [\fR| [username [groupname]]] + \fBnotify_deleted [\fR| [username [:][groupname]]] # for ANY state transition. # "notify" script is called AFTER the notify_* script(s) and @@ -1579,7 +1588,7 @@ The syntax for vrrp_sync_group is : # strings sent are the same ones listed above # ("GROUP"/"INSTANCE", "MASTER"/"BACKUP"/"FAULT"/"STOP"/"DELETED") # (note: DELETED is only applicable to instances) - \fBnotify \fR| [username [groupname]] + \fBnotify \fR| [username [:][groupname]] # The notify fifo output is the same as the last 4 parameters for the "notify" # script, with the addition of "MASTER_RX_LOWER_PRI" instead of state for an @@ -2199,16 +2208,16 @@ The syntax for vrrp_instance is : \fBdebug \fR # notify scripts, alert as above - \fBnotify_master \fR| [username [groupname]] - \fBnotify_backup \fR| [username [groupname]] - \fBnotify_fault \fR| [username [groupname]] + \fBnotify_master \fR| [username [:][groupname]] + \fBnotify_backup \fR| [username [:][groupname]] + \fBnotify_fault \fR| [username [:][groupname]] # executed when stopping vrrp - \fBnotify_stop \fR| [username [groupname]] - \fBnotify \fR| [username [groupname]] + \fBnotify_stop \fR| [username [:][groupname]] + \fBnotify \fR| [username [:][groupname]] # The notify_master_rx_lower_pri script is executed if a master # receives an advert with priority lower than the master's priority. - \fBnotify_master_rx_lower_pri \fR| [username [groupname]] + \fBnotify_master_rx_lower_pri \fR| [username [:][groupname]] # Send vrrp instance priority notifications on notify FIFOs. \fBnotify_priority_changes\fR @@ -2440,10 +2449,10 @@ The syntax for virtual_server is : \fBhysteresis \fR # Script to execute when quorum is gained. - \fBquorum_up \fR| [username [groupname]] + \fBquorum_up \fR| [username [:][groupname]] # Script to execute when quorum is lost. - \fBquorum_down \fR| [username [groupname]] + \fBquorum_down \fR| [username [:][groupname]] # IP family for a fwmark service (only needed if all real servers are tunnelled # and persistence_granularity is not specified). Defaults to inet if not specified. @@ -2506,10 +2515,10 @@ The syntax for virtual_server is : # Script to execute when healthchecker # considers service as up. - \fBnotify_up \fR| [username [groupname]] + \fBnotify_up \fR| [username [:][groupname]] # Script to execute when healthchecker # considers service as down. - \fBnotify_down \fR| [username [groupname]] + \fBnotify_down \fR| [username [:][groupname]] # maximum number of connections to server \fButhreshold \fR @@ -2714,9 +2723,9 @@ The syntax for virtual_server is : # Specify the username/groupname that the script should # be run under. - # If GROUPNAME is not specified, the group of the user + # If groupname is not specified, the group of the user # is used - \fBuser \fRUSERNAME [GROUPNAME] + \fBuser \fRusername [:][groupname] } # BFD instance name to check diff --git a/keepalived/bfd/bfd_daemon.c b/keepalived/bfd/bfd_daemon.c index bc85498434..0011d77a3c 100644 --- a/keepalived/bfd/bfd_daemon.c +++ b/keepalived/bfd/bfd_daemon.c @@ -154,6 +154,7 @@ start_bfd(__attribute__((unused)) data_t *prev_global_data) alloc_bfd_buffer(); init_data(conf_file, bfd_init_keywords, false); + notify_resource_release(); if (reload) init_global_data(global_data, prev_global_data, true); diff --git a/keepalived/check/check_daemon.c b/keepalived/check/check_daemon.c index f78126f83f..9621bf91ba 100644 --- a/keepalived/check/check_daemon.c +++ b/keepalived/check/check_daemon.c @@ -325,6 +325,7 @@ start_check(data_t *prev_global_data) } init_data(conf_file, check_init_keywords, false); + notify_resource_release(); if (reload) init_global_data(global_data, prev_global_data, true); diff --git a/keepalived/check/check_data.c b/keepalived/check/check_data.c index 9e5fa4ec88..f3816e1215 100644 --- a/keepalived/check/check_data.c +++ b/keepalived/check/check_data.c @@ -472,12 +472,19 @@ dump_tracking_rs(FILE *fp, const void *data) static void dump_notify_vs_rs_script(FILE *fp, const notify_script_t *script, const char *type, const char *state) { + int i; + if (script->path) conf_write(fp, " %s %s notify script = %s, params = %s, uid:gid %u:%u", type, state, - script->path, cmd_str(script), script->uid, script->gid); + script->path, cmd_str(script), script->user_id.uid, script->user_id.gid); else conf_write(fp, " %s %s notify script = %s, uid:gid %u:%u", type, state, - cmd_str(script), script->uid, script->gid); + cmd_str(script), script->user_id.uid, script->user_id.gid); + if (script->user_id.num_sup_grp) { + conf_write(fp, " Supplementary groups:"); + for (i = 0; i < script->user_id.num_sup_grp; i++) + conf_write(fp, " %u", script->user_id.sup_grp[i]); + } } static void diff --git a/keepalived/check/check_misc.c b/keepalived/check/check_misc.c index a4d76719d0..711cff12e8 100644 --- a/keepalived/check/check_misc.c +++ b/keepalived/check/check_misc.c @@ -67,6 +67,7 @@ dump_misc_check(FILE *fp, const checker_t *checker) { const misc_checker_t *misck_checker = checker->data; char time_str[26]; + int i; conf_write(fp, " Keepalive method = MISC_CHECK"); if (misck_checker->script.path) @@ -74,7 +75,12 @@ dump_misc_check(FILE *fp, const checker_t *checker) conf_write(fp, " script = %s", cmd_str(&misck_checker->script)); conf_write(fp, " timeout = %lu", misck_checker->timeout/TIMER_HZ); conf_write(fp, " dynamic = %s", misck_checker->dynamic ? "YES" : "NO"); - conf_write(fp, " uid:gid = %u:%u", misck_checker->script.uid, misck_checker->script.gid); + conf_write(fp, " uid:gid = %u:%u", misck_checker->script.user_id.uid, misck_checker->script.user_id.gid); + if (misck_checker->script.user_id.num_sup_grp) { + conf_write(fp, " Supplementary groups:"); + for (i = 0; i < misck_checker->script.user_id.num_sup_grp; i++) + conf_write(fp, " %u", misck_checker->script.user_id.sup_grp[i]); + } ctime_r(&misck_checker->last_ran.tv_sec, time_str); conf_write(fp, " Last ran = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.24s.%6.6" PRI_tv_usec ")", misck_checker->last_ran.tv_sec, misck_checker->last_ran.tv_usec, time_str, misck_checker->last_ran.tv_usec); conf_write(fp, " Last status = %u", misck_checker->last_exit_code); @@ -172,7 +178,7 @@ misc_user_handler(const vector_t *strvec) return; } - if (set_script_uid_gid(strvec, 1, &new_misck_checker->script.uid, &new_misck_checker->script.gid)) { + if (set_script_uid_gid(strvec, 1, &new_misck_checker->script.user_id)) { report_config_error(CONFIG_GENERAL_ERROR, "Failed to set uid/gid for misc checker script %s - removing", cmd_str(&new_misck_checker->script)); dequeue_new_checker(); } @@ -193,7 +199,7 @@ misc_end_handler(void) if (!script_user_set) { - if (get_default_script_user(&new_misck_checker->script.uid, &new_misck_checker->script.gid)) { + if (get_default_script_user(&new_misck_checker->script.user_id)) { report_config_error(CONFIG_GENERAL_ERROR, "Unable to set default user for misc script %s - removing", cmd_str(&new_misck_checker->script)); dequeue_new_checker(); return; diff --git a/keepalived/core/Makefile.am b/keepalived/core/Makefile.am index a588596aa2..a00c6bb205 100644 --- a/keepalived/core/Makefile.am +++ b/keepalived/core/Makefile.am @@ -15,7 +15,7 @@ noinst_LIBRARIES = libcore.a libcore_a_SOURCES = main.c daemon.c pidfile.c layer4.c smtp.c \ global_data.c global_parser.c keepalived_netlink.c \ - namespaces.c + namespaces.c notify.c libcore_a_LIBADD = EXTRA_libcore_a_SOURCES = diff --git a/keepalived/core/global_data.c b/keepalived/core/global_data.c index cc3f29c385..311955f3d1 100644 --- a/keepalived/core/global_data.c +++ b/keepalived/core/global_data.c @@ -448,6 +448,7 @@ free_global_data(data_t **datap) FREE_CONST_PTR(data->network_namespace_ipvs); FREE_CONST_PTR(data->instance_name); FREE_CONST_PTR(data->process_name); + FREE_CONST_PTR(data->default_script_user_id.sup_grp); #ifdef _WITH_VRRP_ FREE_CONST_PTR(data->vrrp_process_name); #endif @@ -600,7 +601,9 @@ open_dump_file(const char *default_file_name) static void write_fifo_details(FILE *fp, const notify_fifo_t *fifo, const char *type) { - conf_write(fp, " %s notify fifo = %s, uid:gid %u:%u", type, fifo->name, fifo->uid, fifo->gid); + int i; + + conf_write(fp, " %s notify fifo = %s, uid:gid %u:%u", type, fifo->name, fifo->user_id.uid, fifo->user_id.gid); if (!fifo->script) return; @@ -610,14 +613,20 @@ write_fifo_details(FILE *fp, const notify_fifo_t *fifo, const char *type) type, fifo->script->path, cmd_str(fifo->script), - fifo->script->uid, - fifo->script->gid); + fifo->script->user_id.uid, + fifo->script->user_id.gid); else conf_write(fp, " %s notify fifo script = %s, uid:gid %u:%u", type, cmd_str(fifo->script), - fifo->script->uid, - fifo->script->gid); + fifo->script->user_id.uid, + fifo->script->user_id.gid); + + if (fifo->script->user_id.num_sup_grp) { + conf_write(fp, " Supplementary groups:"); + for (i = 0; i < fifo->script->user_id.num_sup_grp; i++) + conf_write(fp, " %u", fifo->script->user_id.sup_grp[i]); + } } void @@ -632,8 +641,7 @@ dump_global_data(FILE *fp, data_t * data) struct tm tm; #endif unsigned val; - uid_t uid; - gid_t gid; + int i; if (!data) return; @@ -664,6 +672,7 @@ dump_global_data(FILE *fp, data_t * data) conf_write(fp, " BFD process name = %s", data->bfd_process_name); #endif conf_write(fp, " %s symlinks in script paths", data->use_symlinks ? "Keep" : "Replace"); + conf_write(fp, " %set supplementary groups for all scripts", data->set_supplementary_groups ? "S" : "Don't s"); if (data->router_id) conf_write(fp, " Router ID = %s", data->router_id); if (data->smtp_server.ss_family) { @@ -716,14 +725,14 @@ dump_global_data(FILE *fp, data_t * data) if (data->startup_script) conf_write(fp, " Startup script = %s, uid:gid %u:%u, timeout %u", cmd_str(data->startup_script), - data->startup_script->uid, - data->startup_script->gid, + data->startup_script->user_id.uid, + data->startup_script->user_id.gid, data->startup_script_timeout); if (data->shutdown_script) conf_write(fp, " Shutdown script = %s, uid:gid %u:%u timeout %u", cmd_str(data->shutdown_script), - data->shutdown_script->uid, - data->shutdown_script->gid, + data->shutdown_script->user_id.uid, + data->shutdown_script->user_id.gid, data->shutdown_script_timeout); #ifdef _WITH_VRRP_ conf_write(fp, " Dynamic interfaces = %s", data->dynamic_interfaces ? "true" : "false"); @@ -921,8 +930,14 @@ dump_global_data(FILE *fp, data_t * data) conf_write(fp, " DBus no interface name = %s", data->dbus_no_interface_name ? data->dbus_no_interface_name : dbus_no_interface_name); #endif conf_write(fp, " Script security %s", script_security ? "enabled" : "disabled"); - if (!get_default_script_user(&uid, &gid)) - conf_write(fp, " Default script uid:gid %u:%u", uid, gid); + if (global_data->default_script_uid_set) { + conf_write(fp, " Default script uid:gid %u:%u", global_data->default_script_user_id.uid, global_data->default_script_user_id.gid); + if (global_data->default_script_user_id.sup_grp) { + conf_write(fp, " Supplementary groups:"); + for (i = 0; i < global_data->default_script_user_id.num_sup_grp; i++) + conf_write(fp, " %u", global_data->default_script_user_id.sup_grp[i]); + } + } #ifdef _WITH_VRRP_ conf_write(fp, " vrrp_netlink_cmd_rcv_bufs = %u", global_data->vrrp_netlink_cmd_rcv_bufs); conf_write(fp, " vrrp_netlink_cmd_rcv_bufs_force = %d", global_data->vrrp_netlink_cmd_rcv_bufs_force); diff --git a/keepalived/core/global_parser.c b/keepalived/core/global_parser.c index d3e6c565c2..1e6354c6f6 100644 --- a/keepalived/core/global_parser.c +++ b/keepalived/core/global_parser.c @@ -172,6 +172,21 @@ use_symlink_path_handler(const vector_t *strvec) global_data->use_symlinks = res; } static void +set_supplementary_groups_handler(const vector_t *strvec) +{ + int res = true; + + if (vector_size(strvec) >= 2) { + res = check_true_false(strvec_slot(strvec, 1)); + if (res == -1) { + report_config_error(CONFIG_GENERAL_ERROR, "Invalid set_supplementary_groups parameter %s", strvec_slot(strvec, 1)); + return; + } + } + + global_data->set_supplementary_groups = res; +} +static void routerid_handler(const vector_t *strvec) { if (vector_size(strvec) < 2) { @@ -1473,11 +1488,11 @@ notify_fifo(const vector_t *strvec, const char *type, notify_fifo_t *fifo) } if (vector_size(strvec) > 2) { - if (set_script_uid_gid(strvec, 2, &fifo->uid, &fifo->gid)) { + if (set_script_uid_gid(strvec, 2, &fifo->user_id)) { log_message(LOG_INFO, "Invalid user/group for %s fifo %s - ignoring", type, fifo->name); return; } - } else if (get_default_script_user(&fifo->uid, &fifo->gid)) { + } else if (get_default_script_user(&fifo->user_id)) { log_message(LOG_INFO, "Failed to set default user for %s fifo %s - ignoring", type, fifo->name); return; } @@ -2612,6 +2627,7 @@ init_global_keywords(bool global_active) install_keyword("bfd_process_name", &bfd_process_name_handler); #endif install_keyword("use_symlink_paths", &use_symlink_path_handler); + install_keyword("set_supplementary_groups", &set_supplementary_groups_handler); install_keyword("router_id", &routerid_handler); install_keyword("notification_email_from", &emailfrom_handler); install_keyword("smtp_server", &smtpserver_handler); diff --git a/keepalived/core/main.c b/keepalived/core/main.c index 6a1cd3a054..2948a9c3ba 100644 --- a/keepalived/core/main.c +++ b/keepalived/core/main.c @@ -476,6 +476,7 @@ read_config_file(bool write_config_copy) #endif init_data(conf_file, global_init_keywords, write_config_copy); + notify_resource_release(); #ifndef _ONE_PROCESS_DEBUG_ if (write_config_copy) @@ -890,7 +891,7 @@ print_parent_data(__attribute__((unused)) thread_ref_t thread) void reinitialise_global_vars(void) { - reset_default_script_user(); + /* Currently there is nothing that needs reinitialising */ } /* SIGHUP/USR1/USR2/STATS_CLEAR handler */ @@ -1048,8 +1049,10 @@ start_validate_reload_conf_child(void) script.args = argv; script.num_args = argc; script.flags = SC_EXECABLE; - script.uid = 0; - script.gid = 0; + script.user_id.uid = 0; + script.user_id.gid = 0; + script.user_id.num_sup_grp = 0; + script.user_id.sup_grp = NULL; if (truncate(global_data->reload_check_config, 0) && errno != ENOENT) { /* The file exists, but truncate failed. It might be a character diff --git a/lib/notify.c b/keepalived/core/notify.c similarity index 85% rename from lib/notify.c rename to keepalived/core/notify.c index cb9de6d781..afd341f6ff 100644 --- a/lib/notify.c +++ b/keepalived/core/notify.c @@ -49,20 +49,13 @@ #include "parser.h" #include "keepalived_magic.h" #include "scheduler.h" +#include "global_data.h" /* Save our uid/gid */ uid_t our_uid; gid_t our_gid; -/* Default user/group for script execution */ -static uid_t default_script_uid; -static gid_t default_script_gid; - -/* Have we got a default user OK? */ -static bool default_script_uid_set = false; -static bool default_user_fail = false; - /* Script security enabled */ bool script_security = false; @@ -84,24 +77,24 @@ set_symlinks(bool state) } static bool -set_script_env(uid_t uid, gid_t gid) +set_script_env(const user_id_t *user_id) { - if (gid != our_gid) { - if (setgid(gid) < 0) { - log_message(LOG_ALERT, "Couldn't setgid: %u (%m)", gid); + if (user_id->gid != our_gid) { + if (setgid(user_id->gid) < 0) { + log_message(LOG_ALERT, "Couldn't setgid: %u (%m)", user_id->gid); return true; } } - /* Clear any extra supplementary groups */ - if (setgroups(1, &gid) < 0) { - log_message(LOG_ALERT, "Couldn't setgroups: %u (%m)", gid); + /* Set/clear any extra supplementary groups */ + if (setgroups(user_id->num_sup_grp, user_id->sup_grp) < 0) { + log_message(LOG_ALERT, "Couldn't setgroups for %u (%m)", user_id->uid); return true; } - if (uid != our_uid) { - if (setuid(uid) < 0) { - log_message(LOG_ALERT, "Couldn't setuid: %u (%m)", uid); + if (user_id->uid != our_uid) { + if (setuid(user_id->uid) < 0) { + log_message(LOG_ALERT, "Couldn't setuid: %u (%m)", user_id->uid); return true; } } @@ -252,7 +245,7 @@ system_call_script(thread_master_t *m, thread_func_t func, void * arg, unsigned ); #endif - if (set_script_env(script->uid, script->gid)) + if (set_script_env(&script->user_id)) exit(0); /* Move us into our own process group, so if the script needs to be killed @@ -323,8 +316,8 @@ fifo_open(notify_fifo_t* fifo, thread_func_t script_exit, const char *type) if (!(ret = mkfifo(fifo->name, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH))) { fifo->created_fifo = true; - if ((fifo->uid != our_uid || fifo->gid != our_gid) && - chown(fifo->name, our_uid != fifo->uid ? fifo->uid : (uid_t)-1, our_gid != fifo->gid ? fifo->gid : (gid_t)-1)) + if ((fifo->user_id.uid != our_uid || fifo->user_id.gid != our_gid) && + chown(fifo->name, our_uid != fifo->user_id.uid ? fifo->user_id.uid : (uid_t)-1, our_gid != fifo->user_id.gid ? fifo->user_id.gid : (gid_t)-1)) log_message(LOG_INFO, "Failed to set uid:gid for fifo %s", fifo->name); } else { sav_errno = errno; @@ -346,7 +339,7 @@ fifo_open(notify_fifo_t* fifo, thread_func_t script_exit, const char *type) fifo->created_fifo = false; } } else if (fstat(fifo->fd, &st) || !S_ISFIFO(st.st_mode) || - (st.st_uid != our_uid && st.st_uid != fifo->uid)) { + (st.st_uid != our_uid && st.st_uid != fifo->user_id.uid)) { /* On EEXIST we may have opened a file an attacker planted, * so refuse anything that is not a fifo we own. */ log_message(LOG_INFO, "%snotify fifo %s is not a fifo we own - not using", type, fifo->name); @@ -551,18 +544,18 @@ find_path(notify_script_t *script) if (file_len > NAME_MAX) return ENAMETOOLONG; - if (script->uid != our_uid || script->gid != our_gid) { + if (script->user_id.uid != our_uid || script->user_id.gid != our_gid) { /* Set file access to the relevant uid/gid */ - if (script->gid != our_gid) { - if (setegid(script->gid)) { - log_message(LOG_INFO, "Unable to set egid to %u (%m)", script->gid); + if (script->user_id.gid != our_gid) { + if (setegid(script->user_id.gid)) { + log_message(LOG_INFO, "Unable to set egid to %u (%m)", script->user_id.gid); return EACCES; } } - if (script->uid != our_uid) { - if (seteuid(script->uid)) { - log_message(LOG_INFO, "Unable to set euid to %u (%m)", script->uid); + if (script->user_id.uid != our_uid) { + if (seteuid(script->user_id.uid)) { + log_message(LOG_INFO, "Unable to set euid to %u (%m)", script->user_id.uid); ret_val = EACCES; goto exit; } @@ -579,8 +572,8 @@ find_path(notify_script_t *script) sgid_num = getgroups(sgid_num, sgid_list); sgid_list[sgid_num++] = our_gid; - /* Clear the supplementary group list */ - if (setgroups(1, &script->gid)) { + /* Set/clear the supplementary group list */ + if (setgroups(script->user_id.num_sup_grp, script->user_id.sup_grp)) { log_message(LOG_INFO, "Unable to set supplementary gids (%m)"); ret_val = EACCES; goto exit; @@ -614,7 +607,7 @@ find_path(notify_script_t *script) if (!ret) { if (!S_ISREG(buf.st_mode)) errno = EACCES; - else if (!is_executable(&buf, script->uid, script->gid)) { + else if (!is_executable(&buf, script->user_id.uid, script->user_id.gid)) { errno = EACCES; } else { /* Success */ @@ -667,9 +660,9 @@ find_path(notify_script_t *script) exit: /* Restore root euid/egid */ - if (script->gid != our_gid && setegid(our_gid)) + if (script->user_id.gid != our_gid && setegid(our_gid)) log_message(LOG_INFO, "Unable to restore egid after script search (%m)"); - if (script->uid != our_uid && seteuid(our_uid)) + if (script->user_id.uid != our_uid && seteuid(our_uid)) log_message(LOG_INFO, "Unable to restore euid after script search (%m)"); /* restore supplementary groups */ @@ -875,17 +868,17 @@ check_script_secure(notify_script_t *script, } /* Check script accessible by the user running it */ - if (script->gid != our_gid || script->uid != our_uid) { + if (script->user_id.gid != our_gid || script->user_id.uid != our_uid) { /* Save parent death signal */ prctl(PR_GET_PDEATHSIG, &sav_death_sig); - if ((script->gid != our_gid && setegid(script->gid)) || - (script->uid != our_uid && seteuid(script->uid))) { - log_message(LOG_INFO, "Unable to set uid:gid %u:%u for script %s - disabling", script->uid, script->gid, script->args[0]); + if ((script->user_id.gid != our_gid && setegid(script->user_id.gid)) || + (script->user_id.uid != our_uid && seteuid(script->user_id.uid))) { + log_message(LOG_INFO, "Unable to set uid:gid %u:%u for script %s - disabling", script->user_id.uid, script->user_id.gid, script->args[0]); - if ((script->uid != our_uid && seteuid(our_uid)) || - (script->gid != our_gid && setegid(our_gid))) - log_message(LOG_INFO, "Unable to restore uid:gid from %u:%u %u:%u after script %s", our_uid, our_gid, script->uid, script->gid, script->args[0]); + if ((script->user_id.uid != our_uid && seteuid(our_uid)) || + (script->user_id.gid != our_gid && setegid(our_gid))) + log_message(LOG_INFO, "Unable to restore uid:gid from %u:%u %u:%u after script %s", our_uid, our_gid, script->user_id.uid, script->user_id.gid, script->args[0]); return SC_INHIBIT; } @@ -894,10 +887,10 @@ check_script_secure(notify_script_t *script, real_path = realpath(script->args[0], NULL); sav_errno = errno; - if (script->gid != our_gid || script->uid != our_uid) { - if ((script->gid != our_gid && setegid(our_gid)) || - (script->uid != our_uid && seteuid(our_uid))) - log_message(LOG_INFO, "Unable to restore uid:gid %u:%u from %u:%u after checking script %s", our_uid, our_gid, script->uid, script->gid, script->args[0]); + if (script->user_id.gid != our_gid || script->user_id.uid != our_uid) { + if ((script->user_id.gid != our_gid && setegid(our_gid)) || + (script->user_id.uid != our_uid && seteuid(our_uid))) + log_message(LOG_INFO, "Unable to restore uid:gid %u:%u from %u:%u after checking script %s", our_uid, our_gid, script->user_id.uid, script->user_id.gid, script->args[0]); /* Restore parent death signal */ prctl(PR_SET_PDEATHSIG, sav_death_sig); @@ -946,14 +939,14 @@ check_script_secure(notify_script_t *script, flags = SC_ISSCRIPT; /* We have the final file. Check if root is executing it, or it is set uid/gid root. */ - if (is_executable(&file_buf, script->uid, script->gid)) { + if (is_executable(&file_buf, script->user_id.uid, script->user_id.gid)) { flags |= SC_EXECUTABLE; - if (script->uid == 0 || script->gid == 0 || + if (script->user_id.uid == 0 || script->user_id.gid == 0 || (file_buf.st_uid == 0 && (file_buf.st_mode & S_IXUSR) && (file_buf.st_mode & S_ISUID)) || (file_buf.st_gid == 0 && (file_buf.st_mode & S_IXGRP) && (file_buf.st_mode & S_ISGID))) need_script_protection = true; } else - log_message(LOG_INFO, "WARNING - script '%s' is not executable for uid:gid %u:%u - disabling.", script->args[0], script->uid, script->gid); + log_message(LOG_INFO, "WARNING - script '%s' is not executable for uid:gid %u:%u - disabling.", script->args[0], script->user_id.uid, script->user_id.gid); /* Default to execable */ script->flags |= SC_EXECABLE; @@ -1029,16 +1022,19 @@ set_pwnam_buf_len(void) } static bool -set_uid_gid(const char *username, const char *groupname, uid_t *uid_p, gid_t *gid_p) +set_uid_gid(const char *username, const char *groupname, user_id_t *user_id) { uid_t uid; gid_t gid; + gid_t user_gid; // The gid of the user struct passwd pwd; struct passwd *pwd_p; struct group grp; struct group *grp_p; int ret; char *buf; + bool set_sup_grps = global_data->set_supplementary_groups; + gid_t *sup_grp; if (!getpwnam_buf_len) set_pwnam_buf_len(); @@ -1058,6 +1054,16 @@ set_uid_gid(const char *username, const char *groupname, uid_t *uid_p, gid_t *gi uid = pwd.pw_uid; gid = pwd.pw_gid; + user_gid = gid; + + /* If groupname starts with a : then we will need to set the supplementary groups */ + if (groupname && groupname[0] == ':') { + set_sup_grps = true; + if (groupname[1]) + groupname++; + else + groupname = NULL; + } if (groupname) { if ((ret = getgrnam_r(groupname, &grp, buf, getpwnam_buf_len, &grp_p))) { @@ -1073,8 +1079,25 @@ set_uid_gid(const char *username, const char *groupname, uid_t *uid_p, gid_t *gi gid = grp.gr_gid; } - *uid_p = uid; - *gid_p = gid; + user_id->num_sup_grp = 0; + user_id->sup_grp = NULL; + + if (set_sup_grps) { + getgrouplist(username, user_gid, NULL, &user_id->num_sup_grp); + + sup_grp = MALLOC(user_id->num_sup_grp * sizeof(gid_t)); + + if (getgrouplist(username, user_gid, sup_grp, &user_id->num_sup_grp) == -1) { + log_message(LOG_INFO, "get supplementary groups for %s returned -1", username); + + FREE(sup_grp); + user_id->num_sup_grp = 0; + } else + user_id->sup_grp = sup_grp; + } + + user_id->uid = uid; + user_id->gid = gid; FREE(buf); @@ -1082,55 +1105,75 @@ set_uid_gid(const char *username, const char *groupname, uid_t *uid_p, gid_t *gi } /* The default script user/group is keepalived_script if it exists, or our uid/gid otherwise */ -void +static void reset_default_script_user(void) { - default_script_uid_set = false; - default_user_fail = false; + global_data->default_script_uid_set = false; + global_data->default_user_fail = false; + + if (global_data->default_script_user_id.sup_grp) { + FREE_CONST(global_data->default_script_user_id.sup_grp); + global_data->default_script_user_id.sup_grp = NULL; + global_data->default_script_user_id.num_sup_grp = 0; + } } bool set_default_script_user(const char *username, const char *groupname) { + /* Clear out any existing entry */ + reset_default_script_user(); + /* Even if we fail to set it, there is no point in trying again */ - default_script_uid_set = true; + global_data->default_script_uid_set = true; - if (set_uid_gid(username, groupname, &default_script_uid, &default_script_gid)) - default_user_fail = true; + if (set_uid_gid(username, groupname, &global_data->default_script_user_id)) + global_data->default_user_fail = true; - return default_user_fail; + return global_data->default_user_fail; } bool -get_default_script_user(uid_t *uid, gid_t *gid) +get_default_script_user(user_id_t *user_id) { const char *default_user = "keepalived_script"; + gid_t *sup_grp; - if (default_user_fail) + if (global_data->default_user_fail) return true; - if (!default_script_uid_set) { + if (!global_data->default_script_uid_set) { /* Even if we fail to set it, there is no point in trying again */ - default_script_uid_set = true; + global_data->default_script_uid_set = true; - default_script_uid = our_uid; - default_script_gid = our_gid; + global_data->default_script_user_id.uid = our_uid; + global_data->default_script_user_id.gid = our_gid; - if (set_uid_gid(default_user, NULL, &default_script_uid, &default_script_gid) && script_security) { + if (set_uid_gid(default_user, NULL, &global_data->default_script_user_id) && script_security) { report_config_error(CONFIG_GENERAL_ERROR, "Unable to set default user %s for script", default_user); - default_user_fail = true; + global_data->default_user_fail = true; return true; } } - *uid = default_script_uid; - *gid = default_script_gid; + user_id->uid = global_data->default_script_user_id.uid; + user_id->gid = global_data->default_script_user_id.gid; + + if (global_data->default_script_user_id.sup_grp) { + sup_grp = MALLOC(global_data->default_script_user_id.num_sup_grp * sizeof(*global_data->default_script_user_id.sup_grp)); + memcpy(sup_grp, global_data->default_script_user_id.sup_grp, global_data->default_script_user_id.num_sup_grp * sizeof(*global_data->default_script_user_id.sup_grp)); + user_id->sup_grp = sup_grp; + user_id->num_sup_grp = global_data->default_script_user_id.num_sup_grp; + } else { + user_id->sup_grp = NULL; + user_id->num_sup_grp = 0; + } return false; } bool -set_script_uid_gid(const vector_t *strvec, unsigned keyword_offset, uid_t *uid_p, gid_t *gid_p) +set_script_uid_gid(const vector_t *strvec, unsigned keyword_offset, user_id_t *user_id) { const char *username; const char *groupname; @@ -1141,7 +1184,7 @@ set_script_uid_gid(const vector_t *strvec, unsigned keyword_offset, uid_t *uid_p else groupname = NULL; - return set_uid_gid(username, groupname, uid_p, gid_p); + return set_uid_gid(username, groupname, user_id); } void @@ -1214,14 +1257,14 @@ notify_script_init(int extra_params, const char *type) script->flags = 0; if (vector_size(strvec_qe) > 2) { - if (set_script_uid_gid(strvec_qe, 2, &script->uid, &script->gid)) { + if (set_script_uid_gid(strvec_qe, 2, &script->user_id)) { log_message(LOG_INFO, "Invalid user/group for %s script %s - ignoring", type, script->args[0]); FREE_CONST(script->args); FREE(script); free_strvec(strvec_qe); return NULL; } - } else if (get_default_script_user(&script->uid, &script->gid)) { + } else if (get_default_script_user(&script->user_id)) { log_message(LOG_INFO, "Failed to set default user for %s script %s - ignoring", type, script->args[0]); FREE_CONST(script->args); FREE(script); @@ -1255,6 +1298,7 @@ notify_free_script(notify_script_t *script) { FREE_CONST_PTR(script->path); FREE_CONST(script->args); + FREE_CONST_PTR(script->user_id.sup_grp); } void diff --git a/keepalived/include/global_data.h b/keepalived/include/global_data.h index 25b7d92c18..d9959dd946 100644 --- a/keepalived/include/global_data.h +++ b/keepalived/include/global_data.h @@ -121,6 +121,11 @@ typedef struct _data { notify_script_t *shutdown_script; unsigned shutdown_script_timeout; bool use_symlinks; + bool set_supplementary_groups; + user_id_t default_script_user_id; /* Default user/group for script execution */ + bool default_script_uid_set; /* Have we got a default user OK? */ + bool default_user_fail; + #ifndef _ONE_PROCESS_DEBUG_ const char *reload_check_config; /* log file name for validating new configuration before reloading */ const char *reload_time_file; diff --git a/lib/notify.h b/keepalived/include/notify.h similarity index 91% rename from lib/notify.h rename to keepalived/include/notify.h index aa716c4001..c221d56899 100644 --- a/lib/notify.h +++ b/keepalived/include/notify.h @@ -56,37 +56,31 @@ typedef enum { SCRIPT_INIT_STATE_FAILED, } script_init_state_t; +typedef struct _user_id { + uid_t uid; /* uid of user to execute script */ + gid_t gid; /* gid of group to execute script */ + int num_sup_grp; /* number of supplementary groups */ + const gid_t *sup_grp; /* array of supplementary groups */ +} user_id_t; + /* notify_script details */ typedef struct _notify_script { const char **args; /* Script args - should be "char const * const *" */ int num_args; /* Used for notify script when adding last 4 parameters */ const char *path; /* The path to the executable if different from args[0] */ int flags; - uid_t uid; /* uid of user to execute script */ - gid_t gid; /* gid of group to execute script */ + user_id_t user_id; /* uid, gid, sup gids */ } notify_script_t; /* notify_fifo details */ typedef struct _notify_fifo { const char *name; int fd; - uid_t uid; /* uid of user of fifo if create */ - gid_t gid; /* gid of group of fifo */ + user_id_t user_id; /* uid, gid, sup gids */ bool created_fifo; /* We created the FIFO */ notify_script_t *script; /* Script to run to process FIFO */ } notify_fifo_t; -static inline void -free_notify_script(notify_script_t **script) -{ - if (!*script) - return; - FREE_PTR((*script)->args); - FREE_CONST_PTR((*script)->path); - FREE_PTR(*script); - *script = NULL; -} - /* Script security enabled */ extern bool script_security; @@ -102,10 +96,9 @@ extern void child_killed_thread(thread_ref_t); extern void script_killall(thread_master_t *, int, bool); extern unsigned check_script_secure(notify_script_t *, magic_t); extern unsigned check_notify_script_secure(notify_script_t **, magic_t); -extern void reset_default_script_user(void); extern bool set_default_script_user(const char *, const char *); -extern bool get_default_script_user(uid_t *, gid_t *); -extern bool set_script_uid_gid(const vector_t *, unsigned, uid_t *, gid_t *); +extern bool get_default_script_user(user_id_t *); +extern bool set_script_uid_gid(const vector_t *, unsigned, user_id_t *); extern void set_script_params_array(const vector_t *, notify_script_t *, unsigned); extern notify_script_t* notify_script_init(int, const char *); extern void add_script_param(notify_script_t *, const char *); @@ -117,4 +110,14 @@ extern void set_our_uid_gid(void); extern void register_notify_addresses(void); #endif +static inline void +free_notify_script(notify_script_t **script) +{ + if (!*script) + return; + notify_free_script(*script); + FREE_PTR(*script); + *script = NULL; +} + #endif diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c index 194c142fc5..b93c0d965a 100644 --- a/keepalived/vrrp/vrrp.c +++ b/keepalived/vrrp/vrrp.c @@ -5564,8 +5564,9 @@ clear_diff_script(void) if (nvscript) { /* Check if the scripts are the same */ if (vscript->script.num_args != nvscript->script.num_args || - vscript->script.uid != nvscript->script.uid || - vscript->script.gid != nvscript->script.gid || + vscript->script.user_id.uid != nvscript->script.user_id.uid || + vscript->script.user_id.gid != nvscript->script.user_id.gid || + vscript->script.user_id.num_sup_grp != nvscript->script.user_id.num_sup_grp || !vscript->script.path != !nvscript->script.path || (vscript->script.path && strcmp(vscript->script.path, nvscript->script.path))) diff --git a/keepalived/vrrp/vrrp_daemon.c b/keepalived/vrrp/vrrp_daemon.c index 11cd305cec..a9dcde3e41 100644 --- a/keepalived/vrrp/vrrp_daemon.c +++ b/keepalived/vrrp/vrrp_daemon.c @@ -528,6 +528,7 @@ start_vrrp(data_t *prev_global_data) } init_data(conf_file, vrrp_init_keywords, false); + notify_resource_release(); /* Update process name if necessary */ if ((!prev_global_data && // startup diff --git a/keepalived/vrrp/vrrp_data.c b/keepalived/vrrp/vrrp_data.c index 5cedec8048..78cbb09e8c 100644 --- a/keepalived/vrrp/vrrp_data.c +++ b/keepalived/vrrp/vrrp_data.c @@ -189,15 +189,23 @@ free_sync_group_list(list_head_t *l) static void dump_notify_script(FILE *fp, const notify_script_t *script, const char *type) { + int i; + if (!script) return; if (script->path) conf_write(fp, " %s state transition script = %s, params = %s, uid:gid %u:%u" - , type, script->path, cmd_str(script), script->uid, script->gid); + , type, script->path, cmd_str(script), script->user_id.uid, script->user_id.gid); else conf_write(fp, " %s state transition script = %s, uid:gid %u:%u" - , type, cmd_str(script), script->uid, script->gid); + , type, cmd_str(script), script->user_id.uid, script->user_id.gid); + + if (script->user_id.num_sup_grp) { + conf_write(fp, " Supplementary groups:"); + for (i = 0; i < script->user_id.num_sup_grp; i++) + conf_write(fp, " %u", script->user_id.sup_grp[i]); + } } static void @@ -278,6 +286,7 @@ free_vscript(vrrp_script_t *vscript) { list_del_init(&vscript->e_list); free_tracking_obj_list(&vscript->tracking_vrrp); + FREE_CONST(vscript->script.user_id.sup_grp); FREE_CONST(vscript->sname); notify_free_script(&vscript->script); FREE(vscript); @@ -325,7 +334,7 @@ dump_vscript(FILE *fp, const vrrp_script_t *vscript) } conf_write(fp, " Init state = %s", str); conf_write(fp, " Status = %s", vscript->result >= vscript->rise ? "GOOD" : "BAD"); - conf_write(fp, " Script uid:gid = %u:%u", vscript->script.uid, vscript->script.gid); + conf_write(fp, " Script uid:gid = %u:%u", vscript->script.user_id.uid, vscript->script.user_id.gid); conf_write(fp, " VRRP instances :"); dump_tracking_obj_list(fp, &vscript->tracking_vrrp, dump_tracking_vrrp); conf_write(fp, " State = %s", diff --git a/keepalived/vrrp/vrrp_parser.c b/keepalived/vrrp/vrrp_parser.c index cb404a459b..1bd633d764 100644 --- a/keepalived/vrrp/vrrp_parser.c +++ b/keepalived/vrrp/vrrp_parser.c @@ -1814,7 +1814,7 @@ vrrp_vscript_fall_handler(const vector_t *strvec) static void vrrp_vscript_user_handler(const vector_t *strvec) { - if (set_script_uid_gid(strvec, 1, ¤t_vscr->script.uid, ¤t_vscr->script.gid)) { + if (set_script_uid_gid(strvec, 1, ¤t_vscr->script.user_id)) { report_config_error(CONFIG_GENERAL_ERROR, "Unable to set uid/gid for script %s" , cmd_str(¤t_vscr->script)); remove_script = true; @@ -1833,7 +1833,7 @@ vrrp_vscript_end_handler(void) remove_script = true; } else if (!remove_script && !script_user_set) { - if (get_default_script_user(¤t_vscr->script.uid, ¤t_vscr->script.gid)) { + if (get_default_script_user(¤t_vscr->script.user_id)) { report_config_error(CONFIG_GENERAL_ERROR, "Unable to set default user for vrrp" " script %s - removing" , current_vscr->sname); diff --git a/lib/Makefile.am b/lib/Makefile.am index a32a2961e5..d030e1d336 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -15,11 +15,11 @@ BUILT_SOURCES = $(GIT_COMMIT_FILE) noinst_LIBRARIES = liblib.a -liblib_a_SOURCES = memory.c utils.c notify.c timer.c scheduler.c \ +liblib_a_SOURCES = memory.c utils.c timer.c scheduler.c \ vector.c html.c parser.c signals.c logger.c \ list_head.c rbtree.c process.c json_writer.c \ bitops.h timer.h scheduler.h vector.h parser.h \ - signals.h notify.h logger.h memory.h html.h utils.h \ + signals.h logger.h memory.h html.h utils.h \ keepalived_magic.h list_head.h rbtree_ka.h rbtree.h \ rbtree_types.h process.h rbtree_augmented.h assert_debug.h \ json_writer.h warnings.h container.h align.h sockaddr.h diff --git a/lib/parser.c b/lib/parser.c index f5fe970151..764b74c327 100644 --- a/lib/parser.c +++ b/lib/parser.c @@ -57,7 +57,6 @@ #include "list_head.h" #include "rttables.h" #include "scheduler.h" -#include "notify.h" #include "bitops.h" #include "utils.h" #include "process.h" @@ -3396,8 +3395,6 @@ init_data(const char *conf_file, const vector_t * (*init_keywords) (void), bool free_keywords(keywords); free_parser_data(); - - notify_resource_release(); } int