From bba4b2a6a49506560196203236fdfdb52067c4f8 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Sun, 30 Jun 2019 17:21:25 +0800 Subject: [PATCH] Apply CVE white list in interceptor Interceptor will filter the vulnerability in whitelist while calculating the serverity of an image and determine whether or not to block client form pulling it. It will use the system level whitelist in this commit, another commit will switch to project level whitelist based on setting in a project. Signed-off-by: Daniel Jiang --- src/common/dao/scan_job.go | 5 +- src/common/models/cve_whitelist.go | 19 ++- src/common/models/cve_whitelist_test.go | 72 ++++++++++ src/common/models/scan_job.go | 36 ----- src/common/models/sev.go | 26 ++++ src/core/api/repository.go | 18 +-- src/core/api/utils.go | 33 ----- src/core/proxy/interceptors.go | 38 +++-- src/core/proxy/proxy.go | 10 +- src/pkg/scan/vuln.go | 136 ++++++++++++++++++ src/pkg/scan/vuln_test.go | 178 ++++++++++++++++++++++++ 11 files changed, 470 insertions(+), 101 deletions(-) create mode 100644 src/common/models/cve_whitelist_test.go create mode 100644 src/common/models/sev.go create mode 100644 src/pkg/scan/vuln.go create mode 100644 src/pkg/scan/vuln_test.go diff --git a/src/common/dao/scan_job.go b/src/common/dao/scan_job.go index 6aa151bc7..fe4aa6ab9 100644 --- a/src/common/dao/scan_job.go +++ b/src/common/dao/scan_job.go @@ -15,12 +15,11 @@ package dao import ( + "encoding/json" + "fmt" "github.com/astaxie/beego/orm" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/log" - - "encoding/json" - "fmt" "time" ) diff --git a/src/common/models/cve_whitelist.go b/src/common/models/cve_whitelist.go index 318bf7693..90badb372 100644 --- a/src/common/models/cve_whitelist.go +++ b/src/common/models/cve_whitelist.go @@ -33,6 +33,23 @@ type CVEWhitelistItem struct { } // TableName ... -func (r *CVEWhitelist) TableName() string { +func (c *CVEWhitelist) TableName() string { return "cve_whitelist" } + +// CVESet returns the set of CVE id of the items in the whitelist to help filter the vulnerability list +func (c *CVEWhitelist) CVESet() map[string]struct{} { + r := map[string]struct{}{} + for _, it := range c.Items { + r[it.CVEID] = struct{}{} + } + return r +} + +// IsExpired returns whether the whitelist is expired +func (c *CVEWhitelist) IsExpired() bool { + if c.ExpiresAt == nil { + return false + } + return time.Now().Unix() >= *c.ExpiresAt +} diff --git a/src/common/models/cve_whitelist_test.go b/src/common/models/cve_whitelist_test.go new file mode 100644 index 000000000..cb47e7021 --- /dev/null +++ b/src/common/models/cve_whitelist_test.go @@ -0,0 +1,72 @@ +// Copyright Project Harbor 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 models + +import ( + "github.com/stretchr/testify/assert" + "reflect" + "testing" + "time" +) + +func TestCVEWhitelist_All(t *testing.T) { + future := int64(4411494000) + now := time.Now().Unix() + cases := []struct { + input CVEWhitelist + cveset map[string]struct{} + expired bool + }{ + { + input: CVEWhitelist{ + ID: 1, + ProjectID: 0, + Items: []CVEWhitelistItem{}, + }, + cveset: map[string]struct{}{}, + expired: false, + }, + { + input: CVEWhitelist{ + ID: 1, + ProjectID: 0, + Items: []CVEWhitelistItem{}, + ExpiresAt: &now, + }, + cveset: map[string]struct{}{}, + expired: true, + }, + { + input: CVEWhitelist{ + ID: 2, + ProjectID: 3, + Items: []CVEWhitelistItem{ + {CVEID: "CVE-1999-0067"}, + {CVEID: "CVE-2016-7654321"}, + }, + ExpiresAt: &future, + }, + cveset: map[string]struct{}{ + "CVE-1999-0067": {}, + "CVE-2016-7654321": {}, + }, + expired: false, + }, + } + for _, c := range cases { + assert.Equal(t, c.expired, c.input.IsExpired()) + assert.True(t, reflect.DeepEqual(c.cveset, c.input.CVESet())) + } +} diff --git a/src/common/models/scan_job.go b/src/common/models/scan_job.go index 8a26fd741..75546223d 100644 --- a/src/common/models/scan_job.go +++ b/src/common/models/scan_job.go @@ -34,31 +34,6 @@ type ScanJob struct { UpdateTime time.Time `orm:"column(update_time);auto_now" json:"update_time"` } -// Severity represents the severity of a image/component in terms of vulnerability. -type Severity int64 - -// Sevxxx is the list of severity of image after scanning. -const ( - _ Severity = iota - SevNone - SevUnknown - SevLow - SevMedium - SevHigh -) - -// String is the output function for sererity variable -func (sev Severity) String() string { - name := []string{"negligible", "unknown", "low", "medium", "high"} - i := int64(sev) - switch { - case i >= 1 && i <= int64(SevHigh): - return name[i-1] - default: - return "unknown" - } -} - // TableName is required by by beego orm to map ScanJob to table img_scan_job func (s *ScanJob) TableName() string { return ScanJobTable @@ -101,17 +76,6 @@ type ImageScanReq struct { Tag string `json:"tag"` } -// VulnerabilityItem is an item in the vulnerability result returned by vulnerability details API. -type VulnerabilityItem struct { - ID string `json:"id"` - Severity Severity `json:"severity"` - Pkg string `json:"package"` - Version string `json:"version"` - Description string `json:"description"` - Link string `json:"link"` - Fixed string `json:"fixedVersion,omitempty"` -} - // ScanAllPolicy is represent the json request and object for scan all policy, the parm is het type ScanAllPolicy struct { Type string `json:"type"` diff --git a/src/common/models/sev.go b/src/common/models/sev.go new file mode 100644 index 000000000..3ccf89753 --- /dev/null +++ b/src/common/models/sev.go @@ -0,0 +1,26 @@ +package models + +// Severity represents the severity of a image/component in terms of vulnerability. +type Severity int64 + +// Sevxxx is the list of severity of image after scanning. +const ( + _ Severity = iota + SevNone + SevUnknown + SevLow + SevMedium + SevHigh +) + +// String is the output function for severity variable +func (sev Severity) String() string { + name := []string{"negligible", "unknown", "low", "medium", "high"} + i := int64(sev) + switch { + case i >= 1 && i <= int64(SevHigh): + return name[i-1] + default: + return "unknown" + } +} diff --git a/src/core/api/repository.go b/src/core/api/repository.go index d75c0dff3..6f6c63cb7 100644 --- a/src/core/api/repository.go +++ b/src/core/api/repository.go @@ -17,6 +17,7 @@ package api import ( "encoding/json" "fmt" + "github.com/goharbor/harbor/src/pkg/scan" "io/ioutil" "net/http" "sort" @@ -34,7 +35,6 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils" - "github.com/goharbor/harbor/src/common/utils/clair" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/common/utils/notary" "github.com/goharbor/harbor/src/common/utils/registry" @@ -1036,21 +1036,9 @@ func (ra *RepositoryAPI) VulnerabilityDetails() { ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } - res := []*models.VulnerabilityItem{} - overview, err := dao.GetImgScanOverview(digest) + res, err := scan.VulnListByDigest(digest) if err != nil { - ra.SendInternalServerError(fmt.Errorf("failed to get the scan overview, error: %v", err)) - return - } - if overview != nil && len(overview.DetailsKey) > 0 { - clairClient := clair.NewClient(config.ClairEndpoint(), nil) - log.Debugf("The key for getting details: %s", overview.DetailsKey) - details, err := clairClient.GetResult(overview.DetailsKey) - if err != nil { - ra.SendInternalServerError(fmt.Errorf("Failed to get scan details from Clair, error: %v", err)) - return - } - res = transformVulnerabilities(details) + log.Errorf("Failed to get vulnerability list for image: %s:%s", repository, tag) } ra.Data["json"] = res ra.ServeJSON() diff --git a/src/core/api/utils.go b/src/core/api/utils.go index ae7f30f7a..4b7305524 100644 --- a/src/core/api/utils.go +++ b/src/core/api/utils.go @@ -24,7 +24,6 @@ import ( commonhttp "github.com/goharbor/harbor/src/common/http" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils" - "github.com/goharbor/harbor/src/common/utils/clair" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/common/utils/registry" "github.com/goharbor/harbor/src/common/utils/registry/auth" @@ -279,35 +278,3 @@ func repositoryExist(name string, client *registry.Repository) (bool, error) { } return len(tags) != 0, nil } - -// transformVulnerabilities transforms the returned value of Clair API to a list of VulnerabilityItem -func transformVulnerabilities(layerWithVuln *models.ClairLayerEnvelope) []*models.VulnerabilityItem { - res := []*models.VulnerabilityItem{} - l := layerWithVuln.Layer - if l == nil { - return res - } - features := l.Features - if features == nil { - return res - } - for _, f := range features { - vulnerabilities := f.Vulnerabilities - if vulnerabilities == nil { - continue - } - for _, v := range vulnerabilities { - vItem := &models.VulnerabilityItem{ - ID: v.Name, - Pkg: f.Name, - Version: f.Version, - Severity: clair.ParseClairSev(v.Severity), - Fixed: v.FixedBy, - Link: v.Link, - Description: v.Description, - } - res = append(res, vItem) - } - } - return res -} diff --git a/src/core/proxy/interceptors.go b/src/core/proxy/interceptors.go index b8a3fe3b8..fb656edce 100644 --- a/src/core/proxy/interceptors.go +++ b/src/core/proxy/interceptors.go @@ -2,7 +2,6 @@ package proxy import ( "encoding/json" - "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/clair" @@ -11,6 +10,7 @@ import ( "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/promgr" coreutils "github.com/goharbor/harbor/src/core/utils" + "github.com/goharbor/harbor/src/pkg/scan" "context" "fmt" @@ -303,27 +303,41 @@ func (vh vulnerableHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) vh.next.ServeHTTP(rw, req) return } - overview, err := dao.GetImgScanOverview(img.digest) + // TODO: Get whitelist based on project setting + wl, err := dao.GetSysCVEWhitelist() if err != nil { - log.Errorf("failed to get ImgScanOverview with repo: %s, reference: %s, digest: %s. Error: %v", img.repository, img.reference, img.digest, err) - http.Error(rw, marshalError("PROJECT_POLICY_VIOLATION", "Failed to get ImgScanOverview."), http.StatusPreconditionFailed) + log.Errorf("Failed to get the whitelist, error: %v", err) + http.Error(rw, marshalError("PROJECT_POLICY_VIOLATION", "Failed to get CVE whitelist."), http.StatusPreconditionFailed) return } - // severity is 0 means that the image fails to scan or not scanned successfully. - if overview == nil || overview.Sev == 0 { - log.Debugf("cannot get the image scan overview info, failing the response.") - http.Error(rw, marshalError("PROJECT_POLICY_VIOLATION", "Cannot get the image severity."), http.StatusPreconditionFailed) + vl, err := scan.VulnListByDigest(img.digest) + if err != nil { + log.Errorf("Failed to get the vulnerability list, error: %v", err) + http.Error(rw, marshalError("PROJECT_POLICY_VIOLATION", "Failed to get vulnerabilities."), http.StatusPreconditionFailed) return } - imageSev := overview.Sev - if imageSev >= int(projectVulnerableSeverity) { - log.Debugf("the image severity: %q is higher then project setting: %q, failing the response.", models.Severity(imageSev), projectVulnerableSeverity) - http.Error(rw, marshalError("PROJECT_POLICY_VIOLATION", fmt.Sprintf("The severity of vulnerability of the image: %q is equal or higher than the threshold in project setting: %q.", models.Severity(imageSev), projectVulnerableSeverity)), http.StatusPreconditionFailed) + filtered := vl.ApplyWhitelist(*wl) + msg := vh.filterMsg(img, filtered) + log.Info(msg) + if int(vl.Severity()) >= int(projectVulnerableSeverity) { + log.Debugf("the image severity: %q is higher then project setting: %q, failing the response.", vl.Severity(), projectVulnerableSeverity) + http.Error(rw, marshalError("PROJECT_POLICY_VIOLATION", fmt.Sprintf("The severity of vulnerability of the image: %q is equal or higher than the threshold in project setting: %q.", vl.Severity(), projectVulnerableSeverity)), http.StatusPreconditionFailed) return } vh.next.ServeHTTP(rw, req) } +func (vh vulnerableHandler) filterMsg(img imageInfo, filtered scan.VulnerabilityList) string { + filterMsg := fmt.Sprintf("Image: %s/%s:%s, digest: %s, vulnerabilities fitered by whitelist:", img.projectName, img.repository, img.reference, img.digest) + if len(filtered) == 0 { + filterMsg = fmt.Sprintf("%s none.", filterMsg) + } + for _, v := range filtered { + filterMsg = fmt.Sprintf("%s ID: %s, severity: %s;", filterMsg, v.ID, v.Severity) + } + return filterMsg +} + func matchNotaryDigest(img imageInfo) (bool, error) { if NotaryEndpoint == "" { NotaryEndpoint = config.InternalNotaryEndpoint() diff --git a/src/core/proxy/proxy.go b/src/core/proxy/proxy.go index eadbfed38..bc0d7f44a 100644 --- a/src/core/proxy/proxy.go +++ b/src/core/proxy/proxy.go @@ -38,7 +38,15 @@ func Init(urls ...string) error { return err } Proxy = httputil.NewSingleHostReverseProxy(targetURL) - handlers = handlerChain{head: readonlyHandler{next: urlHandler{next: multipleManifestHandler{next: listReposHandler{next: contentTrustHandler{next: vulnerableHandler{next: Proxy}}}}}}} + handlers = handlerChain{ + head: readonlyHandler{ + next: urlHandler{ + next: multipleManifestHandler{ + next: listReposHandler{ + next: contentTrustHandler{ + next: vulnerableHandler{ + next: Proxy, + }}}}}}} return nil } diff --git a/src/pkg/scan/vuln.go b/src/pkg/scan/vuln.go new file mode 100644 index 000000000..a4ccac027 --- /dev/null +++ b/src/pkg/scan/vuln.go @@ -0,0 +1,136 @@ +// Copyright Project Harbor 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 scan + +import ( + "fmt" + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/utils/clair" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/core/config" + "reflect" +) + +// VulnerabilityItem represents a vulnerability reported by scanner +type VulnerabilityItem struct { + ID string `json:"id"` + Severity models.Severity `json:"severity"` + Pkg string `json:"package"` + Version string `json:"version"` + Description string `json:"description"` + Link string `json:"link"` + Fixed string `json:"fixedVersion,omitempty"` +} + +// VulnerabilityList is a list of vulnerabilities, which should be scanner-agnostic +type VulnerabilityList []VulnerabilityItem + +// ApplyWhitelist filters out the CVE defined in the whitelist in the parm. +// It returns the items that are filtered for the caller to track or log. +func (vl *VulnerabilityList) ApplyWhitelist(whitelist models.CVEWhitelist) VulnerabilityList { + filtered := VulnerabilityList{} + if whitelist.IsExpired() { + log.Info("The input whitelist is expired, skip filtering") + return filtered + } + s := whitelist.CVESet() + r := (*vl)[:0] + for _, v := range *vl { + if _, ok := s[v.ID]; ok { + log.Debugf("Filtered Vulnerability in whitelist, CVE ID: %s, severity: %s", v.ID, v.Severity) + filtered = append(filtered, v) + } else { + r = append(r, v) + } + } + val := reflect.ValueOf(vl) + val.Elem().SetLen(len(r)) + return filtered +} + +// Severity returns the highest severity of the vulnerabilities in the list +func (vl *VulnerabilityList) Severity() models.Severity { + s := models.SevNone + for _, v := range *vl { + if v.Severity > s { + s = v.Severity + } + } + return s +} + +// HasCVE returns whether the vulnerability list has the vulnerability with CVE ID in the parm +func (vl *VulnerabilityList) HasCVE(id string) bool { + for _, v := range *vl { + if v.ID == id { + return true + } + } + return false +} + +// VulnListFromClairResult transforms the returned value of Clair API to a VulnerabilityList +func VulnListFromClairResult(layerWithVuln *models.ClairLayerEnvelope) VulnerabilityList { + res := VulnerabilityList{} + if layerWithVuln == nil { + return res + } + l := layerWithVuln.Layer + if l == nil { + return res + } + features := l.Features + if features == nil { + return res + } + for _, f := range features { + vulnerabilities := f.Vulnerabilities + if vulnerabilities == nil { + continue + } + for _, v := range vulnerabilities { + vItem := VulnerabilityItem{ + ID: v.Name, + Pkg: f.Name, + Version: f.Version, + Severity: clair.ParseClairSev(v.Severity), + Fixed: v.FixedBy, + Link: v.Link, + Description: v.Description, + } + res = append(res, vItem) + } + } + return res +} + +// VulnListByDigest returns the VulnerabilityList based on the scan result of artifact with the digest in the parm +func VulnListByDigest(digest string) (VulnerabilityList, error) { + var res VulnerabilityList + overview, err := dao.GetImgScanOverview(digest) + if err != nil { + return res, err + } + if overview == nil || len(overview.DetailsKey) == 0 { + return res, fmt.Errorf("unable to get the scan result for digest: %s, the artifact is not scanned", digest) + } + c := clair.NewClient(config.ClairEndpoint(), nil) + clairRes, err := c.GetResult(overview.DetailsKey) + if err != nil { + return res, fmt.Errorf("failed to get scan result from Clair, error: %v", err) + } + return VulnListFromClairResult(clairRes), nil +} diff --git a/src/pkg/scan/vuln_test.go b/src/pkg/scan/vuln_test.go new file mode 100644 index 000000000..fce594cd9 --- /dev/null +++ b/src/pkg/scan/vuln_test.go @@ -0,0 +1,178 @@ +// Copyright Project Harbor 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 scan + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/stretchr/testify/assert" + "os" + "testing" +) + +var ( + past = int64(1561967574) + vulnList1 = VulnerabilityList{} + vulnList2 = VulnerabilityList{ + {ID: "CVE-2018-10754", + Severity: models.SevLow, + Pkg: "ncurses", + Version: "6.0+20161126-1+deb9u2", + }, + { + ID: "CVE-2018-6485", + Severity: models.SevHigh, + Pkg: "glibc", + Version: "2.24-11+deb9u4", + }, + } + whiteList1 = models.CVEWhitelist{ + ExpiresAt: &past, + Items: []models.CVEWhitelistItem{ + {CVEID: "CVE-2018-6485"}, + }, + } + whiteList2 = models.CVEWhitelist{ + Items: []models.CVEWhitelistItem{ + {CVEID: "CVE-2018-6485"}, + }, + } + whiteList3 = models.CVEWhitelist{ + Items: []models.CVEWhitelistItem{ + {CVEID: "CVE-2018-6485"}, + {CVEID: "CVE-2018-10754"}, + {CVEID: "CVE-2019-12817"}, + }, + } +) + +func TestMain(m *testing.M) { + dao.PrepareTestForPostgresSQL() + os.Exit(m.Run()) +} + +func TestVulnerabilityList_HasCVE(t *testing.T) { + cases := []struct { + input VulnerabilityList + cve string + result bool + }{ + { + input: vulnList1, + cve: "CVE-2018-10754", + result: false, + }, + { + input: vulnList2, + cve: "CVE-2018-10754", + result: true, + }, + } + for _, c := range cases { + assert.Equal(t, c.result, c.input.HasCVE(c.cve)) + } +} + +func TestVulnerabilityList_Severity(t *testing.T) { + cases := []struct { + input VulnerabilityList + expect models.Severity + }{ + { + input: vulnList1, + expect: models.SevNone, + }, { + input: vulnList2, + expect: models.SevHigh, + }, + } + for _, c := range cases { + assert.Equal(t, c.expect, c.input.Severity()) + } +} + +func TestVulnerabilityList_ApplyWhitelist(t *testing.T) { + cases := []struct { + vl VulnerabilityList + wl models.CVEWhitelist + expectFiltered VulnerabilityList + expectSev models.Severity + }{ + { + vl: vulnList2, + wl: whiteList1, + expectFiltered: VulnerabilityList{}, + expectSev: models.SevHigh, + }, + { + vl: vulnList2, + wl: whiteList2, + expectFiltered: VulnerabilityList{ + { + ID: "CVE-2018-6485", + Severity: models.SevHigh, + Pkg: "glibc", + Version: "2.24-11+deb9u4", + }, + }, + expectSev: models.SevLow, + }, + { + vl: vulnList1, + wl: whiteList3, + expectFiltered: VulnerabilityList{}, + expectSev: models.SevNone, + }, + { + vl: vulnList2, + wl: whiteList3, + expectFiltered: VulnerabilityList{ + {ID: "CVE-2018-10754", + Severity: models.SevLow, + Pkg: "ncurses", + Version: "6.0+20161126-1+deb9u2", + }, + { + ID: "CVE-2018-6485", + Severity: models.SevHigh, + Pkg: "glibc", + Version: "2.24-11+deb9u4", + }, + }, + expectSev: models.SevNone, + }, + } + for _, c := range cases { + filtered := c.vl.ApplyWhitelist(c.wl) + assert.Equal(t, c.expectFiltered, filtered) + assert.Equal(t, c.vl.Severity(), c.expectSev) + } +} + +func TestVulnListByDigest(t *testing.T) { + _, err := VulnListByDigest("notexist") + assert.NotNil(t, err) +} + +func TestVulnListFromClairResult(t *testing.T) { + l := VulnListFromClairResult(nil) + assert.Equal(t, VulnerabilityList{}, l) + lv := &models.ClairLayerEnvelope{ + Layer: nil, + Error: nil, + } + l2 := VulnListFromClairResult(lv) + assert.Equal(t, VulnerabilityList{}, l2) +}