Skip to content

Commit a6986e1

Browse files
committed
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.
1 parent c97ced3 commit a6986e1

3 files changed

Lines changed: 28 additions & 10 deletions

File tree

src/cfgparse-global.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
169169
else if (strcmp(args[0], "uid") == 0) {
170170
if (alertif_too_many_args(1, file, linenum, args, &err_code))
171171
goto out;
172-
if (global.uid != 0) {
172+
if (global.uid >= 0) {
173173
ha_alert("parsing [%s:%d] : user/uid already specified. Continuing.\n", file, linenum);
174174
err_code |= ERR_ALERT;
175175
goto out;
@@ -189,7 +189,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
189189
else if (strcmp(args[0], "gid") == 0) {
190190
if (alertif_too_many_args(1, file, linenum, args, &err_code))
191191
goto out;
192-
if (global.gid != 0) {
192+
if (global.gid >= 0) {
193193
ha_alert("parsing [%s:%d] : group/gid already specified. Continuing.\n", file, linenum);
194194
err_code |= ERR_ALERT;
195195
goto out;
@@ -222,7 +222,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
222222
struct passwd *ha_user;
223223
if (alertif_too_many_args(1, file, linenum, args, &err_code))
224224
goto out;
225-
if (global.uid != 0) {
225+
if (global.uid >= 0) {
226226
ha_alert("parsing [%s:%d] : user/uid already specified. Continuing.\n", file, linenum);
227227
err_code |= ERR_ALERT;
228228
goto out;
@@ -250,7 +250,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
250250
struct group *ha_group;
251251
if (alertif_too_many_args(1, file, linenum, args, &err_code))
252252
goto out;
253-
if (global.gid != 0) {
253+
if (global.gid >= 0) {
254254
ha_alert("parsing [%s:%d] : gid/group was already specified. Continuing.\n", file, linenum);
255255
err_code |= ERR_ALERT;
256256
goto out;

src/haproxy.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ static unsigned long stopping_tgroup_mask; /* Thread groups acknowledging stoppi
157157

158158
/* global options */
159159
struct global global = {
160+
.uid = -1, // not set
161+
.gid = -1, // not set
160162
.hard_stop_after = TICK_ETERNITY,
161163
.close_spread_time = TICK_ETERNITY,
162164
.close_spread_end = TICK_ETERNITY,
@@ -3084,7 +3086,7 @@ static void set_identity(const char *program_name)
30843086
{
30853087
int from_uid __maybe_unused = geteuid();
30863088

3087-
if (global.gid) {
3089+
if (global.gid > 0) {
30883090
if (getgroups(0, NULL) > 0 && setgroups(0, NULL) == -1)
30893091
ha_warning("[%s.main()] Failed to drop supplementary groups. Using 'gid'/'group'"
30903092
" without 'uid'/'user' is generally useless.\n", program_name);
@@ -3104,7 +3106,7 @@ static void set_identity(const char *program_name)
31043106
}
31053107
#endif
31063108

3107-
if (global.uid && setuid(global.uid) == -1) {
3109+
if (global.uid > 0 && setuid(global.uid) == -1) {
31083110
ha_alert("[%s.main()] Cannot set uid %d.\n", program_name, global.uid);
31093111
protocol_unbind_all();
31103112
exit(1);
@@ -3463,7 +3465,7 @@ int main(int argc, char **argv)
34633465
* and ruid by set_identity() just above, so it's better to
34643466
* remind the user to fix uncoherent settings.
34653467
*/
3466-
if (global.uid) {
3468+
if (global.uid > 0) {
34673469
ha_alert("[%s.main()] Some configuration options require full "
34683470
"privileges, so global.uid cannot be changed.\n", argv[0]);
34693471
#if defined(USE_LINUX_CAP)
@@ -3482,6 +3484,22 @@ int main(int argc, char **argv)
34823484
" might not work well.\n", argv[0]);
34833485
}
34843486

3487+
if (global.uid < 0 && geteuid() == 0)
3488+
ha_warning("[%s.main()] HAProxy was started as the root user and does "
3489+
"not make use of 'user' nor 'uid' global options to drop the "
3490+
"privileges. This is generally considered as a bad practice "
3491+
"security-wise. If running as root is intentional, please make "
3492+
"it explicit using 'uid 0' or 'user root', and also please "
3493+
"consider using the 'chroot' directive to isolate the process "
3494+
"into a totally empty and read-only directory if possible."
3495+
#if defined(USE_LINUX_CAP)
3496+
" Also, since your operating system supports it, always prefer "
3497+
"relying on capabilities with unprivileged users than running "
3498+
"with full privileges (look for 'setcap' in the configuration"
3499+
"manual)."
3500+
#endif
3501+
"\n", argv[0]);
3502+
34853503
/*
34863504
* This is only done in daemon mode because we might want the
34873505
* logs on stdout in mworker mode. If we're NOT in QUIET mode,

src/linuxcap.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ int prepare_caps_from_permitted_set(int from_uid, int to_uid)
9494
return 0;
9595

9696
/* will change ruid and euid later in set_identity() */
97-
if (to_uid)
97+
if (to_uid > 0)
9898
return 0;
9999

100100
/* 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)
167167
if (from_uid != 0)
168168
return 0;
169169

170-
if (!to_uid)
170+
if (to_uid <= 0)
171171
return 0;
172172

173173
if (!caplist)
@@ -216,7 +216,7 @@ int finalize_caps_after_setuid(int from_uid, int to_uid)
216216
if (from_uid != 0)
217217
return 0;
218218

219-
if (!to_uid)
219+
if (to_uid <= 0)
220220
return 0;
221221

222222
if (!caplist)

0 commit comments

Comments
 (0)