diff --git a/libcontainer/env.go b/libcontainer/env.go new file mode 100644 index 00000000000..36c5014f95d --- /dev/null +++ b/libcontainer/env.go @@ -0,0 +1,56 @@ +package libcontainer + +import ( + "errors" + "fmt" + "os" + "slices" + "strings" +) + +// prepareEnv checks supplied environment variables for validity, removes +// duplicates (leaving the last value only), and sets PATH from env, if found. +// Returns the deduplicated environment, and a flag telling if HOME is found. +func prepareEnv(env []string) ([]string, bool, error) { + // Clear the current environment (better be safe than sorry). + os.Clearenv() + + if env == nil { + return nil, false, nil + } + // Deduplication code based on dedupEnv from Go 1.22 os/exec. + + // Construct the output in reverse order, to preserve the + // last occurrence of each key. + out := make([]string, 0, len(env)) + saw := make(map[string]bool, len(env)) + for n := len(env); n > 0; n-- { + kv := env[n-1] + i := strings.IndexByte(kv, '=') + if i == -1 { + return nil, false, errors.New("invalid environment variable: missing '='") + } + if i == 0 { + return nil, false, errors.New("invalid environment variable: name cannot be empty") + } + key := kv[:i] + if saw[key] { // Duplicate. + continue + } + saw[key] = true + if strings.IndexByte(kv, 0) >= 0 { + return nil, false, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key) + } + if key == "PATH" { + // Needs to be set as it is used for binary lookup. + if err := os.Setenv("PATH", kv[5:]); err != nil { + return nil, false, err + } + } + out = append(out, kv) + } + // Restore the original order. + slices.Reverse(out) + + return out, saw["HOME"], nil +} diff --git a/libcontainer/env_test.go b/libcontainer/env_test.go new file mode 100644 index 00000000000..72d63b4af23 --- /dev/null +++ b/libcontainer/env_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/init_linux.go b/libcontainer/init_linux.go index e0a65d5cc8f..2e4cc521ddd 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -11,7 +11,6 @@ import ( "runtime" "runtime/debug" "strconv" - "strings" "syscall" "github.com/containerd/console" @@ -196,10 +195,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 +215,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 +234,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock config: config, logPipe: logPipe, dmzExe: dmzExe, + addHome: !homeSet, } return i.Init() case initStandard: @@ -249,37 +247,13 @@ 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 '='") - } - name, val := p[0], p[1] - if name == "" { - return 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) - } - if strings.IndexByte(val, 0) >= 0 { - return fmt.Errorf("invalid environment variable %q: value contains nul byte (\\x00)", name) - } - if err := os.Setenv(name, val); err != nil { - return err - } - } - return nil -} - // verifyCwd ensures that the current directory is actually inside the mount // namespace root of the current process. func verifyCwd() error { @@ -308,8 +282,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 +329,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 +447,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 +530,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/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) }