pass LISTEN_* environment into container

Make sure that Podman passes the LISTEN_* environment into containers. Similar to runc, LISTEN_PID is set to 1. Also remove conditionally passing the LISTEN_FDS as extra files. The condition was wrong (inverted) and introduced to fix #3572 which related to running under varlink which has been dropped entirely with Podman 3.0. Note that the NOTIFY_SOCKET and LISTEN_* variables are cleared when running `system service`. Fixes: #10443 Signed-off-by: Daniel J Walsh <[email protected]> Signed-off-by: Valentin Rothberg <[email protected]>
containers · Aug 31, 2021 · 10144b7 · 10144b7
1 parent 8ab84b4
commit 10144b7
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 24 deletions.
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
@@ -773,6 +773,18 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		}
 	}
 
+	// Pass down the LISTEN_* environment (see #10443).
+	for _, key := range []string{"LISTEN_PID", "LISTEN_FDS", "LISTEN_FDNAMES"} {
+		if val, ok := os.LookupEnv(key); ok {
+			// Force the PID to `1` since we cannot rely on (all
+			// versions of) all runtimes to do it for us.
+			if key == "LISTEN_PID" {
+				val = "1"
+			}
+			g.AddProcessEnv(key, val)
+		}
+	}
+
 	return g.Config, nil
 }
 

diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go
@@ -438,7 +438,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
 	// 	}
 	// }
 
