From d8ae5618afb9b5f91d049065b67bc368610a3457 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 17 Aug 2018 14:40:12 -0400 Subject: [PATCH] Get absolute path of file before checking whitelist Issue 291 pointed out that symlink "../proc/self/mounts" in the fedora image wasn't being extracted properly and kaniko was erroring out. This is because the file path wasn't absolute so kaniko wasn't recognizing it as a whitelisted path. With this change, we first resolve a path to it's absolute path before checking the whitelist. --- .../dockerfiles/Dockerfile_test_multistage | 4 ++ pkg/snapshot/snapshot.go | 12 ++++- pkg/util/fs_util.go | 44 ++++++++++++++----- pkg/util/fs_util_test.go | 6 ++- 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_multistage b/integration/dockerfiles/Dockerfile_test_multistage index cf2c5804c9..25246c815d 100644 --- a/integration/dockerfiles/Dockerfile_test_multistage +++ b/integration/dockerfiles/Dockerfile_test_multistage @@ -8,6 +8,10 @@ COPY --from=0 $foopath context/b* /foo/ FROM second COPY --from=base /context/foo /new/foo +# This base image contains symlinks with relative paths to whitelisted directories +# We need to test they're extracted correctly +FROM fedora@sha256:c4cc32b09c6ae3f1353e7e33a8dda93dc41676b923d6d89afa996b421cc5aa48 + FROM base ARG file COPY --from=second /foo ${file} diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 2ba85f18b4..f0a88cbe16 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -95,7 +95,11 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { if val, ok := snapshottedFiles[file]; ok && val { continue } - if util.CheckWhitelist(file) && !isBuildFile(file) { + whitelisted, err := util.CheckWhitelist(file) + if err != nil { + return false, err + } + if whitelisted && !isBuildFile(file) { logrus.Infof("Not adding %s to layer, as it's whitelisted", file) continue } @@ -168,7 +172,11 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { // Now create the tar. for path, info := range memFs { - if util.CheckWhitelist(path) { + whitelisted, err := util.CheckWhitelist(path) + if err != nil { + return false, err + } + if whitelisted { logrus.Debugf("Not adding %s to layer, as it's whitelisted", path) continue } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index e06d11be19..1a4f958bb9 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -90,13 +90,20 @@ func GetFSFromImage(root string, img v1.Image) error { logrus.Infof("Not adding %s because it was added by a prior layer", path) continue } - - if CheckWhitelist(path) && !checkWhitelistRoot(root) { + whitelisted, err := CheckWhitelist(path) + if err != nil { + return err + } + if whitelisted && !checkWhitelistRoot(root) { logrus.Infof("Not adding %s because it is whitelisted", path) continue } if hdr.Typeflag == tar.TypeSymlink { - if CheckWhitelist(hdr.Linkname) { + whitelisted, err := CheckWhitelist(hdr.Linkname) + if err != nil { + return err + } + if whitelisted { logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname) continue } @@ -115,7 +122,11 @@ func GetFSFromImage(root string, img v1.Image) error { func DeleteFilesystem() error { logrus.Info("Deleting filesystem...") err := filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, err error) error { - if CheckWhitelist(path) || ChildDirInWhitelist(path, constants.RootDir) { + whitelisted, err := CheckWhitelist(path) + if err != nil { + return err + } + if whitelisted || ChildDirInWhitelist(path, constants.RootDir) { logrus.Debugf("Not deleting %s, as it's whitelisted", path) return nil } @@ -247,13 +258,18 @@ func checkWhiteouts(path string, whiteouts map[string]struct{}) bool { return false } -func CheckWhitelist(path string) bool { +func CheckWhitelist(path string) (bool, error) { + abs, err := filepath.Abs(path) + if err != nil { + logrus.Infof("unable to get absolute path for %s", path) + return false, err + } for _, wl := range whitelist { - if HasFilepathPrefix(path, wl) { - return true + if HasFilepathPrefix(abs, wl) { + return true, nil } } - return false + return false, nil } func checkWhitelistRoot(root string) bool { @@ -313,7 +329,11 @@ func RelativeFiles(fp string, root string) ([]string, error) { fullPath := filepath.Join(root, fp) logrus.Debugf("Getting files and contents at root %s", fullPath) err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error { - if CheckWhitelist(path) && !HasFilepathPrefix(path, root) { + whitelisted, err := CheckWhitelist(path) + if err != nil { + return err + } + if whitelisted && !HasFilepathPrefix(path, root) { return nil } if err != nil { @@ -334,7 +354,11 @@ func Files(root string) ([]string, error) { var files []string logrus.Debugf("Getting files and contents at root %s", root) err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { - if CheckWhitelist(path) { + whitelisted, err := CheckWhitelist(path) + if err != nil { + return err + } + if whitelisted { return nil } files = append(files, path) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 21ff3a7661..2ffd0f61df 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -266,7 +266,11 @@ func Test_CheckWhitelist(t *testing.T) { whitelist = original }() whitelist = tt.args.whitelist - if got := CheckWhitelist(tt.args.path); got != tt.want { + got, err := CheckWhitelist(tt.args.path) + if err != nil { + t.Fatalf("error checking whitelist: %v", err) + } + if got != tt.want { t.Errorf("CheckWhitelist() = %v, want %v", got, tt.want) } })