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/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() 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 {