Skip to content

Commit

Permalink
allow 'system-usernames' with libseccomp > 2.4 and golang-seccomp > 0…
Browse files Browse the repository at this point in the history
….9.0

libseccomp < 2.4 generates unpredicatable BPFs where at runtime the
policy enforcement for argument filtered rules is sometimes wrong. The
BPF generation is known to be sensitive to rule order and while the PFC
is correct, a disassembly of the BPF indicates problematic logic that is
also sensitive to the runtime environment. Incorrect enforcement was
observed with secondary architectures and differing kernel versions
across different distros.

libseccomp 2.4 includes a rewrite of the BPF to, in part, better handle
argument filtering and it now generates predictable BPFs (as seen from
the PFCs and BPF disassemblies) without the problematic logic that is
sensitive to runtime environments. (As an aside, it also orders the
syscalls based on priorities, which should yield performance gains for
snaps with heavy syscall use). As such, require that the
'system-usernames' features be dependent on snap-seccomp being compiled
against libseccomp >= 2.4.

Likewise, due to seccomp/libseccomp-golang#22,
golang-seccomp <= 0.9.0 cannot create correct BPFs for this feature.
The package does not contain any version information, but we know that
ActLog was implemented in the library after this issue was fixed, so
base the decision on that. ActLog is first available in 0.9.1.

References:
canonical#6681
  • Loading branch information
