From 0c2028dfbce78720e894319e7b51ecfef66d8fc8 Mon Sep 17 00:00:00 2001 From: zhzhuang-zju Date: Sat, 19 Oct 2024 14:29:59 +0800 Subject: [PATCH] karmadactl init: add CRDs archive verification to enhance file system robustness Signed-off-by: zhzhuang-zju --- pkg/karmadactl/cmdinit/kubernetes/deploy.go | 73 ++++++++++++++-- .../cmdinit/kubernetes/deploy_test.go | 87 +++++++++++++++++++ pkg/karmadactl/cmdinit/utils/util.go | 46 ++++++++++ pkg/karmadactl/cmdinit/utils/util_test.go | 12 +++ 4 files changed, 212 insertions(+), 6 deletions(-) diff --git a/pkg/karmadactl/cmdinit/kubernetes/deploy.go b/pkg/karmadactl/cmdinit/kubernetes/deploy.go index 46ad340b5760..068810717f97 100644 --- a/pkg/karmadactl/cmdinit/kubernetes/deploy.go +++ b/pkg/karmadactl/cmdinit/kubernetes/deploy.go @@ -17,12 +17,14 @@ limitations under the License. package kubernetes import ( + "archive/tar" "context" "encoding/base64" "fmt" "net" "os" "path" + "path/filepath" "strings" "time" @@ -51,6 +53,8 @@ var ( "cn": "registry.cn-hangzhou.aliyuncs.com/google_containers", } + crdsArchive = []string{"crds", "crds/bases", "crds/patches"} + certList = []string{ globaloptions.CaCertAndKeyName, options.EtcdCaCertAndKeyName, @@ -366,19 +370,34 @@ func (i *CommandInitOption) genCerts() error { // prepareCRD download or unzip `crds.tar.gz` to `options.DataPath` func (i *CommandInitOption) prepareCRD() error { + var filename string if strings.HasPrefix(i.CRDs, "http") { - filename := i.KarmadaDataPath + "/" + path.Base(i.CRDs) + filename = i.KarmadaDataPath + "/" + path.Base(i.CRDs) klog.Infof("download crds file:%s", i.CRDs) if err := utils.DownloadFile(i.CRDs, filename); err != nil { return err } - if err := utils.DeCompress(filename, i.KarmadaDataPath); err != nil { - return err + } else { + filename = i.CRDs + klog.Infoln("local crds file name:", i.CRDs) + } + + if err := utils.CheckGzFiles(filename, checkCtlCrdsTar); err != nil { + return fmt.Errorf("inValid crd tar, err: %w", err) + } + + if err := utils.DeCompress(filename, i.KarmadaDataPath); err != nil { + return err + } + + for _, archive := range crdsArchive { + expectedDir := filepath.Join(i.KarmadaDataPath, archive) + exist, _ := utils.PathExists(expectedDir) + if !exist { + return fmt.Errorf("lacking the necessary file path: %s", expectedDir) } - return nil } - klog.Infoln("local crds file name:", i.CRDs) - return utils.DeCompress(i.CRDs, i.KarmadaDataPath) + return nil } func (i *CommandInitOption) createCertsSecrets() error { @@ -744,3 +763,45 @@ func generateServerURL(serverIP string, nodePort int32) (string, error) { func SupportedStorageMode() []string { return []string{etcdStorageModeEmptyDir, etcdStorageModeHostPath, etcdStorageModePVC} } + +// checkCtlCrdsTar checks if the CRDs package complies with file specifications. +// It verifies the following: +// 1. Whether the path is clean. +// 2. Whether the file directory structure meets expectations. +func checkCtlCrdsTar(header *tar.Header) error { + switch header.Typeflag { + case tar.TypeDir: + // in Unix-like systems, directory paths in tar archives end with a slash (/) to distinguish them from file paths. + if strings.HasSuffix(header.Name, "/") && len(header.Name) > 1 { + if !isCleanPath(header.Name[:len(header.Name)-1]) { + return fmt.Errorf("the given file contains unclean file dir: %s", header.Name) + } + } + if !isExpectedPath(header.Name, crdsArchive) { + return fmt.Errorf("the given file contains unexpected file dir: %s", header.Name) + } + case tar.TypeReg: + if !isCleanPath(header.Name) { + return fmt.Errorf("the given file contains unclean file path: %s", header.Name) + } + if !isExpectedPath(header.Name, crdsArchive) { + return fmt.Errorf("the given file contains unexpected file path: %s", header.Name) + } + default: + fmt.Printf("unknown type: %v in %s\n", header.Typeflag, header.Name) + } + return nil +} + +func isExpectedPath(path string, expectedDirs []string) bool { + for _, dir := range expectedDirs { + if path == dir || strings.HasPrefix(path, dir+"/") { + return true + } + } + return false +} + +func isCleanPath(path string) bool { + return path == filepath.Clean(path) +} diff --git a/pkg/karmadactl/cmdinit/kubernetes/deploy_test.go b/pkg/karmadactl/cmdinit/kubernetes/deploy_test.go index 5523a1f475a7..ce7706e32a1e 100644 --- a/pkg/karmadactl/cmdinit/kubernetes/deploy_test.go +++ b/pkg/karmadactl/cmdinit/kubernetes/deploy_test.go @@ -17,12 +17,15 @@ limitations under the License. package kubernetes import ( + "archive/tar" "context" + "fmt" "net" "os" "testing" "time" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" @@ -496,3 +499,87 @@ func TestKarmadaSchedulerImage(t *testing.T) { }) } } + +func TestCheckCtlCrdsTar(t *testing.T) { + testItems := []struct { + name string + header *tar.Header + expectedErr error + }{ + { + name: "unclean file dir 'crds/../'", + header: &tar.Header{ + Name: "crds/../", + Typeflag: tar.TypeDir, + }, + expectedErr: fmt.Errorf("the given file contains unclean file dir: %s", "crds/../"), + }, + { + name: "unexpected file dir '../crds'", + header: &tar.Header{ + Name: "../crds", + Typeflag: tar.TypeDir, + }, + expectedErr: fmt.Errorf("the given file contains unexpected file dir: %s", "../crds"), + }, + { + name: "unexpected file dir '..'", + header: &tar.Header{ + Name: "..", + Typeflag: tar.TypeDir, + }, + expectedErr: fmt.Errorf("the given file contains unexpected file dir: %s", ".."), + }, + { + name: "expected file dir 'crds/'", + header: &tar.Header{ + Name: "crds/", + Typeflag: tar.TypeDir, + }, + expectedErr: nil, + }, + { + name: "expected file dir 'crds'", + header: &tar.Header{ + Name: "crds", + Typeflag: tar.TypeDir, + }, + expectedErr: nil, + }, + { + name: "unclean file path 'crds/../a.yaml'", + header: &tar.Header{ + Name: "crds/../a.yaml", + Typeflag: tar.TypeReg, + }, + expectedErr: fmt.Errorf("the given file contains unclean file path: %s", "crds/../a.yaml"), + }, + { + name: "unexpected file path '../crds/a.yaml'", + header: &tar.Header{ + Name: "../crds/a.yaml", + Typeflag: tar.TypeReg, + }, + expectedErr: fmt.Errorf("the given file contains unexpected file path: %s", "../crds/a.yaml"), + }, + { + name: "unexpected file path '../a.yaml'", + header: &tar.Header{ + Name: "../a.yaml", + Typeflag: tar.TypeReg, + }, + expectedErr: fmt.Errorf("the given file contains unexpected file path: %s", "../a.yaml"), + }, + { + name: "expected file path 'crds/a.yaml'", + header: &tar.Header{ + Name: "crds/a.yaml", + Typeflag: tar.TypeReg, + }, + expectedErr: nil, + }, + } + for _, item := range testItems { + assert.Equal(t, item.expectedErr, checkCtlCrdsTar(item.header)) + } +} diff --git a/pkg/karmadactl/cmdinit/utils/util.go b/pkg/karmadactl/cmdinit/utils/util.go index 3c7c4d551e1b..395b14daf816 100644 --- a/pkg/karmadactl/cmdinit/utils/util.go +++ b/pkg/karmadactl/cmdinit/utils/util.go @@ -131,6 +131,38 @@ func DeCompress(file, targetPath string) error { return nil } +// CheckGzFiles check if the given file meet the check function. +func CheckGzFiles(file string, check func(*tar.Header) error) error { + r, err := os.Open(file) + if err != nil { + return err + } + defer r.Close() + + gr, err := gzip.NewReader(r) + if err != nil { + return fmt.Errorf("new reader failed. %v", err) + } + defer gr.Close() + + tr := tar.NewReader(gr) + for { + header, err := tr.Next() + if err != nil { + if err == io.EOF { + break + } + return err + } + + if err = check(header); err != nil { + return err + } + } + + return nil +} + // ioCopyN fix Potential DoS vulnerability via decompression bomb. func ioCopyN(outFile *os.File, tr *tar.Reader) error { for { @@ -157,3 +189,17 @@ func ListFiles(path string) []string { } return files } + +// PathExists check whether the path is exist +func PathExists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + + if os.IsNotExist(err) { + return false, nil + } + + return false, err +} diff --git a/pkg/karmadactl/cmdinit/utils/util_test.go b/pkg/karmadactl/cmdinit/utils/util_test.go index 20a550a4f2fa..44033e4e012c 100644 --- a/pkg/karmadactl/cmdinit/utils/util_test.go +++ b/pkg/karmadactl/cmdinit/utils/util_test.go @@ -20,6 +20,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "fmt" "io" "net/http" "net/http/httptest" @@ -62,6 +63,13 @@ func TestDownloadFile(t *testing.T) { return buf }() + check := func(header *tar.Header) error { + if header.Name != testFileName { + return fmt.Errorf("expected: %s, but got: %s", testFileName, header.Name) + } + return nil + } + s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { if _, err := io.Copy(rw, serverTar); err != nil { t.Fatal(err) @@ -82,6 +90,10 @@ func TestDownloadFile(t *testing.T) { if err != nil { t.Fatal(err) } + err = CheckGzFiles(downloadTar, check) + if err != nil { + t.Fatal(err) + } err = DeCompress(downloadTar, tmpDir) if err != nil { t.Fatal(err)