diff options
| -rw-r--r-- | .cirrus.yml | 2 | ||||
| -rw-r--r-- | contrib/cirrus/lib.sh | 8 | ||||
| -rwxr-xr-x | contrib/cirrus/runner.sh | 19 | ||||
| -rwxr-xr-x | hack/get_ci_vm.sh | 316 | ||||
| -rw-r--r-- | libpod/container_exec.go | 27 | ||||
| -rw-r--r-- | libpod/container_validate.go | 6 | ||||
| -rw-r--r-- | libpod/oci_conmon_exec_linux.go | 124 | ||||
| -rw-r--r-- | libpod/oci_conmon_linux.go | 120 | ||||
| -rw-r--r-- | pkg/specgen/generate/namespaces.go | 10 | ||||
| -rw-r--r-- | test/e2e/exec_test.go | 14 |
10 files changed, 254 insertions, 392 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index 074f2f4e0..b77464bae 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -30,7 +30,7 @@ env: PRIOR_UBUNTU_NAME: "ubuntu-2004" # Google-cloud VM Images - IMAGE_SUFFIX: "c5961289315909632" + IMAGE_SUFFIX: "c5501386583441408" FEDORA_CACHE_IMAGE_NAME: "fedora-${IMAGE_SUFFIX}" PRIOR_FEDORA_CACHE_IMAGE_NAME: "prior-fedora-${IMAGE_SUFFIX}" UBUNTU_CACHE_IMAGE_NAME: "ubuntu-${IMAGE_SUFFIX}" diff --git a/contrib/cirrus/lib.sh b/contrib/cirrus/lib.sh index 451a267b3..2cd28e34a 100644 --- a/contrib/cirrus/lib.sh +++ b/contrib/cirrus/lib.sh @@ -13,8 +13,12 @@ set -a _waserrexit=0 if [[ "$SHELLOPTS" =~ errexit ]]; then _waserrexit=1; fi set +e # Assumed in F33 for setting global vars -source /etc/profile -source /etc/environment +if [[ -r "/etc/automation_environment" ]]; then + source /etc/automation_environment +else # prior to automation library v2.0, this was necessary + source /etc/profile + source /etc/environment +fi if [[ -r "/etc/ci_environment" ]]; then source /etc/ci_environment; fi USER="$(whoami)" HOME="$(getent passwd $USER | cut -d : -f 6)" diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh index f52e107cc..8d3a6b987 100755 --- a/contrib/cirrus/runner.sh +++ b/contrib/cirrus/runner.sh @@ -269,13 +269,18 @@ function _run_release() { } logformatter() { - # Use similar format as human-friendly task name from .cirrus.yml - # shellcheck disable=SC2154 - output_name="$TEST_FLAVOR-$PODBIN_NAME-$DISTRO_NV-$PRIV_NAME-$TEST_ENVIRON" - # Requires stdin and stderr combined! - cat - \ - |& awk --file "${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/timestamp.awk" \ - |& "${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/logformatter" "$output_name" + if [[ "$CI" == "true" ]]; then + # Use similar format as human-friendly task name from .cirrus.yml + # shellcheck disable=SC2154 + output_name="$TEST_FLAVOR-$PODBIN_NAME-$DISTRO_NV-$PRIV_NAME-$TEST_ENVIRON" + # Requires stdin and stderr combined! + cat - \ + |& awk --file "${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/timestamp.awk" \ + |& "${CIRRUS_WORKING_DIR}/${SCRIPT_BASE}/logformatter" "$output_name" + else + # Assume script is run by a human, they want output immediatly + cat - + fi } # Handle local|remote integration|system testing in a uniform way diff --git a/hack/get_ci_vm.sh b/hack/get_ci_vm.sh index 4f6c42a06..1a4804857 100755 --- a/hack/get_ci_vm.sh +++ b/hack/get_ci_vm.sh @@ -3,270 +3,68 @@ # # For help and usage information, simply execute the script w/o any arguments. # -# This script is intended to be run by podman developers who need to debug -# problems specifically related to Cirrus-CI automated testing. However, -# because it's only loosely coupled to the `.cirrus.yml` configuration, it must -# orchestrate VMs in GCP directly. This means users need to have -# pre-authorization (access) to manipulate google-cloud resources. Additionally, -# there are no guarantees it will remain in-sync with other automation-related -# scripts. Therefore it may not always function for everybody in every -# future scenario without updates/modifications/tweaks. -# -# When successful, you will end up connected to a GCP VM with with a clone of -# the upstream podman repository 'master' branch, using a remote named 'origin'. -# If you want to customize this behavior, you will want to use a "hook" script. -# Please use this example carefully, since git setups vary by person, you -# will probably need to make local edits. -# -# https://gist.github.com/cevich/626a0790c0b476d5cd2a5a76fbdae0a1 +# This script is intended to be run by Red Hat podman developers who need +# to debug problems specifically related to Cirrus-CI automated testing. +# It requires that you have been granted prior access to create VMs in +# google-cloud. For non-Red Hat contributors, VMs are available as-needed, +# with supervision upon request. set -e -RED="\e[1;31m" -YEL="\e[1;32m" -NOR="\e[0m" -USAGE_WARNING=" -${YEL}WARNING: This will not work without podman,${NOR} - ${YEL}and prior authorization to use the libpod GCP project.${NOR} -" -# These values come from .cirrus.yml gce_instance clause -ZONE="${ZONE:-us-central1-a}" -CPUS="2" -MEMORY="4Gb" -DISK="200" -PROJECT="libpod-218412" -GOSRC="/var/tmp/go/src/github.com/containers/podman" -GIT_REPO="https://github.com/containers/podman.git" - -# Container image with necessary runtime elements -GCLOUD_IMAGE="${GCLOUD_IMAGE:-docker.io/google/cloud-sdk:alpine}" -GCLOUD_CFGDIR=".config/gcloud" - -SCRIPT_FILENAME=$(basename ${BASH_SOURCE[0]}) -HOOK_FILENAME="hook_${SCRIPT_FILENAME}" +SCRIPT_FILEPATH=$(realpath "${BASH_SOURCE[0]}") +SCRIPT_DIRPATH=$(dirname "$SCRIPT_FILEPATH") +REPO_DIRPATH=$(realpath "$SCRIPT_DIRPATH/../") -# Shared tmp directory between container and us -TMPDIR=$(mktemp -d --tmpdir ${SCRIPT_FILENAME}_tmpdir_XXXXXX) - -show_usage() { - echo -e "\n${RED}ERROR: $1${NOR}" - echo -e "${YEL}Usage: $SCRIPT_FILENAME <image_name>${NOR}" - echo "" - if [[ -r ".cirrus.yml" ]] - then - echo -e "${YEL}Some possible image_name values (from .cirrus.yml):${NOR}" - image_hints - echo "" - echo -e "${YEL}Optional:${NOR} If a $HOME/$GCLOUD_CFGDIR/$HOOK_FILENAME executable exists during" - echo "VM creation, it will be executed remotely after cloning" - echo "$GIT_REPO. The" - echo "current local working branch name and commit ID, will be provided as" - echo "it's arguments." +# Help detect if we were called by get_ci_vm container +GET_CI_VM="${GET_CI_VM:-0}" +in_get_ci_vm() { + if ((GET_CI_VM==0)); then + echo "Error: $1 is not intended for use in this context" + exit 2 fi - exit 1 -} - -LIBPODROOT=$(realpath "$(dirname ${BASH_SOURCE[0]})/../") -# else: Assume $PWD is the root of the libpod repository -[[ "$LIBPODROOT" != "/" ]] || \ - show_usage "Must execute script from within clone of containers/podman repo." - -[[ "$UID" -ne 0 ]] || \ - show_usage "Must execute script as a regular (non-root) user." - -[[ "${LIBPODROOT#$HOME}" != "$LIBPODROOT" ]] || \ - show_usage "Clone of containers/podman must be a subdirectory of \$HOME ($HOME)" - -# Disable SELinux labeling to allow read-only mounting of repository files -PGCLOUD="podman run -it --rm --security-opt label=disable -v $TMPDIR:$TMPDIR -v $HOME/.config/gcloud:/root/.config/gcloud -v $HOME/.config/gcloud/ssh:/root/.ssh -v $LIBPODROOT:$LIBPODROOT:ro $GCLOUD_IMAGE gcloud --configuration=libpod --project=$PROJECT" -SCP_CMD="$PGCLOUD compute scp" - -showrun() { - echo '+ '$(printf " %q" "$@") > /dev/stderr - echo "" - "$@" -} - -cleanup() { - RET=$? - set +e - wait - - # set GCLOUD_DEBUG to leave tmpdir behind for postmortem - # shellcheck disable=SC2154 - test -z "$GCLOUD_DEBUG" && rm -rf $TMPDIR - - # Not always called from an exit handler, but should always exit when called - exit $RET -} -trap cleanup EXIT - -delvm() { - echo -e "\n" - echo -e "\n${YEL}Offering to Delete $VMNAME${NOR}" - echo -e "${RED}(Deletion might take a minute or two)${NOR}" - echo -e "${YEL}Note: It's safe to answer N, then re-run script again later.${NOR}" - showrun $CLEANUP_CMD # prompts for Yes/No - cleanup -} - -get_env_vars() { - # Deal with both YAML and embedded shell-like substitutions in values - # if substitution fails, fall back to printing naked env. var as-is. - python3 -c ' -import sys,yaml,re -env=yaml.load(open(".cirrus.yml"), Loader=yaml.SafeLoader)["env"] -dollar_env_var=re.compile(r"\$(\w+)") -dollarcurly_env_var=re.compile(r"\$\{(\w+)\}") -class ReIterKey(dict): - def __missing__(self, key): - # Cirrus-CI provides some runtime-only env. vars. Avoid - # breaking this hack-script if/when any are present in YAML - return "${0}".format(key) -rep=r"{\1}" # Convert env vars markup to -> str.format_map(re_iter_key) markup -out=ReIterKey() -for k,v in env.items(): - if "ENCRYPTED" not in str(v) and bool(v): - out[k]=dollar_env_var.sub(rep, dollarcurly_env_var.sub(rep, str(v))) -for k,v in out.items(): - sys.stdout.write("{0}=\"{1}\"\n".format(k, str(v).format_map(out))) - ' -} - -image_hints() { - get_env_vars | fgrep '_CACHE_IMAGE_NAME' | awk -F "=" '{print $2}' } -unset VM_IMAGE_NAME -unset VMNAME -unset CREATE_CMD -unset SSH_CMD -unset CLEANUP_CMD -declare -xa ENVS -parse_args(){ - local arg - echo -e "$USAGE_WARNING" - - if [[ "$USER" =~ "root" ]] - then - show_usage "This script must be run as a regular user." - fi - - [[ "$#" -eq 1 ]] || \ - show_usage "Must specify a VM Image name to use, and the test flavor." - - VM_IMAGE_NAME="$1" - - # Word-splitting is desirable in this case. - # Values are used literally (with '=') as args to future `env` command. - # get_env_vars() will take care of properly quoting it's output. - # shellcheck disable=SC2207,SC2191 - ENVS=( - $(get_env_vars) - VM_IMAGE_NAME="$VM_IMAGE_NAME" - UPSTREAM_REMOTE="upstream" - ) - - VMNAME="${VMNAME:-${USER}-${VM_IMAGE_NAME}}" - - CREATE_CMD="$PGCLOUD compute instances create --zone=$ZONE --image=${VM_IMAGE_NAME} --custom-cpu=$CPUS --custom-memory=$MEMORY --boot-disk-size=$DISK --labels=in-use-by=$USER $VMNAME" - - SSH_CMD="$PGCLOUD compute ssh root@$VMNAME" - - CLEANUP_CMD="$PGCLOUD compute instances delete --zone $ZONE --delete-disks=all $VMNAME" -} - -# Returns true if user has run an 'init' and has a valid token for -# the specific project-id and named-configuration arguments in $PGCLOUD. -function has_valid_credentials() { - if $PGCLOUD info |& grep -Eq 'Account:.*None'; then - return 1 - fi - - # It's possible for 'gcloud info' to list expired credentials, - # e.g. 'ERROR: ... invalid grant: Bad Request' - if $PGCLOUD auth print-access-token |& grep -q 'ERROR'; then - return 1 - fi - - return 0 -} - -##### main - -[[ "${LIBPODROOT%%${LIBPODROOT##$HOME}}" == "$HOME" ]] || \ - show_usage "Repo clone must be sub-dir of $HOME" - -cd "$LIBPODROOT" - -parse_args "$@" -mkdir -p $TMPDIR/.ssh -mkdir -p {$HOME,$TMPDIR}/.config/gcloud/ssh -chmod 700 {$HOME,$TMPDIR}/.config/gcloud/ssh $TMPDIR/.ssh - -echo -e "\n${YEL}Pulling gcloud image...${NOR}" -podman pull $GCLOUD_IMAGE - -if ! has_valid_credentials -then - echo -e "\n${YEL}WARNING: Can't find gcloud configuration for libpod, running init.${NOR}" - echo -e " ${RED}Please choose \"#1: Re-initialize\" and \"login\" if asked.${NOR}" - showrun $PGCLOUD init --project=$PROJECT --console-only --skip-diagnostics - - # Verify it worked (account name == someone@example.com) - $PGCLOUD info > $TMPDIR/gcloud-info-after-init - if egrep -q "Account:.*None" $TMPDIR/gcloud-info-after-init - then - echo -e "${RED}ERROR: Could not initialize libpod configuration in gcloud.${NOR}" - exit 5 - fi - - # If this is the only config, make it the default to avoid - # persistent warnings from gcloud about there being no default. - [[ -r "$HOME/.config/gcloud/configurations/config_default" ]] || \ - ln "$HOME/.config/gcloud/configurations/config_libpod" \ - "$HOME/.config/gcloud/configurations/config_default" +# get_ci_vm APIv1 container entrypoint calls into this script +# to obtain required repo. specific configuration options. +if [[ "$1" == "--config" ]]; then + in_get_ci_vm "$1" + cat <<EOF +DESTDIR="/var/tmp/go/src/github.com/containers/podman" +UPSTREAM_REPO="https://github.com/containers/podman.git" +CI_ENVFILE="/etc/ci_environment" +GCLOUD_PROJECT="libpod-218412" +GCLOUD_IMGPROJECT="libpod-218412" +GCLOUD_CFG="libpod" +GCLOUD_ZONE="${GCLOUD_ZONE:-us-central1-a}" +GCLOUD_CPUS="2" +GCLOUD_MEMORY="4Gb" +GCLOUD_DISK="200" +EOF +elif [[ "$1" == "--setup" ]]; then + in_get_ci_vm "$1" + # get_ci_vm container entrypoint calls us with this option on the + # Cirrus-CI environment instance, to perform repo.-specific setup. + cd $REPO_DIRPATH + echo "+ Loading ./contrib/cirrus/lib.sh" > /dev/stderr + source ./contrib/cirrus/lib.sh + echo "+ Mimicking .cirrus.yml clone_script and build_task" > /dev/stderr + make install.tools + make vendor + make podman + make podman-remote + echo "+ Running environment setup" > /dev/stderr + ./contrib/cirrus/setup_environment.sh +else + # Create and access VM for specified Cirrus-CI task + mkdir -p $HOME/.config/gcloud/ssh + podman run -it --rm \ + --tz=local \ + -e NAME="$USER" \ + -e SRCDIR=/src \ + -e GCLOUD_ZONE="$GCLOUD_ZONE" \ + -e DEBUG="${DEBUG:-0}" \ + -v $REPO_DIRPATH:/src:O \ + -v $HOME/.config/gcloud:/root/.config/gcloud:z \ + -v $HOME/.config/gcloud/ssh:/root/.ssh:z \ + quay.io/libpod/get_ci_vm:latest "$@" fi - -trap delvm EXIT # Allow deleting VM if CTRL-C during create -echo -e "\n${YEL}Trying to creating a VM named $VMNAME${NOR}\n${YEL}in GCE region/zone $ZONE${NOR}" -echo -e "For faster terminal access, export ZONE='<something-closer>'" -echo -e 'Zone-list at: https://cloud.google.com/compute/docs/regions-zones/\n' -if showrun $CREATE_CMD; then # Freshly created VM needs initial setup - - echo -e "\n${YEL}Waiting up to 30s for ssh port to open${NOR}" - ATTEMPTS=10 - trap "exit 1" INT - while ((ATTEMPTS)) && ! $SSH_CMD --command "true"; do - let "ATTEMPTS--" - echo -e "${RED}Nope, not yet.${NOR}" - sleep 3s - done - trap - INT - if ! ((ATTEMPTS)); then - echo -e "\n${RED}Failed${NOR}" - exit 7 - fi - echo -e "${YEL}Got it. Cloning upstream repository as a starting point.${NOR}" - - showrun $SSH_CMD -- "mkdir -p $GOSRC" - showrun $SSH_CMD -- "git clone --progress $GIT_REPO $GOSRC" - - if [[ -x "$HOME/$GCLOUD_CFGDIR/$HOOK_FILENAME" ]]; then - echo -e "\n${YEL}Copying hook to VM and executing (ignoring errors).${NOR}" - $PGCLOUD compute scp "/root/$GCLOUD_CFGDIR/$HOOK_FILENAME" root@$VMNAME:. - if ! showrun $SSH_CMD -- "cd $GOSRC && bash /root/$HOOK_FILENAME $(git branch --show-current) $(git rev-parse HEAD)"; then - echo "-e ${RED}Hook exited: $?${NOR}" - fi - fi -fi - -echo -e "\n${YEL}Generating connection script for $VMNAME.${NOR}" -echo -e "Note: Script can be re-used in another terminal if needed." -echo -e "${RED}(option to delete VM presented upon exiting).${NOR}" -# TODO: This is fairly fragile, specifically the quoting for the remote command. -echo '#!/bin/bash' > $TMPDIR/ssh -echo "$SSH_CMD -- -t 'cd $GOSRC && exec env ${ENVS[*]} bash -il'" >> $TMPDIR/ssh -chmod +x $TMPDIR/ssh - -showrun $TMPDIR/ssh 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() { |