karmada-operator: add CRDs archive verification to enhance file syste…

…m robustness Signed-off-by: zhzhuang-zju <m17799853869@163.com>
zhzhuang-zju · Nov 27, 2024 · 63590cb · 63590cb
1 parent ccdf485
commit 63590cb
Show file tree

Hide file tree

Showing 3 changed files with 215 additions and 0 deletions.
diff --git a/operator/pkg/tasks/init/crd.go b/operator/pkg/tasks/init/crd.go
@@ -23,13 +23,15 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 
 	"k8s.io/klog/v2"
 
 	operatorv1alpha1 "github.com/karmada-io/karmada/operator/pkg/apis/operator/v1alpha1"
 	"github.com/karmada-io/karmada/operator/pkg/util"
 	"github.com/karmada-io/karmada/operator/pkg/workflow"
+	"github.com/karmada-io/karmada/pkg/util/validation"
 )
 
 var (
@@ -53,6 +55,10 @@ func NewPrepareCrdsTask() workflow.Task {
 				Name: "Unpack",
 				Run:  runUnpack,
 			},
+			{
+				Name: "post-check",
+				Run:  postCheck,
+			},
 		},
 	}
 }
@@ -154,6 +160,9 @@ func runUnpack(r workflow.RunData) error {
 	exist, _ := util.PathExists(crdsPath)
 	if !exist {
 		klog.V(2).InfoS("[runUnpack] CRD yaml files do not exist, unpacking tar file", "unpackDir", crdsDir)
+		if err = validation.ValidateTarball(crdsTarPath, validation.ValidateCrdsTarBall); err != nil {
+			return fmt.Errorf("[unpack] inValid crd tar, err: %w", err)
+		}
 		if err := util.Unpack(crdsTarPath, crdsDir); err != nil {
 			return fmt.Errorf("[unpack] failed to unpack crd tar, err: %w", err)
 		}
@@ -165,6 +174,29 @@ func runUnpack(r workflow.RunData) error {
 	return nil
 }
 
+func postCheck(r workflow.RunData) error {
+	data, ok := r.(InitData)
+	if !ok {
+		return errors.New("post-check task invoked with an invalid data struct")
+	}
+
+	crdsDir, err := getCrdsDir(data)
+	if err != nil {
+		return fmt.Errorf("[post-check] failed to get CRD dir, err: %w", err)
+	}
+
+	for _, archive := range validation.CrdsArchive {
+		expectedDir := filepath.Join(crdsDir, archive)
+		exist, _ := util.PathExists(expectedDir)
+		if !exist {
+			return fmt.Errorf("[post-check] Lacking the necessary file path: %s", expectedDir)
+		}
+	}
+
+	klog.V(2).InfoS("[post-check] Successfully post-check the crd tar archive", "karmada", klog.KObj(data))
+	return nil
+}
+
 func existCrdsTar(crdsDir string) bool {
 	files := util.ListFiles(crdsDir)
 	klog.V(2).InfoS("[existCrdsTar] Checking for CRD tar file in directory", "directory", crdsDir)

diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go
@@ -17,7 +17,13 @@ limitations under the License.
 package validation
 
 import (
+	"archive/tar"
+	"compress/gzip"
 	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/go-openapi/jsonpointer"
 	corev1 "k8s.io/api/core/v1"
@@ -372,3 +378,85 @@ func validateOverrideOperator(operator policyv1alpha1.OverriderOperator, value a
 	}
 	return allErrs
 }
+
+// CrdsArchive defines the expected tar archive.
+var CrdsArchive = []string{"crds", "crds/bases", "crds/patches"}
+
+// ValidateTarball opens a .tar.gz file, and validates each
+// entry in the tar archive using a provided validate function.
+func ValidateTarball(tarball string, validate func(*tar.Header) error) error {
+	r, err := os.Open(tarball)
+	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 = validate(header); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// ValidateCrdsTarBall 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 ValidateCrdsTarBall(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)
+			}
+		} else {
+			if !isCleanPath(header.Name) {
+				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:
+		return fmt.Errorf("unknown type: %v in %s", 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/util/validation/validation_test.go b/pkg/util/validation/validation_test.go
@@ -17,9 +17,12 @@ limitations under the License.
 package validation
 
 import (
+	"archive/tar"
+	"fmt"
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/utils/ptr"
@@ -717,3 +720,95 @@ func TestValidateApplicationFailover(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckOperatorCrdsTar(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: "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, ValidateCrdsTarBall(item.header))
+	}
+}