From b0948a5cd098582a8050abe12191f05bfaace132 Mon Sep 17 00:00:00 2001 From: Uzinn Kagurazaka Date: Tue, 6 Aug 2024 01:01:43 +0800 Subject: [PATCH] Quadlet: fix filters failure when the search paths are symlinks Rootless units placed in `users` would be loaded for root when `/etc/containers/systemd` is a symlink. In this case, since `UnitDirAdmin` is hardcoded, `userLevelFilter` always returns `true`. If `/etc/containers/systemd/users` is a symlink, any user would load other users' units. Fix the above two problems. Fixes: #23483 Signed-off-by: Uzinn Kagurazaka --- cmd/quadlet/main.go | 26 ++++-- cmd/quadlet/main_test.go | 167 ++++++++++++++++++++++++++++----------- 2 files changed, 140 insertions(+), 53 deletions(-) diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index fd5a792308..4ad7c8f8b2 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -34,10 +34,6 @@ var ( versionFlag bool // True if -version is used ) -const ( - SystemUserDirLevel = 5 -) - var ( // data saved between logToKmsg calls noKmsg = false @@ -59,6 +55,12 @@ var ( } ) +var ( + unitDirAdminUser string + resolvedUnitDirAdminUser string + systemUserDirLevel int +) + // We log directly to /dev/kmsg, because that is the only way to get information out // of the generator into the system logs. func logToKmsg(s string) bool { @@ -115,6 +117,14 @@ func getUnitDirs(rootless bool) []string { unitDirsEnv := os.Getenv("QUADLET_UNIT_DIRS") dirs := make([]string, 0) + unitDirAdminUser = filepath.Join(quadlet.UnitDirAdmin, "users") + var err error + if resolvedUnitDirAdminUser, err = filepath.EvalSymlinks(unitDirAdminUser); err != nil { + Debugf("Error occurred resolving path %q: %s", unitDirAdminUser, err) + resolvedUnitDirAdminUser = unitDirAdminUser + } + systemUserDirLevel = len(strings.Split(resolvedUnitDirAdminUser, string(os.PathSeparator))) + if len(unitDirsEnv) > 0 { for _, eachUnitDir := range strings.Split(unitDirsEnv, ":") { if !filepath.IsAbs(eachUnitDir) { @@ -185,10 +195,10 @@ func appendSubPaths(dirs []string, path string, isUserFlag bool, filterPtr func( func nonNumericFilter(_path string, isUserFlag bool) bool { // when running in rootless, recursive walk directories that are non numeric // ignore sub dirs under the `users` directory which correspond to a user id - if strings.Contains(_path, filepath.Join(quadlet.UnitDirAdmin, "users")) { + if strings.HasPrefix(_path, resolvedUnitDirAdminUser) { listDirUserPathLevels := strings.Split(_path, string(os.PathSeparator)) - if len(listDirUserPathLevels) > SystemUserDirLevel { - if !(regexp.MustCompile(`^[0-9]*$`).MatchString(listDirUserPathLevels[SystemUserDirLevel])) { + if len(listDirUserPathLevels) > systemUserDirLevel { + if !(regexp.MustCompile(`^[0-9]*$`).MatchString(listDirUserPathLevels[systemUserDirLevel])) { return true } } @@ -201,7 +211,7 @@ func nonNumericFilter(_path string, isUserFlag bool) bool { func userLevelFilter(_path string, isUserFlag bool) bool { // if quadlet generator is run rootless, do not recurse other user sub dirs // if quadlet generator is run as root, ignore users sub dirs - if strings.Contains(_path, filepath.Join(quadlet.UnitDirAdmin, "users")) { + if strings.HasPrefix(_path, resolvedUnitDirAdminUser) { if isUserFlag { return true } diff --git a/cmd/quadlet/main_test.go b/cmd/quadlet/main_test.go index 15704d8344..6ee7bebf4e 100644 --- a/cmd/quadlet/main_test.go +++ b/cmd/quadlet/main_test.go @@ -1,10 +1,16 @@ +//go:build linux + package main import ( + "fmt" "os" + "os/exec" "os/user" "path" "path/filepath" + "strconv" + "syscall" "testing" "github.com/containers/podman/v5/pkg/systemd/quadlet" @@ -47,59 +53,130 @@ func TestIsUnambiguousName(t *testing.T) { } func TestUnitDirs(t *testing.T) { - rootDirs := []string{} - rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirTemp, false, userLevelFilter) - rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirAdmin, false, userLevelFilter) - rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirDistro, false, userLevelFilter) - unitDirs := getUnitDirs(false) - assert.Equal(t, unitDirs, rootDirs, "rootful unit dirs should match") - - configDir, err := os.UserConfigDir() - assert.Nil(t, err) u, err := user.Current() assert.Nil(t, err) + uidInt, err := strconv.Atoi(u.Uid) + assert.Nil(t, err) - rootlessDirs := []string{} + if os.Getenv("_UNSHARED") != "true" { + unitDirs := getUnitDirs(false) + rootDirs := []string{} + rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirTemp, false, userLevelFilter) + rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirAdmin, false, userLevelFilter) + rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirDistro, false, userLevelFilter) + assert.Equal(t, unitDirs, rootDirs, "rootful unit dirs should match") - runtimeDir, found := os.LookupEnv("XDG_RUNTIME_DIR") - if found { - rootlessDirs = appendSubPaths(rootlessDirs, path.Join(runtimeDir, "containers/systemd"), false, nil) - } - rootlessDirs = appendSubPaths(rootlessDirs, path.Join(configDir, "containers/systemd"), false, nil) - rootlessDirs = appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) - rootlessDirs = appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) - rootlessDirs = append(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users")) + configDir, err := os.UserConfigDir() + assert.Nil(t, err) - unitDirs = getUnitDirs(true) - assert.Equal(t, unitDirs, rootlessDirs, "rootless unit dirs should match") + rootlessDirs := []string{} - name, err := os.MkdirTemp("", "dir") - assert.Nil(t, err) - // remove the temporary directory at the end of the program - defer os.RemoveAll(name) + runtimeDir, found := os.LookupEnv("XDG_RUNTIME_DIR") + if found { + rootlessDirs = appendSubPaths(rootlessDirs, path.Join(runtimeDir, "containers/systemd"), false, nil) + } + rootlessDirs = appendSubPaths(rootlessDirs, path.Join(configDir, "containers/systemd"), false, nil) + rootlessDirs = appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) + rootlessDirs = appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) + rootlessDirs = append(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users")) - t.Setenv("QUADLET_UNIT_DIRS", name) - unitDirs = getUnitDirs(false) - assert.Equal(t, unitDirs, []string{name}, "rootful should use environment variable") + unitDirs = getUnitDirs(true) + assert.Equal(t, unitDirs, rootlessDirs, "rootless unit dirs should match") - unitDirs = getUnitDirs(true) - assert.Equal(t, unitDirs, []string{name}, "rootless should use environment variable") + name, err := os.MkdirTemp("", "dir") + assert.Nil(t, err) + // remove the temporary directory at the end of the program + defer os.RemoveAll(name) - symLinkTestBaseDir, err := os.MkdirTemp("", "podman-symlinktest") - assert.Nil(t, err) - // remove the temporary directory at the end of the program - defer os.RemoveAll(symLinkTestBaseDir) + t.Setenv("QUADLET_UNIT_DIRS", name) + unitDirs = getUnitDirs(false) + assert.Equal(t, unitDirs, []string{name}, "rootful should use environment variable") - actualDir := filepath.Join(symLinkTestBaseDir, "actual") - err = os.Mkdir(actualDir, 0755) - assert.Nil(t, err) - innerDir := filepath.Join(actualDir, "inner") - err = os.Mkdir(innerDir, 0755) - assert.Nil(t, err) - symlink := filepath.Join(symLinkTestBaseDir, "symlink") - err = os.Symlink(actualDir, symlink) - assert.Nil(t, err) - t.Setenv("QUADLET_UNIT_DIRS", actualDir) - unitDirs = getUnitDirs(true) - assert.Equal(t, unitDirs, []string{actualDir, innerDir}, "directory resolution should follow symlink") + unitDirs = getUnitDirs(true) + assert.Equal(t, unitDirs, []string{name}, "rootless should use environment variable") + + symLinkTestBaseDir, err := os.MkdirTemp("", "podman-symlinktest") + assert.Nil(t, err) + // remove the temporary directory at the end of the program + defer os.RemoveAll(symLinkTestBaseDir) + + actualDir := filepath.Join(symLinkTestBaseDir, "actual") + err = os.Mkdir(actualDir, 0755) + assert.Nil(t, err) + innerDir := filepath.Join(actualDir, "inner") + err = os.Mkdir(innerDir, 0755) + assert.Nil(t, err) + symlink := filepath.Join(symLinkTestBaseDir, "symlink") + err = os.Symlink(actualDir, symlink) + assert.Nil(t, err) + t.Setenv("QUADLET_UNIT_DIRS", symlink) + unitDirs = getUnitDirs(true) + assert.Equal(t, unitDirs, []string{actualDir, innerDir}, "directory resolution should follow symlink") + + // because chroot is only available for root, + // unshare the namespace and map user to root + c := exec.Command("/proc/self/exe", os.Args[1:]...) + c.Stdin = os.Stdin + c.Stdout = os.Stdout + c.Stderr = os.Stderr + c.SysProcAttr = &syscall.SysProcAttr{ + Cloneflags: syscall.CLONE_NEWUSER, + UidMappings: []syscall.SysProcIDMap{ + { + ContainerID: 0, + HostID: uidInt, + Size: 1, + }, + }, + } + c.Env = append(os.Environ(), "_UNSHARED=true") + err = c.Run() + assert.Nil(t, err) + } else { + fmt.Println(os.Args) + + symLinkTestBaseDir, err := os.MkdirTemp("", "podman-symlinktest2") + assert.Nil(t, err) + defer os.RemoveAll(symLinkTestBaseDir) + rootF, err := os.Open("/") + assert.Nil(t, err) + defer rootF.Close() + defer func() { + err := rootF.Chdir() + assert.Nil(t, err) + err = syscall.Chroot(".") + assert.Nil(t, err) + }() + err = syscall.Chroot(symLinkTestBaseDir) + assert.Nil(t, err) + + err = os.MkdirAll(quadlet.UnitDirAdmin, 0755) + assert.Nil(t, err) + err = os.RemoveAll(quadlet.UnitDirAdmin) + assert.Nil(t, err) + + systemdDir := filepath.Join("/", "systemd") + userDir := filepath.Join("/", "users") + err = os.Mkdir(systemdDir, 0755) + assert.Nil(t, err) + err = os.Mkdir(userDir, 0755) + assert.Nil(t, err) + err = os.Symlink(userDir, filepath.Join(systemdDir, "users")) + assert.Nil(t, err) + err = os.Symlink(systemdDir, quadlet.UnitDirAdmin) + assert.Nil(t, err) + + uidDir := filepath.Join(userDir, u.Uid) + err = os.Mkdir(uidDir, 0755) + assert.Nil(t, err) + uidDir2 := filepath.Join(userDir, strconv.Itoa(uidInt+1)) + err = os.Mkdir(uidDir2, 0755) + assert.Nil(t, err) + + t.Setenv("QUADLET_UNIT_DIRS", "") + unitDirs := getUnitDirs(false) + assert.NotContains(t, unitDirs, userDir, "rootful should not contain rootless") + unitDirs = getUnitDirs(true) + assert.NotContains(t, unitDirs, uidDir2, "rootless should not contain other users'") + } }