From eba94a15c89994f1be5c3bde6802d960971eb755 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 10 Nov 2020 11:11:39 +0100 Subject: [PATCH] pkg/cri/server: remove dependency on libcontainer/apparmor, libcontainer/utils recent versions of libcontainer/apparmor simplified the AppArmor check to only check if the host supports AppArmor, but no longer checks if apparmor_parser is installed, or if we're running docker-in-docker; https://github.com/opencontainers/runc/commit/bfb4ea1b1b2452bb3ddc8183b35dec4914bb5128 > The `apparmor_parser` binary is not really required for a system to run > AppArmor from a runc perspective. How to apply the profile is more in > the responsibility of higher level runtimes like Podman and Docker, > which may do the binary check on their own. This patch copies the logic from libcontainer/apparmor, and restores the additional checks. Signed-off-by: Sebastiaan van Stijn --- pkg/cri/server/apparmor.go | 48 ++++++++ pkg/cri/server/apparmor_unsupported.go | 24 ++++ pkg/cri/server/helpers_linux.go | 8 +- .../runc/libcontainer/apparmor/apparmor.go | 60 ---------- .../apparmor/apparmor_disabled.go | 20 ---- .../runc/libcontainer/utils/cmsg.go | 93 --------------- .../runc/libcontainer/utils/utils.go | 112 ------------------ .../runc/libcontainer/utils/utils_unix.go | 68 ----------- 8 files changed, 78 insertions(+), 355 deletions(-) create mode 100644 pkg/cri/server/apparmor.go create mode 100644 pkg/cri/server/apparmor_unsupported.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/utils/cmsg.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/utils/utils.go delete mode 100644 vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go diff --git a/pkg/cri/server/apparmor.go b/pkg/cri/server/apparmor.go new file mode 100644 index 000000000..1e6586d6c --- /dev/null +++ b/pkg/cri/server/apparmor.go @@ -0,0 +1,48 @@ +// +build apparmor,linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package server + +import ( + "io/ioutil" + "os" + "sync" +) + +var ( + appArmorSupported bool + checkAppArmor sync.Once +) + +// hostSupportsAppArmor returns true if apparmor is enabled for the host, if +// apparmor_parser is enabled, and if we are not running docker-in-docker. +// +// It is a modified version of libcontainer/apparmor.IsEnabled(), which does not +// check for apparmor_parser to be present, or if we're running docker-in-docker. +func hostSupportsAppArmor() bool { + checkAppArmor.Do(func() { + // see https://github.com/docker/docker/commit/de191e86321f7d3136ff42ff75826b8107399497 + if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil && os.Getenv("container") == "" { + if _, err = os.Stat("/sbin/apparmor_parser"); err == nil { + buf, err := ioutil.ReadFile("/sys/module/apparmor/parameters/enabled") + appArmorSupported = err == nil && len(buf) > 1 && buf[0] == 'Y' + } + } + }) + return appArmorSupported +} diff --git a/pkg/cri/server/apparmor_unsupported.go b/pkg/cri/server/apparmor_unsupported.go new file mode 100644 index 000000000..a814325c3 --- /dev/null +++ b/pkg/cri/server/apparmor_unsupported.go @@ -0,0 +1,24 @@ +// +build !apparmor !linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package server + +//nolint: deadcode, unused +func hostSupportsAppArmor() bool { + return false +} diff --git a/pkg/cri/server/helpers_linux.go b/pkg/cri/server/helpers_linux.go index 6fc70ede8..48c9fbfa6 100644 --- a/pkg/cri/server/helpers_linux.go +++ b/pkg/cri/server/helpers_linux.go @@ -32,7 +32,6 @@ import ( "github.com/containerd/containerd/mount" "github.com/containerd/containerd/pkg/seccomp" "github.com/containerd/containerd/pkg/seutil" - runcapparmor "github.com/opencontainers/runc/libcontainer/apparmor" "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" @@ -141,8 +140,13 @@ func checkSelinuxLevel(level string) error { return nil } +// apparmorEnabled returns true if apparmor is enabled, supported by the host, +// if apparmor_parser is installed, and if we are not running docker-in-docker. func (c *criService) apparmorEnabled() bool { - return runcapparmor.IsEnabled() && !c.config.DisableApparmor + if c.config.DisableApparmor { + return false + } + return hostSupportsAppArmor() } func (c *criService) seccompEnabled() bool { diff --git a/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go b/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go deleted file mode 100644 index debfc1e48..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor.go +++ /dev/null @@ -1,60 +0,0 @@ -// +build apparmor,linux - -package apparmor - -import ( - "fmt" - "io/ioutil" - "os" - - "github.com/opencontainers/runc/libcontainer/utils" -) - -// IsEnabled returns true if apparmor is enabled for the host. -func IsEnabled() bool { - if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil && os.Getenv("container") == "" { - if _, err = os.Stat("/sbin/apparmor_parser"); err == nil { - buf, err := ioutil.ReadFile("/sys/module/apparmor/parameters/enabled") - return err == nil && len(buf) > 1 && buf[0] == 'Y' - } - } - return false -} - -func setProcAttr(attr, value string) error { - // Under AppArmor you can only change your own attr, so use /proc/self/ - // instead of /proc// like libapparmor does - path := fmt.Sprintf("/proc/self/attr/%s", attr) - - f, err := os.OpenFile(path, os.O_WRONLY, 0) - if err != nil { - return err - } - defer f.Close() - - if err := utils.EnsureProcHandle(f); err != nil { - return err - } - - _, err = fmt.Fprintf(f, "%s", value) - return err -} - -// changeOnExec reimplements aa_change_onexec from libapparmor in Go -func changeOnExec(name string) error { - value := "exec " + name - if err := setProcAttr("exec", value); err != nil { - return fmt.Errorf("apparmor failed to apply profile: %s", err) - } - return nil -} - -// ApplyProfile will apply the profile with the specified name to the process after -// the next exec. -func ApplyProfile(name string) error { - if name == "" { - return nil - } - - return changeOnExec(name) -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go b/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go deleted file mode 100644 index d4110cf0b..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/apparmor/apparmor_disabled.go +++ /dev/null @@ -1,20 +0,0 @@ -// +build !apparmor !linux - -package apparmor - -import ( - "errors" -) - -var ErrApparmorNotEnabled = errors.New("apparmor: config provided but apparmor not supported") - -func IsEnabled() bool { - return false -} - -func ApplyProfile(name string) error { - if name != "" { - return ErrApparmorNotEnabled - } - return nil -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/utils/cmsg.go b/vendor/github.com/opencontainers/runc/libcontainer/utils/cmsg.go deleted file mode 100644 index c8a9364d5..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/utils/cmsg.go +++ /dev/null @@ -1,93 +0,0 @@ -// +build linux - -package utils - -/* - * Copyright 2016, 2017 SUSE LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import ( - "fmt" - "os" - - "golang.org/x/sys/unix" -) - -// MaxSendfdLen is the maximum length of the name of a file descriptor being -// sent using SendFd. The name of the file handle returned by RecvFd will never -// be larger than this value. -const MaxNameLen = 4096 - -// oobSpace is the size of the oob slice required to store a single FD. Note -// that unix.UnixRights appears to make the assumption that fd is always int32, -// so sizeof(fd) = 4. -var oobSpace = unix.CmsgSpace(4) - -// RecvFd waits for a file descriptor to be sent over the given AF_UNIX -// socket. The file name of the remote file descriptor will be recreated -// locally (it is sent as non-auxiliary data in the same payload). -func RecvFd(socket *os.File) (*os.File, error) { - // For some reason, unix.Recvmsg uses the length rather than the capacity - // when passing the msg_controllen and other attributes to recvmsg. So we - // have to actually set the length. - name := make([]byte, MaxNameLen) - oob := make([]byte, oobSpace) - - sockfd := socket.Fd() - n, oobn, _, _, err := unix.Recvmsg(int(sockfd), name, oob, 0) - if err != nil { - return nil, err - } - - if n >= MaxNameLen || oobn != oobSpace { - return nil, fmt.Errorf("recvfd: incorrect number of bytes read (n=%d oobn=%d)", n, oobn) - } - - // Truncate. - name = name[:n] - oob = oob[:oobn] - - scms, err := unix.ParseSocketControlMessage(oob) - if err != nil { - return nil, err - } - if len(scms) != 1 { - return nil, fmt.Errorf("recvfd: number of SCMs is not 1: %d", len(scms)) - } - scm := scms[0] - - fds, err := unix.ParseUnixRights(&scm) - if err != nil { - return nil, err - } - if len(fds) != 1 { - return nil, fmt.Errorf("recvfd: number of fds is not 1: %d", len(fds)) - } - fd := uintptr(fds[0]) - - return os.NewFile(fd, string(name)), nil -} - -// SendFd sends a file descriptor over the given AF_UNIX socket. In -// addition, the file.Name() of the given file will also be sent as -// non-auxiliary data in the same payload (allowing to send contextual -// information for a file descriptor). -func SendFd(socket *os.File, name string, fd uintptr) error { - if len(name) >= MaxNameLen { - return fmt.Errorf("sendfd: filename too long: %s", name) - } - oob := unix.UnixRights(int(fd)) - return unix.Sendmsg(int(socket.Fd()), []byte(name), oob, nil, 0) -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/utils/utils.go b/vendor/github.com/opencontainers/runc/libcontainer/utils/utils.go deleted file mode 100644 index 40ccfaa1a..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/utils/utils.go +++ /dev/null @@ -1,112 +0,0 @@ -package utils - -import ( - "encoding/json" - "io" - "os" - "path/filepath" - "strings" - "unsafe" - - "golang.org/x/sys/unix" -) - -const ( - exitSignalOffset = 128 -) - -// ResolveRootfs ensures that the current working directory is -// not a symlink and returns the absolute path to the rootfs -func ResolveRootfs(uncleanRootfs string) (string, error) { - rootfs, err := filepath.Abs(uncleanRootfs) - if err != nil { - return "", err - } - return filepath.EvalSymlinks(rootfs) -} - -// ExitStatus returns the correct exit status for a process based on if it -// was signaled or exited cleanly -func ExitStatus(status unix.WaitStatus) int { - if status.Signaled() { - return exitSignalOffset + int(status.Signal()) - } - return status.ExitStatus() -} - -// WriteJSON writes the provided struct v to w using standard json marshaling -func WriteJSON(w io.Writer, v interface{}) error { - data, err := json.Marshal(v) - if err != nil { - return err - } - _, err = w.Write(data) - return err -} - -// CleanPath makes a path safe for use with filepath.Join. This is done by not -// only cleaning the path, but also (if the path is relative) adding a leading -// '/' and cleaning it (then removing the leading '/'). This ensures that a -// path resulting from prepending another path will always resolve to lexically -// be a subdirectory of the prefixed path. This is all done lexically, so paths -// that include symlinks won't be safe as a result of using CleanPath. -func CleanPath(path string) string { - // Deal with empty strings nicely. - if path == "" { - return "" - } - - // Ensure that all paths are cleaned (especially problematic ones like - // "/../../../../../" which can cause lots of issues). - path = filepath.Clean(path) - - // If the path isn't absolute, we need to do more processing to fix paths - // such as "../../../..//some/path". We also shouldn't convert absolute - // paths to relative ones. - if !filepath.IsAbs(path) { - path = filepath.Clean(string(os.PathSeparator) + path) - // This can't fail, as (by definition) all paths are relative to root. - path, _ = filepath.Rel(string(os.PathSeparator), path) - } - - // Clean the path again for good measure. - return filepath.Clean(path) -} - -// SearchLabels searches a list of key-value pairs for the provided key and -// returns the corresponding value. The pairs must be separated with '='. -func SearchLabels(labels []string, query string) string { - for _, l := range labels { - parts := strings.SplitN(l, "=", 2) - if len(parts) < 2 { - continue - } - if parts[0] == query { - return parts[1] - } - } - return "" -} - -// Annotations returns the bundle path and user defined annotations from the -// libcontainer state. We need to remove the bundle because that is a label -// added by libcontainer. -func Annotations(labels []string) (bundle string, userAnnotations map[string]string) { - userAnnotations = make(map[string]string) - for _, l := range labels { - parts := strings.SplitN(l, "=", 2) - if len(parts) < 2 { - continue - } - if parts[0] == "bundle" { - bundle = parts[1] - } else { - userAnnotations[parts[0]] = parts[1] - } - } - return -} - -func GetIntSize() int { - return int(unsafe.Sizeof(1)) -} diff --git a/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go b/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go deleted file mode 100644 index 1576f2d4a..000000000 --- a/vendor/github.com/opencontainers/runc/libcontainer/utils/utils_unix.go +++ /dev/null @@ -1,68 +0,0 @@ -// +build !windows - -package utils - -import ( - "fmt" - "os" - "strconv" - - "golang.org/x/sys/unix" -) - -// EnsureProcHandle returns whether or not the given file handle is on procfs. -func EnsureProcHandle(fh *os.File) error { - var buf unix.Statfs_t - if err := unix.Fstatfs(int(fh.Fd()), &buf); err != nil { - return fmt.Errorf("ensure %s is on procfs: %v", fh.Name(), err) - } - if buf.Type != unix.PROC_SUPER_MAGIC { - return fmt.Errorf("%s is not on procfs", fh.Name()) - } - return nil -} - -// CloseExecFrom applies O_CLOEXEC to all file descriptors currently open for -// the process (except for those below the given fd value). -func CloseExecFrom(minFd int) error { - fdDir, err := os.Open("/proc/self/fd") - if err != nil { - return err - } - defer fdDir.Close() - - if err := EnsureProcHandle(fdDir); err != nil { - return err - } - - fdList, err := fdDir.Readdirnames(-1) - if err != nil { - return err - } - for _, fdStr := range fdList { - fd, err := strconv.Atoi(fdStr) - // Ignore non-numeric file names. - if err != nil { - continue - } - // Ignore descriptors lower than our specified minimum. - if fd < minFd { - continue - } - // Intentionally ignore errors from unix.CloseOnExec -- the cases where - // this might fail are basically file descriptors that have already - // been closed (including and especially the one that was created when - // ioutil.ReadDir did the "opendir" syscall). - unix.CloseOnExec(fd) - } - return nil -} - -// NewSockPair returns a new unix socket pair -func NewSockPair(name string) (parent *os.File, child *os.File, err error) { - fds, err := unix.Socketpair(unix.AF_LOCAL, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) - if err != nil { - return nil, nil, err - } - return os.NewFile(uintptr(fds[1]), name+"-p"), os.NewFile(uintptr(fds[0]), name+"-c"), nil -}