From 58948dfb341e59ea46d6a549b2395cbab6d67f61 Mon Sep 17 00:00:00 2001 From: Seila Gamo Date: Thu, 18 Jul 2024 10:41:24 +0200 Subject: [PATCH] internal/report: detect unused exclusions --- cmd/lava/internal/scan/scan.go | 1 + internal/config/config.go | 2 + internal/report/human.tmpl | 34 ++++++++++ internal/report/humanprinter.go | 36 +++++++---- internal/report/humanprinter_test.go | 3 +- internal/report/jsonprinter.go | 2 +- internal/report/jsonprinter_test.go | 2 +- internal/report/report.go | 58 +++++++++++++---- internal/report/report_test.go | 93 ++++++++++++++++++++++++++++ 9 files changed, 205 insertions(+), 26 deletions(-) diff --git a/cmd/lava/internal/scan/scan.go b/cmd/lava/internal/scan/scan.go index fc4dc81..1f11f0d 100644 --- a/cmd/lava/internal/scan/scan.go +++ b/cmd/lava/internal/scan/scan.go @@ -36,6 +36,7 @@ that have been found. - 1: Command error - 2: Syntax error - 3: Check error + - 4: Not matching exclusions - 100: Informational vulnerabilities found - 101: Low severity vulnerabilities found - 102: Medium severity vulnerabilities found diff --git a/internal/config/config.go b/internal/config/config.go index 30b5aa8..2c6f3b7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -189,6 +189,8 @@ type ReportConfig struct { // instance, accepted risks, false positives, etc. Exclusions []Exclusion `yaml:"exclusions"` + ErrorOnNotMatchedExclusions bool `yaml:"error_not_matched_exclusions"` + // Metrics is the file where the metrics will be written. // If Metrics is an empty string or not specified in the yaml file, then // the metrics report is not saved. diff --git a/internal/report/human.tmpl b/internal/report/human.tmpl index 0e8a106..7990413 100644 --- a/internal/report/human.tmpl +++ b/internal/report/human.tmpl @@ -5,6 +5,9 @@ {{- if .Vulns}} {{template "vulns" . -}} {{end -}} +{{- if not .AllExclMatched}} +{{template "exclsNotMatched" . -}} +{{end -}} {{- end -}} @@ -162,6 +165,37 @@ Number of excluded vulnerabilities not included in the summary table: {{.Exclude {{end}}{{end}} {{- end -}} +{{- /* exclsNotMatched is the template used to render the details of a the not matched exclusions. */ -}} +{{define "exclsNotMatched" -}} +{{"The following exclusions don't match any finding:" | red }} +{{range $excl := .Exclusions}} + {{- if not $excl.Matched -}} + {{template "excl" .}} + {{end -}} +{{end}} +{{- end -}} + +{{- /* excl is the template used to render an exclusion. */ -}} +{{- define "excl" -}} +{{- if .Target}} +{{"Target: " | bold}}{{.Target | trim -}} +{{end -}} +{{- if .Description}} +{{"Description: " | bold}}{{.Description | trim -}} +{{end -}} +{{- if .Resource}} +{{"Resource: " | bold}}{{.Resource | trim -}} +{{end -}} +{{- if .Fingerprint}} +{{"Fingerprint: " | bold}}{{.Fingerprint | trim -}} +{{end -}} +{{- if .Summary}} +{{"Summary: " | bold}}{{.Summary | trim -}} +{{end -}} +{{- if not .ExpirationDate.IsZero}} +{{"Expiration Date: " | bold}}{{.ExpirationDate.String | trim -}} +{{end -}} +{{- end -}} {{- /* Render the report. */ -}} {{- template "report" . -}} diff --git a/internal/report/humanprinter.go b/internal/report/humanprinter.go index 07ec66a..001a019 100644 --- a/internal/report/humanprinter.go +++ b/internal/report/humanprinter.go @@ -40,7 +40,8 @@ var ( ) // Print renders the scan results in a human-readable format. -func (prn humanPrinter) Print(w io.Writer, vulns []vulnerability, summ summary, status []checkStatus) error { +func (prn humanPrinter) Print(w io.Writer, vulns []vulnerability, summ summary, + status []checkStatus, exclusions []reportExclusion) error { // count the total non-excluded vulnerabilities found. var total int for _, ss := range summ.count { @@ -52,18 +53,31 @@ func (prn humanPrinter) Print(w io.Writer, vulns []vulnerability, summ summary, stats[s.String()] = summ.count[s] } + // check if there are exclusions not matched. + var allMatched = true + for _, e := range exclusions { + if !e.Matched { + allMatched = false + break + } + } + data := struct { - Stats map[string]int - Total int - Excluded int - Vulns []vulnerability - Status []checkStatus + Stats map[string]int + Total int + Excluded int + Vulns []vulnerability + Status []checkStatus + AllExclMatched bool + Exclusions []reportExclusion }{ - Stats: stats, - Total: total, - Excluded: summ.excluded, - Vulns: vulns, - Status: status, + Stats: stats, + Total: total, + Excluded: summ.excluded, + Vulns: vulns, + Status: status, + AllExclMatched: allMatched, + Exclusions: exclusions, } if err := humanTmpl.Execute(w, data); err != nil { diff --git a/internal/report/humanprinter_test.go b/internal/report/humanprinter_test.go index 42fd092..9a42d62 100644 --- a/internal/report/humanprinter_test.go +++ b/internal/report/humanprinter_test.go @@ -18,6 +18,7 @@ func TestUserFriendlyPrinter_Print(t *testing.T) { vulnerabilities []vulnerability summ summary status []checkStatus + exclusions []reportExclusion want []string }{ { @@ -193,7 +194,7 @@ func TestUserFriendlyPrinter_Print(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var buf bytes.Buffer w := humanPrinter{} - if err := w.Print(&buf, tt.vulnerabilities, tt.summ, tt.status); err != nil { + if err := w.Print(&buf, tt.vulnerabilities, tt.summ, tt.status, tt.exclusions); err != nil { t.Errorf("unexpected error value: %v", err) } text := buf.String() diff --git a/internal/report/jsonprinter.go b/internal/report/jsonprinter.go index 42061a9..6ce1b47 100644 --- a/internal/report/jsonprinter.go +++ b/internal/report/jsonprinter.go @@ -12,7 +12,7 @@ import ( type jsonPrinter struct{} // Print renders the scan results in JSON format. -func (prn jsonPrinter) Print(w io.Writer, vulns []vulnerability, _ summary, _ []checkStatus) error { +func (prn jsonPrinter) Print(w io.Writer, vulns []vulnerability, _ summary, _ []checkStatus, _ []reportExclusion) error { enc := json.NewEncoder(w) enc.SetIndent("", " ") if err := enc.Encode(vulns); err != nil { diff --git a/internal/report/jsonprinter_test.go b/internal/report/jsonprinter_test.go index cc034be..11d502e 100644 --- a/internal/report/jsonprinter_test.go +++ b/internal/report/jsonprinter_test.go @@ -163,7 +163,7 @@ func TestJsonPrinter_Print(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var buf bytes.Buffer w := jsonPrinter{} - err := w.Print(&buf, tt.vulnerabilities, summary{}, nil) + err := w.Print(&buf, tt.vulnerabilities, summary{}, nil, nil) if (err == nil) != tt.wantNilErr { t.Errorf("unexpected error value: %v", err) } diff --git a/internal/report/report.go b/internal/report/report.go index 2c532e9..c4ddc18 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "log/slog" "os" "regexp" "slices" @@ -28,7 +29,13 @@ type Writer struct { isStdout bool minSeverity config.Severity showSeverity config.Severity - exclusions []config.Exclusion + exclusions []reportExclusion + eome bool +} + +type reportExclusion struct { + config.Exclusion + Matched bool } // timeNow is set by tests to mock the current time. @@ -64,13 +71,19 @@ func NewWriter(cfg config.ReportConfig) (Writer, error) { showSeverity = cfg.Severity } + var exclusions []reportExclusion + for _, e := range cfg.Exclusions { + exclusions = append(exclusions, reportExclusion{Exclusion: e, Matched: false}) + } + return Writer{ prn: prn, w: w, isStdout: isStdout, minSeverity: cfg.Severity, showSeverity: showSeverity, - exclusions: cfg.Exclusions, + exclusions: exclusions, + eome: cfg.ErrorOnNotMatchedExclusions, }, nil } @@ -96,7 +109,7 @@ func (writer Writer) Write(er engine.Report) (ExitCode, error) { status := mkStatus(er) exitCode := writer.calculateExitCode(summ, status) - if err = writer.prn.Print(writer.w, fvulns, summ, status); err != nil { + if err = writer.prn.Print(writer.w, fvulns, summ, status, writer.exclusions); err != nil { return exitCode, fmt.Errorf("print report: %w", err) } @@ -140,8 +153,8 @@ func (writer Writer) parseReport(er engine.Report) ([]vulnerability, error) { // isExcluded returns whether the provided [report.Vulnerability] is // excluded based on the [Writer] configuration and the affected target. -func (writer Writer) isExcluded(v report.Vulnerability, target string) (bool, error) { - for _, excl := range writer.exclusions { +func (writer *Writer) isExcluded(v report.Vulnerability, target string) (bool, error) { + for i, excl := range writer.exclusions { if !excl.ExpirationDate.IsZero() && excl.ExpirationDate.Before(timeNow()) { continue } @@ -183,6 +196,8 @@ func (writer Writer) isExcluded(v report.Vulnerability, target string) (bool, er continue } } + + writer.exclusions[i].Matched = true return true, nil } return false, nil @@ -224,6 +239,24 @@ func (writer Writer) calculateExitCode(summ summary, status []checkStatus) ExitC } } + var notMatched bool + // Log all the not-matched exclusions. + for i, e := range writer.exclusions { + if !e.Matched { + notMatched = true + var loggerLevel func(msg string, args ...any) + if writer.eome { + loggerLevel = slog.Error + } else { + loggerLevel = slog.Warn + } + loggerLevel("The exclusion doesn't much with any finding", "index", i) + } + } + if writer.eome && notMatched { + return ExitCodeNotMatchedExclusions + } + for sev := config.SeverityCritical; sev >= writer.minSeverity; sev-- { if summ.count[sev] > 0 { diff := sev - config.SeverityInfo @@ -243,7 +276,7 @@ type vulnerability struct { // A printer renders a Vulcan report in a specific format. type printer interface { - Print(w io.Writer, vulns []vulnerability, summ summary, status []checkStatus) error + Print(w io.Writer, vulns []vulnerability, summ summary, status []checkStatus, exclusions []reportExclusion) error } // scoreToSeverity converts a CVSS score into a [config.Severity]. @@ -324,10 +357,11 @@ type ExitCode int // Exit codes depending on the maximum severity found. const ( - ExitCodeCheckError ExitCode = 3 - ExitCodeInfo ExitCode = 100 - ExitCodeLow ExitCode = 101 - ExitCodeMedium ExitCode = 102 - ExitCodeHigh ExitCode = 103 - ExitCodeCritical ExitCode = 104 + ExitCodeCheckError ExitCode = 3 + ExitCodeNotMatchedExclusions ExitCode = 4 + ExitCodeInfo ExitCode = 100 + ExitCodeLow ExitCode = 101 + ExitCodeMedium ExitCode = 102 + ExitCodeHigh ExitCode = 103 + ExitCodeCritical ExitCode = 104 ) diff --git a/internal/report/report_test.go b/internal/report/report_test.go index b8189a2..0eec580 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -210,6 +210,63 @@ func TestWriter_calculateExitCode(t *testing.T) { }, want: ExitCodeCheckError, }, + { + name: "not matched exclusions (warn)", + summ: summary{ + count: map[config.Severity]int{ + config.SeverityCritical: 0, + config.SeverityHigh: 0, + config.SeverityMedium: 1, + config.SeverityLow: 1, + config.SeverityInfo: 1, + }, + }, + status: []checkStatus{ + { + Checktype: "Checktype1", + Target: "Target1", + Status: "FINISHED", + }, + }, + rConfig: config.ReportConfig{ + Severity: config.SeverityHigh, + Exclusions: []config.Exclusion{ + { + Summary: "Unused exclusion", + }, + }, + }, + want: 0, + }, + { + name: "not matched exclusions (error)", + summ: summary{ + count: map[config.Severity]int{ + config.SeverityCritical: 0, + config.SeverityHigh: 0, + config.SeverityMedium: 1, + config.SeverityLow: 1, + config.SeverityInfo: 1, + }, + }, + status: []checkStatus{ + { + Checktype: "Checktype1", + Target: "Target1", + Status: "FINISHED", + }, + }, + rConfig: config.ReportConfig{ + Severity: config.SeverityHigh, + ErrorOnNotMatchedExclusions: true, + Exclusions: []config.Exclusion{ + { + Summary: "Unused exclusion", + }, + }, + }, + want: 4, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -694,6 +751,42 @@ func TestWriter_isExcluded(t *testing.T) { } } +func TestWriter_isExcluded_exclusionMatched(t *testing.T) { + v := vreport.Vulnerability{ + Summary: "Vulnerability Summary 1", + Score: 6.7, + } + target := "." + rConfig := config.ReportConfig{ + Exclusions: []config.Exclusion{ + { + Summary: "Summary 1", + Description: "Excluded vulnerabilities Summary 1", + }, + { + Summary: "Unused exclusion", + }, + }, + } + wantMatches := []bool{true, false} + w, err := NewWriter(rConfig) + if err != nil { + t.Fatalf("unable to create a report writer: %v", err) + } + got, err := w.isExcluded(v, target) + if err != nil { + t.Errorf("unexpected error value: %v", err) + } + if !got { + t.Errorf("unexpected excluded value: got: %v, want: %v", got, true) + } + for i, want := range wantMatches { + if w.exclusions[i].Matched != want { + t.Errorf("unexpected matched value: got: %v, want: %v", w.exclusions[i].Matched, want) + } + } +} + func TestMkSummary(t *testing.T) { tests := []struct { name string