opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-08-06 11:32:07 +08:00
Code Issues Packages Projects Releases Wiki Activity

Ensure that --userns=keep-id sets user in config

Browse Source 
One of the side-effects of the `--userns=keep-id` command is
switching the default user of the container to the UID of the
user running Podman (though this can still be overridden by the
`--user` flag). However, it did this by setting the UID and GID
in the OCI spec, and not by informing Libpod of its intention to
switch users via the `WithUser()` option. Because of this, a lot
of the code that should have triggered when the container ran
with a non-root user was not triggering. In the case of the issue
that this fixed, the code to remove capabilities from non-root
users was not triggering. Adjust the keep-id code to properly
inform Libpod of our intention to use a non-root user to fix
this.

Also, fix an annoying race around short-running exec sessions
where Podman would always print a warning that the exec session
had already stopped.

Fixes #9919

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
This commit is contained in:
Matthew Heon
2021-04-05 15:49:35 -04:00
committed by Matthew Heon
parent 3fae801a37
commit 541252afa7
6 changed files with 178 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
libpod/container_exec.go
View File

@ -696,6 +696,24 @@ func (c *Container) ExecResize(sessionID string, newSize define.TerminalSize) er
return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot resize container %s exec session %s as it is not running", c.ID(), session.ID())
}
// The exec session may have exited since we last updated.
// Needed to prevent race conditions around short-running exec sessions.
running, err := c.ociRuntime.ExecUpdateStatus(c, session.ID())
if err != nil {
return err
}
if !running {
session.State = define.ExecStateStopped
if err := c.save(); err != nil {
logrus.Errorf("Error saving state of container %s: %v", c.ID(), err)
}
return errors.Wrapf(define.ErrExecSessionStateInvalid, "cannot resize container %s exec session %s as it has stopped", c.ID(), session.ID())
}
// Make sure the exec session is still running.
return c.ociRuntime.ExecAttachResize(c, sessionID, newSize)
}
@ -720,8 +738,13 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi
logrus.Debugf("Sending resize events to exec session %s", sessionID)
for resizeRequest := range resize {
if err := c.ExecResize(sessionID, resizeRequest); err != nil {
// Assume the exec session went down.
logrus.Warnf("Error resizing exec session %s: %v", sessionID, err)
if errors.Cause(err) == define.ErrExecSessionStateInvalid {
// The exec session stopped
// before we could resize.
logrus.Infof("Missed resize on exec session %s, already stopped", sessionID)
} else {
logrus.Warnf("Error resizing exec session %s: %v", sessionID, err)
}
return
}
}

6
libpod/container_validate.go
View File

@ -126,5 +126,11 @@ func (c *Container) validate() error {
}
}
// If User in the OCI spec is set, require that c.config.User is set for
// security reasons (a lot of our code relies on c.config.User).
if c.config.User == "" && (c.config.Spec.Process.User.UID != 0 || c.config.Spec.Process.User.GID != 0) {
return errors.Wrapf(define.ErrInvalidArg, "please set User explicitly via WithUser() instead of in OCI spec directly")
}
return nil
}

124
libpod/oci_conmon_exec_linux.go
View File

