Attachment 680418 Details for Bug 894626 – Propoxed upstream patch

[patch] Propoxed upstream patch

hap-vuln-supp-groups.diff (text/plain), 2.34 KB, created by Jan Lieskovsky on 2013-01-17 18:04:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Lieskovsky

Created: 2013-01-17 18:04:05 UTC

Size: 2.34 KB

patch

obsolete

>commit 0da2cb0ad427223955a601bd1220a5500cb4d2f1
>Author: Michael Scherer <misc@zarb.org>
>Date:   Sat Jan 12 18:35:19 2013 +0100
>
>    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
>      wheel:x:10:root,misc
>
>diff --git a/doc/configuration.txt b/doc/configuration.txt
>index 5117404..c88592a 100644
>--- a/doc/configuration.txt
>+++ b/doc/configuration.txt
>@@ -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>
>diff --git a/src/haproxy.c b/src/haproxy.c
>index 0d826d2..4503a01 100644
>--- a/src/haproxy.c
>+++ b/src/haproxy.c
>@@ -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) {

Actions: View | Diff

Attachments on bug 894626: 677460 | 678262 | 680418