From fbf35a65dc9b1438d131cbf9221a2814c18ffac1 Mon Sep 17 00:00:00 2001 From: "John R. Lenton" Date: Tue, 6 Mar 2018 14:00:41 +0100 Subject: progress: tweak ansimeter cvvis use to no longer confuse minicom minicom's vt220 implementation was improperly displaying half of cvvis. It seems it can't handle CPI codes separated by ;, and fails to ignore the unknown escape. This simplifies it, but it still does what we want (the cvvis we were using, taken from terminfo for xterm, was additionally doing an 'echo on', which we don't need). Additionally I received feedback about the spinner being nastier than the old slash-based one, so I moved to that. ... some day I might make that customisable :-) --- progress/ansimeter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/progress/ansimeter.go b/progress/ansimeter.go index c6007d6e65..30838ac4a4 100644 --- a/progress/ansimeter.go +++ b/progress/ansimeter.go @@ -51,7 +51,7 @@ var ( // make cursor invisible cursorInvisible = "\033[?25l" // make cursor visible - cursorVisible = "\033[?12;25h" + cursorVisible = "\033[?25h" // turn on reverse video enterReverseMode = "\033[7m" // go back to normal video @@ -152,7 +152,7 @@ func (p *ANSIMeter) Set(current float64) { fmt.Fprint(stdout, "\r", enterReverseMode, string(msg[:i]), exitAttributeMode, string(msg[i:])) } -var spinner = []string{".", "o", "O", "o"} +var spinner = []string{"/", "-", "\\", "|"} func (p *ANSIMeter) Spin(msgstr string) { msg := []rune(msgstr) -- cgit v1.2.3 From 1b05ab9be2257958c4a0baca286f8b79a01a300c Mon Sep 17 00:00:00 2001 From: Andrea Azzarone Date: Tue, 6 Mar 2018 16:21:05 +0100 Subject: polkit: ensure error is properly set if dialog is dismissed --- polkit/authority.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkit/authority.go b/polkit/authority.go index b39ca0ca92..a4093eaac1 100644 --- a/polkit/authority.go +++ b/polkit/authority.go @@ -49,7 +49,7 @@ func checkAuthorization(subject authSubject, actionId string, details map[string err = authority.Call( "org.freedesktop.PolicyKit1.Authority.CheckAuthorization", 0, subject, actionId, details, flags, "").Store(&result) - if err != nil && !result.IsAuthorized { + if err != nil || !result.IsAuthorized { if result.IsChallenge { err = ErrInteraction } else if result.Details["polkit.dismissed"] != "" { -- cgit v1.2.3 From 92443bedf9d2ebc8d18afabc3e439be7efeb9b1b Mon Sep 17 00:00:00 2001 From: James Henstridge Date: Thu, 8 Mar 2018 13:43:09 +0100 Subject: xdgopenproxy: integrate xdg-open implementation into snapctl (#4794) * xdgopenproxy: add a Go implementation of the xdg-open proxy client * xdgopenproxy: add tests * xdgopenproxy: test that passed file descriptor is for expected file * xdgopenproxy: fix copyright years --- cmd/snapctl/main.go | 8 ++ xdgopenproxy/export_test.go | 22 ++++++ xdgopenproxy/xdgopenproxy.go | 63 ++++++++++++++++ xdgopenproxy/xdgopenproxy_test.go | 155 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 248 insertions(+) create mode 100644 xdgopenproxy/export_test.go create mode 100644 xdgopenproxy/xdgopenproxy.go create mode 100644 xdgopenproxy/xdgopenproxy_test.go diff --git a/cmd/snapctl/main.go b/cmd/snapctl/main.go index bf71634378..4b825400ad 100644 --- a/cmd/snapctl/main.go +++ b/cmd/snapctl/main.go @@ -25,6 +25,7 @@ import ( "github.com/snapcore/snapd/client" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/xdgopenproxy" ) var clientConfig = client.Config{ @@ -46,6 +47,13 @@ func main() { os.Exit(1) } } + if len(os.Args) == 3 && os.Args[1] == "user-open" { + if err := xdgopenproxy.Run(os.Args[2]); err != nil { + fmt.Fprintf(os.Stderr, "user-open error: %v\n", err) + os.Exit(1) + } + os.Exit(0) + } // no internal command, route via snapd stdout, stderr, err := run() diff --git a/xdgopenproxy/export_test.go b/xdgopenproxy/export_test.go new file mode 100644 index 0000000000..1771008637 --- /dev/null +++ b/xdgopenproxy/export_test.go @@ -0,0 +1,22 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package xdgopenproxy + +var Launch = launch diff --git a/xdgopenproxy/xdgopenproxy.go b/xdgopenproxy/xdgopenproxy.go new file mode 100644 index 0000000000..60642bb50e --- /dev/null +++ b/xdgopenproxy/xdgopenproxy.go @@ -0,0 +1,63 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +// Package xdgopenproxy provides a client for snap userd's xdg-open D-Bus proxy +package xdgopenproxy + +import ( + "net/url" + "syscall" + + "github.com/godbus/dbus" +) + +func Run(urlOrFile string) error { + bus, err := dbus.SessionBus() + if err != nil { + return err + } + defer bus.Close() + launcher := bus.Object("io.snapcraft.Launcher", "/io/snapcraft/Launcher") + return launch(launcher, urlOrFile) +} + +func launch(launcher dbus.BusObject, urlOrFile string) error { + if u, err := url.Parse(urlOrFile); err == nil { + if u.Scheme == "file" { + return openFile(launcher, u.Path) + } else if u.Scheme != "" { + return openUrl(launcher, urlOrFile) + } + } + return openFile(launcher, urlOrFile) +} + +func openUrl(launcher dbus.BusObject, url string) error { + return launcher.Call("io.snapcraft.Launcher.OpenURL", 0, url).Err +} + +func openFile(launcher dbus.BusObject, filename string) error { + fd, err := syscall.Open(filename, syscall.O_RDONLY, 0) + if err != nil { + return err + } + defer syscall.Close(fd) + + return launcher.Call("io.snapcraft.Launcher.OpenFile", 0, "", dbus.UnixFD(fd)).Err +} diff --git a/xdgopenproxy/xdgopenproxy_test.go b/xdgopenproxy/xdgopenproxy_test.go new file mode 100644 index 0000000000..f62cceee81 --- /dev/null +++ b/xdgopenproxy/xdgopenproxy_test.go @@ -0,0 +1,155 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package xdgopenproxy_test + +import ( + "fmt" + "io/ioutil" + "net/url" + "os" + "path/filepath" + "syscall" + "testing" + + "github.com/godbus/dbus" + + . "gopkg.in/check.v1" + + "github.com/snapcore/snapd/xdgopenproxy" +) + +func Test(t *testing.T) { TestingT(t) } + +type xdgOpenSuite struct{} + +var _ = Suite(&xdgOpenSuite{}) + +func (s *xdgOpenSuite) TestOpenURL(c *C) { + launcher := fakeBusObject(func(method string, args ...interface{}) error { + c.Check(method, Equals, "io.snapcraft.Launcher.OpenURL") + c.Check(args, DeepEquals, []interface{}{"http://example.org"}) + return nil + }) + c.Check(xdgopenproxy.Launch(launcher, "http://example.org"), IsNil) +} + +func (s *xdgOpenSuite) TestOpenFile(c *C) { + path := filepath.Join(c.MkDir(), "test.txt") + c.Assert(ioutil.WriteFile(path, []byte("Hello world"), 0644), IsNil) + + launcher := fakeBusObject(func(method string, args ...interface{}) error { + c.Check(method, Equals, "io.snapcraft.Launcher.OpenFile") + c.Assert(args, HasLen, 2) + c.Check(args[0], Equals, "") + c.Check(fdMatchesFile(int(args[1].(dbus.UnixFD)), path), IsNil) + return nil + }) + c.Check(xdgopenproxy.Launch(launcher, path), IsNil) +} + +func (s *xdgOpenSuite) TestOpenFileURL(c *C) { + path := filepath.Join(c.MkDir(), "test.txt") + c.Assert(ioutil.WriteFile(path, []byte("Hello world"), 0644), IsNil) + + launcher := fakeBusObject(func(method string, args ...interface{}) error { + c.Check(method, Equals, "io.snapcraft.Launcher.OpenFile") + c.Assert(args, HasLen, 2) + c.Check(args[0], Equals, "") + c.Check(fdMatchesFile(int(args[1].(dbus.UnixFD)), path), IsNil) + return nil + }) + + u := url.URL{Scheme: "file", Path: path} + c.Check(xdgopenproxy.Launch(launcher, u.String()), IsNil) +} + +func (s *xdgOpenSuite) TestOpenDir(c *C) { + dir := c.MkDir() + + launcher := fakeBusObject(func(method string, args ...interface{}) error { + c.Check(method, Equals, "io.snapcraft.Launcher.OpenFile") + c.Assert(args, HasLen, 2) + c.Check(args[0], Equals, "") + c.Check(fdMatchesFile(int(args[1].(dbus.UnixFD)), dir), IsNil) + return nil + }) + c.Check(xdgopenproxy.Launch(launcher, dir), IsNil) +} + +func (s *xdgOpenSuite) TestOpenMissingFile(c *C) { + path := filepath.Join(c.MkDir(), "no-such-file.txt") + + launcher := fakeBusObject(func(method string, args ...interface{}) error { + c.Error("unexpected D-Bus call") + return nil + }) + c.Check(xdgopenproxy.Launch(launcher, path), ErrorMatches, "no such file or directory") +} + +func (s *xdgOpenSuite) TestOpenUnreadableFile(c *C) { + path := filepath.Join(c.MkDir(), "test.txt") + c.Assert(ioutil.WriteFile(path, []byte("Hello world"), 0644), IsNil) + c.Assert(os.Chmod(path, 0), IsNil) + + launcher := fakeBusObject(func(method string, args ...interface{}) error { + c.Error("unexpected D-Bus call") + return nil + }) + c.Check(xdgopenproxy.Launch(launcher, path), ErrorMatches, "permission denied") +} + +func fdMatchesFile(fd int, filename string) error { + var fdStat, fileStat syscall.Stat_t + if err := syscall.Fstat(fd, &fdStat); err != nil { + return err + } + if err := syscall.Stat(filename, &fileStat); err != nil { + return err + } + if fdStat.Dev != fileStat.Dev || fdStat.Ino != fileStat.Ino { + return fmt.Errorf("File descriptor and fd do not match") + } + return nil +} + +// fakeBusObject is a dbus.BusObject implementation that forwards +// Call invocations +type fakeBusObject func(method string, args ...interface{}) error + +func (f fakeBusObject) Call(method string, flags dbus.Flags, args ...interface{}) *dbus.Call { + err := f(method, args...) + return &dbus.Call{Err: err} +} + +func (f fakeBusObject) Go(method string, flags dbus.Flags, ch chan *dbus.Call, args ...interface{}) *dbus.Call { + return nil +} + +func (f fakeBusObject) GetProperty(prop string) (dbus.Variant, error) { + return dbus.Variant{}, nil +} + +func (f fakeBusObject) Destination() string { + return "" +} + +func (f fakeBusObject) Path() dbus.ObjectPath { + return "" +} -- cgit v1.2.3 From cf8e06572e0d3a963627667288749692e2c24848 Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Thu, 8 Mar 2018 14:14:27 +0100 Subject: snap-confine, snap-seccomp: utilize new seccomp logging features (#3998) * snap-confine, snap-seccomp: Default to SECCOMP_RET_ERRNO The seccomp policy has historically used SECCOMP_RET_KILL to forcefully kill a snap process that bumps into the walls of the sandbox. However, killing the snap is not very user friendly. Changing the policy to use SECCOMP_RET_ERRNO to return -1 with errno set to EPERM has been desired but the kernel would not log those denials which could leave users and developers confused about why their applications were experiencing errors. The 4.14 Linux kernel contains new seccomp logging controls which allows snapd to request SECCOMP_RET_ERRNO to be logged. This patch makes use of the new logging controls and switches the default action of the seccomp policy to SECCOMP_RET_ERRNO so that snaps aren't killed when the perform an illegal system call. Signed-off-by: Tyler Hicks --- cmd/snap-confine/seccomp-support.c | 31 ++++- cmd/snap-seccomp/export_test.go | 8 ++ cmd/snap-seccomp/main.go | 104 +++++++++++---- cmd/snap-seccomp/main_test.go | 253 +++++++++++++++++++------------------ interfaces/seccomp/backend.go | 3 + interfaces/seccomp/backend_test.go | 51 ++++++++ interfaces/system_key.go | 6 +- interfaces/system_key_test.go | 14 +- release/seccomp.go | 65 ++++++++++ release/seccomp_test.go | 50 ++++++++ vendor/vendor.json | 6 +- 11 files changed, 430 insertions(+), 161 deletions(-) create mode 100644 release/seccomp.go create mode 100644 release/seccomp_test.go diff --git a/cmd/snap-confine/seccomp-support.c b/cmd/snap-confine/seccomp-support.c index 76d8bde4e2..f8f86f2084 100644 --- a/cmd/snap-confine/seccomp-support.c +++ b/cmd/snap-confine/seccomp-support.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -35,6 +36,21 @@ #include "../libsnap-confine-private/string-utils.h" #include "../libsnap-confine-private/utils.h" +#ifndef SECCOMP_FILTER_FLAG_LOG +#define SECCOMP_FILTER_FLAG_LOG 2 +#endif + +#ifndef seccomp +// prototype because we build with -Wstrict-prototypes +int seccomp(unsigned int operation, unsigned int flags, void *args); + +int seccomp(unsigned int operation, unsigned int flags, void *args) +{ + errno = 0; + return syscall(__NR_seccomp, operation, flags, args); +} +#endif + static const char *filter_profile_dir = "/var/lib/snapd/seccomp/bpf/"; // MAX_BPF_SIZE is an arbitrary limit. @@ -202,8 +218,19 @@ int sc_apply_seccomp_bpf(const char *filter_profile) .len = num_read / sizeof(struct sock_filter), .filter = (struct sock_filter *)bpf, }; - if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog) != 0) { - die("cannot apply seccomp profile"); + if (seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_LOG, &prog) != 0) { + if (errno == ENOSYS) { + debug("kernel doesn't support the seccomp(2) syscall"); + } else if (errno == EINVAL) { + debug + ("kernel may not support the SECCOMP_FILTER_FLAG_LOG flag"); + } + + debug + ("falling back to prctl(2) syscall to load seccomp filter"); + if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog) != 0) { + die("cannot apply seccomp profile"); + } } // drop privileges again debug("dropping privileges after loading seccomp profile"); diff --git a/cmd/snap-seccomp/export_test.go b/cmd/snap-seccomp/export_test.go index c49bd4d5df..d2722dc126 100644 --- a/cmd/snap-seccomp/export_test.go +++ b/cmd/snap-seccomp/export_test.go @@ -39,3 +39,11 @@ func MockArchUbuntuKernelArchitecture(f func() string) (restore func()) { archUbuntuKernelArchitecture = realArchUbuntuKernelArchitecture } } + +func MockErrnoOnDenial(i int16) (retore func()) { + origErrnoOnDenial := errnoOnDenial + errnoOnDenial = i + return func() { + errnoOnDenial = origErrnoOnDenial + } +} diff --git a/cmd/snap-seccomp/main.go b/cmd/snap-seccomp/main.go index 54855a35ce..91b0425350 100644 --- a/cmd/snap-seccomp/main.go +++ b/cmd/snap-seccomp/main.go @@ -122,6 +122,9 @@ package main //#define SCMP_ARCH_S390X ARCH_BAD //#endif // +//#ifndef SECCOMP_RET_LOG +//#define SECCOMP_RET_LOG 0x7ffc0000U +//#endif // //typedef struct seccomp_data kernel_seccomp_data; // @@ -390,6 +393,7 @@ var seccompResolver = map[string]uint64{ const ( SeccompRetAllow = C.SECCOMP_RET_ALLOW SeccompRetKill = C.SECCOMP_RET_KILL + SeccompRetLog = C.SECCOMP_RET_LOG ) // UbuntuArchToScmpArch takes a dpkg architecture and converts it to @@ -610,49 +614,93 @@ func addSecondaryArches(secFilter *seccomp.ScmpFilter) error { return nil } +var errnoOnDenial int16 = C.EPERM + +func preprocess(content []byte) (unrestricted, complain bool) { + scanner := bufio.NewScanner(bytes.NewBuffer(content)) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + switch line { + case "@unrestricted": + unrestricted = true + case "@complain": + complain = true + } + } + return unrestricted, complain +} + +func complainAction() seccomp.ScmpAction { + // XXX: Work around some distributions not having a new enough + // libseccomp-golang that declares ActLog. Instead, we'll guess at its + // value by adding one to ActAllow and then verify that the string + // representation is what we expect for ActLog. The value and string is + // defined in https://github.com/seccomp/libseccomp-golang/pull/29. + // + // Ultimately, the fix for this workaround is to be able to use the + // GetApi() function created in the PR above. It'll tell us if the + // kernel, libseccomp, and libseccomp-golang all support ActLog. + var actLog seccomp.ScmpAction = seccomp.ActAllow + 1 + + if actLog.String() == "Action: Log system call" { + return actLog + } + + // Because ActLog is functionally ActAllow with logging, if we don't + // support ActLog, fallback to ActLog. + return seccomp.ActAllow +} + func compile(content []byte, out string) error { var err error var secFilter *seccomp.ScmpFilter - secFilter, err = seccomp.NewFilter(seccomp.ActKill) + unrestricted, complain := preprocess(content) + switch { + case unrestricted: + return osutil.AtomicWrite(out, bytes.NewBufferString("@unrestricted\n"), 0644, 0) + case complain: + var complainAct seccomp.ScmpAction = complainAction() + + secFilter, err = seccomp.NewFilter(complainAct) + if err != nil { + if complainAct != seccomp.ActAllow { + // ActLog is only supported in newer versions + // of the kernel, libseccomp, and + // libseccomp-golang. Attempt to fall back to + // ActAllow before erroring out. + complainAct = seccomp.ActAllow + secFilter, err = seccomp.NewFilter(complainAct) + } + } + + // Set unrestricted to 'true' to fallback to the pre-ActLog + // behavior of simply setting the allow filter without adding + // any rules. + if complainAct == seccomp.ActAllow { + unrestricted = true + } + default: + secFilter, err = seccomp.NewFilter(seccomp.ActErrno.SetReturnCode(errnoOnDenial)) + } if err != nil { return fmt.Errorf("cannot create seccomp filter: %s", err) } - if err := addSecondaryArches(secFilter); err != nil { return err } - scanner := bufio.NewScanner(bytes.NewBuffer(content)) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - - // special case: unrestricted means we stop early, we just - // write this special tag and evalulate in snap-confine - if line == "@unrestricted" { - return osutil.AtomicWrite(out, bytes.NewBufferString(line+"\n"), 0644, 0) - } - // complain mode is a "allow-all" filter for now until - // we can land https://github.com/snapcore/snapd/pull/3998 - if line == "@complain" { - secFilter, err = seccomp.NewFilter(seccomp.ActAllow) - if err != nil { - return fmt.Errorf("cannot create seccomp filter: %s", err) - } - if err := addSecondaryArches(secFilter); err != nil { - return err + if !unrestricted { + scanner := bufio.NewScanner(bytes.NewBuffer(content)) + for scanner.Scan() { + if err := parseLine(scanner.Text(), secFilter); err != nil { + return fmt.Errorf("cannot parse line: %s", err) } - break } - - // look for regular syscall/arg rule - if err := parseLine(line, secFilter); err != nil { - return fmt.Errorf("cannot parse line: %s", err) + if scanner.Err(); err != nil { + return err } } - if scanner.Err(); err != nil { - return err - } if osutil.GetenvBool("SNAP_SECCOMP_DEBUG") { secFilter.ExportPFC(os.Stdout) diff --git a/cmd/snap-seccomp/main_test.go b/cmd/snap-seccomp/main_test.go index ed315af9a0..8795170694 100644 --- a/cmd/snap-seccomp/main_test.go +++ b/cmd/snap-seccomp/main_test.go @@ -51,6 +51,11 @@ type snapSeccompSuite struct { var _ = Suite(&snapSeccompSuite{}) +const ( + Deny = iota + Allow +) + var seccompBpfLoaderContent = []byte(` #include #include @@ -127,32 +132,30 @@ int main(int argc, char* argv[]) var seccompSyscallRunnerContent = []byte(` #define _GNU_SOURCE +#include #include #include #include int main(int argc, char** argv) { - int l[7]; + int l[7], syscall_ret, ret = 0; for (int i = 0; i < 7; i++) l[i] = atoi(argv[i + 1]); // There might be architecture-specific requirements. see "man syscall" // for details. - syscall(l[0], l[1], l[2], l[3], l[4], l[5], l[6]); - syscall(SYS_exit, 0, 0, 0, 0, 0, 0); + 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) { + ret = 10; + } + syscall(SYS_exit, ret, 0, 0, 0, 0, 0); return 0; } `) -func lastKmsg() string { - output, err := exec.Command("dmesg").CombinedOutput() - if err != nil { - return err.Error() - } - l := strings.Split(string(output), "\n") - return fmt.Sprintf("Showing last 10 lines of dmesg:\n%s", strings.Join(l[len(l)-10:], "\n")) -} - func (s *snapSeccompSuite) SetUpSuite(c *C) { + main.MockErrnoOnDenial(911) + // build seccomp-load helper s.seccompBpfLoader = filepath.Join(c.MkDir(), "seccomp_bpf_loader") err := ioutil.WriteFile(s.seccompBpfLoader+".c", seccompBpfLoaderContent, 0644) @@ -312,13 +315,13 @@ restart_syscall cmd.Stderr = os.Stderr err = cmd.Run() switch expected { - case main.SeccompRetAllow: + case Allow: if err != nil { - c.Fatalf("unexpected error for %q (failed to run %q): %s", seccompWhitelist, lastKmsg(), err) + c.Fatalf("unexpected error for %q (failed to run %q)", seccompWhitelist, err) } - case main.SeccompRetKill: + case Deny: if err == nil { - c.Fatalf("unexpected success for %q %q (ran but should have failed %s)", seccompWhitelist, bpfInput, lastKmsg()) + c.Fatalf("unexpected success for %q %q (ran but should have failed)", seccompWhitelist, bpfInput) } default: c.Fatalf("unknown expected result %v", expected) @@ -343,8 +346,8 @@ func (s *snapSeccompSuite) TestUnrestricted(c *C) { // // Eg to test that the rule 'read >=2' is allowed with 'read(2)' and 'read(3)' // and denied with 'read(1)' and 'read(0)', add the following tests: -// {"read >=2", "read;native;2", main.SeccompRetAllow}, -// {"read >=2", "read;native;3", main.SeccompRetAllow}, +// {"read >=2", "read;native;2", Allow}, +// {"read >=2", "read;native;3", Allow}, // {"read >=2", "read;native;1", main.SeccompRetKill}, // {"read >=2", "read;native;0", main.SeccompRetKill}, func (s *snapSeccompSuite) TestCompile(c *C) { @@ -355,98 +358,98 @@ func (s *snapSeccompSuite) TestCompile(c *C) { expected int }{ // special - {"@complain", "execve", main.SeccompRetAllow}, + {"@complain", "execve", Allow}, // trivial allow - {"read", "read", main.SeccompRetAllow}, - {"read\nwrite\nexecve\n", "write", main.SeccompRetAllow}, + {"read", "read", Allow}, + {"read\nwrite\nexecve\n", "write", Allow}, // trivial denial - {"read", "ioctl", main.SeccompRetKill}, + {"read", "ioctl", Deny}, // test argument filtering syntax, we currently support: // >=, <=, !, <, >, | // modifiers. // reads >= 2 are ok - {"read >=2", "read;native;2", main.SeccompRetAllow}, - {"read >=2", "read;native;3", main.SeccompRetAllow}, + {"read >=2", "read;native;2", Allow}, + {"read >=2", "read;native;3", Allow}, // but not reads < 2, those get killed - {"read >=2", "read;native;1", main.SeccompRetKill}, - {"read >=2", "read;native;0", main.SeccompRetKill}, + {"read >=2", "read;native;1", Deny}, + {"read >=2", "read;native;0", Deny}, // reads <= 2 are ok - {"read <=2", "read;native;0", main.SeccompRetAllow}, - {"read <=2", "read;native;1", main.SeccompRetAllow}, - {"read <=2", "read;native;2", main.SeccompRetAllow}, + {"read <=2", "read;native;0", Allow}, + {"read <=2", "read;native;1", Allow}, + {"read <=2", "read;native;2", Allow}, // but not reads >2, those get killed - {"read <=2", "read;native;3", main.SeccompRetKill}, - {"read <=2", "read;native;4", main.SeccompRetKill}, + {"read <=2", "read;native;3", Deny}, + {"read <=2", "read;native;4", Deny}, // reads that are not 2 are ok - {"read !2", "read;native;1", main.SeccompRetAllow}, - {"read !2", "read;native;3", main.SeccompRetAllow}, + {"read !2", "read;native;1", Allow}, + {"read !2", "read;native;3", Allow}, // but not 2, this gets killed - {"read !2", "read;native;2", main.SeccompRetKill}, + {"read !2", "read;native;2", Deny}, // reads > 2 are ok - {"read >2", "read;native;4", main.SeccompRetAllow}, - {"read >2", "read;native;3", main.SeccompRetAllow}, + {"read >2", "read;native;4", Allow}, + {"read >2", "read;native;3", Allow}, // but not reads <= 2, those get killed - {"read >2", "read;native;2", main.SeccompRetKill}, - {"read >2", "read;native;1", main.SeccompRetKill}, + {"read >2", "read;native;2", Deny}, + {"read >2", "read;native;1", Deny}, // reads < 2 are ok - {"read <2", "read;native;0", main.SeccompRetAllow}, - {"read <2", "read;native;1", main.SeccompRetAllow}, + {"read <2", "read;native;0", Allow}, + {"read <2", "read;native;1", Allow}, // but not reads >= 2, those get killed - {"read <2", "read;native;2", main.SeccompRetKill}, - {"read <2", "read;native;3", main.SeccompRetKill}, + {"read <2", "read;native;2", Deny}, + {"read <2", "read;native;3", Deny}, // FIXME: test maskedEqual better - {"read |1", "read;native;1", main.SeccompRetAllow}, - {"read |1", "read;native;2", main.SeccompRetKill}, + {"read |1", "read;native;1", Allow}, + {"read |1", "read;native;2", Deny}, // exact match, reads == 2 are ok - {"read 2", "read;native;2", main.SeccompRetAllow}, + {"read 2", "read;native;2", Allow}, // but not those != 2 - {"read 2", "read;native;3", main.SeccompRetKill}, - {"read 2", "read;native;1", main.SeccompRetKill}, + {"read 2", "read;native;3", Deny}, + {"read 2", "read;native;1", Deny}, // test actual syscalls and their expected usage - {"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", main.SeccompRetAllow}, - {"ioctl - TIOCSTI", "ioctl;native;-,99", main.SeccompRetKill}, - {"ioctl - !TIOCSTI", "ioctl;native;-,TIOCSTI", main.SeccompRetKill}, + {"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", Allow}, + {"ioctl - TIOCSTI", "ioctl;native;-,99", Deny}, + {"ioctl - !TIOCSTI", "ioctl;native;-,TIOCSTI", Deny}, // test_bad_seccomp_filter_args_clone - {"setns - CLONE_NEWNET", "setns;native;-,99", main.SeccompRetKill}, - {"setns - CLONE_NEWNET", "setns;native;-,CLONE_NEWNET", main.SeccompRetAllow}, + {"setns - CLONE_NEWNET", "setns;native;-,99", Deny}, + {"setns - CLONE_NEWNET", "setns;native;-,CLONE_NEWNET", Allow}, // test_bad_seccomp_filter_args_mknod - {"mknod - |S_IFIFO", "mknod;native;-,S_IFIFO", main.SeccompRetAllow}, - {"mknod - |S_IFIFO", "mknod;native;-,99", main.SeccompRetKill}, + {"mknod - |S_IFIFO", "mknod;native;-,S_IFIFO", Allow}, + {"mknod - |S_IFIFO", "mknod;native;-,99", Deny}, // test_bad_seccomp_filter_args_prctl - {"prctl PR_CAP_AMBIENT_RAISE", "prctl;native;PR_CAP_AMBIENT_RAISE", main.SeccompRetAllow}, - {"prctl PR_CAP_AMBIENT_RAISE", "prctl;native;99", main.SeccompRetKill}, + {"prctl PR_CAP_AMBIENT_RAISE", "prctl;native;PR_CAP_AMBIENT_RAISE", Allow}, + {"prctl PR_CAP_AMBIENT_RAISE", "prctl;native;99", Deny}, // test_bad_seccomp_filter_args_prio - {"setpriority PRIO_PROCESS 0 >=0", "setpriority;native;PRIO_PROCESS,0,19", main.SeccompRetAllow}, - {"setpriority PRIO_PROCESS 0 >=0", "setpriority;native;99", main.SeccompRetKill}, + {"setpriority PRIO_PROCESS 0 >=0", "setpriority;native;PRIO_PROCESS,0,19", Allow}, + {"setpriority PRIO_PROCESS 0 >=0", "setpriority;native;99", Deny}, // test_bad_seccomp_filter_args_quotactl - {"quotactl Q_GETQUOTA", "quotactl;native;Q_GETQUOTA", main.SeccompRetAllow}, - {"quotactl Q_GETQUOTA", "quotactl;native;99", main.SeccompRetKill}, + {"quotactl Q_GETQUOTA", "quotactl;native;Q_GETQUOTA", Allow}, + {"quotactl Q_GETQUOTA", "quotactl;native;99", Deny}, // test_bad_seccomp_filter_args_termios - {"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", main.SeccompRetAllow}, - {"ioctl - TIOCSTI", "ioctl;native;-,99", main.SeccompRetKill}, + {"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", Allow}, + {"ioctl - TIOCSTI", "ioctl;native;-,99", Deny}, // u:root g:root - {"fchown - u:root g:root", "fchown;native;-,0,0", main.SeccompRetAllow}, - {"fchown - u:root g:root", "fchown;native;-,99,0", main.SeccompRetKill}, - {"chown - u:root g:root", "chown;native;-,0,0", main.SeccompRetAllow}, - {"chown - u:root g:root", "chown;native;-,99,0", main.SeccompRetKill}, + {"fchown - u:root g:root", "fchown;native;-,0,0", Allow}, + {"fchown - u:root g:root", "fchown;native;-,99,0", Deny}, + {"chown - u:root g:root", "chown;native;-,0,0", Allow}, + {"chown - u:root g:root", "chown;native;-,99,0", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -469,12 +472,12 @@ func (s *snapSeccompSuite) TestCompileSocket(c *C) { }{ // test_bad_seccomp_filter_args_socket - {"socket AF_UNIX", "socket;native;AF_UNIX", main.SeccompRetAllow}, - {"socket AF_UNIX", "socket;native;99", main.SeccompRetKill}, - {"socket - SOCK_STREAM", "socket;native;-,SOCK_STREAM", main.SeccompRetAllow}, - {"socket - SOCK_STREAM", "socket;native;-,99", main.SeccompRetKill}, - {"socket AF_CONN", "socket;native;AF_CONN", main.SeccompRetAllow}, - {"socket AF_CONN", "socket;native;99", main.SeccompRetKill}, + {"socket AF_UNIX", "socket;native;AF_UNIX", Allow}, + {"socket AF_UNIX", "socket;native;99", Deny}, + {"socket - SOCK_STREAM", "socket;native;-,SOCK_STREAM", Allow}, + {"socket - SOCK_STREAM", "socket;native;-,99", Deny}, + {"socket AF_CONN", "socket;native;AF_CONN", Allow}, + {"socket AF_CONN", "socket;native;99", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -585,15 +588,15 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsSocket(c *C) { seccompWhitelist := fmt.Sprintf("socket %s_%s", pre, i) bpfInputGood := fmt.Sprintf("socket;native;%s_%s", pre, i) bpfInputBad := "socket;native;99999" - s.runBpf(c, seccompWhitelist, bpfInputGood, main.SeccompRetAllow) - s.runBpf(c, seccompWhitelist, bpfInputBad, main.SeccompRetKill) + s.runBpf(c, seccompWhitelist, bpfInputGood, Allow) + s.runBpf(c, seccompWhitelist, bpfInputBad, Deny) for _, j := range []string{"SOCK_STREAM", "SOCK_DGRAM", "SOCK_SEQPACKET", "SOCK_RAW", "SOCK_RDM", "SOCK_PACKET"} { seccompWhitelist := fmt.Sprintf("socket %s_%s %s", pre, i, j) bpfInputGood := fmt.Sprintf("socket;native;%s_%s,%s", pre, i, j) bpfInputBad := fmt.Sprintf("socket;native;%s_%s,9999", pre, i) - s.runBpf(c, seccompWhitelist, bpfInputGood, main.SeccompRetAllow) - s.runBpf(c, seccompWhitelist, bpfInputBad, main.SeccompRetKill) + s.runBpf(c, seccompWhitelist, bpfInputGood, Allow) + s.runBpf(c, seccompWhitelist, bpfInputBad, Deny) } } } @@ -603,8 +606,8 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsSocket(c *C) { seccompWhitelist := fmt.Sprintf("socket %s - %s", j, i) bpfInputGood := fmt.Sprintf("socket;native;%s,0,%s", j, i) bpfInputBad := fmt.Sprintf("socket;native;%s,0,99", j) - s.runBpf(c, seccompWhitelist, bpfInputGood, main.SeccompRetAllow) - s.runBpf(c, seccompWhitelist, bpfInputBad, main.SeccompRetKill) + s.runBpf(c, seccompWhitelist, bpfInputGood, Allow) + s.runBpf(c, seccompWhitelist, bpfInputBad, Deny) } } } @@ -615,10 +618,10 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsQuotactl(c *C) { // good input seccompWhitelist := fmt.Sprintf("quotactl %s", arg) bpfInputGood := fmt.Sprintf("quotactl;native;%s", arg) - s.runBpf(c, seccompWhitelist, bpfInputGood, main.SeccompRetAllow) + s.runBpf(c, seccompWhitelist, bpfInputGood, Allow) // bad input for _, bad := range []string{"quotactl;native;99999", "read;native;"} { - s.runBpf(c, seccompWhitelist, bad, main.SeccompRetKill) + s.runBpf(c, seccompWhitelist, bad, Deny) } } } @@ -629,22 +632,22 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsPrctl(c *C) { // good input seccompWhitelist := fmt.Sprintf("prctl %s", arg) bpfInputGood := fmt.Sprintf("prctl;native;%s", arg) - s.runBpf(c, seccompWhitelist, bpfInputGood, main.SeccompRetAllow) + s.runBpf(c, seccompWhitelist, bpfInputGood, Allow) // bad input for _, bad := range []string{"prctl;native;99999", "setpriority;native;"} { - s.runBpf(c, seccompWhitelist, bad, main.SeccompRetKill) + s.runBpf(c, seccompWhitelist, bad, Deny) } if arg == "PR_CAP_AMBIENT" { for _, j := range []string{"PR_CAP_AMBIENT_RAISE", "PR_CAP_AMBIENT_LOWER", "PR_CAP_AMBIENT_IS_SET", "PR_CAP_AMBIENT_CLEAR_ALL"} { seccompWhitelist := fmt.Sprintf("prctl %s %s", arg, j) bpfInputGood := fmt.Sprintf("prctl;native;%s,%s", arg, j) - s.runBpf(c, seccompWhitelist, bpfInputGood, main.SeccompRetAllow) + s.runBpf(c, seccompWhitelist, bpfInputGood, Allow) for _, bad := range []string{ fmt.Sprintf("prctl;native;%s,99999", arg), "setpriority;native;", } { - s.runBpf(c, seccompWhitelist, bad, main.SeccompRetKill) + s.runBpf(c, seccompWhitelist, bad, Deny) } } } @@ -659,19 +662,19 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsClone(c *C) { expected int }{ // good input - {"setns - CLONE_NEWIPC", "setns;native;-,CLONE_NEWIPC", main.SeccompRetAllow}, - {"setns - CLONE_NEWNET", "setns;native;-,CLONE_NEWNET", main.SeccompRetAllow}, - {"setns - CLONE_NEWNS", "setns;native;-,CLONE_NEWNS", main.SeccompRetAllow}, - {"setns - CLONE_NEWPID", "setns;native;-,CLONE_NEWPID", main.SeccompRetAllow}, - {"setns - CLONE_NEWUSER", "setns;native;-,CLONE_NEWUSER", main.SeccompRetAllow}, - {"setns - CLONE_NEWUTS", "setns;native;-,CLONE_NEWUTS", main.SeccompRetAllow}, + {"setns - CLONE_NEWIPC", "setns;native;-,CLONE_NEWIPC", Allow}, + {"setns - CLONE_NEWNET", "setns;native;-,CLONE_NEWNET", Allow}, + {"setns - CLONE_NEWNS", "setns;native;-,CLONE_NEWNS", Allow}, + {"setns - CLONE_NEWPID", "setns;native;-,CLONE_NEWPID", Allow}, + {"setns - CLONE_NEWUSER", "setns;native;-,CLONE_NEWUSER", Allow}, + {"setns - CLONE_NEWUTS", "setns;native;-,CLONE_NEWUTS", Allow}, // bad input - {"setns - CLONE_NEWIPC", "setns;native;-,99", main.SeccompRetKill}, - {"setns - CLONE_NEWNET", "setns;native;-,99", main.SeccompRetKill}, - {"setns - CLONE_NEWNS", "setns;native;-,99", main.SeccompRetKill}, - {"setns - CLONE_NEWPID", "setns;native;-,99", main.SeccompRetKill}, - {"setns - CLONE_NEWUSER", "setns;native;-,99", main.SeccompRetKill}, - {"setns - CLONE_NEWUTS", "setns;native;-,99", main.SeccompRetKill}, + {"setns - CLONE_NEWIPC", "setns;native;-,99", Deny}, + {"setns - CLONE_NEWNET", "setns;native;-,99", Deny}, + {"setns - CLONE_NEWNS", "setns;native;-,99", Deny}, + {"setns - CLONE_NEWPID", "setns;native;-,99", Deny}, + {"setns - CLONE_NEWUSER", "setns;native;-,99", Deny}, + {"setns - CLONE_NEWUTS", "setns;native;-,99", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -685,17 +688,17 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsMknod(c *C) { expected int }{ // good input - {"mknod - S_IFREG", "mknod;native;-,S_IFREG", main.SeccompRetAllow}, - {"mknod - S_IFCHR", "mknod;native;-,S_IFCHR", main.SeccompRetAllow}, - {"mknod - S_IFBLK", "mknod;native;-,S_IFBLK", main.SeccompRetAllow}, - {"mknod - S_IFIFO", "mknod;native;-,S_IFIFO", main.SeccompRetAllow}, - {"mknod - S_IFSOCK", "mknod;native;-,S_IFSOCK", main.SeccompRetAllow}, + {"mknod - S_IFREG", "mknod;native;-,S_IFREG", Allow}, + {"mknod - S_IFCHR", "mknod;native;-,S_IFCHR", Allow}, + {"mknod - S_IFBLK", "mknod;native;-,S_IFBLK", Allow}, + {"mknod - S_IFIFO", "mknod;native;-,S_IFIFO", Allow}, + {"mknod - S_IFSOCK", "mknod;native;-,S_IFSOCK", Allow}, // bad input - {"mknod - S_IFREG", "mknod;native;-,999", main.SeccompRetKill}, - {"mknod - S_IFCHR", "mknod;native;-,999", main.SeccompRetKill}, - {"mknod - S_IFBLK", "mknod;native;-,999", main.SeccompRetKill}, - {"mknod - S_IFIFO", "mknod;native;-,999", main.SeccompRetKill}, - {"mknod - S_IFSOCK", "mknod;native;-,999", main.SeccompRetKill}, + {"mknod - S_IFREG", "mknod;native;-,999", Deny}, + {"mknod - S_IFCHR", "mknod;native;-,999", Deny}, + {"mknod - S_IFBLK", "mknod;native;-,999", Deny}, + {"mknod - S_IFIFO", "mknod;native;-,999", Deny}, + {"mknod - S_IFSOCK", "mknod;native;-,999", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -709,13 +712,13 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsPrio(c *C) { expected int }{ // good input - {"setpriority PRIO_PROCESS", "setpriority;native;PRIO_PROCESS", main.SeccompRetAllow}, - {"setpriority PRIO_PGRP", "setpriority;native;PRIO_PGRP", main.SeccompRetAllow}, - {"setpriority PRIO_USER", "setpriority;native;PRIO_USER", main.SeccompRetAllow}, + {"setpriority PRIO_PROCESS", "setpriority;native;PRIO_PROCESS", Allow}, + {"setpriority PRIO_PGRP", "setpriority;native;PRIO_PGRP", Allow}, + {"setpriority PRIO_USER", "setpriority;native;PRIO_USER", Allow}, // bad input - {"setpriority PRIO_PROCESS", "setpriority;native;99", main.SeccompRetKill}, - {"setpriority PRIO_PGRP", "setpriority;native;99", main.SeccompRetKill}, - {"setpriority PRIO_USER", "setpriority;native;99", main.SeccompRetKill}, + {"setpriority PRIO_PROCESS", "setpriority;native;99", Deny}, + {"setpriority PRIO_PGRP", "setpriority;native;99", Deny}, + {"setpriority PRIO_USER", "setpriority;native;99", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -729,9 +732,9 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsTermios(c *C) { expected int }{ // good input - {"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", main.SeccompRetAllow}, + {"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", Allow}, // bad input - {"ioctl - TIOCSTI", "quotactl;native;-,99", main.SeccompRetKill}, + {"ioctl - TIOCSTI", "quotactl;native;-,99", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -750,17 +753,15 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsUidGid(c *C) { }{ // good input. 'root' is guaranteed to be '0' and 'daemon' uid // was determined at runtime - {"setuid u:root", "setuid;native;0", main.SeccompRetAllow}, - {"setuid u:daemon", fmt.Sprintf("setuid;native;%v", daemonUid), - main.SeccompRetAllow}, - {"setgid g:root", "setgid;native;0", main.SeccompRetAllow}, - {"setgid g:daemon", fmt.Sprintf("setgid;native;%v", daemonUid), - main.SeccompRetAllow}, + {"setuid u:root", "setuid;native;0", Allow}, + {"setuid u:daemon", fmt.Sprintf("setuid;native;%v", daemonUid), Allow}, + {"setgid g:root", "setgid;native;0", Allow}, + {"setgid g:daemon", fmt.Sprintf("setgid;native;%v", daemonUid), Allow}, // bad input - {"setuid u:root", "setuid;native;99", main.SeccompRetKill}, - {"setuid u:daemon", "setuid;native;99", main.SeccompRetKill}, - {"setgid g:root", "setgid;native;99", main.SeccompRetKill}, - {"setgid g:daemon", "setgid;native;99", main.SeccompRetKill}, + {"setuid u:root", "setuid;native;99", Deny}, + {"setuid u:daemon", "setuid;native;99", Deny}, + {"setgid g:root", "setgid;native;99", Deny}, + {"setgid g:daemon", "setgid;native;99", Deny}, } { s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected) } @@ -774,8 +775,8 @@ func (s *snapSeccompSuite) TestCompatArchWorks(c *C) { expected int }{ // on amd64 we add compat i386 - {"amd64", "read", "read;i386", main.SeccompRetAllow}, - {"amd64", "read", "read;amd64", main.SeccompRetAllow}, + {"amd64", "read", "read;i386", Allow}, + {"amd64", "read", "read;amd64", Allow}, } { // It is tricky to mock the architecture here because // seccomp is always adding the native arch to the seccomp diff --git a/interfaces/seccomp/backend.go b/interfaces/seccomp/backend.go index 81c19bf114..d72bd3a506 100644 --- a/interfaces/seccomp/backend.go +++ b/interfaces/seccomp/backend.go @@ -167,6 +167,9 @@ func addContent(securityTag string, opts interfaces.ConfinementOptions, snippetF if opts.DevMode && !opts.JailMode { // NOTE: This is understood by snap-confine buffer.WriteString("@complain\n") + if !release.SecCompSupportsAction("log") { + buffer.WriteString("# complain mode logging unavailable\n") + } } buffer.Write(defaultTemplate) diff --git a/interfaces/seccomp/backend_test.go b/interfaces/seccomp/backend_test.go index cec2bcdc18..55fa5b3feb 100644 --- a/interfaces/seccomp/backend_test.go +++ b/interfaces/seccomp/backend_test.go @@ -263,6 +263,8 @@ var combineSnippetsScenarios = []combineSnippetsScenario{{ func (s *backendSuite) TestCombineSnippets(c *C) { restore := release.MockForcedDevmode(false) defer restore() + restore = release.MockSecCompActions([]string{"log"}) + defer restore() // NOTE: replace the real template with a shorter variant restore = seccomp.MockTemplate([]byte("default\n")) @@ -334,3 +336,52 @@ func (s *backendSuite) TestBindIsAddedForForcedDevModeSystems(c *C) { profile := filepath.Join(dirs.SnapSeccompDir, "snap.samba.smbd") c.Assert(profile+".src", testutil.FileContains, "\nbind\n") } + +const ClassicYamlV1 = ` +name: test-classic +version: 1 +developer: acme +confinement: classic +apps: + sh: + ` + +func (s *backendSuite) TestSystemKeyRetLogSupported(c *C) { + restore := release.MockSecCompActions([]string{"allow", "errno", "kill", "log", "trace", "trap"}) + defer restore() + + snapInfo := s.InstallSnap(c, interfaces.ConfinementOptions{DevMode: true}, ifacetest.SambaYamlV1, 0) + profile := filepath.Join(dirs.SnapSeccompDir, "snap.samba.smbd") + c.Assert(profile+".src", Not(testutil.FileContains), "# complain mode logging unavailable\n") + s.RemoveSnap(c, snapInfo) + + snapInfo = s.InstallSnap(c, interfaces.ConfinementOptions{DevMode: false}, ifacetest.SambaYamlV1, 0) + profile = filepath.Join(dirs.SnapSeccompDir, "snap.samba.smbd") + c.Assert(profile+".src", Not(testutil.FileContains), "# complain mode logging unavailable\n") + s.RemoveSnap(c, snapInfo) + + snapInfo = s.InstallSnap(c, interfaces.ConfinementOptions{Classic: true}, ClassicYamlV1, 0) + profile = filepath.Join(dirs.SnapSeccompDir, "snap.test-classic.sh") + c.Assert(profile+".src", Not(testutil.FileContains), "# complain mode logging unavailable\n") + s.RemoveSnap(c, snapInfo) +} + +func (s *backendSuite) TestSystemKeyRetLogUnsupported(c *C) { + restore := release.MockSecCompActions([]string{"allow", "errno", "kill", "trace", "trap"}) + defer restore() + + snapInfo := s.InstallSnap(c, interfaces.ConfinementOptions{DevMode: true}, ifacetest.SambaYamlV1, 0) + profile := filepath.Join(dirs.SnapSeccompDir, "snap.samba.smbd") + c.Assert(profile+".src", testutil.FileContains, "# complain mode logging unavailable\n") + s.RemoveSnap(c, snapInfo) + + snapInfo = s.InstallSnap(c, interfaces.ConfinementOptions{DevMode: false}, ifacetest.SambaYamlV1, 0) + profile = filepath.Join(dirs.SnapSeccompDir, "snap.samba.smbd") + c.Assert(profile+".src", Not(testutil.FileContains), "# complain mode logging unavailable\n") + s.RemoveSnap(c, snapInfo) + + snapInfo = s.InstallSnap(c, interfaces.ConfinementOptions{Classic: true}, ClassicYamlV1, 0) + profile = filepath.Join(dirs.SnapSeccompDir, "snap.test-classic.sh") + c.Assert(profile+".src", Not(testutil.FileContains), "# complain mode logging unavailable\n") + s.RemoveSnap(c, snapInfo) +} diff --git a/interfaces/system_key.go b/interfaces/system_key.go index cec587b234..cbfb6a22c0 100644 --- a/interfaces/system_key.go +++ b/interfaces/system_key.go @@ -41,6 +41,7 @@ type systemKey struct { NFSHome bool `yaml:"nfs-home"` OverlayRoot string `yaml:"overlay-root"` Core string `yaml:"core,omitempty"` + SecCompActions []string `yaml:"seccomp-features"` } var ( @@ -61,7 +62,7 @@ func generateSystemKey() *systemKey { } sk.BuildID = buildID - // Add apparmor-feature (which is already sorted) + // Add apparmor-features (which is already sorted) sk.AppArmorFeatures = release.AppArmorFeatures() // Add if home is using NFS, if so we need to have a different @@ -86,6 +87,9 @@ func generateSystemKey() *systemKey { // FIXME: what about core18? the snapd snap? sk.Core, _ = os.Readlink(filepath.Join(dirs.SnapMountDir, "core/current")) + // Add seccomp-features + sk.SecCompActions = release.SecCompActions + return &sk } diff --git a/interfaces/system_key_test.go b/interfaces/system_key_test.go index 10756bd146..486e363830 100644 --- a/interfaces/system_key_test.go +++ b/interfaces/system_key_test.go @@ -58,6 +58,9 @@ func (s *systemKeySuite) TestInterfaceSystemKey(c *C) { restore := interfaces.MockIsHomeUsingNFS(func() (bool, error) { return false, nil }) defer restore() + restore2 := release.MockSecCompActions([]string{"allow", "errno", "kill", "log", "trace", "trap"}) + defer restore2() + systemKey := interfaces.SystemKey() apparmorFeatures := release.AppArmorFeatures() @@ -67,6 +70,15 @@ func (s *systemKeySuite) TestInterfaceSystemKey(c *C) { } else { apparmorFeaturesStr = "\n- " + strings.Join(apparmorFeatures, "\n- ") + "\n" } + + seccompActions := release.SecCompActions + var seccompActionsStr string + if len(seccompActions) == 0 { + seccompActionsStr = " []\n" + } else { + seccompActionsStr = "\n- " + strings.Join(seccompActions, "\n- ") + "\n" + } + nfsHome, err := osutil.IsHomeUsingNFS() c.Assert(err, IsNil) overlayRoot, err := osutil.IsRootWritableOverlay() @@ -74,7 +86,7 @@ func (s *systemKeySuite) TestInterfaceSystemKey(c *C) { c.Check(systemKey, Equals, fmt.Sprintf(`build-id: %s apparmor-features:%snfs-home: %v overlay-root: "%v" -`, s.buildID, apparmorFeaturesStr, nfsHome, overlayRoot)) +seccomp-features:%s`, s.buildID, apparmorFeaturesStr, nfsHome, overlayRoot, seccompActionsStr)) } func (ts *systemKeySuite) TestInterfaceDigest(c *C) { diff --git a/release/seccomp.go b/release/seccomp.go new file mode 100644 index 0000000000..2c456a0478 --- /dev/null +++ b/release/seccomp.go @@ -0,0 +1,65 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package release + +import ( + "io/ioutil" + "sort" + "strings" +) + +var ( + secCompAvailableActionsPath = "/proc/sys/kernel/seccomp/actions_avail" +) + +var SecCompActions []string + +func init() { + SecCompActions = getSecCompActions() +} + +func MockSecCompActions(actions []string) (restore func()) { + old := SecCompActions + SecCompActions = actions + return func() { SecCompActions = old } +} + +// SecCompActions returns a sorted list of seccomp actions like +// []string{"allow", "errno", "kill", "log", "trace", "trap"}. +func getSecCompActions() []string { + var actions []string + contents, err := ioutil.ReadFile(secCompAvailableActionsPath) + if err != nil { + return actions + } + + seccompActions := strings.Split(strings.TrimRight(string(contents), "\n"), " ") + sort.Strings(seccompActions) + + return seccompActions +} + +func SecCompSupportsAction(action string) bool { + i := sort.SearchStrings(SecCompActions, action) + if i < len(SecCompActions) && SecCompActions[i] == action { + return true + } + return false +} diff --git a/release/seccomp_test.go b/release/seccomp_test.go new file mode 100644 index 0000000000..851a357e7c --- /dev/null +++ b/release/seccomp_test.go @@ -0,0 +1,50 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package release_test + +import ( + . "gopkg.in/check.v1" + + "github.com/snapcore/snapd/release" +) + +type seccompSuite struct{} + +var _ = Suite(&seccompSuite{}) + +func (s *seccompSuite) TestInterfaceSystemKey(c *C) { + reset := release.MockSecCompActions([]string{}) + defer reset() + c.Check(release.SecCompActions, DeepEquals, []string{}) + + reset = release.MockSecCompActions([]string{"allow", "errno", "kill", "log", "trace", "trap"}) + defer reset() + c.Check(release.SecCompActions, DeepEquals, []string{"allow", "errno", "kill", "log", "trace", "trap"}) +} + +func (s *seccompSuite) TestSecCompSupportsAction(c *C) { + reset := release.MockSecCompActions([]string{}) + defer reset() + c.Check(release.SecCompSupportsAction("log"), Equals, false) + + reset = release.MockSecCompActions([]string{"allow", "errno", "kill", "log", "trace", "trap"}) + defer reset() + c.Check(release.SecCompSupportsAction("log"), Equals, true) +} diff --git a/vendor/vendor.json b/vendor/vendor.json index 9f47e495f5..136d157e6a 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -49,10 +49,10 @@ "revisionTime": "2015-02-12T09:37:50Z" }, { - "checksumSHA1": "EoJqr2ZG7jODFsCnKwrn4JWRS+Y=", + "checksumSHA1": "qUzU7BwMvcmsdozh1XVd0SAarAY=", "path": "github.com/mvo5/libseccomp-golang", - "revision": "84e1d1c75beaa58be6a76d2fc94d95eb8c1583b6", - "revisionTime": "2017-06-14T13:46:31Z" + "revision": "e0e036d8f7d25f0c63e96896b99547e9d5f71617", + "revisionTime": "2017-10-05T08:38:35Z" }, { "checksumSHA1": "lG6diF/yE9cGgQIKRAlsaeYAjO4=", -- cgit v1.2.3 From 4bd7644b8f1f7c434d8306ebdb5c342317643648 Mon Sep 17 00:00:00 2001 From: "John R. Lenton" Date: Thu, 8 Mar 2018 14:19:06 +0100 Subject: cmd/snap-seccomp: Cancel the atomic file on error, not just Close We had a minor bug in cmd/snap-seccomp/main.go, where we did essentially fout, _ := osutil.NewAtomicFile(...) defer fout.Close() if err := doStuff(fout); err != nil { return err } return fout.Commit() which, on error, would leave the tempfile lying around. This changes the Close to a Cancel, which does the right thing on error (and becomes a NOP once the atomic file is successfully committed). --- cmd/snap-seccomp/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/snap-seccomp/main.go b/cmd/snap-seccomp/main.go index 91b0425350..a4627d1cf3 100644 --- a/cmd/snap-seccomp/main.go +++ b/cmd/snap-seccomp/main.go @@ -711,7 +711,8 @@ func compile(content []byte, out string) error { if err != nil { return err } - defer fout.Close() + // Cancel once Committed is a NOP + defer fout.Cancel() if err := secFilter.ExportBPF(fout.File); err != nil { return err -- cgit v1.2.3 From fd048f8b470e35dfcb6b31901cbb23bcf0caddd0 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Thu, 8 Mar 2018 15:10:00 +0100 Subject: polkit: do not shadow dbus errors, avoid panic in case of errors With the current code, when the dbus call fails with an error we will shadow it with our own error. In the process of doing so, we try to access a map in authResult that is uninitialized, accessing a key will cause a panic. Signed-off-by: Maciej Borzecki --- polkit/authority.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/polkit/authority.go b/polkit/authority.go index a4093eaac1..fe35541b2c 100644 --- a/polkit/authority.go +++ b/polkit/authority.go @@ -49,7 +49,10 @@ func checkAuthorization(subject authSubject, actionId string, details map[string err = authority.Call( "org.freedesktop.PolicyKit1.Authority.CheckAuthorization", 0, subject, actionId, details, flags, "").Store(&result) - if err != nil || !result.IsAuthorized { + if err != nil { + return false, err + } + if !result.IsAuthorized { if result.IsChallenge { err = ErrInteraction } else if result.Details["polkit.dismissed"] != "" { -- cgit v1.2.3