From 03a5a7476e93a7fc3726b6e83c558404db664590 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 26 Apr 2017 13:31:28 -0400 Subject: [PATCH 1/3] Vendor updated libseccomp-golang for bugfix Syscall argument handling was bugged in previous releases. Per-argument match rules were handled with OR logic when they should have used AND logic. The updated version of the bindings resolves this issue. As a side effect, the minimum supported version of Libseccomp has been raised from v2.1.0 to v2.2.0. Signed-off-by: Matthew Heon --- vendor.conf | 2 +- .../seccomp/libseccomp-golang/README | 25 +++ .../seccomp/libseccomp-golang/seccomp.go | 105 ++++++----- .../libseccomp-golang/seccomp_internal.go | 168 +++++++++--------- 4 files changed, 167 insertions(+), 133 deletions(-) diff --git a/vendor.conf b/vendor.conf index 1266ee48..8139d696 100644 --- a/vendor.conf +++ b/vendor.conf @@ -5,7 +5,7 @@ github.com/opencontainers/runtime-spec v1.0.0 # Core libcontainer functionality. github.com/mrunalp/fileutils ed869b029674c0e9ce4c0dfa781405c2d9946d08 github.com/opencontainers/selinux v1.0.0-rc1 -github.com/seccomp/libseccomp-golang 32f571b70023028bd57d9288c20efbcb237f3ce0 +github.com/seccomp/libseccomp-golang 84e90a91acea0f4e51e62bc1a75de18b1fc0790f github.com/sirupsen/logrus a3f95b5c423586578a4e099b11a46c2479628cac github.com/syndtr/gocapability db04d3cc01c8b54962a58ec7e491717d06cfcc16 github.com/vishvananda/netlink 1e2e08e8a2dcdacaae3f14ac44c5cfa31361f270 diff --git a/vendor/github.com/seccomp/libseccomp-golang/README b/vendor/github.com/seccomp/libseccomp-golang/README index 64cab691..66839a46 100644 --- a/vendor/github.com/seccomp/libseccomp-golang/README +++ b/vendor/github.com/seccomp/libseccomp-golang/README @@ -24,3 +24,28 @@ please note that a Google account is not required to subscribe to the mailing list. -> https://groups.google.com/d/forum/libseccomp + +Documentation is also available at: + + -> https://godoc.org/github.com/seccomp/libseccomp-golang + +* Installing the package + +The libseccomp-golang bindings require at least Go v1.2.1 and GCC v4.8.4; +earlier versions may yield unpredictable results. If you meet these +requirements you can install this package using the command below: + + $ go get github.com/seccomp/libseccomp-golang + +* Testing the Library + +A number of tests and lint related recipes are provided in the Makefile, if +you want to run the standard regression tests, you can excute the following: + + $ make check + +In order to execute the 'make lint' recipe the 'golint' tool is needed, it +can be found at: + + -> https://github.com/golang/lint + diff --git a/vendor/github.com/seccomp/libseccomp-golang/seccomp.go b/vendor/github.com/seccomp/libseccomp-golang/seccomp.go index b2c010fc..d3d9e6bf 100644 --- a/vendor/github.com/seccomp/libseccomp-golang/seccomp.go +++ b/vendor/github.com/seccomp/libseccomp-golang/seccomp.go @@ -27,6 +27,28 @@ import "C" // Exported types +// VersionError denotes that the system libseccomp version is incompatible +// with this package. +type VersionError struct { + message string + minimum string +} + +func (e VersionError) Error() string { + format := "Libseccomp version too low: " + if e.message != "" { + format += e.message + ": " + } + format += "minimum supported is " + if e.minimum != "" { + format += e.minimum + ": " + } else { + format += "2.2.0: " + } + format += "detected %d.%d.%d" + return fmt.Sprintf(format, verMajor, verMinor, verMicro) +} + // ScmpArch represents a CPU architecture. Seccomp can restrict syscalls on a // per-architecture basis. type ScmpArch uint @@ -54,8 +76,8 @@ type ScmpSyscall int32 const ( // Valid architectures recognized by libseccomp - // ARM64 and all MIPS architectures are unsupported by versions of the - // library before v2.2 and will return errors if used + // PowerPC and S390(x) architectures are unavailable below library version + // v2.3.0 and will returns errors if used with incompatible libraries // ArchInvalid is a placeholder to ensure uninitialized ScmpArch // variables are invalid @@ -151,6 +173,10 @@ const ( // GetArchFromString returns an ScmpArch constant from a string representing an // architecture func GetArchFromString(arch string) (ScmpArch, error) { + if err := ensureSupportedVersion(); err != nil { + return ArchInvalid, err + } + switch strings.ToLower(arch) { case "x86": return ArchX86, nil @@ -298,7 +324,7 @@ func (a ScmpAction) GetReturnCode() int16 { // GetLibraryVersion returns the version of the library the bindings are built // against. // The version is formatted as follows: Major.Minor.Micro -func GetLibraryVersion() (major, minor, micro int) { +func GetLibraryVersion() (major, minor, micro uint) { return verMajor, verMinor, verMicro } @@ -338,6 +364,10 @@ func (s ScmpSyscall) GetNameByArch(arch ScmpArch) (string, error) { // Returns the number of the syscall, or an error if no syscall with that name // was found. func GetSyscallFromName(name string) (ScmpSyscall, error) { + if err := ensureSupportedVersion(); err != nil { + return 0, err + } + cString := C.CString(name) defer C.free(unsafe.Pointer(cString)) @@ -355,6 +385,9 @@ func GetSyscallFromName(name string) (ScmpSyscall, error) { // Returns the number of the syscall, or an error if an invalid architecture is // passed or a syscall with that name was not found. func GetSyscallFromNameByArch(name string, arch ScmpArch) (ScmpSyscall, error) { + if err := ensureSupportedVersion(); err != nil { + return 0, err + } if err := sanitizeArch(arch); err != nil { return 0, err } @@ -386,6 +419,10 @@ func GetSyscallFromNameByArch(name string, arch ScmpArch) (ScmpSyscall, error) { func MakeCondition(arg uint, comparison ScmpCompareOp, values ...uint64) (ScmpCondition, error) { var condStruct ScmpCondition + if err := ensureSupportedVersion(); err != nil { + return condStruct, err + } + if comparison == CompareInvalid { return condStruct, fmt.Errorf("invalid comparison operator") } else if arg > 5 { @@ -413,6 +450,10 @@ func MakeCondition(arg uint, comparison ScmpCompareOp, values ...uint64) (ScmpCo // GetNativeArch returns architecture token representing the native kernel // architecture func GetNativeArch() (ScmpArch, error) { + if err := ensureSupportedVersion(); err != nil { + return ArchInvalid, err + } + arch := C.seccomp_arch_native() return archFromNative(arch) @@ -435,6 +476,10 @@ type ScmpFilter struct { // Returns a reference to a valid filter context, or nil and an error if the // filter context could not be created or an invalid default action was given. func NewFilter(defaultAction ScmpAction) (*ScmpFilter, error) { + if err := ensureSupportedVersion(); err != nil { + return nil, err + } + if err := sanitizeAction(defaultAction); err != nil { return nil, err } @@ -449,6 +494,13 @@ func NewFilter(defaultAction ScmpAction) (*ScmpFilter, error) { filter.valid = true runtime.SetFinalizer(filter, filterFinalizer) + // Enable TSync so all goroutines will receive the same rules + // If the kernel does not support TSYNC, allow us to continue without error + if err := filter.setFilterAttr(filterAttrTsync, 0x1); err != nil && err != syscall.ENOTSUP { + filter.Release() + return nil, fmt.Errorf("could not create filter - error setting tsync bit: %v", err) + } + return filter, nil } @@ -505,7 +557,7 @@ func (f *ScmpFilter) Release() { // The source filter src will be released as part of the process, and will no // longer be usable or valid after this call. // To be merged, filters must NOT share any architectures, and all their -// attributes (Default Action, Bad Arch Action, No New Privs and TSync bools) +// attributes (Default Action, Bad Arch Action, and No New Privs bools) // must match. // The filter src will be merged into the filter this is called on. // The architectures of the src filter not present in the destination, and all @@ -678,30 +730,6 @@ func (f *ScmpFilter) GetNoNewPrivsBit() (bool, error) { return true, nil } -// GetTsyncBit returns whether Thread Synchronization will be enabled on the -// filter being loaded, or an error if an issue was encountered retrieving the -// value. -// Thread Sync ensures that all members of the thread group of the calling -// process will share the same Seccomp filter set. -// Tsync is a fairly recent addition to the Linux kernel and older kernels -// lack support. If the running kernel does not support Tsync and it is -// requested in a filter, Libseccomp will not enable TSync support and will -// proceed as normal. -// This function is unavailable before v2.2 of libseccomp and will return an -// error. -func (f *ScmpFilter) GetTsyncBit() (bool, error) { - tSync, err := f.getFilterAttr(filterAttrTsync) - if err != nil { - return false, err - } - - if tSync == 0 { - return false, nil - } - - return true, nil -} - // SetBadArchAction sets the default action taken on a syscall for an // architecture not in the filter, or an error if an issue was encountered // setting the value. @@ -728,27 +756,6 @@ func (f *ScmpFilter) SetNoNewPrivsBit(state bool) error { return f.setFilterAttr(filterAttrNNP, toSet) } -// SetTsync sets whether Thread Synchronization will be enabled on the filter -// being loaded. Returns an error if setting Tsync failed, or the filter is -// invalid. -// Thread Sync ensures that all members of the thread group of the calling -// process will share the same Seccomp filter set. -// Tsync is a fairly recent addition to the Linux kernel and older kernels -// lack support. If the running kernel does not support Tsync and it is -// requested in a filter, Libseccomp will not enable TSync support and will -// proceed as normal. -// This function is unavailable before v2.2 of libseccomp and will return an -// error. -func (f *ScmpFilter) SetTsync(enable bool) error { - var toSet C.uint32_t = 0x0 - - if enable { - toSet = 0x1 - } - - return f.setFilterAttr(filterAttrTsync, toSet) -} - // SetSyscallPriority sets a syscall's priority. // This provides a hint to the filter generator in libseccomp about the // importance of this syscall. High-priority syscalls are placed diff --git a/vendor/github.com/seccomp/libseccomp-golang/seccomp_internal.go b/vendor/github.com/seccomp/libseccomp-golang/seccomp_internal.go index ab67a3de..5b6a79ad 100644 --- a/vendor/github.com/seccomp/libseccomp-golang/seccomp_internal.go +++ b/vendor/github.com/seccomp/libseccomp-golang/seccomp_internal.go @@ -7,7 +7,6 @@ package seccomp import ( "fmt" - "os" "syscall" ) @@ -21,43 +20,15 @@ import ( #include #if SCMP_VER_MAJOR < 2 -#error Minimum supported version of Libseccomp is v2.1.0 -#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR < 1 -#error Minimum supported version of Libseccomp is v2.1.0 +#error Minimum supported version of Libseccomp is v2.2.0 +#elif SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR < 2 +#error Minimum supported version of Libseccomp is v2.2.0 #endif #define ARCH_BAD ~0 const uint32_t C_ARCH_BAD = ARCH_BAD; -#ifndef SCMP_ARCH_AARCH64 -#define SCMP_ARCH_AARCH64 ARCH_BAD -#endif - -#ifndef SCMP_ARCH_MIPS -#define SCMP_ARCH_MIPS ARCH_BAD -#endif - -#ifndef SCMP_ARCH_MIPS64 -#define SCMP_ARCH_MIPS64 ARCH_BAD -#endif - -#ifndef SCMP_ARCH_MIPS64N32 -#define SCMP_ARCH_MIPS64N32 ARCH_BAD -#endif - -#ifndef SCMP_ARCH_MIPSEL -#define SCMP_ARCH_MIPSEL ARCH_BAD -#endif - -#ifndef SCMP_ARCH_MIPSEL64 -#define SCMP_ARCH_MIPSEL64 ARCH_BAD -#endif - -#ifndef SCMP_ARCH_MIPSEL64N32 -#define SCMP_ARCH_MIPSEL64N32 ARCH_BAD -#endif - #ifndef SCMP_ARCH_PPC #define SCMP_ARCH_PPC ARCH_BAD #endif @@ -102,12 +73,6 @@ const uint32_t C_ACT_ERRNO = SCMP_ACT_ERRNO(0); const uint32_t C_ACT_TRACE = SCMP_ACT_TRACE(0); const uint32_t C_ACT_ALLOW = SCMP_ACT_ALLOW; -// If TSync is not supported, make sure it doesn't map to a supported filter attribute -// Don't worry about major version < 2, the minimum version checks should catch that case -#if SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR < 2 -#define SCMP_FLTATR_CTL_TSYNC _SCMP_CMP_MIN -#endif - const uint32_t C_ATTRIBUTE_DEFAULT = (uint32_t)SCMP_FLTATR_ACT_DEFAULT; const uint32_t C_ATTRIBUTE_BADARCH = (uint32_t)SCMP_FLTATR_ACT_BADARCH; const uint32_t C_ATTRIBUTE_NNP = (uint32_t)SCMP_FLTATR_CTL_NNP; @@ -125,25 +90,61 @@ const int C_VERSION_MAJOR = SCMP_VER_MAJOR; const int C_VERSION_MINOR = SCMP_VER_MINOR; const int C_VERSION_MICRO = SCMP_VER_MICRO; +#if SCMP_VER_MAJOR == 2 && SCMP_VER_MINOR >= 3 +unsigned int get_major_version() +{ + return seccomp_version()->major; +} + +unsigned int get_minor_version() +{ + return seccomp_version()->minor; +} + +unsigned int get_micro_version() +{ + return seccomp_version()->micro; +} +#else +unsigned int get_major_version() +{ + return (unsigned int)C_VERSION_MAJOR; +} + +unsigned int get_minor_version() +{ + return (unsigned int)C_VERSION_MINOR; +} + +unsigned int get_micro_version() +{ + return (unsigned int)C_VERSION_MICRO; +} +#endif + typedef struct scmp_arg_cmp* scmp_cast_t; -// Wrapper to create an scmp_arg_cmp struct -void* -make_struct_arg_cmp( - unsigned int arg, - int compare, - uint64_t a, - uint64_t b - ) +void* make_arg_cmp_array(unsigned int length) { - struct scmp_arg_cmp *s = malloc(sizeof(struct scmp_arg_cmp)); + return calloc(length, sizeof(struct scmp_arg_cmp)); +} - s->arg = arg; - s->op = compare; - s->datum_a = a; - s->datum_b = b; +// Wrapper to add an scmp_arg_cmp struct to an existing arg_cmp array +void add_struct_arg_cmp( + struct scmp_arg_cmp* arr, + unsigned int pos, + unsigned int arg, + int compare, + uint64_t a, + uint64_t b + ) +{ + arr[pos].arg = arg; + arr[pos].op = compare; + arr[pos].datum_a = a; + arr[pos].datum_b = b; - return s; + return; } */ import "C" @@ -178,26 +179,26 @@ var ( // Error thrown on bad filter context errBadFilter = fmt.Errorf("filter is invalid or uninitialized") // Constants representing library major, minor, and micro versions - verMajor = int(C.C_VERSION_MAJOR) - verMinor = int(C.C_VERSION_MINOR) - verMicro = int(C.C_VERSION_MICRO) + verMajor = uint(C.get_major_version()) + verMinor = uint(C.get_minor_version()) + verMicro = uint(C.get_micro_version()) ) // Nonexported functions // Check if library version is greater than or equal to the given one -func checkVersionAbove(major, minor, micro int) bool { +func checkVersionAbove(major, minor, micro uint) bool { return (verMajor > major) || (verMajor == major && verMinor > minor) || (verMajor == major && verMinor == minor && verMicro >= micro) } -// Init function: Verify library version is appropriate -func init() { - if !checkVersionAbove(2, 1, 0) { - fmt.Fprintf(os.Stderr, "Libseccomp version too low: minimum supported is 2.1.0, detected %d.%d.%d", C.C_VERSION_MAJOR, C.C_VERSION_MINOR, C.C_VERSION_MICRO) - os.Exit(-1) +// Ensure that the library is supported, i.e. >= 2.2.0. +func ensureSupportedVersion() error { + if !checkVersionAbove(2, 2, 0) { + return VersionError{} } + return nil } // Filter helpers @@ -216,10 +217,6 @@ func (f *ScmpFilter) getFilterAttr(attr scmpFilterAttr) (C.uint32_t, error) { return 0x0, errBadFilter } - if !checkVersionAbove(2, 2, 0) && attr == filterAttrTsync { - return 0x0, fmt.Errorf("the thread synchronization attribute is not supported in this version of the library") - } - var attribute C.uint32_t retCode := C.seccomp_attr_get(f.filterCtx, attr.toNative(), &attribute) @@ -239,10 +236,6 @@ func (f *ScmpFilter) setFilterAttr(attr scmpFilterAttr, value C.uint32_t) error return errBadFilter } - if !checkVersionAbove(2, 2, 0) && attr == filterAttrTsync { - return fmt.Errorf("the thread synchronization attribute is not supported in this version of the library") - } - retCode := C.seccomp_attr_set(f.filterCtx, attr.toNative(), value) if retCode != 0 { return syscall.Errno(-1 * retCode) @@ -254,12 +247,9 @@ func (f *ScmpFilter) setFilterAttr(attr scmpFilterAttr, value C.uint32_t) error // DOES NOT LOCK OR CHECK VALIDITY // Assumes caller has already done this // Wrapper for seccomp_rule_add_... functions -func (f *ScmpFilter) addRuleWrapper(call ScmpSyscall, action ScmpAction, exact bool, cond C.scmp_cast_t) error { - var length C.uint - if cond != nil { - length = 1 - } else { - length = 0 +func (f *ScmpFilter) addRuleWrapper(call ScmpSyscall, action ScmpAction, exact bool, length C.uint, cond C.scmp_cast_t) error { + if length != 0 && cond == nil { + return fmt.Errorf("null conditions list, but length is nonzero") } var retCode C.int @@ -273,6 +263,8 @@ func (f *ScmpFilter) addRuleWrapper(call ScmpSyscall, action ScmpAction, exact b return fmt.Errorf("unrecognized syscall") } else if syscall.Errno(-1*retCode) == syscall.EPERM { return fmt.Errorf("requested action matches default action of filter") + } else if syscall.Errno(-1*retCode) == syscall.EINVAL { + return fmt.Errorf("two checks on same syscall argument") } else if retCode != 0 { return syscall.Errno(-1 * retCode) } @@ -290,22 +282,32 @@ func (f *ScmpFilter) addRuleGeneric(call ScmpSyscall, action ScmpAction, exact b } if len(conds) == 0 { - if err := f.addRuleWrapper(call, action, exact, nil); err != nil { + if err := f.addRuleWrapper(call, action, exact, 0, nil); err != nil { return err } } else { // We don't support conditional filtering in library version v2.1 if !checkVersionAbove(2, 2, 1) { - return fmt.Errorf("conditional filtering requires libseccomp version >= 2.2.1") + return VersionError{ + message: "conditional filtering is not supported", + minimum: "2.2.1", + } } - for _, cond := range conds { - cmpStruct := C.make_struct_arg_cmp(C.uint(cond.Argument), cond.Op.toNative(), C.uint64_t(cond.Operand1), C.uint64_t(cond.Operand2)) - defer C.free(cmpStruct) + argsArr := C.make_arg_cmp_array(C.uint(len(conds))) + if argsArr == nil { + return fmt.Errorf("error allocating memory for conditions") + } + defer C.free(argsArr) - if err := f.addRuleWrapper(call, action, exact, C.scmp_cast_t(cmpStruct)); err != nil { - return err - } + for i, cond := range conds { + C.add_struct_arg_cmp(C.scmp_cast_t(argsArr), C.uint(i), + C.uint(cond.Argument), cond.Op.toNative(), + C.uint64_t(cond.Operand1), C.uint64_t(cond.Operand2)) + } + + if err := f.addRuleWrapper(call, action, exact, C.uint(len(conds)), C.scmp_cast_t(argsArr)); err != nil { + return err } } From bbc847a457c9b9e847f05cc3e7ff536312222e61 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 26 Apr 2017 13:38:34 -0400 Subject: [PATCH 2/3] Add integration tests for multi-argument Seccomp filters Signed-off-by: Matthew Heon --- libcontainer/integration/seccomp_test.go | 104 +++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/libcontainer/integration/seccomp_test.go b/libcontainer/integration/seccomp_test.go index 055f887f..767a149d 100644 --- a/libcontainer/integration/seccomp_test.go +++ b/libcontainer/integration/seccomp_test.go @@ -217,3 +217,107 @@ func TestSeccompDenyWriteConditional(t *testing.T) { t.Fatalf("Expected output %s but got %s\n", expected, actual) } } + +func TestSeccompPermitWriteMultipleConditions(t *testing.T) { + if testing.Short() { + return + } + + rootfs, err := newRootfs() + if err != nil { + t.Fatal(err) + } + defer remove(rootfs) + + config := newTemplateConfig(rootfs) + config.Seccomp = &configs.Seccomp{ + DefaultAction: configs.Allow, + Syscalls: []*configs.Syscall{ + { + Name: "write", + Action: configs.Errno, + Args: []*configs.Arg{ + { + Index: 0, + Value: 2, + Op: configs.EqualTo, + }, + { + Index: 2, + Value: 0, + Op: configs.NotEqualTo, + }, + }, + }, + }, + } + + 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) + } + // We don't need to verify the actual thing printed + // Just that something was written to stdout + if len(buffers.Stdout.String()) == 0 { + t.Fatalf("Nothing was written to stdout, write call failed!\n") + } +} + +func TestSeccompDenyWriteMultipleConditions(t *testing.T) { + if testing.Short() { + return + } + + // Only test if library version is v2.2.1 or higher + // Conditional filtering will always error in v2.2.0 and lower + major, minor, micro := libseccomp.GetLibraryVersion() + if (major == 2 && minor < 2) || (major == 2 && minor == 2 && micro < 1) { + return + } + + rootfs, err := newRootfs() + if err != nil { + t.Fatal(err) + } + defer remove(rootfs) + + config := newTemplateConfig(rootfs) + config.Seccomp = &configs.Seccomp{ + DefaultAction: configs.Allow, + Syscalls: []*configs.Syscall{ + { + Name: "write", + Action: configs.Errno, + Args: []*configs.Arg{ + { + Index: 0, + Value: 2, + Op: configs.EqualTo, + }, + { + Index: 2, + Value: 0, + Op: configs.NotEqualTo, + }, + }, + }, + }, + } + + 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) + } + + expected := "" + actual := strings.Trim(buffers.Stderr.String(), "\n") + if actual != expected { + t.Fatalf("Expected output %s but got %s\n", expected, actual) + } +} From 472fa3d05443083a41d58da9adc43e4722126e3c Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 12 May 2017 13:31:01 -0400 Subject: [PATCH 3/3] Update Travis config to use trusty-backports libseccomp Signed-off-by: Matthew Heon --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 98d50f30..95c3bd8c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,8 +20,9 @@ env: - BUILDTAGS="seccomp apparmor selinux ambient" before_install: + - echo "deb http://archive.ubuntu.com/ubuntu trusty-backports main restricted universe multiverse" | sudo tee -a /etc/apt/sources.list - sudo apt-get -qq update - - sudo apt-get install -y libseccomp-dev libapparmor-dev + - sudo apt-get install -y libapparmor-dev libseccomp-dev/trusty-backports - go get -u github.com/golang/lint/golint - go get -u github.com/vbatts/git-validation - env | grep TRAVIS_