enable notary v2 policy checker (#18927)

add notary v2 pull policy, when it enables, the artifact cannot be pull without the notation signature.

Signed-off-by: wang yan <wangyan@vmware.com>

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
+// ContentTrust handle docker pull content trust check
-func Cosign() func(http.Handler) http.Handler {
+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,56 @@ 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 signature.
 		if pro.ContentTrustCosignEnabled() {
-			art, err := artifact.Ctl.GetByReference(ctx, af.Repository, af.Reference, &artifact.Option{
+			if err := signatureChecking(ctx, r, af, pro.ProjectID, model.TypeCosignSignature); err != nil {
-				WithAccessory: true,
+				return err
-			})
+			}
-			if err != nil {
+		}
+		if pro.ContentTrustEnabled() {
+			if err := signatureChecking(ctx, r, af, pro.ProjectID, model.TypeNotationSignature); err != nil {
 				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
-			}
-
-			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
-			}
 		}
-
 		return nil
 	})
 }
+
+func signatureChecking(ctx context.Context, r *http.Request, af lib.ArtifactInfo, projectID int64, signatureType string) error {
+	logger := log.G(ctx)
+
+	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
+}

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{})
 }

src/server/middleware/util/util.go

@@ -63,12 +63,14 @@ 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.
+	// 2, cosign/notation 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")) {
+				strings.Contains(r.UserAgent(), "cosign")) ||
+			(secCtx.Can(r.Context(), rbac.ActionPush, project.NewNamespace(projectID).Resource(rbac.ResourceRepository)) &&
+				strings.Contains(r.UserAgent(), "notation")) {
 			return true, nil
 		}
 	}

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().

tests/apitests/python/test_project_level_policy_content_trust.py

@@ -45,7 +45,7 @@ class TestProjects(unittest.TestCase):
             4. Image(IA) should exist;
             5. Pull image(IA) successfully;
             6. Enable content trust in project(PA) configuration;
-            7. Pull image(IA) failed and the reason is "The image is not signed in Cosign".
+            7. Pull image(IA) failed and the reason is "The image is not signed".
         Tear down:
             1. Delete repository(RA) by user(UA);
             2. Delete project(PA);
@@ -79,12 +79,12 @@ class TestProjects(unittest.TestCase):
         self.project.update_project(TestProjects.project_content_trust_id, metadata = {"enable_content_trust_cosign": "true"}, **TestProjects.USER_CONTENT_TRUST_CLIENT)
         self.project.get_project(TestProjects.project_content_trust_id)
 
-        #7. Pull image(IA) failed and the reason is "The image is not signed in Cosign".
+        #7. Pull image(IA) failed and the reason is "The image is not signed".
         docker_image_clean_all()
         restart_process("containerd")
         restart_process("dockerd")
         time.sleep(30)
-        pull_harbor_image(harbor_server, ADMIN_CLIENT["username"], ADMIN_CLIENT["password"], TestProjects.repo_name, tag, expected_error_message = "The image is not signed in Cosign")
+        pull_harbor_image(harbor_server, ADMIN_CLIENT["username"], ADMIN_CLIENT["password"], TestProjects.repo_name, tag, expected_error_message = "The image is not signed")
 
 if __name__ == '__main__':
     unittest.main()

tests/robot-cases/Group1-Nightly/Common.robot

@@ -773,7 +773,7 @@ Test Case - Cosign And Cosign Deployment Security Policy
     Go Into Project  project${d}
     Go Into Repo  project${d}  ${image}
     Should Not Be Signed By Cosign  ${tag}
-    Cannot Pull Image  ${ip}  ${user}  ${pwd}  project${d}  ${image}:${tag}  err_msg=The image is not signed in Cosign.
+    Cannot Pull Image  ${ip}  ${user}  ${pwd}  project${d}  ${image}:${tag}  err_msg=The image is not signed.
     Cosign Generate Key Pair
     Cosign Verify  ${ip}/project${d}/${image}:${tag}  ${false}