Fix multiple Windows tar issues

- When on the server side handling files, use filepath
- When loading files into the tarball, use path
- When on the clientside use filepath
- Use our own tar logic to avoid tarx and bugs with path/filepath
- Use splat on Windows as we do for linux.

Fixes #1266
Fixes #1229

Signed-off-by: John Schnake <jschnake@vmware.com>

go.mod

@@ -29,7 +29,6 @@ require (
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
 	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.4.0
-	github.com/viniciuschiele/tarx v0.0.0-20151205142357-6e3da540444d
 	golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975
 	golang.org/x/sync v0.0.0-20190423024810-112230192c58
 	gopkg.in/yaml.v2 v2.2.8

go.sum

@@ -244,8 +244,6 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
 github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
-github.com/viniciuschiele/tarx v0.0.0-20151205142357-6e3da540444d h1:Z8Bp/K+wf1+IUvu5Vk2Ml/mMI7fmZTT7rQjkLC8pAmQ=
-github.com/viniciuschiele/tarx v0.0.0-20151205142357-6e3da540444d/go.mod h1:8uo3DXfN526YN7JjAp4JkOMm4foTW4vPzPHaAzb4xiY=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
 go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=

pkg/client/results/reader.go

@@ -23,13 +23,14 @@ import (
 	"encoding/json"
 	"encoding/xml"
 	"fmt"
-	"gopkg.in/yaml.v3"
 	"io"
 	"os"
 	"path"
 	"path/filepath"
 	"strings"
 
+	"gopkg.in/yaml.v3"
+
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
 
 	goversion "github.com/hashicorp/go-version"
@@ -210,7 +211,7 @@ func (r *Reader) WalkFiles(walkfn filepath.WalkFunc) error {
 			header.FileInfo(),
 			tr,
 		}
-		err = walkfn(filepath.Clean(header.Name), info, err)
+		err = walkfn(path.Clean(header.Name), info, err)
 	}
 
 	if err == errStopWalk || err == io.EOF {
@@ -290,7 +291,7 @@ func (r *Reader) Metadata() string {
 func (r *Reader) ServerVersionFile() string {
 	switch r.Version {
 	case VersionEight:
-		return "serverversion/serverversion.json"
+		return path.Join("serverversion", "serverversion.json")
 	default:
 		return defaultServerVersionFile
 	}
@@ -306,7 +307,7 @@ func (r *Reader) NamespacedResources() string {
 func (r *Reader) NonNamespacedResources() string {
 	switch r.Version {
 	case VersionEight:
-		return "resources/non-ns/"
+		return path.Join("resources", "non-ns")
 	default:
 		return nonNamespacedResourcesDir
 	}
@@ -315,7 +316,7 @@ func (r *Reader) NonNamespacedResources() string {
 // NodesFile returns the path to the file that lists the nodes of the Kubernetes
 // cluster.
 func (r *Reader) NodesFile() string {
-	return filepath.Join(r.NonNamespacedResources(), defaultNodesFile)
+	return path.Join(r.NonNamespacedResources(), defaultNodesFile)
 }
 
 // ServerGroupsFile returns the path to the groups the Kubernetes API supported at the time of the run.
@@ -330,7 +331,7 @@ func ConfigFile(version string) string {
 	case VersionEight:
 		return "config.json"
 	default:
-		return "meta/config.json"
+		return path.Join("meta", "config.json")
 	}
 }
 

pkg/client/retrieve.go

@@ -19,42 +19,28 @@ package client
 import (
 	"archive/tar"
 	"compress/gzip"
-	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
 	pluginaggregation "github.com/vmware-tanzu/sonobuoy/pkg/plugin/aggregation"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/remotecommand"
 )
 
-const (
-	nodeOSLabelBeta = "beta.kubernetes.io/os"
-	nodeOSLabel     = "kubernetes.io/os"
-	nodeOSWindows   = "windows"
-)
-
 var (
-	linuxTarCommand = []string{
+	tarCmd = []string{
 		"/sonobuoy",
 		"splat",
 		config.AggregatorResultsPath,
 	}
-
-	winTarCommand = []string{
-		"powershell.exe",
-		"-Command",
-		fmt.Sprintf(`& {tar -cC %s --format pax -f - *.tar.gz}`, config.AggregatorResultsPath),
-	}
 )
 
 // RetrieveResults copies results from a sonobuoy run into a Reader in tar format.
@@ -81,11 +67,6 @@ func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, <-chan
 		return nil, nil, errors.Wrap(err, "failed to get the name of the aggregator pod to fetch results from")
 	}
 
-	tarCommand := linuxTarCommand
-	if isWin, _ := isPodRunningOnWindowsNode(client, cfg.Namespace, podName); isWin {
-		tarCommand = winTarCommand
-	}
-
 	restClient := client.CoreV1().RESTClient()
 	req := restClient.Post().
 		Resource("pods").
@@ -95,7 +76,7 @@ func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, <-chan
 		Param("container", config.AggregatorContainerName)
 	req.VersionedParams(&corev1.PodExecOptions{
 		Container: config.AggregatorContainerName,
-		Command:   tarCommand,
+		Command:   tarCmd,
 		Stdin:     false,
 		Stdout:    true,
 		Stderr:    false,
@@ -121,34 +102,6 @@ func (c *SonobuoyClient) RetrieveResults(cfg *RetrieveConfig) (io.Reader, <-chan
 	return reader, ec, nil
 }
 
-func isPodRunningOnWindowsNode(client kubernetes.Interface, ns, podName string) (bool, error) {
-	pod, err := client.CoreV1().Pods(ns).Get(context.TODO(), podName, metav1.GetOptions{})
-	if err != nil {
-		return false, errors.Wrapf(err, "unable to get pod %v in namespace %v", podName, ns)
-	}
-
-	nodeName := pod.Spec.NodeName
-	if nodeName == "" {
-		return false, nil
-	}
-
-	node, err := client.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
-	if err != nil {
-		return false, errors.Wrapf(err, "unable to get node %v", nodeName)
-	}
-
-	for k, v := range node.ObjectMeta.Labels {
-		if k == nodeOSLabel && v == nodeOSWindows {
-			return true, nil
-		}
-		if k == nodeOSLabelBeta && v == nodeOSWindows {
-			return true, nil
-		}
-	}
-
-	return false, nil
-}
-
 /** Everything below this marker originally was copy/pasta'd from k8s/k8s. The modification are:
   exporting UntarAll, returning the list of files created, and the fix for undrained readers  **/
 
@@ -160,6 +113,7 @@ func UntarAll(reader io.Reader, destFile, prefix string) (filenames []string, re
 	filenames = []string{}
 	// Adding compression per `splat` subcommand implementation
 	gzReader, err := gzip.NewReader(reader)
+
 	if err != nil {
 		returnErr = err
 		return
@@ -194,6 +148,13 @@ func UntarAll(reader io.Reader, destFile, prefix string) (filenames []string, re
 			}
 			break
 		}
+
+		// Avoid naively joining paths and allowing escape of intended directory.
+		// Recommended by github CodeQL; go/zipslip
+		if strings.Contains(header.Name, "..") {
+			continue
+		}
+
 		entrySeq++
 		mode := header.FileInfo().Mode()
 		outFileName := filepath.Join(destFile, header.Name[len(prefix):])

pkg/discovery/discovery.go

@@ -24,7 +24,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"path"
 	"path/filepath"
 	"time"
 
@@ -34,11 +33,11 @@ import (
 	"github.com/vmware-tanzu/sonobuoy/pkg/errlog"
 	"github.com/vmware-tanzu/sonobuoy/pkg/plugin"
 	pluginaggregation "github.com/vmware-tanzu/sonobuoy/pkg/plugin/aggregation"
+	"github.com/vmware-tanzu/sonobuoy/pkg/tarball"
 
 	"github.com/pkg/errors"
 	"github.com/rifflock/lfshook"
 	"github.com/sirupsen/logrus"
-	"github.com/viniciuschiele/tarx"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
@@ -81,8 +80,8 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 	// 1. Create the directory which will store the results, including the
 	// `meta` directory inside it (which we always need regardless of
 	// config)
-	outpath := path.Join(cfg.ResultsDir, cfg.UUID)
-	metapath := path.Join(outpath, MetaLocation)
+	outpath := filepath.Join(cfg.ResultsDir, cfg.UUID)
+	metapath := filepath.Join(outpath, MetaLocation)
 	err = os.MkdirAll(metapath, 0755)
 	if err != nil {
 		errlog.LogError(errors.Wrap(err, "could not create directory to store results"))
@@ -92,7 +91,7 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 	// Write logs to the configured results location. All log levels
 	// should write to the same log file
 	pathmap := make(lfshook.PathMap)
-	logfile := path.Join(metapath, "run.log")
+	logfile := filepath.Join(metapath, "run.log")
 	for _, level := range logrus.AllLevels {
 		pathmap[level] = logfile
 	}
@@ -135,7 +134,7 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 
 	// 3. Dump the config.json we used to run our test
 	if blob, err := json.Marshal(cfg); err == nil {
-		if err = ioutil.WriteFile(path.Join(metapath, "config.json"), blob, 0644); err != nil {
+		if err = ioutil.WriteFile(filepath.Join(metapath, "config.json"), blob, 0644); err != nil {
 			errlog.LogError(errors.Wrap(err, "could not write config.json file"))
 			return errCount + 1
 		}
@@ -209,7 +208,7 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 
 	// 6. Dump the query times
 	trackErrorsFor("recording query times")(
-		recorder.DumpQueryData(path.Join(metapath, "query-time.json")),
+		recorder.DumpQueryData(filepath.Join(metapath, "query-time.json")),
 	)
 
 	// 7. Clean up after the plugins
@@ -245,13 +244,14 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 	blob, err := json.Marshal(runInfo)
 	trackErrorsFor("marshalling run info")(err)
 	if err == nil {
-		err = ioutil.WriteFile(path.Join(metapath, results.InfoFile), blob, 0644)
+		err = ioutil.WriteFile(filepath.Join(metapath, results.InfoFile), blob, 0644)
 		trackErrorsFor("saving" + results.InfoFile)(err)
 	}
 
 	// 8. tarball up results YYYYMMDDHHMM_sonobuoy_UID.tar.gz
-	tb := cfg.ResultsDir + "/" + t.Format("200601021504") + "_sonobuoy_" + cfg.UUID + ".tar.gz"
-	err = tarx.Compress(tb, outpath, &tarx.CompressOptions{Compression: tarx.Gzip})
+	filename := fmt.Sprintf("%v_sonobuoy_%v.tar.gz", t.Format("200601021504"), cfg.UUID)
+	tb := filepath.Join(cfg.ResultsDir, filename)
+	err = tarball.DirToTarball(outpath, tb, true)
 	if err == nil {
 		defer os.RemoveAll(outpath)
 	}

pkg/discovery/queries.go

@@ -19,7 +19,7 @@ package discovery
 import (
 	"context"
 	"os"
-	"path"
+	"path/filepath"
 	"time"
 
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
@@ -149,9 +149,9 @@ func QueryResources(
 	}
 
 	// 1. Create the parent directory we will use to store the results
-	outdir := path.Join(cfg.OutputDir(), ClusterResourceLocation)
+	outdir := filepath.Join(cfg.OutputDir(), ClusterResourceLocation)
 	if ns != nil {
-		outdir = path.Join(cfg.OutputDir(), NSResourceLocation, *ns)
+		outdir = filepath.Join(cfg.OutputDir(), NSResourceLocation, *ns)
 	}
 
 	if err := os.MkdirAll(outdir, 0755); err != nil {
@@ -189,7 +189,7 @@ func QueryResources(
 
 		query := func() (time.Duration, error) {
 			return timedListQuery(
-				outdir+"/",
+				outdir,
 				groupText+"_"+gvr.Version+"_"+gvr.Resource+".json",
 				lister,
 			)

pkg/plugin/aggregation/aggregator_test.go

@@ -33,10 +33,10 @@ import (
 	"time"
 
 	"github.com/kylelemons/godebug/pretty"
-	"github.com/viniciuschiele/tarx"
 	"github.com/vmware-tanzu/sonobuoy/pkg/backplane/ca/authtest"
 	"github.com/vmware-tanzu/sonobuoy/pkg/plugin"
 	pluginutils "github.com/vmware-tanzu/sonobuoy/pkg/plugin/driver/utils"
+	"github.com/vmware-tanzu/sonobuoy/pkg/tarball"
 )
 
 func TestAggregation(t *testing.T) {
@@ -491,7 +491,7 @@ func makeTarWithContents(t *testing.T, filename string, fileContents []byte) (ta
 		return
 	}
 
-	err = tarx.Compress(tarfile, tardir, &tarx.CompressOptions{Compression: tarx.Gzip})
+	err = tarball.DirToTarball(tardir, tarfile, true)
 	if err != nil {
 		t.Fatalf("Could not create tar file %v: %v", tarfile, err)
 		return