From 8f5f0031c7e503cf2674afd135d2e964f24627e6 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Fri, 5 Jul 2019 20:05:52 +0800 Subject: [PATCH] Enable project level CVE whitelist This commit update the project API to support "reuse_sys_cve_whitelist" setting in project metadata and "cve_whitelist" in project request. Also modify the interceptor to support project level CVE whitelist if the reuse flag is false. Signed-off-by: Daniel Jiang --- docs/swagger.yaml | 3 + src/common/models/pro_meta.go | 21 ++--- src/common/models/project.go | 17 +++- src/core/api/project.go | 4 +- src/core/api/sys_cve_whitelist.go | 13 ++- src/core/api/sys_cve_whitelist_test.go | 16 ++++ src/core/promgr/promgr.go | 73 +++++++++------- src/core/proxy/interceptor_test.go | 10 ++- src/core/proxy/interceptors.go | 35 +++++--- src/pkg/scan/whitelist/manager.go | 79 ++++++++++++++++++ src/pkg/scan/whitelist/validator.go | 60 +++++++++++++ src/pkg/scan/whitelist/validator_test.go | 102 +++++++++++++++++++++++ 12 files changed, 370 insertions(+), 63 deletions(-) create mode 100644 src/pkg/scan/whitelist/manager.go create mode 100644 src/pkg/scan/whitelist/validator.go create mode 100644 src/pkg/scan/whitelist/validator_test.go diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 804d4a44c..f3fdd32c7 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -3639,6 +3639,9 @@ definitions: metadata: description: The metadata of the project. $ref: '#/definitions/ProjectMetadata' + cve_whitelist: + description: The CVE whitelist of this project. + $ref: '#/definitions/CVEWhitelist' ProjectMetadata: type: object properties: diff --git a/src/common/models/pro_meta.go b/src/common/models/pro_meta.go index 97427ac6a..d9952714c 100644 --- a/src/common/models/pro_meta.go +++ b/src/common/models/pro_meta.go @@ -20,16 +20,17 @@ import ( // keys of project metadata and severity values const ( - ProMetaPublic = "public" - ProMetaEnableContentTrust = "enable_content_trust" - ProMetaPreventVul = "prevent_vul" // prevent vulnerable images from being pulled - ProMetaSeverity = "severity" - ProMetaAutoScan = "auto_scan" - SeverityNone = "negligible" - SeverityLow = "low" - SeverityMedium = "medium" - SeverityHigh = "high" - SeverityCritical = "critical" + ProMetaPublic = "public" + ProMetaEnableContentTrust = "enable_content_trust" + ProMetaPreventVul = "prevent_vul" // prevent vulnerable images from being pulled + ProMetaSeverity = "severity" + ProMetaAutoScan = "auto_scan" + ProMetaReuseSysCVEWhitelist = "reuse_sys_cve_whitelist" + SeverityNone = "negligible" + SeverityLow = "low" + SeverityMedium = "medium" + SeverityHigh = "high" + SeverityCritical = "critical" ) // ProjectMetadata holds the metadata of a project. diff --git a/src/common/models/project.go b/src/common/models/project.go index bebadcdd1..e6030a3d5 100644 --- a/src/common/models/project.go +++ b/src/common/models/project.go @@ -36,6 +36,7 @@ type Project struct { RepoCount int64 `orm:"-" json:"repo_count"` ChartCount uint64 `orm:"-" json:"chart_count"` Metadata map[string]string `orm:"-" json:"metadata"` + CVEWhitelist CVEWhitelist `orm:"-" json:"cve_whitelist"` } // GetMetadata ... @@ -83,6 +84,15 @@ func (p *Project) VulPrevented() bool { return isTrue(prevent) } +// ReuseSysCVEWhitelist ... +func (p *Project) ReuseSysCVEWhitelist() bool { + r, ok := p.GetMetadata(ProMetaReuseSysCVEWhitelist) + if !ok { + return true + } + return isTrue(r) +} + // Severity ... func (p *Project) Severity() string { severity, exist := p.GetMetadata(ProMetaSeverity) @@ -154,9 +164,10 @@ type BaseProjectCollection struct { // ProjectRequest holds informations that need for creating project API type ProjectRequest struct { - Name string `json:"project_name"` - Public *int `json:"public"` // deprecated, reserved for project creation in replication - Metadata map[string]string `json:"metadata"` + Name string `json:"project_name"` + Public *int `json:"public"` // deprecated, reserved for project creation in replication + Metadata map[string]string `json:"metadata"` + CVEWhitelist CVEWhitelist `json:"cve_whitelist"` } // ProjectQueryResult ... diff --git a/src/core/api/project.go b/src/core/api/project.go index aaf0f2e02..702a035ea 100644 --- a/src/core/api/project.go +++ b/src/core/api/project.go @@ -158,6 +158,7 @@ func (p *ProjectAPI) Post() { if _, ok := pro.Metadata[models.ProMetaPublic]; !ok { pro.Metadata[models.ProMetaPublic] = strconv.FormatBool(false) } + // populate owner := p.SecurityCtx.GetUsername() // set the owner as the system admin when the API being called by replication @@ -460,7 +461,8 @@ func (p *ProjectAPI) Put() { if err := p.ProjectMgr.Update(p.project.ProjectID, &models.Project{ - Metadata: req.Metadata, + Metadata: req.Metadata, + CVEWhitelist: req.CVEWhitelist, }); err != nil { p.ParseAndHandleError(fmt.Sprintf("failed to update project %d", p.project.ProjectID), err) diff --git a/src/core/api/sys_cve_whitelist.go b/src/core/api/sys_cve_whitelist.go index 3185d8e34..921abb83f 100644 --- a/src/core/api/sys_cve_whitelist.go +++ b/src/core/api/sys_cve_whitelist.go @@ -17,15 +17,16 @@ package api import ( "errors" "fmt" - "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/pkg/scan/whitelist" "net/http" ) // SysCVEWhitelistAPI Handles the requests to manage system level CVE whitelist type SysCVEWhitelistAPI struct { BaseController + manager whitelist.Manager } // Prepare validates the request initially @@ -41,11 +42,12 @@ func (sca *SysCVEWhitelistAPI) Prepare() { sca.SendForbiddenError(errors.New(msg)) return } + sca.manager = whitelist.NewDefaultManager() } // Get handles the GET request to retrieve the system level CVE whitelist func (sca *SysCVEWhitelistAPI) Get() { - l, err := dao.GetSysCVEWhitelist() + l, err := sca.manager.GetSys() if err != nil { sca.SendInternalServerError(err) return @@ -67,7 +69,12 @@ func (sca *SysCVEWhitelistAPI) Put() { sca.SendBadRequestError(errors.New(msg)) return } - if _, err := dao.UpdateCVEWhitelist(l); err != nil { + if err := sca.manager.SetSys(l); err != nil { + if whitelist.IsInvalidErr(err) { + log.Errorf("Invalid CVE whitelist: %v", err) + sca.SendBadRequestError(err) + return + } sca.SendInternalServerError(err) return } diff --git a/src/core/api/sys_cve_whitelist_test.go b/src/core/api/sys_cve_whitelist_test.go index 0cbd648e7..d484b79f2 100644 --- a/src/core/api/sys_cve_whitelist_test.go +++ b/src/core/api/sys_cve_whitelist_test.go @@ -90,6 +90,22 @@ func TestSysCVEWhitelistAPIPut(t *testing.T) { }, code: http.StatusBadRequest, }, + // 400 + { + request: &testingRequest{ + method: http.MethodPut, + url: url, + bodyJSON: models.CVEWhitelist{ + ExpiresAt: &s, + Items: []models.CVEWhitelistItem{ + {CVEID: "CVE-2019-12310"}, + {CVEID: "CVE-2019-12310"}, + }, + }, + credential: sysAdmin, + }, + code: http.StatusBadRequest, + }, // 200 { request: &testingRequest{ diff --git a/src/core/promgr/promgr.go b/src/core/promgr/promgr.go index b2de26ff3..5a0a9bc12 100644 --- a/src/core/promgr/promgr.go +++ b/src/core/promgr/promgr.go @@ -16,6 +16,7 @@ package promgr import ( "fmt" + "github.com/goharbor/harbor/src/pkg/scan/whitelist" "strconv" "github.com/goharbor/harbor/src/common/models" @@ -44,6 +45,7 @@ type defaultProjectManager struct { pmsDriver pmsdriver.PMSDriver metaMgrEnabled bool // if metaMgrEnabled is enabled, metaMgr will be used to CURD metadata metaMgr metamgr.ProjectMetadataManager + whitelistMgr whitelist.Manager } // NewDefaultProjectManager returns an instance of defaultProjectManager, @@ -56,6 +58,7 @@ func NewDefaultProjectManager(driver pmsdriver.PMSDriver, metaMgrEnabled bool) P } if metaMgrEnabled { mgr.metaMgr = metamgr.NewDefaultProjectMetadataManager() + mgr.whitelistMgr = whitelist.NewDefaultManager() } return mgr } @@ -77,6 +80,11 @@ func (d *defaultProjectManager) Get(projectIDOrName interface{}) (*models.Projec for k, v := range meta { project.Metadata[k] = v } + wl, err := d.whitelistMgr.Get(project.ProjectID) + if err != nil { + return nil, err + } + project.CVEWhitelist = *wl } return project, nil } @@ -85,9 +93,12 @@ func (d *defaultProjectManager) Create(project *models.Project) (int64, error) { if err != nil { return 0, err } - if len(project.Metadata) > 0 && d.metaMgrEnabled { - if err = d.metaMgr.Add(id, project.Metadata); err != nil { - log.Errorf("failed to add metadata for project %s: %v", project.Name, err) + if d.metaMgrEnabled { + d.whitelistMgr.CreateEmpty(project.ProjectID) + if len(project.Metadata) > 0 { + if err = d.metaMgr.Add(id, project.Metadata); err != nil { + log.Errorf("failed to add metadata for project %s: %v", project.Name, err) + } } } return id, nil @@ -110,37 +121,40 @@ func (d *defaultProjectManager) Delete(projectIDOrName interface{}) error { } func (d *defaultProjectManager) Update(projectIDOrName interface{}, project *models.Project) error { - if len(project.Metadata) > 0 && d.metaMgrEnabled { - pro, err := d.Get(projectIDOrName) - if err != nil { + pro, err := d.Get(projectIDOrName) + if err != nil { + return err + } + if pro == nil { + return fmt.Errorf("project %v not found", projectIDOrName) + } + // TODO transaction? + if d.metaMgrEnabled { + if err := d.whitelistMgr.Set(pro.ProjectID, project.CVEWhitelist); err != nil { return err } - if pro == nil { - return fmt.Errorf("project %v not found", projectIDOrName) - } - - // TODO transaction? - metaNeedUpdated := map[string]string{} - metaNeedCreated := map[string]string{} - if pro.Metadata == nil { - pro.Metadata = map[string]string{} - } - for key, value := range project.Metadata { - _, exist := pro.Metadata[key] - if exist { - metaNeedUpdated[key] = value - } else { - metaNeedCreated[key] = value + if len(project.Metadata) > 0 { + metaNeedUpdated := map[string]string{} + metaNeedCreated := map[string]string{} + if pro.Metadata == nil { + pro.Metadata = map[string]string{} + } + for key, value := range project.Metadata { + _, exist := pro.Metadata[key] + if exist { + metaNeedUpdated[key] = value + } else { + metaNeedCreated[key] = value + } + } + if err = d.metaMgr.Add(pro.ProjectID, metaNeedCreated); err != nil { + return err + } + if err = d.metaMgr.Update(pro.ProjectID, metaNeedUpdated); err != nil { + return err } } - if err = d.metaMgr.Add(pro.ProjectID, metaNeedCreated); err != nil { - return err - } - if err = d.metaMgr.Update(pro.ProjectID, metaNeedUpdated); err != nil { - return err - } } - return d.pmsDriver.Update(projectIDOrName, project) } @@ -179,6 +193,7 @@ func (d *defaultProjectManager) List(query *models.ProjectQueryParam) (*models.P project.Metadata = meta } } + // the whitelist is not populated deliberately return result, nil } diff --git a/src/core/proxy/interceptor_test.go b/src/core/proxy/interceptor_test.go index ab73f27bf..db9912602 100644 --- a/src/core/proxy/interceptor_test.go +++ b/src/core/proxy/interceptor_test.go @@ -166,9 +166,10 @@ func TestPMSPolicyChecker(t *testing.T) { Name: name, OwnerID: 1, Metadata: map[string]string{ - models.ProMetaEnableContentTrust: "true", - models.ProMetaPreventVul: "true", - models.ProMetaSeverity: "low", + models.ProMetaEnableContentTrust: "true", + models.ProMetaPreventVul: "true", + models.ProMetaSeverity: "low", + models.ProMetaReuseSysCVEWhitelist: "false", }, }) require.Nil(t, err) @@ -180,9 +181,10 @@ func TestPMSPolicyChecker(t *testing.T) { contentTrustFlag := getPolicyChecker().contentTrustEnabled("project_for_test_get_sev_low") assert.True(t, contentTrustFlag) - projectVulnerableEnabled, projectVulnerableSeverity := getPolicyChecker().vulnerablePolicy("project_for_test_get_sev_low") + projectVulnerableEnabled, projectVulnerableSeverity, wl := getPolicyChecker().vulnerablePolicy("project_for_test_get_sev_low") assert.True(t, projectVulnerableEnabled) assert.Equal(t, projectVulnerableSeverity, models.SevLow) + assert.Empty(t, wl.Items) } func TestMatchNotaryDigest(t *testing.T) { diff --git a/src/core/proxy/interceptors.go b/src/core/proxy/interceptors.go index fb656edce..6e28aaa6e 100644 --- a/src/core/proxy/interceptors.go +++ b/src/core/proxy/interceptors.go @@ -11,6 +11,7 @@ import ( "github.com/goharbor/harbor/src/core/promgr" coreutils "github.com/goharbor/harbor/src/core/utils" "github.com/goharbor/harbor/src/pkg/scan" + "github.com/goharbor/harbor/src/pkg/scan/whitelist" "context" "fmt" @@ -82,7 +83,7 @@ type policyChecker interface { // contentTrustEnabled returns whether a project has enabled content trust. contentTrustEnabled(name string) bool // vulnerablePolicy returns whether a project has enabled vulnerable, and the project's severity. - vulnerablePolicy(name string) (bool, models.Severity) + vulnerablePolicy(name string) (bool, models.Severity, models.CVEWhitelist) } type pmsPolicyChecker struct { @@ -97,13 +98,28 @@ func (pc pmsPolicyChecker) contentTrustEnabled(name string) bool { } return project.ContentTrustEnabled() } -func (pc pmsPolicyChecker) vulnerablePolicy(name string) (bool, models.Severity) { +func (pc pmsPolicyChecker) vulnerablePolicy(name string) (bool, models.Severity, models.CVEWhitelist) { project, err := pc.pm.Get(name) + wl := models.CVEWhitelist{} if err != nil { log.Errorf("Unexpected error when getting the project, error: %v", err) - return true, models.SevUnknown + return true, models.SevUnknown, wl } - return project.VulPrevented(), clair.ParseClairSev(project.Severity()) + mgr := whitelist.NewDefaultManager() + if project.ReuseSysCVEWhitelist() { + w, err := mgr.GetSys() + if err != nil { + return project.VulPrevented(), clair.ParseClairSev(project.Severity()), wl + } + wl = *w + } else { + w, err := mgr.Get(project.ProjectID) + if err != nil { + return project.VulPrevented(), clair.ParseClairSev(project.Severity()), wl + } + wl = *w + } + return project.VulPrevented(), clair.ParseClairSev(project.Severity()), wl } // newPMSPolicyChecker returns an instance of an pmsPolicyChecker @@ -298,25 +314,18 @@ func (vh vulnerableHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) vh.next.ServeHTTP(rw, req) return } - projectVulnerableEnabled, projectVulnerableSeverity := getPolicyChecker().vulnerablePolicy(img.projectName) + projectVulnerableEnabled, projectVulnerableSeverity, wl := getPolicyChecker().vulnerablePolicy(img.projectName) if !projectVulnerableEnabled { vh.next.ServeHTTP(rw, req) return } - // TODO: Get whitelist based on project setting - wl, err := dao.GetSysCVEWhitelist() - if err != nil { - 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 - } 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 } - filtered := vl.ApplyWhitelist(*wl) + filtered := vl.ApplyWhitelist(wl) msg := vh.filterMsg(img, filtered) log.Info(msg) if int(vl.Severity()) >= int(projectVulnerableSeverity) { diff --git a/src/pkg/scan/whitelist/manager.go b/src/pkg/scan/whitelist/manager.go new file mode 100644 index 000000000..5aa793b9a --- /dev/null +++ b/src/pkg/scan/whitelist/manager.go @@ -0,0 +1,79 @@ +// 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 whitelist + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/jobservice/logger" +) + +// Manager defines the interface of CVE whitelist manager, it support both system level and project level whitelists +type Manager interface { + // CreateEmpty creates empty whitelist for given project + CreateEmpty(projectID int64) error + // Set sets the whitelist for given project (create or update) + Set(projectID int64, list models.CVEWhitelist) error + // Get gets the whitelist for given project + Get(projectID int64) (*models.CVEWhitelist, error) + // SetSys sets system level whitelist + SetSys(list models.CVEWhitelist) error + // GetSys gets system level whitelist + GetSys() (*models.CVEWhitelist, error) +} + +type defaultManager struct{} + +// CreateEmpty creates empty whitelist for given project +func (d *defaultManager) CreateEmpty(projectID int64) error { + l := models.CVEWhitelist{ + ProjectID: projectID, + } + _, err := dao.UpdateCVEWhitelist(l) + if err != nil { + logger.Errorf("Failed to create empty CVE whitelist for project: %d, error: %v", projectID, err) + } + return err +} + +// Set sets the whitelist for given project (create or update) +func (d *defaultManager) Set(projectID int64, list models.CVEWhitelist) error { + list.ProjectID = projectID + if err := Validate(list); err != nil { + return err + } + _, err := dao.UpdateCVEWhitelist(list) + return err +} + +// Get gets the whitelist for given project +func (d *defaultManager) Get(projectID int64) (*models.CVEWhitelist, error) { + return dao.GetCVEWhitelist(projectID) +} + +// SetSys sets the system level whitelist +func (d *defaultManager) SetSys(list models.CVEWhitelist) error { + return d.Set(0, list) +} + +// GetSys gets the system level whitelist +func (d *defaultManager) GetSys() (*models.CVEWhitelist, error) { + return d.Get(0) +} + +// NewDefaultManager return a new instance of defaultManager +func NewDefaultManager() Manager { + return &defaultManager{} +} diff --git a/src/pkg/scan/whitelist/validator.go b/src/pkg/scan/whitelist/validator.go new file mode 100644 index 000000000..cef2a17df --- /dev/null +++ b/src/pkg/scan/whitelist/validator.go @@ -0,0 +1,60 @@ +// 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 whitelist + +import ( + "fmt" + "github.com/goharbor/harbor/src/common/models" + "regexp" +) + +type invalidErr struct { + msg string +} + +func (ie *invalidErr) Error() string { + return ie.msg +} + +// NewInvalidErr ... +func NewInvalidErr(s string) error { + return &invalidErr{ + msg: s, + } +} + +// IsInvalidErr checks if the error is an invalidErr +func IsInvalidErr(err error) bool { + _, ok := err.(*invalidErr) + return ok +} + +const cveIDPattern = `^CVE-\d{4}-\d+$` + +// Validate help validates the CVE whitelist, to ensure the CVE ID is valid and there's no duplication +func Validate(wl models.CVEWhitelist) error { + m := map[string]struct{}{} + re := regexp.MustCompile(cveIDPattern) + for _, it := range wl.Items { + if !re.MatchString(it.CVEID) { + return &invalidErr{fmt.Sprintf("invalid CVE ID: %s", it.CVEID)} + } + if _, ok := m[it.CVEID]; ok { + return &invalidErr{fmt.Sprintf("duplicate CVE ID in whitelist: %s", it.CVEID)} + } + m[it.CVEID] = struct{}{} + } + return nil +} diff --git a/src/pkg/scan/whitelist/validator_test.go b/src/pkg/scan/whitelist/validator_test.go new file mode 100644 index 000000000..e147d2364 --- /dev/null +++ b/src/pkg/scan/whitelist/validator_test.go @@ -0,0 +1,102 @@ +// 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 whitelist + +import ( + "fmt" + "github.com/goharbor/harbor/src/common/models" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestIsInvalidErr(t *testing.T) { + cases := []struct { + instance error + expect bool + }{ + { + instance: nil, + expect: false, + }, + { + instance: fmt.Errorf("whatever"), + expect: false, + }, + { + instance: NewInvalidErr("This is true"), + expect: true, + }, + } + + for n, c := range cases { + t.Logf("Executing TestIsInvalidErr case: %d\n", n) + assert.Equal(t, c.expect, IsInvalidErr(c.instance)) + } +} + +func TestValidate(t *testing.T) { + cases := []struct { + l models.CVEWhitelist + noError bool + }{ + { + l: models.CVEWhitelist{ + Items: nil, + }, + noError: true, + }, + { + l: models.CVEWhitelist{ + Items: []models.CVEWhitelistItem{}, + }, + noError: true, + }, + { + l: models.CVEWhitelist{ + Items: []models.CVEWhitelistItem{ + {CVEID: "breakit"}, + }, + }, + noError: false, + }, + { + l: models.CVEWhitelist{ + Items: []models.CVEWhitelistItem{ + {CVEID: "CVE-2014-456132"}, + {CVEID: "CVE-2014-7654321"}, + }, + }, + noError: true, + }, + { + l: models.CVEWhitelist{ + Items: []models.CVEWhitelistItem{ + {CVEID: "CVE-2014-456132"}, + {CVEID: "CVE-2014-456132"}, + {CVEID: "CVE-2014-7654321"}, + }, + }, + noError: false, + }, + } + for n, c := range cases { + t.Logf("Executing TestValidate case: %d\n", n) + e := Validate(c.l) + assert.Equal(t, c.noError, e == nil) + if e != nil { + assert.True(t, IsInvalidErr(e)) + } + } +}