From 60dcf63ce2eb18f64da960ee9299f5ad72a94036 Mon Sep 17 00:00:00 2001 From: John Akre Date: Tue, 25 Feb 2020 11:06:49 -0800 Subject: [PATCH] Preserve file ownership when using content chroots When using content chroots, file uid/gid values should be preserved when copying them to the full chroot. Fixes #727 Signed-off-by: John Akre --- bat/tests/content-chroot/run.bats | 45 ++++++++++++++++++++++++------- builder/bundles.go | 23 +++++++++++++++- helpers/helpers.go | 40 ++++++++++++++++++++++----- 3 files changed, 91 insertions(+), 17 deletions(-) diff --git a/bat/tests/content-chroot/run.bats b/bat/tests/content-chroot/run.bats index 7f85a9d..7195842 100755 --- a/bat/tests/content-chroot/run.bats +++ b/bat/tests/content-chroot/run.bats @@ -11,13 +11,20 @@ setup() { customDir="$BATS_TEST_DIRNAME"/custom-content fullChroot=update/image/10/full - # Create custom content chroot with directories, a file, and symlinks + # Create custom content chroot with directories, a file, and symlinks. + # Non-root ownership and non-default permissions are also tested. mkdir -p "$customDir"/usr/bin + sudo chown root:root "$customDir"/usr + sudo chown root:root "$customDir"/usr/bin mkdir -m 744 "$customDir"/dirPerm - touch "$customDir"/usr/bin/foo - chmod 744 "$customDir"/usr/bin/foo + chown 1000:1000 "$customDir"/dirPerm + sudo touch "$customDir"/usr/bin/foo + sudo chmod 744 "$customDir"/usr/bin/foo + sudo chown 1000:1000 "$customDir"/usr/bin/foo ln -s usr "$customDir"/dirLink + chown -h 1000:1000 "$customDir"/dirLink ln -s usr/bin/foo "$customDir"/fileLink + chown -h 1000:1000 "$customDir"/fileLink mixer-init-stripped-down "$CLRVER" 10 @@ -27,6 +34,11 @@ setup() { echo "content(custom-content)" >> "$LOCAL_BUNDLE_DIR"/bundle1 mixer-bundle-add "bundle1" + # Create bundle with identical content chroot + create-empty-local-bundle "bundle2" + echo "content(custom-content)" >> "$LOCAL_BUNDLE_DIR"/bundle2 + mixer-bundle-add "bundle2" + mixer-build-bundles > "$LOGDIR"/build_bundles.log mixer-build-update > "$LOGDIR"/build_update.log @@ -39,14 +51,22 @@ setup() { dirLink2=$(readlink $fullChroot/dirLink) [[ "$dirLink1" = "$dirLink2" ]] - # Check expected permissions copied to full chroot - filePerm1=$(stat -c '%A' $customDir/usr/bin/foo) - filePerm2=$(stat -c '%A' $fullChroot/usr/bin/foo) - [[ "$filePerm1" = "$filePerm2" ]] + # Check expected permissions and ownership copied to full chroot + fileLinkStat1=$(stat -c '%A:%U:%G' $customDir/fileLink) + fileLinkStat2=$(stat -c '%A:%U:%G' $fullChroot/fileLink) + [[ "$fileLinkStat1" = "$fileLinkStat2" ]] - dirPerm1=$(stat -c '%A' $customDir/dirPerm) - dirPerm2=$(stat -c '%A' $fullChroot/dirPerm) - [[ "$dirPerm1" = "$dirPerm2" ]] + dirLinkStat1=$(stat -c '%A:%U:%G' $customDir/dirLink) + dirLinkStat2=$(stat -c '%A:%U:%G' $fullChroot/dirLink) + [[ "$dirLinkStat1" = "$dirLinkStat2" ]] + + fileStat1=$(stat -c '%A:%U:%G' $customDir/usr/bin/foo) + fileStat2=$(stat -c '%A:%U:%G' $fullChroot/usr/bin/foo) + [[ "$fileStat1" = "$fileStat2" ]] + + dirStat1=$(stat -c '%A:%U:%G' $customDir/dirPerm) + dirStat2=$(stat -c '%A:%U:%G' $fullChroot/dirPerm) + [[ "$dirStat1" = "$dirStat2" ]] # Verify that manifest contains the content chroot files and the # bsdiff executable @@ -55,6 +75,11 @@ setup() { grep -P "\t10\t/usr/bin/bsdiff" update/www/10/Manifest.bundle1 grep -P "L...\t.*\t10\t/fileLink" update/www/10/Manifest.bundle1 grep -P "L...\t.*\t10\t/dirLink" update/www/10/Manifest.bundle1 + + grep -P "\t10\t/usr/bin/foo" update/www/10/Manifest.bundle2 + grep -P "\t10\t/dirPerm" update/www/10/Manifest.bundle2 + grep -P "L...\t.*\t10\t/fileLink" update/www/10/Manifest.bundle2 + grep -P "L...\t.*\t10\t/dirLink" update/www/10/Manifest.bundle2 } # vi: ft=sh ts=8 sw=2 sts=2 et tw=80 diff --git a/builder/bundles.go b/builder/bundles.go index 5f39d93..334c675 100644 --- a/builder/bundles.go +++ b/builder/bundles.go @@ -899,6 +899,19 @@ func addBundleContentChroots(set *bundleSet, fullDir string) error { if fullInfo.Mode().Perm() != fi.Mode().Perm() { return errors.Errorf("Directory permission mismatch: %s, %s", fullChrootFile, path) } + + srcDir, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return errors.Errorf("Cannot get directory ownership: %s", path) + } + targDir, ok := fullInfo.Sys().(*syscall.Stat_t) + if !ok { + return errors.Errorf("Cannot get directory ownership: %s", fullChrootFile) + } + if srcDir.Uid != targDir.Uid || srcDir.Gid != targDir.Gid { + return errors.Errorf("Directory ownership mismatch: %s, %s", fullChrootFile, path) + } + return nil } @@ -917,7 +930,15 @@ func addBundleContentChroots(set *bundleSet, fullDir string) error { } if fi.IsDir() { - return os.Mkdir(fullChrootFile, fi.Mode().Perm()) + if err = os.Mkdir(fullChrootFile, fi.Mode().Perm()); err != nil { + return err + } + + dirStat, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return errors.Errorf("Cannot get directory ownership: %s", path) + } + return os.Chown(fullChrootFile, int(dirStat.Uid), int(dirStat.Gid)) } // Do not resolve symlinks so that the links can be copied, do not diff --git a/helpers/helpers.go b/helpers/helpers.go index 594a947..ee42b05 100644 --- a/helpers/helpers.go +++ b/helpers/helpers.go @@ -35,6 +35,7 @@ import ( "path/filepath" "sort" "strings" + "syscall" "time" "github.com/pkg/errors" @@ -205,13 +206,13 @@ func CopyFileNoOverwrite(dest, src string) error { } // CopyFileWithOptions copies a file, overwriting the destination if it exist and allows -// options to be set for following links, syncing to disk, or preserving file permissions. -func CopyFileWithOptions(dest, src string, resolveLinks, sync, useSrcPerms bool) error { - return copyFileWithFlags(dest, src, os.O_RDWR|os.O_CREATE|os.O_TRUNC, resolveLinks, sync, useSrcPerms) +// options to be set for following links, syncing to disk, or preserving file permissions/ownership. +func CopyFileWithOptions(dest, src string, resolveLinks, sync, preserveSrc bool) error { + return copyFileWithFlags(dest, src, os.O_RDWR|os.O_CREATE|os.O_TRUNC, resolveLinks, sync, preserveSrc) } // copyFileWithFlags General purpose copy file function -func copyFileWithFlags(dest, src string, flags int, resolveLinks, sync, useSrcPerms bool) error { +func copyFileWithFlags(dest, src string, flags int, resolveLinks, sync, preserveSrc bool) error { srcInfo, err := os.Lstat(src) if err != nil { return err @@ -221,7 +222,22 @@ func copyFileWithFlags(dest, src string, flags int, resolveLinks, sync, useSrcPe if err != nil { return err } - return os.Symlink(srcLink, dest) + if err = os.Symlink(srcLink, dest); err != nil { + return err + } + + if preserveSrc { + srcStat, ok := srcInfo.Sys().(*syscall.Stat_t) + if !ok { + return errors.Errorf("Cannot get file ownership: %s", src) + } + + err = os.Lchown(dest, int(srcStat.Uid), int(srcStat.Gid)) + if err != nil { + return err + } + } + return nil } source, err := os.Open(src) @@ -233,7 +249,7 @@ func copyFileWithFlags(dest, src string, flags int, resolveLinks, sync, useSrcPe }() var perms os.FileMode - if useSrcPerms { + if preserveSrc { perms = srcInfo.Mode() } else { perms = 0666 @@ -252,6 +268,18 @@ func copyFileWithFlags(dest, src string, flags int, resolveLinks, sync, useSrcPe return err } + if preserveSrc { + srcStat, ok := srcInfo.Sys().(*syscall.Stat_t) + if !ok { + return errors.Errorf("Cannot get file ownership: %s", src) + } + + err = os.Chown(dest, int(srcStat.Uid), int(srcStat.Gid)) + if err != nil { + return err + } + } + if sync { err = destination.Sync() if err != nil {