diff --git a/agent/agentexec/cli_linux.go b/agent/agentexec/cli_linux.go index 9c6568c81811b..e357ba41acb27 100644 --- a/agent/agentexec/cli_linux.go +++ b/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,6 +49,14 @@ func CLI() error { return xerrors.Errorf("no exec command provided %+v", os.Args) } + if *oom == unset { + // If an explicit oom score isn't set, we use the default. + *oom, err = defaultOOMScore() + if err != nil { + return xerrors.Errorf("get default oom score: %w", err) + } + } + if *nice == unset { // If an explicit nice score isn't set, we use the default. *nice, err = defaultNiceScore() @@ -56,20 +65,23 @@ func CLI() error { } } - if *oom == unset { - // If an explicit oom score isn't set, we use the default. - *oom, err = defaultOOMScore() - if err != nil { - return xerrors.Errorf("get default oom 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 { + printfStdErr("failed to drop effective caps: %v", err) } - err = unix.Setpriority(unix.PRIO_PROCESS, 0, *nice) + // 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 { - // 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 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 +} diff --git a/agent/agentexec/cli_linux_test.go b/agent/agentexec/cli_linux_test.go index aef43a3b6b23b..088e2af874302 100644 --- a/agent/agentexec/cli_linux_test.go +++ b/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 +} diff --git a/go.mod b/go.mod index ccf59c9e2a0c6..fc5b6d394b26a 100644 --- a/go.mod +++ b/go.mod @@ -212,6 +212,7 @@ require ( github.com/google/go-github/v61 v61.0.0 github.com/mocktools/go-smtp-mock/v2 v2.4.0 github.com/natefinch/atomic v1.0.1 + kernel.org/pub/linux/libs/security/libcap/cap v1.2.73 ) require ( @@ -248,6 +249,7 @@ require ( github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 // indirect + kernel.org/pub/linux/libs/security/libcap/psx v1.2.73 // indirect ) require ( diff --git a/go.sum b/go.sum index e8e9d02b1a693..6483c3505565c 100644 --- a/go.sum +++ b/go.sum @@ -1266,6 +1266,10 @@ howett.net/plist v1.0.0 h1:7CrbWYbPPO/PyNy38b2EB/+gYbjCe2DXBxgtOOZbSQM= howett.net/plist v1.0.0/go.mod h1:lqaXoTrLY4hg8tnEzNru53gicrbv7rrk+2xJA/7hw9g= inet.af/peercred v0.0.0-20210906144145-0893ea02156a h1:qdkS8Q5/i10xU2ArJMKYhVa1DORzBfYS/qA2UK2jheg= inet.af/peercred v0.0.0-20210906144145-0893ea02156a/go.mod h1:FjawnflS/udxX+SvpsMgZfdqx2aykOlkISeAsADi5IU= +kernel.org/pub/linux/libs/security/libcap/cap v1.2.73 h1:Th2b8jljYqkyZKS3aD3N9VpYsQpHuXLgea+SZUIfODA= +kernel.org/pub/linux/libs/security/libcap/cap v1.2.73/go.mod h1:hbeKwKcboEsxARYmcy/AdPVN11wmT/Wnpgv4k4ftyqY= +kernel.org/pub/linux/libs/security/libcap/psx v1.2.73 h1:SEAEUiPVylTD4vqqi+vtGkSnXeP2FcRO3FoZB1MklMw= +kernel.org/pub/linux/libs/security/libcap/psx v1.2.73/go.mod h1:+l6Ee2F59XiJ2I6WR5ObpC1utCQJZ/VLsEbQCD8RG24= lukechampine.com/uint128 v1.3.0 h1:cDdUVfRwDUDovz610ABgFD17nXD4/uDgVHl2sC3+sbo= lukechampine.com/uint128 v1.3.0/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk= modernc.org/cc/v3 v3.41.0 h1:QoR1Sn3YWlmA1T4vLaKZfawdVtSiGx8H+cEojbC7v1Q=