@ -2,18 +2,23 @@ package libpod
import (
"fmt"
"io/ioutil"
"net/http"
"os"
"os/exec"
"path/filepath"
"strings"
"syscall"
"time"
"github.com/containers/common/pkg/capabilities"
"github.com/containers/common/pkg/config"
"github.com/containers/podman/v3/libpod/define"
"github.com/containers/podman/v3/pkg/errorhandling"
"github.com/containers/podman/v3/pkg/lookup"
"github.com/containers/podman/v3/pkg/util"
"github.com/containers/podman/v3/utils"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
@ -654,3 +659,122 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp
}
}
}
// prepareProcessExec returns the path of the process.json used in runc exec -p
// caller is responsible to close the returned *os.File if needed.
func prepareProcessExec(c *Container, options *ExecOptions, env []string, sessionID string) (*os.File, error) {
f, err := ioutil.TempFile(c.execBundlePath(sessionID), "exec-process-")
if err != nil {
return nil, err
}
pspec := new(spec.Process)
if err := JSONDeepCopy(c.config.Spec.Process, pspec); err != nil {
return nil, err
}
pspec.SelinuxLabel = c.config.ProcessLabel
pspec.Args = options.Cmd
// We need to default this to false else it will inherit terminal as true
// from the container.
pspec.Terminal = false
if options.Terminal {
pspec.Terminal = true
}
if len(env) > 0 {
pspec.Env = append(pspec.Env, env...)
}
if options.Cwd != "" {
pspec.Cwd = options.Cwd
}
var addGroups []string
var sgids []uint32
// if the user is empty, we should inherit the user that the container is currently running with
user := options.User
if user == "" {
logrus.Debugf("Set user to %s", c.config.User)
user = c.config.User
addGroups = c.config.Groups
}
overrides := c.getUserOverrides()
execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, overrides)
if err != nil {
return nil, err
}
if len(addGroups) > 0 {
sgids, err = lookup.GetContainerGroups(addGroups, c.state.Mountpoint, overrides)
if err != nil {
return nil, 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
}
ctrSpec, err := c.specFromState()
if err != nil {
return nil, err
}
allCaps, err := capabilities.BoundingSet()
if err != nil {
return nil, err
}
if options.Privileged {
pspec.Capabilities.Bounding = allCaps
} else {
pspec.Capabilities.Bounding = ctrSpec.Process.Capabilities.Bounding
}
if execUser.Uid == 0 {
pspec.Capabilities.Effective = pspec.Capabilities.Bounding
pspec.Capabilities.Inheritable = pspec.Capabilities.Bounding
pspec.Capabilities.Permitted = pspec.Capabilities.Bounding
pspec.Capabilities.Ambient = pspec.Capabilities.Bounding
} else {
if user == c.config.User {
pspec.Capabilities.Effective = ctrSpec.Process.Capabilities.Effective
pspec.Capabilities.Inheritable = ctrSpec.Process.Capabilities.Effective
pspec.Capabilities.Permitted = ctrSpec.Process.Capabilities.Effective
pspec.Capabilities.Ambient = ctrSpec.Process.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 nil, err
}
if err := ioutil.WriteFile(f.Name(), processJSON, 0644); err != nil {
return nil, err
}
return f, nil
}

120
libpod/oci_conmon_linux.go
View File

@ -22,7 +22,6 @@ import (
"text/template"
"time"
"github.com/containers/common/pkg/capabilities"
"github.com/containers/common/pkg/config"
conmonConfig "github.com/containers/conmon/runner/config"
"github.com/containers/podman/v3/libpod/define"
@ -30,7 +29,6 @@ import (
"github.com/containers/podman/v3/pkg/cgroups"
"github.com/containers/podman/v3/pkg/checkpoint/crutils"
"github.com/containers/podman/v3/pkg/errorhandling"
"github.com/containers/podman/v3/pkg/lookup"
"github.com/containers/podman/v3/pkg/rootless"
"github.com/containers/podman/v3/pkg/util"
"github.com/containers/podman/v3/utils"
@ -1195,124 +1193,6 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
return nil
}
// prepareProcessExec returns the path of the process.json used in runc exec -p
// caller is responsible to close the returned *os.File if needed.
func prepareProcessExec(c *Container, options *ExecOptions, env []string, sessionID string) (*os.File, error) {
f, err := ioutil.TempFile(c.execBundlePath(sessionID), "exec-process-")
if err != nil {
return nil, err
}
pspec := new(spec.Process)
if err := JSONDeepCopy(c.config.Spec.Process, pspec); err != nil {
return nil, err
}
pspec.SelinuxLabel = c.config.ProcessLabel
pspec.Args = options.Cmd
// We need to default this to false else it will inherit terminal as true
// from the container.
pspec.Terminal = false
if options.Terminal {
pspec.Terminal = true
}
if len(env) > 0 {
pspec.Env = append(pspec.Env, env...)
}
if options.Cwd != "" {
pspec.Cwd = options.Cwd
}
var addGroups []string
var sgids []uint32
// if the user is empty, we should inherit the user that the container is currently running with
user := options.User
if user == "" {
user = c.config.User
addGroups = c.config.Groups
}
overrides := c.getUserOverrides()
execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, overrides)
if err != nil {
return nil, err
}
if len(addGroups) > 0 {
sgids, err = lookup.GetContainerGroups(addGroups, c.state.Mountpoint, overrides)
if err != nil {
return nil, 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
}
ctrSpec, err := c.specFromState()
if err != nil {
return nil, err
}
allCaps, err := capabilities.BoundingSet()
if err != nil {
return nil, err
}
if options.Privileged {
pspec.Capabilities.Bounding = allCaps
} else {
pspec.Capabilities.Bounding = ctrSpec.Process.Capabilities.Bounding
}
if execUser.Uid == 0 {
pspec.Capabilities.Effective = pspec.Capabilities.Bounding
pspec.Capabilities.Inheritable = pspec.Capabilities.Bounding
pspec.Capabilities.Permitted = pspec.Capabilities.Bounding
pspec.Capabilities.Ambient = pspec.Capabilities.Bounding
} else {
if user == c.config.User {
pspec.Capabilities.Effective = ctrSpec.Process.Capabilities.Effective
pspec.Capabilities.Inheritable = ctrSpec.Process.Capabilities.Effective
pspec.Capabilities.Permitted = ctrSpec.Process.Capabilities.Effective
pspec.Capabilities.Ambient = ctrSpec.Process.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 nil, err
}
if err := ioutil.WriteFile(f.Name(), processJSON, 0644); err != nil {
return nil, err
}
return f, nil
}
// configureConmonEnv gets the environment values to add to conmon's exec struct
// TODO this may want to be less hardcoded/more configurable in the future
func (r *ConmonOCIRuntime) configureConmonEnv(ctr *Container, runtimeDir string) ([]string, []*os.File) {

10
pkg/specgen/generate/namespaces.go
View File

@ -157,6 +157,16 @@ func namespaceOptions(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.
case specgen.KeepID:
if rootless.IsRootless() {
toReturn = append(toReturn, libpod.WithAddCurrentUserPasswdEntry())
// If user is not overridden, set user in the container
// to user running Podman.
if s.User == "" {
_, uid, gid, err := util.GetKeepIDMapping()
if err != nil {
return nil, err
}
toReturn = append(toReturn, libpod.WithUser(fmt.Sprintf("%d:%d", uid, gid)))
}
} else {
// keep-id as root doesn't need a user namespace
s.UserNS.NSMode = specgen.Host

14
test/e2e/exec_test.go
View File

@ -119,6 +119,19 @@ var _ = Describe("Podman exec", func() {
Expect(session.ExitCode()).To(Equal(100))
})
It("podman exec in keep-id container drops privileges", func() {
SkipIfNotRootless("This function is not enabled for rootful podman")
ctrName := "testctr1"
testCtr := podmanTest.Podman([]string{"run", "-d", "--name", ctrName, "--userns=keep-id", ALPINE, "top"})
testCtr.WaitWithDefaultTimeout()
Expect(testCtr.ExitCode()).To(Equal(0))
session := podmanTest.Podman([]string{"exec", ctrName, "grep", "CapEff", "/proc/self/status"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(session.OutputToString()).To(ContainSubstring("0000000000000000"))
})
It("podman exec --privileged", func() {
session := podmanTest.Podman([]string{"run", "--privileged", "--rm", ALPINE, "sh", "-c", "grep ^CapBnd /proc/self/status | cut -f 2"})
session.WaitWithDefaultTimeout()
@ -143,7 +156,6 @@ var _ = Describe("Podman exec", func() {
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(session.OutputToString()).To(ContainSubstring(bndPerms))
})
It("podman exec --privileged", func() {