MEDIUM: init: set NO_NEW_PRIVS by default when supported

HAProxy doesn't need to call executables at run time (except when using
external checks which are strongly recommended against), and is even expected
to isolate itself into an empty chroot. As such, there basically is no valid
reason to allow a setuid executable to be called without the user being fully
aware of the risks. In a situation where haproxy would need to call external
checks and/or disable chroot, exploiting a vulnerability in a library or in
haproxy itself could lead to the execution of an external program. On Linux
it is possible to lock the process so that any setuid bit present on such an
executable is ignored. This significantly reduces the risk of privilege
escalation in such a situation. This is what haproxy does by default. In case
this causes a problem to an external check (for example one which would need
the "ping" command), then it is possible to disable this protection by
explicitly adding this directive in the global section. If enabled, it is
possible to turn it back off by prefixing it with the "no" keyword.

Before the option:

  $ socat - /tmp/sock1 <<< "expert-mode on; debug dev exec sudo /bin/id"
  uid=0(root) gid=0(root) groups=0(root

After the option:
  $ socat - /tmp/sock1 <<< "expert-mode on; debug dev exec sudo /bin/id"
  sudo: effective uid is not 0, is /usr/bin/sudo on a file system with the
        'nosuid' option set or an NFS file system without root privileges?

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 8938195a2cc5019d75d2c8ef68f2c65f926da34e..862fa72d4711b005bb04df927af9d6e57d735c7d 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -595,6 +595,7 @@ The following keywords are supported in the "global" section :
    - h1-case-adjust
    - h1-case-adjust-file
    - insecure-fork-wanted
+   - insecure-setuid-wanted
    - log
    - log-tag
    - log-send-hostname
@@ -825,8 +826,11 @@ deviceatlas-properties-cookie <name>
 external-check
   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.
-  See "option external-check", and "insecure-fork-wanted".
+  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".
 
 gid <number>
   Changes the process' group ID to <number>. It is recommended that the group
@@ -923,6 +927,22 @@ insecure-fork-wanted
   agents instead of external checks). This option supports the "no" prefix to
   disable it.
 
+insecure-setuid-wanted
+  HAProxy doesn't need to call executables at run time (except when using
+  external checks which are strongly recommended against), and is even expected
+  to isolate itself into an empty chroot. As such, there basically is no valid
+  reason to allow a setuid executable to be called without the user being fully
+  aware of the risks. In a situation where haproxy would need to call external
+  checks and/or disable chroot, exploiting a vulnerability in a library or in
+  haproxy itself could lead to the execution of an external program. On Linux
+  it is possible to lock the process so that any setuid bit present on such an
+  executable is ignored. This significantly reduces the risk of privilege
+  escalation in such a situation. This is what haproxy does by default. In case
+  this causes a problem to an external check (for example one which would need
+  the "ping" command), then it is possible to disable this protection by
+  explicitly adding this directive in the global section. If enabled, it is
+  possible to turn it back off by prefixing it with the "no" keyword.
+
 log <address> [len <length>] [format <format>] [sample <ranges>:<smp_size>]
     <facility> [max level [min level]]
   Adds a global syslog server. Several global servers can be defined. They

diff --git a/include/types/global.h b/include/types/global.h
index 8c4a7c55b44c49f43282babb259fd52d26328c3f..b44f1c7c7f78493e326cf20127381d95438819d5 100644 (file)
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -75,6 +75,7 @@
 #define GTUNE_USE_EVPORTS        (1<<14)
 #define GTUNE_STRICT_LIMITS      (1<<15)
 #define GTUNE_INSECURE_FORK      (1<<16)
+#define GTUNE_INSECURE_SETUID    (1<<17)
 
 /* SSL server verify mode */
 enum {

diff --git a/src/cfgparse-global.c b/src/cfgparse-global.c
index c083a05d5974b2938d057308f46d58ea3271280b..3306f8cb80a14ed0e7f3a9538f70cccea30ae211 100644 (file)
--- a/src/cfgparse-global.c
+++ b/src/cfgparse-global.c
@@ -102,6 +102,14 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
                else
                        global.tune.options |=  GTUNE_INSECURE_FORK;
        }
+       else if (!strcmp(args[0], "insecure-setuid-wanted")) { /* "no insecure-setuid-wanted" or "insecure-setuid-wanted" */
+               if (alertif_too_many_args(0, file, linenum, args, &err_code))
+                       goto out;
+               if (kwm == KWM_NO)
+                       global.tune.options &= ~GTUNE_INSECURE_SETUID;
+               else
+                       global.tune.options |=  GTUNE_INSECURE_SETUID;
+       }
        else if (!strcmp(args[0], "nosplice")) {
                if (alertif_too_many_args(0, file, linenum, args, &err_code))
                        goto out;

diff --git a/src/haproxy.c b/src/haproxy.c
index 7ba3ae1ba457dc073fcee99e58efa40bbc823bff..b6e59dcf356ab746f6e6b70f1781870f453dc410 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2749,6 +2749,25 @@ static void *run_thread_poll_loop(void *data)
        pthread_mutex_unlock(&init_mutex);
 #endif
 
+#if defined(PR_SET_NO_NEW_PRIVS) && defined(USE_PRCTL)
+       /* Let's refrain from using setuid executables. This way the impact of
+        * an eventual vulnerability in a library remains limited. It may
+        * impact external checks but who cares about them anyway ? In the
+        * worst case it's possible to disable the option. Obviously we do this
+        * in workers only. We can't hard-fail on this one as it really is
+        * implementation dependent though we're interested in feedback, hence
+        * the warning.
+        */
+       if (!(global.tune.options & GTUNE_INSECURE_SETUID) && !master) {
+               static int warn_fail;
+               if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1 && !_HA_ATOMIC_XADD(&warn_fail, 1)) {
+                       ha_warning("Failed to disable setuid, please report to developers with detailed "
+                                  "information about your operating system. You can silence this warning "
+                                  "by adding 'insecure-setuid-wanted' in the 'global' section.\n");
+               }
+       }
+#endif
+
 #if defined(RLIMIT_NPROC)
        /* all threads have started, it's now time to prevent any new thread
         * or process from starting. Obviously we do this in workers only. We