From 7e3fb502a80a005962306302878ec525af2c43f7 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 May 2018 12:56:25 +0200 Subject: [PATCH] Change SafeMakeDir to resolve symlinks in mounter implementation Kubelet should not resolve symlinks outside of mounter interface. Only mounter interface knows, how to resolve them properly on the host. As consequence, declaration of SafeMakeDir changes to simplify the implementation: from SafeMakeDir(fullPath string, base string, perm os.FileMode) to SafeMakeDir(subdirectoryInBase string, base string, perm os.FileMode) --- pkg/kubelet/kubelet_pods.go | 13 +++++-------- pkg/util/mount/mount.go | 16 +++++++++------- pkg/util/mount/mount_linux.go | 15 ++++++++++++--- pkg/util/mount/mount_windows.go | 10 ++++++++-- pkg/util/mount/nsenter_mount.go | 17 +++++++++++++++-- 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index c1d2c54f090..55dfce9a589 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -174,10 +174,7 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h return nil, cleanupAction, fmt.Errorf("unable to provision SubPath `%s`: %v", mount.SubPath, err) } - volumePath, err := filepath.EvalSymlinks(hostPath) - if err != nil { - return nil, cleanupAction, err - } + volumePath := hostPath hostPath = filepath.Join(volumePath, mount.SubPath) if subPathExists, err := mounter.ExistsPath(hostPath); err != nil { @@ -192,10 +189,10 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h if err != nil { return nil, cleanupAction, err } - - if err := mounter.SafeMakeDir(hostPath, volumePath, perm); err != nil { - glog.Errorf("failed to mkdir %q: %v", hostPath, err) - return nil, cleanupAction, err + if err := mounter.SafeMakeDir(mount.SubPath, volumePath, perm); err != nil { + // Don't pass detailed error back to the user because it could give information about host filesystem + glog.Errorf("failed to create subPath directory for volumeMount %q of container %q: %v", mount.Name, container.Name, err) + return nil, cleanupAction, fmt.Errorf("failed to create subPath directory for volumeMount %q of container %q", mount.Name, container.Name) } } hostPath, cleanupAction, err = mounter.PrepareSafeSubpath(mountutil.Subpath{ diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index 06ee97e5d27..49351394dd8 100644 --- a/pkg/util/mount/mount.go +++ b/pkg/util/mount/mount.go @@ -84,13 +84,15 @@ type Interface interface { // MakeDir creates a new directory. // Will operate in the host mount namespace if kubelet is running in a container MakeDir(pathname string) error - // SafeMakeDir makes sure that the created directory does not escape given - // base directory mis-using symlinks. The directory is created in the same - // mount namespace as where kubelet is running. Note that the function makes - // sure that it creates the directory somewhere under the base, nothing - // else. E.g. if the directory already exists, it may exists outside of the - // base due to symlinks. - SafeMakeDir(pathname string, base string, perm os.FileMode) error + // SafeMakeDir creates subdir within given base. It makes sure that the + // created directory does not escape given base directory mis-using + // symlinks. Note that the function makes sure that it creates the directory + // somewhere under the base, nothing else. E.g. if the directory already + // exists, it may exist outside of the base due to symlinks. + // This method should be used if the directory to create is inside volume + // that's under user control. User must not be able to use symlinks to + // escape the volume to create directories somewhere else. + SafeMakeDir(subdir string, base string, perm os.FileMode) error // Will operate in the host mount namespace if kubelet is running in a container. // Error is returned on any other error than "file not found". ExistsPath(pathname string) (bool, error) diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index a72774ab415..9998792893f 100644 --- a/pkg/util/mount/mount_linux.go +++ b/pkg/util/mount/mount_linux.go @@ -955,8 +955,15 @@ func removeEmptyDirs(baseDir, endDir string) error { return nil } -func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { - return doSafeMakeDir(pathname, base, perm) +func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error { + realBase, err := filepath.EvalSymlinks(base) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", base, err) + } + + realFullPath := filepath.Join(realBase, subdir) + + return doSafeMakeDir(realFullPath, realBase, perm) } func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { @@ -1001,7 +1008,9 @@ func getMode(pathname string) (os.FileMode, error) { return info.Mode(), nil } -// This implementation is shared between Linux and NsEnterMounter +// This implementation is shared between Linux and NsEnterMounter. Both pathname +// and base must be either already resolved symlinks or thet will be resolved in +// kubelet's mount namespace (in case it runs containerized). func doSafeMakeDir(pathname string, base string, perm os.FileMode) error { glog.V(4).Infof("Creating directory %q within base %q", pathname, base) diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index 3079b8c65de..64d39a59f7e 100644 --- a/pkg/util/mount/mount_windows.go +++ b/pkg/util/mount/mount_windows.go @@ -472,8 +472,14 @@ func (mounter *Mounter) GetMode(pathname string) (os.FileMode, error) { } // SafeMakeDir makes sure that the created directory does not escape given base directory mis-using symlinks. -func (mounter *Mounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { - return doSafeMakeDir(pathname, base, perm) +func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error { + realBase, err := filepath.EvalSymlinks(base) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", base, err) + } + + realFullPath := filepath.Join(realBase, subdir) + return doSafeMakeDir(realFullPath, realBase, perm) } func doSafeMakeDir(pathname string, base string, perm os.FileMode) error { diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index 4a920d63b51..dfc690a64bc 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/pkg/util/mount/nsenter_mount.go @@ -327,8 +327,21 @@ func (mounter *NsenterMounter) PrepareSafeSubpath(subPath Subpath) (newHostPath return newHostPath, cleanupAction, err } -func (mounter *NsenterMounter) SafeMakeDir(pathname string, base string, perm os.FileMode) error { - return doSafeMakeDir(pathname, base, perm) +func (mounter *NsenterMounter) SafeMakeDir(subdir string, base string, perm os.FileMode) error { + fullSubdirPath := filepath.Join(base, subdir) + evaluatedSubdirPath, err := mounter.ne.EvalSymlinks(fullSubdirPath, false /* mustExist */) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", fullSubdirPath, err) + } + kubeletSubdirPath := mounter.ne.KubeletPath(evaluatedSubdirPath) + + evaluatedBase, err := mounter.ne.EvalSymlinks(base, true /* mustExist */) + if err != nil { + return fmt.Errorf("error resolving symlinks in %s: %s", base, err) + } + kubeletBase := mounter.ne.KubeletPath(evaluatedBase) + + return doSafeMakeDir(kubeletSubdirPath, kubeletBase, perm) } func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) {