From 86d1e3d3cc9482002007603216c8511b0751a236 Mon Sep 17 00:00:00 2001 From: Anirudh Oppiliappan Date: Tue, 16 Jun 2026 12:02:55 +0300 Subject: [PATCH] knotserver/sandbox: narrow global config grant to single file - Add unit and integration tests for sandbox and path behavior - Define a RuleSpec to construct Landlock ruleset - Enforce $HOME/.config/git/config for git config (was previously granting the entirety of $HOME) Signed-off-by: Anirudh Oppiliappan --- .../sandbox/sandbox_integration_test.go | 250 ++++++++++++++++++ knotserver/sandbox/sandbox_linux.go | 92 ++++--- knotserver/sandbox/sandbox_linux_test.go | 71 +++++ 3 files changed, 382 insertions(+), 31 deletions(-) create mode 100644 knotserver/sandbox/sandbox_integration_test.go diff --git a/knotserver/sandbox/sandbox_integration_test.go b/knotserver/sandbox/sandbox_integration_test.go new file mode 100644 index 00000000..9e4f4d18 --- /dev/null +++ b/knotserver/sandbox/sandbox_integration_test.go @@ -0,0 +1,250 @@ +//go:build linux && integration + +package sandbox + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/landlock-lsm/go-landlock/landlock" +) + +// This file exercises actual Landlock enforcement. It works by re-execing the +// test binary with an env var that makes it run the "child" code path: the +// child applies the ruleset built by BuildRuleSpec and attempts a specific +// filesystem operation, exiting 0 (allowed) or non-zero (denied). +// +// Build tag: linux && integration. Run with: +// go test -tags integration ./knotserver/sandbox/... + +const ( + childEnv = "TANGLED_SANDBOX_INT_CHILD" + childRepo = "TANGLED_SANDBOX_INT_REPO" + childTarget = "TANGLED_SANDBOX_INT_TARGET" + childOp = "TANGLED_SANDBOX_INT_OP" +) + +func TestMain(m *testing.M) { + if os.Getenv(childEnv) == "1" { + runChild() + return + } + os.Exit(m.Run()) +} + +func runChild() { + repoPath := os.Getenv(childRepo) + target := os.Getenv(childTarget) + op := os.Getenv(childOp) + + spec := BuildRuleSpec([]string{repoPath}) + rules := []landlock.Rule{ + landlock.RODirs(spec.SystemRO...).IgnoreIfMissing(), + landlock.RWFiles(spec.DevRW...).WithIoctlDev().IgnoreIfMissing(), + landlock.RWDirs(spec.TmpRW...).IgnoreIfMissing(), + } + if spec.GitConfigRO != "" { + rules = append(rules, landlock.ROFiles(spec.GitConfigRO).IgnoreIfMissing()) + } + for _, p := range spec.RepoRW { + rules = append(rules, landlock.RWDirs(p).WithRefer()) + } + if err := landlock.V8.BestEffort().RestrictPaths(rules...); err != nil { + fmt.Fprintf(os.Stderr, "restrict failed: %v\n", err) + os.Exit(2) + } + + var opErr error + switch op { + case "read": + _, opErr = os.ReadFile(target) + case "write": + opErr = os.WriteFile(target, []byte("x"), 0644) + case "list": + _, opErr = os.ReadDir(target) + default: + fmt.Fprintf(os.Stderr, "unknown op %q\n", op) + os.Exit(3) + } + + if opErr != nil { + fmt.Fprintln(os.Stderr, opErr) + os.Exit(1) + } + os.Exit(0) +} + +// runUnderSandbox spawns the test binary as a child, applies Landlock with +// the given repoPath as the granted RW path, then attempts op on target. +// Returns true if the op was allowed, false if denied. +func runUnderSandbox(t *testing.T, repoPath, target, op string) (allowed bool, output string) { + t.Helper() + cmd := exec.Command(os.Args[0]) + cmd.Env = append(os.Environ(), + childEnv+"=1", + childRepo+"="+repoPath, + childTarget+"="+target, + childOp+"="+op, + ) + out, err := cmd.CombinedOutput() + if err == nil { + return true, string(out) + } + if exit, ok := err.(*exec.ExitError); ok && exit.ExitCode() == 1 { + return false, string(out) + } + t.Fatalf("child exited unexpectedly: %v\noutput: %s", err, out) + return false, "" +} + +func skipIfNoLandlock(t *testing.T) { + t.Helper() + if !probeLandlock() { + t.Skip("Landlock not available on this kernel") + } +} + +func TestSandboxIntegration_AllowsOwnRepoRead(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + repo := filepath.Join(root, "did:plc:abc") + target := filepath.Join(repo, "HEAD") + mustMkdirAll(t, repo) + mustWriteFile(t, target, "ref: refs/heads/main\n") + + allowed, out := runUnderSandbox(t, repo, target, "read") + if !allowed { + t.Errorf("expected read of own repo to be allowed; child said: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_DeniesOtherRepoRead(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + myRepo := filepath.Join(root, "did:plc:abc") + otherRepo := filepath.Join(root, "did:plc:xyz") + secret := filepath.Join(otherRepo, "secret-key") + mustMkdirAll(t, myRepo) + mustMkdirAll(t, otherRepo) + mustWriteFile(t, secret, "TOPSECRET\n") + + allowed, out := runUnderSandbox(t, myRepo, secret, "read") + if allowed { + t.Errorf("expected read of other repo to be denied; child read: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_AllowsGlobalConfigRead(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + myRepo := filepath.Join(root, "did:plc:abc") + cfgDir := filepath.Join(root, ".config", "git") + cfg := filepath.Join(cfgDir, "config") + mustMkdirAll(t, myRepo) + mustMkdirAll(t, cfgDir) + mustWriteFile(t, cfg, "[user]\n\tname = test\n") + + // the child reads $HOME via BuildRuleSpec, so set HOME for it explicitly. + t.Setenv("HOME", root) + + allowed, out := runUnderSandbox(t, myRepo, cfg, "read") + if !allowed { + t.Errorf("expected read of $HOME/.config/git/config to be allowed; child said: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_DeniesGlobalConfigSibling(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + myRepo := filepath.Join(root, "did:plc:abc") + cfgDir := filepath.Join(root, ".config", "git") + sibling := filepath.Join(cfgDir, "attributes") + mustMkdirAll(t, myRepo) + mustMkdirAll(t, cfgDir) + mustWriteFile(t, sibling, "*.bin -text\n") + + t.Setenv("HOME", root) + + allowed, out := runUnderSandbox(t, myRepo, sibling, "read") + if allowed { + t.Errorf("expected read of sibling in .config/git/ to be denied (only the config file is granted); child read: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_DeniesScanPathSibling(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + myRepo := filepath.Join(root, "did:plc:abc") + dbFile := filepath.Join(root, "knotserver.db") + mustMkdirAll(t, myRepo) + mustWriteFile(t, dbFile, "fake db contents\n") + + allowed, out := runUnderSandbox(t, myRepo, dbFile, "read") + if allowed { + t.Errorf("expected read of sibling file (knotserver.db) to be denied; child read: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_DeniesScanPathListing(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + myRepo := filepath.Join(root, "did:plc:abc") + mustMkdirAll(t, myRepo) + + allowed, out := runUnderSandbox(t, myRepo, root, "list") + if allowed { + t.Errorf("expected listing scan path to be denied; child output: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_AllowsOwnRepoWrite(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + repo := filepath.Join(root, "did:plc:abc") + target := filepath.Join(repo, "new-file") + mustMkdirAll(t, repo) + + allowed, out := runUnderSandbox(t, repo, target, "write") + if !allowed { + t.Errorf("expected write to own repo to be allowed; child said: %s", strings.TrimSpace(out)) + } +} + +func TestSandboxIntegration_DeniesSystemWrite(t *testing.T) { + skipIfNoLandlock(t) + + root := t.TempDir() + repo := filepath.Join(root, "did:plc:abc") + mustMkdirAll(t, repo) + + // /etc is granted RO; writes must be denied. + allowed, out := runUnderSandbox(t, repo, "/etc/sandbox-test-should-fail", "write") + if allowed { + t.Errorf("expected write to /etc to be denied; child output: %s", strings.TrimSpace(out)) + } +} + +func mustMkdirAll(t *testing.T, p string) { + t.Helper() + if err := os.MkdirAll(p, 0o755); err != nil { + t.Fatal(err) + } +} + +func mustWriteFile(t *testing.T, p, content string) { + t.Helper() + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} diff --git a/knotserver/sandbox/sandbox_linux.go b/knotserver/sandbox/sandbox_linux.go index 3614d316..a927912b 100644 --- a/knotserver/sandbox/sandbox_linux.go +++ b/knotserver/sandbox/sandbox_linux.go @@ -80,6 +80,54 @@ func (l *LandlockBackend) WrapMulti(paths []string, cmd *exec.Cmd) (*exec.Cmd, e func (l *LandlockBackend) Name() string { return "landlock" } +// RuleSpec describes the paths a sandbox should grant access to, grouped by +// access tier. It is the input to the Landlock ruleset construction and is +// exposed so the path-derivation logic can be tested independently of any +// actual kernel-level enforcement. +type RuleSpec struct { + // SystemRO is the set of system directories granted read+execute. + SystemRO []string + // GitConfigRO is the global git config file, granted read-only access + // at file granularity. Empty when $HOME is not set. + GitConfigRO string + // DevRW is the set of device-file directories granted read/write + + // ioctl access (needed so /dev/null works under Landlock V5+). + DevRW []string + // TmpRW is the set of directories granted read/write for temporary + // patch and object files. + TmpRW []string + // RepoRW is the set of repository directories granted read/write + // access including the REFER right (for cross-directory rename in + // receive-pack's quarantine migration). + RepoRW []string +} + +// BuildRuleSpec derives the set of paths the sandbox should grant to each +// access tier given the repository paths the subprocess operates on. +func BuildRuleSpec(repoPaths []string) RuleSpec { + return buildRuleSpec(repoPaths, os.Getenv("HOME")) +} + +// buildRuleSpec is the testable variant of BuildRuleSpec that takes $HOME +// explicitly instead of reading it from the environment. +func buildRuleSpec(repoPaths []string, home string) RuleSpec { + var gitConfig string + if home != "" { + // the only thing the sandboxed git subprocess needs from $HOME is the + // global config file. granting just that one file (not the whole + // .config tree) keeps everything else under $HOME outside the ruleset. + gitConfig = filepath.Join(home, ".config", "git", "config") + } + + return RuleSpec{ + SystemRO: []string{"/usr", "/bin", "/lib", "/lib64", "/nix", "/etc"}, + GitConfigRO: gitConfig, + DevRW: []string{"/dev"}, + TmpRW: []string{"/tmp"}, + RepoRW: append([]string(nil), repoPaths...), + } +} + // ApplyLandlock applies a Landlock ruleset to the current process then // exec's into gitArgs. Called from the hidden "sandbox-exec" subcommand. func ApplyLandlock(repoPaths []string, gitArgs []string) error { @@ -87,37 +135,19 @@ func ApplyLandlock(repoPaths []string, gitArgs []string) error { return fmt.Errorf("sandbox-exec: no command specified") } - // collect unique parent directories so git can read global config - // under $HOME/.config/git/config. repo contents stay DAC-locked - // (0700) so other repos can't actually be read. - parents := map[string]struct{}{} - for _, p := range repoPaths { - parents[filepath.Dir(p)] = struct{}{} - } - parentSlice := make([]string, 0, len(parents)) - for p := range parents { - parentSlice = append(parentSlice, p) - } - - // each repo gets full read/write plus REFER (needed for git's quarantine - // rename in receive-pack, which moves objects across directories). - repoRules := make([]landlock.Rule, len(repoPaths)) - for i, p := range repoPaths { - repoRules[i] = landlock.RWDirs(p).WithRefer() - } - - rules := append([]landlock.Rule{ - // system dirs: read + execute only, no writes - landlock.RODirs("/usr", "/bin", "/lib", "/lib64", "/nix", "/etc").IgnoreIfMissing(), - // /dev/null and friends: read/write files + ioctl (V5+ restricts ioctl - // on device files; WithIoctlDev keeps /dev/null fully accessible) - landlock.RWFiles("/dev").WithIoctlDev().IgnoreIfMissing(), - // parent dirs: read + execute so git can traverse to the repo and read - // global git config; 0700 DAC permissions prevent cross-repo reads - landlock.RODirs(parentSlice...).IgnoreIfMissing(), - // /tmp: read/write for temporary patch and object files - landlock.RWDirs("/tmp").IgnoreIfMissing(), - }, repoRules...) + spec := BuildRuleSpec(repoPaths) + + rules := []landlock.Rule{ + landlock.RODirs(spec.SystemRO...).IgnoreIfMissing(), + landlock.RWFiles(spec.DevRW...).WithIoctlDev().IgnoreIfMissing(), + landlock.RWDirs(spec.TmpRW...).IgnoreIfMissing(), + } + if spec.GitConfigRO != "" { + rules = append(rules, landlock.ROFiles(spec.GitConfigRO).IgnoreIfMissing()) + } + for _, p := range spec.RepoRW { + rules = append(rules, landlock.RWDirs(p).WithRefer()) + } // V8.BestEffort enforces the strongest ruleset the running kernel supports, // up to V8. RestrictPaths also sets PR_SET_NO_NEW_PRIVS automatically. diff --git a/knotserver/sandbox/sandbox_linux_test.go b/knotserver/sandbox/sandbox_linux_test.go index 614fa23e..d2d6d5d5 100644 --- a/knotserver/sandbox/sandbox_linux_test.go +++ b/knotserver/sandbox/sandbox_linux_test.go @@ -4,11 +4,82 @@ package sandbox import ( "os/exec" + "reflect" "strings" "syscall" "testing" ) +func TestBuildRuleSpec_SingleRepo(t *testing.T) { + spec := buildRuleSpec([]string{"/home/git/did:plc:abc"}, "/home/git") + + if got, want := spec.GitConfigRO, "/home/git/.config/git/config"; got != want { + t.Errorf("GitConfigRO = %q, want %q", got, want) + } + if got, want := spec.RepoRW, []string{"/home/git/did:plc:abc"}; !reflect.DeepEqual(got, want) { + t.Errorf("RepoRW = %q, want %q", got, want) + } + if got, want := spec.SystemRO, []string{"/usr", "/bin", "/lib", "/lib64", "/nix", "/etc"}; !reflect.DeepEqual(got, want) { + t.Errorf("SystemRO = %q, want %q", got, want) + } + if got, want := spec.TmpRW, []string{"/tmp"}; !reflect.DeepEqual(got, want) { + t.Errorf("TmpRW = %q, want %q", got, want) + } + if got, want := spec.DevRW, []string{"/dev"}; !reflect.DeepEqual(got, want) { + t.Errorf("DevRW = %q, want %q", got, want) + } +} + +func TestBuildRuleSpec_GitConfigFollowsHome(t *testing.T) { + // the granted git config path must follow $HOME, not the repo path. this + // is what makes the merge case work: tmpDir is under /tmp, but the + // subprocess still resolves the global config from $HOME/.config/git/config. + spec := buildRuleSpec([]string{"/tmp/git-clone-XYZ"}, "/home/git") + + if got, want := spec.GitConfigRO, "/home/git/.config/git/config"; got != want { + t.Errorf("GitConfigRO = %q, want %q", got, want) + } +} + +func TestBuildRuleSpec_NoHome(t *testing.T) { + // empty $HOME should not produce a bogus "/.config/git/config" entry. + spec := buildRuleSpec([]string{"/home/git/did:plc:abc"}, "") + + if spec.GitConfigRO != "" { + t.Errorf("GitConfigRO = %q, want empty", spec.GitConfigRO) + } +} + +func TestBuildRuleSpec_NeverGrantsScanPath(t *testing.T) { + // the scan path (the repo's parent) must NEVER appear in any RW or RO + // list. granting it would expose other repos and the knot DB via + // Landlock RO + DAC group bits. this is the key invariant the rule + // tightening was meant to enforce. + spec := buildRuleSpec([]string{"/home/git/did:plc:abc"}, "/home/git") + + parent := "/home/git" + for _, group := range [][]string{spec.SystemRO, spec.DevRW, spec.TmpRW, spec.RepoRW} { + for _, p := range group { + if p == parent { + t.Errorf("scan path %q must not appear in the ruleset; found in %q", parent, group) + } + } + } + if spec.GitConfigRO == parent { + t.Errorf("scan path %q must not be granted as GitConfigRO", parent) + } +} + +func TestBuildRuleSpec_EmptyInput(t *testing.T) { + spec := buildRuleSpec(nil, "") + if spec.GitConfigRO != "" { + t.Errorf("GitConfigRO should be empty for nil input and no $HOME, got %q", spec.GitConfigRO) + } + if len(spec.RepoRW) != 0 { + t.Errorf("RepoRW should be empty for nil input, got %q", spec.RepoRW) + } +} + func TestLandlockBackend_Name(t *testing.T) { if (&LandlockBackend{}).Name() != "landlock" { t.Error("Name should return \"landlock\"") -- 2.51.2