diff --git a/src/server/middleware/contenttrust/cosign.go b/src/server/middleware/contenttrust/contentrust.go similarity index 51% rename from src/server/middleware/contenttrust/cosign.go rename to src/server/middleware/contenttrust/contentrust.go index 6fba6eaa0295..69d729425e82 100644 --- a/src/server/middleware/contenttrust/cosign.go +++ b/src/server/middleware/contenttrust/contentrust.go @@ -15,6 +15,7 @@ package contenttrust import ( + "context" "net/http" "github.com/goharbor/harbor/src/controller/artifact" @@ -27,13 +28,11 @@ import ( "github.com/goharbor/harbor/src/server/middleware/util" ) -// Cosign handle docker pull content trust check -func Cosign() func(http.Handler) http.Handler { +// ContentTrust handle docker pull content trust check +func ContentTrust() func(http.Handler) http.Handler { return middleware.BeforeRequest(func(r *http.Request) error { ctx := r.Context() - logger := log.G(ctx) - none := lib.ArtifactInfo{} af := lib.GetArtifactInfo(ctx) if af == none { @@ -44,42 +43,57 @@ func Cosign() func(http.Handler) http.Handler { return err } - // If cosign policy enabled, it has to at least have one cosign signature. + // If signature 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{ - WithAccessory: true, - }) - if err != nil { + if err := signatureChecking(ctx, r, af, pro.ProjectID, model.TypeCosignSignature); err != nil { return err } - ok, err := util.SkipPolicyChecking(r, pro.ProjectID, art.ID) - if err != nil { + } + if pro.ContentTrustEnabled() { + if err := signatureChecking(ctx, r, af, pro.ProjectID, model.TypeNotationSignature); 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 - } + } + return nil + }) +} - if len(art.Accessories) == 0 { - pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Cosign.") - return pkgE - } +func signatureChecking(ctx context.Context, r *http.Request, af lib.ArtifactInfo, projectID int64, signatureType string) error { + logger := log.G(ctx) - var hasCosignSignature bool - for _, acc := range art.Accessories { - if acc.GetData().Type == model.TypeCosignSignature { - hasCosignSignature = true - break - } - } - if !hasCosignSignature { - pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Cosign.") - return pkgE - } - } + art, err := artifact.Ctl.GetByReference(ctx, af.Repository, af.Reference, &artifact.Option{ + WithAccessory: true, + }) + if err != nil { + return err + } + ok, err := util.SkipPolicyChecking(r, projectID, art.ID) + if err != nil { + return err + } + if ok { + logger.Debugf("skip the checking of pulling artifact %s@%s", af.Repository, af.Digest) return nil - }) + } + + if len(art.Accessories) == 0 { + pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed.") + return pkgE + } + + var hasSignature bool + for _, acc := range art.Accessories { + if acc.GetData().Type == signatureType { + hasSignature = true + break + } + } + if !hasSignature { + pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed.") + return pkgE + } + + return nil } diff --git a/src/server/middleware/contenttrust/cosign_test.go b/src/server/middleware/contenttrust/contentrust_test.go similarity index 68% rename from src/server/middleware/contenttrust/cosign_test.go rename to src/server/middleware/contenttrust/contentrust_test.go index 0935b9503a40..6db4174a5622 100644 --- a/src/server/middleware/contenttrust/cosign_test.go +++ b/src/server/middleware/contenttrust/contentrust_test.go @@ -38,7 +38,7 @@ import ( accessorytesting "github.com/goharbor/harbor/src/testing/pkg/accessory" ) -type CosignMiddlewareTestSuite struct { +type ContentTrustMiddlewareTestSuite struct { suite.Suite originalArtifactController artifact.Controller @@ -56,7 +56,7 @@ type CosignMiddlewareTestSuite struct { next http.Handler } -func (suite *CosignMiddlewareTestSuite) SetupTest() { +func (suite *ContentTrustMiddlewareTestSuite) SetupTest() { suite.originalArtifactController = artifact.Ctl suite.artifactController = &artifacttesting.Controller{} artifact.Ctl = suite.artifactController @@ -89,13 +89,13 @@ func (suite *CosignMiddlewareTestSuite) SetupTest() { } -func (suite *CosignMiddlewareTestSuite) TearDownTest() { +func (suite *ContentTrustMiddlewareTestSuite) TearDownTest() { artifact.Ctl = suite.originalArtifactController project.Ctl = suite.originalProjectController accessory.Mgr = suite.originalAccessMgr } -func (suite *CosignMiddlewareTestSuite) makeRequest(setHeader ...bool) *http.Request { +func (suite *ContentTrustMiddlewareTestSuite) makeRequest(setHeader ...bool) *http.Request { req := httptest.NewRequest("GET", "/v1/library/photon/manifests/2.0", nil) info := lib.ArtifactInfo{ Repository: "library/photon", @@ -111,29 +111,29 @@ func (suite *CosignMiddlewareTestSuite) makeRequest(setHeader ...bool) *http.Req return req.WithContext(lib.WithArtifactInfo(req.Context(), info)) } -func (suite *CosignMiddlewareTestSuite) TestGetArtifactFailed() { +func (suite *ContentTrustMiddlewareTestSuite) TestGetArtifactFailed() { mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) mock.OnAnything(suite.artifactController, "GetByReference").Return(nil, fmt.Errorf("error")) req := suite.makeRequest() rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusInternalServerError) } -func (suite *CosignMiddlewareTestSuite) TestGetProjectFailed() { +func (suite *ContentTrustMiddlewareTestSuite) TestGetProjectFailed() { mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) mock.OnAnything(suite.projectController, "GetByName").Return(nil, fmt.Errorf("err")) req := suite.makeRequest() rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusInternalServerError) } -func (suite *CosignMiddlewareTestSuite) TestContentTrustDisabled() { +func (suite *ContentTrustMiddlewareTestSuite) TestContentTrustDisabled() { mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) suite.project.Metadata[proModels.ProMetaEnableContentTrustCosign] = "false" mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) @@ -141,19 +141,19 @@ func (suite *CosignMiddlewareTestSuite) TestContentTrustDisabled() { req := suite.makeRequest() rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusOK) } -func (suite *CosignMiddlewareTestSuite) TestNoneArtifact() { +func (suite *ContentTrustMiddlewareTestSuite) TestNoneArtifact() { req := httptest.NewRequest("GET", "/v1/library/photon/manifests/nonexist", nil) rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusNotFound) } -func (suite *CosignMiddlewareTestSuite) TestAuthenticatedUserPulling() { +func (suite *ContentTrustMiddlewareTestSuite) 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) @@ -166,11 +166,11 @@ func (suite *CosignMiddlewareTestSuite) TestAuthenticatedUserPulling() { req = req.WithContext(security.NewContext(req.Context(), securityCtx)) rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusPreconditionFailed) } -func (suite *CosignMiddlewareTestSuite) TestScannerPulling() { +func (suite *ContentTrustMiddlewareTestSuite) 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) @@ -183,11 +183,11 @@ func (suite *CosignMiddlewareTestSuite) TestScannerPulling() { req = req.WithContext(security.NewContext(req.Context(), securityCtx)) rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusOK) } -func (suite *CosignMiddlewareTestSuite) TestCosignPulling() { +func (suite *ContentTrustMiddlewareTestSuite) 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) @@ -200,12 +200,12 @@ func (suite *CosignMiddlewareTestSuite) TestCosignPulling() { req = req.WithContext(security.NewContext(req.Context(), securityCtx)) rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusOK) } // pull a public project a un-signed image when policy checker is enabled. -func (suite *CosignMiddlewareTestSuite) TestUnAuthenticatedUserPulling() { +func (suite *ContentTrustMiddlewareTestSuite) 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) @@ -217,12 +217,12 @@ func (suite *CosignMiddlewareTestSuite) TestUnAuthenticatedUserPulling() { req := suite.makeRequest() rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusPreconditionFailed) } // pull cosign signature when policy checker is enabled. -func (suite *CosignMiddlewareTestSuite) TestSignaturePulling() { +func (suite *ContentTrustMiddlewareTestSuite) TestCosignSignaturePulling() { mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) acc := &basemodel.Default{ @@ -240,10 +240,70 @@ func (suite *CosignMiddlewareTestSuite) TestSignaturePulling() { req := suite.makeRequest() rr := httptest.NewRecorder() - Cosign()(suite.next).ServeHTTP(rr, req) + ContentTrust()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusOK) +} + +// notation signature checking when policy checker is enabled. +func (suite *ContentTrustMiddlewareTestSuite) TestNotationSignaturePulling() { + 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, + SubArtifactDigest: suite.artifact.Digest, + Type: accessorymodel.TypeNotationSignature, + }, + } + suite.project.Metadata[proModels.ProMetaEnableContentTrust] = "true" + suite.project.Metadata[proModels.ProMetaEnableContentTrustCosign] = "false" + suite.artifact.Accessories = []accessorymodel.Accessory{acc} + mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{ + acc, + }, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + ContentTrust()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusOK) +} + +// notation & cosign signature when both policy checker are enabled. +func (suite *ContentTrustMiddlewareTestSuite) TestBothSignaturePulling() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) + acc1 := &basemodel.Default{ + Data: accessorymodel.AccessoryData{ + ID: 1, + ArtifactID: 2, + SubArtifactDigest: suite.artifact.Digest, + Type: accessorymodel.TypeNotationSignature, + }, + } + acc2 := &basemodel.Default{ + Data: accessorymodel.AccessoryData{ + ID: 1, + ArtifactID: 3, + SubArtifactDigest: suite.artifact.Digest, + Type: accessorymodel.TypeCosignSignature, + }, + } + suite.project.Metadata[proModels.ProMetaEnableContentTrust] = "true" + suite.project.Metadata[proModels.ProMetaEnableContentTrustCosign] = "true" + suite.artifact.Accessories = []accessorymodel.Accessory{acc1, acc2} + mock.OnAnything(suite.accessMgr, "List").Return([]accessorymodel.Accessory{ + acc1, acc2, + }, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + ContentTrust()(suite.next).ServeHTTP(rr, req) suite.Equal(rr.Code, http.StatusOK) } func TestCosignMiddlewareTestSuite(t *testing.T) { - suite.Run(t, &CosignMiddlewareTestSuite{}) + suite.Run(t, &ContentTrustMiddlewareTestSuite{}) } diff --git a/src/server/registry/route.go b/src/server/registry/route.go index 7c783e9483ea..833f8c78de65 100644 --- a/src/server/registry/route.go +++ b/src/server/registry/route.go @@ -54,7 +54,7 @@ func RegisterRoutes() { Path("/*/manifests/:reference"). Middleware(metric.InjectOpIDMiddleware(metric.ManifestOperationID)). Middleware(repoproxy.ManifestMiddleware()). - Middleware(contenttrust.Cosign()). + Middleware(contenttrust.ContentTrust()). Middleware(vulnerable.Middleware()). HandlerFunc(getManifest) root.NewRoute(). @@ -62,7 +62,7 @@ func RegisterRoutes() { Path("/*/manifests/:reference"). Middleware(metric.InjectOpIDMiddleware(metric.ManifestOperationID)). Middleware(repoproxy.ManifestMiddleware()). - Middleware(contenttrust.Cosign()). + Middleware(contenttrust.ContentTrust()). Middleware(vulnerable.Middleware()). HandlerFunc(getManifest) root.NewRoute().