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).

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 073bb2f749bc661808e058b60cf1b2676bfdfc64..fe0669df921f8d88a288bbd24ab68e3df821963f 100644 (file)
--- 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 <number>
   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 f051832ae121402730dce3b316554b617676ab1e..e302de45dfc0c6be4e889f368ae7700668981917 100644 (file)
--- 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 7834ea18fe1dfbb9c7c6a6ecdf85274724479542..9e7e3324bcf199fe09b4490a37c44f0bbdd361b1 100644 (file)
--- 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 0fd35a117857f51f84bf1550a1dda7a6fb47b14f..c667b1635cc0feb7e1392b872707895f0d282e2b 100644 (file)
--- 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",