From 1de44daf7d61828be1effd970817ee59ed8750d7 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 23 Nov 2023 16:48:48 +0100 Subject: [PATCH] MINOR: ext-check: add an option to preserve environment variables In Github issue #2128, @jvincze84 explained the complexity of using external checks in some advanced setups due to the systematic purge of environment variables, and expressed the desire to preserve the existing environment. During the discussion an agreement was found around having an option to "external-check" to do that and that solution was tested and confirmed to work by user @nyxi. This patch just cleans this up, implements the option as "preserve-env" and documents it. The default behavior does not change, the environment is still purged, unless "preserve-env" is passed. The choice of not using "import-env" instead was made so that we could later use it to name specific variables that have to be imported instead of keeping the whole environment. The patch is simple enough that it could be backported if needed (and was in fact tested on 2.6 first). --- doc/configuration.txt | 12 +++++++++--- include/haproxy/global-t.h | 2 +- src/cfgparse-global.c | 9 ++++++++- src/extcheck.c | 18 +++++++++++++++++- 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 073bb2f749..fe0669df92 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -1541,14 +1541,20 @@ expose-experimental-directives This statement must appear before using directives tagged as experimental or the config file will be rejected. -external-check +external-check [preserve-env] Allows the use of an external agent to perform health checks. This is disabled by default as a security precaution, and even when enabled, checks may still fail unless "insecure-fork-wanted" is enabled as well. If the program launched makes use of a setuid executable (it should really not), you may also need to set "insecure-setuid-wanted" in the global section. - See "option external-check", and "insecure-fork-wanted", and - "insecure-setuid-wanted". + By default, the checks start with a clean environment which only contains + variables defined in the "external-check" command in the backend section. It + may sometimes be desirable to preserve the environment though, for example + when complex scripts retrieve their extra paths or information there. This + can be done by appending the "preserve-env" keyword. In this case however it + is strongly advised not to run a setuid nor as a privileged user, as this + exposes the check program to potential attacks. See "option external-check", + and "insecure-fork-wanted", and "insecure-setuid-wanted" for extra details. fd-hard-limit Sets an upper bound to the maximum number of file descriptors that the diff --git a/include/haproxy/global-t.h b/include/haproxy/global-t.h index f051832ae1..e302de45df 100644 --- a/include/haproxy/global-t.h +++ b/include/haproxy/global-t.h @@ -105,7 +105,7 @@ struct proxy; struct global { int uid; int gid; - int external_check; + int external_check; /* 0=disabled, 1=enabled, 2=enabled with env */ int nbthread; int mode; unsigned int hard_stop_after; /* maximum time allowed to perform a soft-stop */ diff --git a/src/cfgparse-global.c b/src/cfgparse-global.c index 7834ea18fe..9e7e3324bc 100644 --- a/src/cfgparse-global.c +++ b/src/cfgparse-global.c @@ -586,9 +586,16 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm) } } else if (strcmp(args[0], "external-check") == 0) { - if (alertif_too_many_args(0, file, linenum, args, &err_code)) + if (alertif_too_many_args(1, file, linenum, args, &err_code)) goto out; global.external_check = 1; + if (strcmp(args[1], "preserve-env") == 0) { + global.external_check = 2; + } else { + ha_alert("parsing [%s:%d] : '%s' only supports 'preserve-env' as an argument, found '%s'.\n", file, linenum, args[0], args[1]); + err_code |= ERR_ALERT | ERR_FATAL; + goto out; + } } /* user/group name handling */ else if (strcmp(args[0], "user") == 0) { diff --git a/src/extcheck.c b/src/extcheck.c index 0fd35a1178..c667b1635c 100644 --- a/src/extcheck.c +++ b/src/extcheck.c @@ -426,7 +426,10 @@ static int connect_proc_chk(struct task *t) (unsigned int)limit.rlim_cur, (unsigned int)limit.rlim_max); } - environ = check->envp; + if (global.external_check < 2) { + /* fresh new env for each check */ + environ = check->envp; + } /* Update some environment variables and command args: curconn, server addr and server port */ EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_CURCONN, ultoa_r(s->cur_sess, buf, sizeof(buf)), fail); @@ -445,6 +448,19 @@ static int connect_proc_chk(struct task *t) EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_ADDR, check->argv[3], fail); EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_PORT, check->argv[4], fail); + if (global.external_check >= 2) { + /* environment is preserved, let's merge new vars */ + int i; + + for (i = 0; check->envp[i] && *check->envp[i]; i++) { + char *delim = strchr(check->envp[i], '='); + if (!delim) + continue; + *(delim++) = 0; + if (setenv(check->envp[i], delim, 1) != 0) + goto fail; + } + } haproxy_unblock_signals(); execvp(px->check_command, check->argv); ha_alert("Failed to exec process for external health check: %s. Aborting.\n",