diff options
| -rw-r--r-- | Magefile.go | 249 | ||||
| -rw-r--r-- | docs/sudo-hardening-plan.md | 108 | ||||
| -rw-r--r-- | docs/sudo-rules-for-ior.txt | 88 |
3 files changed, 240 insertions, 205 deletions
diff --git a/Magefile.go b/Magefile.go index a708fb7..25b1a97 100644 --- a/Magefile.go +++ b/Magefile.go @@ -5,7 +5,6 @@ package main import ( "bufio" - "encoding/json" "errors" "fmt" "go/format" @@ -203,13 +202,15 @@ func TestWithName() error { fmt.Println("Running integration test", testName, "(requires root)...") env := goEnv() forwardEnv(env, "HOME", "GOPATH", "GOMODCACHE", "PATH", "GOTOOLCHAIN") - return runGoTestWithProgress(env, - "./integrationtests/...", - "-run", "^"+testName+"$", - "-failfast", - "-timeout=30m", - "-count=1", - "-json", + if err := compileIntegrationTestBinary(env); err != nil { + return err + } + return runIntegrationTestBinary(env, + "-test.run", "^"+testName+"$", + "-test.failfast", + "-test.timeout=30m", + "-test.count=1", + "-test.v", ) } return sh.RunWithV(goEnv(), "go", "test", "./...", "-run", "^"+testName+"$", "-v", "-failfast") @@ -519,6 +520,29 @@ func IntegrationTestSerial() error { return runIntegrationTests(false) } +func compileIntegrationTestBinary(env map[string]string) error { + return sh.RunWithV(env, "go", "test", "-c", "./integrationtests/...", "-o", "integrationtests.test") +} + +func runIntegrationTestBinary(env map[string]string, args ...string) error { + envList := make([]string, 0, len(env)) + for k, v := range env { + envList = append(envList, k+"="+v) + } + slices.Sort(envList) + + cmd := exec.Command("./integrationtests.test", args...) + cmd.Dir = "integrationtests" + cmd.Env = append(os.Environ(), envList...) + if os.Geteuid() == 0 { + return cmd.Run() + } + sudoCmd := exec.Command("sudo", append([]string{"-n", "-E", "./integrationtests.test"}, args...)...) + sudoCmd.Dir = "integrationtests" + sudoCmd.Env = cmd.Env + return sudoCmd.Run() +} + func runIntegrationTests(parallel bool) error { mg.SerialDeps(All) if err := buildWorkloadBinary(); err != nil { @@ -528,16 +552,19 @@ func runIntegrationTests(parallel bool) error { env := goEnv() forwardEnv(env, "HOME", "GOPATH", "GOMODCACHE", "GOTOOLCHAIN") + if err := compileIntegrationTestBinary(env); err != nil { + return err + } + timeout := "30m" if !parallel { timeout = "90m" } args := []string{ - "./integrationtests/...", - "-failfast", - "-timeout=" + timeout, - "-count=1", + "-test.failfast", + "-test.timeout=" + timeout, + "-test.count=1", } if parallel { @@ -547,13 +574,12 @@ func runIntegrationTests(parallel bool) error { } env[integrationParallelE] = "1" fmt.Printf("Running integration tests in parallel (requires root, parallel=%d)...\n", parallelism) - args = append(args, "-parallel", strconv.Itoa(parallelism)) + args = append(args, "-test.parallel", strconv.Itoa(parallelism)) } else { fmt.Println("Running integration tests serially (requires root)...") } - args = append(args, "-json") - return runGoTestWithProgress(env, args...) + return runIntegrationTestBinary(env, args...) } func resolveIntegrationParallelism() (int, error) { @@ -810,7 +836,7 @@ func sudoOutput(cmd string, args ...string) (string, error) { if os.Geteuid() == 0 { return sh.Output(cmd, args...) } - return sh.Output("sudo", append([]string{cmd}, args...)...) + return sh.Output("sudo", append([]string{"-n", cmd}, args...)...) } func sudoRunWithEnv(env map[string]string, cmd string, args ...string) error { @@ -822,8 +848,8 @@ func sudoRunWithEnv(env map[string]string, cmd string, args ...string) error { keys = append(keys, k) } slices.Sort(keys) - sudoArgs := make([]string, 0, 1+len(keys)+1+len(args)) - sudoArgs = append(sudoArgs, "env") + sudoArgs := make([]string, 0, 2+len(keys)+1+len(args)) + sudoArgs = append(sudoArgs, "-n", "env") for _, k := range keys { sudoArgs = append(sudoArgs, k+"="+env[k]) } @@ -897,193 +923,6 @@ func sortLinesWithLocale(lines []string) (string, error) { return string(output), nil } -type goTestEvent struct { - Action string `json:"Action"` - Package string `json:"Package"` - Test string `json:"Test"` - Output string `json:"Output"` -} - -// buildGoTestCmd constructs the exec.Cmd for running `go test -json` with the -// given env vars. When not root it wraps the command with `sudo env KEY=VAL …` -// so elevated integration tests inherit the correct CGO/LIBBPFGO environment. -func buildGoTestCmd(env map[string]string, cmdArgs []string) *exec.Cmd { - if os.Geteuid() == 0 { - cmd := exec.Command("go", cmdArgs...) - cmd.Env = append(os.Environ(), envToList(env)...) - return cmd - } - keys := make([]string, 0, len(env)) - for k := range env { - keys = append(keys, k) - } - slices.Sort(keys) - sudoArgs := make([]string, 0, 1+len(keys)+1+len(cmdArgs)) - sudoArgs = append(sudoArgs, "env") - for _, k := range keys { - sudoArgs = append(sudoArgs, k+"="+env[k]) - } - sudoArgs = append(sudoArgs, "go") - sudoArgs = append(sudoArgs, cmdArgs...) - return exec.Command("sudo", sudoArgs...) -} - -// startProgressTicker prints the set of currently-running tests every 15 s. -// Call close(done) to stop the ticker goroutine. -func startProgressTicker(running map[string]time.Time, done <-chan struct{}) { - ticker := time.NewTicker(15 * time.Second) - go func() { - defer ticker.Stop() - for { - select { - case <-done: - return - case <-ticker.C: - if len(running) == 0 { - fmt.Println("Integration tests still running... waiting for next test event") - continue - } - names := make([]string, 0, len(running)) - for k := range running { - names = append(names, k) - } - slices.Sort(names) - fmt.Println("Integration tests running:", strings.Join(names, ", ")) - } - } - }() -} - -// drainTestEvents reads JSON test events from scanner, updates the running map, -// and prints human-readable RUN/PASS/FAIL/SKIP/LOG lines. -func drainTestEvents(scanner *bufio.Scanner, running map[string]time.Time) { - for scanner.Scan() { - line := scanner.Bytes() - var ev goTestEvent - if err := json.Unmarshal(line, &ev); err != nil { - fmt.Println(string(line)) - continue - } - if ev.Test == "" { - continue - } - key := ev.Package + "/" + ev.Test - switch ev.Action { - case "run": - running[key] = time.Now() - fmt.Println("RUN ", key) - case "pass": - delete(running, key) - fmt.Println("PASS", key) - case "fail": - delete(running, key) - fmt.Println("FAIL", key) - case "skip": - delete(running, key) - fmt.Println("SKIP", key) - case "output": - msg := strings.TrimSpace(ev.Output) - if msg != "" && shouldPrintTestLog(msg) { - fmt.Println("LOG ", key, "-", msg) - } - } - } -} - -// runGoTestWithProgress runs `go test -json` (via sudo when not root), streams -// progress to stdout every 15 s, and returns a non-nil error on test failure. -func runGoTestWithProgress(env map[string]string, args ...string) error { - cmd := buildGoTestCmd(env, append([]string{"test"}, args...)) - - stdout, err := cmd.StdoutPipe() - if err != nil { - return err - } - stderr, err := cmd.StderrPipe() - if err != nil { - return err - } - if err := cmd.Start(); err != nil { - return err - } - - // Forward stderr from the test binary so build errors are always visible. - go func() { _, _ = io.Copy(os.Stderr, stderr) }() - - scanner := bufio.NewScanner(stdout) - scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) - - running := map[string]time.Time{} - done := make(chan struct{}) - startProgressTicker(running, done) - - drainTestEvents(scanner, running) - close(done) - - if err := scanner.Err(); err != nil { - return err - } - return cmd.Wait() -} - -func envToList(env map[string]string) []string { - if len(env) == 0 { - return nil - } - out := make([]string, 0, len(env)) - for k, v := range env { - out = append(out, k+"="+v) - } - slices.Sort(out) - return out -} - -func shouldPrintTestLog(msg string) bool { - // Always keep error/failure lines. - if strings.Contains(msg, "--- FAIL:") || - strings.Contains(msg, " FAIL ") || - strings.Contains(msg, "panic:") || - strings.Contains(strings.ToLower(msg), "error") || - strings.Contains(strings.ToLower(msg), "expected event not found") { - return true - } - - // Drop high-volume attach/debug noise from ior startup in integration tests. - noisePrefixes := []string{ - "=== RUN", - "___", - "|_ _|", - "| |", - "|___", - "v0.0.0", - "libbpf:", - "Attaching tracepoint ", - "Attached prog handle_ ", - "Attached tracepoint", - "Attaching sys_", - "Not attaching sys_", - "Collecting flame graph stats", - "Starting flamegraph worker", - "Waiting for stats to be ready", - "Stopping event loop", - "Waiting for flamegraph", - "Worker ", - "Writing ", - "Good bye...", - "Statistics:", - "duration:", - "tracepoints:", - "syscalls:", - "syscalls after filter:", - } - for _, p := range noisePrefixes { - if strings.HasPrefix(msg, p) { - return false - } - } - return true -} - func isIntegrationTest(testName string) (bool, error) { out, err := sh.OutputWith(goEnv(), "go", "test", "./integrationtests/...", "-list", ".") if err != nil { diff --git a/docs/sudo-hardening-plan.md b/docs/sudo-hardening-plan.md new file mode 100644 index 0000000..9610d01 --- /dev/null +++ b/docs/sudo-hardening-plan.md @@ -0,0 +1,108 @@ +# Sudo Hardening Plan for I/O Riot NG (ior) + +## Problem Statement + +The current build system (`Magefile.go`) silently wraps entire `go test` invocations—and several generation subcommands—with `sudo` when the invoking user is not root. This means: + +1. **You must trust `mage`** (an arbitrary-code build tool) with root privileges. +2. **An attacker who can modify `Magefile.go`** (or any helper it imports) gets immediate privilege escalation. +3. **It is impossible to audit what actually ran as root**, because the `sudo` command is assembled dynamically inside Go code. + +## Goal + +Run `mage` **entirely as an unprivileged user**. Only the **minimal set of specific sub-processes** that genuinely require elevated privileges (`CAP_BPF` / root) should ever run under `sudo`, and they should be invoked with `sudo -n` so no interactive password prompt can be exploited by a malicious build script. + +Granular `/etc/sudoers.d/` rules can then grant **password-less, command-locked** access to exactly those subprocesses. + +## Threat Model + +| Asset | Risk | Mitigation | +|---|---|---| +| `Magefile.go` | Malicious code injection → arbitrary root | `mage` never runs as root; `sudo` is only used for bounded, hard-coded subcommands | +| `go test` | `init()`, `_testmain.go`, or imported packages run arbitrary code at startup | Integration tests are compiled to a static binary first; only the final binary runs under `sudo` | +| `/sys/kernel/tracing` | Leak of kernel tracepoint data | Read-only access to public tracepoint format files (already world-readable on many systems, but the path traversal needs root on locked-down kernels) | +| `bpftool` | Arbitrary BPF object loading | Restricted to the single read-only invocation: `bpftool btf dump file /sys/kernel/btf/vmlinux format c` | +| `ior` binary | Full `CAP_BPF` access | Acceptable because it is the product being tested; access is via a compiled test binary with a deterministic path | + +## What Needs Root (and Why) + +| Operation | Needs root? | Reason | +|---|---|---| +| Compiling Go / BPF object | **No** | Pure toolchain work | +| `mage test` (unit tests) | **No** | Mocks / stubs; no real BPF attachment | +| `mage generate` / `mage world` | **Partially** | `bpftool btf dump ...` and reading `/sys/kernel/tracing/events/syscalls/*/format` | +| `mage integrationTest` | **Yes** | Spawns `./ior`, which attaches BPF tracepoints | +| `mage integrationTestSerial` | **Yes** | Same | +| `mage demo` / `mage demoOne` | **Yes** | Tapes launch `./ior` (BPF) and `pkill` | +| `mage installDemoTools` | **Yes** | `dnf install` system-wide package | + +## Design Principles + +1. **Compile as user, run binary as root.** + - Build the integration-test binary (`go test -c`) while fully unprivileged. + - Only the resulting `integrationtests.test` binary runs under `sudo`. +2. **Explicit `sudo -n` for every elevated command.** + - No silent wrapping. + - If `sudo -n` fails, the build fails with a clear error telling the admin which sudo rule is missing. +3. **Hard-code elevated commands; no dynamic construction.** + - The exact command line passed to `sudo` must be easily auditable in `Magefile.go`. +4. **Sudoers rules are command-granular.** + - Each rule locks the user to a single command with exact arguments (or wildcards only where strictly necessary). + +## Changes to `Magefile.go` + +### 1. Remove automatic `sudo` wrapping from `buildGoTestCmd` + +Current behavior: when `os.Geteuid() != 0`, the function manufactures a `sudo env … go test …` command. **Delete this logic.** Going forward `buildGoTestCmd` returns a plain `go` command with no privilege elevation. + +### 2. Add compiled-binary helper for integration tests + +Two new helpers are introduced: + +- `compileIntegrationTestBinary(env)` – runs `go test -c ./integrationtests/...` with the required `CGO_*` environment. +- `runIntegrationTestBinary(env, args…)` – elevates **only** the compiled binary via `sudo -n -E ./integrationtests.test …`, running it from the `integrationtests/` directory so that relative path resolution (`../ior`, `../ioworkload`) matches `go test` behaviour. + +`IntegrationTest`, `IntegrationTestSerial`, and `testWithName` (when the target is an integration test) are updated to: + +1. build `ior`, `ioworkload`, and compile the test binary as an unprivileged user; +2. invoke `runIntegrationTestBinary` to execute the tests under `sudo`. + +### 3. Harden existing `sudo*` helpers + +- `sudoOutput` is updated to prefix every elevated call with `sudo -n`. +- `sudoRunWithEnv` is updated to prefix every elevated call with `sudo -n env …`. +- `ensureSudoTimestamp` and `startSudoKeepalive` are **retained but restricted** to the `Demo` targets (they are harmless there, and the demo still needs a warm sudo timestamp). + +### 4. What stays the same + +- `mage build`, `mage test`, `mage testRace`, `mage generateTracepointsGo`, `mage generateTypesGo` – no root required. +- `ensureVMLINUX` and `readSyscallFormats` already call `sudoOutput` for discrete commands; they remain elevated, but now via `sudo -n`. + +## Impact on Existing Workflows + +| Before | After | +|---|---| +| `mage integrationTest` as non-root → auto `sudo env … go test …` | `mage integrationTest` as non-root → compiles as user, then `sudo -n -E ./integrationtests.test …` | +| Password prompt may appear mid-build | `sudo -n` fails fast if rule missing; no surprise prompts | +| JSON progress ticker from `go test -json` | Dropped for integration-test path; plain `-test.v` style output is printed directly. (The compiled test binary does not support the `go test` JSON protocol.) | +| `mage testWithName TEST_NAME=TestAioSetup` | Same compiled-binary dance when target is an integration test | +| `mage generate` | Unchanged UX; `sudo -n bpftool …` and `sudo -n sh -c 'cat /sys/kernel/tracing/…'` run automatically if sudoers rules exist | + +## Files Modified + +- `Magefile.go` – removes implicit sudo wrapping for `go test`; adds compiled-binary helpers. +- `docs/sudo-rules-for-ior.txt` – copy-paste rules for `/etc/sudoers.d/`. + +## Deployment Steps for the Admin + +1. As **root**, copy `docs/sudo-rules-for-ior.txt` into `/etc/sudoers.d/ior`. +2. Replace `%developers` with the actual Unix group or usernames that need to run tests. +3. Validate syntax: `visudo -c -f /etc/sudoers.d/ior`. +4. As **unprivileged user**, verify: `sudo -n bpftool btf dump file /sys/kernel/btf/vmlinux format c | head -3`. +5. Run `mage integrationTest` as the unprivileged user. + +## Known Limitations + +- **`mage demo`** is **not covered** by the granular rules. The demo tapes run a helper script (`run-tape.sh`) that performs multiple privileged actions (`pkill`, launching `./ior`). Running the demo still requires a broad `NOPASSWD` rule for that script, or running `mage demo` as root. +- **`mage installDemoTools`** requires `dnf` access and remains outside the scope of automated test rules. +- The compiled integration-test binary is invoked from the **repository root**. Sudoers rules using a wildcard path (`…/ior/integrationtests.test`) assume the repository is checked out somewhere predictable (e.g., `/home/<user>/git/ior/`). If the checkout lives in arbitrary locations, use a wrapper script at a fixed path. diff --git a/docs/sudo-rules-for-ior.txt b/docs/sudo-rules-for-ior.txt new file mode 100644 index 0000000..07ae9e5 --- /dev/null +++ b/docs/sudo-rules-for-ior.txt @@ -0,0 +1,88 @@ +# ============================================================================= +# Sudoers rules for I/O Riot NG (ior) build/test hardening +# ============================================================================= +# Purpose: +# Allow unprivileged users to run ONLY the specific commands that require +# root/CAP_BPF, while keeping Mage (the build tool) itself unprivileged. +# +# Installation (as root): +# 1. Replace "%developers" below with the actual Unix group or username. +# 2. Copy this file to /etc/sudoers.d/ior +# 3. chmod 440 /etc/sudoers.d/ior +# 4. visudo -c -f /etc/sudoers.d/ior +# +# Rules are ordered from most specific to least specific. +# Use "!env_reset" or pass env vars explicitly via sudo's env_keep if needed. +# ============================================================================= + +# ----------------------------------------------------------------------------- +# 1. Read-only kernel BTF dump (used by mage generate / mage world) +# ----------------------------------------------------------------------------- +# The exact invocation in Magefile.go is: +# sudo -n bpftool btf dump file /sys/kernel/btf/vmlinux format c +%developers ALL=(root) NOPASSWD: /usr/sbin/bpftool btf dump file /sys/kernel/btf/vmlinux format c + +# ----------------------------------------------------------------------------- +# 2. Read tracepoint format files (used by mage generate / mage world) +# ----------------------------------------------------------------------------- +# The exact invocation in Magefile.go is: +# sudo -n sh -c 'LC_ALL=C find /sys/kernel/tracing/events/syscalls \ +# -maxdepth 2 -mindepth 2 -name format | sort | xargs cat' +# Because the shell expands the command, we whitelist /bin/sh with the exact +# argument string. Adjust the path to /bin/sh or /usr/bin/sh as appropriate. +%developers ALL=(root) NOPASSWD: /bin/sh -c LC_ALL\=C find /sys/kernel/tracing/events/syscalls -maxdepth 2 -mindepth 2 -name format | sort | xargs cat + +# ----------------------------------------------------------------------------- +# 3. Run the compiled integration test binary (used by mage integrationTest, +# mage integrationTestSerial, mage testWithName for integration targets) +# ----------------------------------------------------------------------------- +# The binary is produced by "go test -c ./integrationtests" and lands in the +# repository root as "integrationtests.test". Because Go produces a static +# binary at a predictable name, we lock the rule to that path. +# +# If your repo is checked out in a non-standard location, you may need a +# symlink or wrapper script at a fixed path (e.g. /usr/local/bin/ior-test-runner) +# and update Magefile.go accordingly. +# +# IMPORTANT: we must preserve the environment (CGO_CFLAGS, CGO_LDFLAGS, LIBBPFGO, +# HOME, PATH, etc.) that mage sets before invoking sudo. Sudoers does not allow +# arbitrary env preservation by default, but "SETENV" on the Cmnd_Spec enables +# the caller to preserve the environment via "sudo -n -E ./binary". +# +# Alternatively, add the needed env vars to env_keep in your main sudoers file +# and omit SETENV here. The minimal set is: CGO_CFLAGS, CGO_LDFLAGS, GOARCH, +# GOOS, LIBBPFGO, HOME, GOPATH, GOMODCACHE, PATH, GOTOOLCHAIN. +%developers ALL=(root) NOPASSWD:SETENV: /home/*/*/ior/integrationtests.test + +# If you prefer a tighter path, uncomment and adjust the line below instead: +# %developers ALL=(root) NOPASSWD:SETENV: /home/paul/git/ior/integrationtests.test + +# ----------------------------------------------------------------------------- +# 4. Run the ior binary itself (optional — only if you want to run ior via sudo +# from scripts outside of mage) +# ----------------------------------------------------------------------------- +%developers ALL=(root) NOPASSWD:SETENV: /home/*/*/ior/ior + +# ----------------------------------------------------------------------------- +# 5. Demo tooling (OUT OF SCOPE for granular rules) +# ----------------------------------------------------------------------------- +# The demo pipeline (mage demo / mage demoOne) spawns a shell script that calls +# sudo multiple times for pkill, ttyd, and launching ior. This is inherently +# broader and should be handled separately—either by running the demo as root, +# or by creating a dedicated demo-user with a broader but still locked-down +# sudoers snippet. +# +# Example BROADER rule (use with caution): +# %developers ALL=(root) NOPASSWD: /bin/bash /home/*/*/ior/docs/tutorial/scripts/run-tape.sh * +# %developers ALL=(root) NOPASSWD: /usr/bin/pkill -TERM -f /ior$ +# %developers ALL=(root) NOPASSWD: /usr/bin/ttyd * + +# ----------------------------------------------------------------------------- +# 6. Package installation (OUT OF SCOPE — admin task only) +# ----------------------------------------------------------------------------- +# mage installDemoTools runs "sudo dnf install -y ttyd". This is a one-time +# admin operation and should NOT be included in automated CI/test sudoers. + +# ============================================================================= +# END OF FILE +# ============================================================================= |