]> Cypherpunks.ru repositories - gostls13.git/commitdiff
syscall: skip TestDeathSignalSetuid if exec fails with a permission error
authorBryan C. Mills <bcmills@google.com>
Wed, 20 Sep 2023 15:50:17 +0000 (11:50 -0400)
committerGopher Robot <gobot@golang.org>
Wed, 20 Sep 2023 21:23:17 +0000 (21:23 +0000)
Also explicitly look up user "nobody" (or "gopher" on the Go builders)
if running as root, instead of hard-coding UID/GID 99.

Fixes #62719.

Change-Id: I9fa8955f2c239804fa775f2478a5274af9330822
Reviewed-on: https://go-review.googlesource.com/c/go/+/529795
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/syscall/exec_pdeathsig_test.go

index 46ce33443da0349eb6ae95d782df1db7020166ea..a907afd900a0021fce82e0502351d3b352624b03 100644 (file)
@@ -14,26 +14,26 @@ import (
        "os"
        "os/exec"
        "os/signal"
+       "os/user"
        "path/filepath"
+       "strconv"
+       "strings"
        "syscall"
        "testing"
-       "time"
 )
 
-func TestDeathSignal(t *testing.T) {
-       if os.Getuid() != 0 {
-               t.Skip("skipping root only test")
-       }
-       if testing.Short() && testenv.Builder() != "" && os.Getenv("USER") == "swarming" {
-               // The Go build system's swarming user is known not to be root.
-               // Unfortunately, it sometimes appears as root due the current
-               // implementation of a no-network check using 'unshare -n -r'.
-               // Since this test does need root to work, we need to skip it.
-               t.Skip("skipping root only test on a non-root builder")
+// TestDeathSignalSetuid verifies that a command run with a different UID still
+// receives PDeathsig; it is a regression test for https://go.dev/issue/9686.
+func TestDeathSignalSetuid(t *testing.T) {
+       if testing.Short() {
+               t.Skipf("skipping test that copies its binary into temp dir")
        }
 
-       // Copy the test binary to a location that a non-root user can read/execute
-       // after we drop privileges
+       // Copy the test binary to a location that another user can read/execute
+       // after we drop privileges.
+       //
+       // TODO(bcmills): Why do we believe that another users will be able to
+       // execute a binary in this directory? (It could be mounted noexec.)
        tempDir, err := os.MkdirTemp("", "TestDeathSignal")
        if err != nil {
                t.Fatalf("cannot create temporary directory: %v", err)
@@ -61,8 +61,8 @@ func TestDeathSignal(t *testing.T) {
                t.Fatalf("failed to close test binary %q, %v", tmpBinary, err)
        }
 
-       cmd := exec.Command(tmpBinary)
-       cmd.Env = append(os.Environ(), "GO_DEATHSIG_PARENT=1")
+       cmd := testenv.Command(t, tmpBinary)
+       cmd.Env = append(cmd.Environ(), "GO_DEATHSIG_PARENT=1")
        chldStdin, err := cmd.StdinPipe()
        if err != nil {
                t.Fatalf("failed to create new stdin pipe: %v", err)
@@ -71,10 +71,17 @@ func TestDeathSignal(t *testing.T) {
        if err != nil {
                t.Fatalf("failed to create new stdout pipe: %v", err)
        }
-       cmd.Stderr = os.Stderr
+       stderr := new(strings.Builder)
+       cmd.Stderr = stderr
 
        err = cmd.Start()
-       defer cmd.Wait()
+       defer func() {
+               chldStdin.Close()
+               cmd.Wait()
+               if stderr.Len() > 0 {
+                       t.Logf("stderr:\n%s", stderr)
+               }
+       }()
        if err != nil {
                t.Fatalf("failed to start first child process: %v", err)
        }
@@ -84,21 +91,57 @@ func TestDeathSignal(t *testing.T) {
        if got, err := chldPipe.ReadString('\n'); got == "start\n" {
                syscall.Kill(cmd.Process.Pid, syscall.SIGTERM)
 
-               go func() {
-                       time.Sleep(5 * time.Second)
-                       chldStdin.Close()
-               }()
-
                want := "ok\n"
                if got, err = chldPipe.ReadString('\n'); got != want {
                        t.Fatalf("expected %q, received %q, %v", want, got, err)
                }
+       } else if got == "skip\n" {
+               t.Skipf("skipping: parent could not run child program as selected user")
        } else {
                t.Fatalf("did not receive start from child, received %q, %v", got, err)
        }
 }
 
 func deathSignalParent() {
+       var (
+               u   *user.User
+               err error
+       )
+       if os.Getuid() == 0 {
+               tryUsers := []string{"nobody"}
+               if testenv.Builder() != "" {
+                       tryUsers = append(tryUsers, "gopher")
+               }
+               for _, name := range tryUsers {
+                       u, err = user.Lookup(name)
+                       if err == nil {
+                               break
+                       }
+                       fmt.Fprintf(os.Stderr, "Lookup(%q): %v\n", name, err)
+               }
+       }
+       if u == nil {
+               // If we couldn't find an unprivileged user to run as, try running as
+               // the current user. (Empirically this still causes the call to Start to
+               // fail with a permission error if running as a non-root user on Linux.)
+               u, err = user.Current()
+               if err != nil {
+                       fmt.Fprintln(os.Stderr, err)
+                       os.Exit(1)
+               }
+       }
+
+       uid, err := strconv.ParseUint(u.Uid, 10, 32)
+       if err != nil {
+               fmt.Fprintf(os.Stderr, "invalid UID: %v\n", err)
+               os.Exit(1)
+       }
+       gid, err := strconv.ParseUint(u.Gid, 10, 32)
+       if err != nil {
+               fmt.Fprintf(os.Stderr, "invalid GID: %v\n", err)
+               os.Exit(1)
+       }
+
        cmd := exec.Command(os.Args[0])
        cmd.Env = append(os.Environ(),
                "GO_DEATHSIG_PARENT=",
@@ -107,16 +150,18 @@ func deathSignalParent() {
        cmd.Stdin = os.Stdin
        cmd.Stdout = os.Stdout
        attrs := syscall.SysProcAttr{
-               Pdeathsig: syscall.SIGUSR1,
-               // UID/GID 99 is the user/group "nobody" on RHEL/Fedora and is
-               // unused on Ubuntu
-               Credential: &syscall.Credential{Uid: 99, Gid: 99},
+               Pdeathsig:  syscall.SIGUSR1,
+               Credential: &syscall.Credential{Uid: uint32(uid), Gid: uint32(gid)},
        }
        cmd.SysProcAttr = &attrs
 
-       err := cmd.Start()
-       if err != nil {
-               fmt.Fprintf(os.Stderr, "death signal parent error: %v\n", err)
+       fmt.Fprintf(os.Stderr, "starting process as user %q\n", u.Username)
+       if err := cmd.Start(); err != nil {
+               fmt.Fprintln(os.Stderr, err)
+               if testenv.SyscallIsNotSupported(err) {
+                       fmt.Println("skip")
+                       os.Exit(0)
+               }
                os.Exit(1)
        }
        cmd.Wait()