fix: fix oom_score adjustments failing if caps set (#15758)

- Fixes an issue where oom scores would fail to be adjusted in cases
where the `coder` binary has capabilities set on it. This is because
`PR_SET_DUMPABLE` is set to `0` when a process is executed with elevated
capabilities. The fix is to flip `PR_SET_DUMPABLE` to `1` prior to
writing to `oom_score_adj`.

agent/agentexec/cli_linux.go

@@ -15,6 +15,7 @@ import (
 
 	"golang.org/x/sys/unix"
 	"golang.org/x/xerrors"
+	"kernel.org/pub/linux/libs/security/libcap/cap"
 )
 
 // CLI runs the agent-exec command. It should only be called by the cli package.
@@ -48,14 +49,6 @@ func CLI() error {
 		return xerrors.Errorf("no exec command provided %+v", os.Args)
 	}
 
-	if *nice == unset {
-		// If an explicit nice score isn't set, we use the default.
-		*nice, err = defaultNiceScore()
-		if err != nil {
-			return xerrors.Errorf("get default nice score: %w", err)
-		}
-	}
-
 	if *oom == unset {
 		// If an explicit oom score isn't set, we use the default.
 		*oom, err = defaultOOMScore()
@@ -64,12 +57,31 @@ func CLI() error {
 		}
 	}
 
-	err = unix.Setpriority(unix.PRIO_PROCESS, 0, *nice)
+	if *nice == unset {
+		// If an explicit nice score isn't set, we use the default.
+		*nice, err = defaultNiceScore()
+		if err != nil {
+			return xerrors.Errorf("get default nice score: %w", err)
+		}
+	}
+
+	// We drop effective caps prior to setting dumpable so that we limit the
+	// impact of someone attempting to hijack the process (i.e. with a debugger)
+	// to take advantage of the capabilities of the agent process. We encourage
+	// users to set cap_net_admin on the agent binary for improved networking
+	// performance and doing so results in the process having its SET_DUMPABLE
+	// attribute disabled (meaning we cannot adjust the oom score).
+	err = dropEffectiveCaps()
 	if err != nil {
-		// We alert the user instead of failing the command since it can be difficult to debug
-		// for a template admin otherwise. It's quite possible (and easy) to set an
-		// inappriopriate value for niceness.
-		printfStdErr("failed to adjust niceness to %d for cmd %+v: %v", *nice, args, err)
+		printfStdErr("failed to drop effective caps: %v", err)
+	}
+
+	// Set dumpable to 1 so that we can adjust the oom score. If the process
+	// doesn't have capabilities or has an suid/sgid bit set, this is already
+	// set.
+	err = unix.Prctl(unix.PR_SET_DUMPABLE, 1, 0, 0, 0)
+	if err != nil {
+		printfStdErr("failed to set dumpable: %v", err)
 	}
 
 	err = writeOOMScoreAdj(*oom)
@@ -77,7 +89,21 @@ func CLI() error {
 		// We alert the user instead of failing the command since it can be difficult to debug
 		// for a template admin otherwise. It's quite possible (and easy) to set an
 		// inappriopriate value for oom_score_adj.
-		printfStdErr("failed to adjust oom score to %d for cmd %+v: %v", *oom, args, err)
+		printfStdErr("failed to adjust oom score to %d for cmd %+v: %v", *oom, execArgs(os.Args), err)
+	}
+
+	// Set dumpable back to 0 just to be safe. It's not inherited for execve anyways.
+	err = unix.Prctl(unix.PR_SET_DUMPABLE, 0, 0, 0, 0)
+	if err != nil {
+		printfStdErr("failed to unset dumpable: %v", err)
+	}
+
+	err = unix.Setpriority(unix.PRIO_PROCESS, 0, *nice)
+	if err != nil {
+		// We alert the user instead of failing the command since it can be difficult to debug
+		// for a template admin otherwise. It's quite possible (and easy) to set an
+		// inappriopriate value for niceness.
+		printfStdErr("failed to adjust niceness to %d for cmd %+v: %v", *nice, args, err)
 	}
 
 	path, err := exec.LookPath(args[0])
@@ -151,3 +177,16 @@ func execArgs(args []string) []string {
 func printfStdErr(format string, a ...any) {
 	_, _ = fmt.Fprintf(os.Stderr, "coder-agent: %s\n", fmt.Sprintf(format, a...))
 }
+
+func dropEffectiveCaps() error {
+	proc := cap.GetProc()
+	err := proc.ClearFlag(cap.Effective)
+	if err != nil {
+		return xerrors.Errorf("clear effective caps: %w", err)
+	}
+	err = proc.SetProc()
+	if err != nil {
+		return xerrors.Errorf("set proc: %w", err)
+	}
+	return nil
+}

agent/agentexec/cli_linux_test.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/unix"
+	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/v2/testutil"
 )
@@ -50,6 +51,32 @@ func TestCLI(t *testing.T) {
 		requireOOMScore(t, cmd.Process.Pid, expectedOOM)
 		requireNiceScore(t, cmd.Process.Pid, expectedNice)
 	})
