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 (file)
index 0000000..b1f24d6
--- /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 317645fae545fa0ee5ec7cbb6917b3f348695a2d..31a40995595314c6c65533b0b65aec5fd812e124 100644 (file)
--- 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 979b6a247a66821ce3a6477fa91e5e22765f5c27..6ad20f6af10cb2082ae1fd643d395625a3914714 100644 (file)
--- 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 fcb481c078af50881ff4e85bf11d72c268014a42..abeed56b13659ac06ca5cea2105c8878e0515dae 100644 (file)
--- 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 af59c5d00a8448ca503aef026a2891a95c6d1640..e4f047f3f4e6b076fe10899a8cbbc22ca2380f8d 100644 (file)
--- 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