helper/seccomp: improve error handling

This passes both errno and libseccomp return value.

Signed-off-by: Ophestra <cat@gensokyo.uk>

5 files changed, 142 insertions, 50 deletions

diff --git a/helper/seccomp/export_test.go b/helper/seccomp/export_test.go
index 40f81c7a..ff6f250e 100644
--- a/helper/seccomp/export_test.go
+++ b/helper/seccomp/export_test.go
@@ -115,7 +115,7 @@ func TestExport(t *testing.T) {
 			t.Errorf("Read: error = %v", err)
 			return
 		}
-		if err := e.Close(); err == nil || err.Error() != "seccomp_export_bpf failed: operation canceled" {
+		if err := e.Close(); err == nil || !errors.Is(err, syscall.ECANCELED) || !errors.Is(err, syscall.EBADF) {
 			t.Errorf("Close: error = %v", err)
 			return
 		}
diff --git a/helper/seccomp/seccomp-build.c b/helper/seccomp/seccomp-build.c
index 827d3b8c..4b2114f4 100644
--- a/helper/seccomp/seccomp-build.c
+++ b/helper/seccomp/seccomp-build.c
@@ -27,28 +27,27 @@ struct f_syscall_act {
 
 #define LEN(arr) (sizeof(arr) / sizeof((arr)[0]))
 
-#define SECCOMP_RULESET_ADD(ruleset) do {                                                                      \
-  if (opts & F_VERBOSE) F_println("adding seccomp ruleset \"" #ruleset "\"");                                  \
-  for (int i = 0; i < LEN(ruleset); i++) {                                                                     \
-    assert(ruleset[i].m_errno == EPERM || ruleset[i].m_errno == ENOSYS);                                       \
-                                                                                                               \
-    if (ruleset[i].arg)                                                                                        \
-      ret = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ruleset[i].m_errno), ruleset[i].syscall, 1, *ruleset[i].arg); \
-    else                                                                                                       \
-      ret = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ruleset[i].m_errno), ruleset[i].syscall, 0);                  \
-                                                                                                               \
-    if (ret == -EFAULT) {                                                                                      \
-      res = 4;                                                                                                 \
-      goto out;                                                                                                \
-    } else if (ret < 0) {                                                                                      \
-      res = 5;                                                                                                 \
-      errno = -ret;                                                                                            \
-      goto out;                                                                                                \
-    }                                                                                                          \
-  }                                                                                                            \
+#define SECCOMP_RULESET_ADD(ruleset) do {                                                                         \
+  if (opts & F_VERBOSE) F_println("adding seccomp ruleset \"" #ruleset "\"");                                     \
+  for (int i = 0; i < LEN(ruleset); i++) {                                                                        \
+    assert(ruleset[i].m_errno == EPERM || ruleset[i].m_errno == ENOSYS);                                          \
+                                                                                                                  \
+    if (ruleset[i].arg)                                                                                           \
+      *ret_p = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ruleset[i].m_errno), ruleset[i].syscall, 1, *ruleset[i].arg); \
+    else                                                                                                          \
+      *ret_p = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ruleset[i].m_errno), ruleset[i].syscall, 0);                  \
+                                                                                                                  \
+    if (*ret_p == -EFAULT) {                                                                                      \
+      res = 4;                                                                                                    \
+      goto out;                                                                                                   \
+    } else if (*ret_p < 0) {                                                                                      \
+      res = 5;                                                                                                    \
+      goto out;                                                                                                   \
+    }                                                                                                             \
+  }                                                                                                               \
 } while (0)
 