Jamie Strandboge committed Jul 15, 2019
1 parent a6914ef commit 5ce5ff1
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 7 deletions.
10 changes: 5 additions & 5 deletions interfaces/system_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ var (
isHomeUsingNFS = osutil.IsHomeUsingNFS
mockedSystemKey *systemKey

seccompCompilerVersionInfo = seccompCompilerVersionInfoImpl
SeccompCompilerVersionInfo = seccompCompilerVersionInfoImpl

readBuildID = osutil.ReadBuildID
)
Expand Down Expand Up @@ -139,7 +139,7 @@ func generateSystemKey() (*systemKey, error) {
// Add seccomp-features
sk.SecCompActions = release.SecCompActions()

versionInfo, err := seccompCompilerVersionInfo(filepath.Join(filepath.Dir(snapdPath), "snap-seccomp"))
versionInfo, err := SeccompCompilerVersionInfo(filepath.Join(filepath.Dir(snapdPath), "snap-seccomp"))
if err != nil {
logger.Noticef("cannot determine seccomp compiler version in generateSystemKey: %v", err)
return nil, err
Expand Down Expand Up @@ -258,9 +258,9 @@ func MockSystemKey(s string) func() {
}

func MockSeccompCompilerVersionInfo(s func(p string) (string, error)) (restore func()) {
old := seccompCompilerVersionInfo
seccompCompilerVersionInfo = s
old := SeccompCompilerVersionInfo
SeccompCompilerVersionInfo = s
return func() {
seccompCompilerVersionInfo = old
SeccompCompilerVersionInfo = old
}
}
48 changes: 48 additions & 0 deletions overlord/snapstate/check_snap.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,20 @@ package snapstate

import (
"fmt"
"path/filepath"
"regexp"
"strconv"
"strings"

"github.com/snapcore/snapd/arch"
"github.com/snapcore/snapd/cmd"
"github.com/snapcore/snapd/interfaces"
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/osutil"
"github.com/snapcore/snapd/overlord/snapstate/backend"
"github.com/snapcore/snapd/overlord/state"
"github.com/snapcore/snapd/release"
seccomp_compiler "github.com/snapcore/snapd/sandbox/seccomp"
"github.com/snapcore/snapd/snap"
)

Expand Down Expand Up @@ -432,6 +435,51 @@ func checkSystemUsernames(si *snap.Info) error {
if len(si.SystemUsernames) == 0 {
return nil
}

// Run /.../snap-seccomp version-info
path, err := cmd.InternalToolPath("snapd")
if err != nil {
return err
}
vi, err := interfaces.SeccompCompilerVersionInfo(filepath.Join(filepath.Dir(path), "snap-seccomp"))
if err != nil {
return fmt.Errorf("Could not obtain seccomp compiler information: %v", err)
}
libseccompVersion, err := seccomp_compiler.VersionInfo(vi).LibseccompVersion()
if err != nil {
return err
}

// Parse <libseccomp version>
tmp := strings.Split(libseccompVersion, ".")
maj, err := strconv.Atoi(tmp[0])
if err != nil {
return fmt.Errorf("Could not obtain seccomp compiler information: %v", err)
}
min, err := strconv.Atoi(tmp[1])
if err != nil {
return fmt.Errorf("Could not obtain seccomp compiler information: %v", err)
}
// libseccomp < 2.4 has significant argument filtering bugs that we
// cannot reliably work around with this feature.
if maj < 2 || (maj == 2 && min < 4) {
return fmt.Errorf(`This snap requires that snapd be compiled against libseccomp >= 2.4.`)
}

// Due to https://github.com/seccomp/libseccomp-golang/issues/22,
// golang-seccomp <= 0.9.0 cannot create correct BPFs for this feature.
// The package does not contain any version information, but we know
// that ActLog was implemented in the library after this issue was
// fixed, so base the decision on that. ActLog is first available in
// 0.9.1.
res, err := seccomp_compiler.VersionInfo(vi).HasFeature("bpf-actlog")
if err != nil {
return err
}
if !res {
return fmt.Errorf(`This snap requires that snapd be compiled against golang-seccomp >= 0.9.1`)
}

for _, user := range si.SystemUsernames {
if !osutil.IsValidUsername(user) {
return fmt.Errorf(`Invalid system username "%s"`, user)
Expand Down
45 changes: 45 additions & 0 deletions overlord/snapstate/check_snap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/snapcore/snapd/arch"
"github.com/snapcore/snapd/cmd"
"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/interfaces"
"github.com/snapcore/snapd/overlord/state"
"github.com/snapcore/snapd/release"
"github.com/snapcore/snapd/snap"
Expand Down Expand Up @@ -889,62 +890,106 @@ var systemUsernamesTests = []struct {
classic bool
noGroup bool
noUser bool
scVer string
error string
}{{
sysIDs: "[snap_daemon]",
scVer: "dead 2.4.1 deadbeef bpf-actlog",
}, {
sysIDs: "[snap_daemon]",
classic: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
}, {
sysIDs: "[snap_daemon, allowed-not]",
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `Unsupported system username "allowed-not"`,
}, {
sysIDs: "[allowed-not, snap_daemon]",
classic: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `Unsupported system username "allowed-not"`,
}, {
sysIDs: "[inv@lid]",
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `Invalid system username "inv@lid"`,
}, {
sysIDs: "['inv@lid']",
classic: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `Invalid system username "inv@lid"`,
}, {
sysIDs: "[snap_daemon]",
noGroup: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `This snap requires that the \"snap_daemon\" system user and group are present on the system.`,
}, {
sysIDs: "[snap_daemon]",
classic: true,
noGroup: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `This snap requires that the \"snap_daemon\" system user and group are present on the system.`,
}, {
sysIDs: "[snap_daemon]",
noUser: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `This snap requires that the \"snap_daemon\" system user and group are present on the system.`,
}, {
sysIDs: "[snap_daemon]",
classic: true,
noUser: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `This snap requires that the \"snap_daemon\" system user and group are present on the system.`,
}, {
sysIDs: "[snap_daemon]",
noUser: true,
noGroup: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `This snap requires that the \"snap_daemon\" system user and group are present on the system.`,
}, {
sysIDs: "[snap_daemon]",
classic: true,
noUser: true,
noGroup: true,
scVer: "dead 2.4.1 deadbeef bpf-actlog",
error: `This snap requires that the \"snap_daemon\" system user and group are present on the system.`,
}, {
sysIDs: "[snap_daemon]",
scVer: "dead 2.3.3 deadbeef bpf-actlog",
error: `This snap requires that snapd be compiled against libseccomp >= 2.4.`,
}, {
sysIDs: "[snap_daemon]",
classic: true,
scVer: "dead 2.3.3 deadbeef bpf-actlog",
error: `This snap requires that snapd be compiled against libseccomp >= 2.4.`,
}, {
sysIDs: "[snap_daemon]",
classic: true,
scVer: "dead 3.0.0 deadbeef bpf-actlog",
}, {
sysIDs: "[snap_daemon]",
classic: true,
scVer: "dead 3.0.0 deadbeef bpf-actlog",
}, {
sysIDs: "[daemon]",
scVer: "dead 2.4.1 deadbeef -",
error: `This snap requires that snapd be compiled against golang-seccomp >= 0.9.1`,
}, {
sysIDs: "[snap_daemon]",
classic: true,
scVer: "dead 2.4.1 deadbeef -",
error: `This snap requires that snapd be compiled against golang-seccomp >= 0.9.1`,
}}

func (s *checkSnapSuite) TestCheckSnapSystemUsernames(c *C) {
restore := release.MockOnClassic(false)
defer restore()

for _, test := range systemUsernamesTests {
restore = interfaces.MockSeccompCompilerVersionInfo(func(_ string) (string, error) {
return test.scVer, nil
})
defer restore()

release.OnClassic = test.classic
if test.noGroup {
restore = snapstate.MockFindGid(func(name string) (uint64, error) {
Expand Down
3 changes: 1 addition & 2 deletions sandbox/seccomp/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ func (c *Compiler) VersionInfo() (string, error) {
// VersionInfo represents information about the seccomp compilter
type VersionInfo string

// LibseccompVersion parses VersionInfo and provides the
// libseccomp version
// LibseccompVersion parses VersionInfo and provides the libseccomp version
func (vi VersionInfo) LibseccompVersion() (string, error) {
if match := validVersionInfo.Match([]byte(vi)); !match {
return "", fmt.Errorf("invalid format of version-info: %q", vi)
Expand Down

0 comments on commit 5ce5ff1

Please sign in to comment.