You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
320 lines
11 KiB
320 lines
11 KiB
From 69daa67c436a8fdeb0149aa5cb0112f03fdb699f Mon Sep 17 00:00:00 2001
|
|
From: Matthew Heon <mheon@redhat.com>
|
|
Date: Mon, 25 Jan 2021 14:18:07 -0500
|
|
Subject: [PATCH] Correct handling of capabilities
|
|
|
|
Ensure that capabilities are properly handled for non-root users
|
|
in privileged containers. We do not want to give full caps, but
|
|
instead only CapInh and CapEff (others should be all-zeroes).
|
|
|
|
Fixing `podman run` is easy - the same code as the Podman 1.6 fix
|
|
works there. The `podman exec` command is far more challenging.
|
|
Exec received a complete rewrite to use Conmon at some point
|
|
before Podman 1.6, and gained many capabilities in the process.
|
|
One of those was the ability to actually tweak the capabilities
|
|
of the exec process - 1.0 did not have that. Since it was needed
|
|
to resolve this CVE, I was forced to backport a large bit of the
|
|
1.0 -> 1.6 exec changes (passing a Process block to the OCI
|
|
runtime, and using `prepareProcessExec()` to prepare said block).
|
|
I am honestly uncomfortable with the size and scope of this
|
|
change but I don't see another way around this.
|
|
|
|
Fixes CVE-2021-20188
|
|
|
|
Signed-off-by: Matthew Heon <mheon@redhat.com>
|
|
---
|
|
libpod/container_api.go | 24 +------
|
|
libpod/oci.go | 148 ++++++++++++++++++++++++++++++++--------
|
|
pkg/spec/spec.go | 8 +++
|
|
3 files changed, 132 insertions(+), 48 deletions(-)
|
|
|
|
diff -up libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/container_api.go.orig libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/container_api.go
|
|
--- libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/container_api.go.orig 2019-02-11 16:26:46.000000000 +0100
|
|
+++ libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/container_api.go 2021-02-12 10:38:48.767172399 +0100
|
|
@@ -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) er
|
|
// 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
|
|
if conState != ContainerStateRunning {
|
|
return errors.Errorf("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
|
|
|
|
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()
|
|
diff -up libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/oci.go.orig libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/oci.go
|
|
--- libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/oci.go.orig 2019-02-11 16:26:46.000000000 +0100
|
|
+++ libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/libpod/oci.go 2021-02-12 10:38:48.768172416 +0100
|
|
@@ -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(ct
|
|
// 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 *Co
|
|
|
|
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 *Co
|
|
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
|
|
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
|
|
+}
|
|
diff -up libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/pkg/spec/spec.go.orig libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/pkg/spec/spec.go
|
|
--- libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/pkg/spec/spec.go.orig 2019-02-11 16:26:46.000000000 +0100
|
|
+++ libpod-921f98f8795eb9fcb19ce581020cfdeff6dee09f/pkg/spec/spec.go 2021-02-12 10:38:48.768172416 +0100
|
|
@@ -325,6 +325,14 @@ func CreateConfigToOCISpec(config *Creat
|
|
}
|
|
} 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
|