summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libpod/container_exec.go27
-rw-r--r--libpod/container_validate.go6
-rw-r--r--libpod/oci_conmon_exec_linux.go124
-rw-r--r--libpod/oci_conmon_linux.go120
-rw-r--r--pkg/specgen/generate/namespaces.go10
-rw-r--r--test/e2e/exec_test.go14
6 files changed, 178 insertions, 123 deletions
diff --git a/libpod/container_exec.go b/libpod/container_exec.go
index bb43287d9..8d8ed14aa 100644
--- a/libpod/container_exec.go
+++ b/libpod/container_exec.go
@@ -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
}
}
diff --git a/libpod/container_validate.go b/libpod/container_validate.go
index 245121a91..aae96ae85 100644
--- a/libpod/container_validate.go
+++ b/libpod/container_validate.go
@@ -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
}
diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go
index 173edba2b..b43316951 100644
--- a/libpod/oci_conmon_exec_linux.go
+++ b/libpod/oci_conmon_exec_linux.go
@@ -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
+}
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 1c7089e5d..f26ca67ce 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -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) {
diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go
index 845dfdad7..b52e8d100 100644
--- a/pkg/specgen/generate/namespaces.go
+++ b/pkg/specgen/generate/namespaces.go
@@ -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
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index df86eab15..e6f63a391 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -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() {
generated by cgit v1.2.3-54-g00ecf (git 2.39.0) at 2024-11-08 03:10:00 +0000