From 2848001dfb08501f1cda250883d081c495a97bb0 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Fri, 18 Mar 2022 14:04:32 +0300 Subject: [PATCH] [#1246] object/acl: Return more concise description for eACL errors Signed-off-by: Evgenii Stratonikov --- pkg/services/object/acl/acl.go | 43 +++++++++++++++++--------- pkg/services/object/acl/v2/errors.go | 9 +++--- pkg/services/object/acl/v2/service.go | 44 +++++++++++++-------------- pkg/services/object/acl/v2/types.go | 6 ++-- 4 files changed, 59 insertions(+), 43 deletions(-) diff --git a/pkg/services/object/acl/acl.go b/pkg/services/object/acl/acl.go index e6ce799f2..64d76daaf 100644 --- a/pkg/services/object/acl/acl.go +++ b/pkg/services/object/acl/acl.go @@ -57,6 +57,15 @@ type Checker struct { state netmap.State } +// Various EACL check errors. +var ( + errEACLDeniedByRule = errors.New("denied by rule") + errBearerExpired = errors.New("bearer token has expired") + errBearerInvalidSignature = errors.New("bearer token has invalid signature") + errBearerNotSignedByOwner = errors.New("bearer token is not signed by the container owner") + errBearerInvalidOwner = errors.New("bearer token owner differs from the request sender") +) + // NewChecker creates Checker. // Panics if at least one of the parameter is nil. func NewChecker(prm *CheckerPrm) *Checker { @@ -124,9 +133,9 @@ func (c *Checker) StickyBitCheck(info v2.RequestInfo, owner *owner.ID) bool { } // CheckEACL is a main check function for extended ACL. -func (c *Checker) CheckEACL(msg interface{}, reqInfo v2.RequestInfo) bool { +func (c *Checker) CheckEACL(msg interface{}, reqInfo v2.RequestInfo) error { if basicACLHelper(reqInfo.BasicACL()).Final() { - return true + return nil } // if bearer token is not allowed, then ignore it @@ -142,15 +151,18 @@ func (c *Checker) CheckEACL(msg interface{}, reqInfo v2.RequestInfo) bool { if reqInfo.Bearer().Empty() { table, err = c.eaclSrc.GetEACL(reqInfo.ContainerID()) if err != nil { - return errors.Is(err, container.ErrEACLNotFound) + if errors.Is(err, container.ErrEACLNotFound) { + return nil + } + return err } } else { table = reqInfo.Bearer().EACLTable() } // if bearer token is not present, isValidBearer returns true - if !isValidBearer(reqInfo, c.state) { - return false + if err := isValidBearer(reqInfo, c.state); err != nil { + return err } hdrSrcOpts := make([]eaclV2.Option, 0, 3) @@ -186,36 +198,39 @@ func (c *Checker) CheckEACL(msg interface{}, reqInfo v2.RequestInfo) bool { WithEACLTable(table), ) - return action == eaclSDK.ActionAllow + if action != eaclSDK.ActionAllow { + return errEACLDeniedByRule + } + return nil } -// isValidBearer returns true if bearer token correctly signed by authorized +// isValidBearer checks whether bearer token was correctly signed by authorized // entity. This method might be defined on whole ACL service because it will // require fetching current epoch to check lifetime. -func isValidBearer(reqInfo v2.RequestInfo, st netmap.State) bool { +func isValidBearer(reqInfo v2.RequestInfo, st netmap.State) error { token := reqInfo.Bearer() // 0. Check if bearer token is present in reqInfo. It might be non nil // empty structure. if token == nil || token.Empty() { - return true + return nil } // 1. First check token lifetime. Simplest verification. if !isValidLifetime(token, st.CurrentEpoch()) { - return false + return errBearerExpired } // 2. Then check if bearer token is signed correctly. if err := token.VerifySignature(); err != nil { - return false // invalid signature + return errBearerInvalidSignature } // 3. Then check if container owner signed this token. tokenIssuerKey := unmarshalPublicKey(token.Signature().Key()) if !isOwnerFromKey(reqInfo.ContainerOwner(), tokenIssuerKey) { // TODO: #767 in this case we can issue all owner keys from neofs.id and check once again - return false + return errBearerNotSignedByOwner } // 4. Then check if request sender has rights to use this token. @@ -224,11 +239,11 @@ func isValidBearer(reqInfo v2.RequestInfo, st netmap.State) bool { requestSenderKey := unmarshalPublicKey(reqInfo.SenderKey()) if !isOwnerFromKey(tokenOwnerField, requestSenderKey) { // TODO: #767 in this case we can issue all owner keys from neofs.id and check once again - return false + return errBearerInvalidOwner } } - return true + return nil } func isValidLifetime(t *bearerSDK.BearerToken, epoch uint64) bool { diff --git a/pkg/services/object/acl/v2/errors.go b/pkg/services/object/acl/v2/errors.go index 3baaff466..249db466e 100644 --- a/pkg/services/object/acl/v2/errors.go +++ b/pkg/services/object/acl/v2/errors.go @@ -17,18 +17,19 @@ var ( ErrInvalidVerb = errors.New("session token verb is invalid") ) -const accessDeniedReasonFmt = "access to operation %v is denied by %s check" +const accessDeniedACLReasonFmt = "access to operation %s is denied by basic ACL check" +const accessDeniedEACLReasonFmt = "access to operation %s is denied by extended ACL check: %v" func basicACLErr(info RequestInfo) error { var errAccessDenied apistatus.ObjectAccessDenied - errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedReasonFmt, info.operation, "basic ACL")) + errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedACLReasonFmt, info.operation)) return errAccessDenied } -func eACLErr(info RequestInfo) error { +func eACLErr(info RequestInfo, err error) error { var errAccessDenied apistatus.ObjectAccessDenied - errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedReasonFmt, info.operation, "extended ACL")) + errAccessDenied.WriteReason(fmt.Sprintf(accessDeniedEACLReasonFmt, info.operation, err)) return errAccessDenied } diff --git a/pkg/services/object/acl/v2/service.go b/pkg/services/object/acl/v2/service.go index 4d32ad384..254e77917 100644 --- a/pkg/services/object/acl/v2/service.go +++ b/pkg/services/object/acl/v2/service.go @@ -131,8 +131,8 @@ func (b Service) Get(request *objectV2.GetRequest, stream object.GetObjectStream if !b.checker.CheckBasicACL(reqInfo) { return basicACLErr(reqInfo) - } else if !b.checker.CheckEACL(request, reqInfo) { - return eACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) } return b.next.Get(request, &getStreamBasicChecker{ @@ -178,14 +178,14 @@ func (b Service) Head( if !b.checker.CheckBasicACL(reqInfo) { return nil, basicACLErr(reqInfo) - } else if !b.checker.CheckEACL(request, reqInfo) { - return nil, eACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) } resp, err := b.next.Head(ctx, request) if err == nil { - if !b.checker.CheckEACL(resp, reqInfo) { - err = eACLErr(reqInfo) + if err = b.checker.CheckEACL(resp, reqInfo); err != nil { + err = eACLErr(reqInfo, err) } } @@ -216,8 +216,8 @@ func (b Service) Search(request *objectV2.SearchRequest, stream object.SearchStr if !b.checker.CheckBasicACL(reqInfo) { return basicACLErr(reqInfo) - } else if !b.checker.CheckEACL(request, reqInfo) { - return eACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) } return b.next.Search(request, &searchStreamBasicChecker{ @@ -254,8 +254,8 @@ func (b Service) Delete( if !b.checker.CheckBasicACL(reqInfo) { return nil, basicACLErr(reqInfo) - } else if !b.checker.CheckEACL(request, reqInfo) { - return nil, eACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) } return b.next.Delete(ctx, request) @@ -286,8 +286,8 @@ func (b Service) GetRange(request *objectV2.GetRangeRequest, stream object.GetOb if !b.checker.CheckBasicACL(reqInfo) { return basicACLErr(reqInfo) - } else if !b.checker.CheckEACL(request, reqInfo) { - return eACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) } return b.next.GetRange(request, &rangeStreamBasicChecker{ @@ -324,8 +324,8 @@ func (b Service) GetRangeHash( if !b.checker.CheckBasicACL(reqInfo) { return nil, basicACLErr(reqInfo) - } else if !b.checker.CheckEACL(request, reqInfo) { - return nil, eACLErr(reqInfo) + } else if err := b.checker.CheckEACL(request, reqInfo); err != nil { + return nil, eACLErr(reqInfo, err) } return b.next.GetRangeHash(ctx, request) @@ -368,8 +368,8 @@ func (p putStreamBasicChecker) Send(request *objectV2.PutRequest) error { if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, ownerID) { return basicACLErr(reqInfo) - } else if !p.source.checker.CheckEACL(request, reqInfo) { - return eACLErr(reqInfo) + } else if err := p.source.checker.CheckEACL(request, reqInfo); err != nil { + return eACLErr(reqInfo, err) } } @@ -382,8 +382,8 @@ func (p putStreamBasicChecker) CloseAndRecv() (*objectV2.PutResponse, error) { func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error { if _, ok := resp.GetBody().GetObjectPart().(*objectV2.GetObjectPartInit); ok { - if !g.checker.CheckEACL(resp, g.info) { - return eACLErr(g.info) + if err := g.checker.CheckEACL(resp, g.info); err != nil { + return eACLErr(g.info, err) } } @@ -391,16 +391,16 @@ func (g *getStreamBasicChecker) Send(resp *objectV2.GetResponse) error { } func (g *rangeStreamBasicChecker) Send(resp *objectV2.GetRangeResponse) error { - if !g.checker.CheckEACL(resp, g.info) { - return eACLErr(g.info) + if err := g.checker.CheckEACL(resp, g.info); err != nil { + return eACLErr(g.info, err) } return g.GetObjectRangeStream.Send(resp) } func (g *searchStreamBasicChecker) Send(resp *objectV2.SearchResponse) error { - if !g.checker.CheckEACL(resp, g.info) { - return eACLErr(g.info) + if err := g.checker.CheckEACL(resp, g.info); err != nil { + return eACLErr(g.info, err) } return g.SearchStream.Send(resp) diff --git a/pkg/services/object/acl/v2/types.go b/pkg/services/object/acl/v2/types.go index 7bdae84df..0ce0146ec 100644 --- a/pkg/services/object/acl/v2/types.go +++ b/pkg/services/object/acl/v2/types.go @@ -10,9 +10,9 @@ type ACLChecker interface { // CheckBasicACL must return true only if request // passes basic ACL validation. CheckBasicACL(RequestInfo) bool - // CheckEACL must return true only if request - // passes extended ACL validation. - CheckEACL(interface{}, RequestInfo) bool + // CheckEACL must return non-nil error if request + // doesn't pass extended ACL validation. + CheckEACL(interface{}, RequestInfo) error // StickyBitCheck must return true only if sticky bit // is disabled or enabled but request contains correct // owner field.