+
+	t.Run("Capabilities", func(t *testing.T) {
+		testdir := filepath.Dir(TestBin)
+		capDir := filepath.Join(testdir, "caps")
+		err := os.Mkdir(capDir, 0o755)
+		require.NoError(t, err)
+		bin := buildBinary(capDir)
+		// Try to set capabilities on the binary. This should work fine in CI but
+		// it's possible some developers may be working in an environment where they don't have the necessary permissions.
+		err = setCaps(t, bin, "cap_net_admin")
+		if os.Getenv("CI") != "" {
+			require.NoError(t, err)
+		} else if err != nil {
+			t.Skipf("unable to set capabilities for test: %v", err)
+		}
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		cmd, path := binCmd(ctx, t, bin, 123, 12)
+		err = cmd.Start()
+		require.NoError(t, err)
+		go cmd.Wait()
+
+		waitForSentinel(ctx, t, cmd, path)
+		// This is what we're really testing, a binary with added capabilities requires setting dumpable.
+		requireOOMScore(t, cmd.Process.Pid, 123)
+		requireNiceScore(t, cmd.Process.Pid, 12)
+	})
 }
 
 func requireNiceScore(t *testing.T, pid int, score int) {
@@ -94,7 +121,7 @@ func waitForSentinel(ctx context.Context, t *testing.T, cmd *exec.Cmd, path stri
 	}
 }
 
-func cmd(ctx context.Context, t *testing.T, oom, nice int) (*exec.Cmd, string) {
+func binCmd(ctx context.Context, t *testing.T, bin string, oom, nice int) (*exec.Cmd, string) {
 	var (
 		args = execArgs(oom, nice)
 		dir  = t.TempDir()
@@ -103,7 +130,7 @@ func cmd(ctx context.Context, t *testing.T, oom, nice int) (*exec.Cmd, string) {
 
 	args = append(args, "sh", "-c", fmt.Sprintf("touch %s && sleep 10m", file))
 	//nolint:gosec
-	cmd := exec.CommandContext(ctx, TestBin, args...)
+	cmd := exec.CommandContext(ctx, bin, args...)
 
 	// We set this so we can also easily kill the sleep process the shell spawns.
 	cmd.SysProcAttr = &syscall.SysProcAttr{
@@ -127,6 +154,10 @@ func cmd(ctx context.Context, t *testing.T, oom, nice int) (*exec.Cmd, string) {
 	return cmd, file
 }
 
+func cmd(ctx context.Context, t *testing.T, oom, nice int) (*exec.Cmd, string) {
+	return binCmd(ctx, t, TestBin, oom, nice)
+}
+
 func expectedOOMScore(t *testing.T) int {
 	t.Helper()
 
@@ -171,3 +202,14 @@ func execArgs(oom int, nice int) []string {
 	execArgs = append(execArgs, "--")
 	return execArgs
 }
+
+func setCaps(t *testing.T, bin string, caps ...string) error {
+	t.Helper()
+
+	setcap := fmt.Sprintf("sudo -n setcap %s=ep %s", strings.Join(caps, ", "), bin)
+	out, err := exec.CommandContext(context.Background(), "sh", "-c", setcap).CombinedOutput()
+	if err != nil {
+		return xerrors.Errorf("setcap %q (%s): %w", setcap, out, err)
+	}
+	return nil
+}