From 29c3f8becb875de308be9238f5ca0ba9f35eb7bd Mon Sep 17 00:00:00 2001 From: Ophestra Date: Wed, 12 Mar 2025 15:52:48 +0900 Subject: [PATCH] helper/seccomp: improve error handling This passes both errno and libseccomp return value. Signed-off-by: Ophestra --- helper/seccomp/export_test.go | 2 +- helper/seccomp/seccomp-build.c | 61 ++++++++++++++----------------- helper/seccomp/seccomp-build.h | 2 +- helper/seccomp/seccomp.go | 62 ++++++++++++++++++++++++-------- helper/seccomp/seccomp_test.go | 65 ++++++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 50 deletions(-) create mode 100644 helper/seccomp/seccomp_test.go diff --git a/helper/seccomp/export_test.go b/helper/seccomp/export_test.go index 40f81c7..ff6f250 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 827d3b8..4b2114f 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 0ae201d..7f1be76 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 fc19247..05c314c 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 0000000..c04179c --- /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()) + }) +}