Merge pull request #9304 from mheon/100_cve_2021_20188

[v1.0] Correct handling of capabilities

Author: baude

libpod/container_api.go

@@ -2,7 +2,6 @@ package libpod
 
 import (
 	"context"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"strconv"
@@ -11,9 +10,7 @@ import (
 
 	"github.com/containers/libpod/libpod/driver"
 	"github.com/containers/libpod/pkg/inspect"
-	"github.com/containers/libpod/pkg/lookup"
 	"github.com/containers/storage/pkg/stringid"
-	"github.com/docker/docker/daemon/caps"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -263,8 +260,6 @@ func (c *Container) Kill(signal uint) error {
 // TODO allow specifying streams to attach to
 // TODO investigate allowing exec without attaching
 func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir string) error {
-	var capList []string
-
 	locked := false
 	if !c.batched {
 		locked = true
@@ -287,22 +282,8 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 	if conState != ContainerStateRunning {
 		return errors.Wrapf(ErrCtrStateInvalid, "cannot exec into container that is not running")
 	}
-	if privileged || c.config.Privileged {
-		capList = caps.GetAllCapabilities()
-	}
 
-	// If user was set, look it up in the container to get a UID to use on
-	// the host
-	hostUser := ""
-	if user != "" {
-		execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, nil)
-		if err != nil {
-			return err
-		}
-
-		// runc expects user formatted as uid:gid
-		hostUser = fmt.Sprintf("%d:%d", execUser.Uid, execUser.Gid)
-	}
+	isPrivileged := privileged || c.config.Privileged
 
 	// Generate exec session ID
 	// Ensure we don't conflict with an existing session ID
@@ -324,10 +305,11 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 
 	logrus.Debugf("Creating new exec session in container %s with session id %s", c.ID(), sessionID)
 
-	execCmd, err := c.runtime.ociRuntime.execContainer(c, cmd, capList, env, tty, workDir, hostUser, sessionID)
+	execCmd, processFile, err := c.runtime.ociRuntime.execContainer(c, cmd, env, tty, workDir, user, sessionID, isPrivileged)
 	if err != nil {
 		return errors.Wrapf(err, "error exec %s", c.ID())
 	}
+	defer os.Remove(processFile)
 	chWait := make(chan error)
 	go func() {
 		chWait <- execCmd.Wait()

libpod/oci.go

@@ -15,10 +15,12 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/containers/libpod/pkg/lookup"
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/coreos/go-systemd/activation"
 	"github.com/cri-o/ocicni/pkg/ocicni"
+	"github.com/docker/docker/daemon/caps"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/opencontainers/selinux/go-selinux/label"
@@ -735,18 +737,23 @@ func (r *OCIRuntime) unpauseContainer(ctr *Container) error {
 // TODO: Add --detach support
 // TODO: Convert to use conmon
 // TODO: add --pid-file and use that to generate exec session tracking
-func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty bool, cwd, user, sessionID string) (*exec.Cmd, error) {
+func (r *OCIRuntime) execContainer(c *Container, cmd, env []string, tty bool, cwd, user, sessionID string, privileged bool) (*exec.Cmd, string, error) {
 	if len(cmd) == 0 {
-		return nil, errors.Wrapf(ErrInvalidArg, "must provide a command to execute")
+		return nil, "", errors.Wrapf(ErrInvalidArg, "must provide a command to execute")
 	}
 
 	if sessionID == "" {
-		return nil, errors.Wrapf(ErrEmptyID, "must provide a session ID for exec")
+		return nil, "", errors.Wrapf(ErrEmptyID, "must provide a session ID for exec")
 	}
 
 	runtimeDir, err := util.GetRootlessRuntimeDir()
 	if err != nil {
-		return nil, err
+		return nil, "", err
+	}
+
+	processFile, err := prepareProcessExec(c, cmd, env, tty, cwd, user, sessionID, privileged)
+	if err != nil {
+		return nil, "", err
 	}
 
 	args := []string{}
@@ -756,34 +763,14 @@ func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty
 
 	args = append(args, "exec")
 
-	if cwd != "" {
-		args = append(args, "--cwd", cwd)
-	}
+	args = append(args, "--process", processFile)
 
 	args = append(args, "--pid-file", c.execPidPath(sessionID))
 
-	if tty {
-		args = append(args, "--tty")
-	} else {
-		args = append(args, "--tty=false")
-	}
-
-	if user != "" {
-		args = append(args, "--user", user)
-	}
-
 	if c.config.Spec.Process.NoNewPrivileges {
 		args = append(args, "--no-new-privs")
 	}
 
-	for _, cap := range capAdd {
-		args = append(args, "--cap", cap)
-	}
-
-	for _, envVar := range env {
-		args = append(args, "--env", envVar)
-	}
-
 	// Append container ID and command
 	args = append(args, c.ID())
 	args = append(args, cmd...)
@@ -797,10 +784,10 @@ func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty
 	execCmd.Env = append(execCmd.Env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir))
 
 	if err := execCmd.Start(); err != nil {
-		return nil, errors.Wrapf(err, "cannot start container %s", c.ID())
+		return nil, "", errors.Wrapf(err, "cannot start container %s", c.ID())
 	}
 
-	return execCmd, nil
+	return execCmd, processFile, nil
 }
 
 // execStopContainer stops all active exec sessions in a container
@@ -892,3 +879,110 @@ func (r *OCIRuntime) checkpointContainer(ctr *Container, options ContainerCheckp
 	args = append(args, ctr.ID())
 	return utils.ExecCmdWithStdStreams(os.Stdin, os.Stdout, os.Stderr, nil, r.path, args...)
 }
+
+// prepareProcessExec returns the path of the process.json used in runc exec -p.
+// Returns path to the created exec process file. This will need to be removed
+// by the caller when they're done, best effort.
+func prepareProcessExec(c *Container, cmd, env []string, tty bool, cwd, user, sessionID string, privileged bool) (string, error) {
+	filename := filepath.Join(c.bundlePath(), fmt.Sprintf("exec-process-%s", sessionID))
+	f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY, 0600)
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+
+	pspec := c.config.Spec.Process
+	pspec.SelinuxLabel = c.config.ProcessLabel
+	pspec.Args = cmd
+	// We need to default this to false else it will inherit terminal as true
+	// from the container.
+	pspec.Terminal = false
+	if tty {
+		pspec.Terminal = true
+	}
+	if len(env) > 0 {
+		pspec.Env = append(pspec.Env, env...)
+	}
+
+	if cwd != "" {
+		pspec.Cwd = cwd
+
+	}
+
+	var addGroups []string
+	var sgids []uint32
+
+	// if the user is empty, we should inherit the user that the container is currently running with
+	if user == "" {
+		user = c.config.User
+		addGroups = c.config.Groups
+	}
+
+	execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, nil)
+	if err != nil {
+		return "", err
+	}
+
+	if len(addGroups) > 0 {
+		sgids, err = lookup.GetContainerGroups(addGroups, c.state.Mountpoint, nil)
+		if err != nil {
+			return "", errors.Wrapf(err, "error looking up supplemental groups for container %s exec session %s", c.ID(), sessionID)
+		}
+	}
+
+	// If user was set, look it up in the container to get a UID to use on
+	// the host
+	if user != "" || len(sgids) > 0 {
+		if user != "" {
+			for _, sgid := range execUser.Sgids {
+				sgids = append(sgids, uint32(sgid))
+			}
+		}
+		processUser := spec.User{
+			UID:            uint32(execUser.Uid),
+			GID:            uint32(execUser.Gid),
+			AdditionalGids: sgids,
+		}
+
+		pspec.User = processUser
+	}
+
+	allCaps := caps.GetAllCapabilities()
+	pspec.Capabilities.Effective = []string{}
+	if privileged {
+		pspec.Capabilities.Bounding = allCaps
+	} else {
+		pspec.Capabilities.Bounding = []string{}
+	}
+	pspec.Capabilities.Inheritable = pspec.Capabilities.Bounding
+	if execUser.Uid == 0 {
+		pspec.Capabilities.Effective = pspec.Capabilities.Bounding
+		pspec.Capabilities.Permitted = pspec.Capabilities.Bounding
+		pspec.Capabilities.Ambient = pspec.Capabilities.Bounding
+	} else {
+		pspec.Capabilities.Permitted = pspec.Capabilities.Effective
+		pspec.Capabilities.Ambient = pspec.Capabilities.Effective
+	}
+
+	hasHomeSet := false
+	for _, s := range pspec.Env {
+		if strings.HasPrefix(s, "HOME=") {
+			hasHomeSet = true
+			break
+		}
+	}
+	if !hasHomeSet {
+		pspec.Env = append(pspec.Env, fmt.Sprintf("HOME=%s", execUser.Home))
+	}
+
+	processJSON, err := json.Marshal(pspec)
+	if err != nil {
+		return "", err
+	}
+
+	if err := ioutil.WriteFile(filename, processJSON, 0644); err != nil {
+		return "", err
+	}
+
+	return filename, nil
+}

pkg/spec/spec.go

@@ -325,6 +325,14 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint
 		}
 	} else {
 		g.SetupPrivileged(true)
+		if config.User != "" {
+			user := strings.SplitN(config.User, ":", 2)[0]
+			if user != "root" && user != "0" {
+				g.Spec().Process.Capabilities.Effective = []string{}
+				g.Spec().Process.Capabilities.Permitted = []string{}
+				g.Spec().Process.Capabilities.Ambient = []string{}
+			}
+		}
 	}
 
 	// HANDLE SECCOMP