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> 

4 files changed, 57 insertions, 12 deletions

diff --git a/cmd/snap-seccomp/export_test.go b/cmd/snap-seccomp/export_test.go
index 48d23d846d..4545b5ad3e 100644
--- a/cmd/snap-seccomp/export_test.go
+++ b/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
	}
 }
 
diff --git a/cmd/snap-seccomp/main.go b/cmd/snap-seccomp/main.go
index 40d3dfbbdf..ba9ae17707 100644
--- a/cmd/snap-seccomp/main.go
+++ b/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)
diff --git a/cmd/snap-seccomp/main_test.go b/cmd/snap-seccomp/main_test.go
index 8e79ce78b3..cba333b3d7 100644
--- a/cmd/snap-seccomp/main_test.go
+++ b/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},
diff --git a/tests/main/security-seccomp/task.yaml b/tests/main/security-seccomp/task.yaml
index e1ac973f54..6289937395 100644
--- a/tests/main/security-seccomp/task.yaml
+++ b/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
@@ -91,3 +92,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\)'