[#1420] object/acl: Fix correlation of object session to request

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
This commit is contained in:
Leonard Lyubich 2022-09-16 14:22:09 +04:00 committed by fyrchik
parent 5834f9807e
commit e54b52ec03
7 changed files with 155 additions and 88 deletions

View file

@ -25,6 +25,7 @@ Changelog for NeoFS Node
- Fail startup if metabase has an old version (#1809) - Fail startup if metabase has an old version (#1809)
- Storage nodes could enter the network with any state (#1796) - Storage nodes could enter the network with any state (#1796)
- Missing check of new state value in `ControlService.SetNetmapStatus` (#1797) - Missing check of new state value in `ControlService.SetNetmapStatus` (#1797)
- Correlation of object session to request (#1420)
### Removed ### Removed
- Remove WIF and NEP2 support in `neofs-cli`'s --wallet flag (#1128) - Remove WIF and NEP2 support in `neofs-cli`'s --wallet flag (#1128)
@ -46,6 +47,10 @@ Storage nodes under maintenance are not excluded from the network map, but don't
serve object operations. (*) can be fetched from network configuration via serve object operations. (*) can be fetched from network configuration via
`neofs-cli netmap netinfo` command. `neofs-cli netmap netinfo` command.
When issuing an object session token for root (virtual, "big") objects,
additionally include all members of the split-chain. If session context
includes root object only, it is not spread to physical ("small") objects.
## [0.32.0] - 2022-09-14 - Pungdo (풍도, 楓島) ## [0.32.0] - 2022-09-14 - Pungdo (풍도, 楓島)
### Added ### Added

View file

@ -63,7 +63,7 @@ func Prepare(cmd *cobra.Command, cnr cid.ID, obj *oid.ID, key *ecdsa.PrivateKey,
tok.BindContainer(cnr) tok.BindContainer(cnr)
if obj != nil { if obj != nil {
tok.LimitByObject(*obj) tok.LimitByObjects(*obj)
} }
err := tok.Sign(*key) err := tok.Sign(*key)

4
go.mod
View file

@ -17,9 +17,9 @@ require (
github.com/nspcc-dev/hrw v1.0.9 github.com/nspcc-dev/hrw v1.0.9
github.com/nspcc-dev/neo-go v0.99.2 github.com/nspcc-dev/neo-go v0.99.2
github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220809123759-3094d3e0c14b // indirect github.com/nspcc-dev/neo-go/pkg/interop v0.0.0-20220809123759-3094d3e0c14b // indirect
github.com/nspcc-dev/neofs-api-go/v2 v2.13.2-0.20220919124434-cf868188ef9c github.com/nspcc-dev/neofs-api-go/v2 v2.13.2-0.20221004142957-5fc2644c680d
github.com/nspcc-dev/neofs-contract v0.15.5-0.20220930133158-d95bc535894c github.com/nspcc-dev/neofs-contract v0.15.5-0.20220930133158-d95bc535894c
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.6.0.20220926102839-c6576c8112ee github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.6.0.20221005093951-1325b4f27218
github.com/nspcc-dev/tzhash v1.6.1 github.com/nspcc-dev/tzhash v1.6.1
github.com/panjf2000/ants/v2 v2.4.0 github.com/panjf2000/ants/v2 v2.4.0
github.com/paulmach/orb v0.2.2 github.com/paulmach/orb v0.2.2

BIN
go.sum

Binary file not shown.

View file

@ -11,6 +11,7 @@ import (
"github.com/nspcc-dev/neofs-node/pkg/services/object" "github.com/nspcc-dev/neofs-node/pkg/services/object"
"github.com/nspcc-dev/neofs-sdk-go/container/acl" "github.com/nspcc-dev/neofs-sdk-go/container/acl"
cid "github.com/nspcc-dev/neofs-sdk-go/container/id" cid "github.com/nspcc-dev/neofs-sdk-go/container/id"
oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
sessionSDK "github.com/nspcc-dev/neofs-sdk-go/session" sessionSDK "github.com/nspcc-dev/neofs-sdk-go/session"
"github.com/nspcc-dev/neofs-sdk-go/user" "github.com/nspcc-dev/neofs-sdk-go/user"
"go.uber.org/zap" "go.uber.org/zap"
@ -113,11 +114,23 @@ func (b Service) Get(request *objectV2.GetRequest, stream object.GetObjectStream
return err return err
} }
obj, err := getObjectIDFromRequestBody(request.GetBody())
if err != nil {
return err
}
sTok, err := originalSessionToken(request.GetMetaHeader()) sTok, err := originalSessionToken(request.GetMetaHeader())
if err != nil { if err != nil {
return err return err
} }
if sTok != nil {
err = assertSessionRelation(*sTok, cnr, obj)
if err != nil {
return err
}
}
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
if err != nil { if err != nil {
return err return err
@ -135,12 +148,7 @@ func (b Service) Get(request *objectV2.GetRequest, stream object.GetObjectStream
return err return err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(request.GetBody()) reqInfo.obj = obj
if err != nil {
return err
}
useObjectIDFromSession(&reqInfo, sTok)
if !b.checker.CheckBasicACL(reqInfo) { if !b.checker.CheckBasicACL(reqInfo) {
return basicACLErr(reqInfo) return basicACLErr(reqInfo)
@ -172,11 +180,23 @@ func (b Service) Head(
return nil, err return nil, err
} }
obj, err := getObjectIDFromRequestBody(request.GetBody())
if err != nil {
return nil, err
}
sTok, err := originalSessionToken(request.GetMetaHeader()) sTok, err := originalSessionToken(request.GetMetaHeader())
if err != nil { if err != nil {
return nil, err return nil, err
} }
if sTok != nil {
err = assertSessionRelation(*sTok, cnr, obj)
if err != nil {
return nil, err
}
}
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
if err != nil { if err != nil {
return nil, err return nil, err
@ -194,12 +214,7 @@ func (b Service) Head(
return nil, err return nil, err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(request.GetBody()) reqInfo.obj = obj
if err != nil {
return nil, err
}
useObjectIDFromSession(&reqInfo, sTok)
if !b.checker.CheckBasicACL(reqInfo) { if !b.checker.CheckBasicACL(reqInfo) {
return nil, basicACLErr(reqInfo) return nil, basicACLErr(reqInfo)
@ -228,6 +243,13 @@ func (b Service) Search(request *objectV2.SearchRequest, stream object.SearchStr
return err return err
} }
if sTok != nil {
err = assertSessionRelation(*sTok, id, nil)
if err != nil {
return err
}
}
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
if err != nil { if err != nil {
return err return err
@ -245,11 +267,6 @@ func (b Service) Search(request *objectV2.SearchRequest, stream object.SearchStr
return err return err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(request.GetBody())
if err != nil {
return err
}
if !b.checker.CheckBasicACL(reqInfo) { if !b.checker.CheckBasicACL(reqInfo) {
return basicACLErr(reqInfo) return basicACLErr(reqInfo)
} else if err := b.checker.CheckEACL(request, reqInfo); err != nil { } else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
@ -271,11 +288,23 @@ func (b Service) Delete(
return nil, err return nil, err
} }
obj, err := getObjectIDFromRequestBody(request.GetBody())
if err != nil {
return nil, err
}
sTok, err := originalSessionToken(request.GetMetaHeader()) sTok, err := originalSessionToken(request.GetMetaHeader())
if err != nil { if err != nil {
return nil, err return nil, err
} }
if sTok != nil {
err = assertSessionRelation(*sTok, cnr, obj)
if err != nil {
return nil, err
}
}
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
if err != nil { if err != nil {
return nil, err return nil, err
@ -293,12 +322,7 @@ func (b Service) Delete(
return nil, err return nil, err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(request.GetBody()) reqInfo.obj = obj
if err != nil {
return nil, err
}
useObjectIDFromSession(&reqInfo, sTok)
if !b.checker.CheckBasicACL(reqInfo) { if !b.checker.CheckBasicACL(reqInfo) {
return nil, basicACLErr(reqInfo) return nil, basicACLErr(reqInfo)
@ -315,11 +339,23 @@ func (b Service) GetRange(request *objectV2.GetRangeRequest, stream object.GetOb
return err return err
} }
obj, err := getObjectIDFromRequestBody(request.GetBody())
if err != nil {
return err
}
sTok, err := originalSessionToken(request.GetMetaHeader()) sTok, err := originalSessionToken(request.GetMetaHeader())
if err != nil { if err != nil {
return err return err
} }
if sTok != nil {
err = assertSessionRelation(*sTok, cnr, obj)
if err != nil {
return err
}
}
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
if err != nil { if err != nil {
return err return err
@ -337,11 +373,7 @@ func (b Service) GetRange(request *objectV2.GetRangeRequest, stream object.GetOb
return err return err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(request.GetBody()) reqInfo.obj = obj
if err != nil {
return err
}
useObjectIDFromSession(&reqInfo, sTok)
if !b.checker.CheckBasicACL(reqInfo) { if !b.checker.CheckBasicACL(reqInfo) {
return basicACLErr(reqInfo) return basicACLErr(reqInfo)
@ -364,11 +396,23 @@ func (b Service) GetRangeHash(
return nil, err return nil, err
} }
obj, err := getObjectIDFromRequestBody(request.GetBody())
if err != nil {
return nil, err
}
sTok, err := originalSessionToken(request.GetMetaHeader()) sTok, err := originalSessionToken(request.GetMetaHeader())
if err != nil { if err != nil {
return nil, err return nil, err
} }
if sTok != nil {
err = assertSessionRelation(*sTok, cnr, obj)
if err != nil {
return nil, err
}
}
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
if err != nil { if err != nil {
return nil, err return nil, err
@ -386,12 +430,7 @@ func (b Service) GetRangeHash(
return nil, err return nil, err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(request.GetBody()) reqInfo.obj = obj
if err != nil {
return nil, err
}
useObjectIDFromSession(&reqInfo, sTok)
if !b.checker.CheckBasicACL(reqInfo) { if !b.checker.CheckBasicACL(reqInfo) {
return nil, basicACLErr(reqInfo) return nil, basicACLErr(reqInfo)
@ -427,6 +466,18 @@ func (p putStreamBasicChecker) Send(request *objectV2.PutRequest) error {
return fmt.Errorf("invalid object owner: %w", err) return fmt.Errorf("invalid object owner: %w", err)
} }
objV2 := part.GetObjectID()
var obj *oid.ID
if objV2 != nil {
obj = new(oid.ID)
err = obj.ReadFromV2(*objV2)
if err != nil {
return err
}
}
var sTok *sessionSDK.Object var sTok *sessionSDK.Object
if tokV2 := request.GetMetaHeader().GetSessionToken(); tokV2 != nil { if tokV2 := request.GetMetaHeader().GetSessionToken(); tokV2 != nil {
@ -436,6 +487,11 @@ func (p putStreamBasicChecker) Send(request *objectV2.PutRequest) error {
if err != nil { if err != nil {
return fmt.Errorf("invalid session token: %w", err) return fmt.Errorf("invalid session token: %w", err)
} }
err = assertSessionRelation(*sTok, cnr, obj)
if err != nil {
return err
}
} }
bTok, err := originalBearerToken(request.GetMetaHeader()) bTok, err := originalBearerToken(request.GetMetaHeader())
@ -455,12 +511,7 @@ func (p putStreamBasicChecker) Send(request *objectV2.PutRequest) error {
return err return err
} }
reqInfo.obj, err = getObjectIDFromRequestBody(part) reqInfo.obj = obj
if err != nil {
return err
}
useObjectIDFromSession(&reqInfo, sTok)
if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, idOwner) { if !p.source.checker.CheckBasicACL(reqInfo) || !p.source.checker.StickyBitCheck(reqInfo, idOwner) {
return basicACLErr(reqInfo) return basicACLErr(reqInfo)

View file

@ -93,24 +93,12 @@ func originalSessionToken(header *sessionV2.RequestMetaHeader) (*sessionSDK.Obje
return &tok, nil return &tok, nil
} }
func getObjectIDFromRequestBody(body interface{}) (*oid.ID, error) { // getObjectIDFromRequestBody decodes oid.ID from the common interface of the
var idV2 *refsV2.ObjectID // object reference's holders. Returns an error if object ID is missing in the request.
func getObjectIDFromRequestBody(body interface{ GetAddress() *refsV2.Address }) (*oid.ID, error) {
switch v := body.(type) { idV2 := body.GetAddress().GetObjectID()
default:
return nil, nil
case interface {
GetObjectID() *refsV2.ObjectID
}:
idV2 = v.GetObjectID()
case interface {
GetAddress() *refsV2.Address
}:
idV2 = v.GetAddress().GetObjectID()
}
if idV2 == nil { if idV2 == nil {
return nil, nil return nil, errors.New("missing object ID")
} }
var id oid.ID var id oid.ID
@ -123,34 +111,6 @@ func getObjectIDFromRequestBody(body interface{}) (*oid.ID, error) {
return &id, nil return &id, nil
} }
func useObjectIDFromSession(req *RequestInfo, token *sessionSDK.Object) {
if token == nil {
return
}
// TODO(@cthulhu-rider): It'd be nice to not pull object identifiers from
// the token, but assert them. Track #1420
var tokV2 sessionV2.Token
token.WriteToV2(&tokV2)
ctx, ok := tokV2.GetBody().GetContext().(*sessionV2.ObjectSessionContext)
if !ok {
panic(fmt.Sprintf("wrong object session context %T, is it verified?", tokV2.GetBody().GetContext()))
}
idV2 := ctx.GetAddress().GetObjectID()
if idV2 == nil {
return
}
req.obj = new(oid.ID)
err := req.obj.ReadFromV2(*idV2)
if err != nil {
panic(fmt.Sprintf("unexpected protocol violation error after correct session token decoding: %v", err))
}
}
func ownerFromToken(token *sessionSDK.Object) (*user.ID, *keys.PublicKey, error) { func ownerFromToken(token *sessionSDK.Object) (*user.ID, *keys.PublicKey, error) {
// 1. First check signature of session token. // 1. First check signature of session token.
if !token.VerifySignature() { if !token.VerifySignature() {
@ -231,3 +191,24 @@ func assertVerb(tok sessionSDK.Object, op acl.Op) bool {
return false return false
} }
// assertSessionRelation checks if given token describing the NeoFS session
// relates to the given container and optional object. Missing object
// means that the context isn't bound to any NeoFS object in the container.
// Returns no error iff relation is correct. Criteria:
//
// session is bound to the given container
// object is not specified or session is bound to this object
//
// Session MUST be bound to the particular container, otherwise behavior is undefined.
func assertSessionRelation(tok sessionSDK.Object, cnr cid.ID, obj *oid.ID) error {
if !tok.AssertContainer(cnr) {
return errors.New("requested container is not related to the session")
}
if obj != nil && !tok.AssertObject(*obj) {
return errors.New("requested object is not related to the session")
}
return nil
}

View file

@ -10,6 +10,8 @@ import (
"github.com/nspcc-dev/neofs-api-go/v2/session" "github.com/nspcc-dev/neofs-api-go/v2/session"
bearertest "github.com/nspcc-dev/neofs-sdk-go/bearer/test" bearertest "github.com/nspcc-dev/neofs-sdk-go/bearer/test"
aclsdk "github.com/nspcc-dev/neofs-sdk-go/container/acl" aclsdk "github.com/nspcc-dev/neofs-sdk-go/container/acl"
cidtest "github.com/nspcc-dev/neofs-sdk-go/container/id/test"
oidtest "github.com/nspcc-dev/neofs-sdk-go/object/id/test"
sessionSDK "github.com/nspcc-dev/neofs-sdk-go/session" sessionSDK "github.com/nspcc-dev/neofs-sdk-go/session"
sessiontest "github.com/nspcc-dev/neofs-sdk-go/session/test" sessiontest "github.com/nspcc-dev/neofs-sdk-go/session/test"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -104,3 +106,31 @@ func TestIsVerbCompatible(t *testing.T) {
} }
} }
} }
func TestAssertSessionRelation(t *testing.T) {
var tok sessionSDK.Object
cnr := cidtest.ID()
cnrOther := cidtest.ID()
obj := oidtest.ID()
objOther := oidtest.ID()
// make sure ids differ, otherwise test won't work correctly
require.False(t, cnrOther.Equals(cnr))
require.False(t, objOther.Equals(obj))
// bind session to the container (required)
tok.BindContainer(cnr)
// test container-global session
require.NoError(t, assertSessionRelation(tok, cnr, nil))
require.NoError(t, assertSessionRelation(tok, cnr, &obj))
require.Error(t, assertSessionRelation(tok, cnrOther, nil))
require.Error(t, assertSessionRelation(tok, cnrOther, &obj))
// limit the session to the particular object
tok.LimitByObjects(obj)
// test fixed object session (here obj arg must be non-nil everywhere)
require.NoError(t, assertSessionRelation(tok, cnr, &obj))
require.Error(t, assertSessionRelation(tok, cnr, &objOther))
}