MEDIUM: init: always warn when running as root without being asked to

Like many exposed network deamons, haproxy does normally not need to run
as root and strongly recommends against this, unless strictly necessary.
On some operating systems, capabilities even totally alleviate this need.

Lately, maybe due to a raise of containerization or automated config
generation or a bit of both, we've observed a resurgence of this bad
practice, possibly due to the fact that users are just not aware of the
conditions they're using their daemon.

Let's add a warning at boot when starting as root without having requested
it using "uid" or "user". And take this opportunity for warning the user
about the existence of capabilities when supported, and encouraging the
use of a chroot.

This is achieved by leaving global.uid set to -1 by default, allowing us
to detect if it was explicitly set or not.

diff --git a/src/cfgparse-global.c b/src/cfgparse-global.c
index 94a57e713d85cd9199e2e2f82b00f95c2c14c995..b5a756d7a2e6066deb8fba9e7071ac6629f77e84 100644 (file)
--- a/src/cfgparse-global.c
+++ b/src/cfgparse-global.c
@@ -169,7 +169,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
        else if (strcmp(args[0], "uid") == 0) {
                if (alertif_too_many_args(1, file, linenum, args, &err_code))
                        goto out;
-               if (global.uid != 0) {
+               if (global.uid >= 0) {
                        ha_alert("parsing [%s:%d] : user/uid already specified. Continuing.\n", file, linenum);
                        err_code |= ERR_ALERT;
                        goto out;
@@ -189,7 +189,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
        else if (strcmp(args[0], "gid") == 0) {
                if (alertif_too_many_args(1, file, linenum, args, &err_code))
                        goto out;
-               if (global.gid != 0) {
+               if (global.gid >= 0) {
                        ha_alert("parsing [%s:%d] : group/gid already specified. Continuing.\n", file, linenum);
                        err_code |= ERR_ALERT;
                        goto out;
@@ -222,7 +222,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
                struct passwd *ha_user;
                if (alertif_too_many_args(1, file, linenum, args, &err_code))
                        goto out;
-               if (global.uid != 0) {
+               if (global.uid >= 0) {
                        ha_alert("parsing [%s:%d] : user/uid already specified. Continuing.\n", file, linenum);
                        err_code |= ERR_ALERT;
                        goto out;
@@ -250,7 +250,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
                struct group *ha_group;
                if (alertif_too_many_args(1, file, linenum, args, &err_code))
                        goto out;
-               if (global.gid != 0) {
+               if (global.gid >= 0) {
                        ha_alert("parsing [%s:%d] : gid/group was already specified. Continuing.\n", file, linenum);
                        err_code |= ERR_ALERT;
                        goto out;

diff --git a/src/haproxy.c b/src/haproxy.c
index b69b90cee7368f138eaed4c10e4f99dfa1aa91b3..c56a79024a20efe825abd38036798a8d008747d7 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -157,6 +157,8 @@ static unsigned long stopping_tgroup_mask; /* Thread groups acknowledging stoppi
 
 /* global options */
 struct global global = {
+       .uid = -1, // not set
+       .gid = -1, // not set
        .hard_stop_after = TICK_ETERNITY,
        .close_spread_time = TICK_ETERNITY,
        .close_spread_end = TICK_ETERNITY,
@@ -3084,7 +3086,7 @@ static void set_identity(const char *program_name)
 {
        int from_uid __maybe_unused = geteuid();
 
-       if (global.gid) {
+       if (global.gid > 0) {
                if (getgroups(0, NULL) > 0 && setgroups(0, NULL) == -1)
                        ha_warning("[%s.main()] Failed to drop supplementary groups. Using 'gid'/'group'"
                                   " without 'uid'/'user' is generally useless.\n", program_name);
@@ -3104,7 +3106,7 @@ static void set_identity(const char *program_name)
        }
 #endif
 
-       if (global.uid && setuid(global.uid) == -1) {
+       if (global.uid > 0 && setuid(global.uid) == -1) {
                ha_alert("[%s.main()] Cannot set uid %d.\n", program_name, global.uid);
                protocol_unbind_all();
                exit(1);
@@ -3463,7 +3465,7 @@ int main(int argc, char **argv)
                 * and ruid by set_identity() just above, so it's better to
                 * remind the user to fix uncoherent settings.
                 */
-               if (global.uid) {
+               if (global.uid > 0) {
                        ha_alert("[%s.main()] Some configuration options require full "
                                 "privileges, so global.uid cannot be changed.\n", argv[0]);
 #if defined(USE_LINUX_CAP)
@@ -3482,6 +3484,22 @@ int main(int argc, char **argv)
                           " might not work well.\n", argv[0]);
        }
 
+       if (global.uid < 0 && geteuid() == 0)
+               ha_warning("[%s.main()] HAProxy was started as the root user and does "
+                          "not make use of 'user' nor 'uid' global options to drop the "
+                          "privileges. This is generally considered as a bad practice "
+                          "security-wise. If running as root is intentional, please make "
+                          "it explicit using 'uid 0' or 'user root', and also please "
+                          "consider using the 'chroot' directive to isolate the process "
+                          "into a totally empty and read-only directory if possible."
+#if defined(USE_LINUX_CAP)
+                          " Also, since your operating system supports it, always prefer "
+                          "relying on capabilities with unprivileged users than running "
+                          "with full privileges (look for 'setcap' in the configuration"
+                          "manual)."
+#endif
+                          "\n", argv[0]);
+
        /*
         * This is only done in daemon mode because we might want the
         * logs on stdout in mworker mode. If we're NOT in QUIET mode,

diff --git a/src/linuxcap.c b/src/linuxcap.c
index 90072d2aa9c7d907f9b6160d09a225faca04a7e5..4693d1f1165bc676055fa3e1e272a36035a6f361 100644 (file)
--- a/src/linuxcap.c
+++ b/src/linuxcap.c
@@ -94,7 +94,7 @@ int prepare_caps_from_permitted_set(int from_uid, int to_uid)
                return 0;
 
        /* will change ruid and euid later in set_identity() */
-       if (to_uid)
+       if (to_uid > 0)
                return 0;
 
        /* first, let's check if CAP_NET_ADMIN or CAP_NET_RAW is already in
@@ -167,7 +167,7 @@ int prepare_caps_for_setuid(int from_uid, int to_uid)
        if (from_uid != 0)
                return 0;
 
-       if (!to_uid)
+       if (to_uid <= 0)
                return 0;
 
        if (!caplist)
@@ -216,7 +216,7 @@ int finalize_caps_after_setuid(int from_uid, int to_uid)
        if (from_uid != 0)
                return 0;
 
-       if (!to_uid)
+       if (to_uid <= 0)
                return 0;
 
        if (!caplist)