From b557bd74663b9aacaa8753a1e62872e24aa3d319 Mon Sep 17 00:00:00 2001 From: Yi-Min Lin Date: Wed, 7 Oct 2026 17:12:13 -0700 Subject: [PATCH 1/2] deploy: grant executor capabilities in upgrade-database.yml (#414) upgrade-database.yml activated and started the candidate executor without the file capabilities the executor role grants, so until a full deploy followed the executor ran without CAP_BPF, CAP_PERFMON, CAP_NET_ADMIN and CAP_NET_RAW. Move the getcap/setcap logic into tasks/executor-capabilities.yml and import it from the executor role, rollout-executors.yml and upgrade-database.yml so the three cannot drift. upgrade-database.yml grants the set to the staged candidate before stopping the service; a refusal stops there with the service and database untouched, as in the rollout. The role keeps warning instead of failing, and still restarts the executor when the set changes. The rollout now also skips setcap when the binary already carries the set, and grants it under executor_ambient_caps as the role and verify.yml already expected. test_rollout.py runs upgrade-database.yml against the fixture hosts and checks that setcap precedes stop/upgrade/start, is skipped when the set is present or BPF is disabled, and that a refusal stops before the service is touched. Co-Authored-By: Claude Opus 5.5 --- deploy/README.md | 12 ++- deploy/ansible/roles/executor/tasks/main.yml | 59 +++-------- deploy/ansible/rollout-executors.yml | 15 ++- .../ansible/tasks/executor-capabilities.yml | 70 ++++++++++++ deploy/ansible/upgrade-database.yml | 15 ++- deploy/test/test_rollout.py | 100 +++++++++++++++--- 6 files changed, 199 insertions(+), 72 deletions(-) create mode 100644 deploy/ansible/tasks/executor-capabilities.yml diff --git a/deploy/README.md b/deploy/README.md index bd96be51..9ae3074c 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -433,9 +433,11 @@ executor's identity. ### Executor capabilities and vantage-point metadata -The executor role gives the installed executor binary exactly the file -capabilities in `executor_capabilities` -([`group_vars/executors.yml`](ansible/group_vars/executors.yml)): +The executor role, `rollout-executors.yml` and `upgrade-database.yml` give +the installed executor binary exactly the file capabilities in +`executor_capabilities` +([`group_vars/executors.yml`](ansible/group_vars/executors.yml)), all through +[`ansible/tasks/executor-capabilities.yml`](ansible/tasks/executor-capabilities.yml): | Capability | Needed for | | --- | --- | @@ -449,7 +451,9 @@ Without the first three an executor reports `enforcement_mode: fallback`, without `cap_net_raw` also `tagging.ipv4: none` and no ICMP. `cap_sys_resource` is not granted: from Linux 5.11 BPF memory is charged to the memory cgroup, and on an older kernel the unit sets `LimitMEMLOCK=infinity` instead. A host -that refuses file capabilities keeps deploying with a warning; +that refuses file capabilities keeps deploying with a warning (a rollout or +database upgrade, which starts that binary in place of a running executor, +stops instead); [`ansible/verify.yml`](ansible/verify.yml) fails on any executor whose binary lacks the set. Set `executor_enable_bpf: false` for such a host. diff --git a/deploy/ansible/roles/executor/tasks/main.yml b/deploy/ansible/roles/executor/tasks/main.yml index 35992682..ecd6dd1c 100644 --- a/deploy/ansible/roles/executor/tasks/main.yml +++ b/deploy/ansible/roles/executor/tasks/main.yml @@ -88,55 +88,20 @@ notify: Restart executor # Without these the eBPF tagger cannot load and probes leave untagged, so this -# is what makes packet attribution verifiable; executor_capabilities in -# group_vars/executors.yml says what each one is for. The binary gets exactly -# that set: setcap replaces whatever the file carried before. Some hosts -# (restricted containers, noexec/nosuid or non-xattr filesystems) reject file -# capabilities outright; that costs those nodes their attribution tags but -# must not fail the whole deploy, so the failure is reported rather than -# fatal, and verify.yml reports the missing capabilities. Set -# executor_enable_bpf=false on such a host to skip the attempt entirely. +# is what makes packet attribution verifiable. Some hosts (restricted +# containers, noexec/nosuid or non-xattr filesystems) reject file capabilities +# outright; that costs those nodes their attribution tags but must not fail the +# whole deploy, so the failure is reported rather than fatal, and verify.yml +# reports the missing capabilities. Set executor_enable_bpf=false on such a +# host to skip the attempt entirely. The same task file grants them in +# rollout-executors.yml and upgrade-database.yml. - name: Set the executor's capabilities on its binary - when: executor_enable_bpf | default(true) | bool + ansible.builtin.import_tasks: "{{ role_path }}/../../tasks/executor-capabilities.yml" vars: - # The installed payload itself, not the link next to it: file - # capabilities live on the executable. - executor_release_binary: "{{ payload_prefix }}/lib/debuglet/{{ debuglet_release.version }}/bin/debuglet-executor" - block: - - name: Read the executor binary's file capabilities - ansible.builtin.command: - argv: [getcap, "{{ executor_release_binary }}"] - register: executor_getcap - changed_when: false - failed_when: false - check_mode: false - - # getcap prints "PATH cap_a,cap_b=ep" for a file that carries them. - - name: Grant exactly the executor's capabilities - ansible.builtin.command: - argv: - - setcap - - "{{ executor_capabilities | join(',') }}=ep" - - "{{ executor_release_binary }}" - register: executor_setcap - failed_when: false - changed_when: executor_setcap.rc == 0 - # A running executor keeps the capabilities it started with. - notify: Restart executor - when: >- - ((executor_getcap.stdout | default('') | regex_search('\\s(\\S+)=ep\\s*$', '\\1') or ['']) | first).split(',') | sort - != executor_capabilities | sort - - - name: Warn when the capabilities could not be set - ansible.builtin.debug: - msg: >- - Could not set {{ executor_capabilities | join(',') }} on - {{ executor_release_binary }} - ({{ executor_setcap.stderr | default('') | trim | default('setcap failed', true) }}). - This node will run without the eBPF egress tagger, so its probes - will carry no attribution tag and cannot be verified, and without - cap_net_raw it offers no ICMP. - when: executor_setcap.rc | default(0) != 0 + executor_capabilities_binary: "{{ payload_prefix }}/lib/debuglet/{{ debuglet_release.version }}/bin/debuglet-executor" + executor_capabilities_required: false + # A running executor keeps the capabilities it started with. + notify: Restart executor - name: Install executor systemd unit ansible.builtin.template: diff --git a/deploy/ansible/rollout-executors.yml b/deploy/ansible/rollout-executors.yml index a1927c64..d652c818 100644 --- a/deploy/ansible/rollout-executors.yml +++ b/deploy/ansible/rollout-executors.yml @@ -63,15 +63,14 @@ ansible.builtin.include_role: name: payload - # Exactly executor_capabilities (group_vars/executors.yml); setcap - # replaces whatever the staged file carried. + # Exactly executor_capabilities (group_vars/executors.yml), shared with + # the executor role and upgrade-database.yml. A refusal stops the rollout + # here, before the running executor is drained. - name: Grant the executor's required kernel capabilities - ansible.builtin.command: - argv: - - setcap - - "{{ executor_capabilities | join(',') }}=ep" - - "{{ payload_prefix }}/lib/debuglet/{{ deploy_version }}/bin/debuglet-executor" - when: executor_enable_bpf | default(true) | bool and not (executor_ambient_caps | default(false) | bool) + ansible.builtin.import_tasks: tasks/executor-capabilities.yml + vars: + executor_capabilities_binary: "{{ payload_prefix }}/lib/debuglet/{{ deploy_version }}/bin/debuglet-executor" + executor_capabilities_required: true - name: Refuse a release incompatible with the current schema ansible.builtin.command: diff --git a/deploy/ansible/tasks/executor-capabilities.yml b/deploy/ansible/tasks/executor-capabilities.yml new file mode 100644 index 00000000..da030b46 --- /dev/null +++ b/deploy/ansible/tasks/executor-capabilities.yml @@ -0,0 +1,70 @@ +--- +# Give one installed executor binary exactly executor_capabilities +# (group_vars/executors.yml says what each one is for). This is the only place +# the capabilities are granted: the executor role, rollout-executors.yml and +# upgrade-database.yml all import it, so an executor started by any of them +# runs with the same set. Without it the eBPF tagger and packet counter cannot +# load and probes leave untagged. +# +# setcap replaces whatever the file carried before. It runs only when getcap +# reports a different set, so a repeat run changes nothing, and in check mode +# getcap still runs while setcap does not. Nothing is granted when +# executor_enable_bpf is false. The unit's ambient set +# (executor_ambient_caps) does not replace the file capabilities: it is the +# same list, and verify.yml requires them on the binary either way. +# +# Inputs: +# executor_capabilities_binary the installed executable itself, not the +# link to it: file capabilities live on the +# executable +# executor_capabilities_required true: a refused setcap fails the host, for +# playbooks that are about to start that +# binary in place of a running executor. +# false (default): it is reported as a +# warning, for hosts (restricted containers, +# noexec/nosuid or non-xattr filesystems) +# that reject file capabilities outright +# +# Registers executor_setcap, which is changed when the capabilities were +# replaced: a running executor keeps the capabilities it started with, so the +# caller restarts it. + +- name: Read the executor binary's file capabilities + ansible.builtin.command: + argv: [getcap, "{{ executor_capabilities_binary }}"] + register: executor_getcap + changed_when: false + failed_when: false + check_mode: false + when: executor_enable_bpf | default(true) | bool + +# getcap prints "PATH cap_a,cap_b=ep" for a file that carries them. +- name: Grant exactly the executor's capabilities + ansible.builtin.command: + argv: + - setcap + - "{{ executor_capabilities | join(',') }}=ep" + - "{{ executor_capabilities_binary }}" + register: executor_setcap + failed_when: >- + executor_setcap.rc | default(0) != 0 + and executor_capabilities_required | default(false) | bool + changed_when: executor_setcap.rc | default(1) == 0 + when: + - executor_enable_bpf | default(true) | bool + - >- + ((executor_getcap.stdout | default('') | regex_search('\\s(\\S+)=ep\\s*$', '\\1') or ['']) | first).split(',') | sort + != executor_capabilities | sort + +- name: Warn when the capabilities could not be set + ansible.builtin.debug: + msg: >- + Could not set {{ executor_capabilities | join(',') }} on + {{ executor_capabilities_binary }} + ({{ executor_setcap.stderr | default('') | trim | default('setcap failed', true) }}). + This node will run without the eBPF egress tagger, so its probes + will carry no attribution tag and cannot be verified, and without + cap_net_raw it offers no ICMP. + when: + - executor_enable_bpf | default(true) | bool + - executor_setcap.rc | default(0) != 0 diff --git a/deploy/ansible/upgrade-database.yml b/deploy/ansible/upgrade-database.yml index 8fce1ba3..f88a0451 100644 --- a/deploy/ansible/upgrade-database.yml +++ b/deploy/ansible/upgrade-database.yml @@ -19,7 +19,10 @@ # upgrade_accept_data_loss=true, naming the data concerned; # 6. requires twice the size of the database and its -wal and -shm companions # to be free on the database's filesystem (one copy for the backup, one for -# the migration's own journal); +# the migration's own journal); on an executor it also gives the candidate +# binary exactly executor_capabilities (tasks/executor-capabilities.yml, +# the same task file the executor role uses), so the executor it starts in +# step 11 can load its eBPF programs; # 7. stops the service; 8. copies the database and its companions into a new # backup-TIMESTAMP directory next to it; 9. runs the candidate daemon's # -upgrade-database mode as the service user through runuser, so the @@ -313,6 +316,16 @@ and the service were not touched. quiet: true + # The candidate is started below in place of the running executor, + # so it needs exactly the capabilities the executor role grants; + # without them its eBPF packet counter and tagger cannot load. A + # refusal stops here, before the service or the database is touched. + - name: Grant the candidate executor its kernel capabilities + ansible.builtin.import_tasks: tasks/executor-capabilities.yml + vars: + executor_capabilities_binary: "{{ payload_prefix }}/lib/debuglet/{{ debuglet_release.version }}/bin/debuglet-executor" + executor_capabilities_required: true + - name: Name the backup directory ansible.builtin.set_fact: upgrade_backup_dir: "{{ upgrade_database | dirname }}/backup-{{ now(utc=true, fmt='%Y%m%dT%H%M%SZ') }}" diff --git a/deploy/test/test_rollout.py b/deploy/test/test_rollout.py index ad2942b9..ff10ac34 100644 --- a/deploy/test/test_rollout.py +++ b/deploy/test/test_rollout.py @@ -56,10 +56,20 @@ [ ! -f "$prefix/lib/debuglet/$version/bad-tree" ] || exit 18 mkdir -p "$prefix/lib/debuglet/$version/bin" "$prefix/lib/debuglet/$version/share/debuglet" "$prefix/bin" printf '{}\n' > "$prefix/lib/debuglet/$version/share/debuglet/manifest.json" -printf '#!/bin/sh\n[ ! -f "%s/incompatible" ]\n' "$ROLLOUT_FIXTURE" > "$prefix/lib/debuglet/$version/bin/debuglet-executor" +cp "$ROLLOUT_FIXTURE/executor-fixture" "$prefix/lib/debuglet/$version/bin/debuglet-executor" chmod 755 "$prefix/lib/debuglet/$version/bin/debuglet-executor" cp "$prefix/lib/debuglet/$version/bin/debuglet-executor" "$prefix/lib/debuglet/$version/bin/debuglet-dispatcher" ''' +# The packaged daemon: -check-database answers 3 (an upgrade is needed) while +# the fixture marks the schema outdated, and -upgrade-database is traced. +EXECUTOR = r'''#!/bin/sh +[ ! -f "$ROLLOUT_FIXTURE/incompatible" ] || exit 1 +case " $* " in + *" -check-database "*) [ ! -f "$ROLLOUT_FIXTURE/outdated" ] || { echo 'fixture schema outdated' >&2; exit 3; }; echo 'fixture schema current';; + *" -upgrade-database "*) echo upgrade >> "$ROLLOUT_FIXTURE/trace"; echo 'fixture schema upgraded';; +esac +''' +CAPABILITIES = 'cap_net_admin,cap_net_raw,cap_perfmon,cap_bpf' CLIENT = r'''#!PYTHON import json, os, sys from pathlib import Path @@ -93,6 +103,7 @@ def setUp(self): for name, code in [('systemctl', SYSTEMCTL), ('client', CLIENT)]: p = self.work / name p.write_text(code.replace('PYTHON', sys.executable, 1)); p.chmod(0o755) + (self.work / 'executor-fixture').write_text(EXECUTOR) Path('/run/systemd/system').mkdir(parents=True, exist_ok=True) (self.fixture.directory / 'install.sh').write_text(INSTALLER) sums = self.fixture.directory / 'SHA256SUMS' @@ -137,6 +148,26 @@ def run_rollout(self): cwd=self.playbooks, env=self.env, text=True, capture_output=True, timeout=180) return p, (self.work / 'trace').read_text().splitlines() if (self.work / 'trace').exists() else [] + def run_upgrade(self): + p = subprocess.run(['ansible-playbook', '-i', str(self.work / 'inventory.yml'), + '-e', '@' + str(self.work / 'settings.yml'), 'upgrade-database.yml'], + cwd=self.playbooks, env=self.env, text=True, capture_output=True, timeout=180) + return p, (self.work / 'trace').read_text().splitlines() if (self.work / 'trace').exists() else [] + + def enable_bpf(self, getcap='', setcap_rc=0): + """Turn the BPF path on with fixture getcap/setcap. getcap reports + getcap (empty: no capabilities); setcap traces its arguments.""" + settings = self.work / 'settings.yml' + value = yaml.safe_load(settings.read_text()); value['executor_enable_bpf'] = True + settings.write_text(yaml.safe_dump(value)) + report = '#!/bin/sh\n' + ('printf "%%s %s\\n" "$1"\n' % getcap if getcap else '') + 'exit 0\n' + trace = '#!/bin/sh\nprintf "setcap %%s %%s\\n" "$1" "$2" >> %s\nexit %d\n' % (self.work / 'trace', setcap_rc) + for name, code in [('getcap', report), ('setcap', trace)]: + path = self.work / name; path.write_text(code); path.chmod(0o755) + + def candidate(self, host): + return str(self.work / host / 'prefix/lib/debuglet' / VERSION / 'bin/debuglet-executor') + def test_serial_success_records_verified_backups_and_measurements(self): p, trace = self.run_rollout() self.assertEqual(p.returncode, 0, p.stdout + p.stderr) @@ -180,21 +211,66 @@ def test_existing_candidate_is_reverified_before_execution(self): self.assertFalse((self.work / 'second/prefix').exists()) def test_missing_required_capabilities_stop_before_drain(self): - settings = self.work / 'settings.yml' - value = yaml.safe_load(settings.read_text()); value['executor_enable_bpf'] = True - settings.write_text(yaml.safe_dump(value)) - setcap_args = self.work / 'setcap-args' - for name, code in [('getcap', '#!/bin/sh\nexit 0\n'), - ('setcap', '#!/bin/sh\nprintf "%%s\\n" "$1" >> %s\nexit 1\n' % setcap_args)]: - path = self.work / name; path.write_text(code); path.chmod(0o755) + self.enable_bpf(setcap_rc=1) p, trace = self.run_rollout() self.assertNotEqual(p.returncode, 0, p.stdout + p.stderr) - self.assertIn("Grant the executor's required kernel capabilities", p.stdout) - # Exactly the set the eBPF tagger and the pure-Go tagger need. - self.assertEqual(setcap_args.read_text().splitlines(), ['cap_net_admin,cap_net_raw,cap_perfmon,cap_bpf=ep']) - self.assertEqual(trace, []) + self.assertIn("Grant exactly the executor's capabilities", p.stdout) + # Exactly the set the eBPF tagger and the pure-Go tagger need, and + # nothing stopped. + self.assertEqual(trace, ['setcap %s=ep %s' % (CAPABILITIES, self.candidate('canary'))]) + self.assertFalse((self.work / 'second/prefix').exists()) + + def test_rollout_grants_capabilities_before_starting_each_host(self): + self.enable_bpf() + p, trace = self.run_rollout() + self.assertEqual(p.returncode, 0, p.stdout + p.stderr) + self.assertEqual([t for t in trace if not t.startswith('measure')], + ['setcap %s=ep %s' % (CAPABILITIES, self.candidate('canary')), + 'stop fixture-canary', 'start fixture-canary', + 'setcap %s=ep %s' % (CAPABILITIES, self.candidate('second')), + 'stop fixture-second', 'start fixture-second']) + + # upgrade-database.yml starts the candidate executor itself (#414): it must + # carry the role's capabilities before it is started. + def test_upgrade_database_grants_capabilities_before_starting_candidate(self): + (self.work / 'outdated').touch() + self.enable_bpf() + p, trace = self.run_upgrade() + self.assertEqual(p.returncode, 0, p.stdout + p.stderr) + self.assertEqual(trace, [ + 'setcap %s=ep %s' % (CAPABILITIES, self.candidate('canary')), + 'stop fixture-canary', 'upgrade', 'start fixture-canary', + 'setcap %s=ep %s' % (CAPABILITIES, self.candidate('second')), + 'stop fixture-second', 'upgrade', 'start fixture-second']) + for host in ('canary', 'second'): + self.assertEqual(os.readlink(self.work / host / 'prefix/bin/debuglet-executor'), self.candidate(host)) + + def test_upgrade_database_keeps_capabilities_already_granted(self): + (self.work / 'outdated').touch() + # getcap lists them in another order; the set is what counts. + self.enable_bpf(getcap='cap_bpf,cap_perfmon,cap_net_raw,cap_net_admin=ep') + p, trace = self.run_upgrade() + self.assertEqual(p.returncode, 0, p.stdout + p.stderr) + self.assertEqual(trace, ['stop fixture-canary', 'upgrade', 'start fixture-canary', + 'stop fixture-second', 'upgrade', 'start fixture-second']) + + def test_upgrade_database_refused_capabilities_leave_service_and_database(self): + (self.work / 'outdated').touch() + self.enable_bpf(setcap_rc=1) + p, trace = self.run_upgrade() + self.assertNotEqual(p.returncode, 0, p.stdout + p.stderr) + self.assertIn("Grant exactly the executor's capabilities", p.stdout) + self.assertEqual(trace, ['setcap %s=ep %s' % (CAPABILITIES, self.candidate('canary'))]) + self.assertEqual((self.work / 'canary/state/executor-dev/executor.db').read_text(), 'unchanged fixture state') self.assertFalse((self.work / 'second/prefix').exists()) + def test_upgrade_database_without_bpf_grants_nothing(self): + (self.work / 'outdated').touch() + p, trace = self.run_upgrade() + self.assertEqual(p.returncode, 0, p.stdout + p.stderr) + self.assertEqual(trace, ['stop fixture-canary', 'upgrade', 'start fixture-canary', + 'stop fixture-second', 'upgrade', 'start fixture-second']) + if __name__ == '__main__': unittest.main() From 4450cdf7f82e3a00588fde7911cba4b93aa6cbfe Mon Sep 17 00:00:00 2001 From: Yi-Min Lin Date: Wed, 7 Oct 2026 17:12:13 -0700 Subject: [PATCH 2/2] executor: fall back when the eBPF counter load fails (#414) In packet_counter = "auto" mode a load failure other than EPERM, EACCES or EINVAL was tagged ErrCleanupUnconfirmed, so the executor exited with "packet counter cleanup unconfirmed" and systemd restarted it in a loop. Without CAP_BPF, cilium/ebpf can report the map create as "prealloc maps not supported" (its refused feature probe), which carries no errno. A load attaches nothing: attach runs only after a successful load, and loading creates no TCX link or pin. Whatever descriptors cilium/ebpf may not have released count no packet and close with the process, so every load failure is now a clean rollback and the factory falls back. A failed attach whose release fails still refuses to fall back. When the process lacks CAP_BPF or CAP_NET_ADMIN (and CAP_SYS_ADMIN), the load error now names the missing capabilities and matches os.ErrPermission, so the fallback reason is not_permitted rather than a MEMLOCK or prealloc hint. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 21 +++ internal/executor/cleanup/errors.go | 8 +- internal/executor/ratelimit/ebpf/bpf_count.go | 30 +++-- .../executor/ratelimit/ebpf/bpf_count_test.go | 124 ++++++++++++++++-- .../ratelimit/ebpf/capabilities_linux.go | 89 +++++++++++++ .../executor/ratelimit/packetcount_test.go | 55 ++++---- 6 files changed, 276 insertions(+), 51 deletions(-) create mode 100644 internal/executor/ratelimit/ebpf/capabilities_linux.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 405cbf96..4f556a3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,27 @@ changes; the linked API and deployment documentation contains operational detail databases require the explicit upgrade to schema 26. See `docs/operations/configuration.md#destination-limits-and-opt-outs`. +### Fixed +- `deploy/ansible/upgrade-database.yml` gives the candidate executor binary + exactly `executor_capabilities` before it activates and starts it (#414). + It used to start the executor without CAP_BPF, CAP_PERFMON, CAP_NET_ADMIN + and CAP_NET_RAW until a full deployment followed. A host that refuses the + capabilities now stops the upgrade before the service or database is + touched. The executor role, `rollout-executors.yml` and + `upgrade-database.yml` now grant them through one task file + (`deploy/ansible/tasks/executor-capabilities.yml`). `rollout-executors.yml` + now also grants them when `executor_ambient_caps` is set, as the role and + `verify.yml` already expected, and skips `setcap` when the binary already + carries the set. +- With `packet_counter = "auto"`, an executor whose eBPF load fails falls + back to the userspace counter instead of exiting with `packet counter + cleanup unconfirmed` and being restarted in a loop (#414). A load attaches + nothing, so any load failure is a clean rollback. A failed attach whose + release cannot be confirmed still stops the executor. When the process + lacks CAP_BPF or CAP_NET_ADMIN, the logged error names the missing + capability rather than cilium/ebpf's "MEMLOCK may be too low" or "prealloc + maps not supported" hint, and the fallback reason is `not_permitted`. + ## [0.3.0-rc.1] - 2026-10-06 ### Security diff --git a/internal/executor/cleanup/errors.go b/internal/executor/cleanup/errors.go index 7426b269..eb732200 100644 --- a/internal/executor/cleanup/errors.go +++ b/internal/executor/cleanup/errors.go @@ -21,6 +21,10 @@ import "errors" // The returned error also retains each underlying release error for errors.Is. var ErrCleanupFailed = errors.New("packet counter cleanup failed") -// ErrCleanupUnconfirmed means the BPF loader failed and did not expose the result -// of its internally owned rollback. It does not assert that cleanup failed. +// ErrCleanupUnconfirmed means a constructor failed after acquiring a resource +// that could outlive its failure, such as an attachment, and could not observe +// whether its rollback released it. It does not assert that cleanup failed. +// A failed eBPF object load does not carry it: loading attaches nothing, so +// whatever the loader left unreleased cannot outlive the process or count a +// packet, and the factory falls back. var ErrCleanupUnconfirmed = errors.New("packet counter cleanup unconfirmed") diff --git a/internal/executor/ratelimit/ebpf/bpf_count.go b/internal/executor/ratelimit/ebpf/bpf_count.go index 3e08ff84..34be51d3 100644 --- a/internal/executor/ratelimit/ebpf/bpf_count.go +++ b/internal/executor/ratelimit/ebpf/bpf_count.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "github.com/netsec-ethz/debuglet/internal/bitrate" - "github.com/netsec-ethz/debuglet/internal/executor/cleanup" "github.com/netsec-ethz/debuglet/internal/executor/debuglet/socket/netutil" "github.com/netsec-ethz/debuglet/internal/executor/ratelimit/destinations" "io" @@ -53,15 +52,18 @@ func NewBPFCount(iface *net.Interface) (*BpfCount, error) { } return attached, err }, + missingCapabilities: missingCapabilities, }) } // Construction-fixed test seams model ownership transfer, not kernel behavior. // load transfers its resources only on success. attach transfers any returned // nonnil handle, including a handle returned alongside an error. +// missingCapabilities, when set, explains a failed load. type counterDependencies struct { - load func() (countObjects, []counterResource, error) - attach func(link.TCXOptions) (io.Closer, error) + load func() (countObjects, []counterResource, error) + attach func(link.TCXOptions) (io.Closer, error) + missingCapabilities func() ([]string, error) } func newBPFCount(iface *net.Interface, deps counterDependencies) (*BpfCount, error) { @@ -70,17 +72,17 @@ func newBPFCount(iface *net.Interface, deps counterDependencies) (*BpfCount, err } objs, resources, err := deps.load() if err != nil { - err = fmt.Errorf("failed to load eBPF objects: %w", err) - // EPERM, EACCES and EINVAL are what an unprivileged process sees, - // depending on the kernel and the failing step, and what a kernel - // that does not accept a program or map returns; the caller falls - // back on them. Any other errno is not such a refusal and stays fatal - // for the operator to look at. In every case cilium/ebpf closes the - // objects a failed load created. - if errors.Is(err, syscall.EPERM) || errors.Is(err, syscall.EACCES) || errors.Is(err, syscall.EINVAL) { - return nil, err - } - return nil, errors.Join(cleanup.ErrCleanupUnconfirmed, err) + // A failed load attached nothing, so it is a clean rollback for the + // caller's fallback decision whatever its cause: attach is reached + // only after a successful load, and loading creates no TCX link and, + // without pin options, nothing that outlives this process. What + // cilium/ebpf may not have released are map and program descriptors, + // which count no packet without an attachment and close with the + // process. Only a failed attach can leave a hook in place, and that + // rollback is observed below. The caller falls back and reports the + // cause; when the process lacks a required capability (#414) the + // error names it and reads as a permission failure. + return nil, explainLoadFailure(fmt.Errorf("failed to load eBPF objects: %w", err), deps.missingCapabilities) } bc := &BpfCount{objs: objs, cleanup: counterCleanup{resources: resources}, interfaceIndex: uint32(iface.Index)} diff --git a/internal/executor/ratelimit/ebpf/bpf_count_test.go b/internal/executor/ratelimit/ebpf/bpf_count_test.go index a4d99eb9..2dc63f4a 100644 --- a/internal/executor/ratelimit/ebpf/bpf_count_test.go +++ b/internal/executor/ratelimit/ebpf/bpf_count_test.go @@ -7,6 +7,8 @@ import ( "fmt" "io" "net" + "os" + "strings" "sync" "sync/atomic" "syscall" @@ -17,6 +19,7 @@ import ( "github.com/cilium/ebpf/link" "github.com/google/uuid" "github.com/netsec-ethz/debuglet/internal/executor/cleanup" + "golang.org/x/sys/unix" ) func TestCounterObjectOwnershipList(t *testing.T) { @@ -262,25 +265,29 @@ func TestCounterLoadFailureDoesNotRepeatLibraryCleanup(t *testing.T) { }, attach: func(link.TCXOptions) (io.Closer, error) { attached = true; return nil, nil }, }) - if count != nil || !errors.Is(err, loadErr) || !errors.Is(err, cleanup.ErrCleanupUnconfirmed) { + if count != nil || !errors.Is(err, loadErr) { t.Fatalf("load failure=%v/%v", count, err) } - if errors.Is(err, cleanup.ErrCleanupFailed) || partial.calls.Load() != 1 || attached { + if errors.Is(err, cleanup.ErrCleanupFailed) || errors.Is(err, cleanup.ErrCleanupUnconfirmed) || partial.calls.Load() != 1 || attached { t.Fatal("library rollback was repeated or its unobserved result invented") } } -func TestCounterLoadRefusalIsPlainError(t *testing.T) { +// A load attaches nothing, so every load failure is a clean rollback the +// factory falls back from, including the ones that are not a refusal errno: +// on a kernel 7.0 host without CAP_BPF the map create surfaced as cilium's +// "prealloc maps not supported" feature-probe error (#414). +func TestCounterLoadFailureIsCleanRollback(t *testing.T) { for _, tc := range []struct { - name string - cause error - refused bool + name string + cause error }{ - {"eperm", syscall.EPERM, true}, - {"eacces", syscall.EACCES, true}, - {"einval", syscall.EINVAL, true}, - {"other_errno", syscall.ENOMEM, false}, - {"no_errno", errors.New("malformed object"), false}, + {"eperm", syscall.EPERM}, + {"eacces", syscall.EACCES}, + {"einval", syscall.EINVAL}, + {"other_errno", syscall.ENOMEM}, + {"unsupported_feature", fmt.Errorf("prealloc maps not supported (requires >= v4.6): %w", ebpf.ErrNotSupported)}, + {"no_errno", errors.New("malformed object")}, } { t.Run(tc.name, func(t *testing.T) { var attached bool @@ -293,13 +300,104 @@ func TestCounterLoadRefusalIsPlainError(t *testing.T) { if count != nil || !errors.Is(err, tc.cause) || attached { t.Fatalf("load failure=%v/%v attached=%t", count, err, attached) } - if errors.Is(err, cleanup.ErrCleanupFailed) || errors.Is(err, cleanup.ErrCleanupUnconfirmed) == tc.refused { - t.Fatalf("refused=%t classified as %v", tc.refused, err) + if errors.Is(err, cleanup.ErrCleanupFailed) || errors.Is(err, cleanup.ErrCleanupUnconfirmed) { + t.Fatalf("attachment-free load failure classified as %v", err) } }) } } +// An attach that returned a handle whose release then failed may leave the +// hook in place, so it stays a refusal to fall back, unlike a load failure. +func TestCounterAttachRollbackFailureStaysFatal(t *testing.T) { + resources, _ := counterTestObjects() + attachErr, linkErr := errors.New("attach"), errors.New("link close") + egress := &counterProbe{err: linkErr} + count, err := newBPFCount(&net.Interface{Index: 1}, counterDependencies{ + load: func() (countObjects, []counterResource, error) { return countObjects{}, resources, nil }, + attach: func(link.TCXOptions) (io.Closer, error) { return egress, attachErr }, + missingCapabilities: func() ([]string, error) { + t.Error("attach failure explained as a load failure") + return nil, nil + }, + }) + if count != nil || !errors.Is(err, attachErr) || !errors.Is(err, linkErr) || !errors.Is(err, cleanup.ErrCleanupFailed) { + t.Fatalf("attach rollback failure=%v/%v", count, err) + } +} + +func TestCounterLoadFailureNamesMissingCapabilities(t *testing.T) { + // cilium/ebpf's own text for an unprivileged map create. + loadErr := fmt.Errorf("map create: %w (MEMLOCK may be too low, consider rlimit.RemoveMemlock)", syscall.EPERM) + misleading := fmt.Errorf("map create: prealloc maps not supported (requires >= v4.6): %w", ebpf.ErrNotSupported) + for _, tc := range []struct { + name string + cause error + missing []string + capErr error + want string + }{ + {"eperm_without_caps", loadErr, []string{"CAP_BPF", "CAP_NET_ADMIN"}, nil, "executor lacks CAP_BPF, CAP_NET_ADMIN"}, + {"feature_probe_without_caps", misleading, []string{"CAP_BPF"}, nil, "executor lacks CAP_BPF"}, + {"with_caps", misleading, nil, nil, ""}, + {"caps_unreadable", misleading, nil, syscall.ENOSYS, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := newBPFCount(&net.Interface{Index: 1}, counterDependencies{ + load: func() (countObjects, []counterResource, error) { return countObjects{}, nil, tc.cause }, + missingCapabilities: func() ([]string, error) { + return tc.missing, tc.capErr + }, + }) + if !errors.Is(err, tc.cause) || errors.Is(err, cleanup.ErrCleanupUnconfirmed) { + t.Fatalf("lost cause or refused fallback: %v", err) + } + if tc.want == "" { + if strings.Contains(err.Error(), "executor lacks") || (errors.Is(err, os.ErrPermission) && !errors.Is(tc.cause, syscall.EPERM)) { + t.Fatalf("capabilities blamed without being missing: %v", err) + } + return + } + if !strings.HasPrefix(err.Error(), tc.want+" (") || !strings.Contains(err.Error(), "setcap") { + t.Fatalf("error does not name the missing capabilities: %v", err) + } + // It reads as not_permitted to the fallback reason. + if !errors.Is(err, os.ErrPermission) { + t.Fatalf("missing capability is not a permission failure: %v", err) + } + }) + } +} + +func TestMissingLoadCapabilities(t *testing.T) { + bit := func(caps ...uint) (set uint64) { + for _, c := range caps { + set |= 1 << c + } + return set + } + for _, tc := range []struct { + name string + effective uint64 + want string + }{ + {"none", 0, "CAP_BPF,CAP_NET_ADMIN"}, + {"bpf_only", bit(unix.CAP_BPF), "CAP_NET_ADMIN"}, + {"perfmon_is_not_needed", bit(unix.CAP_BPF, unix.CAP_NET_ADMIN), ""}, + {"executor_capabilities", bit(unix.CAP_BPF, unix.CAP_NET_ADMIN, unix.CAP_PERFMON, unix.CAP_NET_RAW), ""}, + {"sys_admin", bit(unix.CAP_SYS_ADMIN), ""}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := strings.Join(missingFrom(tc.effective), ","); got != tc.want { + t.Fatalf("missing=%q want %q", got, tc.want) + } + }) + } + if _, err := missingCapabilities(); err != nil { + t.Fatalf("read own capabilities: %v", err) + } +} + func TestCounterNilInterfaceDoesNotAcquireResources(t *testing.T) { var loaded bool count, err := newBPFCount(nil, counterDependencies{ diff --git a/internal/executor/ratelimit/ebpf/capabilities_linux.go b/internal/executor/ratelimit/ebpf/capabilities_linux.go new file mode 100644 index 00000000..916a0acf --- /dev/null +++ b/internal/executor/ratelimit/ebpf/capabilities_linux.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 ETH Zurich + +//go:build linux + +package ebpf + +import ( + "errors" + "os" + "strings" + + "golang.org/x/sys/unix" +) + +// loadCapabilities are the capabilities the kernel checks when the counter's +// maps and programs are loaded: CAP_BPF for map create and program load, and +// CAP_NET_ADMIN for a traffic-control program. CAP_SYS_ADMIN stands in for +// both, and is the only way to hold them before Linux 5.8 added CAP_BPF. +// CAP_PERFMON is left out: the tagger's verifier needs it, not this load, so +// naming it here could blame a load failure on it wrongly. +var loadCapabilities = []struct { + name string + bit uint +}{ + {"CAP_BPF", unix.CAP_BPF}, + {"CAP_NET_ADMIN", unix.CAP_NET_ADMIN}, +} + +// missingCapabilities names the load capabilities absent from this +// process's effective set, in loadCapabilities order. +func missingCapabilities() ([]string, error) { + header := unix.CapUserHeader{Version: unix.LINUX_CAPABILITY_VERSION_3} + var data [2]unix.CapUserData + if err := unix.Capget(&header, &data[0]); err != nil { + return nil, err + } + effective := uint64(data[1].Effective)<<32 | uint64(data[0].Effective) + return missingFrom(effective), nil +} + +func missingFrom(effective uint64) []string { + if effective&(1<= v4.6): %w", errors.ErrUnsupported), FallbackUnsupported}, } { t.Run(tc.name, func(t *testing.T) { core, logs := observer.New(zapcore.DebugLevel) count, err := newPacketCount(&net.Interface{Index: 8}, zap.New(core), func(*net.Interface) (PacketCount, error) { - loadErr := fmt.Errorf("failed to load eBPF objects: map create: %w", tc.cause) - if tc.fatal { - return nil, errors.Join(cleanup.ErrCleanupUnconfirmed, loadErr) - } - return nil, loadErr + return nil, fmt.Errorf("failed to load eBPF objects: map create: %w", tc.cause) }) - if tc.fatal { - if count != nil || !errors.Is(err, tc.cause) || logs.Len() != 0 { - t.Fatalf("unrefused load fell back or lost cause: %v/%v, logs=%d", count, err, logs.Len()) - } - return - } if err != nil || count == nil || count.Type() != "fallback" { - t.Fatalf("refused load fallback=%v/%v", count, err) + t.Fatalf("load failure fallback=%v/%v", count, err) } t.Cleanup(func() { _ = count.Close() }) + if got := FallbackReason(count); got != tc.reason { + t.Errorf("fallback reason=%q want %q", got, tc.reason) + } warnings := logs.FilterLevelExact(zapcore.WarnLevel).All() if logs.Len() != 1 || len(warnings) != 1 || !strings.Contains(fmt.Sprint(warnings[0].ContextMap()["error"]), tc.cause.Error()) { t.Fatalf("fallback logs=%v", logs.All()) @@ -95,6 +89,23 @@ func TestPacketCounterLoadRefusalFallsBack(t *testing.T) { } } +// A failure that may have left an attachment in place never falls back, even +// when it also reports a load-like cause. +func TestPacketCounterUnconfirmedAttachmentStaysFatal(t *testing.T) { + for _, marker := range []error{cleanup.ErrCleanupFailed, cleanup.ErrCleanupUnconfirmed} { + t.Run(marker.Error(), func(t *testing.T) { + core, logs := observer.New(zapcore.DebugLevel) + cause := fmt.Errorf("failed to attach egress TCX: %w", syscall.EPERM) + count, err := newPacketCount(&net.Interface{Index: 8}, zap.New(core), func(*net.Interface) (PacketCount, error) { + return nil, errors.Join(cause, marker) + }) + if count != nil || !errors.Is(err, marker) || !errors.Is(err, syscall.EPERM) || logs.Len() != 0 { + t.Fatalf("possible attachment fell back: %v/%v, logs=%d", count, err, logs.Len()) + } + }) + } +} + func TestPacketCounterNewWithoutPrivilegeFallsBack(t *testing.T) { iface, err := net.InterfaceByName("lo") if err != nil {