[#1246] object/acl: Return more concise description for eACL errors

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>

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) {
+	if err := isValidBearer(reqInfo, c.state); err != nil {
-		return false
+		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 {

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
 }

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) {
+	} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-		return eACLErr(reqInfo)
+		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) {
+	} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-		return nil, eACLErr(reqInfo)
+		return nil, eACLErr(reqInfo, err)
 	}
 
 	resp, err := b.next.Head(ctx, request)
 	if err == nil {
-		if !b.checker.CheckEACL(resp, reqInfo) {
+		if err = b.checker.CheckEACL(resp, reqInfo); err != nil {
-			err = eACLErr(reqInfo)
+			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) {
+	} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-		return eACLErr(reqInfo)
+		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) {
+	} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-		return nil, eACLErr(reqInfo)
+		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) {
+	} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-		return eACLErr(reqInfo)
+		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) {
+	} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
-		return nil, eACLErr(reqInfo)
+		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) {
+		} else if err := p.source.checker.CheckEACL(request, reqInfo); err != nil {
-			return eACLErr(reqInfo)
+			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) {
+		if err := g.checker.CheckEACL(resp, g.info); err != nil {
-			return eACLErr(g.info)
+			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) {
+	if err := g.checker.CheckEACL(resp, g.info); err != nil {
-		return eACLErr(g.info)
+		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) {
+	if err := g.checker.CheckEACL(resp, g.info); err != nil {
-		return eACLErr(g.info)
+		return eACLErr(g.info, err)
 	}
 
 	return g.SearchStream.Send(resp)

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
+	// CheckEACL must return non-nil error if request
-	// passes extended ACL validation.
+	// doesn't pass extended ACL validation.
-	CheckEACL(interface{}, RequestInfo) bool
+	CheckEACL(interface{}, RequestInfo) error
 	// StickyBitCheck must return true only if sticky bit
 	// is disabled or enabled but request contains correct
 	// owner field.