nixos/containers: add oci-seccomp-bpf-hook #96761

saschagrunert · 2020-08-31T12:37:23Z

Motivation for this change

Add the OCI seccomp bpf hook per default to the contianers.conf file. The hook will result in a no-op if the annotation is not present.

Things done

cc @NixOS/podman

zowoq

Requesting changes for now as this is a large dependency to add. (kernel, etc)

zowoq · 2020-08-31T12:44:05Z

The 20.09 branch off is very soon, I'd like to leave this until after. We can backport it if we do go ahead with it.

nixos/modules/virtualisation/containers.nix

saschagrunert · 2020-08-31T12:45:41Z

The 20.09 branch off is very soon, I'd like to leave this until after. We can backport it if we do go ahead with it.

Sounds good. Do we wanna make it optional?

I'm still not sure if we need something like programs.bcc.enable = true;, but I tested it in nixos-shell and the current patch worked there.

zowoq · 2020-08-31T22:12:10Z

programs.bcc uses config.boot.kernelPackages.bcc so it uses whatever the system kernel is set to, this hook will always depend on the default kernel, does that make a difference?

saschagrunert · 2020-09-01T07:22:01Z

programs.bcc uses config.boot.kernelPackages.bcc so it uses whatever the system kernel is set to, this hook will always depend on the default kernel, does that make a difference?

Yes I think so. The hook has to choose the same kernel sources like present on the system. I switched to the latest 5.8 kernel and applied a small patch to the hook:

diff --git a/oci-seccomp-bpf-hook.go b/oci-seccomp-bpf-hook.go
index 416c6f6..6d075d9 100644
--- a/oci-seccomp-bpf-hook.go
+++ b/oci-seccomp-bpf-hook.go
@@ -10,7 +10,6 @@ import (
 	"io/ioutil"
 	"log/syslog"
 	"os"
-	"os/exec"
 	"os/signal"
 	"path/filepath"
 	"runtime"
@@ -20,6 +19,7 @@ import (
 	"sync"
 	"syscall"
 	"time"
+	"unsafe"
 
 	"github.com/iovisor/gobpf/bcc"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
@@ -30,6 +30,12 @@ import (
 	logrus_syslog "github.com/sirupsen/logrus/hooks/syslog"
 )
 
+/*
+#cgo LDFLAGS: -lbcc
+#include <bcc/bcc_common.h>
+*/
+import "C"
+
 const (
 	// BPFTimeout is the timeout in seconds to wait for the child process to signal
 	// that the eBPF program finished compiling and attached to the tracee.
@@ -95,23 +101,25 @@ func main() {
 	}
 }
 
-// modprobe the specified module.
-func modprobe(module string) error {
-	bin, err := exec.LookPath("modprobe")
-	if err != nil {
-		// Fallback to `/usr/sbin/modprobe`.  The environment may be
-		// empty.  If that doesn't exist either, we'll fail below.
-		bin = "/usr/sbin/modprobe"
+func initBPF() error {
+	logrus.Info("Initializing BFP")
+	cs := C.CString("")
+	defer C.free(unsafe.Pointer(cs))
+
+	c := C.bpf_module_create_c_from_string(cs, 2, nil, 0, true, nil)
+	if c == nil {
+		return errors.New("building BPF module from string failed")
+
 	}
-	return exec.Command(bin, module).Run()
+	logrus.Info("BFP up and running")
+	return nil
 }
 
 // detachAndTrace re-executes the current executable to "fork" in go-ish way and
 // traces the provided PID.
 func detachAndTrace() error {
-	logrus.Info("Trying to load `kheaders` module")
-	if err := modprobe("kheaders"); err != nil {
-		logrus.Infof("Loading `kheaders` failed, continuing in hope kernel headers reside on disk: %v", err)
+	if err := initBPF(); err != nil {
+		return errors.Wrap(err, "init BPF")
 	}
 
 	// Read the State spec from stdin and unmarshal it.

Then running the hook should build an example BPF module which automatically loads the kernel headers, but:

Error: chdir(/nix/store/6wccjk70rnw61jy2xfn357v6aljy394v-linux-5.7.18-dev/lib/modules/5.8.4/build):

The funny part is now, this issue only happens if the hook gets executed through Podman. If I run the hook manually (with sudo on my current user), then it seems to pick the right path and every further invocation of the hook (even through podman) will work from now on.

I applied a small patch

zowoq · 2020-09-01T07:49:14Z

@saschagrunert Can you try #96892 please?

saschagrunert · 2020-09-01T08:54:49Z

Requires now #96892

zowoq · 2020-09-01T09:54:06Z

Sorry, I focused on the programs.bcc part and didn't comment on this:

Do we wanna make it optional?

I'd be in favour of making it optional.

saschagrunert · 2020-09-01T10:51:21Z

Sorry, I focused on the programs.bcc part and didn't comment on this:

Do we wanna make it optional?

I'd be in favour of making it optional.

Sounds good, changed the implementation to only modify the config if the hook is enabled.

nixos/modules/virtualisation/containers.nix

saschagrunert · 2020-09-02T10:29:36Z

Rebased on top of the latest master to resolve the issues.

zowoq · 2020-09-02T10:46:39Z

Another nit, sorry. Could you flip the enable around please?

diff --git a/nixos/modules/virtualisation/containers.nix b/nixos/modules/virtualisation/containers.nix
index b82f64ea05f..de97ba3f7bb 100644
--- a/nixos/modules/virtualisation/containers.nix
+++ b/nixos/modules/virtualisation/containers.nix
@@ -43,7 +43,7 @@ in
         '';
       };
 
-    enableOciSeccompBpfHook = mkOption {
+    ociSeccompBpfHook.enable = mkOption {
       type = types.bool;
       default = false;
       description = "Enable the OCI seccomp BPF hook";
@@ -122,7 +122,7 @@ in
       [network]
       cni_plugin_dirs = ["${pkgs.cni-plugins}/bin/"]
 
-      ${lib.optionalString (cfg.enableOciSeccompBpfHook == true) ''
+      ${lib.optionalString (cfg.ociSeccompBpfHook.enable == true) ''
       [engine]
       hooks_dir = [
         "${config.boot.kernelPackages.oci-seccomp-bpf-hook}",

Signed-off-by: Sascha Grunert <sgrunert@suse.com>

saschagrunert · 2020-09-02T10:54:03Z

Another nit, sorry. Could you flip the enable around please?

Sure thing, fixed as suggested 👍

zowoq · 2020-09-02T11:53:19Z

As it's now optional and using the system kernel I think it's fine to include in the next release.

zowoq

Thanks!

saschagrunert requested a review from zowoq as a code owner August 31, 2020 12:37

zowoq requested changes Aug 31, 2020

View reviewed changes

zowoq reviewed Aug 31, 2020

View reviewed changes

nixos/modules/virtualisation/containers.nix Outdated Show resolved Hide resolved

ofborg bot added 6.topic: nixos 8.has: module (update) labels Aug 31, 2020

saschagrunert force-pushed the hook branch from bf83bfb to 3afabb5 Compare August 31, 2020 12:51

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Aug 31, 2020

saschagrunert mentioned this pull request Sep 1, 2020

oci-seccomp-bpf-hook: move to linuxPackages #96892

Merged

10 tasks

saschagrunert force-pushed the hook branch from 3afabb5 to fabcf48 Compare September 1, 2020 08:54

saschagrunert changed the title ~~containers: add oci-seccomp-bpf-hook~~ nixos/containers: add oci-seccomp-bpf-hook Sep 1, 2020

saschagrunert force-pushed the hook branch from fabcf48 to 2ab403f Compare September 1, 2020 10:50

ofborg bot added 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-linux: 0 labels Sep 1, 2020

zowoq reviewed Sep 1, 2020

View reviewed changes

nixos/modules/virtualisation/containers.nix Outdated Show resolved Hide resolved

saschagrunert force-pushed the hook branch 2 times, most recently from 475438d to 9c7c12a Compare September 2, 2020 10:28

nixos/containers: add oci-seccomp-bpf-hook

7e7b68f

Signed-off-by: Sascha Grunert <sgrunert@suse.com>

saschagrunert force-pushed the hook branch from 9c7c12a to 7e7b68f Compare September 2, 2020 10:53

zowoq approved these changes Sep 2, 2020

View reviewed changes

zowoq merged commit 27b0c4b into NixOS:master Sep 2, 2020

saschagrunert deleted the hook branch September 2, 2020 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/containers: add oci-seccomp-bpf-hook #96761

nixos/containers: add oci-seccomp-bpf-hook #96761

saschagrunert commented Aug 31, 2020 •

edited

zowoq left a comment •

edited

zowoq commented Aug 31, 2020

saschagrunert commented Aug 31, 2020

zowoq commented Aug 31, 2020

saschagrunert commented Sep 1, 2020 •

edited

zowoq commented Sep 1, 2020

saschagrunert commented Sep 1, 2020

zowoq commented Sep 1, 2020

saschagrunert commented Sep 1, 2020

saschagrunert commented Sep 2, 2020

zowoq commented Sep 2, 2020

saschagrunert commented Sep 2, 2020

zowoq commented Sep 2, 2020 •

edited

zowoq left a comment

nixos/containers: add oci-seccomp-bpf-hook #96761

nixos/containers: add oci-seccomp-bpf-hook #96761

Conversation

saschagrunert commented Aug 31, 2020 • edited

Motivation for this change

Things done

zowoq left a comment • edited

Choose a reason for hiding this comment

zowoq commented Aug 31, 2020

saschagrunert commented Aug 31, 2020

zowoq commented Aug 31, 2020

saschagrunert commented Sep 1, 2020 • edited

zowoq commented Sep 1, 2020

saschagrunert commented Sep 1, 2020

zowoq commented Sep 1, 2020

saschagrunert commented Sep 1, 2020

saschagrunert commented Sep 2, 2020

zowoq commented Sep 2, 2020

saschagrunert commented Sep 2, 2020

zowoq commented Sep 2, 2020 • edited

zowoq left a comment

Choose a reason for hiding this comment

saschagrunert commented Aug 31, 2020 •

edited

zowoq left a comment •

edited

saschagrunert commented Sep 1, 2020 •

edited

zowoq commented Sep 2, 2020 •

edited