container: replace code with securejoin.OpenInRoot()

when the code was first added, there was no securejoin.OpenInRoot().
Since there is a function already provided by a dependency and already
used in libpod, replace the custom code with securejoin.OpenInRoot().

The new version does not report a symlink that points outside the
root, but it is still resolved relative to the specified mountpoint,
since that is the openat2 semantic.  It does not affect the security
of the function.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This commit is contained in:
Giuseppe Scrivano
2025-03-18 12:18:12 +01:00
parent 98de6f3c10
commit c0627de21d
2 changed files with 11 additions and 31 deletions

View File

@ -741,41 +741,22 @@ func (s *safeMountInfo) Close() {
// The caller is responsible for closing the file descriptor and unmounting the subpath
// when it's no longer needed.
func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountInfo, err error) {
joinedPath := filepath.Clean(filepath.Join(mountPoint, subpath))
fd, err := unix.Open(joinedPath, unix.O_RDONLY|unix.O_PATH, 0)
file, err := securejoin.OpenInRoot(mountPoint, subpath)
if err != nil {
return nil, err
}
f := os.NewFile(uintptr(fd), joinedPath)
defer func() {
if err != nil {
f.Close()
}
}()
// Once we got the file descriptor, we need to check that the subpath is a valid. We
// refer to the open FD so there won't be other path lookups (and no risk to follow a symlink).
fdPath := fmt.Sprintf("/proc/%d/fd/%d", os.Getpid(), f.Fd())
p, err := os.Readlink(fdPath)
if err != nil {
return nil, err
}
relPath, err := filepath.Rel(mountPoint, p)
if err != nil {
return nil, err
}
if relPath == ".." || strings.HasPrefix(relPath, "../") {
return nil, fmt.Errorf("subpath %q is outside of the volume %q", subpath, mountPoint)
}
fi, err := os.Stat(fdPath)
// we need to always reference the file by its fd, that points inside the mountpoint.
fname := fmt.Sprintf("/proc/self/fd/%d", int(file.Fd()))
fi, err := os.Stat(fname)
if err != nil {
return nil, err
}
var npath string
switch {
case fi.Mode()&fs.ModeSymlink != 0:
return nil, fmt.Errorf("file %q is a symlink", joinedPath)
return nil, fmt.Errorf("file %q is a symlink", filepath.Join(mountPoint, subpath))
case fi.IsDir():
npath, err = os.MkdirTemp(c.state.RunDir, "subpath")
if err != nil {
@ -789,11 +770,12 @@ func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountIn
tmp.Close()
npath = tmp.Name()
}
if err := unix.Mount(fdPath, npath, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
if err := unix.Mount(fname, npath, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
return nil, err
}
return &safeMountInfo{
file: f,
file: file,
mountPoint: npath,
}, nil
}

View File

@ -5545,7 +5545,7 @@ spec:
playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml})
playKube.WaitWithDefaultTimeout()
Expect(playKube).Should(ExitWithError(125, fmt.Sprintf(`subpath "testing/onlythis" is outside of the volume "%s/root/volumes/testvol/_data`, podmanTest.TempDir)))
Expect(playKube).Should(ExitWithError(125, fmt.Sprintf("securejoin.OpenInRoot testing/onlythis: openat2 %s/root/volumes/testvol/_data/testing/onlythis: no such file or directory", podmanTest.TempDir)))
})
It("with unsafe hostPath subpaths", func() {
@ -5559,9 +5559,7 @@ spec:
err = generateKubeYaml("pod", pod, kubeYaml)
Expect(err).To(Not(HaveOccurred()))
playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml})
playKube.WaitWithDefaultTimeout()
Expect(playKube).Should(ExitWithError(125, fmt.Sprintf(`subpath "testing/symlink" is outside of the volume "%s"`, hostPathLocation)))
podmanTest.PodmanExitCleanly("kube", "play", kubeYaml)
})
It("with configMap subpaths", func() {