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>

4 files changed, 15 insertions, 13 deletions

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