From 5f9fff3b20ce7eb933978c7a4f9391cb2c9b3d89 Mon Sep 17 00:00:00 2001 From: laurentsimon <64505099+laurentsimon@users.noreply.github.com> Date: Wed, 26 Jan 2022 12:45:39 -0800 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Separate=20check=20from=20policies?= =?UTF-8?q?=20for=20the=20Vulnerabilities=20check=20(#1532)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * raw vulnerabilities seperation * update year * missing files * tests --- checker/raw_result.go | 16 ++++++++ checks/evaluation/vulnerabilities.go | 53 ++++++++++++++++++++++++ checks/raw/vulnerabilities.go | 60 ++++++++++++++++++++++++++++ checks/vulnerabilities.go | 55 +++++++------------------ checks/vulnerabilities_test.go | 2 +- e2e/vulnerabilities_test.go | 6 +-- pkg/json_raw_results.go | 25 ++++++++++++ 7 files changed, 173 insertions(+), 44 deletions(-) create mode 100644 checks/evaluation/vulnerabilities.go create mode 100644 checks/raw/vulnerabilities.go diff --git a/checker/raw_result.go b/checker/raw_result.go index 63350d76..3ffa5503 100644 --- a/checker/raw_result.go +++ b/checker/raw_result.go @@ -17,12 +17,19 @@ package checker // RawResults contains results before a policy // is applied. type RawResults struct { + VulnerabilitiesResults VulnerabilitiesData BinaryArtifactResults BinaryArtifactData SecurityPolicyResults SecurityPolicyData DependencyUpdateToolResults DependencyUpdateToolData BranchProtectionResults BranchProtectionsData } +// VulnerabilitiesData contains the raw results +// for the Vulnerabilities check. +type VulnerabilitiesData struct { + Vulnerabilities []Vulnerability +} + // SecurityPolicyData contains the raw results // for the Security-Policy check. type SecurityPolicyData struct { @@ -111,3 +118,12 @@ type File struct { Type FileType // Type of file. // TODO: add hash. } + +// Vulnerability defines a vulnerability +// from a database. +type Vulnerability struct { + // For OSV: OSV-2020-484 + // For CVE: CVE-2022-23945 + ID string + // TODO(vuln): Add additional fields, if needed. +} diff --git a/checks/evaluation/vulnerabilities.go b/checks/evaluation/vulnerabilities.go new file mode 100644 index 00000000..7d53ce19 --- /dev/null +++ b/checks/evaluation/vulnerabilities.go @@ -0,0 +1,53 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package evaluation + +import ( + "fmt" + "strings" + + "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" +) + +// Vulnerabilities applies the score policy for the Vulnerabilities check. +func Vulnerabilities(name string, dl checker.DetailLogger, + r *checker.VulnerabilitiesData) checker.CheckResult { + if r == nil { + e := sce.WithMessage(sce.ErrScorecardInternal, "empty raw data") + return checker.CreateRuntimeErrorResult(name, e) + } + + score := checker.MaxResultScore + IDs := []string{} + for _, vuln := range r.Vulnerabilities { + IDs = append(IDs, vuln.ID) + score-- + } + + if score < 0 { + score = 0 + } + + if len(IDs) > 0 { + dl.Warn3(&checker.LogMessage{ + Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(IDs, ", ")), + }) + return checker.CreateResultWithScore(name, + fmt.Sprintf("%v existing vulnerabilities detected", len(IDs)), score) + } + + return checker.CreateMaxScoreResult(name, "no vulnerabilities detected") +} diff --git a/checks/raw/vulnerabilities.go b/checks/raw/vulnerabilities.go new file mode 100644 index 00000000..61f90b04 --- /dev/null +++ b/checks/raw/vulnerabilities.go @@ -0,0 +1,60 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package raw + +import ( + "github.com/ossf/scorecard/v4/checker" + "github.com/ossf/scorecard/v4/clients" + sce "github.com/ossf/scorecard/v4/errors" +) + +// Vulnerabilities retrieves the raw data for the Vulnerabilities check. +func Vulnerabilities(c *checker.CheckRequest) (checker.VulnerabilitiesData, error) { + commits, err := c.RepoClient.ListCommits() + if err != nil { + return checker.VulnerabilitiesData{}, + sce.WithMessage(sce.ErrScorecardInternal, "Client.Repositories.ListCommits") + } + + if len(commits) < 1 || commits[0].SHA == "" { + return checker.VulnerabilitiesData{}, + sce.WithMessage(sce.ErrScorecardInternal, "no commits found") + } + + resp, err := c.VulnerabilitiesClient.HasUnfixedVulnerabilities(c.Ctx, commits[0].SHA) + if err != nil { + return checker.VulnerabilitiesData{}, + sce.WithMessage(sce.ErrScorecardInternal, "VulnerabilitiesClient.HasUnfixedVulnerabilities") + } + + vulnIDs := getVulnerabilities(&resp) + vulns := []checker.Vulnerability{} + for _, id := range vulnIDs { + v := checker.Vulnerability{ + ID: id, + // Note: add fields if needed. + } + vulns = append(vulns, v) + } + return checker.VulnerabilitiesData{Vulnerabilities: vulns}, nil +} + +func getVulnerabilities(resp *clients.VulnerabilitiesResponse) []string { + ids := make([]string, 0, len(resp.Vulns)) + for _, vuln := range resp.Vulns { + ids = append(ids, vuln.ID) + } + return ids +} diff --git a/checks/vulnerabilities.go b/checks/vulnerabilities.go index 97222ef1..9a16e414 100644 --- a/checks/vulnerabilities.go +++ b/checks/vulnerabilities.go @@ -1,4 +1,4 @@ -// Copyright 2021 Security Scorecard Authors +// Copyright 2022 Security Scorecard Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,61 +15,36 @@ package checks import ( - "fmt" - "strings" - "github.com/ossf/scorecard/v4/checker" - "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/checks/evaluation" + "github.com/ossf/scorecard/v4/checks/raw" sce "github.com/ossf/scorecard/v4/errors" ) -const ( - // CheckVulnerabilities is the registered name for the OSV check. - CheckVulnerabilities = "Vulnerabilities" -) +// CheckVulnerabilities is the registered name for the OSV check. +const CheckVulnerabilities = "Vulnerabilities" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckVulnerabilities, HasUnfixedVulnerabilities); err != nil { + if err := registerCheck(CheckVulnerabilities, Vulnerabilities); err != nil { // this should never happen panic(err) } } -func getVulnerabilities(resp *clients.VulnerabilitiesResponse) []string { - ids := make([]string, 0, len(resp.Vulns)) - for _, vuln := range resp.Vulns { - ids = append(ids, vuln.ID) - } - return ids -} - -// HasUnfixedVulnerabilities runs Vulnerabilities check. -func HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.CheckResult { - commits, err := c.RepoClient.ListCommits() +// Vulnerabilities runs Vulnerabilities check. +func Vulnerabilities(c *checker.CheckRequest) checker.CheckResult { + rawData, err := raw.Vulnerabilities(c) if err != nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "Client.Repositories.ListCommits") + e := sce.WithMessage(sce.ErrScorecardInternal, err.Error()) return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e) } - if len(commits) < 1 || commits[0].SHA == "" { - return checker.CreateInconclusiveResult(CheckVulnerabilities, "no commits found") + // Set the raw results. + if c.RawResults != nil { + c.RawResults.VulnerabilitiesResults = rawData + return checker.CheckResult{} } - resp, err := c.VulnerabilitiesClient.HasUnfixedVulnerabilities(c.Ctx, commits[0].SHA) - if err != nil { - e := sce.WithMessage(sce.ErrScorecardInternal, "VulnerabilitiesClient.HasUnfixedVulnerabilities") - return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e) - } - - // TODO: take severity into account. - vulnIDs := getVulnerabilities(&resp) - if len(vulnIDs) > 0 { - c.Dlogger.Warn3(&checker.LogMessage{ - Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(vulnIDs, ", ")), - }) - return checker.CreateMinScoreResult(CheckVulnerabilities, "existing vulnerabilities detected") - } - - return checker.CreateMaxScoreResult(CheckVulnerabilities, "no vulnerabilities detected") + return evaluation.Vulnerabilities(CheckVulnerabilities, c.Dlogger, &rawData) } diff --git a/checks/vulnerabilities_test.go b/checks/vulnerabilities_test.go index 0bf642fd..f0cee6c9 100644 --- a/checks/vulnerabilities_test.go +++ b/checks/vulnerabilities_test.go @@ -66,7 +66,7 @@ func TestVulnerabilities(t *testing.T) { Ctx: context.TODO(), VulnerabilitiesClient: mockVulnClient, } - res := HasUnfixedVulnerabilities(&req) + res := Vulnerabilities(&req) if !tt.isError && res.Error != nil { t.Fail() } else if tt.isError && res.Error == nil { diff --git a/e2e/vulnerabilities_test.go b/e2e/vulnerabilities_test.go index 68dd0f8d..42c1776a 100644 --- a/e2e/vulnerabilities_test.go +++ b/e2e/vulnerabilities_test.go @@ -52,7 +52,7 @@ var _ = Describe("E2E TEST:Vulnerabilities", func() { NumberOfDebug: 0, } - result := checks.HasUnfixedVulnerabilities(&req) + result := checks.Vulnerabilities(&req) // UPGRADEv2: to remove. // Old version. Expect(result.Error).Should(BeNil()) @@ -79,12 +79,12 @@ var _ = Describe("E2E TEST:Vulnerabilities", func() { } expected := scut.TestReturn{ Error: nil, - Score: checker.MinResultScore, + Score: checker.MaxResultScore - 3, // 3 vulnerabilities remove 3 points. NumberOfWarn: 1, NumberOfInfo: 0, NumberOfDebug: 0, } - result := checks.HasUnfixedVulnerabilities(&checkRequest) + result := checks.Vulnerabilities(&checkRequest) // UPGRADEv2: to remove. // Old version. Expect(result.Error).Should(BeNil()) diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 6c94320e..8ad40bc0 100644 --- a/pkg/json_raw_results.go +++ b/pkg/json_raw_results.go @@ -65,6 +65,7 @@ type jsonBranchProtection struct { } type jsonRawResults struct { + DatabaseVulnerabilities []jsonDatabaseVulnerability `json:"database-vulnerabilities"` // List of binaries found in the repo. Binaries []jsonFile `json:"binaries"` // List of security policy files found in the repo. @@ -77,6 +78,25 @@ type jsonRawResults struct { BranchProtections []jsonBranchProtection `json:"branch-protections"` } +type jsonDatabaseVulnerability struct { + // For OSV: OSV-2020-484 + // For CVE: CVE-2022-23945 + ID string + // TODO: additional information +} + +//nolint:unparam +func (r *jsonScorecardRawResult) addVulnerbilitiesRawResults(vd *checker.VulnerabilitiesData) error { + r.Results.DatabaseVulnerabilities = []jsonDatabaseVulnerability{} + for _, v := range vd.Vulnerabilities { + r.Results.DatabaseVulnerabilities = append(r.Results.DatabaseVulnerabilities, + jsonDatabaseVulnerability{ + ID: v.ID, + }) + } + return nil +} + //nolint:unparam func (r *jsonScorecardRawResult) addBinaryArtifactRawResults(ba *checker.BinaryArtifactData) error { r.Results.Binaries = []jsonFile{} @@ -148,6 +168,11 @@ func (r *jsonScorecardRawResult) addBranchProtectionRawResults(bp *checker.Branc } func (r *jsonScorecardRawResult) fillJSONRawResults(raw *checker.RawResults) error { + // Vulnerabiliries. + if err := r.addVulnerbilitiesRawResults(&raw.VulnerabilitiesResults); err != nil { + return sce.WithMessage(sce.ErrScorecardInternal, err.Error()) + } + // Binary-Artifacts. if err := r.addBinaryArtifactRawResults(&raw.BinaryArtifactResults); err != nil { return sce.WithMessage(sce.ErrScorecardInternal, err.Error())