diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 20315406..19b0b07d 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -62,6 +62,7 @@ jobs: cd $GITHUB_WORKSPACE if [[ $OS == "linux" && $ARCH == "amd64" ]]; then make OS=$OS ARCH=$ARCH + sudo env "PATH=$PATH" make privileged-test else make OS=$OS ARCH=$ARCH binary binary-minimal binary-debug cli bench exporter-minimal fi diff --git a/Makefile b/Makefile index 8cd8cf2d..98aa887b 100644 --- a/Makefile +++ b/Makefile @@ -11,12 +11,13 @@ STACKER := $(shell which stacker) GOLINTER := $(TOOLSDIR)/bin/golangci-lint NOTATION := $(TOOLSDIR)/bin/notation BATS := $(TOOLSDIR)/bin/bats +TESTDATA := $(TOP_LEVEL)/test/data OS ?= linux ARCH ?= amd64 BENCH_OUTPUT ?= stdout .PHONY: all -all: modcheck swagger binary binary-minimal binary-debug cli bench exporter-minimal verify-config test covhtml test-clean check +all: modcheck swagger binary binary-minimal binary-debug cli bench exporter-minimal verify-config test covhtml check .PHONY: modcheck modcheck: @@ -47,10 +48,7 @@ exporter-minimal: modcheck env CGO_ENABLED=0 GOOS=$(OS) GOARCH=$(ARCH) go build -o bin/zxp-$(OS)-$(ARCH) -buildmode=pie -tags minimal,containers_image_openpgp -v -trimpath ./cmd/zxp .PHONY: test -test: check-skopeo $(NOTATION) - $(shell mkdir -p test/data; cd test/data; ../scripts/gen_certs.sh; cd ${TOP_LEVEL}; skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:7 oci:${TOP_LEVEL}/test/data/zot-test:0.0.1;skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:8 oci:${TOP_LEVEL}/test/data/zot-cve-test:0.0.1) - $(shell sudo mkdir -p /etc/containers/certs.d/127.0.0.1:8089/; sudo cp test/data/client.* test/data/ca.* /etc/containers/certs.d/127.0.0.1:8089/;) - $(shell sudo chmod a=rwx /etc/containers/certs.d/127.0.0.1:8089/*.key) +test: check-skopeo $(TESTDATA) $(NOTATION) go test -tags extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-extended.txt -covermode=atomic ./... go test -tags minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-minimal.txt -covermode=atomic ./... # development-mode unit tests possibly using failure injection @@ -58,6 +56,14 @@ test: check-skopeo $(NOTATION) go test -tags dev,minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-dev-minimal.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... ./pkg/extensions/sync/... -run ^TestInject go test -tags stress,extended,containers_image_openpgp -v -trimpath -race -timeout 15m ./pkg/cli/stress_test.go +.PHONY: privileged-test +privileged-test: check-skopeo $(TESTDATA) $(NOTATION) + go test -tags needprivileges,extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-dev-needprivileges.txt -covermode=atomic ./pkg/storage/... ./pkg/cli/... -run ^TestElevatedPrivileges + +$(TESTDATA): check-skopeo + $(shell mkdir -p ${TESTDATA}; cd ${TESTDATA}; ../scripts/gen_certs.sh; cd ${TOP_LEVEL}; skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:7 oci:${TESTDATA}/zot-test:0.0.1;skopeo --insecure-policy copy -q docker://public.ecr.aws/t0x7q1g8/centos:8 oci:${TESTDATA}/zot-cve-test:0.0.1) + $(shell chmod -R a=rwx ${TESTDATA}) + .PHONY: run-bench run-bench: binary bench bin/zot-$(OS)-$(ARCH) serve examples/config-bench.json & @@ -65,10 +71,6 @@ run-bench: binary bench bin/zb-$(OS)-$(ARCH) -c 10 -n 100 -o $(BENCH_OUTPUT) http://localhost:8080 killall -r zot-* -.PHONY: test-clean -test-clean: - $(shell sudo rm -rf /etc/containers/certs.d/127.0.0.1:8089/) - .PHONY: check-skopeo check-skopeo: skopeo -v || (echo "You need skopeo to be installed in order to run tests"; exit 1) @@ -82,7 +84,7 @@ $(NOTATION): .PHONY: covhtml covhtml: go install github.com/wadey/gocovmerge@latest - gocovmerge coverage-minimal.txt coverage-extended.txt coverage-dev-minimal.txt coverage-dev-extended.txt > coverage.txt + gocovmerge coverage*.txt > coverage.txt go tool cover -html=coverage.txt -o coverage.html $(GOLINTER): diff --git a/pkg/cli/client_elevated_test.go b/pkg/cli/client_elevated_test.go new file mode 100644 index 00000000..139e7e6c --- /dev/null +++ b/pkg/cli/client_elevated_test.go @@ -0,0 +1,123 @@ +//go:build extended && needprivileges +// +build extended,needprivileges + +package cli //nolint:testpackage + +import ( + "bytes" + "context" + "crypto/tls" + "crypto/x509" + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + . "github.com/smartystreets/goconvey/convey" + "gopkg.in/resty.v1" + "zotregistry.io/zot/pkg/api" + "zotregistry.io/zot/pkg/api/config" +) + +func TestElevatedPrivilegesTLSNewControllerPrivilegedCert(t *testing.T) { + Convey("Privileged certs - Make a new controller", t, func() { + cmd := exec.Command("mkdir", "-p", "/etc/containers/certs.d/127.0.0.1:8089/") // nolint: gosec + _, err := cmd.Output() + if err != nil { + panic(err) + } + + defer exec.Command("rm", "-rf", "/etc/containers/certs.d/127.0.0.1:8089/") + + wd, _ := os.Getwd() + os.Chdir("../../test/data") + + clientGlob, _ := filepath.Glob("client.*") + caGlob, _ := filepath.Glob("ca.*") + + for _, file := range clientGlob { + cmd = exec.Command("cp", file, "/etc/containers/certs.d/127.0.0.1:8089/") + res, err := cmd.CombinedOutput() + if err != nil { + panic(string(res)) + } + } + + for _, file := range caGlob { + cmd = exec.Command("cp", file, "/etc/containers/certs.d/127.0.0.1:8089/") + res, err := cmd.CombinedOutput() + if err != nil { + panic(string(res)) + } + } + + allGlob, _ := filepath.Glob("/etc/containers/certs.d/127.0.0.1:8089/*.key") + + for _, file := range allGlob { + cmd = exec.Command("chmod", "a=rwx", file) + res, err := cmd.CombinedOutput() + if err != nil { + panic(string(res)) + } + } + + os.Chdir(wd) + + caCert, err := ioutil.ReadFile(CACert) + So(err, ShouldBeNil) + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + + resty.SetTLSClientConfig(&tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12}) + defer func() { resty.SetTLSClientConfig(nil) }() + conf := config.New() + conf.HTTP.Port = SecurePort2 + conf.HTTP.TLS = &config.TLSConfig{ + Cert: ServerCert, + Key: ServerKey, + CACert: CACert, + } + + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + go func() { + // this blocks + if err := ctlr.Run(context.Background()); err != nil { + return + } + }() + + // wait till ready + for { + _, err := resty.R().Get(BaseURL2) + if err == nil { + break + } + time.Sleep(100 * time.Millisecond) + } + + defer func() { + ctx := context.Background() + _ = ctlr.Server.Shutdown(ctx) + }() + + Convey("Certs in privileged path", func() { + configPath := makeConfigFile( + fmt.Sprintf(`{"configs":[{"_name":"imagetest","url":"%s/v2/_catalog","showspinner":false}]}`, + BaseSecureURL2)) + defer os.Remove(configPath) + + args := []string{"imagetest"} + imageCmd := NewImageCommand(new(searchService)) + imageBuff := bytes.NewBufferString("") + imageCmd.SetOut(imageBuff) + imageCmd.SetErr(imageBuff) + imageCmd.SetArgs(args) + err := imageCmd.Execute() + So(err, ShouldBeNil) + }) + }) +} diff --git a/pkg/cli/client_test.go b/pkg/cli/client_test.go index 159ec552..be151177 100644 --- a/pkg/cli/client_test.go +++ b/pkg/cli/client_test.go @@ -206,62 +206,6 @@ func TestTLSWithoutAuth(t *testing.T) { So(err, ShouldBeNil) }) }) - - Convey("Privileged certs - Make a new controller", t, func() { - caCert, err := ioutil.ReadFile(CACert) - So(err, ShouldBeNil) - caCertPool := x509.NewCertPool() - caCertPool.AppendCertsFromPEM(caCert) - - resty.SetTLSClientConfig(&tls.Config{RootCAs: caCertPool, MinVersion: tls.VersionTLS12}) - defer func() { resty.SetTLSClientConfig(nil) }() - conf := config.New() - conf.HTTP.Port = SecurePort2 - conf.HTTP.TLS = &config.TLSConfig{ - Cert: ServerCert, - Key: ServerKey, - CACert: CACert, - } - - ctlr := api.NewController(conf) - ctlr.Config.Storage.RootDirectory = t.TempDir() - go func() { - // this blocks - if err := ctlr.Run(context.Background()); err != nil { - return - } - }() - - // wait till ready - for { - _, err := resty.R().Get(BaseURL2) - if err == nil { - break - } - time.Sleep(100 * time.Millisecond) - } - - defer func() { - ctx := context.Background() - _ = ctlr.Server.Shutdown(ctx) - }() - - Convey("Certs in privileged path", func() { - configPath := makeConfigFile( - fmt.Sprintf(`{"configs":[{"_name":"imagetest","url":"%s/v2/_catalog","showspinner":false}]}`, - BaseSecureURL2)) - defer os.Remove(configPath) - - args := []string{"imagetest"} - imageCmd := NewImageCommand(new(searchService)) - imageBuff := bytes.NewBufferString("") - imageCmd.SetOut(imageBuff) - imageCmd.SetErr(imageBuff) - imageCmd.SetArgs(args) - err := imageCmd.Execute() - So(err, ShouldBeNil) - }) - }) } func TestTLSBadCerts(t *testing.T) { diff --git a/pkg/storage/storage_fs_elevated_test.go b/pkg/storage/storage_fs_elevated_test.go new file mode 100644 index 00000000..bd7ebd15 --- /dev/null +++ b/pkg/storage/storage_fs_elevated_test.go @@ -0,0 +1,97 @@ +//go:build needprivileges +// +build needprivileges + +package storage_test + +import ( + "bytes" + _ "crypto/sha256" + "io/ioutil" + "os" + "os/exec" + "path" + "strings" + "testing" + + godigest "github.com/opencontainers/go-digest" + "github.com/rs/zerolog" + . "github.com/smartystreets/goconvey/convey" + "zotregistry.io/zot/pkg/extensions/monitoring" + "zotregistry.io/zot/pkg/log" + "zotregistry.io/zot/pkg/storage" +) + +func TestElevatedPrivilegesInvalidDedupe(t *testing.T) { + Convey("Invalid dedupe scenarios", t, func() { + dir := t.TempDir() + + log := log.Logger{Logger: zerolog.New(os.Stdout)} + metrics := monitoring.NewMetricsServer(false, log) + imgStore := storage.NewImageStore(dir, true, storage.DefaultGCDelay, true, true, log, metrics) + + upload, err := imgStore.NewBlobUpload("dedupe1") + So(err, ShouldBeNil) + So(upload, ShouldNotBeEmpty) + + content := []byte("test-data3") + buf := bytes.NewBuffer(content) + buflen := buf.Len() + digest := godigest.FromBytes(content) + blob, err := imgStore.PutBlobChunkStreamed("dedupe1", upload, buf) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + blobDigest1 := strings.Split(digest.String(), ":")[1] + So(blobDigest1, ShouldNotBeEmpty) + + err = imgStore.FinishBlobUpload("dedupe1", upload, buf, digest.String()) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + // Create a file at the same place where FinishBlobUpload will create + err = imgStore.InitRepo("dedupe2") + So(err, ShouldBeNil) + + err = os.MkdirAll(path.Join(dir, "dedupe2", "blobs/sha256"), 0o755) + if err != nil { + panic(err) + } + + err = ioutil.WriteFile(path.Join(dir, "dedupe2", "blobs/sha256", blobDigest1), content, 0o755) // nolint: gosec + if err != nil { + panic(err) + } + + upload, err = imgStore.NewBlobUpload("dedupe2") + So(err, ShouldBeNil) + So(upload, ShouldNotBeEmpty) + + content = []byte("test-data3") + buf = bytes.NewBuffer(content) + buflen = buf.Len() + digest = godigest.FromBytes(content) + blob, err = imgStore.PutBlobChunkStreamed("dedupe2", upload, buf) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + + cmd := exec.Command("chattr", "+i", path.Join(dir, "dedupe2", "blobs/sha256", blobDigest1)) // nolint: gosec + _, err = cmd.Output() + if err != nil { + panic(err) + } + + err = imgStore.FinishBlobUpload("dedupe2", upload, buf, digest.String()) + So(err, ShouldNotBeNil) + So(blob, ShouldEqual, buflen) + + cmd = exec.Command("chattr", "-i", path.Join(dir, "dedupe2", "blobs/sha256", blobDigest1)) // nolint: gosec + _, err = cmd.Output() + if err != nil { + panic(err) + } + + err = imgStore.FinishBlobUpload("dedupe2", upload, buf, digest.String()) + So(err, ShouldBeNil) + So(blob, ShouldEqual, buflen) + }) +} diff --git a/pkg/storage/storage_fs_test.go b/pkg/storage/storage_fs_test.go index 02ae1109..3790714a 100644 --- a/pkg/storage/storage_fs_test.go +++ b/pkg/storage/storage_fs_test.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "math/big" "os" - "os/exec" "path" "strings" "testing" @@ -600,79 +599,6 @@ func TestNegativeCases(t *testing.T) { So(err, ShouldNotBeNil) }) - Convey("Invalid dedupe scenarios", t, func() { - dir := t.TempDir() - - log := log.Logger{Logger: zerolog.New(os.Stdout)} - metrics := monitoring.NewMetricsServer(false, log) - imgStore := storage.NewImageStore(dir, true, storage.DefaultGCDelay, true, true, log, metrics) - - upload, err := imgStore.NewBlobUpload("dedupe1") - So(err, ShouldBeNil) - So(upload, ShouldNotBeEmpty) - - content := []byte("test-data3") - buf := bytes.NewBuffer(content) - buflen := buf.Len() - digest := godigest.FromBytes(content) - blob, err := imgStore.PutBlobChunkStreamed("dedupe1", upload, buf) - So(err, ShouldBeNil) - So(blob, ShouldEqual, buflen) - - blobDigest1 := strings.Split(digest.String(), ":")[1] - So(blobDigest1, ShouldNotBeEmpty) - - err = imgStore.FinishBlobUpload("dedupe1", upload, buf, digest.String()) - So(err, ShouldBeNil) - So(blob, ShouldEqual, buflen) - - // Create a file at the same place where FinishBlobUpload will create - err = imgStore.InitRepo("dedupe2") - So(err, ShouldBeNil) - - err = os.MkdirAll(path.Join(dir, "dedupe2", "blobs/sha256"), 0o755) - if err != nil { - panic(err) - } - - err = ioutil.WriteFile(path.Join(dir, "dedupe2", "blobs/sha256", blobDigest1), content, 0o755) // nolint: gosec - if err != nil { - panic(err) - } - - upload, err = imgStore.NewBlobUpload("dedupe2") - So(err, ShouldBeNil) - So(upload, ShouldNotBeEmpty) - - content = []byte("test-data3") - buf = bytes.NewBuffer(content) - buflen = buf.Len() - digest = godigest.FromBytes(content) - blob, err = imgStore.PutBlobChunkStreamed("dedupe2", upload, buf) - So(err, ShouldBeNil) - So(blob, ShouldEqual, buflen) - - cmd := exec.Command("sudo", "chattr", "+i", path.Join(dir, "dedupe2", "blobs/sha256", blobDigest1)) // nolint: gosec - _, err = cmd.Output() - if err != nil { - panic(err) - } - - err = imgStore.FinishBlobUpload("dedupe2", upload, buf, digest.String()) - So(err, ShouldNotBeNil) - So(blob, ShouldEqual, buflen) - - cmd = exec.Command("sudo", "chattr", "-i", path.Join(dir, "dedupe2", "blobs/sha256", blobDigest1)) // nolint: gosec - _, err = cmd.Output() - if err != nil { - panic(err) - } - - err = imgStore.FinishBlobUpload("dedupe2", upload, buf, digest.String()) - So(err, ShouldBeNil) - So(blob, ShouldEqual, buflen) - }) - Convey("DirExists call with a filename as argument", t, func(c C) { dir := t.TempDir()