diff --git a/src/server/middleware/contenttrust/contenttrust.go b/src/server/middleware/contenttrust/contenttrust.go index d2a30cd95..b37ac216c 100644 --- a/src/server/middleware/contenttrust/contenttrust.go +++ b/src/server/middleware/contenttrust/contenttrust.go @@ -1,83 +1,66 @@ package contenttrust import ( - "net/http" - "net/http/httptest" - + "fmt" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/controller/project" + "github.com/goharbor/harbor/src/jobservice/logger" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/pkg/signature" - serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" + "net/http" ) -// NotaryEndpoint ... -var NotaryEndpoint = "" +var ( + // isArtifactSigned use the sign manager to check the signature, it could handle pull by tag or digtest + // if pull by digest, any tag of the artifact is signed, will return true. + isArtifactSigned = func(req *http.Request, art lib.ArtifactInfo) (bool, error) { + checker, err := signature.GetManager().GetCheckerByRepo(req.Context(), art.Repository) + if err != nil { + return false, err + } + return checker.IsArtifactSigned(art.Digest), nil + } +) // Middleware handle docker pull content trust check func Middleware() func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - doContentTrustCheck, mf := validate(req) - if !doContentTrustCheck { - next.ServeHTTP(rw, req) - return - } - rec := httptest.NewRecorder() - next.ServeHTTP(rec, req) - if rec.Result().StatusCode == http.StatusOK { - match, err := isArtifactSigned(req, mf) - if err != nil { - serror.SendError(rw, err) - return - } - if !match { - pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Notary.") - serror.SendError(rw, pkgE) - return - } - } - middleware.CopyResp(rec, rw) - }) - } -} + return middleware.BeforeRequest(func(r *http.Request) error { + ctx := r.Context() + none := lib.ArtifactInfo{} + if err := middleware.EnsureArtifactDigest(ctx); err != nil { + return err + } + af := lib.GetArtifactInfo(ctx) + if af == none { + return fmt.Errorf("artifactinfo middleware required before this middleware") + } + pro, err := project.Ctl.GetByName(ctx, af.ProjectName) + if err != nil { + return err + } + securityCtx, ok := security.FromContext(ctx) + // only authenticated robot account with scanner pull access can bypass. + if ok && securityCtx.IsAuthenticated() && + securityCtx.Name() == "robot" && + securityCtx.Can(rbac.ActionScannerPull, rbac.NewProjectNamespace(pro.ProjectID).Resource(rbac.ResourceRepository)) { + // 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 + } -func validate(req *http.Request) (bool, lib.ArtifactInfo) { - none := lib.ArtifactInfo{} - if err := middleware.EnsureArtifactDigest(req.Context()); err != nil { - return false, none - } - af := lib.GetArtifactInfo(req.Context()) - if af == none { - return false, none - } - pro, err := project.Ctl.GetByName(req.Context(), af.ProjectName) - if err != nil { - return false, none - } - resource := rbac.NewProjectNamespace(pro.ProjectID).Resource(rbac.ResourceRepository) - securityCtx, ok := security.FromContext(req.Context()) - if !ok { - return false, none - } - if !securityCtx.Can(rbac.ActionScannerPull, resource) { - return false, none - } - if !middleware.GetPolicyChecker().ContentTrustEnabled(af.ProjectName) { - return false, af - } - return true, af -} - -// isArtifactSigned use the sign manager to check the signature, it could handle pull by tag or digtest -// if pull by digest, any tag of the artifact is signed, will return true. -func isArtifactSigned(req *http.Request, art lib.ArtifactInfo) (bool, error) { - checker, err := signature.GetManager().GetCheckerByRepo(req.Context(), art.Repository) - if err != nil { - return false, err - } - return checker.IsArtifactSigned(art.Digest), nil + if pro.ContentTrustEnabled() { + match, err := isArtifactSigned(r, af) + if err != nil { + return err + } + if !match { + pkgE := errors.New(nil).WithCode(errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Notary.") + return pkgE + } + } + return nil + }) } diff --git a/src/server/middleware/contenttrust/contenttrust_test.go b/src/server/middleware/contenttrust/contenttrust_test.go index c1f720fcc..9189e6ec7 100644 --- a/src/server/middleware/contenttrust/contenttrust_test.go +++ b/src/server/middleware/contenttrust/contenttrust_test.go @@ -1 +1,183 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package contenttrust + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/controller/artifact" + "github.com/goharbor/harbor/src/controller/project" + "github.com/goharbor/harbor/src/lib" + securitytesting "github.com/goharbor/harbor/src/testing/common/security" + artifacttesting "github.com/goharbor/harbor/src/testing/controller/artifact" + projecttesting "github.com/goharbor/harbor/src/testing/controller/project" + "github.com/goharbor/harbor/src/testing/mock" + "github.com/stretchr/testify/suite" +) + +type MiddlewareTestSuite struct { + suite.Suite + + originalArtifactController artifact.Controller + artifactController *artifacttesting.Controller + + originalProjectController project.Controller + projectController *projecttesting.Controller + + artifact *artifact.Artifact + project *models.Project + + isArtifactSigned func(req *http.Request, art lib.ArtifactInfo) (bool, error) + next http.Handler +} + +func (suite *MiddlewareTestSuite) SetupTest() { + suite.originalArtifactController = artifact.Ctl + suite.artifactController = &artifacttesting.Controller{} + artifact.Ctl = suite.artifactController + + suite.originalProjectController = project.Ctl + suite.projectController = &projecttesting.Controller{} + project.Ctl = suite.projectController + + suite.isArtifactSigned = isArtifactSigned + suite.artifact = &artifact.Artifact{} + suite.artifact.Type = artifact.ImageType + suite.artifact.ProjectID = 1 + suite.artifact.RepositoryName = "library/photon" + suite.artifact.Digest = "digest" + + suite.project = &models.Project{ + ProjectID: suite.artifact.ProjectID, + Name: "library", + Metadata: map[string]string{ + models.ProMetaEnableContentTrust: "true", + }, + } + + suite.next = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + isArtifactSigned = func(req *http.Request, art lib.ArtifactInfo) (bool, error) { + return false, nil + } +} + +func (suite *MiddlewareTestSuite) TearDownTest() { + artifact.Ctl = suite.originalArtifactController + project.Ctl = suite.originalProjectController +} + +func (suite *MiddlewareTestSuite) makeRequest() *http.Request { + req := httptest.NewRequest("GET", "/v1/library/photon/manifests/2.0", nil) + info := lib.ArtifactInfo{ + Repository: "library/photon", + Reference: "2.0", + Tag: "2.0", + Digest: "", + } + return req.WithContext(lib.WithArtifactInfo(req.Context(), info)) +} + +func (suite *MiddlewareTestSuite) TestGetArtifactFailed() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(nil, fmt.Errorf("error")) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusInternalServerError) +} + +func (suite *MiddlewareTestSuite) 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() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusInternalServerError) +} + +func (suite *MiddlewareTestSuite) TestContentTrustDisabled() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + suite.project.Metadata[models.ProMetaEnableContentTrust] = "false" + mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusOK) +} + +func (suite *MiddlewareTestSuite) TestAuthenticatedUserPulling() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) + securityCtx := &securitytesting.Context{} + mock.OnAnything(securityCtx, "Name").Return("local") + mock.OnAnything(securityCtx, "Can").Return(true, nil) + mock.OnAnything(securityCtx, "IsAuthenticated").Return(true) + + req := suite.makeRequest() + req = req.WithContext(security.NewContext(req.Context(), securityCtx)) + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusPreconditionFailed) +} + +func (suite *MiddlewareTestSuite) TestScannerPulling() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) + securityCtx := &securitytesting.Context{} + mock.OnAnything(securityCtx, "Name").Return("robot") + mock.OnAnything(securityCtx, "Can").Return(true, nil) + mock.OnAnything(securityCtx, "IsAuthenticated").Return(true) + + req := suite.makeRequest() + req = req.WithContext(security.NewContext(req.Context(), securityCtx)) + rr := httptest.NewRecorder() + + Middleware()(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 *MiddlewareTestSuite) TestUnAuthenticatedUserPulling() { + mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil) + mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil) + securityCtx := &securitytesting.Context{} + mock.OnAnything(securityCtx, "Name").Return("local") + mock.OnAnything(securityCtx, "Can").Return(true, nil) + mock.OnAnything(securityCtx, "IsAuthenticated").Return(false) + + req := suite.makeRequest() + rr := httptest.NewRecorder() + + Middleware()(suite.next).ServeHTTP(rr, req) + suite.Equal(rr.Code, http.StatusPreconditionFailed) +} + +func TestMiddlewareTestSuite(t *testing.T) { + suite.Run(t, &MiddlewareTestSuite{}) +} diff --git a/src/server/middleware/util.go b/src/server/middleware/util.go index 75296e1d6..e2ff684f9 100644 --- a/src/server/middleware/util.go +++ b/src/server/middleware/util.go @@ -8,10 +8,7 @@ import ( "regexp" "github.com/docker/distribution/reference" - "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/controller/artifact" - "github.com/goharbor/harbor/src/core/config" - "github.com/goharbor/harbor/src/core/promgr" "github.com/goharbor/harbor/src/lib" "github.com/opencontainers/go-digest" ) @@ -65,40 +62,3 @@ func CopyResp(rec *httptest.ResponseRecorder, rw http.ResponseWriter) { rw.WriteHeader(rec.Result().StatusCode) rw.Write(rec.Body.Bytes()) } - -// PolicyChecker checks the policy of a project by project name, to determine if it's needed to check the image's status under this project. -type PolicyChecker interface { - // contentTrustEnabled returns whether a project has enabled content trust. - ContentTrustEnabled(name string) bool -} - -// PmsPolicyChecker ... -type PmsPolicyChecker struct { - pm promgr.ProjectManager -} - -// ContentTrustEnabled ... -func (pc PmsPolicyChecker) ContentTrustEnabled(name string) bool { - project, err := pc.pm.Get(name) - if err != nil { - log.Errorf("Unexpected error when getting the project, error: %v", err) - return true - } - if project == nil { - log.Debugf("project %s not found", name) - return false - } - return project.ContentTrustEnabled() -} - -// NewPMSPolicyChecker returns an instance of an pmsPolicyChecker -func NewPMSPolicyChecker(pm promgr.ProjectManager) PolicyChecker { - return &PmsPolicyChecker{ - pm: pm, - } -} - -// GetPolicyChecker ... -func GetPolicyChecker() PolicyChecker { - return NewPMSPolicyChecker(config.GlobalProjectMgr) -}