Skip to content
This repository was archived by the owner on Mar 27, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/util/cloud_prepper.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (p CloudPrepper) GetFileSystem() (string, error) {
return "", err
}

return path, getFileSystemFromReference(ref, p.ImageSource, path)
return path, GetFileSystemFromReference(ref, p.ImageSource, path, nil)
}

func (p CloudPrepper) GetConfig() (ConfigSchema, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/daemon_prepper.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (p DaemonPrepper) GetFileSystem() (string, error) {
if err != nil {
return "", err
}
return path, getFileSystemFromReference(ref, src, path)
return path, GetFileSystemFromReference(ref, src, path, nil)
}

func (p DaemonPrepper) GetConfig() (ConfigSchema, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/docker_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func unpackDockerSave(tarPath string, target string) error {
}

for _, layer := range layers {
if err = UnTar(bytes.NewReader(layerMap[layer]), target); err != nil {
if err = UnTar(bytes.NewReader(layerMap[layer]), target, nil); err != nil {
return fmt.Errorf("Could not unpack layer %s: %s", layer, err)
}
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/util/fs_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,22 @@ func CheckSameFile(f1name, f2name string) (bool, error) {
}
return true, nil
}

// HasFilepathPrefix checks if the given file path begins with prefix
func HasFilepathPrefix(path, prefix string) bool {
path = filepath.Clean(path)
prefix = filepath.Clean(prefix)
pathArray := strings.Split(path, "/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use filepath.Split here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it only splits along the last separator

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh. Right.

prefixArray := strings.Split(prefix, "/")

if len(pathArray) < len(prefixArray) {
return false
}
for index := range prefixArray {
if prefixArray[index] == pathArray[index] {
continue
}
return false
}
return true
}
4 changes: 2 additions & 2 deletions pkg/util/image_prep_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func getImageFromTar(tarPath string) (string, error) {
return tempPath, unpackDockerSave(tarPath, tempPath)
}

func getFileSystemFromReference(ref types.ImageReference, imgSrc types.ImageSource, path string) error {
func GetFileSystemFromReference(ref types.ImageReference, imgSrc types.ImageSource, path string, whitelist []string) error {
img, err := ref.NewImage(nil)
if err != nil {
return err
Expand All @@ -151,7 +151,7 @@ func getFileSystemFromReference(ref types.ImageReference, imgSrc types.ImageSour
}
}
tr := tar.NewReader(reader)
if err := unpackTar(tr, path); err != nil {
if err := unpackTar(tr, path, whitelist); err != nil {
return err
}
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/util/tar_prepper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ package util
import (
"encoding/json"
"errors"
"io/ioutil"
"os"
"path/filepath"

"github.com/containers/image/docker/tarfile"
"github.com/docker/docker/client"
"github.com/sirupsen/logrus"
"io/ioutil"
"os"
"path/filepath"
)

type TarPrepper struct {
Expand Down Expand Up @@ -62,7 +61,7 @@ func (p TarPrepper) GetConfig() (ConfigSchema, error) {
return ConfigSchema{}, err
}
defer f.Close()
if err := UnTar(f, tempDir); err != nil {
if err := UnTar(f, tempDir, nil); err != nil {
return ConfigSchema{}, err
}

Expand Down
42 changes: 32 additions & 10 deletions pkg/util/tar_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,14 @@ package util

import (
"archive/tar"
"github.com/sirupsen/logrus"
"io"
"os"
"path/filepath"
"strings"

"github.com/sirupsen/logrus"
)

func unpackTar(tr *tar.Reader, path string) error {
func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
for {
header, err := tr.Next()
if err == io.EOF {
Expand All @@ -37,7 +36,6 @@ func unpackTar(tr *tar.Reader, path string) error {
logrus.Error("Error getting next tar header")
return err
}

if strings.Contains(header.Name, ".wh.") {
rmPath := filepath.Join(path, header.Name)
// Remove the .wh file if it was extracted.
Expand All @@ -54,8 +52,11 @@ func unpackTar(tr *tar.Reader, path string) error {
}
continue
}

target := filepath.Join(path, header.Name)
// Make sure the target isn't part of the whitelist
if checkWhitelist(target, whitelist) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A debug log here will come in handy later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it in the checkWhitelist function

continue
}
mode := header.FileInfo().Mode()
switch header.Typeflag {

Expand All @@ -65,7 +66,7 @@ func unpackTar(tr *tar.Reader, path string) error {
if err := os.MkdirAll(target, mode); err != nil {
return err
}
} else {
// In some cases, MkdirAll doesn't change the permissions, so run Chmod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's usually umask related, but I suppose if the dirs already exist it wouldn't change the permissions either

if err := os.Chmod(target, mode); err != nil {
return err
}
Expand All @@ -81,7 +82,6 @@ func unpackTar(tr *tar.Reader, path string) error {
return err
}
}

// It's possible we end up creating files that can't be overwritten based on their permissions.
// Explicitly delete an existing file before continuing.
if _, err := os.Stat(target); !os.IsNotExist(err) {
Expand All @@ -106,21 +106,43 @@ func unpackTar(tr *tar.Reader, path string) error {
return err
}
currFile.Close()
}
case tar.TypeSymlink:
// It's possible we end up creating files that can't be overwritten based on their permissions.
// Explicitly delete an existing file before continuing.
if _, err := os.Stat(target); !os.IsNotExist(err) {
logrus.Debugf("Removing %s to create symlink.", target)
if err := os.RemoveAll(target); err != nil {
logrus.Debugf("Unable to remove %s: %s", target, err)
}
}

if err = os.Symlink(header.Linkname, target); err != nil {
logrus.Errorf("Failed to create symlink between %s and %s: %s", header.Linkname, target, err)
}
}
}
return nil
}

func checkWhitelist(target string, whitelist []string) bool {
for _, w := range whitelist {
if HasFilepathPrefix(target, w) {
logrus.Debugf("Not extracting %s, as it has prefix %s which is whitelisted", target, w)
return true
}
}
return false
}

// UnTar takes in a path to a tar file and writes the untarred version to the provided target.
// Only untars one level, does not untar nested tars.
func UnTar(r io.Reader, target string) error {
func UnTar(r io.Reader, target string, whitelist []string) error {
if _, ok := os.Stat(target); ok != nil {
os.MkdirAll(target, 0775)
}

tr := tar.NewReader(r)
if err := unpackTar(tr, target); err != nil {
if err := unpackTar(tr, target, whitelist); err != nil {
return err
}
return nil
Expand Down
23 changes: 23 additions & 0 deletions util/fs_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,26 @@ func TestCheckSameFile(t *testing.T) {
}
}
}

func TestHasFilepathPrefix(t *testing.T) {
type test struct {
prefix string
path string
expectedBool bool
}

var tests = []test{
{"/foo", "/foo/bar", true},
{"/foo/", "/foo/bar", true},
{"/foo/bar", "/foo", false},
{"/foo/bar", "/foo/", false},
{"/foo", "/foobar", false},
}

for _, test := range tests {
hasPrefix := pkgutil.HasFilepathPrefix(test.path, test.prefix)
if hasPrefix != test.expectedBool {
t.Errorf("Got unexpected response: %v for prefix: %s and path: %s", hasPrefix, test.prefix, test.path)
}
}
}
22 changes: 15 additions & 7 deletions util/tar_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ import (

func TestUnTar(t *testing.T) {
testCases := []struct {
descrip string
tarPath string
target string
expected string
starter string
err bool
descrip string
tarPath string
target string
expected string
starter string
whitelist []string
err bool
}{
{
descrip: "Tar with files",
Expand Down Expand Up @@ -76,6 +77,13 @@ func TestUnTar(t *testing.T) {
expected: "testTars/la-croix-dir-update-actual",
starter: "testTars/la-croix-starter",
},
{
descrip: "Tar with whitelist",
tarPath: "testTars/la-croix2.tar",
target: "testTars/la-croix-whitelist",
expected: "testTars/la-croix1-actual",
whitelist: []string{"testTars/la-croix-whitelist/nest"},
},
}
for _, test := range testCases {
remove := true
Expand All @@ -86,7 +94,7 @@ func TestUnTar(t *testing.T) {
if err != nil {
t.Errorf("Error opening tar: %s", err)
}
if err := pkgutil.UnTar(r, test.target); err != nil && !test.err {
if err := pkgutil.UnTar(r, test.target, test.whitelist); err != nil && !test.err {
t.Errorf(test.descrip, "Got unexpected error: %s", err)
remove = false
}
Expand Down