diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1d49b22..be57dcc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -112,7 +112,7 @@ jobs: sudo install -m 0755 /tmp/nsjail/nsjail /usr/sbin/nsjail - name: Run sandbox integration tests - run: sudo -E go test -v -run 'TestSandboxedWorker' ./internal/execution/worker/... + run: sudo -E go test -v -run 'TestSandboxedWorker|TestApplySandbox' ./internal/execution/worker/... build_docker: name: Build Docker Image diff --git a/Dockerfile b/Dockerfile index 14906f8..4f5d809 100644 --- a/Dockerfile +++ b/Dockerfile @@ -46,7 +46,7 @@ RUN make -j$(nproc) # Test-only stage: golang base image (Debian bookworm) + nsjail built from source. # Go is pre-installed; all nsjail build deps are in Debian main — no universe needed. # Used by `make test-sandbox`; not referenced by the production image. -FROM golang:1.24 AS test-sandbox +FROM golang:1.25 AS test-sandbox RUN apt-get update && apt-get install -y --no-install-recommends \ autoconf \ bison \ diff --git a/Makefile b/Makefile index cd571ce..e684ca1 100644 --- a/Makefile +++ b/Makefile @@ -25,14 +25,14 @@ test-unit: # Run sandbox integration tests inside a privileged container. # Supports Docker (default) and Podman: CONTAINER_ENGINE=podman make test-sandbox # On Linux with nsjail installed locally, use: -# go test -v -run 'TestSandboxedWorker' ./internal/execution/worker/... +# go test -v -run 'TestSandboxedWorker|TestApplySandbox' ./internal/execution/worker/... test-sandbox: $(CONTAINER_ENGINE) build --target test-sandbox -t shimmy-test-sandbox . $(CONTAINER_ENGINE) run --rm --privileged \ -v $(shell pwd):/workspace \ -w /workspace \ shimmy-test-sandbox \ - go test -v -run 'TestSandboxedWorker' ./internal/execution/worker/... + go test -v -run 'TestSandboxedWorker|TestApplySandbox' ./internal/execution/worker/... lcov: gcov2lcov -infile=coverage.out -outfile=lcov.info diff --git a/README.md b/README.md index e8963d4..896085e 100644 --- a/README.md +++ b/README.md @@ -270,7 +270,27 @@ Enable sandboxing with `--sandbox` and configure it with the flags below: | `--sandbox-memory-mb` | `SANDBOX_MEMORY_LIMIT` | `0` (unlimited) | Memory limit in megabytes | | `--sandbox-max-fds` | `SANDBOX_MAX_FDS` | `0` (nsjail default) | Maximum open file descriptors | | `--sandbox-disable-network` | `SANDBOX_DISABLE_NETWORK` | `false` | Disable network access inside the sandbox | -| `--sandbox-seccomp` | `SANDBOX_SECCOMP` | `false` | Enable seccomp syscall filtering | +| `--sandbox-seccomp-policy-file` | `SANDBOX_SECCOMP_POLICY_FILE` | — | Path to a [kafel](https://github.com/google/kafel) seccomp policy file | +| `--sandbox-seccomp-string` | `SANDBOX_SECCOMP_STRING` | — | Inline kafel seccomp policy (mutually exclusive with the file) | +| `--sandbox-disable-clone-newpid` | `SANDBOX_DISABLE_CLONE_NEWPID` | `false` | Keep the worker in the host PID namespace | +| `--sandbox-disable-clone-newipc` | `SANDBOX_DISABLE_CLONE_NEWIPC` | `false` | Keep the worker in the host IPC namespace | +| `--sandbox-disable-clone-newuts` | `SANDBOX_DISABLE_CLONE_NEWUTS` | `false` | Keep the worker in the host UTS namespace | +| `--sandbox-disable-clone-newcgroup` | `SANDBOX_DISABLE_CLONE_NEWCGROUP` | `false` | Keep the worker in the host cgroup namespace | +| `--sandbox-clone-newuser` | `SANDBOX_CLONE_NEWUSER` | `auto` | User namespace: `auto` (drop when running as uid 0), `enabled` (always keep), `disabled` (always drop) | +| `--sandbox-verbose` | `SANDBOX_VERBOSE` | `false` | Let nsjail log to stderr at full verbosity (default: warnings and errors only) | + +List-valued `SANDBOX_*` env vars are **comma-separated**, e.g. `SANDBOX_RO_BINDS=/usr,/bin,/lib,/lib64`. + +The worker process inherits shimmy's environment (`PATH`, `AWS_*`, …) and, unless +`--cwd` is set, its working directory — so a sandboxed worker behaves like a +non-sandboxed one. `nsjail` runs `execve` (not a `PATH` search), but shimmy resolves +the command against `PATH` before handing it over, so a bare `-c python3` still works. +nsjail's own diagnostics (including cmdline-parse and namespace-setup failures) go to +shimmy's stderr. + +seccomp is off unless you supply an explicit kafel policy; nsjail always applies +`NO_NEW_PRIVS` regardless. Writing a policy that covers your evaluation runtime's +syscall surface (NumPy, Matplotlib, …) is up to you. A typical invocation for an untrusted Python worker: @@ -288,6 +308,24 @@ shimmy -c python3 -a evaluation.py \ > **Note:** nsjail requires either root or user namespace support. In Docker, pass `--privileged` or grant `CAP_SYS_ADMIN`. In Kubernetes, configure the pod's security context accordingly. +#### Constrained hosts (rootless Podman, locked-down Fargate) + +The mount namespace (`CLONE_NEWNS`) is always created — it is what makes the +bind-mount filesystem confinement work — but the other namespaces can be turned off +for hosts that reject nesting them: + +- If the worker fails to start a thread (`pthread_create ... Invalid argument`) or + nsjail reports a namespace-setup error, disable the PID / IPC / UTS / cgroup + namespaces with `--sandbox-disable-clone-newpid` (and the `-newipc` / `-newuts` / + `-newcgroup` variants). +- `--sandbox-clone-newuser` controls the user namespace. `auto` (default) drops it + when shimmy runs as uid 0, which is correct for most container deployments (AWS + ECS/Fargate, Kubernetes) where nested `CLONE_NEWUSER` is blocked. Under **rootless + Podman** the invoking user is mapped to uid 0 inside a user namespace and + `CLONE_NEWUSER` *is* needed — set `--sandbox-clone-newuser=enabled` there. Use + `disabled` to always skip it. +- Alternatively, run the container rootful, or set up `newuidmap`/`newgidmap`. + #### Testing sandboxing locally The sandbox integration tests verify actual security properties — filesystem isolation, CPU limits, network isolation, and stdio passthrough. They skip automatically if `nsjail` is not available. @@ -295,7 +333,7 @@ The sandbox integration tests verify actual security properties — filesystem i **On Linux with nsjail installed:** ```shell -go test -v -run 'TestSandboxedWorker' ./internal/execution/worker/... +go test -v -run 'TestSandboxedWorker|TestApplySandbox' ./internal/execution/worker/... ``` **On macOS (or any platform) via Docker or Podman:** @@ -314,7 +352,7 @@ docker run --rm --privileged \ -e FUNCTION_COMMAND=/bin/sh \ -e FUNCTION_ARGS="-c,cat /etc/shadow" \ -e SANDBOX_ENABLED=true \ - -e SANDBOX_RO_BINDS="/usr:/bin:/lib:/lib64" \ + -e SANDBOX_RO_BINDS="/usr,/bin,/lib,/lib64" \ ghcr.io/lambda-feedback/shimmy serve ``` diff --git a/cmd/root.go b/cmd/root.go index eb6019b..7086d4e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -207,12 +207,59 @@ functions on arbitrary, serverless platforms.` Category: "sandbox", EnvVars: []string{"SANDBOX_DISABLE_NETWORK"}, }, + &cli.StringFlag{ + Name: "sandbox-seccomp-policy-file", + Usage: "path to a kafel seccomp policy file to apply inside the sandbox.", + Category: "sandbox", + EnvVars: []string{"SANDBOX_SECCOMP_POLICY_FILE"}, + }, + &cli.StringFlag{ + Name: "sandbox-seccomp-string", + Usage: "inline kafel seccomp policy to apply inside the sandbox (mutually exclusive with --sandbox-seccomp-policy-file).", + Category: "sandbox", + EnvVars: []string{"SANDBOX_SECCOMP_STRING"}, + }, + &cli.BoolFlag{ + Name: "sandbox-disable-clone-newpid", + Usage: "keep the worker in the host PID namespace (for hosts that reject a nested PID namespace).", + Value: false, + Category: "sandbox", + EnvVars: []string{"SANDBOX_DISABLE_CLONE_NEWPID"}, + }, + &cli.BoolFlag{ + Name: "sandbox-disable-clone-newipc", + Usage: "keep the worker in the host IPC namespace.", + Value: false, + Category: "sandbox", + EnvVars: []string{"SANDBOX_DISABLE_CLONE_NEWIPC"}, + }, + &cli.BoolFlag{ + Name: "sandbox-disable-clone-newuts", + Usage: "keep the worker in the host UTS namespace.", + Value: false, + Category: "sandbox", + EnvVars: []string{"SANDBOX_DISABLE_CLONE_NEWUTS"}, + }, + &cli.BoolFlag{ + Name: "sandbox-disable-clone-newcgroup", + Usage: "keep the worker in the host cgroup namespace.", + Value: false, + Category: "sandbox", + EnvVars: []string{"SANDBOX_DISABLE_CLONE_NEWCGROUP"}, + }, + &cli.StringFlag{ + Name: "sandbox-clone-newuser", + Usage: "user namespace handling: auto (drop when running as uid 0), enabled (always keep), disabled (always drop).", + Value: "auto", + Category: "sandbox", + EnvVars: []string{"SANDBOX_CLONE_NEWUSER"}, + }, &cli.BoolFlag{ - Name: "sandbox-seccomp", - Usage: "enable seccomp syscall filtering inside the sandbox.", + Name: "sandbox-verbose", + Usage: "let nsjail log to stderr at its default verbosity (default: quiet, warnings and errors only).", Value: false, Category: "sandbox", - EnvVars: []string{"SANDBOX_SECCOMP"}, + EnvVars: []string{"SANDBOX_VERBOSE"}, }, }, Before: func(ctx *cli.Context) error { @@ -340,16 +387,23 @@ func parseRootConfig(ctx *cli.Context) (config.Config, error) { "worker-stop-timeout": "runtime.stop.timeout", "worker-start-timeout": "start_timeout", // sandbox - "sandbox": "runtime.sandbox.enabled", - "sandbox-nsjail-path": "runtime.sandbox.nsjail_path", - "sandbox-ro-bind": "runtime.sandbox.ro_binds", - "sandbox-rw-bind": "runtime.sandbox.rw_binds", - "sandbox-tmpfs": "runtime.sandbox.tmpfs", - "sandbox-cpu-time": "runtime.sandbox.cpu_time_limit", - "sandbox-memory-mb": "runtime.sandbox.memory_limit", - "sandbox-max-fds": "runtime.sandbox.max_fds", - "sandbox-disable-network": "runtime.sandbox.disable_network", - "sandbox-seccomp": "runtime.sandbox.seccomp", + "sandbox": "runtime.sandbox.enabled", + "sandbox-nsjail-path": "runtime.sandbox.nsjail_path", + "sandbox-ro-bind": "runtime.sandbox.ro_binds", + "sandbox-rw-bind": "runtime.sandbox.rw_binds", + "sandbox-tmpfs": "runtime.sandbox.tmpfs", + "sandbox-cpu-time": "runtime.sandbox.cpu_time_limit", + "sandbox-memory-mb": "runtime.sandbox.memory_limit", + "sandbox-max-fds": "runtime.sandbox.max_fds", + "sandbox-disable-network": "runtime.sandbox.disable_network", + "sandbox-seccomp-policy-file": "runtime.sandbox.seccomp_policy_file", + "sandbox-seccomp-string": "runtime.sandbox.seccomp_string", + "sandbox-disable-clone-newpid": "runtime.sandbox.disable_clone_newpid", + "sandbox-disable-clone-newipc": "runtime.sandbox.disable_clone_newipc", + "sandbox-disable-clone-newuts": "runtime.sandbox.disable_clone_newuts", + "sandbox-disable-clone-newcgroup": "runtime.sandbox.disable_clone_newcgroup", + "sandbox-clone-newuser": "runtime.sandbox.clone_newuser", + "sandbox-verbose": "runtime.sandbox.verbose", } // parse config using env diff --git a/internal/execution/worker/sandbox.go b/internal/execution/worker/sandbox.go index 83b4143..0f3a955 100644 --- a/internal/execution/worker/sandbox.go +++ b/internal/execution/worker/sandbox.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "os" + "os/exec" "strconv" "go.uber.org/zap" @@ -39,6 +40,24 @@ func applySandbox(config StartConfig, cfg SandboxConfig) (StartConfig, error) { return StartConfig{}, errors.New("cannot sandbox empty command") } + if cfg.SeccompPolicyFile != "" && cfg.SeccompString != "" { + return StartConfig{}, errors.New("seccomp_policy_file and seccomp_string are mutually exclusive") + } + if cfg.SeccompPolicyFile != "" { + if _, err := os.Stat(cfg.SeccompPolicyFile); err != nil { + return StartConfig{}, fmt.Errorf("seccomp policy file %q: %w", cfg.SeccompPolicyFile, err) + } + } + + // nsjail --mode e is execve, not execvp: it does not search PATH. Resolve + // the command to an absolute path here so a bare "python3" still works. + // Absolute and ./relative paths pass through exec.LookPath unchanged. + resolved, err := exec.LookPath(config.Cmd) + if err != nil { + return StartConfig{}, fmt.Errorf("cannot resolve sandboxed command %q: %w", config.Cmd, err) + } + config.Cmd = resolved + return StartConfig{ Cmd: cfg.NsjailPath, // CWD is managed by --cwd inside nsjail; exec.Cmd CWD is irrelevant. @@ -56,8 +75,17 @@ func buildNsjailArgs(config StartConfig, cfg SandboxConfig) []string { // Use 'e' (execve), not 'o' (once/TCP) — we rely on stdio, not a network socket. args = append(args, "--mode", "e") - // Suppress nsjail's own log output so it doesn't pollute worker stderr. - args = append(args, "--log", "/dev/null") + // Forward the parent environment to the jailed child. nsjail clears the + // child env by default; without this the worker loses PATH, AWS_* creds, + // etc. — behaving differently from a non-sandboxed worker. The nsjail + // process itself receives os.Environ()+config.Env from createCmd. + args = append(args, "--keep_env") + + // Logging: by default run quiet so per-run INFO spam stays out of worker + // stderr, while fatal cmdline-parse / namespace-setup errors still surface. + if !cfg.Verbose { + args = append(args, "--quiet") + } // Drop privileges: run worker as nobody unless overridden. user := cfg.User @@ -70,8 +98,16 @@ func buildNsjailArgs(config StartConfig, cfg SandboxConfig) []string { args = append(args, "--chroot", "/") // Preserve the worker's intended working directory inside the sandbox. - if config.Cwd != "" { - args = append(args, "--cwd", config.Cwd) + // Fall back to shimmy's own cwd so a sandboxed worker starts where a + // non-sandboxed one would, instead of nsjail's default "/". + cwd := config.Cwd + if cwd == "" { + if wd, err := os.Getwd(); err == nil { + cwd = wd + } + } + if cwd != "" { + args = append(args, "--cwd", cwd) } // Filesystem: read-only bind mounts. @@ -97,11 +133,36 @@ func buildNsjailArgs(config StartConfig, cfg SandboxConfig) []string { args = append(args, "--disable_clone_newnet") } - // When running as root (e.g. inside a privileged container), skip user - // namespace creation — nested CLONE_NEWUSER is typically blocked by the - // container runtime. setuid/setgid via --user still drops privileges. - if os.Getuid() == 0 { + // Namespaces that constrained hosts (rootless Podman, locked-down Fargate) + // may reject. CLONE_NEWNS is kept unconditionally — it is what makes the + // bind-mount filesystem confinement work. + if cfg.DisableCloneNewpid { + args = append(args, "--disable_clone_newpid") + } + if cfg.DisableCloneNewipc { + args = append(args, "--disable_clone_newipc") + } + if cfg.DisableCloneNewuts { + args = append(args, "--disable_clone_newuts") + } + if cfg.DisableCloneNewcgroup { + args = append(args, "--disable_clone_newcgroup") + } + + // User namespace. "auto" (default) drops it when shimmy runs as root + // (e.g. inside a privileged container) where nested CLONE_NEWUSER is + // typically blocked; setuid/setgid via --user still drops privileges. + // Rootless Podman maps the invoking user to uid 0 inside a userns and + // needs CLONE_NEWUSER — set "enabled" there. + switch cfg.CloneNewuser { + case "disabled": args = append(args, "--disable_clone_newuser") + case "enabled": + // force-keep the user namespace; append nothing + default: // "" / "auto" + if os.Getuid() == 0 { + args = append(args, "--disable_clone_newuser") + } } // Resource limits. @@ -119,9 +180,13 @@ func buildNsjailArgs(config StartConfig, cfg SandboxConfig) []string { args = append(args, "--rlimit_nofile", strconv.Itoa(cfg.MaxFds)) } - // Seccomp: use nsjail's built-in default syscall policy. - if cfg.Seccomp { - args = append(args, "--seccomp_default_policy=1") + // Seccomp: an explicit kafel policy, either inline or from a file. nsjail + // has no "default policy" flag; without a policy the child still runs with + // NO_NEW_PRIVS, which nsjail sets by default. + if cfg.SeccompString != "" { + args = append(args, "--seccomp_string", cfg.SeccompString) + } else if cfg.SeccompPolicyFile != "" { + args = append(args, "--seccomp_policy", cfg.SeccompPolicyFile) } // Separator: everything after "--" is the command to execute. diff --git a/internal/execution/worker/sandbox_config.go b/internal/execution/worker/sandbox_config.go index 4f2f1c8..60a7085 100644 --- a/internal/execution/worker/sandbox_config.go +++ b/internal/execution/worker/sandbox_config.go @@ -36,7 +36,37 @@ type SandboxConfig struct { // DisableNetwork removes network access inside the sandbox. DisableNetwork bool `conf:"disable_network"` - // Seccomp enables syscall filtering via seccomp-bpf using nsjail's - // built-in default policy. Requires kernel seccomp support. - Seccomp bool `conf:"seccomp"` + // DisableCloneNewpid keeps the worker in the host PID namespace. Some + // constrained hosts (rootless Podman, locked-down Fargate) reject a nested + // PID namespace, which surfaces as "pthread_create ... Invalid argument". + DisableCloneNewpid bool `conf:"disable_clone_newpid"` + + // DisableCloneNewipc keeps the worker in the host IPC namespace. + DisableCloneNewipc bool `conf:"disable_clone_newipc"` + + // DisableCloneNewuts keeps the worker in the host UTS namespace. + DisableCloneNewuts bool `conf:"disable_clone_newuts"` + + // DisableCloneNewcgroup keeps the worker in the host cgroup namespace. + DisableCloneNewcgroup bool `conf:"disable_clone_newcgroup"` + + // CloneNewuser controls the user namespace: "" / "auto" drops it only when + // shimmy runs as uid 0 (the usual container case, where nested CLONE_NEWUSER + // is blocked); "enabled" always keeps it (rootless Podman needs it); + // "disabled" always drops it. + CloneNewuser string `conf:"clone_newuser"` + + // SeccompPolicyFile is the path to a kafel seccomp policy file, passed to + // nsjail as --seccomp_policy. Empty = no seccomp filtering (nsjail still + // sets NO_NEW_PRIVS by default). Mutually exclusive with SeccompString. + SeccompPolicyFile string `conf:"seccomp_policy_file"` + + // SeccompString is an inline kafel seccomp policy, passed to nsjail as + // --seccomp_string. Mutually exclusive with SeccompPolicyFile. + SeccompString string `conf:"seccomp_string"` + + // Verbose lets nsjail log to stderr at its default verbosity. When false + // (the default) nsjail runs with --quiet: only warnings, errors and fatal + // cmdline/namespace failures reach stderr. + Verbose bool `conf:"verbose"` } diff --git a/internal/execution/worker/sandbox_test.go b/internal/execution/worker/sandbox_test.go index 495b213..02363e7 100644 --- a/internal/execution/worker/sandbox_test.go +++ b/internal/execution/worker/sandbox_test.go @@ -8,6 +8,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "testing" "time" @@ -147,6 +148,158 @@ func TestApplySandbox_EnvPreserved(t *testing.T) { assert.Equal(t, env, out.Env) } +func TestApplySandbox_KeepEnv(t *testing.T) { + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{}, + ) + require.NoError(t, err) + assert.Contains(t, out.Args, "--keep_env") +} + +func TestApplySandbox_ResolvesBareCommand(t *testing.T) { + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "sh"}, + worker.SandboxConfig{}, + ) + require.NoError(t, err) + + sepIdx := indexOf(out.Args, "--") + require.NotEqual(t, -1, sepIdx) + assert.True(t, filepath.IsAbs(out.Args[sepIdx+1]), + "bare command must be resolved to an absolute path, got %q", out.Args[sepIdx+1]) +} + +func TestApplySandbox_UnresolvableCommand_ReturnsError(t *testing.T) { + _, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "definitely-not-a-real-binary-xyz"}, + worker.SandboxConfig{}, + ) + assert.Error(t, err) +} + +func TestApplySandbox_CwdDefaultsToWorkingDir(t *testing.T) { + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{}, + ) + require.NoError(t, err) + + cwdIdx := indexOf(out.Args, "--cwd") + require.NotEqual(t, -1, cwdIdx, "--cwd must default to shimmy's working directory") + wd, _ := os.Getwd() + assert.Equal(t, wd, out.Args[cwdIdx+1]) +} + +func TestApplySandbox_Quiet(t *testing.T) { + quiet, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{}, + ) + require.NoError(t, err) + assert.Contains(t, quiet.Args, "--quiet") + + verbose, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{Verbose: true}, + ) + require.NoError(t, err) + assert.NotContains(t, verbose.Args, "--quiet") +} + +func TestApplySandbox_NamespaceToggles(t *testing.T) { + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{ + DisableCloneNewpid: true, + DisableCloneNewipc: true, + DisableCloneNewuts: true, + DisableCloneNewcgroup: true, + }, + ) + require.NoError(t, err) + assert.Contains(t, out.Args, "--disable_clone_newpid") + assert.Contains(t, out.Args, "--disable_clone_newipc") + assert.Contains(t, out.Args, "--disable_clone_newuts") + assert.Contains(t, out.Args, "--disable_clone_newcgroup") + + def, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{}, + ) + require.NoError(t, err) + assert.NotContains(t, def.Args, "--disable_clone_newpid") + assert.NotContains(t, def.Args, "--disable_clone_newipc") + assert.NotContains(t, def.Args, "--disable_clone_newuts") + assert.NotContains(t, def.Args, "--disable_clone_newcgroup") +} + +func TestApplySandbox_CloneNewuser(t *testing.T) { + disabled, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{CloneNewuser: "disabled"}, + ) + require.NoError(t, err) + assert.Contains(t, disabled.Args, "--disable_clone_newuser") + + enabled, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{CloneNewuser: "enabled"}, + ) + require.NoError(t, err) + assert.NotContains(t, enabled.Args, "--disable_clone_newuser") +} + +func TestApplySandbox_SeccompString(t *testing.T) { + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{SeccompString: "DEFAULT ALLOW\n"}, + ) + require.NoError(t, err) + assert.True(t, containsPair(out.Args, "--seccomp_string", "DEFAULT ALLOW\n")) + assert.NotContains(t, out.Args, "--seccomp_policy") +} + +func TestApplySandbox_SeccompPolicyFile(t *testing.T) { + dir := t.TempDir() + policy := filepath.Join(dir, "policy.kafel") + require.NoError(t, os.WriteFile(policy, []byte("DEFAULT ALLOW\n"), 0644)) + + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{SeccompPolicyFile: policy}, + ) + require.NoError(t, err) + assert.True(t, containsPair(out.Args, "--seccomp_policy", policy)) + assert.NotContains(t, out.Args, "--seccomp_string") +} + +func TestApplySandbox_SeccompMissingPolicyFile_ReturnsError(t *testing.T) { + _, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{SeccompPolicyFile: "/no/such/policy.kafel"}, + ) + assert.Error(t, err) +} + +func TestApplySandbox_SeccompMutuallyExclusive_ReturnsError(t *testing.T) { + _, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{SeccompString: "DEFAULT ALLOW\n", SeccompPolicyFile: "/tmp/x"}, + ) + assert.Error(t, err) +} + +func TestApplySandbox_NoSeccompByDefault(t *testing.T) { + out, err := worker.ApplySandboxForTest( + worker.StartConfig{Cmd: "/bin/sh"}, + worker.SandboxConfig{}, + ) + require.NoError(t, err) + assert.NotContains(t, out.Args, "--seccomp_string") + assert.NotContains(t, out.Args, "--seccomp_policy") +} + func TestNewSandboxedWorkerFactory_MissingBinary(t *testing.T) { _, err := worker.NewSandboxedWorkerFactory(worker.SandboxConfig{ NsjailPath: "/nonexistent/nsjail", @@ -332,6 +485,122 @@ func TestSandboxedWorker_StdioPassthrough(t *testing.T) { assert.Equal(t, msg, out.String()) } +// TestSandboxedWorker_SeccompString verifies that a kafel seccomp policy is +// accepted by nsjail — a regression guard for the bogus --seccomp_default_policy=1 +// flag that made nsjail fail to parse its cmdline and break every request. +func TestSandboxedWorker_SeccompString(t *testing.T) { + requireNsjail(t) + + factory, err := worker.NewSandboxedWorkerFactory(worker.SandboxConfig{ + NsjailPath: "/usr/sbin/nsjail", + ReadOnlyBinds: []string{"/usr", "/bin", "/lib", "/lib64"}, + SeccompString: "DEFAULT ALLOW\n", + }) + require.NoError(t, err) + + w, err := factory(context.Background(), worker.StartConfig{Cmd: "/bin/true"}, zap.NewNop()) + require.NoError(t, err) + require.NoError(t, w.Start(context.Background())) + + exit, err := w.Wait(context.Background()) + require.NoError(t, err) + assert.True(t, exit.Success(), "nsjail should accept the kafel policy: %s", exit.String()) +} + +// TestSandboxedWorker_InheritsEnvironment verifies that the parent environment +// reaches the jailed child (via --keep_env), so PATH / AWS_* creds are not lost. +func TestSandboxedWorker_InheritsEnvironment(t *testing.T) { + requireNsjail(t) + + t.Setenv("SHIMMY_TEST_VAR", "sandbox-env-ok") + + factory, err := worker.NewSandboxedWorkerFactory(worker.SandboxConfig{ + NsjailPath: "/usr/sbin/nsjail", + ReadOnlyBinds: []string{"/usr", "/bin", "/lib", "/lib64"}, + }) + require.NoError(t, err) + + w, err := factory(context.Background(), worker.StartConfig{ + Cmd: "/bin/sh", + Args: []string{"-c", `printf %s "$SHIMMY_TEST_VAR"`}, + }, zap.NewNop()) + require.NoError(t, err) + + stdout, err := w.ReadPipe() + require.NoError(t, err) + + require.NoError(t, w.Start(context.Background())) + + var out bytes.Buffer + io.Copy(&out, stdout) //nolint:errcheck + + exit, err := w.Wait(context.Background()) + require.NoError(t, err) + assert.True(t, exit.Success(), "expected exit 0: %s", exit.String()) + assert.Equal(t, "sandbox-env-ok", out.String()) +} + +// TestSandboxedWorker_ResolvesBareCommand verifies that a PATH-relative command +// (not an absolute path) is resolved before nsjail's execve. +func TestSandboxedWorker_ResolvesBareCommand(t *testing.T) { + requireNsjail(t) + + factory, err := worker.NewSandboxedWorkerFactory(worker.SandboxConfig{ + NsjailPath: "/usr/sbin/nsjail", + ReadOnlyBinds: []string{"/usr", "/bin", "/lib", "/lib64"}, + }) + require.NoError(t, err) + + w, err := factory(context.Background(), worker.StartConfig{Cmd: "true"}, zap.NewNop()) + require.NoError(t, err) + require.NoError(t, w.Start(context.Background())) + + exit, err := w.Wait(context.Background()) + require.NoError(t, err) + assert.True(t, exit.Success(), "bare 'true' should resolve and exit 0: %s", exit.String()) +} + +// TestSandboxedWorker_DefaultCwd verifies that, with no explicit working +// directory, the worker starts in shimmy's own cwd rather than nsjail's "/". +func TestSandboxedWorker_DefaultCwd(t *testing.T) { + requireNsjail(t) + + // A world-traversable directory under /tmp so uid 65534 can enter it. + dir, err := os.MkdirTemp("", "sandbox-cwd-*") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(dir) }) + require.NoError(t, os.Chmod(dir, 0755)) + dir, err = filepath.EvalSymlinks(dir) + require.NoError(t, err) + + t.Chdir(dir) + + factory, err := worker.NewSandboxedWorkerFactory(worker.SandboxConfig{ + NsjailPath: "/usr/sbin/nsjail", + ReadOnlyBinds: []string{"/usr", "/bin", "/lib", "/lib64", dir}, + }) + require.NoError(t, err) + + w, err := factory(context.Background(), worker.StartConfig{ + Cmd: "/bin/pwd", + Args: []string{"-P"}, + }, zap.NewNop()) + require.NoError(t, err) + + stdout, err := w.ReadPipe() + require.NoError(t, err) + + require.NoError(t, w.Start(context.Background())) + + var out bytes.Buffer + io.Copy(&out, stdout) //nolint:errcheck + + exit, err := w.Wait(context.Background()) + require.NoError(t, err) + assert.True(t, exit.Success(), "expected exit 0: %s", exit.String()) + assert.Equal(t, dir+"\n", out.String()) +} + // helpers func indexOf(slice []string, val string) int { diff --git a/internal/execution/worker/worker_unix.go b/internal/execution/worker/worker_unix.go index a585c3b..5c4eb27 100644 --- a/internal/execution/worker/worker_unix.go +++ b/internal/execution/worker/worker_unix.go @@ -21,7 +21,12 @@ func (p *ProcessWorker) killProcess(force bool) error { } func initCmd(cmd *exec.Cmd) { + // Start the worker in its own session (which also gives it its own process + // group, so killProcess can still signal the whole group via -pgid). A new + // session — rather than just Setpgid — is required for nsjail's own + // setsid() in --mode e to succeed: setsid() returns EPERM when the caller + // is already a process-group leader, which Setpgid alone would make it. cmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, + Setsid: true, } }