From: Wander Lairson Costa <wcosta@mozilla.com>
Date: Fri, 10 Feb 2017 06:10:48 +0000 (-0200)
Subject: syscall: only call setgroups if we need to
X-Git-Tag: go1.9beta1~1526
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=79f6a5c7bd684f2e6007ee505b522440beb86bf0;p=gostls13.git

syscall: only call setgroups if we need to

If the caller set ups a Credential in os/exec.Command,
os/exec.Command.Start will end up calling setgroups(2), even if no
supplementary groups were given.

Only root can call setgroups(2) on BSD kernels, which causes Start to
fail for non-root users when they try to set uid and gid for the new
process.

We fix by introducing a new field to syscall.Credential named
NoSetGroups, and setgroups(2) is only called if it is false.
We make this field with inverted logic to preserve backward
compatibility.

RELNOTES=yes

Change-Id: I3cff1f21c117a1430834f640ef21fd4e87e06804
Reviewed-on: https://go-review.googlesource.com/36697
Reviewed-by: Ian Lance Taylor <iant@golang.org>
---

diff --git a/src/os/exec/exec_posix_test.go b/src/os/exec/exec_posix_test.go
new file mode 100644
index 0000000000..b1f24d6c4e
--- /dev/null
+++ b/src/os/exec/exec_posix_test.go
@@ -0,0 +1,45 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build darwin dragonfly freebsd linux netbsd openbsd solaris
+
+package exec_test
+
+import (
+	"os/user"
+	"strconv"
+	"syscall"
+	"testing"
+)
+
+func TestCredentialNoSetGroups(t *testing.T) {
+	u, err := user.Current()
+	if err != nil {
+		t.Fatalf("error getting current user: %v", err)
+	}
+
+	uid, err := strconv.Atoi(u.Uid)
+	if err != nil {
+		t.Fatalf("error converting Uid=%s to integer: %v", u.Uid, err)
+	}
+
+	gid, err := strconv.Atoi(u.Gid)
+	if err != nil {
+		t.Fatalf("error converting Gid=%s to integer: %v", u.Gid, err)
+	}
+
+	// If NoSetGroups is true, setgroups isn't called and cmd.Run should succeed
+	cmd := helperCommand(t, "echo", "foo")
+	cmd.SysProcAttr = &syscall.SysProcAttr{
+		Credential: &syscall.Credential{
+			Uid:         uint32(uid),
+			Gid:         uint32(gid),
+			NoSetGroups: true,
+		},
+	}
+
+	if err = cmd.Run(); err != nil {
+		t.Errorf("Failed to run command: %v", err)
+	}
+}
diff --git a/src/syscall/exec_bsd.go b/src/syscall/exec_bsd.go
index 317645fae5..31a4099559 100644
--- a/src/syscall/exec_bsd.go
+++ b/src/syscall/exec_bsd.go
@@ -146,9 +146,11 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 		if ngroups > 0 {
 			groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
 		}
-		_, _, err1 = RawSyscall(SYS_SETGROUPS, ngroups, groups, 0)
-		if err1 != 0 {
-			goto childerror
+		if !cred.NoSetGroups {
+			_, _, err1 = RawSyscall(SYS_SETGROUPS, ngroups, groups, 0)
+			if err1 != 0 {
+				goto childerror
+			}
 		}
 		_, _, err1 = RawSyscall(SYS_SETGID, uintptr(cred.Gid), 0, 0)
 		if err1 != 0 {
diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go
index 979b6a247a..6ad20f6af1 100644
--- a/src/syscall/exec_linux.go
+++ b/src/syscall/exec_linux.go
@@ -210,10 +210,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 		if ngroups > 0 {
 			groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
 		}
-		// Don't call setgroups in case of user namespace, gid mappings
-		// and disabled setgroups, because otherwise unprivileged user namespace
-		// will fail with any non-empty SysProcAttr.Credential.
-		if !(sys.GidMappings != nil && !sys.GidMappingsEnableSetgroups && ngroups == 0) {
+		if !(sys.GidMappings != nil && !sys.GidMappingsEnableSetgroups && ngroups == 0) && !cred.NoSetGroups {
 			_, _, err1 = RawSyscall(_SYS_setgroups, ngroups, groups, 0)
 			if err1 != 0 {
 				goto childerror
diff --git a/src/syscall/exec_solaris.go b/src/syscall/exec_solaris.go
index fcb481c078..abeed56b13 100644
--- a/src/syscall/exec_solaris.go
+++ b/src/syscall/exec_solaris.go
@@ -143,9 +143,11 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 		if ngroups > 0 {
 			groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
 		}
-		err1 = setgroups1(ngroups, groups)
-		if err1 != 0 {
-			goto childerror
+		if !cred.NoSetGroups {
+			err1 = setgroups1(ngroups, groups)
+			if err1 != 0 {
+				goto childerror
+			}
 		}
 		err1 = setgid(uintptr(cred.Gid))
 		if err1 != 0 {
diff --git a/src/syscall/exec_unix.go b/src/syscall/exec_unix.go
index af59c5d00a..e4f047f3f4 100644
--- a/src/syscall/exec_unix.go
+++ b/src/syscall/exec_unix.go
@@ -112,9 +112,10 @@ func SetNonblock(fd int, nonblocking bool) (err error) {
 // Credential holds user and group identities to be assumed
 // by a child process started by StartProcess.
 type Credential struct {
-	Uid    uint32   // User ID.
-	Gid    uint32   // Group ID.
-	Groups []uint32 // Supplementary group IDs.
+	Uid         uint32   // User ID.
+	Gid         uint32   // Group ID.
+	Groups      []uint32 // Supplementary group IDs.
+	NoSetGroups bool     // If true, don't set supplementary groups
 }
 
 // ProcAttr holds attributes that will be applied to a new process started