ape: Make services use bearer chains fed router #1216
|
@ -19,6 +19,7 @@ import (
|
|||
containerMorph "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/container/morph"
|
||||
cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
|
||||
"go.uber.org/zap"
|
||||
"google.golang.org/grpc"
|
||||
)
|
||||
|
@ -46,9 +47,13 @@ func initContainerService(_ context.Context, c *cfg) {
|
|||
|
||||
c.shared.frostfsidClient = frostfsIDSubjectProvider
|
||||
|
||||
defaultChainRouter := engine.NewDefaultChainRouterWithLocalOverrides(
|
||||
c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.MorphRuleChainStorage(),
|
||||
c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.LocalStorage(),
|
||||
)
|
||||
service := containerService.NewSignService(
|
||||
&c.key.PrivateKey,
|
||||
containerService.NewAPEServer(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine, cnrRdr,
|
||||
containerService.NewAPEServer(defaultChainRouter, cnrRdr,
|
||||
newCachedIRFetcher(createInnerRingFetcher(c)), c.netMapSource, c.shared.frostfsidClient,
|
||||
containerService.NewExecutionService(containerMorph.NewExecutor(cnrRdr, cnrWrt), c.respSvc),
|
||||
),
|
||||
|
|
|
@ -460,7 +460,8 @@ func createAPEService(c *cfg, splitSvc *objectService.TransportSplitter) *object
|
|||
return objectAPE.NewService(
|
||||
c.log,
|
||||
objectAPE.NewChecker(
|
||||
c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.chainRouter,
|
||||
c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.LocalStorage(),
|
||||
c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.MorphRuleChainStorage(),
|
||||
objectAPE.NewStorageEngineHeaderProvider(c.cfgObject.cfgLocalStorage.localStorage, c.cfgObject.getSvc),
|
||||
c.shared.frostfsidClient,
|
||||
c.netMapSource,
|
||||
|
|
|
@ -1,13 +1,11 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/chainbase"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource"
|
||||
"github.com/google/uuid"
|
||||
"github.com/hashicorp/golang-lru/v2/expirable"
|
||||
"github.com/nspcc-dev/neo-go/pkg/neorpc/result"
|
||||
|
@ -15,11 +13,9 @@ import (
|
|||
)
|
||||
|
||||
type accessPolicyEngine struct {
|
||||
mtx sync.RWMutex
|
||||
|
||||
|
||||
chainRouter engine.ChainRouter
|
||||
|
||||
localOverrideDatabase chainbase.LocalOverrideDatabase
|
||||
|
||||
morphChainStorage engine.MorphRuleChainStorageReader
|
||||
}
|
||||
|
||||
var _ engine.MorphRuleChainStorageReader = (*morphAPEChainCache)(nil)
|
||||
|
@ -70,32 +66,20 @@ func newAccessPolicyEngine(
|
|||
localOverrideDatabase chainbase.LocalOverrideDatabase,
|
||||
) *accessPolicyEngine {
|
||||
return &accessPolicyEngine{
|
||||
chainRouter: engine.NewDefaultChainRouterWithLocalOverrides(
|
||||
morphChainStorage,
|
||||
localOverrideDatabase,
|
||||
),
|
||||
morphChainStorage: morphChainStorage,
|
||||
|
||||
localOverrideDatabase: localOverrideDatabase,
|
||||
}
|
||||
}
|
||||
|
||||
func (a *accessPolicyEngine) IsAllowed(name chain.Name, target engine.RequestTarget, r resource.Request) (status chain.Status, found bool, err error) {
|
||||
a.mtx.RLock()
|
||||
defer a.mtx.RUnlock()
|
||||
|
||||
return a.chainRouter.IsAllowed(name, target, r)
|
||||
func (a *accessPolicyEngine) LocalStorage() engine.LocalOverrideStorage {
|
||||
return a.localOverrideDatabase
|
||||
}
|
||||
|
||||
func (a *accessPolicyEngine) LocalStorage() engine.LocalOverrideStorage {
|
||||
a.mtx.Lock()
|
||||
defer a.mtx.Unlock()
|
||||
|
||||
return a.localOverrideDatabase
|
||||
func (a *accessPolicyEngine) MorphRuleChainStorage() engine.MorphRuleChainStorageReader {
|
||||
return a.morphChainStorage
|
||||
}
|
||||
|
||||
func (a *accessPolicyEngine) LocalOverrideDatabaseCore() chainbase.DatabaseCore {
|
||||
a.mtx.Lock()
|
||||
defer a.mtx.Unlock()
|
||||
|
||||
return a.localOverrideDatabase
|
||||
}
|
||||
|
|
|
@ -65,7 +65,8 @@ func initTreeService(c *cfg) {
|
|||
tree.WithReplicationWorkerCount(treeConfig.ReplicationWorkerCount()),
|
||||
tree.WithAuthorizedKeys(treeConfig.AuthorizedKeys()),
|
||||
tree.WithMetrics(c.metricsCollector.TreeService()),
|
||||
tree.WithAPERouter(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine),
|
||||
tree.WithAPELocalOverrideStorage(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.LocalStorage()),
|
||||
tree.WithAPEMorphRuleStorage(c.cfgObject.cfgAccessPolicyEngine.accessPolicyEngine.MorphRuleChainStorage()),
|
||||
tree.WithNetmapState(c.cfgNetmap.state),
|
||||
)
|
||||
|
||||
|
|
94
pkg/ape/router/bearer_overrides.go
Normal file
|
@ -0,0 +1,94 @@
|
|||
package router
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape"
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
|
||||
cidSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||
policyengine "git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
|
||||
)
|
||||
|
||||
func newTarget(ct ape.ChainTarget) (policyengine.Target, error) {
|
||||
var target policyengine.Target
|
||||
switch ct.TargetType {
|
||||
case ape.TargetTypeContainer:
|
||||
var cid cidSDK.ID
|
||||
err := cid.DecodeString(ct.Name)
|
||||
if err != nil {
|
||||
return target, fmt.Errorf("invalid cid format: %s", target.Name)
|
||||
}
|
||||
target.Type = policyengine.Container
|
||||
case ape.TargetTypeGroup:
|
||||
target.Type = policyengine.Group
|
||||
case ape.TargetTypeNamespace:
|
||||
target.Type = policyengine.Namespace
|
||||
case ape.TargetTypeUser:
|
||||
target.Type = policyengine.User
|
||||
default:
|
||||
return target, fmt.Errorf("unsupported target type: %v", ct.TargetType)
|
||||
}
|
||||
target.Name = ct.Name
|
||||
return target, nil
|
||||
}
|
||||
|
||||
type morphReaderDecorator struct {
|
||||
policyengine.MorphRuleChainStorageReader
|
||||
|
||||
bearerTokenTarget policyengine.Target
|
||||
|
||||
bearerTokenChains []*chain.Chain
|
||||
}
|
||||
|
||||
func newMorphReaderDecorator(r policyengine.MorphRuleChainStorageReader, override bearer.APEOverride) (*morphReaderDecorator, error) {
|
||||
if r == nil {
|
||||
return nil, errors.New("empty morph chain rule reader")
|
||||
}
|
||||
t, err := newTarget(override.Target)
|
||||
if err != nil {
|
||||
acid-ant marked this conversation as resolved
Outdated
acid-ant
commented
How about to pass How about to pass `nil` instead of `stub` and fix `policy-engine` a bit?
aarifullin
commented
From the point of defaultChainRouter morph rules are mandatory. I could disable the check by passing From the point of [defaultChainRouter](https://git.frostfs.info/TrueCloudLab/policy-engine/src/branch/master/pkg/engine/chain_router.go) morph rules are mandatory. I could disable the check by passing `nil`, but I prefer *thorough refactoring* of `defaultChainRouter` in the future.
|
||||
return nil, err
|
||||
}
|
||||
|
||||
bearerTokenChains := make([]*chain.Chain, len(override.Chains))
|
||||
for i := range override.Chains {
|
||||
chain := new(chain.Chain)
|
||||
if err := chain.DecodeBytes(override.Chains[i].Raw); err != nil {
|
||||
return nil, fmt.Errorf("invalid ape chain: %w", err)
|
||||
}
|
||||
bearerTokenChains[i] = chain
|
||||
}
|
||||
|
||||
return &morphReaderDecorator{
|
||||
MorphRuleChainStorageReader: r,
|
||||
bearerTokenTarget: t,
|
||||
bearerTokenChains: bearerTokenChains,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (m *morphReaderDecorator) ListMorphRuleChains(name chain.Name, target policyengine.Target) ([]*chain.Chain, error) {
|
||||
if len(m.bearerTokenChains) > 0 && m.bearerTokenTarget.Type == target.Type {
|
||||
if m.bearerTokenTarget.Name != target.Name {
|
||||
return nil, fmt.Errorf("unexpected bearer token target: %s", m.bearerTokenTarget.Name)
|
||||
}
|
||||
fyrchik
commented
There are container chains in the bearer token which are overridden, so no chains from the contract should be applied. It it true for this implementation? There are container chains in the bearer token which are _overridden_, so no chains from the contract should be applied. It it true for this implementation?
aarifullin
commented
Let's analyze the implementation
So, if the issuer of the bearer token is the container's owner and bearer token contains allowing chains (overrides) for an operation that means an operation would be applied despite policy contract may deny it Let's analyze the implementation
1. *No bearer token*: The standard scheme with default router is used as it was earlier (seems OK)
2. *Bearer token bears the chains for the target container*:
- Local overrides are checked always first -> local overrides *override* both bearer token and policy contract policies (seems OK)
- If local overrides do not match, then we check if the bearer token's chain matches the request properties. If it does *not*, it goes on checking in policy contract. That means the bearer token *overrides* the contract's chains but does not override local overrides.
So, if the issuer of the bearer token is the container's owner and bearer token contains allowing chains (overrides) for an operation that means an operation would be applied despite policy contract may *deny* it
fyrchik
commented
And if we have some restrictions on the namespace level? >If it does not, it goes on checking in policy contract.
And if we have some restrictions on the namespace level?
It seems I can circumvent them by emitting a bearer token for myself.
aarifullin
commented
Okay. You mean that a bearer token can be used as kind of exploit to bypass restrictions on namespace level
Okay. You mean that a bearer token can be used as kind of exploit to bypass restrictions on namespace level
1. Earlier a bearer token lived in a different paradigm - everything was considered on container level. So, a bearer token allowed to bypass restrictions within eACL table, but that wasn't the exploit
2. A bearer token should deal with object operations (I can add this checks) only as it can be validated only for container target (see `isValidBearer`)
3. Namespace policies, in a good way, set container operation restrictions. Yes, we can set restriction on object operation within a namespace but there is no agreement yet. If we agree a namespace policy with object operations has a higher priority, then... we must redesign policy-engine again
|
||||
return m.bearerTokenChains, nil
|
||||
}
|
||||
return m.MorphRuleChainStorageReader.ListMorphRuleChains(name, target)
|
||||
}
|
||||
|
||||
// BearerChainFeedRouter creates a chain router emplacing bearer token rule chains.
|
||||
// Bearer token chains override only container target chains within Policy contract. This means the order of checking
|
||||
// is as follows:
|
||||
//
|
||||
// 1. Local overrides;
|
||||
// 2. Policy contract chains for a namespace target (as namespace chains have higher priority);
|
||||
// 3. Bearer token chains for a container target - if they're not defined, then it checks Policy contract chains;
|
||||
// 4. Checks for the remaining targets.
|
||||
func BearerChainFeedRouter(localOverrideStorage policyengine.LocalOverrideStorage, morphChainStorage policyengine.MorphRuleChainStorageReader, override bearer.APEOverride) (policyengine.ChainRouter, error) {
|
||||
mr, err := newMorphReaderDecorator(morphChainStorage, override)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create morph reader with bearer override error: %w", err)
|
||||
}
|
||||
return policyengine.NewDefaultChainRouterWithLocalOverrides(mr, localOverrideStorage), nil
|
||||
}
|
165
pkg/ape/router/bearer_overrides_test.go
Normal file
|
@ -0,0 +1,165 @@
|
|||
package router_test
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-node/pkg/ape/router"
|
||||
apeSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape"
|
||||
bearerSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine/inmemory"
|
||||
resourcetest "git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource/testutil"
|
||||
nativeschema "git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
const (
|
||||
container = "67ETTZzbzJC6WxdQhHHHsJNCttVMBqYrSoFaUFVDNfiX"
|
||||
rootNs = ""
|
||||
)
|
||||
|
||||
var (
|
||||
allowBySourceIP = &chain.Chain{
|
||||
Rules: []chain.Rule{
|
||||
{
|
||||
Status: chain.Allow,
|
||||
Actions: chain.Actions{Names: []string{nativeschema.MethodPutObject}},
|
||||
Resources: chain.Resources{Names: []string{fmt.Sprintf(nativeschema.ResourceFormatRootContainer, container)}},
|
||||
Condition: []chain.Condition{
|
||||
{
|
||||
Op: chain.CondStringEquals,
|
||||
Kind: chain.KindRequest,
|
||||
Key: "SourceIP",
|
||||
Value: "10.122.1.20",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
denyBySourceIP = &chain.Chain{
|
||||
Rules: []chain.Rule{
|
||||
{
|
||||
Status: chain.AccessDenied,
|
||||
Actions: chain.Actions{Names: []string{nativeschema.MethodPutObject}},
|
||||
Resources: chain.Resources{Names: []string{fmt.Sprintf(nativeschema.ResourceFormatRootContainer, container)}},
|
||||
Condition: []chain.Condition{
|
||||
{
|
||||
Op: chain.CondStringEquals,
|
||||
Kind: chain.KindRequest,
|
||||
Key: "SourceIP",
|
||||
Value: "10.122.1.20",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
func TestBearerChainFedRouter(t *testing.T) {
|
||||
t.Run("no bearer token overrides", func(t *testing.T) {
|
||||
inmem := inmemory.NewInMemoryLocalOverrides()
|
||||
|
||||
inmem.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), denyBySourceIP)
|
||||
inmem.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.ContainerTarget(container), allowBySourceIP)
|
||||
|
||||
_, err := router.BearerChainFeedRouter(inmem.LocalStorage(), inmem.MorphRuleChainStorage(), bearerSDK.APEOverride{})
|
||||
require.Error(t, err)
|
||||
})
|
||||
t.Run("allow by container with deny by bearer overrides", func(t *testing.T) {
|
||||
inmem := inmemory.NewInMemoryLocalOverrides()
|
||||
|
||||
inmem.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.ContainerTarget(container), allowBySourceIP)
|
||||
|
||||
bt := bearerSDK.APEOverride{
|
||||
Target: apeSDK.ChainTarget{
|
||||
TargetType: apeSDK.TargetTypeContainer,
|
||||
Name: container,
|
||||
},
|
||||
Chains: []apeSDK.Chain{{
|
||||
Raw: denyBySourceIP.Bytes(),
|
||||
}},
|
||||
}
|
||||
|
||||
r, err := router.BearerChainFeedRouter(inmem.LocalStorage(), inmem.MorphRuleChainStorage(), bt)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := resourcetest.NewRequest(nativeschema.MethodPutObject,
|
||||
resourcetest.NewResource(fmt.Sprintf(nativeschema.ResourceFormatRootContainer, container), map[string]string{}),
|
||||
map[string]string{
|
||||
"SourceIP": "10.122.1.20",
|
||||
"Actor": "someOwner",
|
||||
},
|
||||
)
|
||||
|
||||
st, found, err := r.IsAllowed(chain.Ingress, engine.NewRequestTarget(rootNs, container), req)
|
||||
require.NoError(t, err)
|
||||
require.True(t, found)
|
||||
require.Equal(t, st, chain.AccessDenied)
|
||||
})
|
||||
t.Run("allow by namespace with deny by bearer overrides", func(t *testing.T) {
|
||||
inmem := inmemory.NewInMemoryLocalOverrides()
|
||||
|
||||
inmem.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.ContainerTarget(container), allowBySourceIP)
|
||||
inmem.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.NamespaceTarget(rootNs), allowBySourceIP)
|
||||
|
||||
bt := bearerSDK.APEOverride{
|
||||
Target: apeSDK.ChainTarget{
|
||||
TargetType: apeSDK.TargetTypeContainer,
|
||||
Name: container,
|
||||
},
|
||||
Chains: []apeSDK.Chain{{
|
||||
Raw: denyBySourceIP.Bytes(),
|
||||
}},
|
||||
}
|
||||
|
||||
r, err := router.BearerChainFeedRouter(inmem.LocalStorage(), inmem.MorphRuleChainStorage(), bt)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := resourcetest.NewRequest(nativeschema.MethodPutObject,
|
||||
resourcetest.NewResource(fmt.Sprintf(nativeschema.ResourceFormatRootContainer, container), map[string]string{}),
|
||||
map[string]string{
|
||||
"SourceIP": "10.122.1.20",
|
||||
"Actor": "someOwner",
|
||||
},
|
||||
)
|
||||
|
||||
st, found, err := r.IsAllowed(chain.Ingress, engine.NewRequestTarget(rootNs, container), req)
|
||||
require.NoError(t, err)
|
||||
require.True(t, found)
|
||||
require.Equal(t, st, chain.AccessDenied)
|
||||
})
|
||||
t.Run("deny by namespace with allow by bearer overrides", func(t *testing.T) {
|
||||
inmem := inmemory.NewInMemoryLocalOverrides()
|
||||
|
||||
inmem.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, engine.NamespaceTarget(rootNs), denyBySourceIP)
|
||||
|
||||
bt := bearerSDK.APEOverride{
|
||||
Target: apeSDK.ChainTarget{
|
||||
TargetType: apeSDK.TargetTypeContainer,
|
||||
Name: container,
|
||||
},
|
||||
Chains: []apeSDK.Chain{{
|
||||
Raw: allowBySourceIP.Bytes(),
|
||||
}},
|
||||
}
|
||||
|
||||
r, err := router.BearerChainFeedRouter(inmem.LocalStorage(), inmem.MorphRuleChainStorage(), bt)
|
||||
require.NoError(t, err)
|
||||
|
||||
req := resourcetest.NewRequest(nativeschema.MethodPutObject,
|
||||
resourcetest.NewResource(fmt.Sprintf(nativeschema.ResourceFormatRootContainer, container), map[string]string{}),
|
||||
map[string]string{
|
||||
"SourceIP": "10.122.1.20",
|
||||
"Actor": "someOwner",
|
||||
},
|
||||
)
|
||||
|
||||
st, found, err := r.IsAllowed(chain.Ingress, engine.NewRequestTarget(rootNs, container), req)
|
||||
require.NoError(t, err)
|
||||
require.True(t, found)
|
||||
require.Equal(t, st, chain.AccessDenied)
|
||||
})
|
||||
}
|
|
@ -3,37 +3,12 @@ package router
|
|||
import (
|
||||
"fmt"
|
||||
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/ape"
|
||||
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/bearer"
|
||||
cidSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
|
||||
apechain "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine/inmemory"
|
||||
)
|
||||
|
||||
func newTarget(ct ape.ChainTarget) (engine.Target, error) {
|
||||
var target engine.Target
|
||||
switch ct.TargetType {
|
||||
case ape.TargetTypeContainer:
|
||||
var cid cidSDK.ID
|
||||
err := cid.DecodeString(ct.Name)
|
||||
if err != nil {
|
||||
return target, fmt.Errorf("invalid cid format: %s", target.Name)
|
||||
}
|
||||
target.Type = engine.Container
|
||||
case ape.TargetTypeGroup:
|
||||
target.Type = engine.Group
|
||||
case ape.TargetTypeNamespace:
|
||||
target.Type = engine.Namespace
|
||||
case ape.TargetTypeUser:
|
||||
target.Type = engine.User
|
||||
default:
|
||||
return target, fmt.Errorf("unsupported target type: %v", ct.TargetType)
|
||||
}
|
||||
target.Name = ct.Name
|
||||
return target, nil
|
||||
}
|
||||
|
||||
// SingleUseRouterWithBearerTokenChains creates chain router with inmemory storage implementation and
|
||||
// fed with APE chains defined in Bearer token.
|
||||
func SingleUseRouterWithBearerTokenChains(overrides []bearer.APEOverride) (engine.ChainRouter, error) {
|
||||
|
|
|
@ -24,7 +24,8 @@ import (
|
|||
)
|
||||
|
||||
type checkerImpl struct {
|
||||
chainRouter policyengine.ChainRouter
|
||||
localOverrideStorage policyengine.LocalOverrideStorage
|
||||
morphChainStorage policyengine.MorphRuleChainStorageReader
|
||||
headerProvider HeaderProvider
|
||||
frostFSIDClient frostfsidcore.SubjectProvider
|
||||
nm netmap.Source
|
||||
|
@ -33,9 +34,10 @@ type checkerImpl struct {
|
|||
nodePK []byte
|
||||
}
|
||||
|
||||
func NewChecker(chainRouter policyengine.ChainRouter, headerProvider HeaderProvider, frostFSIDClient frostfsidcore.SubjectProvider, nm netmap.Source, st netmap.State, cnrSource container.Source, nodePK []byte) Checker {
|
||||
func NewChecker(localOverrideStorage policyengine.LocalOverrideStorage, morphChainStorage policyengine.MorphRuleChainStorageReader, headerProvider HeaderProvider, frostFSIDClient frostfsidcore.SubjectProvider, nm netmap.Source, st netmap.State, cnrSource container.Source, nodePK []byte) Checker {
|
||||
return &checkerImpl{
|
||||
chainRouter: chainRouter,
|
||||
localOverrideStorage: localOverrideStorage,
|
||||
morphChainStorage: morphChainStorage,
|
||||
headerProvider: headerProvider,
|
||||
frostFSIDClient: frostFSIDClient,
|
||||
nm: nm,
|
||||
|
@ -174,28 +176,21 @@ func (c *checkerImpl) CheckAPE(ctx context.Context, prm Prm) error {
|
|||
groups[i] = fmt.Sprintf("%s:%s", prm.Namespace, groups[i])
|
||||
}
|
||||
|
||||
var cr policyengine.ChainRouter
|
||||
if prm.BearerToken != nil && !prm.BearerToken.Impersonate() {
|
||||
if err := isValidBearer(prm.BearerToken, prm.ContainerOwner, prm.Container, pub, c.st); err != nil {
|
||||
return fmt.Errorf("bearer token validation error: %w", err)
|
||||
}
|
||||
btRouter, err := router.SingleUseRouterWithBearerTokenChains([]bearer.APEOverride{prm.BearerToken.APEOverride()})
|
||||
cr, err = router.BearerChainFeedRouter(c.localOverrideStorage, c.morphChainStorage, prm.BearerToken.APEOverride())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
status, found, err := btRouter.IsAllowed(apechain.Ingress, policyengine.NewRequestTargetWithContainer(prm.Container.EncodeToString()), r)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if found && status == apechain.Allow {
|
||||
return nil
|
||||
}
|
||||
if status != apechain.NoRuleFound {
|
||||
return fmt.Errorf("bearer token: method %s: %s", prm.Method, status)
|
||||
return fmt.Errorf("create chain router error: %w", err)
|
||||
}
|
||||
} else {
|
||||
cr = policyengine.NewDefaultChainRouterWithLocalOverrides(c.morphChainStorage, c.localOverrideStorage)
|
||||
}
|
||||
|
||||
rt := policyengine.NewRequestTargetExtended(prm.Namespace, prm.Container.EncodeToString(), fmt.Sprintf("%s:%s", prm.Namespace, pub.Address()), groups)
|
||||
status, ruleFound, err := c.chainRouter.IsAllowed(apechain.Ingress, rt, r)
|
||||
status, ruleFound, err := cr.IsAllowed(apechain.Ingress, rt, r)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
|
@ -469,9 +469,8 @@ func TestAPECheck_BearerTokenOverrides(t *testing.T) {
|
|||
|
||||
ls := inmemory.NewInmemoryLocalStorage()
|
||||
ms := inmemory.NewInmemoryMorphRuleChainStorage()
|
||||
r := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls)
|
||||
|
||||
checker := NewChecker(r, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil)
|
||||
checker := NewChecker(ls, ms, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil)
|
||||
|
||||
prm := Prm{
|
||||
Method: method,
|
||||
|
@ -534,9 +533,7 @@ func TestAPECheck(t *testing.T) {
|
|||
})
|
||||
}
|
||||
|
||||
router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls)
|
||||
|
||||
checker := NewChecker(router, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil)
|
||||
checker := NewChecker(ls, ms, headerProvider, frostfsidProvider, nil, &stMock{}, nil, nil)
|
||||
|
||||
prm := Prm{
|
||||
Method: method,
|
||||
|
@ -639,8 +636,6 @@ func TestPutECChunk(t *testing.T) {
|
|||
MatchType: chain.MatchTypeFirstMatch,
|
||||
})
|
||||
|
||||
router := policyengine.NewDefaultChainRouterWithLocalOverrides(ms, ls)
|
||||
|
||||
node1Key, err := keys.NewPrivateKey()
|
||||
require.NoError(t, err)
|
||||
node1 := netmapSDK.NodeInfo{}
|
||||
|
@ -669,7 +664,7 @@ func TestPutECChunk(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
checker := NewChecker(router, headerProvider, frostfsidProvider, nm, &stMock{}, cs, node1Key.PublicKey().Bytes())
|
||||
checker := NewChecker(ls, ms, headerProvider, frostfsidProvider, nm, &stMock{}, cs, node1Key.PublicKey().Bytes())
|
||||
|
||||
ecParentID := oidtest.ID()
|
||||
chunkHeader := newHeaderObjectSDK(cnr, obj, nil).ToV2().GetHeader()
|
||||
|
@ -711,7 +706,7 @@ func TestPutECChunk(t *testing.T) {
|
|||
t.Run("access allowed for non container node", func(t *testing.T) {
|
||||
otherKey, err := keys.NewPrivateKey()
|
||||
require.NoError(t, err)
|
||||
checker = NewChecker(router, headerProvider, frostfsidProvider, nm, &stMock{}, cs, otherKey.PublicKey().Bytes())
|
||||
checker = NewChecker(ls, ms, headerProvider, frostfsidProvider, nm, &stMock{}, cs, otherKey.PublicKey().Bytes())
|
||||
prm := Prm{
|
||||
Method: nativeschema.MethodPutObject,
|
||||
Container: cnr,
|
||||
|
|
|
@ -140,25 +140,17 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token,
|
|||
return apeErr(err)
|
||||
}
|
||||
|
||||
var cr engine.ChainRouter
|
||||
if bt != nil && !bt.Impersonate() {
|
||||
if err := isValidBearer(bt, container.Value.Owner(), cid, publicKey, s.state); err != nil {
|
||||
return fmt.Errorf("bearer validation error: %w", err)
|
||||
}
|
||||
btRouter, err := router.SingleUseRouterWithBearerTokenChains([]bearer.APEOverride{bt.APEOverride()})
|
||||
cr, err = router.BearerChainFeedRouter(s.localOverrideStorage, s.morphChainStorage, bt.APEOverride())
|
||||
if err != nil {
|
||||
return apeErr(err)
|
||||
}
|
||||
status, found, err := btRouter.IsAllowed(apechain.Ingress, engine.NewRequestTargetWithContainer(cid.EncodeToString()), request)
|
||||
if err != nil {
|
||||
return apeErr(err)
|
||||
}
|
||||
if found && status == apechain.Allow {
|
||||
return nil
|
||||
}
|
||||
if status != apechain.NoRuleFound {
|
||||
err = fmt.Errorf("access to operation %s is denied by access policy engine (bearer token): %s", request.Operation(), status.String())
|
||||
return apeErr(err)
|
||||
return fmt.Errorf("create chain router error: %w", err)
|
||||
}
|
||||
} else {
|
||||
cr = engine.NewDefaultChainRouterWithLocalOverrides(s.morphChainStorage, s.localOverrideStorage)
|
||||
}
|
||||
|
||||
groups, err := aperequest.Groups(s.frostfsidSubjectProvider, publicKey)
|
||||
|
@ -172,7 +164,7 @@ func (s *Service) checkAPE(ctx context.Context, bt *bearer.Token,
|
|||
}
|
||||
|
||||
rt := engine.NewRequestTargetExtended(namespace, cid.EncodeToString(), fmt.Sprintf("%s:%s", namespace, publicKey.Address()), groups)
|
||||
status, found, err := s.router.IsAllowed(apechain.Ingress, rt, request)
|
||||
status, found, err := cr.IsAllowed(apechain.Ingress, rt, request)
|
||||
if err != nil {
|
||||
return apeErr(err)
|
||||
}
|
||||
|
|
|
@ -42,7 +42,8 @@ type cfg struct {
|
|||
containerCacheSize int
|
||||
authorizedKeys [][]byte
|
||||
|
||||
router policyengine.ChainRouter
|
||||
localOverrideStorage policyengine.LocalOverrideStorage
|
||||
morphChainStorage policyengine.MorphRuleChainStorageReader
|
||||
|
||||
metrics MetricsRegister
|
||||
}
|
||||
|
@ -152,9 +153,15 @@ func WithAuthorizedKeys(keys keys.PublicKeys) Option {
|
|||
}
|
||||
}
|
||||
|
||||
func WithAPERouter(router policyengine.ChainRouter) Option {
|
||||
func WithAPELocalOverrideStorage(localOverrideStorage policyengine.LocalOverrideStorage) Option {
|
||||
return func(c *cfg) {
|
||||
c.router = router
|
||||
c.localOverrideStorage = localOverrideStorage
|
||||
}
|
||||
}
|
||||
|
||||
func WithAPEMorphRuleStorage(morphRuleStorage policyengine.MorphRuleChainStorageReader) Option {
|
||||
return func(c *cfg) {
|
||||
c.morphChainStorage = morphRuleStorage
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Why was it protected with mutex before?
TBH, I don't remember. As far as I can recall I put this mutex guard had been added on someone's demand in pull request's review and probably the design was changed but mutex has been left.
When I glanced at the code again I found this guard useless.
After I removed this guard I tested the storage node with k6 (10 writers, 10 readers) - I found no concurrency panics