From 0e89e62feb6ed0af48589c1f9de8cd695e44f87d Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 6 Mar 2015 12:53:06 +1100 Subject: [PATCH 1/2] *: expose getResourcePath and getRootResourcePath wrappers Due to the importance of path safety, the internal sanitisation wrappers for volumes and containers should be exposed so other parts of Docker can benefit from proper path sanitisation. Signed-off-by: Aleksa Sarai (github: cyphar) Upstream-commit: 4377ebd6a758278c1766006c7eb8b777fa175719 Component: engine --- components/engine/daemon/container.go | 47 ++++++++++++++++++++------- components/engine/daemon/volumes.go | 10 +++--- components/engine/volumes/volume.go | 36 ++++++++++++++++---- 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/components/engine/daemon/container.go b/components/engine/daemon/container.go index bdfcbf4477..8eb35c9f52 100644 --- a/components/engine/daemon/container.go +++ b/components/engine/daemon/container.go @@ -211,12 +211,37 @@ func (container *Container) LogEvent(action string) { ) } -func (container *Container) getResourcePath(path string) (string, error) { +// Evaluates `path` in the scope of the container's basefs, with proper path +// sanitisation. Symlinks are all scoped to the basefs of the container, as +// though the container's basefs was `/`. +// +// The basefs of a container is the host-facing path which is bind-mounted as +// `/` inside the container. This method is essentially used to access a +// particular path inside the container as though you were a process in that +// container. +// +// NOTE: The returned path is *only* safely scoped inside the container's basefs +// if no component of the returned path changes (such as a component +// symlinking to a different path) between using this method and using the +// path. See symlink.FollowSymlinkInScope for more details. +func (container *Container) GetResourcePath(path string) (string, error) { cleanPath := filepath.Join("/", path) return symlink.FollowSymlinkInScope(filepath.Join(container.basefs, cleanPath), container.basefs) } -func (container *Container) getRootResourcePath(path string) (string, error) { +// Evaluates `path` in the scope of the container's root, with proper path +// sanitisation. Symlinks are all scoped to the root of the container, as +// though the container's root was `/`. +// +// The root of a container is the host-facing configuration metadata directory. +// Only use this method to safely access the container's `container.json` or +// other metadata files. If in doubt, use container.GetResourcePath. +// +// NOTE: The returned path is *only* safely scoped inside the container's root +// if no component of the returned path changes (such as a component +// symlinking to a different path) between using this method and using the +// path. See symlink.FollowSymlinkInScope for more details. +func (container *Container) GetRootResourcePath(path string) (string, error) { cleanPath := filepath.Join("/", path) return symlink.FollowSymlinkInScope(filepath.Join(container.root, cleanPath), container.root) } @@ -515,7 +540,7 @@ func (streamConfig *StreamConfig) StderrLogPipe() io.ReadCloser { } func (container *Container) buildHostnameFile() error { - hostnamePath, err := container.getRootResourcePath("hostname") + hostnamePath, err := container.GetRootResourcePath("hostname") if err != nil { return err } @@ -529,7 +554,7 @@ func (container *Container) buildHostnameFile() error { func (container *Container) buildHostsFiles(IP string) error { - hostsPath, err := container.getRootResourcePath("hosts") + hostsPath, err := container.GetRootResourcePath("hosts") if err != nil { return err } @@ -895,7 +920,7 @@ func (container *Container) Unmount() error { } func (container *Container) logPath(name string) (string, error) { - return container.getRootResourcePath(fmt.Sprintf("%s-%s.log", container.ID, name)) + return container.GetRootResourcePath(fmt.Sprintf("%s-%s.log", container.ID, name)) } func (container *Container) ReadLog(name string) (io.Reader, error) { @@ -907,11 +932,11 @@ func (container *Container) ReadLog(name string) (io.Reader, error) { } func (container *Container) hostConfigPath() (string, error) { - return container.getRootResourcePath("hostconfig.json") + return container.GetRootResourcePath("hostconfig.json") } func (container *Container) jsonPath() (string, error) { - return container.getRootResourcePath("config.json") + return container.GetRootResourcePath("config.json") } // This method must be exported to be used from the lxc template @@ -981,7 +1006,7 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) { } }() - basePath, err := container.getResourcePath(resource) + basePath, err := container.GetResourcePath(resource) if err != nil { return nil, err } @@ -1083,7 +1108,7 @@ func (container *Container) setupContainerDns() error { if err != nil { return err } - container.ResolvConfPath, err = container.getRootResourcePath("resolv.conf") + container.ResolvConfPath, err = container.GetRootResourcePath("resolv.conf") if err != nil { return err } @@ -1244,7 +1269,7 @@ func (container *Container) initializeNetworking() error { return err } - hostsPath, err := container.getRootResourcePath("hosts") + hostsPath, err := container.GetRootResourcePath("hosts") if err != nil { return err } @@ -1375,7 +1400,7 @@ func (container *Container) setupWorkingDirectory() error { if container.Config.WorkingDir != "" { container.Config.WorkingDir = path.Clean(container.Config.WorkingDir) - pth, err := container.getResourcePath(container.Config.WorkingDir) + pth, err := container.GetResourcePath(container.Config.WorkingDir) if err != nil { return err } diff --git a/components/engine/daemon/volumes.go b/components/engine/daemon/volumes.go index 4d15023ba7..79d83504a2 100644 --- a/components/engine/daemon/volumes.go +++ b/components/engine/daemon/volumes.go @@ -47,7 +47,7 @@ func (container *Container) createVolumes() error { continue } - realPath, err := container.getResourcePath(path) + realPath, err := container.GetResourcePath(path) if err != nil { return err } @@ -336,7 +336,7 @@ func (container *Container) mountVolumes() error { return fmt.Errorf("could not find volume for %s:%s, impossible to mount", source, dest) } - destPath, err := container.getResourcePath(dest) + destPath, err := container.GetResourcePath(dest) if err != nil { return err } @@ -347,7 +347,7 @@ func (container *Container) mountVolumes() error { } for _, mnt := range container.specialMounts() { - destPath, err := container.getResourcePath(mnt.Destination) + destPath, err := container.GetResourcePath(mnt.Destination) if err != nil { return err } @@ -360,7 +360,7 @@ func (container *Container) mountVolumes() error { func (container *Container) unmountVolumes() { for dest := range container.Volumes { - destPath, err := container.getResourcePath(dest) + destPath, err := container.GetResourcePath(dest) if err != nil { logrus.Errorf("error while unmounting volumes %s: %v", destPath, err) continue @@ -372,7 +372,7 @@ func (container *Container) unmountVolumes() { } for _, mnt := range container.specialMounts() { - destPath, err := container.getResourcePath(mnt.Destination) + destPath, err := container.GetResourcePath(mnt.Destination) if err != nil { logrus.Errorf("error while unmounting volumes %s: %v", destPath, err) continue diff --git a/components/engine/volumes/volume.go b/components/engine/volumes/volume.go index 87aa4ad25a..283bc9bca8 100644 --- a/components/engine/volumes/volume.go +++ b/components/engine/volumes/volume.go @@ -114,14 +114,38 @@ func (v *Volume) FromDisk() error { } func (v *Volume) jsonPath() (string, error) { - return v.getRootResourcePath("config.json") -} -func (v *Volume) getRootResourcePath(path string) (string, error) { - cleanPath := filepath.Join("/", path) - return symlink.FollowSymlinkInScope(filepath.Join(v.configPath, cleanPath), v.configPath) + return v.GetRootResourcePath("config.json") } -func (v *Volume) getResourcePath(path string) (string, error) { +// Evalutes `path` in the scope of the volume's root path, with proper path +// sanitisation. Symlinks are all scoped to the root of the volume, as +// though the volume's root was `/`. +// +// The volume's root path is the host-facing path of the root of the volume's +// mountpoint inside a container. +// +// NOTE: The returned path is *only* safely scoped inside the volume's root +// if no component of the returned path changes (such as a component +// symlinking to a different path) between using this method and using the +// path. See symlink.FollowSymlinkInScope for more details. +func (v *Volume) GetResourcePath(path string) (string, error) { cleanPath := filepath.Join("/", path) return symlink.FollowSymlinkInScope(filepath.Join(v.Path, cleanPath), v.Path) } + +// Evalutes `path` in the scope of the volume's config path, with proper path +// sanitisation. Symlinks are all scoped to the root of the config path, as +// though the config path was `/`. +// +// The config path of a volume is not exposed to the container and is just used +// to store volume configuration options and other internal information. If in +// doubt, you probably want to just use v.GetResourcePath. +// +// NOTE: The returned path is *only* safely scoped inside the volume's config +// path if no component of the returned path changes (such as a component +// symlinking to a different path) between using this method and using the +// path. See symlink.FollowSymlinkInScope for more details. +func (v *Volume) GetRootResourcePath(path string) (string, error) { + cleanPath := filepath.Join("/", path) + return symlink.FollowSymlinkInScope(filepath.Join(v.configPath, cleanPath), v.configPath) +} From f124bc620f729dc82f7d6ba30220bfbbd4c3b2f5 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 20 Mar 2015 19:59:15 +1100 Subject: [PATCH 2/2] *: switch to Get(Root)?ResourcePath where appropriate Several parts of the codebase didn't use the correct path sanitisation wrappers. Now that the wrappers have been exposed, use those. Signed-off-by: Aleksa Sarai (github: cyphar) Upstream-commit: b7c3c0cb6988c9a7864649bfe458217336c5fc51 Component: engine --- components/engine/builder/internals.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/components/engine/builder/internals.go b/components/engine/builder/internals.go index ba7d45bcb1..2fe7ca8bad 100644 --- a/components/engine/builder/internals.go +++ b/components/engine/builder/internals.go @@ -32,7 +32,6 @@ import ( "github.com/docker/docker/pkg/parsers" "github.com/docker/docker/pkg/progressreader" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/docker/docker/pkg/tarsum" "github.com/docker/docker/pkg/urlutil" @@ -646,14 +645,12 @@ func (b *Builder) addContext(container *daemon.Container, orig, dest string, dec err error destExists = true origPath = path.Join(b.contextPath, orig) - destPath = path.Join(container.RootfsPath(), dest) + destPath string ) - if destPath != container.RootfsPath() { - destPath, err = symlink.FollowSymlinkInScope(destPath, container.RootfsPath()) - if err != nil { - return err - } + destPath, err = container.GetResourcePath(dest) + if err != nil { + return err } // Preserve the trailing '/'