diff --git a/cmd/snap-confine/seccomp-support.c b/cmd/snap-confine/seccomp-support.c index 7432d79cb152..47735391569b 100644 --- a/cmd/snap-confine/seccomp-support.c +++ b/cmd/snap-confine/seccomp-support.c @@ -17,11 +17,13 @@ #include "config.h" #include "seccomp-support.h" +#include #include #include #include #include #include +#include #include #include #include @@ -45,6 +47,31 @@ static const char *filter_profile_dir = "/var/lib/snapd/seccomp/bpf/"; typedef struct sock_filter bpf_instr; +// keep in sync with snap-seccomp/main.go +// +// header of a seccomp.bin2 filter file, in network byte order (big-endian) +// +// Note that padding is important +// alternatively "__attribute__((__packed__))" could be used +struct sc_seccomp_file_header { + // header: "SC" + char header[2]; + // version: 0x1 + char version; + // flags + char unrestricted; + // unused + char padding[4]; + + // size of allow filter in byte + uint32_t len_allow_filter; + // size of deny filter in byte + uint32_t len_deny_filter; + // reserved for future use + char reserved2[112]; +}; +static_assert(sizeof(struct sc_seccomp_file_header) == 128,"unexpected struct size"); + static void validate_path_has_strict_perms(const char *path) { struct stat stat_buf; @@ -109,12 +136,72 @@ static void validate_bpfpath_is_safe(const char *path) } } +static void sc_cleanup_sock_fprog(struct sock_fprog *prog) { + free(prog->filter); + prog->filter = NULL; +} + +static void sc_must_read_filter_from_file(FILE *file, uint32_t len_bytes, char *what, struct sock_fprog *prog) +{ + prog->len = len_bytes / sizeof(struct sock_filter); + prog->filter = (struct sock_filter *)malloc(len_bytes); + if (prog->filter == NULL) { + die("cannot allocate %u bytes of memory for %s seccomp filter ", len_bytes, what); + } + size_t num_read = fread(prog->filter, 1, len_bytes, file); + if (ferror(file)) { + die("cannot read %s filter", what); + } + if (num_read != len_bytes) { + die("short read for filter %s %zu != %i", what, num_read, len_bytes); + } +} + +static FILE* sc_must_read_header_from_file(const char *profile_path, struct sc_seccomp_file_header *hdr) +{ + FILE *file = fopen(profile_path, "rb"); + if (file == NULL) { + die("cannot open seccomp filter %s", profile_path); + } + size_t num_read = fread(hdr, 1, sizeof(struct sc_seccomp_file_header), file); + if (ferror(file) != 0) { + die("cannot read seccomp profile %s", profile_path); + } + if (num_read < sizeof(struct sc_seccomp_file_header)) { + die("short read on seccomp header: %zu", num_read); + } + if (hdr->header[0] != 'S' || hdr->header[1] != 'C') { + die("unexpected seccomp header: %x%x", hdr->header[0], hdr->header[1]); + } + if (hdr->version != 1) { + die("unexpected seccomp file version: %x", hdr->version); + } + if (hdr->len_allow_filter > MAX_BPF_SIZE) { + die("allow filter size too big %u", hdr->len_allow_filter); + } + if (hdr->len_deny_filter > MAX_BPF_SIZE) { + die("deny filter size too big %u", hdr->len_deny_filter); + } + struct stat buf; + if (fstat(fileno(file), &buf) != 0) { + die("cannot fstat the seccomp file"); + } + off_t expected_size = sizeof(struct sc_seccomp_file_header)+hdr->len_allow_filter+hdr->len_deny_filter; + if (buf.st_size != expected_size) { + die("unexpected filesize %ju != %ju", buf.st_size, expected_size); + } + + return file; +} + bool sc_apply_seccomp_profile_for_security_tag(const char *security_tag) { debug("loading bpf program for security tag %s", security_tag); char profile_path[PATH_MAX] = { 0 }; - sc_must_snprintf(profile_path, sizeof(profile_path), "%s/%s.bin", + struct sock_fprog SC_CLEANUP(sc_cleanup_sock_fprog) prog_allow = { 0 }; + struct sock_fprog SC_CLEANUP(sc_cleanup_sock_fprog) prog_deny = { 0 }; + sc_must_snprintf(profile_path, sizeof(profile_path), "%s/%s.bin2", filter_profile_dir, security_tag); // Wait some time for the security profile to show up. When @@ -152,24 +239,22 @@ bool sc_apply_seccomp_profile_for_security_tag(const char *security_tag) // set on the system. validate_bpfpath_is_safe(profile_path); - /* The extra space has dual purpose. First of all, it is required to detect - * feof() while still being able to correctly read MAX_BPF_SIZE bytes of - * seccomp profile. In addition, because we treat the profile as a - * quasi-string and use sc_streq(), to compare it. The extra space is used - * as a way to ensure the result is a terminated string (though in practice - * it can contain embedded NULs any earlier position). Note that - * sc_read_seccomp_filter knows about the extra space and ensures that the - * buffer is never empty. */ - char bpf[MAX_BPF_SIZE + 1] = { 0 }; - size_t num_read = sc_read_seccomp_filter(profile_path, bpf, sizeof bpf); - if (sc_streq(bpf, "@unrestricted\n")) { + // TODO: strange init to workaround bug in gcc on 14.04 that claims: + // "error: missing braces around initializer" + struct sc_seccomp_file_header hdr = {{0},0}; + FILE *file SC_CLEANUP(sc_cleanup_file) = sc_must_read_header_from_file(profile_path, &hdr); + if (hdr.unrestricted == 0x1) { return false; } - struct sock_fprog prog = { - .len = num_read / sizeof(struct sock_filter), - .filter = (struct sock_filter *)bpf, - }; - sc_apply_seccomp_filter(&prog); + + // populate allow + sc_must_read_filter_from_file(file, hdr.len_allow_filter, "allow", &prog_allow); + sc_must_read_filter_from_file(file, hdr.len_deny_filter, "deny", &prog_deny); + + // apply both filters + sc_apply_seccomp_filter(&prog_deny); + sc_apply_seccomp_filter(&prog_allow); + return true; } diff --git a/cmd/snap-confine/snap-confine.apparmor.in b/cmd/snap-confine/snap-confine.apparmor.in index 73d14c8c781b..99937b470600 100644 --- a/cmd/snap-confine/snap-confine.apparmor.in +++ b/cmd/snap-confine/snap-confine.apparmor.in @@ -149,7 +149,7 @@ # change_profile unsafe /** -> **, # reading seccomp filters - /{tmp/snap.rootfs_*/,}var/lib/snapd/seccomp/bpf/*.bin r, + /{tmp/snap.rootfs_*/,}var/lib/snapd/seccomp/bpf/*.bin2 r, # adding a missing bpf mount mount fstype=bpf options=(rw) bpf -> /sys/fs/bpf/, diff --git a/cmd/snap-seccomp/main.go b/cmd/snap-seccomp/main.go index 4ff3799b0a17..f3686ba2080d 100644 --- a/cmd/snap-seccomp/main.go +++ b/cmd/snap-seccomp/main.go @@ -210,7 +210,9 @@ import "C" import ( "bufio" "bytes" + "encoding/binary" "fmt" + "io" "io/ioutil" "os" "strconv" @@ -583,11 +585,12 @@ var ( errnoOnImplicitDenial int16 = C.EPERM ) -func parseLine(line string, secFilter *seccomp.ScmpFilter) error { +func parseLine(line string, secFilterAllow, secFilterDeny *seccomp.ScmpFilter) error { // ignore comments and empty lines if strings.HasPrefix(line, "#") || line == "" { return nil } + secFilter := secFilterAllow // regular line tokens := strings.Fields(line) @@ -604,6 +607,7 @@ func parseLine(line string, secFilter *seccomp.ScmpFilter) error { if strings.HasPrefix(syscallName, "~") { action = seccomp.ActErrno.SetReturnCode(errnoOnExplicitDenial) syscallName = syscallName[1:] + secFilter = secFilterDeny } secSyscall, err := seccomp.GetSyscallFromName(syscallName) @@ -691,8 +695,11 @@ func parseLine(line string, secFilter *seccomp.ScmpFilter) error { if err = secFilter.AddRuleConditionalExact(secSyscall, action, conds); err != nil { err = secFilter.AddRuleConditional(secSyscall, action, conds) } + if err != nil { + return fmt.Errorf("cannot add rule for line %q: %v", line, err) + } - return err + return nil } // used to mock in tests @@ -789,18 +796,116 @@ func complainAction() seccomp.ScmpAction { return seccomp.ActAllow } +func exportBPF(filter *seccomp.ScmpFilter) (io.ReadCloser, int64, error) { + filterFile, err := os.CreateTemp("", "filter-file") + if err != nil { + return nil, 0, err + } + if err := os.Remove(filterFile.Name()); err != nil { + return nil, 0, err + } + // XXX: would be nice if ExportBPF would support an io.Writer + // but it requires a os.File + if err := filter.ExportBPF(filterFile); err != nil { + return nil, 0, err + } + if _, err := filterFile.Seek(0, 0); err != nil { + return nil, 0, err + } + stat, err := filterFile.Stat() + if err != nil { + return nil, 0, err + } + + return filterFile, stat.Size(), nil +} + +// keep in sync with seccomp-support.c +type scSeccompFileHeader struct { + header [2]byte + version byte + // flags + unrestricted byte + // unused + padding [4]byte + // location of allow/deny, all offsets/len in bytes + lenAllowFilter uint32 + lenDenyFilter uint32 + // reserved for future use + reserved2 [112]byte +} + +func writeUnrestrictedFilter(out string) error { + hdr := scSeccompFileHeader{ + header: [2]byte{'S', 'C'}, + version: 0x1, + // tell snap-confine + unrestricted: 0x1, + } + fout, err := osutil.NewAtomicFile(out, 0644, 0, osutil.NoChown, osutil.NoChown) + if err != nil { + return err + } + defer fout.Cancel() + + if err := binary.Write(fout, arch.Endian(), hdr); err != nil { + return err + } + return fout.Commit() +} + +func writeSeccompFilter(outFile string, filterAllow, filterDeny *seccomp.ScmpFilter) error { + allowBpf, allowSize, err := exportBPF(filterAllow) + if err != nil { + return err + } + defer allowBpf.Close() + + denyBpf, denySize, err := exportBPF(filterDeny) + if err != nil { + return err + } + defer denyBpf.Close() + + fout, err := osutil.NewAtomicFile(outFile, 0644, 0, osutil.NoChown, osutil.NoChown) + if err != nil { + return err + } + defer fout.Cancel() + + hdr := scSeccompFileHeader{ + header: [2]byte{'S', 'C'}, + version: 0x1, + } + hdr.lenAllowFilter = uint32(allowSize) + hdr.lenDenyFilter = uint32(denySize) + // silly go does not give an easy way to use native endian so just + // write big endian and use ntohl() in the C code + if err := binary.Write(fout, arch.Endian(), hdr); err != nil { + return err + } + if _, err := io.Copy(fout, allowBpf); err != nil { + return err + } + if _, err := io.Copy(fout, denyBpf); err != nil { + return err + } + + return fout.Commit() +} + func compile(content []byte, out string) error { var err error - var secFilter *seccomp.ScmpFilter + var secFilterAllow, secFilterDeny *seccomp.ScmpFilter unrestricted, complain := preprocess(content) switch { case unrestricted: - return osutil.AtomicWrite(out, bytes.NewBufferString("@unrestricted\n"), 0644, 0) + return writeUnrestrictedFilter(out) case complain: var complainAct seccomp.ScmpAction = complainAction() - secFilter, err = seccomp.NewFilter(complainAct) + secFilterAllow, err = seccomp.NewFilter(complainAct) if err != nil { if complainAct != seccomp.ActAllow { // ActLog is only supported in newer versions @@ -808,9 +913,16 @@ func compile(content []byte, out string) error { // libseccomp-golang. Attempt to fall back to // ActAllow before erroring out. complainAct = seccomp.ActAllow - secFilter, err = seccomp.NewFilter(complainAct) + secFilterAllow, err = seccomp.NewFilter(complainAct) } } + if err != nil { + return fmt.Errorf("cannot create allow seccomp filter: %s", err) + } + secFilterDeny, err = seccomp.NewFilter(complainAct) + if err != nil { + return fmt.Errorf("cannot create deny seccomp filter: %s", err) + } // Set unrestricted to 'true' to fallback to the pre-ActLog // behavior of simply setting the allow filter without adding @@ -819,19 +931,26 @@ func compile(content []byte, out string) error { unrestricted = true } default: - secFilter, err = seccomp.NewFilter(seccomp.ActErrno.SetReturnCode(errnoOnImplicitDenial)) + secFilterAllow, err = seccomp.NewFilter(seccomp.ActErrno.SetReturnCode(errnoOnImplicitDenial)) + if err != nil { + return fmt.Errorf("cannot create seccomp filter: %s", err) + } + secFilterDeny, err = seccomp.NewFilter(seccomp.ActAllow) + if err != nil { + return fmt.Errorf("cannot create seccomp filter: %s", err) + } } - if err != nil { - return fmt.Errorf("cannot create seccomp filter: %s", err) + if err := addSecondaryArches(secFilterAllow); err != nil { + return err } - if err := addSecondaryArches(secFilter); err != nil { + if err := addSecondaryArches(secFilterDeny); err != nil { return err } if !unrestricted { scanner := bufio.NewScanner(bytes.NewBuffer(content)) for scanner.Scan() { - if err := parseLine(scanner.Text(), secFilter); err != nil { + if err := parseLine(scanner.Text(), secFilterAllow, secFilterDeny); err != nil { return fmt.Errorf("cannot parse line: %s", err) } } @@ -841,21 +960,14 @@ func compile(content []byte, out string) error { } if osutil.GetenvBool("SNAP_SECCOMP_DEBUG") { - secFilter.ExportPFC(os.Stdout) + secFilterAllow.ExportPFC(os.Stdout) + secFilterDeny.ExportPFC(os.Stdout) } - // write atomically - fout, err := osutil.NewAtomicFile(out, 0644, 0, osutil.NoChown, osutil.NoChown) - if err != nil { - return err - } - // Cancel once Committed is a NOP - defer fout.Cancel() - - if err := secFilter.ExportBPF(fout.File); err != nil { + if err := writeSeccompFilter(out, secFilterAllow, secFilterDeny); err != nil { return err } - return fout.Commit() + return nil } // caches for uid and gid lookups diff --git a/cmd/snap-seccomp/main_test.go b/cmd/snap-seccomp/main_test.go index 38db6fa16e84..16f1b8f50fd5 100644 --- a/cmd/snap-seccomp/main_test.go +++ b/cmd/snap-seccomp/main_test.go @@ -37,7 +37,6 @@ import ( main "github.com/snapcore/snapd/cmd/snap-seccomp" "github.com/snapcore/snapd/osutil" "github.com/snapcore/snapd/release" - "github.com/snapcore/snapd/testutil" ) // Hook up check.v1 into the "go test" runner @@ -53,6 +52,7 @@ var _ = Suite(&snapSeccompSuite{}) const ( Deny = iota + DenyExplicit Allow ) @@ -71,31 +71,57 @@ var seccompBpfLoaderContent = []byte(` #define MAX_BPF_SIZE 32 * 1024 +// keep in sync with: +// cmd/snap-confine/seccomp-support.c +// main.go:scSeccompFileHeader +struct sc_seccomp_file_header { + char header[2]; + char version; + char unrestricted; + char padding[4]; + uint32_t len_allow_filter; + uint32_t len_deny_filter; + char reserved2[112]; +}; + int sc_apply_seccomp_bpf(const char* profile_path) { - unsigned char bpf[MAX_BPF_SIZE + 1]; // account for EOF + struct sc_seccomp_file_header hdr = {{0}, 0}; + unsigned char bpf_allow[MAX_BPF_SIZE + 1]; // account for EOF + unsigned char bpf_deny[MAX_BPF_SIZE + 1]; // account for EOF FILE* fp; + fp = fopen(profile_path, "rb"); if (fp == NULL) { fprintf(stderr, "cannot read %s\n", profile_path); return -1; } - - // set 'size' to 1; to get bytes transferred - size_t num_read = fread(bpf, 1, sizeof(bpf), fp); - + fread(&hdr, 1, sizeof(struct sc_seccomp_file_header), fp); + if (ferror(fp) != 0) { + perror("fread() header"); + return -1; + } + fread(bpf_allow, 1, hdr.len_allow_filter, fp); if (ferror(fp) != 0) { perror("fread()"); return -1; - } else if (feof(fp) == 0) { - fprintf(stderr, "file too big\n"); + } + fread(bpf_deny, 1, hdr.len_deny_filter, fp); + if (ferror(fp) != 0) { + perror("fread()"); return -1; } + fclose(fp); - struct sock_fprog prog = { - .len = num_read / sizeof(struct sock_filter), - .filter = (struct sock_filter*)bpf, + struct sock_fprog prog_allow = { + .len = hdr.len_allow_filter / sizeof(struct sock_filter), + .filter = (struct sock_filter*)bpf_allow, + }; + + struct sock_fprog prog_deny = { + .len = hdr.len_deny_filter / sizeof(struct sock_filter), + .filter = (struct sock_filter*)bpf_deny, }; // Set NNP to allow loading seccomp policy into the kernel without @@ -105,10 +131,15 @@ int sc_apply_seccomp_bpf(const char* profile_path) return -1; } - if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) { - perror("prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, ...) failed"); + if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_deny)) { + perror("prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, ...) deny failed"); + return -1; + } + if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog_allow)) { + perror("prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, ...) allow failed"); return -1; } + return 0; } @@ -153,9 +184,12 @@ int main(int argc, char** argv) syscall_ret = syscall(l[0], l[1], l[2], l[3], l[4], l[5], l[6]); // 911 is our mocked errno for implicit denials via unlisted syscalls and // 999 is explicit denial - if (syscall_ret < 0 && (errno == 911 || errno == 999)) { + if (syscall_ret < 0 && errno == 911) { ret = 10; } + if (syscall_ret < 0 && errno == 999) { + ret = 20; + } syscall(SYS_exit, ret, 0, 0, 0, 0, 0); return 0; } @@ -342,13 +376,23 @@ mprotect // else is unexpected (segv, strtoll failure, ...) exitCode, e := osutil.ExitCode(err) c.Assert(e, IsNil) - c.Assert(exitCode == 0 || exitCode == 10, Equals, true, Commentf("unexpected exit code: %v for %v - test setup broken", exitCode, seccompWhitelist)) + c.Assert(exitCode == 0 || exitCode == 10 || exitCode == 20, Equals, true, Commentf("unexpected exit code: %v for %v - test setup broken", exitCode, seccompWhitelist)) switch expected { case Allow: if err != nil { c.Fatalf("unexpected error for %q (failed to run %q)", seccompWhitelist, err) } case Deny: + if exitCode != 10 { + c.Fatalf("unexpected exit code for %q %q (%v != %v)", seccompWhitelist, bpfInput, exitCode, 10) + } + if err == nil { + c.Fatalf("unexpected success for %q %q (ran but should have failed)", seccompWhitelist, bpfInput) + } + case DenyExplicit: + if exitCode != 20 { + c.Fatalf("unexpected exit code for %q %q (%v != %v)", seccompWhitelist, bpfInput, exitCode, 20) + } if err == nil { c.Fatalf("unexpected success for %q %q (ran but should have failed)", seccompWhitelist, bpfInput) } @@ -363,7 +407,10 @@ func (s *snapSeccompSuite) TestUnrestricted(c *C) { err := main.Compile([]byte(inp), outPath) c.Assert(err, IsNil) - c.Check(outPath, testutil.FileEquals, inp) + expected := [128]byte{'S', 'C', 0x1, 0x1} + fileContent, err := ioutil.ReadFile(outPath) + c.Assert(err, IsNil) + c.Check(fileContent, DeepEquals, expected[:]) } // TestCompile iterates over a range of textual seccomp whitelist rules and @@ -395,8 +442,9 @@ func (s *snapSeccompSuite) TestCompile(c *C) { {"read", "read", Allow}, {"read\nwrite\nexecve\n", "write", Allow}, - // trivial denial - {"read", "ioctl", Deny}, + // trivial denial (uses write in allow-list to ensure any + // errors printing is visible) + {"write", "ioctl", Deny}, // test argument filtering syntax, we currently support: // >=, <=, !, <, >, | @@ -451,12 +499,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}, + {"ioctl\n~ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", DenyExplicit}, // 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}, + {"ioctl\n~ioctl - TIOCSTI\n~ioctl - TIOCLINUX\nioctl - !TIOCSTI", "ioctl;native;-,TIOCSTI", DenyExplicit}, + {"ioctl\n~ioctl - TIOCSTI\n~ioctl - TIOCLINUX\nioctl - !TIOCSTI", "ioctl;native;-,TIOCLINUX", DenyExplicit}, + {"ioctl\n~ioctl - TIOCSTI\n~ioctl - TIOCLINUX\nioctl - !TIOCSTI", "ioctl;native;-,TIOCGWINSZ", Allow}, // test_bad_seccomp_filter_args_clone {"setns - CLONE_NEWNET", "setns;native;-,99", Deny}, @@ -473,6 +521,13 @@ func (s *snapSeccompSuite) TestCompile(c *C) { // test_bad_seccomp_filter_args_prio {"setpriority PRIO_PROCESS 0 >=0", "setpriority;native;PRIO_PROCESS,0,19", Allow}, {"setpriority PRIO_PROCESS 0 >=0", "setpriority;native;99", Deny}, + // negative filtering + {"setpriority\n~setpriority PRIO_PROCESS 0 >=0", "setpriority;native;PRIO_PROCESS,0,10", DenyExplicit}, + // mix negative/positiv filtering + // allow setprioty >= 5 but explicitly deny >=10 + {"setpriority PRIO_PROCESS 0 >=5\n~setpriority PRIO_PROCESS 0 >=10", "setpriority;native;PRIO_PROCESS,0,2", Deny}, + {"setpriority PRIO_PROCESS 0 >=5\n~setpriority PRIO_PROCESS 0 >=10", "setpriority;native;PRIO_PROCESS,0,5", Allow}, + {"setpriority PRIO_PROCESS 0 >=5\n~setpriority PRIO_PROCESS 0 >=10", "setpriority;native;PRIO_PROCESS,0,10", DenyExplicit}, // test_bad_seccomp_filter_args_quotactl {"quotactl Q_GETQUOTA", "quotactl;native;Q_GETQUOTA", Allow}, diff --git a/interfaces/seccomp/backend.go b/interfaces/seccomp/backend.go index e44e07d50a77..9deae3b6e95c 100644 --- a/interfaces/seccomp/backend.go +++ b/interfaces/seccomp/backend.go @@ -80,6 +80,16 @@ type Backend struct { versionInfo seccomp.VersionInfo } +// TODO: now that snap-seccomp has full support for deny-listing this +// should be replaced with something like: +// +// ~ioctl - 4294967295|TIOCSTI +// ~ioctl - 4294967295|TIOCLINUX +// +// in the default template. This requires that MaskedEq learns +// to deal with two arguments (see also https://github.com/snapcore/snapd/compare/master...mvo5:rework-seccomp-denylist-incoperate-global.bin?expand=1) +// +// globalProfileLE is generated via cmd/snap-seccomp-blacklist var globalProfileLE = []byte{ 0x20, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x15, 0x00, 0x00, 0x04, 0x3e, 0x00, 0x00, 0xc0, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x35, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x40, @@ -93,6 +103,7 @@ var globalProfileLE = []byte{ 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0x7f, } +// globalProfileBE is generated via cmd/snap-seccomp-blacklist var globalProfileBE = []byte{ 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x15, 0x00, 0x08, 0x80, 0x00, 0x00, 0x16, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x15, 0x00, 0x06, 0x00, 0x00, 0x00, 0x36, @@ -149,7 +160,7 @@ func bpfSrcPath(srcName string) string { } func bpfBinPath(srcName string) string { - return filepath.Join(dirs.SnapSeccompDir, strings.TrimSuffix(srcName, ".src")+".bin") + return filepath.Join(dirs.SnapSeccompDir, strings.TrimSuffix(srcName, ".src")+".bin2") } func parallelCompile(compiler Compiler, profiles []string) error { diff --git a/interfaces/seccomp/backend_test.go b/interfaces/seccomp/backend_test.go index 72bdc039bc7e..9baa20c59a10 100644 --- a/interfaces/seccomp/backend_test.go +++ b/interfaces/seccomp/backend_test.go @@ -133,7 +133,7 @@ func (s *backendSuite) TestInstallingSnapWritesProfiles(c *C) { c.Check(err, IsNil) // and got compiled c.Check(s.snapSeccomp.Calls(), DeepEquals, [][]string{ - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, }) } @@ -146,7 +146,7 @@ func (s *backendSuite) TestInstallingSnapWritesHookProfiles(c *C) { c.Check(err, IsNil) // and got compiled c.Check(s.snapSeccomp.Calls(), DeepEquals, [][]string{ - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, }) } @@ -178,7 +178,7 @@ fi`) // ensure the snap-seccomp from the core snap was used instead c.Check(snapSeccompOnCore.Calls(), DeepEquals, [][]string{ {"snap-seccomp", "version-info"}, // from Initialize() - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, }) raw, err := ioutil.ReadFile(profile + ".src") c.Assert(err, IsNil) @@ -219,7 +219,7 @@ func (s *backendSuite) TestUpdatingSnapToOneWithMoreApps(c *C) { // file called "snap.sambda.nmbd" was created c.Check(err, IsNil) // and got compiled - c.Check(s.snapSeccomp.Calls(), testutil.DeepContains, []string{"snap-seccomp", "compile", profile + ".src", profile + ".bin"}) + c.Check(s.snapSeccomp.Calls(), testutil.DeepContains, []string{"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}) s.snapSeccomp.ForgetCalls() s.RemoveSnap(c, snapInfo) @@ -236,7 +236,7 @@ func (s *backendSuite) TestUpdatingSnapToOneWithHooks(c *C) { // Verify that profile "snap.samba.hook.configure" was created. c.Check(err, IsNil) // and got compiled - c.Check(s.snapSeccomp.Calls(), testutil.DeepContains, []string{"snap-seccomp", "compile", profile + ".src", profile + ".bin"}) + c.Check(s.snapSeccomp.Calls(), testutil.DeepContains, []string{"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}) s.snapSeccomp.ForgetCalls() s.RemoveSnap(c, snapInfo) @@ -647,7 +647,7 @@ func (s *backendSuite) TestRebuildsWithVersionInfoWhenNeeded(c *C) { c.Check(profile+".src", testutil.FileEquals, s.profileHeader+"\ndefault\n") c.Check(s.snapSeccomp.Calls(), DeepEquals, [][]string{ - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, }) // unchanged snap-seccomp version will not trigger a rebuild @@ -673,7 +673,7 @@ fi`) c.Check(s.snapSeccomp.Calls(), HasLen, 2) c.Check(s.snapSeccomp.Calls(), DeepEquals, [][]string{ // compilation from first Setup() - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, // initialization with new version {"snap-seccomp", "version-info"}, }) @@ -686,11 +686,11 @@ fi`) c.Check(s.snapSeccomp.Calls(), HasLen, 3) c.Check(s.snapSeccomp.Calls(), DeepEquals, [][]string{ // compilation from first Setup() - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, // initialization with new version {"snap-seccomp", "version-info"}, // compilation of profiles with new compiler version - {"snap-seccomp", "compile", profile + ".src", profile + ".bin"}, + {"snap-seccomp", "compile", profile + ".src", profile + ".bin2"}, }) } @@ -914,7 +914,7 @@ func (s *backendSuite) TestParallelCompileHappy(c *C) { c.Assert(m.profiles, DeepEquals, profiles) for _, p := range profiles { - c.Check(filepath.Join(dirs.SnapSeccompDir, p+".bin"), testutil.FileEquals, "done "+p+".bin") + c.Check(filepath.Join(dirs.SnapSeccompDir, p+".bin2"), testutil.FileEquals, "done "+p+".bin2") } } @@ -940,10 +940,10 @@ func (s *backendSuite) TestParallelCompileError(c *C) { } m := mockedSyncedFailingCompiler{ // pretend compilation of those 2 fails - whichFail: []string{"profile-005.bin", "profile-009.bin"}, + whichFail: []string{"profile-005.bin2", "profile-009.bin2"}, } err = seccomp.ParallelCompile(&m, profiles) - c.Assert(err, ErrorMatches, "cannot compile .*/bpf/profile-00[59]: failed profile-00[59].bin") + c.Assert(err, ErrorMatches, "cannot compile .*/bpf/profile-00[59]: failed profile-00[59].bin2") // make sure all compiled profiles were removed d, err := os.Open(dirs.SnapSeccompDir) @@ -957,16 +957,17 @@ func (s *backendSuite) TestParallelCompileError(c *C) { func (s *backendSuite) TestParallelCompileRemovesFirst(c *C) { err := os.MkdirAll(dirs.SnapSeccompDir, 0755) c.Assert(err, IsNil) - err = ioutil.WriteFile(filepath.Join(dirs.SnapSeccompDir, "profile-001.bin"), nil, 0755) + err = ioutil.WriteFile(filepath.Join(dirs.SnapSeccompDir, "profile-001.bin2"), nil, 0755) c.Assert(err, IsNil) - // make profiles directory non-accessible err = os.Chmod(dirs.SnapSeccompDir, 0000) c.Assert(err, IsNil) + err = os.Chmod(dirs.SnapSeccompDir, 0500) + c.Assert(err, IsNil) defer os.Chmod(dirs.SnapSeccompDir, 0755) m := mockedSyncedCompiler{} err = seccomp.ParallelCompile(&m, []string{"profile-001"}) - c.Assert(err, ErrorMatches, "remove .*/profile-001.bin: permission denied") + c.Assert(err, ErrorMatches, "remove .*/profile-001.bin2: permission denied") } diff --git a/interfaces/seccomp/template.go b/interfaces/seccomp/template.go index e92bafc3bdef..3006e791bd44 100644 --- a/interfaces/seccomp/template.go +++ b/interfaces/seccomp/template.go @@ -205,14 +205,7 @@ inotify_rm_watch # TODO: this should be scaled back even more ~ioctl - TIOCSTI ~ioctl - TIOCLINUX -# restrict argument otherwise will match all uses of ioctl() and allow the rules -# that were disallowed above -# TODO: Fix the need to keep TIOCLINUX here - the issue is a unrestricted -# allow for "ioctl" here makes libseccomp "optimize" the deny rules -# above away and the generated bpf becomes just "allow ioctl". -# We should fix this by creating a way to make "AND" rules, so -# this becomes "ioctl - !TIOCSTI&&!TIOCLINUX" and remove the "~" again. -ioctl - !TIOCSTI +ioctl io_cancel io_destroy diff --git a/tests/main/install-store-laaaarge/task.yaml b/tests/main/install-store-laaaarge/task.yaml index 273987c875bc..9fb9a0d28886 100644 --- a/tests/main/install-store-laaaarge/task.yaml +++ b/tests/main/install-store-laaaarge/task.yaml @@ -2,7 +2,8 @@ summary: snap install a large snap from the store (bigger than tmpfs) prepare: | tests.systemd stop-unit snapd.service snapd.socket - mount -t tmpfs -o rw,nosuid,nodev,size=4 none /tmp + # tempfs needs to be big enough to hold the seccomp tmp file + mount -t tmpfs -o rw,nosuid,nodev,size=256k none /tmp systemctl start snapd.{socket,service} restore: | @@ -12,6 +13,6 @@ restore: | systemctl start snapd.{socket,service} execute: | - # test-snapd-sh is about 8k, tmpfs is 4k :-) - snap install test-snapd-sh - snap remove test-snapd-sh + # test-snapd-go-webserver is about 1MB, tmpfs is 256k :-) + snap install test-snapd-go-webserver + snap remove test-snapd-go-webserver diff --git a/tests/main/preseed/task.yaml b/tests/main/preseed/task.yaml index 6f8bc2d7dd08..a14d387074ae 100644 --- a/tests/main/preseed/task.yaml +++ b/tests/main/preseed/task.yaml @@ -120,7 +120,7 @@ execute: | # the list of expected profiles isn't exhaustive, we're just checking some critical ones for prof in snap.lxd.lxd snap.lxd.hook.install snap.lxd.hook.configure snap.lxd.daemon; do test -f "$AA_PROFILES/$prof" - test -f "$SECCOMP_PROFILES/$prof.bin" + test -f "$SECCOMP_PROFILES/$prof.bin2" done echo "Checking that mount units have been created and enabled on the target image" diff --git a/tests/main/security-profiles/task.yaml b/tests/main/security-profiles/task.yaml index e2eb49936ffb..164c22750bef 100644 --- a/tests/main/security-profiles/task.yaml +++ b/tests/main/security-profiles/task.yaml @@ -17,7 +17,7 @@ execute: | for profile in snap.test-snapd-tools.block snap.test-snapd-tools.cat snap.test-snapd-tools.echo snap.test-snapd-tools.fail snap.test-snapd-tools.success do MATCH "^${profile} \\(enforce\\)$" <<<"$loaded_profiles" - [ -f "$seccomp_profile_directory/${profile}.bin" ] + [ -f "$seccomp_profile_directory/${profile}.bin2" ] done echo "Security profiles are generated and loaded for hooks" @@ -25,4 +25,4 @@ execute: | loaded_profiles=$(cat /sys/kernel/security/apparmor/profiles) echo "$loaded_profiles" | MATCH '^snap.basic-hooks.hook.configure \(enforce\)$' - [ -f "$seccomp_profile_directory/snap.basic-hooks.hook.configure.bin" ] + [ -f "$seccomp_profile_directory/snap.basic-hooks.hook.configure.bin2" ] diff --git a/tests/main/security-seccomp/task.yaml b/tests/main/security-seccomp/task.yaml index 85df4699005d..afb28e59dd3a 100644 --- a/tests/main/security-seccomp/task.yaml +++ b/tests/main/security-seccomp/task.yaml @@ -25,7 +25,7 @@ details: | environment: SRC: /var/lib/snapd/seccomp/bpf/snap.test-snapd-setpriority.test-snapd-setpriority.src - BIN: /var/lib/snapd/seccomp/bpf/snap.test-snapd-setpriority.test-snapd-setpriority.bin + BIN: /var/lib/snapd/seccomp/bpf/snap.test-snapd-setpriority.test-snapd-setpriority.bin2 AAP: /var/lib/snapd/apparmor/profiles/snap.test-snapd-setpriority.test-snapd-setpriority prepare: | @@ -93,10 +93,14 @@ execute: | echo "and check that negative nice fails" test-snapd-setpriority -10 | MATCH 'Operation not permitted \(EPERM\)' + # TODO: filtering on setpriority is a bit confusing as it is not part + # of the "negative args" filter added in ec7c9f27c97 so the fact that + # negative args are denied is a bit magic echo "Explicitly deny arg filtered setpriority rule" - sed 's/^\(setpriority.*\)/~\1/g' "$SRC".orig > "$SRC" + cp -a "$SRC".orig "$SRC" + echo '~setpriority PRIO_PROCESS 0 >10' >> "$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 explicit requested fails with explicit denial" + test-snapd-setpriority 11 | MATCH 'Insufficient privileges \(EACCES\)' echo "and check that negative nice still fails with implicit denial" test-snapd-setpriority -10 | MATCH 'Operation not permitted \(EPERM\)' diff --git a/tests/main/snap-seccomp/task.yaml b/tests/main/snap-seccomp/task.yaml index b81a663bd912..dd6c475545c9 100644 --- a/tests/main/snap-seccomp/task.yaml +++ b/tests/main/snap-seccomp/task.yaml @@ -1,8 +1,6 @@ summary: Ensure that the snap-seccomp bpf handling works -# FIXME: once $(snap debug confinment) can be used (in 2.27+) remove -# the systems line -systems: [ubuntu-16*, ubuntu-18*] +systems: [ubuntu-*] # Start early as it takes a long time. priority: 100 @@ -16,26 +14,30 @@ execute: | snap install test-snapd-sh test-snapd-sh.sh -c 'echo hello' | MATCH hello - # FIXME: use dirs.sh in 2.27+ - echo "Ensure snap-seccomp is statically linked" - if ldd /usr/lib/snapd/snap-seccomp | MATCH libseccomp ; then - echo "found dynamically linked libseccomp, we need a staticly linked one" - exit 1 + # The 16.04 build is static as it goes into the snapd snap but 14.04 + # is dynamically linked. + if ! os.query is-trusty; then + echo "Ensure snap-seccomp is statically linked" + # FIXME: use dirs.sh in 2.27+ + if ldd /usr/lib/snapd/snap-seccomp | MATCH libseccomp ; then + echo "found dynamically linked libseccomp, we need a staticly linked one" + exit 1 + fi fi # from the old test_complain echo "Test that the @complain keyword works" - rm -f "${PROFILE}.bin" + rm -f "${PROFILE}.bin2" cat >"${PROFILE}.src" <"${PROFILE}.src" <"${PROFILE}.src" <"${PROFILE}.src" <&1 ); then echo "test-snapd-sh.sh should fail with invalid seccomp profile" exit 1 fi - echo "$output" | MATCH "cannot apply seccomp profile: Invalid argument" + echo "$output" | MATCH "unexpected seccomp header: .*" - echo "Add huge snapd.test-snapd-sh.bin to ensure size limit works" - dd if=/dev/zero of="${PROFILE}.bin" count=50 bs=1M + echo "Add huge snapd.test-snapd-sh filters to ensure size limit works" + dd if=/dev/zero of="${PROFILE}.bin2" count=50 bs=1M if output=$(test-snapd-sh.sh -c 'echo hello' 2>&1 ); then echo "test-snapd-sh.sh should fail with big seccomp profile" exit 1 fi - echo "$output" | MATCH "cannot fit .* to memory buffer" + # TODO: adjust the test so that the header is valid and the profile big + #echo "$output" | MATCH "cannot fit .* to memory buffer" - echo "Ensure the code cannot not run with a missing .bin profile" - rm -f "${PROFILE}.bin" + echo "Ensure the code cannot not run with a missing filter profile" + rm -f "${PROFILE}.bin2" if test-snapd-sh.sh -c 'echo hello'; then echo "filtering broken: program should have failed to run" exit 1 fi echo "Ensure the code cannot not run with an empty seccomp profile" - rm -f "${PROFILE}.bin" + rm -f "${PROFILE}.bin2" echo "" > "${PROFILE}.src" - $SNAP_SECCOMP compile "${PROFILE}.src" "${PROFILE}.bin" + $SNAP_SECCOMP compile "${PROFILE}.src" "${PROFILE}.bin2" if test-snapd-sh.sh -c 'echo hello'; then echo "filtering broken: program should have failed to run" exit 1 fi echo "Ensure snap-confine waits for security profiles to appear" - rm -f "${PROFILE}.bin" + rm -f "${PROFILE}.bin2" cat >"${PROFILE}.src" <