Container: BPF token support #15009

mihalicyn · 2025-02-17T15:47:47Z

Good description at https://docs.ebpf.io/linux/concepts/token/

Linux kernel 6.9+ is required for testing

Manual test scenario:

lxc launch ubuntu:oracular o
lxc config set o security.delegate_bpf=true
lxc config set o security.delegate_bpf.progs=socket_filter
lxc shell o
git clone https://github.com/libbpf/libbpf-bootstrap.git
cd libbpf-bootstrap/examples/c
make
./sockfilter
# in another terminal
ping -c 4 localhost

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

simondeziel · 2025-02-18T17:14:44Z

doc/metadata.txt

+
+```
+
+```{config:option} security.delegate_bpf.attachs instance-security


Suggested change

```{config:option} security.delegate_bpf.attachs instance-security

```{config:option} security.delegate_bpf.attaches instance-security

Not sure about this one but I think it needs an e in the plural form.

Actually, I'm just following the naming from the kernel https://github.com/torvalds/linux/blob/2408a807bfc3f738850ef5ad5e3fd59d66168996/kernel/bpf/inode.c#L813

But I agree that attaches looks better :) So let's decide what we want to do here.

Following the kernel convention feels like less friction, thanks for sharing.

or "attach_types"?

attach_types seems best!

simondeziel · 2025-02-18T17:16:33Z

lxd/instance/drivers/driver_lxc.go

@@ -1071,6 +1071,13 @@ func (d *lxc) initLXC(config bool) (*liblxc.Container, error) {
 		}
 	}

