Fixes for various bugs/issues logged as a part of the test day. (#17232)

Closes:
* CVE Data Export API IDOR issue
* https://github.com/goharbor/harbor/issues/17199
* https://github.com/goharbor/harbor/issues/17193
* https://github.com/goharbor/harbor/issues/17188
* https://github.com/goharbor/harbor/issues/17184

Signed-off-by: prahaladdarkin <prahaladd@vmware.com>
This commit is contained in:
prahaladdarkin 2022-07-26 14:20:54 +05:30 committed by GitHub
parent 02eae9dede
commit d53af792ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 133 additions and 36 deletions

View File

@ -12,7 +12,6 @@ import (
"github.com/opencontainers/go-digest"
"github.com/goharbor/harbor/src/jobservice/job"
"github.com/goharbor/harbor/src/jobservice/logger"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/pkg/project"
"github.com/goharbor/harbor/src/pkg/scan/export"
@ -76,12 +75,13 @@ func (sde *ScanDataExport) Run(ctx job.Context, params job.Parameters) error {
}
mode := params[export.JobModeKey].(string)
logger := ctx.GetLogger()
logger.Infof("Scan data export job started in mode : %v", mode)
sde.init()
fileName := fmt.Sprintf("%s/scandata_export_%v.csv", sde.scanDataExportDirPath, params["JobId"])
// ensure that CSV files are cleared post the completion of the Run.
defer sde.cleanupCsvFile(fileName, params)
defer sde.cleanupCsvFile(ctx, fileName, params)
err := sde.writeCsvFile(ctx, params, fileName)
if err != nil {
logger.Errorf("error when writing data to CSV: %v", err)
@ -133,6 +133,7 @@ func (sde *ScanDataExport) Run(ctx job.Context, params job.Parameters) error {
func (sde *ScanDataExport) updateExecAttributes(ctx job.Context, params job.Parameters, err error, hash digest.Digest) error {
execID := int64(params["JobId"].(float64))
exec, err := sde.execMgr.Get(ctx.SystemContext(), execID)
logger := ctx.GetLogger()
if err != nil {
logger.Errorf("Export Job Id = %v. Error when fetching execution record for update : %v", params["JobId"], err)
return err
@ -153,6 +154,7 @@ func (sde *ScanDataExport) writeCsvFile(ctx job.Context, params job.Parameters,
systemContext := ctx.SystemContext()
defer csvFile.Close()
logger := ctx.GetLogger()
if err != nil {
logger.Errorf("Failed to create CSV export file %s. Error : %v", fileName, err)
return err
@ -329,7 +331,8 @@ func (sde *ScanDataExport) init() {
}
}
func (sde *ScanDataExport) cleanupCsvFile(fileName string, params job.Parameters) {
func (sde *ScanDataExport) cleanupCsvFile(ctx job.Context, fileName string, params job.Parameters) {
logger := ctx.GetLogger()
if _, err := os.Stat(fileName); os.IsNotExist(err) {
logger.Infof("Export Job Id = %v, CSV Export File = %s does not exist. Nothing to do", params["JobId"], fileName)
return

View File

@ -20,7 +20,7 @@ const (
VulnScanReportQuery = `select row_number() over() as result_row_id, project.project_id as project_id, project."name" as project_name, harbor_user.user_id as user_id, harbor_user.username as project_owner, repository.repository_id, repository.name as repository_name,
scanner_registration.id as scanner_id, scanner_registration."name" as scanner_name,
vulnerability_record.cve_id, vulnerability_record.package, vulnerability_record.severity,
vulnerability_record.cvss_score_v3, vulnerability_record.cvss_score_v2, vulnerability_record.cvss_vector_v3, vulnerability_record.cvss_vector_v2, vulnerability_record.cwe_ids from report_vulnerability_record inner join scan_report on report_vulnerability_record.report_uuid = scan_report.uuid
vulnerability_record.cvss_score_v3, vulnerability_record.cvss_score_v2, vulnerability_record.cvss_vector_v3, vulnerability_record.cvss_vector_v2, vulnerability_record.cwe_ids, vulnerability_record.package_version, vulnerability_record.fixed_version from report_vulnerability_record inner join scan_report on report_vulnerability_record.report_uuid = scan_report.uuid
inner join artifact on scan_report.digest = artifact.digest
left outer join artifact_reference on artifact.id = artifact_reference.child_id
inner join vulnerability_record on report_vulnerability_record.vuln_record_id = vulnerability_record.id
@ -29,19 +29,20 @@ inner join repository on artifact.repository_id = repository.repository_id
inner join tag on tag.repository_id = repository.repository_id
inner join harbor_user on project.owner_id = harbor_user.user_id
inner join scanner_registration on scan_report.registration_uuid = scanner_registration.uuid `
ArtifactBylabelQueryTemplate = "select distinct artifact.id from artifact inner join label_reference on artifact.id = label_reference.artifact_id inner join harbor_label on label_reference.label_id = harbor_label.id and harbor_label.id in (%s)"
SQLAnd = " and "
SQLOr = " or "
RepositoryIDColumn = "repository.repository_id"
ProjectIDColumn = "project.project_id"
TagIDColumn = "tag.id"
ArtifactParentIDColumn = "artifact_reference.parent_id"
ArtifactIDColumn = "artifact.id"
GroupBy = " group by "
GroupByCols = `package, vulnerability_record.severity, vulnerability_record.cve_id, project.project_id, harbor_user.user_id ,
ArtifactBylabelQueryTemplate = "select distinct artifact.id from artifact inner join label_reference on artifact.id = label_reference.artifact_id inner join harbor_label on label_reference.label_id = harbor_label.id and harbor_label.id in (%s)"
ArtifactByLabelRepoFilterTemplate = " and artifact.repository_id in (%s)"
SQLAnd = " and "
SQLOr = " or "
RepositoryIDColumn = "repository.repository_id"
ProjectIDColumn = "project.project_id"
TagIDColumn = "tag.id"
ArtifactParentIDColumn = "artifact_reference.parent_id"
ArtifactIDColumn = "artifact.id"
GroupBy = " group by "
GroupByCols = `package, vulnerability_record.severity, vulnerability_record.cve_id, project.project_id, harbor_user.user_id ,
repository.repository_id, scanner_registration.id, vulnerability_record.cvss_score_v3,
vulnerability_record.cvss_score_v2, vulnerability_record.cvss_vector_v3, vulnerability_record.cvss_vector_v2,
vulnerability_record.cwe_ids`
vulnerability_record.cwe_ids, vulnerability_record.package_version, vulnerability_record.fixed_version`
JobModeExport = "export"
JobModeKey = "mode"
)
@ -107,7 +108,7 @@ func NewManager() Manager {
func (em *exportManager) Fetch(ctx context.Context, params Params) ([]Data, error) {
exportData := make([]Data, 0)
artifactIdsWithLabel, err := em.getArtifactsWithLabel(ctx, params.Labels)
artifactIdsWithLabel, err := em.getArtifactsWithLabel(ctx, params.Labels, params.Repositories)
if err != nil {
return nil, err
}
@ -226,24 +227,38 @@ func (em *exportManager) buildIDFilterFragmentWithInForMultipleCols(ids []int64,
// the specified label ids.
// Within Harbor, labels are attached to the root artifact whereas scan results
// are associated with the child artifact.
func (em *exportManager) getArtifactsWithLabel(ctx context.Context, ids []int64) ([]int64, error) {
func (em *exportManager) getArtifactsWithLabel(ctx context.Context, ids []int64, repositoryIds []int64) ([]int64, error) {
artifactIds := make([]int64, 0)
if len(ids) == 0 {
return artifactIds, nil
}
artifactQueryBuilder := strings.Builder{}
strIds := make([]string, 0)
for _, id := range ids {
strIds = append(strIds, strconv.FormatInt(id, 10))
}
artifactQuery := fmt.Sprintf(ArtifactBylabelQueryTemplate, strings.Join(strIds, ","))
logger.Infof("Triggering artifact query : %s", artifactQuery)
artifactQueryFragment := fmt.Sprintf(ArtifactBylabelQueryTemplate, strings.Join(strIds, ","))
artifactQueryBuilder.WriteString(artifactQueryFragment)
// append any repository filters
strRepoIds := make([]string, 0)
for _, repoID := range repositoryIds {
strRepoIds = append(strRepoIds, strconv.FormatInt(repoID, 10))
}
if len(repositoryIds) > 0 {
repoFilterFragment := fmt.Sprintf(ArtifactByLabelRepoFilterTemplate, strings.Join(strRepoIds, ","))
artifactQueryBuilder.WriteString(repoFilterFragment)
}
logger.Infof("Triggering artifact query : %s", artifactQueryBuilder.String())
ormer, err := orm.FromContext(ctx)
if err != nil {
return nil, err
}
numRows, err := ormer.Raw(artifactQuery).QueryRows(&artifactIds)
numRows, err := ormer.Raw(artifactQueryFragment).QueryRows(&artifactIds)
if err != nil {
return nil, err
}

View File

@ -217,7 +217,7 @@ func (suite *ExportManagerSuite) generateVulnerabilityRecordsForReport(registrat
vulnV2 := new(daoscan.VulnerabilityRecord)
vulnV2.CVEID = fmt.Sprintf("CVE-ID%d", i)
vulnV2.Package = fmt.Sprintf("Package%d", i)
vulnV2.PackageVersion = "NotAvailable"
vulnV2.PackageVersion = "Package-0.9.0"
vulnV2.PackageType = "Unknown"
vulnV2.Fix = "1.0.0"
vulnV2.URLs = "url1"

View File

@ -9,20 +9,21 @@ import (
// Data models a single row of the exported scan vulnerability data
type Data struct {
ID int64 `orm:"column(result_row_id)" csv:"RowId"`
ProjectName string `orm:"column(project_name)" csv:"Project"`
ProjectOwner string `orm:"column(project_owner)" csv:"Owner"`
ScannerName string `orm:"column(scanner_name)" csv:"Scanner"`
Repository string `orm:"column(repository_name)" csv:"Repository"`
ArtifactDigest string `orm:"column(artifact_digest)" csv:"Artifact Digest"`
CVEId string `orm:"column(cve_id)" csv:"CVE"`
Package string `orm:"column(package)" csv:"Package"`
Severity string `orm:"column(severity)" csv:"Severity"`
CVSSScoreV3 string `orm:"column(cvss_score_v3)" csv:"CVSS V3 Score"`
CVSSScoreV2 string `orm:"column(cvss_score_v2)" csv:"CVSS V2 Score"`
CVSSVectorV3 string `orm:"column(cvss_vector_v3)" csv:"CVSS V3 Vector"`
CVSSVectorV2 string `orm:"column(cvss_vector_v2)" csv:"CVSS V2 Vector"`
CWEIds string `orm:"column(cwe_ids)" csv:"CWE Ids"`
ID int64 `orm:"column(result_row_id)" csv:"RowId"`
ProjectName string `orm:"column(project_name)" csv:"Project"`
ProjectOwner string `orm:"column(project_owner)" csv:"Owner"`
ScannerName string `orm:"column(scanner_name)" csv:"Scanner"`
Repository string `orm:"column(repository_name)" csv:"Repository"`
CVEId string `orm:"column(cve_id)" csv:"CVE"`
Package string `orm:"column(package)" csv:"Package"`
Version string `orm:"column(package_version)" csv:"Current Version"`
FixVersion string `orm:"column(fixed_version)" csv:"Fixed in version"`
Severity string `orm:"column(severity)" csv:"Severity"`
CVSSScoreV3 string `orm:"column(cvss_score_v3)" csv:"CVSS V3 Score"`
CVSSScoreV2 string `orm:"column(cvss_score_v2)" csv:"CVSS V2 Score"`
CVSSVectorV3 string `orm:"column(cvss_vector_v3)" csv:"CVSS V3 Vector"`
CVSSVectorV2 string `orm:"column(cvss_vector_v2)" csv:"CVSS V2 Vector"`
CWEIds string `orm:"column(cwe_ids)" csv:"CWE Ids"`
}
// Request encapsulates the filters to be provided when exporting the data for a scan.

View File

@ -7,6 +7,8 @@ import (
"net/http"
"strings"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/go-openapi/runtime"
"github.com/go-openapi/runtime/middleware"
"github.com/go-openapi/strfmt"
@ -107,6 +109,20 @@ func (se *scanDataExportAPI) GetScanDataExportExecution(ctx context.Context, par
return se.SendError(ctx, err)
}
execution, err := se.scanDataExportCtl.GetExecution(ctx, params.ExecutionID)
if err != nil {
return se.SendError(ctx, err)
}
// check if the execution being fetched is owned by the current user
secContext, err := se.GetSecurityContext(ctx)
if err != nil {
return se.SendError(ctx, err)
}
if secContext.GetUsername() != execution.UserName {
err = errors.New(nil).WithCode(errors.ForbiddenCode)
return se.SendError(ctx, err)
}
if err != nil {
return se.SendError(ctx, err)
}

View File

@ -252,7 +252,7 @@ func (suite *ScanExportTestSuite) TestExportScanDataNoPrivileges() {
}
func (suite *ScanExportTestSuite) TestGetScanDataExportExecution() {
suite.Security.On("GetUsername").Return("test-user")
suite.Security.On("IsAuthenticated").Return(true).Once()
suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once()
url := "/export/cve/execution/100"
@ -284,6 +284,33 @@ func (suite *ScanExportTestSuite) TestGetScanDataExportExecution() {
}
func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionUserNotOwnerOfExport() {
suite.Security.On("GetUsername").Return("test-user")
suite.Security.On("IsAuthenticated").Return(true).Once()
suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once()
url := "/export/cve/execution/100"
endTime := time.Now()
startTime := endTime.Add(-10 * time.Minute)
execution := &export.Execution{
ID: 100,
UserID: 3,
Status: "Success",
StatusMessage: "",
Trigger: "MANUAL",
StartTime: startTime,
EndTime: endTime,
ExportDataDigest: "datadigest",
UserName: "test-user1",
JobName: "test-job",
FilePresent: false,
}
mock.OnAnything(suite.scanExportCtl, "GetExecution").Return(execution, nil).Once()
res, err := suite.DoReq(http.MethodGet, url, nil)
suite.Equal(http.StatusForbidden, res.StatusCode)
suite.Equal(nil, err)
}
func (suite *ScanExportTestSuite) TestDownloadScanData() {
suite.Security.On("GetUsername").Return("test-user")
suite.Security.On("IsAuthenticated").Return(true).Once()
@ -476,6 +503,41 @@ func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionList() {
suite.Equal(int64(100), respData.Items[0].ID)
}
func (suite *ScanExportTestSuite) TestGetScanDataExportExecutionListFilterNotOwned() {
suite.Security.On("GetUsername").Return("test-user")
suite.Security.On("IsAuthenticated").Return(true).Once()
suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once()
url, err := url2.Parse("/export/cve/executions")
params := url2.Values{}
params.Add("user_name", "test-user")
url.RawQuery = params.Encode()
endTime := time.Now()
startTime := endTime.Add(-10 * time.Minute)
executionOwned := &export.Execution{
ID: 100,
UserID: 3,
Status: "Success",
StatusMessage: "",
Trigger: "MANUAL",
StartTime: startTime,
EndTime: endTime,
ExportDataDigest: "datadigest",
JobName: "test-job",
UserName: "test-user",
}
fmt.Println("URL string : ", url.String())
mock.OnAnything(suite.scanExportCtl, "ListExecutions").Return([]*export.Execution{executionOwned}, nil).Once()
res, err := suite.DoReq(http.MethodGet, url.String(), nil)
suite.Equal(200, res.StatusCode)
suite.Equal(nil, err)
respData := models.ScanDataExportExecutionList{}
json.NewDecoder(res.Body).Decode(&respData)
suite.Equal(1, len(respData.Items))
suite.Equal(int64(100), respData.Items[0].ID)
}
func TestScanExportTestSuite(t *testing.T) {
suite.Run(t, &ScanExportTestSuite{})
}