Skip to content

Commit

Permalink
fixed issue where vulns were only getting logged when a severe vuln w…
Browse files Browse the repository at this point in the history 
…as found; also improved logging of vulns to include occurrence name and disclaimer that these vulns are only related to package binary issues
  • Loading branch information
@yodahekinsew
yodahekinsew committed Aug 3, 2020
1 parent 62b6885 commit 70c0096
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 58 deletions.
7 changes: 7 additions & 0 deletions cip.go
View file
Expand Up @@ -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 {
Expand Down
47 changes: 27 additions & 20 deletions lib/dockerregistry/checks.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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)))
}

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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
Expand Down
108 changes: 74 additions & 34 deletions lib/dockerregistry/checks_test.go
View file
Expand Up @@ -354,20 +354,20 @@ 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
}
}

var tests = []struct {
name string
severityThreshold int
edges map[reg.PromotionEdge]interface{}
vulnerabilities map[reg.Digest][]*grafeaspb.VulnerabilityOccurrence
vulnerabilities map[reg.Digest][]*grafeaspb.Occurrence
expected error
}{
{
Expand All @@ -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,
},
},
},
},
},
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -454,32 +482,44 @@ 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",
int(grafeaspb.Severity_MEDIUM),
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,
},
},
},
},
},
Expand Down
9 changes: 5 additions & 4 deletions lib/dockerregistry/types.go
View file
Expand Up @@ -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.
Expand Down

0 comments on commit 70c0096

Please sign in to comment.