Skip to content

Commit

Permalink
enable notary v2 policy checker
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
wy65701436 committed Jul 13, 2023
1 parent 5cce621 commit 7567e68
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package contenttrust

import (
"context"
"net/http"

"github.com/goharbor/harbor/src/controller/artifact"
Expand All @@ -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 {
Expand All @@ -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 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -111,49 +111,49 @@ 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)

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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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{
Expand All @@ -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{})
}
6 changes: 4 additions & 2 deletions src/server/middleware/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/server/registry/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ 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().
Method(http.MethodHead).
Path("/*/manifests/:reference").
Middleware(metric.InjectOpIDMiddleware(metric.ManifestOperationID)).
Middleware(repoproxy.ManifestMiddleware()).
Middleware(contenttrust.Cosign()).
Middleware(contenttrust.ContentTrust()).
Middleware(vulnerable.Middleware()).
HandlerFunc(getManifest)
root.NewRoute().
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def testProjectLevelPolicyContentTrust(self):
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);
Expand Down Expand Up @@ -79,12 +79,12 @@ def testProjectLevelPolicyContentTrust(self):
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()
Expand Down
Loading

0 comments on commit 7567e68

Please sign in to comment.