-int32_t f_build_filter(int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts opts) {
+int32_t f_build_filter(int *ret_p, int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts opts) {
   int32_t res = 0; // refer to resErr for meaning
   int allow_multiarch = opts & F_MULTIARCH;
   int allowed_personality = PER_LINUX;
@@ -229,8 +228,6 @@ int32_t f_build_filter(int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts
   } else
     errno = 0;
 
-  int ret;
-
   // We only really need to handle arches on multiarch systems.
   // If only one arch is supported the default is fine
   if (arch != 0) {
@@ -239,18 +236,16 @@ int32_t f_build_filter(int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts
     // allow the target arch, but we can't really disallow the
     // native arch at this point, because then bubblewrap
     // couldn't continue running.
-    ret = seccomp_arch_add(ctx, arch);
-    if (ret < 0 && ret != -EEXIST) {
+    *ret_p = seccomp_arch_add(ctx, arch);
+    if (*ret_p < 0 && *ret_p != -EEXIST) {
       res = 2;
-      errno = -ret;
       goto out;
     }
 
     if (allow_multiarch && multiarch != 0) {
-      ret = seccomp_arch_add(ctx, multiarch);
-      if (ret < 0 && ret != -EEXIST) {
+      *ret_p = seccomp_arch_add(ctx, multiarch);
+      if (*ret_p < 0 && *ret_p != -EEXIST) {
         res = 3;
-        errno = -ret;
         goto out;
       }
     }
@@ -286,17 +281,15 @@ int32_t f_build_filter(int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts
   seccomp_rule_add_exact(ctx, SCMP_ACT_ERRNO(EAFNOSUPPORT), SCMP_SYS(socket), 1, SCMP_A0(SCMP_CMP_GE, last_allowed_family + 1));
 
   if (fd < 0) {
-    ret = seccomp_load(ctx);
-    if (ret != 0) {
+    *ret_p = seccomp_load(ctx);
+    if (*ret_p != 0) {
       res = 7;
-      errno = -ret;
       goto out;
     }
   } else {
-    ret = seccomp_export_bpf(ctx, fd);
-    if (ret != 0) {
+    *ret_p = seccomp_export_bpf(ctx, fd);
+    if (*ret_p != 0) {
       res = 6;
-      errno = -ret;
       goto out;
     }
   }
diff --git a/helper/seccomp/seccomp-build.h b/helper/seccomp/seccomp-build.h
index 0ae201d2..7f1be761 100644
--- a/helper/seccomp/seccomp-build.h
+++ b/helper/seccomp/seccomp-build.h
@@ -20,4 +20,4 @@ typedef enum {
 } f_syscall_opts;
 
 extern void F_println(char *v);
-int32_t f_build_filter(int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts opts);
\ No newline at end of file
+int32_t f_build_filter(int *ret_p, int fd, uint32_t arch, uint32_t multiarch, f_syscall_opts opts);
\ No newline at end of file
diff --git a/helper/seccomp/seccomp.go b/helper/seccomp/seccomp.go
index fc192474..05c314c9 100644
--- a/helper/seccomp/seccomp.go
+++ b/helper/seccomp/seccomp.go
@@ -10,19 +10,51 @@ import (
 	"errors"
 	"fmt"
 	"runtime"
+	"syscall"
 )
 
 var CPrintln func(v ...any)
 
-var resErr = [...]error{
-	0: nil,
-	1: errors.New("seccomp_init failed"),
-	2: errors.New("seccomp_arch_add failed"),
-	3: errors.New("seccomp_arch_add failed (multiarch)"),
-	4: errors.New("internal libseccomp failure"),
-	5: errors.New("seccomp_rule_add failed"),
-	6: errors.New("seccomp_export_bpf failed"),
-	7: errors.New("seccomp_load failed"),
+// LibraryError represents a libseccomp error.
+type LibraryError struct {
+	Prefix  string
+	Seccomp syscall.Errno
+	Errno   error
+}
+
+func (e *LibraryError) Error() string {
+	if e.Seccomp == 0 {
+		if e.Errno == nil {
+			panic("invalid libseccomp error")
+		}
+		return fmt.Sprintf("%s: %s", e.Prefix, e.Errno)
+	}
+	if e.Errno == nil {
+		return fmt.Sprintf("%s: %s", e.Prefix, e.Seccomp)
+	}
+	return fmt.Sprintf("%s: %s (%s)", e.Prefix, e.Seccomp, e.Errno)
+}
+
+func (e *LibraryError) Is(err error) bool {
+	if e == nil {
+		return err == nil
+	}
+	if ef, ok := err.(*LibraryError); ok {
+		return *e == *ef
+	}
+	return (e.Seccomp != 0 && errors.Is(err, e.Seccomp)) ||
+		(e.Errno != nil && errors.Is(err, e.Errno))
+}
+
+var resPrefix = [...]string{
+	0: "",
+	1: "seccomp_init failed",
+	2: "seccomp_arch_add failed",
+	3: "seccomp_arch_add failed (multiarch)",
+	4: "internal libseccomp failure",
+	5: "seccomp_rule_add failed",
+	6: "seccomp_export_bpf failed",
+	7: "seccomp_load failed",
 }
 
 type SyscallOpts = C.f_syscall_opts
@@ -71,12 +103,14 @@ func buildFilter(fd int, opts SyscallOpts) error {
 		opts |= flagVerbose
 	}
 
-	res, err := C.f_build_filter(C.int(fd), arch, multiarch, opts)
-	if re := resErr[res]; re != nil {
-		if err == nil {
-			return re
+	var ret C.int
+	res, err := C.f_build_filter(&ret, C.int(fd), arch, multiarch, opts)
+	if prefix := resPrefix[res]; prefix != "" {
+		return &LibraryError{
+			prefix,
+			-syscall.Errno(ret),
+			err,
 		}
-		return fmt.Errorf("%s: %v", re.Error(), err)
 	}
 	return err
 }
diff --git a/helper/seccomp/seccomp_test.go b/helper/seccomp/seccomp_test.go
new file mode 100644
index 00000000..c04179c1
--- /dev/null
+++ b/helper/seccomp/seccomp_test.go
@@ -0,0 +1,65 @@
+package seccomp_test
+
+import (
+	"errors"
+	"runtime"
+	"syscall"
+	"testing"
+
+	"git.gensokyo.uk/security/fortify/helper/seccomp"
+)
+
+func TestLibraryError(t *testing.T) {
+	testCases := []struct {
+		name    string
+		sample  *seccomp.LibraryError
+		want    string
+		wantIs  bool
+		compare error
+	}{
+		{
+			"full",
+			&seccomp.LibraryError{Prefix: "seccomp_export_bpf failed", Seccomp: syscall.ECANCELED, Errno: syscall.EBADF},
+			"seccomp_export_bpf failed: operation canceled (bad file descriptor)",
+			true,
+			&seccomp.LibraryError{Prefix: "seccomp_export_bpf failed", Seccomp: syscall.ECANCELED, Errno: syscall.EBADF},
+		},
+		{
+			"errno only",
+			&seccomp.LibraryError{Prefix: "seccomp_init failed", Errno: syscall.ENOMEM},
+			"seccomp_init failed: cannot allocate memory",
+			false,
+			nil,
+		},
+		{
+			"seccomp only",
+			&seccomp.LibraryError{Prefix: "internal libseccomp failure", Seccomp: syscall.EFAULT},
+			"internal libseccomp failure: bad address",
+			true,
+			syscall.EFAULT,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if errors.Is(tc.sample, tc.compare) != tc.wantIs {
+				t.Errorf("errors.Is(%#v, %#v) did not return %v",
+					tc.sample, tc.compare, tc.wantIs)
+			}
+
+			if got := tc.sample.Error(); got != tc.want {
+				t.Errorf("Error: %q, want %q",
+					got, tc.want)
+			}
+		})
+	}
+
+	t.Run("invalid", func(t *testing.T) {
+		wantPanic := "invalid libseccomp error"
+		defer func() {
+			if r := recover(); r != wantPanic {
+				t.Errorf("panic: %q, want %q", r, wantPanic)
+			}
+		}()
+		runtime.KeepAlive(new(seccomp.LibraryError).Error())
+	})
+}