BUG/MEDIUM: remove supplementary groups when changing gid

Without it, haproxy will retain the group membership of root, which may
give more access than intended to the process. For example, haproxy would
still be in the wheel group on Fedora 18, as seen with :

  # haproxy -f /etc/haproxy/haproxy.cfg

  # ps a -o pid,user,group,command | grep hapr
  3545 haproxy  haproxy  haproxy -f /etc/haproxy/haproxy.cfg
  4356 root     root     grep --color=auto hapr
  # grep Group /proc/3545/status
  Groups: 0 1 2 3 4 6 10
  # getent group wheel
  wheel10:root,misc

[WT: The issue has been investigated by independent security research team
     and realized by itself not being able to allow security exploitation.
     Additionally, dropping groups is not allowed to unprivileged users,
     though this mode of deployment is quite common. Thus a warning is
     emitted in this case to inform the user. The fix could be backported
     into all supported versions as the issue has always been there. ]
This commit is contained in:
Michael Scherer 2013-01-12 18:35:19 +01:00 committed by Willy Tarreau
parent 973ca49795
commit ab012dd394
2 changed files with 13 additions and 4 deletions

View File

@ -532,6 +532,8 @@ gid <number>
Changes the process' group ID to <number>. It is recommended that the group
ID is dedicated to HAProxy or to a small set of similar daemons. HAProxy must
be started with a user belonging to this group, or with superuser privileges.
Note that if haproxy is started from a user having supplementary groups, it
will only be able to drop these groups if started with superuser privileges.
See also "group" and "uid".
group <group name>

View File

@ -44,6 +44,7 @@
#include <sys/resource.h>
#include <time.h>
#include <syslog.h>
#include <grp.h>
#ifdef USE_CPU_AFFINITY
#define __USE_GNU
#include <sched.h>
@ -1416,10 +1417,16 @@ int main(int argc, char **argv)
*/
/* setgid / setuid */
if (global.gid && setgid(global.gid) == -1) {
Alert("[%s.main()] Cannot set gid %d.\n", argv[0], global.gid);
protocol_unbind_all();
exit(1);
if (global.gid) {
if (getgroups(0, NULL) > 0 && setgroups(0, NULL) == -1)
Warning("[%s.main()] Failed to drop supplementary groups. Using 'gid'/'group'"
" without 'uid'/'user' is generally useless.\n", argv[0]);
if (setgid(global.gid) == -1) {
Alert("[%s.main()] Cannot set gid %d.\n", argv[0], global.gid);
protocol_unbind_all();
exit(1);
}
}
if (global.uid && setuid(global.uid) == -1) {