Skip to content

feat(sandboxpolicy): enforce the file and identity sections and export section verdicts - #1075

Open
AlonLiwsky wants to merge 91 commits into
kubescape:feat/sandbox-policy-network-gadgetfrom
AlonLiwsky:feat/sandbox-policy-file-gadget
Open

AlonLiwsky wants to merge 91 commits into
kubescape:feat/sandbox-policy-network-gadgetfrom
AlonLiwsky:feat/sandbox-policy-file-gadget

Conversation

@AlonLiwsky

@AlonLiwsky AlonLiwsky commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Overview

This enforces the file section of SandboxPolicy with a new portable BPF LSM gadget, built the same way as the privilege and network gadgets: one node-wide instance, a cgroup → policy-slot bindings map, and the policy as frozen map data. Binding changes apply live (no pod restart); a policy change replaces the instance with overlap.

Stacked on #1073: the base is #1073's branch, so the diff shows only this PR. Merge after #1073.

Production code is about 3,800 lines: 2,800 in the gadget (BPF program and header, compiler, runtime), 1,000 in node-agent. Tests are about 3,600 lines.

1. The gadget — pkg/ebpf/gadgets/enforce_file/ (portable, no node-agent or Kubernetes concepts)

  • Hooks per access kind:

    • read / write: lsm/file_open (by f_mode; O_TRUNC is write), path_truncate
    • create: path_mknod (covers open(O_CREAT), openat2, io_uring), path_mkdir, path_symlink, path_link
    • delete: path_unlink, path_rmdir
    • rename: path_rename (both names)
    • chmod: path_chmod, path_chown, and inode_setxattr / inode_set_acl for file capabilities and POSIX ACLs (setcap, setfacl). These two hooks have no mount, so they are judged only on the task's root filesystem; elsewhere they fail closed.
    • A create is denied at path_mknod, not at file_open. A deny at file_open would leave an empty file.
  • Paths: bpf_d_path where the kernel allows it. path_mknod uses a bounded dentry walk, because bpf_d_path is refused there and bpf_path_d_path does not exist on 6.8. No kfunc is needed, so 6.8 and 6.12 run the same program.

  • Matching: a component-level shift-and NFA. There is one hash lookup per path component, against frozen per-slot bit masks. Globs: literal, * (one component), ** (any depth), and one * inside a component (cron*, *.sock, rc*.d). That covers every default path class in the CRD design. The hash is FNV-1a with a random seed per instance. A Go simulator runs the same step function, and a C harness checks that the real header agrees with it.

  • Moves: a link or rename source (and both names of RENAME_EXCHANGE) is judged for rename, read and write with the full decision. A new name is judged as create and write (or write and delete when it replaces a file). Renaming a parent directory of a protected path is denied at both ends. So ln /root/.aws/credentials /tmp/x, mv /root/.aws /tmp and mv /tmp/dir /root/.aws don't get around a rule.

  • fromImage: an overlay file with a lower-layer origin (ovl_entry has a lower stack). A copy-up by write, chmod, touch or setxattr does not change it. The runtime checks the private ovl_inode layout in the kernel BTF; if it differs, fromImage rules are reported as not enforced instead of reading a wrong field.

  • Overlay: each overlay open calls file_open twice. The second call is the backing file on an internal mount. The gadget skips it, because a deny there breaks copy-up.

  • Runtime work: the shared arming rule (common/arming.h) keeps runc init, kubectl exec setup and probes out until the workload's exec. Without it, a write default deny breaks docker exec and healthchecks (oom_score_adj).

  • Fail closed: a path the gadget can't resolve (too long, walk too deep, a task whose root is not its mount root after a chroot) gets the most limiting verdict of its access kind (unresolved-path).

  • Merge of several policies follows common/merge/CONTRACT.md per access kind and passes its shared vectors.

  • Cost for unbound containers: file_open is a hot path on every container of the node. The gadget checks the internal mount first and reads the cgroup level with plain loads (BINDINGS_DIRECT_LEVEL, opt-in in common/bindings.h; network and privilege don't change). Program time per open (bpf_stats), measured in the lab on arm64:

    unbound, cgroup depth 2 unbound, depth 4 bound, 51 rules
    6.12 80 ns 91 ns 362–366 ns
    6.8 81 ns 104 ns 440–446 ns

    On 6.8 the first program on file_open also adds about 250 ns of trampoline cost to every open on the node, when no other LSM program uses that hook.

2. The node-agent layer — pkg/sandboxpolicy/

  • The section splitter routes file to a new backend on the generic node owner. compiler.go no longer rejects the file and identity sections.
  • Translator (file.go): per-access defaults (defaultByAccess, or the old object form of default), their modes and origins, rules with access, path, pathClass, fromImage, and except on deny rules.
  • Path classes: the default classes from the CRD design (credentialStore, sshKeys, cloudConfig, shellRc, agentConfig, persistence, packageStore, containerRuntimeSocket, hostDevices), plus SandboxPolicyClasses, plus the policy's own classes.
  • Symlinked paths: the kernel sees resolved paths. So a /var/run/... pattern also gets /run/...; a pattern that ends in a file name also matches the kubelet atomic-writer form dir/..*/file (Secret, ConfigMap and projected volumes); and deny rules on /bin, /sbin, /lib, /lib64 also cover /usr/... (merged-usr images). Without this, the service-account token in credentialStore never matches.
  • Unresolved classes degrade as the design says: a deny rule goes to audit; an allow rule moves the defaults of its access kinds to audit, with Ready=False / ClassUnresolved. As in network, an audit-degraded deny keeps no paths, so it can't weaken its own policy's enforce default. An allow rule the gadget can't enforce does the same, with Ready=False / RuleNotEnforceable.
  • Built-in allows for runtime devices (/dev/null, /dev/zero, /dev/tty, /dev/pts/**, ...) and /dev/shm/**. Identical allow rules from several policies merge, so they don't count twice against the slot limit.
  • Reported as not enforced, with a reason (condition FileNotEnforced): execMap, fsType, actor on file rules, patterns the gadget can't express (?, [...], two * in one component).
  • The engine passes the runtime's mount namespace to the gadget (host-mntns-id), like privilege. Without it, runc init is judged on kind and kubectl exec fails.
  • Config key sandboxPolicyFileImage (default empty: the section reports FileNotConfigured).
  • Small refactor: the except splitting of the local mirror moved to a shared helper, used by network and file.

3. Identity section

  • identity is a readability section, as the CRD design says: the translator expands it into file and network rules. No new gadget.
  • Credential paths and pathClass (credentialStore, sshKeys, cloudConfig, agentConfig) become file read rules. A provisioned path that a deny class also covers (the service-account token) is subtracted from the class, so an identity allow is not cancelled by the deny.
  • imds: true becomes a network rule on the imds class.
  • envRead: true becomes a file read rule on /**/environ with a new gadget rule flag, otherProcess: it matches only the environ file of another process. A process can still read its own environment.
  • Verified on 6.8 and 6.17: another process's /proc/<pid>/environ (also through a second procfs mount) is denied, the task's own is allowed, cmdline and status stay readable. In kind (6.8): the CRD design's identity example denies credential reads and IMDS with the right rule and origin, and keeps the service-account token readable. No measurable cost change.

4. Start gap

  • Before, a container's first command ran before its own file binding existed. Now the file section also binds the pod cgroup before the containers start, like privilege and network: the pod plan keeps the file section when every container of the pod gets the same file policies.
  • The pod binding skips the arming, so the pause container, runc init and OCI hooks are not judged; the workload's first exec arms it. The file backend also starts an empty instance early, so the first pod finds the gadget loaded.

How to Test

  • go test ./pkg/ebpf/gadgets/... ./pkg/sandboxpolicy/...

  • Kernel test, as root on a Linux host with BPF LSM: go test -tags file_kernel ./pkg/ebpf/gadgets/enforce_file/runtime/ (see the README for the env vars). I ran it on 6.8 (Ubuntu 24.04) and 6.12 (Debian 13), arm64, at cgroup depth 2 and 4. It covers every access kind, the move guard at both ends, setfacl / setcap, fromImage after copy-up, chroot, audit, docker exec and healthchecks under a write default deny, and binding changes on a running task (bind after it opened files, moves to child cgroups and out, unbind, rebind).

  • In kind (Kubernetes 1.36, 6.8), live on running pods: write default deny with a workspace allow, credentialStore read deny including the projected service-account token, the hard-link and rename guard, fromImage, audit wouldDeny, unbind restoring access in about 3 s, kubectl exec and exec probes, ClassUnresolved and FileNotEnforced, two policies merged, an except entry, a Secret-volume file matched by **/.docker/config.json, setfacl / setcap under a chmod default deny, and one object with both network and file sections.

  • Start gap in kind (6.8), on a fresh node-agent with the policies applied first: the empty instance is loaded before any pod; a pod whose first command writes /etc is denied and still starts (also 5 pods in a burst, and an init container); an unselected pod is not affected; 0 runc verdicts. Replacing the instance for a new policy combination left no window for a running pod (about 1,150 writes tried, 0 got through).

  • Identity in kind (6.8): the CRD design's identity example in enforce mode denies credential reads and another process's /proc/1/environ, and allows the process's own environment and the service-account token.

Known limits

  • A pod whose containers get different file policies has no pod binding for file: there, a container's first command still runs before its own binding exists.
  • A hard link or bind mount that exists before the policy binds is a second name for the file. A path rule sees only the name used.
  • A bind mount made inside a new user + mount namespace renames paths. The privilege section's user-namespace deny closes this. A chroot into a mount root (for example a volume) is not detected either.
  • Renaming a parent of a floating pattern (a directory that contains .claude/) is not guarded.
  • In audit mode, an open(O_CREAT) of an existing file that is not in the dentry cache can log a false create wouldDeny. The kernel drops that error, so the open works and the enforce verdict is correct.
  • setcap, setfacl and cp -a with ACLs on a volume file (not the root filesystem) fail when the policy has any chmod deny. Removing an ACL is not judged. io_uring with registered credentials on a volume file is not judged for these two hooks.
  • Kernel issue, not caused by the gadget: on kernels without upstream fix 61df7b828204 (v6.11), the kernel skips cap_inode_setxattr whenever BPF LSM is active, so an unprivileged task can set security.* xattrs. I reproduced it on Ubuntu 6.8 with no BPF program loaded. Ubuntu 6.8 also refuses an LSM program that returns 1, so a program can't restore the check there. Where a load probe shows that the old convention works, the gadget returns the hook default when it allows.
  • On images where /lib and /usr/lib are separate directories (for example Alpine), a deny rule on /lib/** also covers /usr/lib/**, because of the merged-usr alias.
  • A Secret key with a subdirectory (items[].path: a/b/file) is not covered by the atomic-writer alias.
  • With overlay metacopy=on (not the containerd default) a data-only lower file is not tested.

Open questions

  1. execMap: not built (mmap_file / file_mprotect). The design marks it default-off because it breaks JITs. Is it needed in this ticket?

Tracking: SUB-9583.

Section verdict export (folded from #1080)

This also reports the privilege, network and file verdicts (enforced denies and audit wouldDeny) to the same receivers that exec authorizations use, so a private exporter (for example the OTLP mapping in armosec/private-node-agent#747) can pick them up. Until now these verdicts only went to the node-agent log.

Boundary (decided): the typed public section events and their queue stay here in node-agent. The private OTLP mapping lives in armosec/private-node-agent#747. Public event dispatch is not folded into the private exporter.

What changes

  • Typed section events (pkg/sandboxpolicy/section_verdict.go): one event per verdict for privilege, network and file, with its own dispatch key, the policy identity (kind, name, uid, generation, namespace) and revision, rule name, origins list, mode, wouldDeny, the target in policy words (for example read, not the C enum name) and the actor (process, container, pod).
  • Binding lookup: the event's cgroup is resolved to its container or pod binding, and the policy set comes from the judging instance's slot table (by generation and slot).
  • Own queue: section events have their own queue and worker and feed the same receivers. The exec exporter is unchanged (exporter.go is identical to feat(sandboxpolicy): enforce the file and identity sections and export section verdicts #1075), so a burst of section denies can never drop or delay an exec verdict. A test sends 1,000 section denies into a 4-entry section queue: 996 section events are dropped and counted, 0 exec verdicts.
  • Counters: kernel ring-buffer drops and section queue drops are exposed through Service.SectionVerdictStats() / SectionExporterStats(). A drop is never silent.
  • Debug receiver behind sandboxPolicyDebugExport (default off): logs one line per exported event, for local testing.
  • One verdict is one kernel check, not one syscall (an audited setns on a mount namespace gives two). The exporter does not dedupe; the doc says so.

Production code is about 950 lines (the section exporter ~150, the debug receiver ~70, the subscriber rewrite), tests about 820, docs about 35.

How to test

  • go test ./pkg/sandboxpolicy/...
  • Kernel export test on 6.8 and 6.17 (arm64): one enforced deny and one audit wouldDeny per section reach a test receiver with binding, policy identity, rule, origins, target and counters.
  • Local kind end-to-end run with the debug receiver on: every one of the 175 denied or audited actions produced its event with the right wouldDeny, rule and pod binding; for a pod with two policies, the event names the deciding policy and both in the set.

Follow-up (not in this PR)

The backend accepts only exec verdicts today. Privilege, network and file need backend changes (rule family per section with an authorization phase, a phase guard on the R-NET-1 / R-FILE-1 observation views, section defaults, target keys), plus the authorization-versus-observation mapping in the private exporter. That is a separate stacked change, not a blocker for this public typed-event PR.

Tracking: SUB-9591.

Add common/root_barrier.h: BPF LSM programs that always deny chroot,
pivot_root, new user namespaces, clone/clone3/unshare with CLONE_NEWNS,
setns into a mount or user namespace (or type 0), and mount-tree changes
for the tasks a gadget-defined predicate covers. Thread creation and setns
into other namespace types stay allowed.

Add common/rootbarrier: attestation that each barrier program has a live
LSM link and that BPF LSM is active, plus a self-test of real denials, for
the Ready step of a gadget that includes the barrier.

Add the root_barrier gadget, which uses the barrier alone with the
enforce_exec bindings shape, and an opt-in kernel test with Inspektor
Gadget alone.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
- Cover child cgroups in the root_barrier gadget (a task can move into a
  child cgroup, and clone3 can start a child in one) and state the
  requirement for gadget-defined predicates.
- Deny fsconfig with the other new mount API syscalls.
- Detect arm64 compat tasks from the saved pstate, not a TIF bit.
- Self-test: match the probe thread in its own PID namespace, prove each
  denial through a per-operation bit in the probe entry instead of the
  shared counter, send no event for probe denials, retry the probe clear,
  and explain a disabled user namespace limit.
- Kernel test: child cgroup, setns type 0, open_tree clone.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
- Record a probe denial before the covered-task branch, so a probe
  thread that the predicate also covers still proves the denial.
- Walk covered cgroups from the top of the hierarchy, so a task at any
  depth below a covered cgroup matches.
- Refuse to cover the root cgroup.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…ot-barrier

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Translate the file section of a policy into the enforce_file document and
drive one node-wide file gadget instance, like the network section.

- file.go: FilePlan and FileReport, a local mirror of defaultByAccess,
  defaultMode(ByAccess), defaultOrigin(ByAccess) and except, path classes
  (built-in, shared, policy-local), the class-unresolved degradation, the
  built-in runtime-devices allow rule, and not-enforced reporting (execMap,
  fsType, fromImage false, globs the gadget cannot match).
- The compiler accepts the file section; identity stays rejected.
- Controller, status (FileNotEnforced, combined compiled counts and
  degraded list), section backend (FileNotConfigured, WithFile).
- igbackend: FileBackend on the node owner, availability = BPF LSM. The
  native engine waits for the file runtime package (build tag
  file_runtime).
- Config key sandboxPolicyFileImage (default empty).

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Path rules for read, write, create, delete, rename and chmod with per-access
defaults, deny exceptions and an image-file (fromImage) condition.

- policy: pattern language (literal, *, **, pre*suf), per-slot compiler with
  the shared merge contract per access kind, segment bits with pattern dedup,
  guard of deny rules for move sources, literal hash table, glob table and a
  random hash seed per slot.
- file_policy.h: shift-and matcher over path components and the decision;
  a native harness test runs it against the Go simulator.
- program.bpf.c: file_open, path_truncate/chmod/chown/mknod/mkdir/symlink/
  link/unlink/rmdir/rename; overlay internal opens skipped; dentry walk for
  path_mknod and the hard link old name; unresolved paths fail closed;
  arming as in enforce_privilege.
- runtime: enforce-file operator (slots and frozen literal map, arming maps)
  and an opt-in kernel test.
- Registered in the portability test, the library README and the gadget list.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
The file engine started enforce_file without operator.oci.ebpf.host-mntns-id,
so the gadget armed tasks against the init task's mount namespace. Where the
container runtime runs in another mount namespace (kind), runc init was armed
and judged: under a write default deny, kubectl exec and exec probes failed on
runc's write to /proc/self/oom_score_adj. Pass the same value as the privilege
engine.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…ot-barrier

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
… after copy-up, fail closed in a chroot

- A move source (old name of a rename or a hard link, both names of
  RENAME_EXCHANGE) is judged as rename, read and write with the full
  decision (rules, except, fromImage, defaults, merge rule 5). The guard
  is now only the parent directories in the patterns of deny rules of
  read or write; a chmod or delete deny no longer blocks every rename.
  Both names of RENAME_EXCHANGE are also judged as write and delete.
- fromImage means the overlay file has a lower-layer origin
  (ovl_entry.__numlower > 0), so a copy-up by touch, setxattr, chmod or
  an allowed write keeps it. The runtime checks the ovl_inode and
  ovl_entry layout in the kernel's BTF and sets FILE_SLOT_FROM_IMAGE
  only when it holds (6.8 and 6.12 do); otherwise no file is an image
  file.
- A task whose root is not the root of its mount (chroot to a
  directory) gets every path judged as unresolved.
- The compiler merges identical allow rules across the documents of a
  slot, so the owner's built-in rules count once.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
… file section

- Every file glob also gets the names the kernel shows for the file: a
  /var/run/... pattern also as /run/..., and an exact file /dir/file
  also as /dir/..*/file (kubelet atomic-writer volumes, such as the
  service account token). Deny and allow rules and except entries.
- A second built-in allow rule, runtime-shm: read, write, create and
  delete in /dev/shm/**.
- A fromImage rule is reported as not enforced when the file gadget
  cannot read this node's overlay inode layout.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Check overlayfs' internal backing open before the binding lookup, so the
second file_open call of every overlay open no longer walks the cgroup
parents. Read f_path.mnt directly and mnt_ns with one kernel read.

bindings.h gets an opt-in BINDINGS_DIRECT_LEVEL: the cgroup level is read
with plain loads, and only when the task's own cgroup is not bound. Gadgets
that do not define it are unchanged.

Program time per unbound open, 6.8 and 6.12: 118-151 -> 75-78 ns at cgroup
level 2, 154-183 -> 100-102 ns at level 4.

Add TestGadgetBindingChanges: a task that already opened files is enforced
right after a bind, stays enforced when it moves into child cgroups of the
bound container, is free outside it and after unbind.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>

# Conflicts:
#	pkg/ebpf/gadgets/enforce_file/program.bpf.c
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
… names

- inode_setxattr (security.capability, POSIX ACL names) and inode_set_acl are judged as chmod. A dentry on the task's root super block is walked to the root; overlayfs' inner call on the upper file system (mounter credentials) is skipped; any other dentry is an unresolved path.
- New names of a rename, hard link or symbolic link (both names of RENAME_EXCHANGE) are judged against a new-name guard: the parents written in deny rules of write or create.
- A new name that does not exist yet (rename, hard link) is judged as create and write.
- The event path is dropped when another task wrote the per-CPU event path meanwhile.
- Remove dead code (guard any_path, a bound check the verifier does not need).

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…ded deny, not-enforceable allow

- The atomic-writer alias (..* before the last segment) applies to every glob whose last two segments are not **, so **/.docker/config.json also matches a Secret volume file. Deny rules also get the /usr form of /bin, /sbin, /lib and /lib64 globs.
- A deny rule degraded by an unresolved class keeps no path, as in the network translator, so it cannot hide its own policy's enforce default deny.
- An allow rule the gadget cannot enforce makes the policy not ready (RuleNotEnforceable).

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…op dead ACL names

Before Linux 6.11 (61df7b828204) security_inode_setxattr runs
cap_inode_setxattr only when the inode_setxattr hooks return 1. The owner
now probes the kernel (no bpf_lsm_inode_xattr_skipcap in BTF, and an
inode_setxattr program that returns 1 loads) and sets the new
setxattr-allow-one parameter; the program then returns 1 on an allow.
On 6.11+ and on kernels with the LSM return range check (6.12, Ubuntu
6.8) it stays 0, so the verifier never sees a return of 1.

POSIX ACLs go to inode_set_acl on every supported kernel (6.2+), so
inode_setxattr judges only security.capability.

Kernel test: unprivileged security.test setxattr (bound container,
unbound container, host uid 65534) is EPERM where the kernel can keep
the check; skipped with the reason where it cannot.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
A write to /bin/sh is judged on /usr/bin/sh. The deny already had the
/usr form; the except now has it too, so 'deny write /bin/**' except
/bin/sh keeps /bin/sh writable. README known limits: the alias also
widens a deny on images that are not merged-usr (Alpine), and nested
Secret keys are not covered.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
A file section that does not translate also refuses the pod plan, like network.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…e_setxattr only before 6.9

PrepareScheduled lost the file section in the merge of the network branch.
From 6.9 IMA and EVM run after BPF LSM, so a return of 1 would skip them.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5401d745-680c-44ab-8843-77926c574cef

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@AlonLiwsky

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
@AlonLiwsky
AlonLiwsky force-pushed the feat/sandbox-policy-file-gadget branch from ee7bd76 to 44f5997 Compare November 4, 2026 20:37
Attach a BPF LSM program that denies chroot with EACCES after the
barrier (the trampoline runs the newest program first) and check that
chroot still fails with EACCES, not the barrier's EPERM.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
The file compiler now keeps an explicit empty origin apart from no origin
and lists, per slot and access kind, the default origins of the policies
that agree with the merged default. A file default verdict takes them all
from there. The kernel test checks that an explicit "" default origin
reaches the exported event as [""] on Linux 6.8 and 6.17.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
PrepareScheduled prepares the file section too (active and slotPolicy), and
the pod plan keeps passing the file plans. Tests follow the base: an
untranslated file section keeps the pod binding (empty plan), and a policy
gets its own prepared plan.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
… branch

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>

# Conflicts:
#	pkg/sandboxpolicy/igbackend/nodeowner.go
…ault origin

The privilege and network compilers now keep an explicit empty origin
apart from no origin and list the default origins per slot. A default
verdict takes them all from there, like file. Kernel cases check that an
explicit "" exports as [""] for a privilege rule and a network default.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
… branch

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>

# Conflicts:
#	pkg/sandboxpolicy/igbackend/network_native.go
#	pkg/sandboxpolicy/igbackend/nodeowner.go
#	pkg/sandboxpolicy/integration/service.go
NewNativeNetworkEngine no longer takes a network tier since the parent removed the cgroup tier.

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
@matthyx

matthyx commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

One integration gate before calling enforcing file policies Ready: the gadget README correctly documents that chroot to a mount root changes the path view without being detected, and that a later user+mount namespace can give a file a second bind-mount name. #1075 currently has no root-barrier inclusion or mandatory privilege-section dependency, while pkg/sandboxpolicy/status.go checks file availability/degradation but not either guard. A file-only enforcing policy can therefore report Ready even though these path-specific denials can be evaded by a workload able to make those changes.

I would not depend on an optional privilege policy to close that gap. Please make #1074's independent root barrier (or an equivalent attested guard) a prerequisite for enforcement-capable file bindings, with exact coverage and strictly nonblocking audit-only bindings; otherwise refuse/mark the enforcing file binding NotReady and retain last-good. This can be a clearly tracked integration follow-up rather than a matcher rewrite in this PR, but it should be a merge/qualification gate for the full-enforcement claim. I have not rerun this PR's tagged kernel tests; the focused Go policy/controller tests on the latest stacked head pass.

@matthyx matthyx removed the needs-rebase PR has merge conflicts and needs to be rebased label Nov 9, 2026
@matthyx matthyx moved this from WIP to Waiting on Author in KS PRs tracking Nov 9, 2026
# Conflicts:
#	pkg/sandboxpolicy/sections_test.go
… branch

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>

# Conflicts:
#	pkg/sandboxpolicy/controller.go
#	pkg/sandboxpolicy/integration/service.go
@AlonLiwsky
AlonLiwsky changed the base branch from feat/sandbox-policy-exec-enforcement to feat/sandbox-policy-network-gadget November 9, 2026 13:24
@AlonLiwsky AlonLiwsky changed the title feat(sandboxpolicy): enforce the SandboxPolicy file and identity sections with a BPF LSM gadget feat(sandboxpolicy): enforce the file and identity sections and export section verdicts Nov 9, 2026
AlonLiwsky and others added 5 commits November 9, 2026 17:09
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
A section whose content does not change keeps its gadget instance, so
the slot table that verdicts read still held the generation the instance
started with. Found on a kind cluster: file and privilege verdicts kept
generation 1 while the policy moved to 4. A rebind or pod plan with the
same content now records the newer identities in the running table.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…r' into feat/file-root-barrier

# Conflicts:
#	pkg/ebpf/gadgets/common/README.md
#	pkg/ebpf/gadgets/common/enforcement/portability_test.go
The file rules name paths, and a task that changes its root or its mount
tree changes what those paths mean. The file gadget now includes the shared
root barrier (from kubescape#1074): every armed task of an enforcing slot, in the
bound cgroup or below it, cannot chroot, pivot_root, create a user
namespace, create or join a mount namespace, or change the mount tree.
Audit slots and unarmed runtime tasks are never covered.

The runtime attests the barrier at Ready (rootbarrier.Verify). A failed
attestation refuses the instance when an initial binding is on an
enforcing slot; otherwise the node owner refuses binds to enforcing slots
and reports them through SlotHealth. Audit-only bindings never depend on
the barrier.

Kernel tests: chroot and unshare -m are denied in an enforcing slot and
chroot works in an audit slot; the otherProcess test mounts its second
procfs before binding. Pass on 6.8 and 6.17 (arm64).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
Review follow-up: the load-time hook check now covers the barrier's hooks,
so a kernel with another signature is refused before any barrier program
attaches (each program returns the previous verdict for every task). The
Ready test now checks that Ready goes on to the controller, and the README
says that the barrier stops changes made after binding only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
@AlonLiwsky

Copy link
Copy Markdown
Collaborator Author

Done in 6606993. I merged #1074's root barrier into this branch, and the file gadget now includes it. Coverage is the exact rule you asked for: armed tasks of an enforcing slot, in the bound cgroup or below it. Audit-only slots and the runtime's own setup are never covered. The runtime attests the barrier at Ready with rootbarrier.Verify, and the hook signatures are also checked before load. If the attestation fails, an instance with an enforcing initial binding is refused (last-good stays). Otherwise, binds to enforcing slots are refused and reported NotReady through SlotHealth, and audit-only bindings never depend on it. Kernel tests on 6.8 and 6.17: chroot and unshare -m fail with EPERM in an enforcing slot, and chroot still works in an audit slot. The README limit now points at the barrier instead of the optional privilege policy. This makes #1075 depend on #1074.

@matthyx matthyx moved this from Waiting on Author to Needs Reviewer in KS PRs tracking Nov 9, 2026
AlonLiwsky and others added 3 commits November 9, 2026 17:57
The attestation walks every BPF link on the host. The kernel answers EAGAIN
for a link that is being created or freed, and the link iterator stops at
that error, so the Ready step of a gadget failed whenever another gadget on
the node started or stopped (seen on 6.8: "listing BPF links: get link for
ID ...: resource temporarily unavailable"). The walk is now retried up to
20 times, 50 ms apart; other errors are not retried.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>
…gadget' into feat/file-root-barrier

Signed-off-by: Alon Liwsky <40373481+AlonLiwsky@users.noreply.github.com>

@matthyx matthyx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alignment review against the enforce_exec reference (kubescape/inspektor-gadget#15, #1054). enforce_exec is the reference implementation for SandboxPolicy enforcement gadgets, and every enforcement gadget is held to the same production quality bar — this is production code, not an MVP:

  • Strict never allows on any fault (fail closed); Audit is strictly nonblocking, even on faults. Unresolved or unsupported policy semantics produce an explicit NotReady, never a silent allow or a silent downgrade.
  • Exact cgroup scope (no descendants) plus host consent; any wider scope is an explicit, consented, per-binding property shown in status.
  • One long-lived shared node-wide BPF collection per enforcement family, with bindings and bounded compiled-policy storage published in place, keeping last-good; no per-binding or per-policy-change collection reloads.
  • Bounded state with exact accounting; witness counters for every fail-closed or fault path, surfaced as degraded/NotReady in status.
  • Ready only after qualification on the exact AL2023 amd64 kernels 6.12.113-138.209 and 6.18.56-125.164, with live tests for races, exhaustion, injected faults, restart/reload (last-good) and coexistence with the other enforcement gadgets attached on the same LSM hooks (a prior LSM denial is never overridden, one gadget's fault never becomes an allow elsewhere).
  • Verdict/event export consistent with #1054.

Summary. A BPF LSM file gadget (file_open, path_*, inode_setxattr/set_acl, component NFA, fromImage), task-storage arming and a root barrier for enforce slots. identity expands into file and network rules. Typed section verdicts go to the #1054 receivers on their own queue.

Verdict: needs changes (one blocking).

The 256 KiB ringbuf per instance (the reference uses 4 KiB) doubles during overlap. Add it to a stated resource budget. NewNativeFileEngine accepts any image string with no digest or qualification check (pkg/sandboxpolicy/igbackend/file_native.go L26). Reuse the image admission from #1054 (imageindex).

Comment thread pkg/sandboxpolicy/file.go
if unresolved {
reasons = append(reasons, "class-unresolved")
if r.Action == api.ActionDeny {
mode, effective = "audit", "audit"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[blocking] This is worse than in the network section: an enforce deny with one unresolved pathClass sets paths = nil, so the resolved paths (for example credentialStore) are not enforced at all, and Ready stays True. Keep the resolved paths at enforce, set NotReady=ClassUnresolved, and add a test that a credential read stays denied.

} file_literals SEC(".maps");

// file_slot_get returns the slot header, or NULL when the slot is not judged.
static __always_inline const struct file_slot *file_slot_get(__u32 slot)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[should] If the cgroup is bound but the slot is missing, file_bound() returns false and file_open allows. The scratch lookup at pkg/ebpf/gadgets/enforce_file/program.bpf.c L202 has the same problem. Fail closed for a bound cgroup and increment a fault counter, as in the network gadget review and the reference (gadgets/enforce_exec/program.bpf.c in kubescape/inspektor-gadget#15 (branch spike/enforce-exec-interface) returns -1 when the policy header is missing).

if (r)
return r;
if (setxattr_allow_one)
return 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[should] return 1 on inode_setxattr is a nonzero return, and the BPF LSM trampoline skips every later BPF LSM program after a nonzero return. Another enforcement gadget attached later on this hook would never get to deny. Enforcement gadgets on the same LSM hooks must compose: one gadget's allow must not suppress another's deny. Attach it last and document that, or avoid the nonzero return where the kernel allows.

// (flags << 30 s>> 63) & -ENOMEM, whose range some verifiers lose
// (Linux 6.17 rejects it: R0 not in [-4095, 0]).
volatile int result = 0;
if ((__u64)clone_flags & CLONE_INTO_CGROUP)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[should] When a mark fails (a tracking fault), an Audit slot still gets -ENOMEM for CLONE_INTO_CGROUP. Audit must be strictly nonblocking, even under tracking faults. Allow it in Audit slots and count VERDICT_COUNT_SECTION_ERROR.

return 0;
if (!file_bound(&e))
return 0;
if (!file_armed(&e))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[should] Arming exempts runtime helpers until the workload execs. The reference Strict profile has "no helper exemption and no inferred exemptions". Keep arming, but document it as an explicit per-binding exemption, and have Strict report NotReady when arming_slot_fault is nonzero (fault paths need witness counters surfaced in status).

Reason string
// Generation is the gadget instance that judged the call (the node
// owner's counter), not a policy generation. Slot indexes its policy table.
Generation uint64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[should] For consistency with #1054: the exec Verdict has an aggregate Revision (Plan.Revision) and an operational_error reason. SectionVerdict has only Generation (an instance counter), and a fault produces no event. Add Revision and emit operational_error for slot or arming faults, so the OTLP mapping can treat all sections the same way.

@matthyx matthyx moved this from Needs Reviewer to Waiting on Author in KS PRs tracking Nov 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Waiting on Author

Development

Successfully merging this pull request may close these issues.

2 participants