skip policy check on pull cosign signature (#16658)

When user enables the cosign policy and triggers the replication, the harbor adapter will try to  pull the cosign siguature if it has to do the further push.
In this case, it has to skip policy check.

Signed-off-by: wang yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2022-04-07 09:59:26 +08:00 committed by GitHub
parent df54e1e13c
commit bb6693b496
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 169 additions and 26 deletions

View File

@ -29,11 +29,6 @@ func Cosign() func(http.Handler) http.Handler {
return err
}
if util.SkipPolicyChecking(r, pro.ProjectID) {
logger.Debugf("artifact %s@%s is pulling by the scanner/cosign, skip the checking", af.Repository, af.Digest)
return nil
}
// If cosign policy enabled, it has to at least have one cosign signature.
if pro.ContentTrustCosignEnabled() {
art, err := artifact.Ctl.GetByReference(ctx, af.Repository, af.Reference, &artifact.Option{
@ -43,6 +38,15 @@ func Cosign() func(http.Handler) http.Handler {
return err
}
ok, err := util.SkipPolicyChecking(r, pro.ProjectID, art.ID)
if err != nil {
return err
}
if ok {
logger.Debugf("artifact %s@%s is pulling by the scanner/cosign, skip the checking", af.Repository, af.Digest)
return nil
}
if len(art.Accessories) == 0 {
pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Cosign.")
return pkgE

View File

@ -16,7 +16,11 @@ package contenttrust
import (
"fmt"
"github.com/goharbor/harbor/src/pkg/accessory"
accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
basemodel "github.com/goharbor/harbor/src/pkg/accessory/model/base"
proModels "github.com/goharbor/harbor/src/pkg/project/models"
accessorytesting "github.com/goharbor/harbor/src/testing/pkg/accessory"
"net/http"
"net/http/httptest"
"testing"
@ -42,6 +46,9 @@ type CosignMiddlewareTestSuite struct {
originalProjectController project.Controller
projectController *projecttesting.Controller
originalAccessMgr accessory.Manager
accessMgr *accessorytesting.Manager
artifact *artifact.Artifact
project *proModels.Project
@ -57,6 +64,10 @@ func (suite *CosignMiddlewareTestSuite) SetupTest() {
suite.projectController = &projecttesting.Controller{}
project.Ctl = suite.projectController
suite.originalAccessMgr = accessory.Mgr
suite.accessMgr = &accessorytesting.Manager{}
accessory.Mgr = suite.accessMgr
suite.artifact = &artifact.Artifact{}
suite.artifact.Type = image.ArtifactTypeImage
suite.artifact.ProjectID = 1
@ -80,6 +91,7 @@ func (suite *CosignMiddlewareTestSuite) SetupTest() {
func (suite *CosignMiddlewareTestSuite) TearDownTest() {
artifact.Ctl = suite.originalArtifactController
project.Ctl = suite.originalProjectController
accessory.Mgr = suite.originalAccessMgr
}
func (suite *CosignMiddlewareTestSuite) makeRequest(setHeader ...bool) *http.Request {
@ -143,6 +155,7 @@ func (suite *CosignMiddlewareTestSuite) TestNoneArtifact() {
func (suite *CosignMiddlewareTestSuite) TestAuthenticatedUserPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("local")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -159,6 +172,7 @@ func (suite *CosignMiddlewareTestSuite) TestAuthenticatedUserPulling() {
func (suite *CosignMiddlewareTestSuite) TestScannerPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("v2token")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -175,6 +189,7 @@ func (suite *CosignMiddlewareTestSuite) TestScannerPulling() {
func (suite *CosignMiddlewareTestSuite) TestCosignPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("v2token")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -192,6 +207,7 @@ func (suite *CosignMiddlewareTestSuite) TestCosignPulling() {
func (suite *CosignMiddlewareTestSuite) TestUnAuthenticatedUserPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("local")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -204,6 +220,29 @@ func (suite *CosignMiddlewareTestSuite) TestUnAuthenticatedUserPulling() {
suite.Equal(rr.Code, http.StatusPreconditionFailed)
}
// pull cosign signature when policy checker is enabled.
func (suite *CosignMiddlewareTestSuite) TestSignaturePulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
acc := &basemodel.Default{
Data: accessorymodel.AccessoryData{
ID: 1,
ArtifactID: 2,
SubArtifactID: 1,
Type: accessorymodel.TypeCosignSignature,
},
}
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{
acc,
}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
Cosign()(suite.next).ServeHTTP(rr, req)
suite.Equal(rr.Code, http.StatusOK)
}
func TestCosignMiddlewareTestSuite(t *testing.T) {
suite.Run(t, &CosignMiddlewareTestSuite{})
}

View File

@ -40,25 +40,28 @@ func Notary() func(http.Handler) http.Handler {
if af == none {
return errors.New("artifactinfo middleware required before this middleware").WithCode(errors.NotFoundCode)
}
pro, err := project.Ctl.GetByName(ctx, af.ProjectName)
if err != nil {
return err
}
if util.SkipPolicyChecking(r, pro.ProjectID) {
// the artifact is pulling by the scanner, skip the checking
logger.Debugf("artifact %s@%s is pulling by the scanner, skip the checking", af.Repository, af.Digest)
return nil
}
if pro.ContentTrustEnabled() {
art, err := artifact.Ctl.GetByReference(ctx, af.Repository, af.Reference, nil)
if err != nil {
return err
}
if len(af.Digest) == 0 {
art, err := artifact.Ctl.GetByReference(ctx, af.Repository, af.Reference, nil)
if err != nil {
return err
}
af.Digest = art.Digest
}
ok, err := util.SkipPolicyChecking(r, pro.ProjectID, art.ID)
if err != nil {
return err
}
if ok {
logger.Debugf("artifact %s@%s is pulling by the scanner/cosign, skip the checking", af.Repository, af.Digest)
return nil
}
match, err := isArtifactSigned(r, af)
if err != nil {
return err

View File

@ -16,7 +16,11 @@ package contenttrust
import (
"fmt"
"github.com/goharbor/harbor/src/pkg/accessory"
accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
basemodel "github.com/goharbor/harbor/src/pkg/accessory/model/base"
proModels "github.com/goharbor/harbor/src/pkg/project/models"
accessorytesting "github.com/goharbor/harbor/src/testing/pkg/accessory"
"net/http"
"net/http/httptest"
"testing"
@ -45,6 +49,9 @@ type MiddlewareTestSuite struct {
artifact *artifact.Artifact
project *proModels.Project
originalAccessMgr accessory.Manager
accessMgr *accessorytesting.Manager
isArtifactSigned func(req *http.Request, art lib.ArtifactInfo) (bool, error)
next http.Handler
}
@ -58,6 +65,10 @@ func (suite *MiddlewareTestSuite) SetupTest() {
suite.projectController = &projecttesting.Controller{}
project.Ctl = suite.projectController
suite.originalAccessMgr = accessory.Mgr
suite.accessMgr = &accessorytesting.Manager{}
accessory.Mgr = suite.accessMgr
suite.isArtifactSigned = isArtifactSigned
suite.artifact = &artifact.Artifact{}
suite.artifact.Type = image.ArtifactTypeImage
@ -85,6 +96,7 @@ func (suite *MiddlewareTestSuite) SetupTest() {
func (suite *MiddlewareTestSuite) TearDownTest() {
artifact.Ctl = suite.originalArtifactController
project.Ctl = suite.originalProjectController
accessory.Mgr = suite.originalAccessMgr
}
func (suite *MiddlewareTestSuite) makeRequest() *http.Request {
@ -143,6 +155,7 @@ func (suite *MiddlewareTestSuite) TestNoneArtifact() {
func (suite *MiddlewareTestSuite) TestAuthenticatedUserPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("local")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -159,6 +172,7 @@ func (suite *MiddlewareTestSuite) TestAuthenticatedUserPulling() {
func (suite *MiddlewareTestSuite) TestScannerPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("v2token")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -176,6 +190,7 @@ func (suite *MiddlewareTestSuite) TestScannerPulling() {
func (suite *MiddlewareTestSuite) TestUnAuthenticatedUserPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("local")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -188,6 +203,29 @@ func (suite *MiddlewareTestSuite) TestUnAuthenticatedUserPulling() {
suite.Equal(rr.Code, http.StatusPreconditionFailed)
}
// pull cosign signature when policy checker is enabled.
func (suite *MiddlewareTestSuite) TestSignaturePulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
acc := &basemodel.Default{
Data: accessorymodel.AccessoryData{
ID: 1,
ArtifactID: 2,
SubArtifactID: 1,
Type: accessorymodel.TypeCosignSignature,
},
}
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{
acc,
}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
Notary()(suite.next).ServeHTTP(rr, req)
suite.Equal(rr.Code, http.StatusOK)
}
func TestMiddlewareTestSuite(t *testing.T) {
suite.Run(t, &MiddlewareTestSuite{})
}

View File

@ -17,6 +17,9 @@ package util
import (
"fmt"
"github.com/goharbor/harbor/src/common/rbac/project"
"github.com/goharbor/harbor/src/lib/q"
"github.com/goharbor/harbor/src/pkg/accessory"
"github.com/goharbor/harbor/src/pkg/accessory/model"
"net/http"
"path"
"strings"
@ -56,18 +59,27 @@ func ParseProjectName(r *http.Request) string {
}
// SkipPolicyChecking ...
func SkipPolicyChecking(r *http.Request, projectID int64) bool {
func SkipPolicyChecking(r *http.Request, projectID, artID int64) (bool, error) {
secCtx, ok := security.FromContext(r.Context())
// 1, scanner pull access can bypass.
// 2, cosign pull can bypass, it needs to pull the manifest before pushing the signature.
// 3, pull cosign signature can bypass.
if ok && secCtx.Name() == "v2token" {
if secCtx.Can(r.Context(), rbac.ActionScannerPull, project.NewNamespace(projectID).Resource(rbac.ResourceRepository)) ||
(secCtx.Can(r.Context(), rbac.ActionPush, project.NewNamespace(projectID).Resource(rbac.ResourceRepository)) &&
strings.Contains(r.UserAgent(), "cosign")) {
return true
return true, nil
}
}
return false
accs, err := accessory.Mgr.List(r.Context(), q.New(q.KeyWords{"ArtifactID": artID}))
if err != nil {
return false, err
}
if len(accs) > 0 && accs[0].GetData().Type == model.TypeCosignSignature {
return true, nil
}
return false, nil
}

View File

@ -61,12 +61,6 @@ func Middleware() func(http.Handler) http.Handler {
return nil
}
if util.SkipPolicyChecking(r, proj.ProjectID) {
// the artifact is pulling by the scanner, skip the checking
logger.Debugf("artifact %s@%s is pulling by the scanner, skip the checking", info.Repository, info.Reference)
return nil
}
art, err := artifactController.GetByReference(ctx, info.Repository, info.Reference, nil)
if err != nil {
if !errors.IsNotFoundErr(err) {
@ -74,6 +68,14 @@ func Middleware() func(http.Handler) http.Handler {
}
return err
}
ok, err := util.SkipPolicyChecking(r, proj.ProjectID, art.ID)
if err != nil {
return err
}
if ok {
logger.Debugf("artifact %s@%s is pulling by the scanner/cosign, skip the checking", info.Repository, info.Digest)
return nil
}
checker := scanChecker()
scannable, err := checker.IsScannable(ctx, art)

View File

@ -16,7 +16,11 @@ package vulnerable
import (
"fmt"
"github.com/goharbor/harbor/src/pkg/accessory"
accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
basemodel "github.com/goharbor/harbor/src/pkg/accessory/model/base"
proModels "github.com/goharbor/harbor/src/pkg/project/models"
accessorytesting "github.com/goharbor/harbor/src/testing/pkg/accessory"
"net/http"
"net/http/httptest"
"testing"
@ -50,6 +54,9 @@ type MiddlewareTestSuite struct {
originalScanController scan.Controller
scanController *scantesting.Controller
originalAccessMgr accessory.Manager
accessMgr *accessorytesting.Manager
checker *scantesting.Checker
scanChecker func() scan.Checker
@ -68,6 +75,10 @@ func (suite *MiddlewareTestSuite) SetupTest() {
suite.projectController = &projecttesting.Controller{}
projectController = suite.projectController
suite.originalAccessMgr = accessory.Mgr
suite.accessMgr = &accessorytesting.Manager{}
accessory.Mgr = suite.accessMgr
suite.originalScanController = scanController
suite.scanController = &scantesting.Controller{}
scanController = suite.scanController
@ -103,7 +114,7 @@ func (suite *MiddlewareTestSuite) TearDownTest() {
artifactController = suite.originalArtifactController
projectController = suite.originalProjectController
scanController = suite.originalScanController
accessory.Mgr = suite.originalAccessMgr
scanChecker = suite.scanChecker
}
@ -167,6 +178,7 @@ func (suite *MiddlewareTestSuite) TestPreventionDisabled() {
func (suite *MiddlewareTestSuite) TestNonScannerPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("local")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -183,6 +195,7 @@ func (suite *MiddlewareTestSuite) TestNonScannerPulling() {
func (suite *MiddlewareTestSuite) TestScannerPulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
securityCtx := &securitytesting.Context{}
mock.OnAnything(securityCtx, "Name").Return("v2token")
mock.OnAnything(securityCtx, "Can").Return(true, nil)
@ -199,6 +212,7 @@ func (suite *MiddlewareTestSuite) TestCheckIsScannableFailed() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(false, fmt.Errorf("error"))
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
@ -211,6 +225,7 @@ func (suite *MiddlewareTestSuite) TestArtifactIsNotScannable() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(false, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
@ -224,6 +239,7 @@ func (suite *MiddlewareTestSuite) TestArtifactNotScanned() {
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.scanController, "GetVulnerable").Return(nil, errors.NotFoundError(nil))
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
@ -237,6 +253,7 @@ func (suite *MiddlewareTestSuite) TestArtifactScanFailed() {
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.scanController, "GetVulnerable").Return(&scan.Vulnerable{ScanStatus: "Error"}, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
@ -250,6 +267,7 @@ func (suite *MiddlewareTestSuite) TestGetVulnerableFailed() {
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.scanController, "GetVulnerable").Return(nil, fmt.Errorf("error"))
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
@ -262,6 +280,7 @@ func (suite *MiddlewareTestSuite) TestNoVulnerabilities() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
mock.OnAnything(suite.scanController, "GetVulnerable").Return(&scan.Vulnerable{
ScanStatus: "Success",
CVEBypassed: []string{"cve-2020"},
@ -279,6 +298,7 @@ func (suite *MiddlewareTestSuite) TestAllowed() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
mock.OnAnything(suite.scanController, "GetVulnerable").Return(&scan.Vulnerable{
ScanStatus: "Success",
Severity: &low,
@ -297,6 +317,7 @@ func (suite *MiddlewareTestSuite) TestPrevented() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
critical := vuln.Critical
@ -342,6 +363,7 @@ func (suite *MiddlewareTestSuite) TestArtifactIsImageIndex() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
mock.OnAnything(suite.checker, "IsScannable").Return(true, nil)
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{}, nil)
mock.OnAnything(suite.scanController, "GetVulnerable").Return(&scan.Vulnerable{
ScanStatus: "Success",
Severity: &critical,
@ -354,6 +376,29 @@ func (suite *MiddlewareTestSuite) TestArtifactIsImageIndex() {
suite.Equal(rr.Code, http.StatusOK)
}
// pull cosign signature when policy checker is enabled.
func (suite *MiddlewareTestSuite) TestSignaturePulling() {
mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
mock.OnAnything(suite.projectController, "Get").Return(suite.project, nil)
acc := &basemodel.Default{
Data: accessorymodel.AccessoryData{
ID: 1,
ArtifactID: 2,
SubArtifactID: 1,
Type: accessorymodel.TypeCosignSignature,
},
}
mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{
acc,
}, nil)
req := suite.makeRequest()
rr := httptest.NewRecorder()
Middleware()(suite.next).ServeHTTP(rr, req)
suite.Equal(rr.Code, http.StatusOK)
}
func TestMiddlewareTestSuite(t *testing.T) {
suite.Run(t, &MiddlewareTestSuite{})
}