From 5b180b4dcaca142fc979caf70b18920c224cc227 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Mon, 16 Oct 2017 16:27:40 -0400 Subject: [PATCH] Fix breaking change in Seccomp profile behavior Multiple conditions were previously allowed to be placed upon the same syscall argument. Restore this behavior. Signed-off-by: Matthew Heon --- libcontainer/integration/seccomp_test.go | 96 ++++++++++++++++++++++++ libcontainer/seccomp/seccomp_linux.go | 61 +++++++++++---- 2 files changed, 142 insertions(+), 15 deletions(-) diff --git a/libcontainer/integration/seccomp_test.go b/libcontainer/integration/seccomp_test.go index 8e2c7cda..9aa24d36 100644 --- a/libcontainer/integration/seccomp_test.go +++ b/libcontainer/integration/seccomp_test.go @@ -220,3 +220,99 @@ func TestSeccompDenyWriteConditional(t *testing.T) { t.Fatalf("Expected output %s but got %s\n", expected, actual) } } + +func TestSeccompMultipleConditionSameArgDeniesStdout(t *testing.T) { + if testing.Short() { + return + } + + rootfs, err := newRootfs() + if err != nil { + t.Fatal(err) + } + defer remove(rootfs) + + // Prevent writing to both stdout and stderr + config := newTemplateConfig(rootfs) + config.Seccomp = &configs.Seccomp{ + DefaultAction: configs.Allow, + Syscalls: []*configs.Syscall{ + { + Name: "write", + Action: configs.Errno, + Args: []*configs.Arg{ + { + Index: 0, + Value: 1, + Op: configs.EqualTo, + }, + { + Index: 0, + Value: 2, + Op: configs.EqualTo, + }, + }, + }, + }, + } + + buffers, exitCode, err := runContainer(config, "", "ls", "/") + if err != nil { + t.Fatalf("%s: %s", buffers, err) + } + if exitCode != 0 { + t.Fatalf("exit code not 0. code %d buffers %s", exitCode, buffers) + } + // Verify that nothing was printed + if len(buffers.Stdout.String()) != 0 { + t.Fatalf("Something was written to stdout, write call succeeded!\n") + } +} + +func TestSeccompMultipleConditionSameArgDeniesStderr(t *testing.T) { + if testing.Short() { + return + } + + rootfs, err := newRootfs() + if err != nil { + t.Fatal(err) + } + defer remove(rootfs) + + // Prevent writing to both stdout and stderr + config := newTemplateConfig(rootfs) + config.Seccomp = &configs.Seccomp{ + DefaultAction: configs.Allow, + Syscalls: []*configs.Syscall{ + { + Name: "write", + Action: configs.Errno, + Args: []*configs.Arg{ + { + Index: 0, + Value: 1, + Op: configs.EqualTo, + }, + { + Index: 0, + Value: 2, + Op: configs.EqualTo, + }, + }, + }, + }, + } + + buffers, exitCode, err := runContainer(config, "", "ls", "/does_not_exist") + if err == nil { + t.Fatalf("Expecting error return, instead got 0") + } + if exitCode == 0 { + t.Fatalf("Busybox should fail with negative exit code, instead got %d!", exitCode) + } + // Verify nothing was printed + if len(buffers.Stderr.String()) != 0 { + t.Fatalf("Something was written to stderr, write call succeeded!\n") + } +} diff --git a/libcontainer/seccomp/seccomp_linux.go b/libcontainer/seccomp/seccomp_linux.go index b9e651d6..eb27df7d 100644 --- a/libcontainer/seccomp/seccomp_linux.go +++ b/libcontainer/seccomp/seccomp_linux.go @@ -25,6 +25,11 @@ var ( SeccompModeFilter = uintptr(2) ) +const ( + // Linux system calls can have at most 6 arguments + syscallMaxArguments int = 6 +) + // Filters given syscalls in a container, preventing them from being used // Started in the container init process, and carried over to all child processes // Setns calls, however, require a separate invocation, as they are not children @@ -182,21 +187,47 @@ func matchCall(filter *libseccomp.ScmpFilter, call *configs.Syscall) error { return err } } else { - // Conditional match - convert the per-arg rules into library format - conditions := []libseccomp.ScmpCondition{} - - for _, cond := range call.Args { - newCond, err := getCondition(cond) - if err != nil { - return err - } - - conditions = append(conditions, newCond) - } - - if err := filter.AddRuleConditional(callNum, callAct, conditions); err != nil { - return err - } + // If two or more arguments have the same condition, + // Revert to old behavior, adding each condition as a separate rule + argCounts := make([]uint, syscallMaxArguments) + conditions := []libseccomp.ScmpCondition{} + + for _, cond := range call.Args { + newCond, err := getCondition(cond) + if err != nil { + return fmt.Errorf("error creating seccomp syscall condition for syscall %s: %w", call.Name, err) + } + + argCounts[cond.Index] += 1 + + conditions = append(conditions, newCond) + } + + hasMultipleArgs := false + for _, count := range argCounts { + if count > 1 { + hasMultipleArgs = true + break + } + } + + if hasMultipleArgs { + // Revert to old behavior + // Add each condition attached to a separate rule + for _, cond := range conditions { + condArr := []libseccomp.ScmpCondition{cond} + + if err := filter.AddRuleConditional(callNum, callAct, condArr); err != nil { + return fmt.Errorf("error adding seccomp rule for syscall %s: %w", call.Name, err) + } + } + } else { + // No conditions share same argument + // Use new, proper behavior + if err := filter.AddRuleConditional(callNum, callAct, conditions); err != nil { + return fmt.Errorf("error adding seccomp rule for syscall %s: %w", call.Name, err) + } + } } return filter.SetSyscallPriority(callNum, call.Priority) -- 2.30.0