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).
This commit is contained in:
Willy Tarreau 2023-11-23 16:48:48 +01:00
parent 0fccee6abe
commit 1de44daf7d
4 changed files with 35 additions and 6 deletions

View File

@ -1541,14 +1541,20 @@ expose-experimental-directives
This statement must appear before using directives tagged as experimental or This statement must appear before using directives tagged as experimental or
the config file will be rejected. 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 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 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 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), 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. you may also need to set "insecure-setuid-wanted" in the global section.
See "option external-check", and "insecure-fork-wanted", and By default, the checks start with a clean environment which only contains
"insecure-setuid-wanted". 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 <number> fd-hard-limit <number>
Sets an upper bound to the maximum number of file descriptors that the Sets an upper bound to the maximum number of file descriptors that the

View File

@ -105,7 +105,7 @@ struct proxy;
struct global { struct global {
int uid; int uid;
int gid; int gid;
int external_check; int external_check; /* 0=disabled, 1=enabled, 2=enabled with env */
int nbthread; int nbthread;
int mode; int mode;
unsigned int hard_stop_after; /* maximum time allowed to perform a soft-stop */ unsigned int hard_stop_after; /* maximum time allowed to perform a soft-stop */

View File

@ -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) { 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; goto out;
global.external_check = 1; 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 */ /* user/group name handling */
else if (strcmp(args[0], "user") == 0) { else if (strcmp(args[0], "user") == 0) {

View File

@ -426,7 +426,10 @@ static int connect_proc_chk(struct task *t)
(unsigned int)limit.rlim_cur, (unsigned int)limit.rlim_max); (unsigned int)limit.rlim_cur, (unsigned int)limit.rlim_max);
} }
if (global.external_check < 2) {
/* fresh new env for each check */
environ = check->envp; environ = check->envp;
}
/* Update some environment variables and command args: curconn, server addr and server port */ /* 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); 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_ADDR, check->argv[3], fail);
EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_PORT, check->argv[4], 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(); haproxy_unblock_signals();
execvp(px->check_command, check->argv); execvp(px->check_command, check->argv);
ha_alert("Failed to exec process for external health check: %s. Aborting.\n", ha_alert("Failed to exec process for external health check: %s. Aborting.\n",