From 7fee57e7cce5b2c182661585ff145a3823ec70e6 Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Wed, 5 Jul 2023 19:08:16 +0300 Subject: [PATCH] fix(CVE): attempt to scan now returns early with an error if trivyDB metadata json is missing (#1548) Also modify zli to retry in case of such errors, assuming the trivyDB will eventually be downloaded by the scheduled task. Signed-off-by: Andrei Aaron --- errors/errors.go | 1 + pkg/cli/cve_cmd.go | 42 ++++++++++-- pkg/cli/cve_cmd_test.go | 67 +++++++++++++++++++ pkg/cli/service.go | 2 +- pkg/extensions/search/cve/trivy/scanner.go | 37 +++++++++- .../search/cve/trivy/scanner_internal_test.go | 20 ++++-- 6 files changed, 159 insertions(+), 10 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 21d1ca36..7474761b 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -46,6 +46,7 @@ var ( ErrEmptyValue = errors.New("cache: empty value") ErrEmptyRepoList = errors.New("search: no repository found") ErrCVESearchDisabled = errors.New("search: CVE search is disabled") + ErrCVEDBNotFound = errors.New("cve: CVE DB is not present") ErrInvalidRepositoryName = errors.New("repository: not a valid repository name") ErrSyncMissingCatalog = errors.New("sync: couldn't fetch upstream registry's catalog") ErrMethodNotSupported = errors.New("storage: method not supported") diff --git a/pkg/cli/cve_cmd.go b/pkg/cli/cve_cmd.go index 7a2f9622..68a4ec2f 100644 --- a/pkg/cli/cve_cmd.go +++ b/pkg/cli/cve_cmd.go @@ -7,6 +7,8 @@ import ( "fmt" "os" "path" + "strings" + "time" "github.com/briandowns/spinner" "github.com/spf13/cobra" @@ -14,6 +16,10 @@ import ( zotErrors "zotregistry.io/zot/errors" ) +const ( + cveDBRetryInterval = 3 +) + func NewCveCommand(searchService SearchService) *cobra.Command { searchCveParams := make(map[string]*string) @@ -148,13 +154,41 @@ func searchCve(searchConfig searchConfig) error { } for _, searcher := range searchers { - found, err := searcher.search(searchConfig) - if found { - if err != nil { + // there can be CVE DB readyness issues on the server side + // we need a retry mechanism for that specific type of errors + maxAttempts := 20 + + for i := 0; i < maxAttempts; i++ { + found, err := searcher.search(searchConfig) + if !found { + // searcher does not support this searchConfig + // exit the attempts loop and try a different searcher + break + } + + if err == nil { + // searcher matcher search config and results are already printed + return nil + } + + if i+1 >= maxAttempts { + // searcher matches search config but there are errors + // this is the last attempt and we cannot retry return err } - return nil + if strings.Contains(err.Error(), zotErrors.ErrCVEDBNotFound.Error()) { + // searches matches search config but CVE DB is not ready server side + // wait and retry a few more times + fmt.Fprintln(searchConfig.resultWriter, + "[warning] CVE DB is not ready [", i, "] - retry in ", cveDBRetryInterval, " seconds") + time.Sleep(cveDBRetryInterval * time.Second) + + continue + } + + // an unrecoverable error occurred + return err } } diff --git a/pkg/cli/cve_cmd_test.go b/pkg/cli/cve_cmd_test.go index 36594f6c..1fa6d032 100644 --- a/pkg/cli/cve_cmd_test.go +++ b/pkg/cli/cve_cmd_test.go @@ -14,7 +14,9 @@ import ( "os" "path" "regexp" + "strconv" "strings" + "sync" "testing" "time" @@ -287,6 +289,48 @@ func TestSearchCVECmd(t *testing.T) { So(err, ShouldBeNil) }) + Convey("Test images by CVE ID - positive with retries", t, func() { + args := []string{"cvetest", "--cve-id", "aCVEID", "--url", "someURL"} + configPath := makeConfigFile(`{"configs":[{"_name":"cvetest","showspinner":false}]}`) + defer os.Remove(configPath) + mockService := mockServiceForRetry{succeedOn: 2} // CVE info will be provided in 2nd attempt + cveCmd := NewCveCommand(&mockService) + buff := bytes.NewBufferString("") + cveCmd.SetOut(buff) + cveCmd.SetErr(buff) + cveCmd.SetArgs(args) + err := cveCmd.Execute() + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + t.Logf("Output: %s", str) + So(strings.TrimSpace(str), ShouldContainSubstring, + "[warning] CVE DB is not ready [ 0 ] - retry in "+strconv.Itoa(cveDBRetryInterval)+" seconds") + So(strings.TrimSpace(str), ShouldContainSubstring, + "IMAGE NAME TAG OS/ARCH DIGEST SIGNED SIZE anImage tag os/arch 6e2f80bf false 123kB") + So(err, ShouldBeNil) + }) + + Convey("Test images by CVE ID - failed after retries", t, func() { + args := []string{"cvetest", "--cve-id", "aCVEID", "--url", "someURL"} + configPath := makeConfigFile(`{"configs":[{"_name":"cvetest","showspinner":false}]}`) + defer os.Remove(configPath) + mockService := mockServiceForRetry{succeedOn: -1} // CVE info will be unavailable on all retries + cveCmd := NewCveCommand(&mockService) + buff := bytes.NewBufferString("") + cveCmd.SetOut(buff) + cveCmd.SetErr(buff) + cveCmd.SetArgs(args) + err := cveCmd.Execute() + space := regexp.MustCompile(`\s+`) + str := space.ReplaceAllString(buff.String(), " ") + t.Logf("Output: %s", str) + So(strings.TrimSpace(str), ShouldContainSubstring, + "[warning] CVE DB is not ready [ 0 ] - retry in "+strconv.Itoa(cveDBRetryInterval)+" seconds") + So(strings.TrimSpace(str), ShouldNotContainSubstring, + "IMAGE NAME TAG OS/ARCH DIGEST SIGNED SIZE anImage tag os/arch 6e2f80bf false 123kB") + So(err, ShouldNotBeNil) + }) + Convey("Test images by CVE ID - invalid CVE ID", t, func() { args := []string{"cvetest", "--cve-id", "invalidCVEID"} configPath := makeConfigFile(`{"configs":[{"_name":"cvetest","showspinner":false}]}`) @@ -1222,3 +1266,26 @@ func getMockCveInfo(repoDB repodb.RepoDB, log log.Logger) cveinfo.CveInfo { RepoDB: repoDB, } } + +type mockServiceForRetry struct { + mockService + retryCounter int + succeedOn int +} + +func (service *mockServiceForRetry) getImagesByCveID(ctx context.Context, config searchConfig, + username, password, cvid string, rch chan stringResult, wtgrp *sync.WaitGroup, +) { + service.retryCounter += 1 + + if service.retryCounter < service.succeedOn || service.succeedOn < 0 { + rch <- stringResult{"", zotErrors.ErrCVEDBNotFound} + close(rch) + + wtgrp.Done() + + return + } + + service.getImageByName(ctx, config, username, password, "anImage", rch, wtgrp) +} diff --git a/pkg/cli/service.go b/pkg/cli/service.go index ca3e5bff..b60bb510 100644 --- a/pkg/cli/service.go +++ b/pkg/cli/service.go @@ -943,7 +943,7 @@ func isContextDone(ctx context.Context) bool { } } -// Query using JQL, the query string is passed as a parameter +// Query using GQL, the query string is passed as a parameter // errors are returned in the stringResult channel, the unmarshalled payload is in resultPtr. func (service searchService) makeGraphQLQuery(ctx context.Context, config searchConfig, username, password, query string, diff --git a/pkg/extensions/search/cve/trivy/scanner.go b/pkg/extensions/search/cve/trivy/scanner.go index bfb25950..9b1fd080 100644 --- a/pkg/extensions/search/cve/trivy/scanner.go +++ b/pkg/extensions/search/cve/trivy/scanner.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "fmt" + "os" "path" "sync" + "github.com/aquasecurity/trivy-db/pkg/metadata" dbTypes "github.com/aquasecurity/trivy-db/pkg/types" "github.com/aquasecurity/trivy/pkg/commands/artifact" "github.com/aquasecurity/trivy/pkg/commands/operation" @@ -155,6 +157,11 @@ func (scanner Scanner) getTrivyOptions(image string) flag.Options { func (scanner Scanner) runTrivy(opts flag.Options) (types.Report, error) { ctx := context.Background() + err := scanner.checkDBPresence() + if err != nil { + return types.Report{}, err + } + runner, err := artifact.NewRunner(ctx, opts) if err != nil { return types.Report{}, err @@ -333,7 +340,7 @@ func (scanner Scanner) ScanImage(image string) (map[string]cvemodel.CVE, error) return cveidMap, nil } -// UpdateDb download the Trivy DB / Cache under the store root directory. +// UpdateDB downloads the Trivy DB / Cache under the store root directory. func (scanner Scanner) UpdateDB() error { // We need a lock as using multiple substores each with it's own DB // can result in a DATARACE because some varibles in trivy-db are global @@ -395,6 +402,34 @@ func (scanner Scanner) updateDB(dbDir string) error { return nil } +// checkDBPresence errors if the DB metadata files cannot be accessed. +func (scanner Scanner) checkDBPresence() error { + result := true + + if scanner.storeController.DefaultStore != nil { + dbDir := path.Join(scanner.storeController.DefaultStore.RootDir(), "_trivy") + if _, err := os.Stat(metadata.Path(dbDir)); err != nil { + result = false + } + } + + if scanner.storeController.SubStore != nil { + for _, storage := range scanner.storeController.SubStore { + dbDir := path.Join(storage.RootDir(), "_trivy") + + if _, err := os.Stat(metadata.Path(dbDir)); err != nil { + result = false + } + } + } + + if !result { + return zerr.ErrCVEDBNotFound + } + + return nil +} + func (scanner Scanner) CompareSeverities(severity1, severity2 string) int { return dbTypes.CompareSeverityString(severity1, severity2) } diff --git a/pkg/extensions/search/cve/trivy/scanner_internal_test.go b/pkg/extensions/search/cve/trivy/scanner_internal_test.go index 189d983b..3651ceee 100644 --- a/pkg/extensions/search/cve/trivy/scanner_internal_test.go +++ b/pkg/extensions/search/cve/trivy/scanner_internal_test.go @@ -15,6 +15,7 @@ import ( ispec "github.com/opencontainers/image-spec/specs-go/v1" . "github.com/smartystreets/goconvey/convey" + zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/extensions/monitoring" "zotregistry.io/zot/pkg/log" @@ -124,6 +125,11 @@ func TestMultipleStoragePath(t *testing.T) { generateTestImage(storeController, img1) generateTestImage(storeController, img2) + // Try to scan without the DB being downloaded + _, err = scanner.ScanImage(img0) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrCVEDBNotFound) + // Download DB since DB download on scan is disabled err = scanner.UpdateDB() So(err, ShouldBeNil) @@ -197,12 +203,20 @@ func TestTrivyLibraryErrors(t *testing.T) { err = repodb.ParseStorage(repoDB, storeController, log) So(err, ShouldBeNil) + img := "zot-test:0.0.1" //nolint:goconst + // Download DB fails for missing DB url scanner := NewScanner(storeController, repoDB, "", "", log) err = scanner.UpdateDB() So(err, ShouldNotBeNil) + // Try to scan without the DB being downloaded + opts := scanner.getTrivyOptions(img) + _, err = scanner.runTrivy(opts) + So(err, ShouldNotBeNil) + So(err, ShouldWrap, zerr.ErrCVEDBNotFound) + // Download DB fails for invalid Java DB scanner = NewScanner(storeController, repoDB, "ghcr.io/project-zot/trivy-db", "ghcr.io/project-zot/trivy-not-db", log) @@ -217,10 +231,8 @@ func TestTrivyLibraryErrors(t *testing.T) { err = scanner.UpdateDB() So(err, ShouldBeNil) - img := "zot-test:0.0.1" - // Scanning image with correct options - opts := scanner.getTrivyOptions(img) + opts = scanner.getTrivyOptions(img) _, err = scanner.runTrivy(opts) So(err, ShouldBeNil) @@ -482,7 +494,7 @@ func TestDefaultTrivyDBUrl(t *testing.T) { So(err, ShouldBeNil) // Scanning image - img := "zot-test:0.0.1" + img := "zot-test:0.0.1" //nolint:goconst opts := scanner.getTrivyOptions(img) _, err = scanner.runTrivy(opts)