From 70c0096821d95e1f3c8503e239ae02f51d6ec89f Mon Sep 17 00:00:00 2001 From: Yodahe Alemu Date: Thu, 30 Jul 2020 14:07:21 -0400 Subject: [PATCH] fixed issue where vulns were only getting logged when a severe vuln was found; also improved logging of vulns to include occurrence name and disclaimer that these vulns are only related to package binary issues --- cip.go | 7 ++ lib/dockerregistry/checks.go | 47 +++++++------ lib/dockerregistry/checks_test.go | 108 ++++++++++++++++++++---------- lib/dockerregistry/types.go | 9 +-- 4 files changed, 113 insertions(+), 58 deletions(-) diff --git a/cip.go b/cip.go index ea451a30..5013b14b 100644 --- a/cip.go +++ b/cip.go @@ -286,6 +286,13 @@ func main() { if *severityThresholdPtr >= 0 { klog.Info("********** START (VULN CHECK) **********") + klog.Info("DISCLAIMER: Vulnerabilities are found as issues with " + + "package binaries within image layers, not necessarily " + + "with the image layers themselves. So a \"fixable\" " + + "vulnerability may not necessarily be immediately" + + "actionable. For example, even though a fixed version " + + "of the binary is available, it doesn't necessarily mean " + + "that a new version of the image layer is available.") } else if *dryRunPtr { klog.Info("********** START (DRY RUN) **********") } else { diff --git a/lib/dockerregistry/checks.go b/lib/dockerregistry/checks.go index 4a957a18..24ada224 100644 --- a/lib/dockerregistry/checks.go +++ b/lib/dockerregistry/checks.go @@ -27,6 +27,8 @@ import ( "strings" "sync" + "k8s.io/klog" + containeranalysis "cloud.google.com/go/containeranalysis/apiv1" "google.golang.org/api/iterator" grafeaspb "google.golang.org/genproto/googleapis/grafeas/v1" @@ -332,29 +334,36 @@ func (check *ImageVulnCheck) Run() error { Error: err}) } - severeOccurrences := 0 + fixableSevereOccurrences := 0 for _, occ := range occurrences { + vuln := occ.GetVulnerability() + vulnErr := ImageVulnError{ + edge.SrcImageTag.ImageName, + edge.Digest, + occ.GetName(), + vuln, + } // The vulnerability check should only reject a PR if it finds // vulnerabilities that are both fixable and severe - if occ.GetFixAvailable() && - IsSevereOccurrence(occ, check.SeverityThreshold) { + if vuln.GetFixAvailable() && + IsSevereOccurrence(vuln, check.SeverityThreshold) { errors = append(errors, Error{ - Context: "vulnerabilities severity check", - Error: ImageVulnError{ - edge.SrcImageTag.ImageName, - edge.Digest, - occ, - }, + Context: "Vulnerability Occurrence w/ Fix Available", + Error: vulnErr, }) - severeOccurrences++ + fixableSevereOccurrences++ + } else { + klog.Error(vulnErr) } } - if severeOccurrences > 0 { + + if fixableSevereOccurrences > 0 { vulnerableImages = append(vulnerableImages, - fmt.Sprintf("%v@%v [%v severe vulnerabilities, %v total]", + fmt.Sprintf("%v@%v [%v fixable severe vulnerabilities, "+ + "%v total]", edge.SrcImageTag.ImageName, edge.Digest, - severeOccurrences, + fixableSevereOccurrences, len(occurrences))) } @@ -378,10 +387,8 @@ func (check *ImageVulnCheck) Run() error { // Error is a function of ImageSizeError and implements the error interface. func (err ImageVulnError) Error() string { - vulnJSON, _ := json.MarshalIndent(err.Vulnerability, "", " ") - return fmt.Sprintf("The image %v@%v contains the following "+ - "vulnerability:\n%v", - err.ImageName, err.Digest, string(vulnJSON)) + vulnJSON, _ := json.MarshalIndent(err, "", " ") + return string(vulnJSON) } // IsSevereOccurrence checks if a vulnerability is a high enough severity to @@ -413,7 +420,7 @@ func parseImageProjectID( func mkRealVulnProducer(client *containeranalysis.Client) ImageVulnProducer { return func( edge PromotionEdge, - ) ([]*grafeaspb.VulnerabilityOccurrence, error) { + ) ([]*grafeaspb.Occurrence, error) { // resourceURL is of the form https://gcr.io/[projectID]/my-image resourceURL := "https://" + path.Join(string(edge.SrcRegistry.Name), string(edge.SrcImageTag.ImageName)) + "@" + string(edge.Digest) @@ -430,7 +437,7 @@ func mkRealVulnProducer(client *containeranalysis.Client) ImageVulnProducer { resourceURL, "VULNERABILITY"), } - var occurrenceList []*grafeaspb.VulnerabilityOccurrence + var occurrenceList []*grafeaspb.Occurrence it := client.GetGrafeasClient().ListOccurrences(ctx, req) for { occ, err := it.Next() @@ -440,7 +447,7 @@ func mkRealVulnProducer(client *containeranalysis.Client) ImageVulnProducer { if err != nil { return nil, fmt.Errorf("occurrence iteration error: %v", err) } - occurrenceList = append(occurrenceList, occ.GetVulnerability()) + occurrenceList = append(occurrenceList, occ) } return occurrenceList, nil diff --git a/lib/dockerregistry/checks_test.go b/lib/dockerregistry/checks_test.go index 5841d220..25fa10f2 100644 --- a/lib/dockerregistry/checks_test.go +++ b/lib/dockerregistry/checks_test.go @@ -354,12 +354,12 @@ func TestImageVulnCheck(t *testing.T) { } mkVulnProducerFake := func( - edgeVulnerabilities map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence, + edgeVulnOccurrences map[reg.Digest][]*grafeaspb.Occurrence, ) reg.ImageVulnProducer { return func( edge reg.PromotionEdge, - ) ([]*grafeaspb.VulnerabilityOccurrence, error) { - return edgeVulnerabilities[edge.Digest], nil + ) ([]*grafeaspb.Occurrence, error) { + return edgeVulnOccurrences[edge.Digest], nil } } @@ -367,7 +367,7 @@ func TestImageVulnCheck(t *testing.T) { name string severityThreshold int edges map[reg.PromotionEdge]interface{} - vulnerabilities map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence + vulnerabilities map[reg.Digest][]*grafeaspb.Occurrence expected error }{ { @@ -377,17 +377,25 @@ func TestImageVulnCheck(t *testing.T) { edge1: nil, edge2: nil, }, - map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence{ + map[reg.Digest][]*grafeaspb.Occurrence{ "sha256:000": { { - Severity: grafeaspb.Severity_LOW, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_LOW, + FixAvailable: true, + }, + }, }, }, "sha256:111": { { - Severity: grafeaspb.Severity_MEDIUM, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_MEDIUM, + FixAvailable: true, + }, + }, }, }, }, @@ -400,23 +408,31 @@ func TestImageVulnCheck(t *testing.T) { edge1: nil, edge2: nil, }, - map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence{ + map[reg.Digest][]*grafeaspb.Occurrence{ "sha256:000": { { - Severity: grafeaspb.Severity_HIGH, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_HIGH, + FixAvailable: true, + }, + }, }, }, "sha256:111": { { - Severity: grafeaspb.Severity_HIGH, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_HIGH, + FixAvailable: true, + }, + }, }, }, }, fmt.Errorf("VulnerabilityCheck: The following vulnerable images were found:\n" + - " bar@sha256:111 [1 severe vulnerabilities, 1 total]\n" + - " foo@sha256:000 [1 severe vulnerabilities, 1 total]"), + " bar@sha256:111 [1 fixable severe vulnerabilities, 1 total]\n" + + " foo@sha256:000 [1 fixable severe vulnerabilities, 1 total]"), }, { "Severity above threshold", @@ -425,27 +441,39 @@ func TestImageVulnCheck(t *testing.T) { edge1: nil, edge2: nil, }, - map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence{ + map[reg.Digest][]*grafeaspb.Occurrence{ "sha256:000": { { - Severity: grafeaspb.Severity_HIGH, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_HIGH, + FixAvailable: true, + }, + }, }, }, "sha256:111": { { - Severity: grafeaspb.Severity_HIGH, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_CRITICAL, + FixAvailable: true, + }, + }, }, { - Severity: grafeaspb.Severity_CRITICAL, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_HIGH, + FixAvailable: true, + }, + }, }, }, }, fmt.Errorf("VulnerabilityCheck: The following vulnerable images were found:\n" + - " bar@sha256:111 [2 severe vulnerabilities, 2 total]\n" + - " foo@sha256:000 [1 severe vulnerabilities, 1 total]"), + " bar@sha256:111 [2 fixable severe vulnerabilities, 2 total]\n" + + " foo@sha256:000 [1 fixable severe vulnerabilities, 1 total]"), }, { "Multiple edges with same source image", @@ -454,16 +482,20 @@ func TestImageVulnCheck(t *testing.T) { edge2: nil, edge3: nil, }, - map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence{ + map[reg.Digest][]*grafeaspb.Occurrence{ "sha256:111": { { - Severity: grafeaspb.Severity_HIGH, - FixAvailable: true, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_HIGH, + FixAvailable: true, + }, + }, }, }, }, fmt.Errorf("VulnerabilityCheck: The following vulnerable images were found:\n" + - " bar@sha256:111 [1 severe vulnerabilities, 1 total]"), + " bar@sha256:111 [1 fixable severe vulnerabilities, 1 total]"), }, { "Multiple vulnerabilities with no fix", @@ -471,15 +503,23 @@ func TestImageVulnCheck(t *testing.T) { map[reg.PromotionEdge]interface{}{ edge1: nil, }, - map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence{ + map[reg.Digest][]*grafeaspb.Occurrence{ "sha256:000": { { - Severity: grafeaspb.Severity_HIGH, - FixAvailable: false, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_HIGH, + FixAvailable: false, + }, + }, }, { - Severity: grafeaspb.Severity_CRITICAL, - FixAvailable: false, + Details: &grafeaspb.Occurrence_Vulnerability{ + Vulnerability: &grafeaspb.VulnerabilityOccurrence{ + Severity: grafeaspb.Severity_CRITICAL, + FixAvailable: false, + }, + }, }, }, }, diff --git a/lib/dockerregistry/types.go b/lib/dockerregistry/types.go index 8370f1ca..30d1eb04 100644 --- a/lib/dockerregistry/types.go +++ b/lib/dockerregistry/types.go @@ -54,16 +54,17 @@ type ImageSizeError struct { // ImageVulnError contains ImageVulnCheck information on images that contain a // vulnerability with a severity level at or above the defined threshold. type ImageVulnError struct { - ImageName ImageName - Digest Digest - Vulnerability *grafeaspb.VulnerabilityOccurrence + ImageName ImageName + Digest Digest + OccurrenceName string + Vulnerability *grafeaspb.VulnerabilityOccurrence } // ImageVulnProducer is used by ImageVulnCheck to get the vulnerabilities for // an image and allows for custom vulnerability producers for testing. type ImageVulnProducer func( edge PromotionEdge, -) ([]*grafeaspb.VulnerabilityOccurrence, error) +) ([]*grafeaspb.Occurrence, error) // CapturedRequests holds a map of all PromotionRequests that were generated. It // is used for both -dry-run and testing.