snap-seccomp: support explicitly blocking of syscalls

snap-seccomp has always implemented an allow-list approach to syscalls - such
that the listed syscalls are allowed and any non-listed will get
blocked. However, in the case where we want to disallow a syscall with
particular arguments, it is only possible to block one instance of the sycall
with a given argument. If a second similar rule is added, each rule effectively
allows the other and so neither get disallowed as a result.

So introduce the concept of explicitly denying system calls listed in the
seccomp profile by prefixing them with a tilde (~). The seccomp action for these
is then EACCES (since EPERM is the default for unmatched syscalls and seccomp
doesn't allow to specify an action which is the same as the default).

This then allows to specify to block various syscall argument combinations as
expected, and so is used as the mechanism to fix CVE-2023-1523.

Signed-off-by: Alex Murray <alex.murray@canonical.com>

cmd/snap-seccomp/export_test.go

@@ -42,11 +42,19 @@ func MockArchDpkgKernelArchitecture(f func() string) (restore func()) {
 	}
 }
 
-func MockErrnoOnDenial(i int16) (retore func()) {
-	origErrnoOnDenial := errnoOnDenial
-	errnoOnDenial = i
+func MockErrnoOnImplicitDenial(i int16) (retore func()) {
+	origErrnoOnImplicitDenial := errnoOnImplicitDenial
+	errnoOnImplicitDenial = i
 	return func() {
-		errnoOnDenial = origErrnoOnDenial
+		errnoOnImplicitDenial = origErrnoOnImplicitDenial
+	}
+}
+
+func MockErrnoOnExplicitDenial(i int16) (retore func()) {
+	origErrnoOnExplicitDenial := errnoOnExplicitDenial
+	errnoOnExplicitDenial = i
+	return func() {
+		errnoOnExplicitDenial = origErrnoOnExplicitDenial
 	}
 }
 

cmd/snap-seccomp/main.go

@@ -34,6 +34,7 @@ package main
 //#include <stdio.h>
 //#include <stdlib.h>
 //#include <string.h>
+//#include <sys/ioctl.h>
 //#include <sys/prctl.h>
 //#include <sys/quota.h>
 //#include <sys/resource.h>
@@ -182,6 +183,11 @@ package main
 // #ifndef PTRACE_SETFPXREGS
 // #define PTRACE_SETFPXREGS 19
 // #endif
+//
+// /* Define TIOCLINUX if needed */
+// #ifndef TIOCLINUX
+// #define TIOCLINUX 0x541C
+// #endif
 import "C"
 
 import (
@@ -381,6 +387,9 @@ var seccompResolver = map[string]uint64{
 	// man 4 tty_ioctl
 	"TIOCSTI": syscall.TIOCSTI,
 
+	// man 2 ioctl_console
+	"TIOCLINUX": C.TIOCLINUX,
+
 	// man 2 quotactl (with what Linux supports)
 	"Q_SYNC":      C.Q_SYNC,
 	"Q_QUOTAON":   C.Q_QUOTAON,
@@ -542,6 +551,8 @@ func readNumber(token string, syscallName string) (uint64, error) {
 	return uint64(uint32(value)), nil
 }
 
+var errnoOnExplicitDenial int16 = C.EACCES
+
 func parseLine(line string, secFilter *seccomp.ScmpFilter) error {
 	// ignore comments and empty lines
 	if strings.HasPrefix(line, "#") || line == "" {
@@ -554,8 +565,17 @@ func parseLine(line string, secFilter *seccomp.ScmpFilter) error {
 		return fmt.Errorf("too many arguments specified for syscall '%s' in line %q", tokens[0], line)
 	}
 
+	// allow the listed syscall but also support explicit denials as well by
+	// prefixing the line with a ~
+	action := seccomp.ActAllow
+
 	// fish out syscall
 	syscallName := tokens[0]
+	if strings.HasPrefix(syscallName, "~") {
+		action = seccomp.ActErrno.SetReturnCode(errnoOnExplicitDenial)
+		syscallName = syscallName[1:]
+	}
+
 	secSyscall, err := seccomp.GetSyscallFromName(syscallName)
 	if err != nil {
 		// FIXME: use structed error in libseccomp-golang when
@@ -638,8 +658,8 @@ func parseLine(line string, secFilter *seccomp.ScmpFilter) error {
 
 	// Default to adding a precise match if possible. Otherwise
 	// let seccomp figure out the architecture specifics.
-	if err = secFilter.AddRuleConditionalExact(secSyscall, seccomp.ActAllow, conds); err != nil {
-		err = secFilter.AddRuleConditional(secSyscall, seccomp.ActAllow, conds)
+	if err = secFilter.AddRuleConditionalExact(secSyscall, action, conds); err != nil {
+		err = secFilter.AddRuleConditional(secSyscall, action, conds)
 	}
 
 	return err
@@ -698,7 +718,7 @@ func addSecondaryArches(secFilter *seccomp.ScmpFilter) error {
 	return nil
 }
 
-var errnoOnDenial int16 = C.EPERM
+var errnoOnImplicitDenial int16 = C.EPERM
 
 func preprocess(content []byte) (unrestricted, complain bool) {
 	scanner := bufio.NewScanner(bytes.NewBuffer(content))
@@ -771,7 +791,7 @@ func compile(content []byte, out string) error {
 			unrestricted = true
 		}
 	default:
-		secFilter, err = seccomp.NewFilter(seccomp.ActErrno.SetReturnCode(errnoOnDenial))
+		secFilter, err = seccomp.NewFilter(seccomp.ActErrno.SetReturnCode(errnoOnImplicitDenial))
 	}
 	if err != nil {
 		return fmt.Errorf("cannot create seccomp filter: %s", err)

cmd/snap-seccomp/main_test.go

@@ -151,8 +151,9 @@ int main(int argc, char** argv)
     // There might be architecture-specific requirements. see "man syscall"
     // for details.
     syscall_ret = syscall(l[0], l[1], l[2], l[3], l[4], l[5], l[6]);
-    // 911 is our mocked errno
-    if (syscall_ret < 0 && errno == 911) {
+    // 911 is our mocked errno for implicit denials via unlisted syscalls and
+    // 999 is explicit denial
+    if (syscall_ret < 0 && (errno == 911 || errno == 999)) {
         ret = 10;
     }
     syscall(SYS_exit, ret, 0, 0, 0, 0, 0);
@@ -161,7 +162,8 @@ int main(int argc, char** argv)
 `)
 
 func (s *snapSeccompSuite) SetUpSuite(c *C) {
-	main.MockErrnoOnDenial(911)
+	main.MockErrnoOnImplicitDenial(911)
+	main.MockErrnoOnExplicitDenial(999)
 
 	// build seccomp-load helper
 	s.seccompBpfLoader = filepath.Join(c.MkDir(), "seccomp_bpf_loader")
@@ -222,7 +224,7 @@ func (s *snapSeccompSuite) SetUpSuite(c *C) {
 // posix_fadvise, pread64, pwrite64, readahead,	sync_file_range, and truncate64.
 //
 // Once we start using those. See `man syscall`
-func (s *snapSeccompSuite) runBpf(c *C, seccompWhitelist, bpfInput string, expected int) {
+func (s *snapSeccompSuite) runBpf(c *C, seccompWhitelist, bpfInput string, expected int) { //
 	// Common syscalls we need to allow for a minimal statically linked
 	// c program.
 	//
@@ -449,6 +451,12 @@ func (s *snapSeccompSuite) TestCompile(c *C) {
 		{"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", Allow},
 		{"ioctl - TIOCSTI", "ioctl;native;-,99", Deny},
 		{"ioctl - !TIOCSTI", "ioctl;native;-,TIOCSTI", Deny},
+		{"~ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", Deny},
+		// also check we can deny multiple uses of ioctl but still allow
+		// others
+		{"~ioctl - TIOCSTI\n~ioctl - TIOCLINUX\nioctl - !TIOCSTI", "ioctl;native;-,TIOCSTI", Deny},
+		{"~ioctl - TIOCSTI\n~ioctl - TIOCLINUX\nioctl - !TIOCSTI", "ioctl;native;-,TIOCLINUX", Deny},
+		{"~ioctl - TIOCSTI\n~ioctl - TIOCLINUX\nioctl - !TIOCSTI", "ioctl;native;-,TIOCGWINSZ", Allow},
 
 		// test_bad_seccomp_filter_args_clone
 		{"setns - CLONE_NEWNET", "setns;native;-,99", Deny},

tests/main/security-seccomp/task.yaml

@@ -7,6 +7,7 @@ details: |
     - use of a bare syscall (ie, no arguments) is allowed
     - use of a syscall with arg filtering is allowed with matching arguments
     - use of a syscall with arg filtering is denied with unmatching arguments
+    - explicit denial of a syscall with matching arguments is denied
     .
     We choose the setpriority syscall for these tests since it is available on
     all architectures and can be easily used to test all of the above. As part of
@@ -81,3 +82,11 @@ execute: |
     test-snapd-setpriority 10  | MATCH 'Successfully used setpriority\(PRIO_PROCESS, 0, 10\)'
     echo "and check that negative nice fails"
     test-snapd-setpriority -10 | MATCH 'Operation not permitted \(EPERM\)'
+
+    echo "Explicitly deny arg filtered setpriority rule"
+    sed 's/^\(setpriority.*\)/~\1/g' "$SRC".orig > "$SRC"
+    snapd.tool exec snap-seccomp compile "$SRC" "$BIN"
+    echo "and check that positive nice fails with explicit denial"
+    test-snapd-setpriority 10  | MATCH 'Insufficient privileges \(EACCES\)'
+    echo "and check that negative nice still fails with implicit denial"
+    test-snapd-setpriority -10 | MATCH 'Operation not permitted \(EPERM\)'