From dd881027c3c556647d5d9f36eda4e9316680647b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 20 Sep 2023 11:50:17 -0400 Subject: [PATCH] syscall: skip TestDeathSignalSetuid if exec fails with a permission error 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 Auto-Submit: Bryan Mills Reviewed-by: Ian Lance Taylor Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot --- src/syscall/exec_pdeathsig_test.go | 103 +++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 29 deletions(-) diff --git a/src/syscall/exec_pdeathsig_test.go b/src/syscall/exec_pdeathsig_test.go index 46ce33443d..a907afd900 100644 --- a/src/syscall/exec_pdeathsig_test.go +++ b/src/syscall/exec_pdeathsig_test.go @@ -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()