From c0c6361cf08d818ffcf31f4b3278a6d62883dd39 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 12 Dec 2014 05:40:16 +1100 Subject: [PATCH] builder: internals: fix incomplete chown walk when fixing permissions This patch fixes the permission fixing code used by addContext, which would not be responsible for Lchown-ing top-level directories added to a destination that didn't exist prior to untar-ing the context. Signed-off-by: Aleksa Sarai (github: cyphar) Upstream-commit: 916cba9c587a3f3ce97b407993fecd96ac2fecaf Component: engine --- components/engine/builder/internals.go | 30 +++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/components/engine/builder/internals.go b/components/engine/builder/internals.go index 6935f4f1c8..c1fd617a56 100644 --- a/components/engine/builder/internals.go +++ b/components/engine/builder/internals.go @@ -615,7 +615,7 @@ func (b *Builder) addContext(container *daemon.Container, orig, dest string, dec } if fi.IsDir() { - return copyAsDirectory(origPath, destPath) + return copyAsDirectory(origPath, destPath, destExists) } // If we are adding a remote file (or we've been told not to decompress), do not try to untar it @@ -649,37 +649,43 @@ func (b *Builder) addContext(container *daemon.Container, orig, dest string, dec resPath = path.Join(destPath, path.Base(origPath)) } - return fixPermissions(origPath, resPath, 0, 0) + return fixPermissions(origPath, resPath, 0, 0, destExists) } -func copyAsDirectory(source, destination string) error { +func copyAsDirectory(source, destination string, destExisted bool) error { if err := chrootarchive.CopyWithTar(source, destination); err != nil { return err } - return fixPermissions(source, destination, 0, 0) + return fixPermissions(source, destination, 0, 0, destExisted) } -func fixPermissions(source, destination string, uid, gid int) error { - // The copied root permission should not be changed for previously existing - // directories. - s, err := os.Stat(destination) - if err != nil && !os.IsNotExist(err) { +func fixPermissions(source, destination string, uid, gid int, destExisted bool) error { + // If the destination didn't already exist, or the destination isn't a + // directory, then we should Lchown the destination. Otherwise, we shouldn't + // Lchown the destination. + destStat, err := os.Stat(destination) + if err != nil { + // This should *never* be reached, because the destination must've already + // been created while untar-ing the context. return err } - fixRootPermission := (err != nil) || !s.IsDir() + doChownDestination := !destExisted || !destStat.IsDir() // We Walk on the source rather than on the destination because we don't // want to change permissions on things we haven't created or modified. return filepath.Walk(source, func(fullpath string, info os.FileInfo, err error) error { - // Do not alter the walk root itself as it potentially existed before. - if !fixRootPermission && (source == fullpath) { + // Do not alter the walk root iff. it existed before, as it doesn't fall under + // the domain of "things we should chown". + if !doChownDestination && (source == fullpath) { return nil } + // Path is prefixed by source: substitute with destination instead. cleaned, err := filepath.Rel(source, fullpath) if err != nil { return err } + fullpath = path.Join(destination, cleaned) return os.Lchown(fullpath, uid, gid) })