+	if shared.IsTrue(d.expandedConfig["security.delegate_bpf"]) {
+		err = lxcSetConfigItem(cc, "lxc.hook.start-host", fmt.Sprintf("%s callhook %s %s %s starthost", d.state.OS.ExecPath, shared.VarPath(""), strconv.Quote(d.Project().Name), strconv.Quote(d.Name())))


I'd like that fmt.Sprintf() to be replaced by string concatenation but I understand you might want to leave it as-is as there are other similar line in that file ;)

simondeziel · 2025-02-18T17:21:02Z

shared/validate/validate_cgo.go

+// IsBpfDelegateOption validates a BPF Token delegation option.
+func IsBpfDelegateOption(delegateOption string) func(value string) error {
+	return func(value string) error {
+		cdelegateOption := C.CString(fmt.Sprintf("delegate_%s", delegateOption))


Suggested change

cdelegateOption := C.CString(fmt.Sprintf("delegate_%s", delegateOption))

cdelegateOption := C.CString("delegate_"+delegateOption)

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

… options Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

tomponline · 2025-02-20T11:36:47Z

lxd-user/callhook/callhook.go

@@ -73,6 +73,15 @@ func HandleContainerHook(lxdPath string, projectName string, instanceRef string,
 	u := api.NewURL().Path("internal", "containers", instanceRef, "on"+hook)
 	u.WithQuery("target", target)

+	if hook == "starthost" {
+		lxcPID := os.Getenv("LXC_PID")


Can we lose this validation and instead still call the starthost URL with a missing or blank lxc_pid query parameter, then let LXD validate that its missing, as the error will then be more accessible in the LXD logs rather than in the lxc debug log.

tomponline · 2025-02-20T11:41:33Z

lxd/instance/drivers/driver_lxc.go

+// mountBpfFs mounts bpffs inside the container.
+func (d *lxc) mountBpfFs(pid int, bpffsParams map[string]string) error {
+	if !d.state.OS.BPFToken {
+		return fmt.Errorf("BPF Token mechanism is not supported by kernel running.")


No full stops on log messages or errors please

tomponline · 2025-02-28T08:22:53Z

lxd/instance/drivers/driver_lxc.go

+		bpffsParams["delegate_progs"],
+		bpffsParams["delegate_attachs"])
+	if err != nil {
+		d.logger.Error("bpffs mount helper has failed", logger.Ctx{"err": err, "stdout": stdout})


is there potentially useful info in stdout we should be collecting and passing to the end user in err?

tomponline · 2025-02-28T08:24:52Z

lxd/instance/drivers/driver_lxc.go

+// onStartHost implements the LXC start-host hook.
+func (d *lxc) onStartHost(args map[string]string) error {
+	if shared.IsFalseOrEmpty(d.expandedConfig["security.delegate_bpf"]) {
+		return nil


Lets move the contents of his function from line 2580 into its own function, suggest d.onStartHostBPFDelegate() or similar, and then reverse this if statement's logic to call that function if shared.IsTrue(d.expandedConfig["security.delegate_bpf"]).

That way we nicely structure the general onStartHost function with the BPF delegation logic, allowing for easier future expansion for other purposes.

tomponline · 2025-02-28T08:25:31Z

lxd/project/limits/permissions.go

@@ -866,6 +866,10 @@ func isContainerLowLevelOptionForbidden(key string) bool {
 		return true
 	}

+	if strings.HasPrefix(key, "security.delegate_bpf") {


good thinking!

tomponline · 2025-02-28T08:26:47Z

shared/validate/validate_cgo.go

+)
+
+// IsBpfDelegateOption validates a BPF Token delegation option.
+func IsBpfDelegateOption(delegateOption string) func(value string) error {


Go's style guide recommends using upper case acronyms in exported functions, so suggest calling this IsBPFDelegationOption - there may be other places in this PR where the same change should occur too.

tomponline · 2025-02-28T08:27:06Z

shared/validate/validate_no_cgo.go

+// IsBpfDelegateOption validates a BPF Token delegation option.
+func IsBpfDelegateOption(delegateOption string) func(value string) error {
+	return func(value string) error {
+		return fmt.Errorf("This should never be called.")


no full stop in error

tomponline

LGTM, a few minor suggestions though.

Thanks!

tomponline · 2025-02-28T08:28:39Z

@mihalicyn we should also expand our docs to explain this feature and give an example of how to use it.

tomponline · 2025-02-28T08:29:57Z

doc/api-extensions.md

@@ -2618,3 +2618,7 @@ Note that the `openid` and `email` scopes are always required.
 ## `project_default_network_and_storage`

 Adds flags --network and --storage. The --network flag adds a network device connected to the specified network to the default profile. The --storage flag adds a root disk device using the specified storage pool to the default profile.
+
+## `bpf_delegation`


As this is container specific, suggest container_bpf_delegation, similar to container_syscall_intercept_bpf_devices that exists today.

github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Feb 17, 2025

mihalicyn requested a review from tomponline February 17, 2025 15:48

mihalicyn force-pushed the bpf_tokens_support branch from fadb3b9 to a2a0bd1 Compare February 18, 2025 13:54

lxd/main_checkfeature: add bpf_token kernel feature detection logic

0832aec

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn force-pushed the bpf_tokens_support branch from a2a0bd1 to cc5f336 Compare February 18, 2025 17:02

mihalicyn marked this pull request as ready for review February 18, 2025 17:03

simondeziel reviewed Feb 18, 2025

View reviewed changes

mihalicyn added 7 commits February 18, 2025 18:29

lxd/main_forkmount: bpffs mount with delegation support

188276b

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

lxd/api_internal: add start-host instance hook

7bbd0f5

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

lxd/instance/lxc: add security.delegate_bpf support

253082b

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

lxd/instance/instancetype/instance: add security.delegate_bpf* config…

db9f6da

… options Signed-off-by: Alexander Mikhalitsyn <[email protected]>

api: add bpf_delegation API extension

77eb3fa

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

doc: Update metadata

ea4a5d1

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

lxd/main_forkmount: Fix linter error

d495777

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

mihalicyn force-pushed the bpf_tokens_support branch from 1e28d4f to d495777 Compare February 18, 2025 17:29

tomponline reviewed Feb 20, 2025

View reviewed changes

tomponline reviewed Feb 28, 2025

View reviewed changes

tomponline requested changes Feb 28, 2025

View reviewed changes

tomponline changed the title ~~bpf token support~~ Container: BPF token support Feb 28, 2025

tomponline reviewed Feb 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Container: BPF token support #15009

Container: BPF token support #15009

mihalicyn commented Feb 17, 2025 •

edited

Loading

simondeziel Feb 18, 2025

mihalicyn Feb 18, 2025

simondeziel Feb 18, 2025

tomponline Feb 19, 2025

minaelee Feb 19, 2025

simondeziel Feb 18, 2025

simondeziel Feb 18, 2025

tomponline Feb 20, 2025

tomponline Feb 20, 2025

tomponline Feb 28, 2025

tomponline Feb 28, 2025

tomponline Feb 28, 2025

tomponline Feb 28, 2025

tomponline Feb 28, 2025

tomponline left a comment

tomponline commented Feb 28, 2025

tomponline Feb 28, 2025


		```

		```{config:option} security.delegate_bpf.attachs instance-security

	cdelegateOption := C.CString(fmt.Sprintf("delegate_%s", delegateOption))
	cdelegateOption := C.CString("delegate_"+delegateOption)

Container: BPF token support #15009

Are you sure you want to change the base?

Container: BPF token support #15009

Conversation

mihalicyn commented Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomponline left a comment

Choose a reason for hiding this comment

tomponline commented Feb 28, 2025

Choose a reason for hiding this comment

mihalicyn commented Feb 17, 2025 •

edited

Loading