-	conmonEnv, extraFiles := r.configureConmonEnv(c, runtimeDir)
+	conmonEnv := r.configureConmonEnv(c, runtimeDir)
 
 	var filesToClose []*os.File
 	if options.PreserveFDs > 0 {
@@ -456,7 +456,6 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
 	execCmd.Env = append(execCmd.Env, conmonEnv...)
 
 	execCmd.ExtraFiles = append(execCmd.ExtraFiles, childSyncPipe, childStartPipe, childAttachPipe)
-	execCmd.ExtraFiles = append(execCmd.ExtraFiles, extraFiles...)
 	execCmd.Dir = c.execBundlePath(sessionID)
 	execCmd.SysProcAttr = &syscall.SysProcAttr{
 		Setpgid: true,

diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
@@ -34,7 +34,6 @@ import (
 	"github.com/containers/podman/v3/utils"
 	"github.com/containers/storage/pkg/homedir"
 	pmount "github.com/containers/storage/pkg/mount"
-	"github.com/coreos/go-systemd/v22/activation"
 	"github.com/coreos/go-systemd/v22/daemon"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -66,7 +65,6 @@ type ConmonOCIRuntime struct {
 	supportsJSON      bool
 	supportsKVM       bool
 	supportsNoCgroups bool
-	sdNotify          bool
 	enableKeyring     bool
 }
 
@@ -105,7 +103,6 @@ func newConmonOCIRuntime(name string, paths []string, conmonPath string, runtime
 	runtime.logSizeMax = runtimeCfg.Containers.LogSizeMax
 	runtime.noPivot = runtimeCfg.Engine.NoPivotRoot
 	runtime.reservePorts = runtimeCfg.Engine.EnablePortReservation
-	runtime.sdNotify = runtimeCfg.Engine.SDNotify
 	runtime.enableKeyring = runtimeCfg.Containers.EnableKeyring
 
 	// TODO: probe OCI runtime for feature and enable automatically if
@@ -1050,8 +1047,22 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 		}
 	}
 
-	if ctr.config.PreserveFDs > 0 {
-		args = append(args, formatRuntimeOpts("--preserve-fds", fmt.Sprintf("%d", ctr.config.PreserveFDs))...)
+	// Pass down the LISTEN_* environment (see #10443).
+	preserveFDs := ctr.config.PreserveFDs
+	if val := os.Getenv("LISTEN_FDS"); val != "" {
+		if ctr.config.PreserveFDs > 0 {
+			logrus.Warnf("Ignoring LISTEN_FDS to preserve custom user-specified FDs")
+		} else {
+			fds, err := strconv.Atoi(val)
+			if err != nil {
+				return fmt.Errorf("converting LISTEN_FDS=%s: %w", val, err)
+			}
+			preserveFDs = uint(fds)
+		}
+	}
+
+	if preserveFDs > 0 {
+		args = append(args, formatRuntimeOpts("--preserve-fds", fmt.Sprintf("%d", preserveFDs))...)
 	}
 
 	if restoreOptions != nil {
@@ -1104,11 +1115,11 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 	}
 
 	// 0, 1 and 2 are stdin, stdout and stderr
-	conmonEnv, envFiles := r.configureConmonEnv(ctr, runtimeDir)
+	conmonEnv := r.configureConmonEnv(ctr, runtimeDir)
 
 	var filesToClose []*os.File
-	if ctr.config.PreserveFDs > 0 {
-		for fd := 3; fd < int(3+ctr.config.PreserveFDs); fd++ {
+	if preserveFDs > 0 {
+		for fd := 3; fd < int(3+preserveFDs); fd++ {
 			f := os.NewFile(uintptr(fd), fmt.Sprintf("fd-%d", fd))
 			filesToClose = append(filesToClose, f)
 			cmd.ExtraFiles = append(cmd.ExtraFiles, f)
@@ -1118,10 +1129,9 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 	cmd.Env = r.conmonEnv
 	// we don't want to step on users fds they asked to preserve
 	// Since 0-2 are used for stdio, start the fds we pass in at preserveFDs+3
-	cmd.Env = append(cmd.Env, fmt.Sprintf("_OCI_SYNCPIPE=%d", ctr.config.PreserveFDs+3), fmt.Sprintf("_OCI_STARTPIPE=%d", ctr.config.PreserveFDs+4))
+	cmd.Env = append(cmd.Env, fmt.Sprintf("_OCI_SYNCPIPE=%d", preserveFDs+3), fmt.Sprintf("_OCI_STARTPIPE=%d", preserveFDs+4))
 	cmd.Env = append(cmd.Env, conmonEnv...)
 	cmd.ExtraFiles = append(cmd.ExtraFiles, childSyncPipe, childStartPipe)
-	cmd.ExtraFiles = append(cmd.ExtraFiles, envFiles...)
 
 	if r.reservePorts && !rootless.IsRootless() && !ctr.config.NetMode.IsSlirp4netns() {
 		ports, err := bindPorts(ctr.config.PortMappings)
@@ -1225,7 +1235,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 
 // 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) {
+func (r *ConmonOCIRuntime) configureConmonEnv(ctr *Container, runtimeDir string) []string {
 	var env []string
 	for _, e := range os.Environ() {
 		if strings.HasPrefix(e, "LC_") {
@@ -1240,17 +1250,7 @@ func (r *ConmonOCIRuntime) configureConmonEnv(ctr *Container, runtimeDir string)
 		env = append(env, fmt.Sprintf("HOME=%s", home))
 	}
 
-	extraFiles := make([]*os.File, 0)
-	if !r.sdNotify {
-		if listenfds, ok := os.LookupEnv("LISTEN_FDS"); ok {
-			env = append(env, fmt.Sprintf("LISTEN_FDS=%s", listenfds), "LISTEN_PID=1")
-			fds := activation.Files(false)
-			extraFiles = append(extraFiles, fds...)
-		}
-	} else {
-		logrus.Debug("disabling SD notify")
-	}
-	return env, extraFiles
+	return env
 }
 
 // sharedConmonArgs takes common arguments for exec and create/restore and formats them for the conmon CLI

diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats
@@ -136,4 +136,46 @@ function service_cleanup() {
     service_cleanup
 }
 
+function set_listen_env() {
+    export LISTEN_PID="100" LISTEN_FDS="1" LISTEN_FDNAMES="listen_fdnames"
+}
+
+function unset_listen_env() {
+    unset LISTEN_PID LISTEN_FDS LISTEN_FDNAMES
+}
+
+function check_listen_env() {
+    local stdenv="$1"
+    local context="$2"
+    if is_remote; then
+	is "$output" "$stdenv" "LISTEN Environment did not pass: $context"
+    else
+	is "$output" "$stdenv
+LISTEN_PID=1
+LISTEN_FDS=1
+LISTEN_FDNAMES=listen_fdnames" "LISTEN Environment passed: $context"
+    fi
+}
+
+@test "podman pass LISTEN environment " {
+    # Note that `--hostname=host1` makes sure that all containers have the same
+    # environment.
+    run_podman run --hostname=host1 --rm $IMAGE printenv
+    stdenv=$output
+
+    # podman run
+    set_listen_env
+    run_podman run --hostname=host1 --rm $IMAGE printenv
+    unset_listen_env
+    check_listen_env "$stdenv" "podman run"
+
+    # podman start
+    run_podman create --hostname=host1 --rm $IMAGE printenv
+    cid="$output"
+    set_listen_env
+    run_podman start --attach $cid
+    unset_listen_env
+    check_listen_env "$stdenv" "podman start"
+}
+
 # vim: filetype=sh