Merge pull request #25617 from giuseppe/use-securejoin-openinroot

container: replace code with securejoin.OpenInRoot()
This commit is contained in:
openshift-merge-bot[bot]
2025-03-19 13:37:37 +00:00
committed by GitHub
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 // The caller is responsible for closing the file descriptor and unmounting the subpath
// when it's no longer needed. // when it's no longer needed.
func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountInfo, err error) { func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountInfo, err error) {
joinedPath := filepath.Clean(filepath.Join(mountPoint, subpath)) file, err := securejoin.OpenInRoot(mountPoint, subpath)
fd, err := unix.Open(joinedPath, unix.O_RDONLY|unix.O_PATH, 0)
if err != nil { if err != nil {
return nil, err 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 { if err != nil {
return nil, err return nil, err
} }
var npath string var npath string
switch { switch {
case fi.Mode()&fs.ModeSymlink != 0: 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(): case fi.IsDir():
npath, err = os.MkdirTemp(c.state.RunDir, "subpath") npath, err = os.MkdirTemp(c.state.RunDir, "subpath")
if err != nil { if err != nil {
@ -789,11 +770,12 @@ func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountIn
tmp.Close() tmp.Close()
npath = tmp.Name() 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 nil, err
} }
return &safeMountInfo{ return &safeMountInfo{
file: f, file: file,
mountPoint: npath, mountPoint: npath,
}, nil }, nil
} }

View File

@ -5545,7 +5545,7 @@ spec:
playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml}) playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml})
playKube.WaitWithDefaultTimeout() 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() { It("with unsafe hostPath subpaths", func() {
@ -5559,9 +5559,7 @@ spec:
err = generateKubeYaml("pod", pod, kubeYaml) err = generateKubeYaml("pod", pod, kubeYaml)
Expect(err).To(Not(HaveOccurred())) Expect(err).To(Not(HaveOccurred()))
playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml}) podmanTest.PodmanExitCleanly("kube", "play", kubeYaml)
playKube.WaitWithDefaultTimeout()
Expect(playKube).Should(ExitWithError(125, fmt.Sprintf(`subpath "testing/symlink" is outside of the volume "%s"`, hostPathLocation)))
}) })
It("with configMap subpaths", func() { It("with configMap subpaths", func() {