Merge pull request #3580 from kolyshkin/fix-seccomp-ssb

seccomp: do not ignore SPEC_ALLOW flag

Author: Mrunal Patel

libcontainer/seccomp/patchbpf/enosys_linux.go

@@ -43,6 +43,11 @@ const uintptr_t C_SET_MODE_FILTER = SECCOMP_SET_MODE_FILTER;
 #endif
 const uintptr_t C_FILTER_FLAG_LOG = SECCOMP_FILTER_FLAG_LOG;
 
+#ifndef SECCOMP_FILTER_FLAG_SPEC_ALLOW
+#	define SECCOMP_FILTER_FLAG_SPEC_ALLOW (1UL << 2)
+#endif
+const uintptr_t C_FILTER_FLAG_SPEC_ALLOW = SECCOMP_FILTER_FLAG_SPEC_ALLOW;
+
 #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER
 #	define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3)
 #endif
@@ -641,8 +646,13 @@ func filterFlags(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (flags
 			flags |= uint(C.C_FILTER_FLAG_LOG)
 		}
 	}
-
-	// TODO: Support seccomp flags not yet added to libseccomp-golang...
+	if apiLevel >= 4 {
+		if ssb, err := filter.GetSSB(); err != nil {
+			return 0, false, fmt.Errorf("unable to fetch SECCOMP_FILTER_FLAG_SPEC_ALLOW bit: %w", err)
+		} else if ssb {
+			flags |= uint(C.C_FILTER_FLAG_SPEC_ALLOW)
+		}
+	}
 
 	for _, call := range config.Syscalls {
 		if call.Action == configs.Notify {
@@ -655,6 +665,9 @@ func filterFlags(config *configs.Seccomp, filter *libseccomp.ScmpFilter) (flags
 }
 
 func sysSeccompSetFilter(flags uint, filter []unix.SockFilter) (fd int, err error) {
+	// This debug output is validated in tests/integration/seccomp.bats
+	// by the SECCOMP_FILTER_FLAG_* test.
+	logrus.Debugf("seccomp filter flags: %d", flags)
 	fprog := unix.SockFprog{
 		Len:    uint16(len(filter)),
 		Filter: &filter[0],

libcontainer/seccomp/seccomp_linux.go

@@ -105,6 +105,7 @@ func InitSeccomp(config *configs.Seccomp) (int, error) {
 			if err := filter.SetSSB(true); err != nil {
 				return -1, fmt.Errorf("error adding SSB flag to seccomp filter: %w", err)
 			}
+		// NOTE when adding more flags, make sure to also modify filterFlags in patchbpf.
 		default:
 			return -1, fmt.Errorf("seccomp flags %q not yet supported by runc", flag)
 		}

tests/integration/seccomp.bats

@@ -70,31 +70,47 @@ function teardown() {
 	# Linux 4.14: SECCOMP_FILTER_FLAG_LOG
 	# Linux 4.17: SECCOMP_FILTER_FLAG_SPEC_ALLOW
 	requires_kernel 4.17
-	SECCOMP_FILTER_FLAGS=(
-		'' # no flag
-		'"SECCOMP_FILTER_FLAG_LOG"'
-		'"SECCOMP_FILTER_FLAG_SPEC_ALLOW"'
-		'"SECCOMP_FILTER_FLAG_TSYNC"'
-		'"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW"'
-		'"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_TSYNC"'
-		'"SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"'
-		'"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"'
+
+	update_config '   .process.args = ["/bin/sh", "-c", "mkdir /dev/shm/foo"]
+			| .process.noNewPrivileges = false
+			| .linux.seccomp = {
+				"defaultAction":"SCMP_ACT_ALLOW",
+				"architectures":["SCMP_ARCH_X86","SCMP_ARCH_X32","SCMP_ARCH_X86_64","SCMP_ARCH_AARCH64","SCMP_ARCH_ARM"],
+				"syscalls":[{"names":["mkdir"], "action":"SCMP_ACT_ERRNO"}]
+			}'
+
+	declare -A FLAGS=(
+		['REMOVE']=0 # No setting, use built-in default.
+		['EMPTY']=0  # Empty set of flags.
+		['"SECCOMP_FILTER_FLAG_LOG"']=2
+		['"SECCOMP_FILTER_FLAG_SPEC_ALLOW"']=4
+		['"SECCOMP_FILTER_FLAG_TSYNC"']=0 # tsync flag is ignored.
+		['"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW"']=6
+		['"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_TSYNC"']=2
+		['"SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"']=4
+		['"SECCOMP_FILTER_FLAG_LOG","SECCOMP_FILTER_FLAG_SPEC_ALLOW","SECCOMP_FILTER_FLAG_TSYNC"']=6
 	)
-	for flags in "${SECCOMP_FILTER_FLAGS[@]}"; do
-		update_config '   .process.args = ["/bin/sh", "-c", "mkdir /dev/shm/foo"]
-				| .process.noNewPrivileges = false
-				| .linux.seccomp = {
-					"defaultAction":"SCMP_ACT_ALLOW",
-					"architectures":["SCMP_ARCH_X86","SCMP_ARCH_X32","SCMP_ARCH_X86_64","SCMP_ARCH_AARCH64","SCMP_ARCH_ARM"],
-					"flags":['"${flags}"'],
-					"syscalls":[{"names":["mkdir"], "action":"SCMP_ACT_ERRNO"}]
-				}'
-
-		# This test checks that the flags are accepted without errors but does
-		# not check they are effectively applied
-		runc run test_busybox
+	for key in "${!FLAGS[@]}"; do
+		case "$key" in
+		'REMOVE')
+			update_config ' del(.linux.seccomp.flags)'
+			;;
+		'EMPTY')
+			update_config ' .linux.seccomp.flags = []'
+			;;
+		*)
+			update_config ' .linux.seccomp.flags = [ '"${key}"' ]'
+			;;
+		esac
+
+		runc --debug run test_busybox
 		[ "$status" -ne 0 ]
 		[[ "$output" == *"mkdir:"*"/dev/shm/foo"*"Operation not permitted"* ]]
+
+		# Check the numeric flags value, as printed in the debug log, is as expected.
+		exp="\"seccomp filter flags: ${FLAGS[$key]}\""
+		echo "flags $key, expecting $exp"
+		[[ "$output" == *"$exp"* ]]
 	done
 }