From 30bb2ddcdfa40fa9872fbdc4bf16ce72f251ba28 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Fri, 16 Aug 2019 02:23:12 +0800 Subject: [PATCH] Avoid overwriting system CVE whitelist by mistake Fixes #8702 Also enforce the code to mitigate the potential risk. Signed-off-by: Daniel Jiang --- src/common/dao/cve_whitelist.go | 28 +++++----------- src/common/dao/cve_whitelist_test.go | 21 ++---------- src/core/promgr/promgr.go | 2 +- src/pkg/scan/whitelist/manager.go | 10 ++++-- src/pkg/scan/whitelist/manager_test.go | 46 ++++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 41 deletions(-) create mode 100644 src/pkg/scan/whitelist/manager_test.go diff --git a/src/common/dao/cve_whitelist.go b/src/common/dao/cve_whitelist.go index 25c1c9b98..645a1c076 100644 --- a/src/common/dao/cve_whitelist.go +++ b/src/common/dao/cve_whitelist.go @@ -21,6 +21,14 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" ) +// CreateCVEWhitelist creates the CVE whitelist +func CreateCVEWhitelist(l models.CVEWhitelist) (int64, error) { + o := GetOrmer() + itemsBytes, _ := json.Marshal(l.Items) + l.ItemsText = string(itemsBytes) + return o.Insert(&l) +} + // UpdateCVEWhitelist Updates the vulnerability white list to DB func UpdateCVEWhitelist(l models.CVEWhitelist) (int64, error) { o := GetOrmer() @@ -30,23 +38,6 @@ func UpdateCVEWhitelist(l models.CVEWhitelist) (int64, error) { return id, err } -// GetSysCVEWhitelist Gets the system level vulnerability white list from DB -func GetSysCVEWhitelist() (*models.CVEWhitelist, error) { - return GetCVEWhitelist(0) -} - -// UpdateSysCVEWhitelist updates the system level CVE whitelist -/* -func UpdateSysCVEWhitelist(l models.CVEWhitelist) error { - if l.ProjectID != 0 { - return fmt.Errorf("system level CVE whitelist cannot set project ID") - } - l.ProjectID = -1 - _, err := UpdateCVEWhitelist(l) - return err -} -*/ - // GetCVEWhitelist Gets the CVE whitelist of the project based on the project ID in parameter func GetCVEWhitelist(pid int64) (*models.CVEWhitelist, error) { o := GetOrmer() @@ -58,8 +49,7 @@ func GetCVEWhitelist(pid int64) (*models.CVEWhitelist, error) { return nil, fmt.Errorf("failed to get CVE whitelist for project %d, error: %v", pid, err) } if len(r) == 0 { - log.Infof("No CVE whitelist found for project %d, returning empty list.", pid) - return &models.CVEWhitelist{ProjectID: pid, Items: []models.CVEWhitelistItem{}}, nil + return nil, nil } else if len(r) > 1 { log.Infof("Multiple CVE whitelists found for project %d, length: %d, returning first element.", pid, len(r)) } diff --git a/src/common/dao/cve_whitelist_test.go b/src/common/dao/cve_whitelist_test.go index 35af2f294..099409de5 100644 --- a/src/common/dao/cve_whitelist_test.go +++ b/src/common/dao/cve_whitelist_test.go @@ -23,12 +23,9 @@ import ( func TestUpdateAndGetCVEWhitelist(t *testing.T) { require.Nil(t, ClearTable("cve_whitelist")) - l, err := GetSysCVEWhitelist() - assert.Nil(t, err) - assert.Equal(t, models.CVEWhitelist{ProjectID: 0, Items: []models.CVEWhitelistItem{}}, *l) l2, err := GetCVEWhitelist(5) assert.Nil(t, err) - assert.Equal(t, models.CVEWhitelist{ProjectID: 5, Items: []models.CVEWhitelistItem{}}, *l2) + assert.Nil(t, l2) longList := []models.CVEWhitelistItem{} for i := 0; i < 50; i++ { @@ -46,15 +43,6 @@ func TestUpdateAndGetCVEWhitelist(t *testing.T) { assert.Equal(t, longList, out1.Items) assert.Equal(t, e, *out1.ExpiresAt) - in2 := models.CVEWhitelist{ProjectID: 3, Items: []models.CVEWhitelistItem{}} - _, err = UpdateCVEWhitelist(in2) - require.Nil(t, err) - // assert.Equal(t, int64(1), n2) - out2, err := GetCVEWhitelist(3) - require.Nil(t, err) - assert.Equal(t, int64(3), out2.ProjectID) - assert.Equal(t, []models.CVEWhitelistItem{}, out2.Items) - sysCVEs := []models.CVEWhitelistItem{ {CVEID: "CVE-2019-10164"}, {CVEID: "CVE-2017-12345"}, @@ -62,11 +50,6 @@ func TestUpdateAndGetCVEWhitelist(t *testing.T) { in3 := models.CVEWhitelist{Items: sysCVEs} _, err = UpdateCVEWhitelist(in3) require.Nil(t, err) - // assert.Equal(t, int64(1), n3) - sysList, err := GetSysCVEWhitelist() - require.Nil(t, err) - assert.Equal(t, int64(0), sysList.ProjectID) - assert.Equal(t, sysCVEs, sysList.Items) - // require.Nil(t, ClearTable("cve_whitelist")) + require.Nil(t, ClearTable("cve_whitelist")) } diff --git a/src/core/promgr/promgr.go b/src/core/promgr/promgr.go index 5a0a9bc12..3ac8f6ca8 100644 --- a/src/core/promgr/promgr.go +++ b/src/core/promgr/promgr.go @@ -94,7 +94,7 @@ func (d *defaultProjectManager) Create(project *models.Project) (int64, error) { return 0, err } if d.metaMgrEnabled { - d.whitelistMgr.CreateEmpty(project.ProjectID) + d.whitelistMgr.CreateEmpty(id) 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) diff --git a/src/pkg/scan/whitelist/manager.go b/src/pkg/scan/whitelist/manager.go index 5aa793b9a..d582e3f10 100644 --- a/src/pkg/scan/whitelist/manager.go +++ b/src/pkg/scan/whitelist/manager.go @@ -17,6 +17,7 @@ package whitelist import ( "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/jobservice/logger" ) @@ -41,7 +42,7 @@ func (d *defaultManager) CreateEmpty(projectID int64) error { l := models.CVEWhitelist{ ProjectID: projectID, } - _, err := dao.UpdateCVEWhitelist(l) + _, err := dao.CreateCVEWhitelist(l) if err != nil { logger.Errorf("Failed to create empty CVE whitelist for project: %d, error: %v", projectID, err) } @@ -60,7 +61,12 @@ func (d *defaultManager) Set(projectID int64, list models.CVEWhitelist) error { // Get gets the whitelist for given project func (d *defaultManager) Get(projectID int64) (*models.CVEWhitelist, error) { - return dao.GetCVEWhitelist(projectID) + wl, err := dao.GetCVEWhitelist(projectID) + if wl == nil && err == nil { + log.Debugf("No CVE whitelist found for project %d, returning empty list.", projectID) + return &models.CVEWhitelist{ProjectID: projectID, Items: []models.CVEWhitelistItem{}}, nil + } + return wl, err } // SetSys sets the system level whitelist diff --git a/src/pkg/scan/whitelist/manager_test.go b/src/pkg/scan/whitelist/manager_test.go new file mode 100644 index 000000000..8dbf6da37 --- /dev/null +++ b/src/pkg/scan/whitelist/manager_test.go @@ -0,0 +1,46 @@ +package whitelist + +import ( + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/stretchr/testify/assert" + "os" + "testing" +) + +func TestMain(m *testing.M) { + + // databases := []string{"mysql", "sqlite"} + databases := []string{"postgresql"} + for _, database := range databases { + log.Infof("run test cases for database: %s", database) + + result := 1 + switch database { + case "postgresql": + dao.PrepareTestForPostgresSQL() + default: + log.Fatalf("invalid database: %s", database) + } + + result = m.Run() + + if result != 0 { + os.Exit(result) + } + } +} + +func TestDefaultManager_CreateEmpty(t *testing.T) { + dm := NewDefaultManager() + assert.NoError(t, dm.CreateEmpty(99)) + assert.Error(t, dm.CreateEmpty(99)) +} + +func TestDefaultManager_Get(t *testing.T) { + dm := NewDefaultManager() + // return empty list + l, err := dm.Get(1234) + assert.Nil(t, err) + assert.Empty(t, l.Items) +}