diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index e0a65d5cc8f..6a2de77a104 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -10,6 +10,7 @@ import ( "path/filepath" "runtime" "runtime/debug" + "slices" "strconv" "strings" "syscall" @@ -196,10 +197,6 @@ func startInitialization() (retErr error) { dmzExe = os.NewFile(uintptr(dmzFd), "runc-dmz") } - // clear the current process's environment to clean any libcontainer - // specific env vars. - os.Clearenv() - defer func() { if err := recover(); err != nil { if err2, ok := err.(error); ok { @@ -220,9 +217,11 @@ func startInitialization() (retErr error) { } func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe, dmzExe *os.File) error { - if err := populateProcessEnvironment(config.Env); err != nil { + env, homeSet, err := prepareEnv(config.Env) + if err != nil { return err } + config.Env = env // Clean the RLIMIT_NOFILE cache in go runtime. // Issue: https://github.com/opencontainers/runc/issues/4195 @@ -237,6 +236,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock config: config, logPipe: logPipe, dmzExe: dmzExe, + addHome: !homeSet, } return i.Init() case initStandard: @@ -249,35 +249,57 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock fifoFile: fifoFile, logPipe: logPipe, dmzExe: dmzExe, + addHome: !homeSet, } return i.Init() } return fmt.Errorf("unknown init type %q", t) } -// populateProcessEnvironment loads the provided environment variables into the -// current processes's environment. -func populateProcessEnvironment(env []string) error { - for _, pair := range env { - p := strings.SplitN(pair, "=", 2) - if len(p) < 2 { - return errors.New("invalid environment variable: missing '='") +// prepareEnv checks supplied environment variables for validity, removes +// duplicates in place (leaving the last value only), and sets PATH from env, +// if found. Returns a flag telling if HOME is found in env. +func prepareEnv(env []string) (newEnv []string, homeSet bool, _ error) { + if env == nil { + return nil, false, nil + } + envs := make(map[string]int) + var dups []int + + for i, pair := range env { + j := strings.IndexByte(pair, '=') + if j == -1 { + return nil, false, errors.New("invalid environment variable: missing '='") } - name, val := p[0], p[1] - if name == "" { - return errors.New("invalid environment variable: name cannot be empty") + if j == 0 { + return nil, false, errors.New("invalid environment variable: name cannot be empty") } - if strings.IndexByte(name, 0) >= 0 { - return fmt.Errorf("invalid environment variable %q: name contains nul byte (\\x00)", name) + key := pair[:j] + if strings.IndexByte(pair, 0) >= 0 { + return nil, false, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key) } - if strings.IndexByte(val, 0) >= 0 { - return fmt.Errorf("invalid environment variable %q: value contains nul byte (\\x00)", name) + if last, ok := envs[key]; ok { + dups = append(dups, last) } - if err := os.Setenv(name, val); err != nil { - return err + envs[key] = i + } + + // Check if HOME is set. + _, homeSet = envs["HOME"] + // Set PATH from env, unless empty. + if i, ok := envs["PATH"]; ok { + if err := os.Setenv("PATH", env[i][5:]); err != nil { + return nil, false, err } } - return nil + + // Remove duplicates, starting from the rightmost one. + slices.SortFunc(dups, func(a, b int) int { return b - a }) + for _, rm := range dups { + env = append(env[:rm], env[rm+1:]...) + } + + return env, homeSet, nil } // verifyCwd ensures that the current directory is actually inside the mount @@ -308,8 +330,8 @@ func verifyCwd() error { // finalizeNamespace drops the caps, sets the correct user // and working dir, and closes any leaked file descriptors -// before executing the command inside the namespace -func finalizeNamespace(config *initConfig) error { +// before executing the command inside the namespace. +func finalizeNamespace(config *initConfig, addHome bool) error { // Ensure that all unwanted fds we may have accidentally // inherited are marked close-on-exec so they stay out of the // container @@ -355,7 +377,7 @@ func finalizeNamespace(config *initConfig) error { if err := system.SetKeepCaps(); err != nil { return fmt.Errorf("unable to set keep caps: %w", err) } - if err := setupUser(config); err != nil { + if err := setupUser(config, addHome); err != nil { return fmt.Errorf("unable to setup user: %w", err) } // Change working directory AFTER the user has been set up, if we haven't done it yet. @@ -473,8 +495,9 @@ func syncParentSeccomp(pipe *syncSocket, seccompFd *os.File) error { return readSync(pipe, procSeccompDone) } -// setupUser changes the groups, gid, and uid for the user inside the container -func setupUser(config *initConfig) error { +// setupUser changes the groups, gid, and uid for the user inside the container, +// and appends user's HOME to config.Env if addHome is true. +func setupUser(config *initConfig, addHome bool) error { // Set up defaults. defaultExecUser := user.ExecUser{ Uid: 0, @@ -555,11 +578,9 @@ func setupUser(config *initConfig) error { return err } - // if we didn't get HOME already, set it based on the user's HOME - if envHome := os.Getenv("HOME"); envHome == "" { - if err := os.Setenv("HOME", execUser.Home); err != nil { - return err - } + // If we didn't get HOME already, set it based on the user's HOME. + if addHome { + config.Env = append(config.Env, "HOME="+execUser.Home) } return nil } diff --git a/libcontainer/init_linux_test.go b/libcontainer/init_linux_test.go new file mode 100644 index 00000000000..72d63b4af23 --- /dev/null +++ b/libcontainer/init_linux_test.go @@ -0,0 +1,40 @@ +package libcontainer + +import ( + "slices" + "testing" +) + +func TestPrepareEnvDedup(t *testing.T) { + tests := []struct { + env, wantEnv []string + }{ + { + env: []string{}, + wantEnv: []string{}, + }, + { + env: []string{"HOME=/root", "FOO=bar"}, + wantEnv: []string{"HOME=/root", "FOO=bar"}, + }, + { + env: []string{"A=a", "A=b", "A=c"}, + wantEnv: []string{"A=c"}, + }, + { + env: []string{"TERM=vt100", "HOME=/home/one", "HOME=/home/two", "TERM=xterm", "HOME=/home/three", "FOO=bar"}, + wantEnv: []string{"TERM=xterm", "HOME=/home/three", "FOO=bar"}, + }, + } + + for _, tc := range tests { + env, _, err := prepareEnv(tc.env) + if err != nil { + t.Error(err) + continue + } + if !slices.Equal(env, tc.wantEnv) { + t.Errorf("want %v, got %v", tc.wantEnv, env) + } + } +} diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index d14198772aa..c515df675e9 100644 --- a/libcontainer/setns_init_linux.go +++ b/libcontainer/setns_init_linux.go @@ -26,6 +26,7 @@ type linuxSetnsInit struct { config *initConfig logPipe *os.File dmzExe *os.File + addHome bool } func (l *linuxSetnsInit) getSessionRingName() string { @@ -101,7 +102,7 @@ func (l *linuxSetnsInit) Init() error { return err } } - if err := finalizeNamespace(l.config); err != nil { + if err := finalizeNamespace(l.config, l.addHome); err != nil { return err } if err := apparmor.ApplyProfile(l.config.AppArmorProfile); err != nil { @@ -143,7 +144,7 @@ func (l *linuxSetnsInit) Init() error { if l.dmzExe != nil { l.config.Args[0] = name - return system.Fexecve(l.dmzExe.Fd(), l.config.Args, os.Environ()) + return system.Fexecve(l.dmzExe.Fd(), l.config.Args, l.config.Env) } // Close all file descriptors we are not passing to the container. This is // necessary because the execve target could use internal runc fds as the @@ -163,5 +164,5 @@ func (l *linuxSetnsInit) Init() error { if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil { return err } - return system.Exec(name, l.config.Args, os.Environ()) + return system.Exec(name, l.config.Args, l.config.Env) } diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index ec2e814370a..edbb20e5b98 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -28,6 +28,7 @@ type linuxStandardInit struct { logPipe *os.File dmzExe *os.File config *initConfig + addHome bool } func (l *linuxStandardInit) getSessionRingParams() (string, uint32, uint32) { @@ -190,7 +191,7 @@ func (l *linuxStandardInit) Init() error { return err } } - if err := finalizeNamespace(l.config); err != nil { + if err := finalizeNamespace(l.config, l.addHome); err != nil { return err } // finalizeNamespace can change user/group which clears the parent death @@ -277,7 +278,7 @@ func (l *linuxStandardInit) Init() error { if l.dmzExe != nil { l.config.Args[0] = name - return system.Fexecve(l.dmzExe.Fd(), l.config.Args, os.Environ()) + return system.Fexecve(l.dmzExe.Fd(), l.config.Args, l.config.Env) } // Close all file descriptors we are not passing to the container. This is // necessary because the execve target could use internal runc fds as the @@ -297,5 +298,5 @@ func (l *linuxStandardInit) Init() error { if err := utils.UnsafeCloseFrom(l.config.PassedFilesCount + 3); err != nil { return err } - return system.Exec(name, l.config.Args, os.Environ()) + return system.Exec(name, l.config.Args, l.config.Env) }