Skip to content

Commit

Permalink
karmadactl init: add CRDs archive verification to enhance file system…
Browse files Browse the repository at this point in the history
… robustness

Signed-off-by: zhzhuang-zju <m17799853869@163.com>
  • Loading branch information
zhzhuang-zju committed Oct 19, 2024
1 parent 29c97e9 commit 0c2028d
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 6 deletions.
73 changes: 67 additions & 6 deletions pkg/karmadactl/cmdinit/kubernetes/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ limitations under the License.
package kubernetes

import (
"archive/tar"
"context"
"encoding/base64"
"fmt"
"net"
"os"
"path"
"path/filepath"
"strings"
"time"

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
87 changes: 87 additions & 0 deletions pkg/karmadactl/cmdinit/kubernetes/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}
}
46 changes: 46 additions & 0 deletions pkg/karmadactl/cmdinit/utils/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
12 changes: 12 additions & 0 deletions pkg/karmadactl/cmdinit/utils/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"archive/tar"
"bytes"
"compress/gzip"
"fmt"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 0c2028d

Please sign in to comment.