TrueCloudLab
/
frostfs-node
19
1
Fork 20
Code Issues 93 Pull Requests 8 Actions Releases Activity

Invalid APE request forming #934

New Issue
Closed
opened 2024-01-25 15:42:07 +00:00 by dkirillov · 15 comments
dkirillov commented 2024-01-25 15:42:07 +00:00

APE chain rules that was set as local overrides work incorrectly

Expected Behavior

Access denies on operations that is specified in chain rules.

Current Behavior

Rules not found so we can create container/put object

Possible Solution

As I can see when we check APE rules in node we not always use appropriate namespace:

Steps to Reproduce (for bugs)

The following test should pass (note for the two last tests we need registered kapusta.nns (nns because ns currently cannot be TLD) in NNS):

import (
	"bytes"
	"context"
	"fmt"
	"testing"

	rpcv2client "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client"
	nodeControl "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control"
	nodeControlSrv "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control/server"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl"
	cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object"
	oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool"
	"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
	"git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
	"github.com/stretchr/testify/require"
)

func TestNodeAPE(t *testing.T) {
	ctx := context.Background()

	// devenv key, make sure this key
	// * is authorized in node control service
	// * registered in frostfid contract in 'kapusta` namespace
	key, err := keys.NewPrivateKeyFromWIF("KxDgvEKzgSBPPfuVfw67oPQBSjidEiqTHURKSDL1R7yGaGYAeYnr")
	require.NoError(t, err)
	var ownerID user.ID
	user.IDFromKey(&ownerID, key.PrivateKey.PublicKey)

	var controlCli client.Client
	controlCli.Init(client.PrmInit{Key: key.PrivateKey})
	err = controlCli.Dial(ctx, client.PrmDial{Endpoint: "s01.frostfs.devenv:8081"})
	require.NoError(t, err)

	var prm pool.InitParameters
	prm.SetKey(&key.PrivateKey)
	prm.AddNode(pool.NewNodeParam(1, "s01.frostfs.devenv:8080", 1))
	p, err := pool.NewPool(prm)
	require.NoError(t, err)
	err = p.Dial(ctx)
	require.NoError(t, err)

	t.Run("check restrict put object", func(t *testing.T) {
		cnrID, err := createContainer(ctx, p, ownerID, "", "")
		require.NoError(t, err)

		chainRestrictObjectPut := &chain.Chain{ID: "chainRestrictObjectPut", Rules: []chain.Rule{{
			Status:    chain.AccessDenied,
			Actions:   chain.Actions{Names: []string{native.MethodPutObject}},
			Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID.String())}},
		}}}

		err = putPolicy(&controlCli, key, "", chainRestrictObjectPut)
		require.NoError(t, err)
		_, err = putObject(ctx, p, cnrID, ownerID, "")
		require.ErrorContains(t, err, "denied")

		err = removePolicy(&controlCli, key, "", chainRestrictObjectPut.ID)
		require.NoError(t, err)
	})

	t.Run("check restrict put object with name", func(t *testing.T) {
		cnrID, err := createContainer(ctx, p, ownerID, "", "")
		require.NoError(t, err)

		chainRestrictObjectPutNamed := &chain.Chain{ID: "chainRestrictObjectPutNamed", Rules: []chain.Rule{{
			Status:    chain.AccessDenied,
			Actions:   chain.Actions{Names: []string{native.MethodPutObject}},
			Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID.String())}},
			Condition: []chain.Condition{{
				Op:     chain.CondStringEquals,
				Object: chain.ObjectResource,
				Key:    "FileName",
				Value:  "objectName",
			}},
		}}}

		err = putPolicy(&controlCli, key, "", chainRestrictObjectPutNamed)
		require.NoError(t, err)
		_, err = putObject(ctx, p, cnrID, ownerID, "objectName")
		require.ErrorContains(t, err, "denied")

		err = removePolicy(&controlCli, key, "", chainRestrictObjectPutNamed.ID)
		require.NoError(t, err)
	})

	t.Run("check restrict create container in custom ns", func(t *testing.T) {
		chainRestrictContainerPut := &chain.Chain{ID: "chainRestrictContainerPut", Rules: []chain.Rule{{
			Status:    chain.AccessDenied,
			Actions:   chain.Actions{Names: []string{native.MethodPutContainer}},
			Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatNamespaceContainers, "kapusta")}},
		}}}

		err = putPolicy(&controlCli, key, "kapusta", chainRestrictContainerPut)
		require.NoError(t, err)
		_, err = createContainer(ctx, p, ownerID, "test30", "kapusta.ns") // make sure kapusta.ns is registered to nns
		require.ErrorContains(t, err, "denied")

		err = removePolicy(&controlCli, key, "kapusta", chainRestrictContainerPut.ID)
		require.NoError(t, err)
	})

	t.Run("check restrict create container in root ns and can create in another ns", func(t *testing.T) {
		chainRestrictContainerPutRoot := &chain.Chain{ID: "chainRestrictContainerPutRoot", Rules: []chain.Rule{{
			Status:    chain.AccessDenied,
			Actions:   chain.Actions{Names: []string{native.MethodPutContainer}},
			Resources: chain.Resources{Names: []string{native.ResourceFormatRootContainers}},
		}}}

		err = putPolicy(&controlCli, key, "", chainRestrictContainerPutRoot)
		require.NoError(t, err)

		_, err = createContainer(ctx, p, ownerID, "test28", "container")
		require.ErrorContains(t, err, "denied")
		fmt.Println(err)
		_, err = createContainer(ctx, p, ownerID, "test27", "kapusta.ns") // make sure kapusta.ns is registered to nns
		require.NoError(t, err)

		err = removePolicy(&controlCli, key, "", chainRestrictContainerPutRoot.ID)
		require.NoError(t, err)
	})
}

func createContainer(ctx context.Context, p *pool.Pool, ownerID user.ID, name, zone string) (cid.ID, error) {
	var pp netmap.PlacementPolicy
	if err := pp.DecodeString("REP 1"); err != nil {
		return cid.ID{}, err
	}

	var cnr container.Container
	cnr.Init()
	cnr.SetPlacementPolicy(pp)
	cnr.SetOwner(ownerID)
	cnr.SetBasicACL(acl.PublicRWExtended)

	if zone != "" {
		var d container.Domain
		d.SetName(name)
		d.SetZone(zone)
		container.WriteDomain(&cnr, d)
	}

	if err := pool.SyncContainerWithNetwork(ctx, &cnr, p); err != nil {
		return cid.ID{}, err
	}

	return p.PutContainer(ctx, pool.PrmContainerPut{ClientParams: client.PrmContainerPut{Container: &cnr}})
}

func putObject(ctx context.Context, p *pool.Pool, cnrID cid.ID, ownerID user.ID, fileName string) (oid.ID, error) {
	attr := object.NewAttribute()
	attr.SetKey("FileName")
	attr.SetValue(fileName)

	obj := object.New()
	obj.SetContainerID(cnrID)
	obj.SetOwnerID(ownerID)
	if fileName != "" {
		obj.SetAttributes(*attr)
	}

	var prmObjPut pool.PrmObjectPut
	prmObjPut.SetHeader(*obj)
	prmObjPut.SetPayload(bytes.NewBufferString("test payload"))

	return p.PutObject(ctx, prmObjPut)
}

func putPolicy(cli *client.Client, key *keys.PrivateKey, namespace string, policyChain *chain.Chain) error {
	req := &nodeControl.AddChainLocalOverrideRequest{
		Body: &nodeControl.AddChainLocalOverrideRequest_Body{
			Target: &nodeControl.ChainTarget{
				Type: nodeControl.ChainTarget_NAMESPACE,
				Name: namespace,
			},
			Chain: policyChain.Bytes(),
		},
	}

	if err := nodeControlSrv.SignMessage(&key.PrivateKey, req); err != nil {
		return fmt.Errorf("sing msg for node control svc: %w", err)
	}

	return cli.ExecRaw(func(c *rpcv2client.Client) error {
		_, err := nodeControl.AddChainLocalOverride(c, req)
		return err
	})
}

func removePolicy(cli *client.Client, key *keys.PrivateKey, namespace string, chainID chain.ID) error {
	req := &nodeControl.RemoveChainLocalOverrideRequest{
		Body: &nodeControl.RemoveChainLocalOverrideRequest_Body{
			Target: &nodeControl.ChainTarget{
				Type: nodeControl.ChainTarget_NAMESPACE,
				Name: namespace,
			},
			ChainId: []byte(chainID),
		},
	}

	if err := nodeControlSrv.SignMessage(&key.PrivateKey, req); err != nil {
		return fmt.Errorf("sing msg for node control svc: %w", err)
	}

	return cli.ExecRaw(func(c *rpcv2client.Client) error {
		_, err := nodeControl.RemoveChainLocalOverride(c, req)
		return err
	})
}

Regression

No

Your Environment

  • Version used: d13e37f70b
  • Server setup and configuration: devenv
APE chain rules that was set as local overrides work incorrectly ## Expected Behavior Access denies on operations that is specified in chain rules. ## Current Behavior Rules not found so we can create container/put object ## Possible Solution As I can see when we check APE rules in node we not always use appropriate namespace: * [this](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/container/ape.go#L175) should not be empty I suppose * when we add local overrides we change [`""` namespace to `root`](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/control/server/policy_engine.go#L22-L24) but when form namespace from request don't do the same and try to find rules in `""` ns though they are in `root`. Actually, for object service (at least for `Send`) we [don't take into account namespace at all](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L458-L463) * [getting namespace for request](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L747) should properly handle custom ns with `.ns` suffix ## Steps to Reproduce (for bugs) The following test should pass (**note** for the two last tests we need registered `kapusta.nns` (nns because `ns` currently cannot be TLD) in `NNS`): ```golang import ( "bytes" "context" "fmt" "testing" rpcv2client "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/rpc/client" nodeControl "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control" nodeControlSrv "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/control/server" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/acl" cid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/container/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/netmap" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" oid "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object/id" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/pool" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/stretchr/testify/require" ) func TestNodeAPE(t *testing.T) { ctx := context.Background() // devenv key, make sure this key // * is authorized in node control service // * registered in frostfid contract in 'kapusta` namespace key, err := keys.NewPrivateKeyFromWIF("KxDgvEKzgSBPPfuVfw67oPQBSjidEiqTHURKSDL1R7yGaGYAeYnr") require.NoError(t, err) var ownerID user.ID user.IDFromKey(&ownerID, key.PrivateKey.PublicKey) var controlCli client.Client controlCli.Init(client.PrmInit{Key: key.PrivateKey}) err = controlCli.Dial(ctx, client.PrmDial{Endpoint: "s01.frostfs.devenv:8081"}) require.NoError(t, err) var prm pool.InitParameters prm.SetKey(&key.PrivateKey) prm.AddNode(pool.NewNodeParam(1, "s01.frostfs.devenv:8080", 1)) p, err := pool.NewPool(prm) require.NoError(t, err) err = p.Dial(ctx) require.NoError(t, err) t.Run("check restrict put object", func(t *testing.T) { cnrID, err := createContainer(ctx, p, ownerID, "", "") require.NoError(t, err) chainRestrictObjectPut := &chain.Chain{ID: "chainRestrictObjectPut", Rules: []chain.Rule{{ Status: chain.AccessDenied, Actions: chain.Actions{Names: []string{native.MethodPutObject}}, Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID.String())}}, }}} err = putPolicy(&controlCli, key, "", chainRestrictObjectPut) require.NoError(t, err) _, err = putObject(ctx, p, cnrID, ownerID, "") require.ErrorContains(t, err, "denied") err = removePolicy(&controlCli, key, "", chainRestrictObjectPut.ID) require.NoError(t, err) }) t.Run("check restrict put object with name", func(t *testing.T) { cnrID, err := createContainer(ctx, p, ownerID, "", "") require.NoError(t, err) chainRestrictObjectPutNamed := &chain.Chain{ID: "chainRestrictObjectPutNamed", Rules: []chain.Rule{{ Status: chain.AccessDenied, Actions: chain.Actions{Names: []string{native.MethodPutObject}}, Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID.String())}}, Condition: []chain.Condition{{ Op: chain.CondStringEquals, Object: chain.ObjectResource, Key: "FileName", Value: "objectName", }}, }}} err = putPolicy(&controlCli, key, "", chainRestrictObjectPutNamed) require.NoError(t, err) _, err = putObject(ctx, p, cnrID, ownerID, "objectName") require.ErrorContains(t, err, "denied") err = removePolicy(&controlCli, key, "", chainRestrictObjectPutNamed.ID) require.NoError(t, err) }) t.Run("check restrict create container in custom ns", func(t *testing.T) { chainRestrictContainerPut := &chain.Chain{ID: "chainRestrictContainerPut", Rules: []chain.Rule{{ Status: chain.AccessDenied, Actions: chain.Actions{Names: []string{native.MethodPutContainer}}, Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatNamespaceContainers, "kapusta")}}, }}} err = putPolicy(&controlCli, key, "kapusta", chainRestrictContainerPut) require.NoError(t, err) _, err = createContainer(ctx, p, ownerID, "test30", "kapusta.ns") // make sure kapusta.ns is registered to nns require.ErrorContains(t, err, "denied") err = removePolicy(&controlCli, key, "kapusta", chainRestrictContainerPut.ID) require.NoError(t, err) }) t.Run("check restrict create container in root ns and can create in another ns", func(t *testing.T) { chainRestrictContainerPutRoot := &chain.Chain{ID: "chainRestrictContainerPutRoot", Rules: []chain.Rule{{ Status: chain.AccessDenied, Actions: chain.Actions{Names: []string{native.MethodPutContainer}}, Resources: chain.Resources{Names: []string{native.ResourceFormatRootContainers}}, }}} err = putPolicy(&controlCli, key, "", chainRestrictContainerPutRoot) require.NoError(t, err) _, err = createContainer(ctx, p, ownerID, "test28", "container") require.ErrorContains(t, err, "denied") fmt.Println(err) _, err = createContainer(ctx, p, ownerID, "test27", "kapusta.ns") // make sure kapusta.ns is registered to nns require.NoError(t, err) err = removePolicy(&controlCli, key, "", chainRestrictContainerPutRoot.ID) require.NoError(t, err) }) } func createContainer(ctx context.Context, p *pool.Pool, ownerID user.ID, name, zone string) (cid.ID, error) { var pp netmap.PlacementPolicy if err := pp.DecodeString("REP 1"); err != nil { return cid.ID{}, err } var cnr container.Container cnr.Init() cnr.SetPlacementPolicy(pp) cnr.SetOwner(ownerID) cnr.SetBasicACL(acl.PublicRWExtended) if zone != "" { var d container.Domain d.SetName(name) d.SetZone(zone) container.WriteDomain(&cnr, d) } if err := pool.SyncContainerWithNetwork(ctx, &cnr, p); err != nil { return cid.ID{}, err } return p.PutContainer(ctx, pool.PrmContainerPut{ClientParams: client.PrmContainerPut{Container: &cnr}}) } func putObject(ctx context.Context, p *pool.Pool, cnrID cid.ID, ownerID user.ID, fileName string) (oid.ID, error) { attr := object.NewAttribute() attr.SetKey("FileName") attr.SetValue(fileName) obj := object.New() obj.SetContainerID(cnrID) obj.SetOwnerID(ownerID) if fileName != "" { obj.SetAttributes(*attr) } var prmObjPut pool.PrmObjectPut prmObjPut.SetHeader(*obj) prmObjPut.SetPayload(bytes.NewBufferString("test payload")) return p.PutObject(ctx, prmObjPut) } func putPolicy(cli *client.Client, key *keys.PrivateKey, namespace string, policyChain *chain.Chain) error { req := &nodeControl.AddChainLocalOverrideRequest{ Body: &nodeControl.AddChainLocalOverrideRequest_Body{ Target: &nodeControl.ChainTarget{ Type: nodeControl.ChainTarget_NAMESPACE, Name: namespace, }, Chain: policyChain.Bytes(), }, } if err := nodeControlSrv.SignMessage(&key.PrivateKey, req); err != nil { return fmt.Errorf("sing msg for node control svc: %w", err) } return cli.ExecRaw(func(c *rpcv2client.Client) error { _, err := nodeControl.AddChainLocalOverride(c, req) return err }) } func removePolicy(cli *client.Client, key *keys.PrivateKey, namespace string, chainID chain.ID) error { req := &nodeControl.RemoveChainLocalOverrideRequest{ Body: &nodeControl.RemoveChainLocalOverrideRequest_Body{ Target: &nodeControl.ChainTarget{ Type: nodeControl.ChainTarget_NAMESPACE, Name: namespace, }, ChainId: []byte(chainID), }, } if err := nodeControlSrv.SignMessage(&key.PrivateKey, req); err != nil { return fmt.Errorf("sing msg for node control svc: %w", err) } return cli.ExecRaw(func(c *rpcv2client.Client) error { _, err := nodeControl.RemoveChainLocalOverride(c, req) return err }) } ``` ## Regression No ## Your Environment <!-- Include as many relevant details about the environment you experienced the bug in --> * Version used: https://git.frostfs.info/TrueCloudLab/frostfs-node/commit/d13e37f70b44dfe7fe002ebf327c0d293093c0e5 * Server setup and configuration: devenv
dkirillov added the
bug
triage
 labels 2024-01-25 15:42:07 +00:00
fyrchik added the
frostfs-node
 label 2024-01-25 15:58:50 +00:00
aarifullin was assigned by fyrchik 2024-01-25 15:58:53 +00:00
fyrchik added this to the v0.38.0 milestone 2024-01-25 15:59:23 +00:00
aarifullin commented 2024-01-26 07:51:02 +00:00
Collaborator

I suppose you are right about setting the empty namespace in Container Service - that approach should be fixed.

But let's consider this test:

t.Run("check restrict put object with name", func(t *testing.T) {
		cnrID, err := createContainer(ctx, p, ownerID, "", "")
		require.Error(t, err) // no error will happen

		chainRestrictObjectPutNamed := &chain.Chain{ID: "chainRestrictObjectPutNamed", Rules: []chain.Rule{{
			Status:    chain.AccessDenied,
			Actions:   chain.Actions{Names: []string{native.MethodPutObject}},
			Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID.String())}},
			Condition: []chain.Condition{{
				Op:     chain.CondStringEquals,
				Object: chain.ObjectResource,
				Key:    "FileName",
				Value:  "objectName",
			}},
		}}}

		err = putPolicy(&controlCli, key, "", chainRestrictObjectPutNamed)
		require.NoError(t, err)
		_, err = putObject(ctx, p, cnrID, ownerID, "objectName")
		require.Error(t, err)
	})

You cannot expect an error because you have just created a rule about restricting actions for a container (with operation related to object manipulating, not container)

I suppose you are right about setting the empty namespace in Container Service - that approach should be fixed. But let's consider this test: ```go t.Run("check restrict put object with name", func(t *testing.T) { cnrID, err := createContainer(ctx, p, ownerID, "", "") require.Error(t, err) // no error will happen chainRestrictObjectPutNamed := &chain.Chain{ID: "chainRestrictObjectPutNamed", Rules: []chain.Rule{{ Status: chain.AccessDenied, Actions: chain.Actions{Names: []string{native.MethodPutObject}}, Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID.String())}}, Condition: []chain.Condition{{ Op: chain.CondStringEquals, Object: chain.ObjectResource, Key: "FileName", Value: "objectName", }}, }}} err = putPolicy(&controlCli, key, "", chainRestrictObjectPutNamed) require.NoError(t, err) _, err = putObject(ctx, p, cnrID, ownerID, "objectName") require.Error(t, err) }) ``` You **cannot** expect an error because you have just created a rule about restricting actions for a container (with operation related to **object** manipulating, not container)
dkirillov commented 2024-01-26 12:41:59 +00:00
Poster

I would expect error because I have just created a rule with native.ResourceFormatRootContainerObjects resource ("native:object//%s/*")

I **would** expect error because I have just created a rule with [native.ResourceFormatRootContainerObjects](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/0a28f0a9924ee89e9e10e6fd6e2fc15e3591aa9c/schema/native/consts.go#L27) resource (`"native:object//%s/*"`)
dkirillov commented 2024-01-29 09:10:29 +00:00
Poster

@aarifullin @fyrchik

Could you clarify a little this (using chain rules from root namespace when handle ListContainers request)?
I wonder If this final behavior or we can expect that namespace will be determined from frostfsid contract (using address of request owner)?

Among other things, can I be able to restrict this listing by owner from request?

cc @alexvanin

@aarifullin @fyrchik Could you clarify a little [this](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/c916a75948d0795afc38676dc8d506177a31476b/pkg/services/container/ape.go#L138) (using chain rules from root namespace when handle `ListContainers` request)? I wonder If this final behavior or we can expect that namespace will be determined from `frostfsid` contract (using address of request owner)? Among other things, can I be able to restrict this listing by owner from request? cc @alexvanin
aarifullin commented 2024-01-30 10:49:53 +00:00
Collaborator

@aarifullin @fyrchik

Could you clarify a little this (using chain rules from root namespace when handle ListContainers request)?
I wonder If this final behavior or we can expect that namespace will be determined from frostfsid contract (using address of request owner)?

Among other things, can I be able to restrict this listing by owner from request?

cc @alexvanin

Please, check this out: #940

can I be able to restrict this listing by owner from request?

This idea is implemented in this PR :)

> @aarifullin @fyrchik > > Could you clarify a little [this](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/c916a75948d0795afc38676dc8d506177a31476b/pkg/services/container/ape.go#L138) (using chain rules from root namespace when handle `ListContainers` request)? > I wonder If this final behavior or we can expect that namespace will be determined from `frostfsid` contract (using address of request owner)? > > Among other things, can I be able to restrict this listing by owner from request? > > cc @alexvanin Please, check this out: https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/940 > can I be able to restrict this listing by owner from request? This idea is implemented in this PR :)
aarifullin commented 2024-01-30 15:05:00 +00:00
Collaborator

Despite I have created the pull-request, some parts of the issue still cannot be reproduced and I suggest to consider them and discuss

  1. The case check restrict put object is OK. You have successfully created the policy and cannot put a new object
  2. The case check restrict put object with name is not OK (FMPOV). You are trying to create a container and expect an error. Why? The previous policy does not relate to container managment. But, further, expecting for object put operation deny is OK
  3. The case check restrict create container in custom ns: you need to not forget to add namespace kapusta to frostfs-id contract for this owner
frostfs-adm morph frostfsid create-namespace --namespace kapusta --rpc-endpoint='http://morph-chain.frostfs.devenv:30333' --alphabet-wallets='frostfs-dev-env/services/ir';

frostfs-adm morph frostfsid create-subject --namespace kapusta --subject-key "022bb4041c50d607ff871dec7e4cd7778388e0ea6849d84ccbd9aa8f32e16a8131" --subject-name subj1 --rpc-endpoint='http://morph-chain.frostfs.devenv:30333' --alphabet-wallets='frostfs-dev-env/services/ir';

Also you need to register ns, kapusta.ns in NNS contract to create container with such domain params. The rule works

  1. check restrict create container in root ns and can create in another ns: You need to expect an error for
_, err = createContainer(ctx, p, ownerID, "test3", "kapusta.ns") // make sure kapusta.nns is registered to nns
require.Error(t, err)

because you left previous rule for the namespace kapusta

As soon as the PR will be merged, container service will scan a namespace for ownerID from frostfs-id contract (Put, List method). Otherwise, a namespace is read from container's Zone property for those methods that suppose a container does exist (Get, SetExtendedACL etc.)

Despite I have created the pull-request, some parts of the issue still cannot be reproduced and I suggest to consider them and discuss 1. The case `check restrict put object` is OK. You have successfully created the policy and cannot put a new object 2. The case `check restrict put object with name` is not OK (FMPOV). You are trying to create a container and expect an error. Why? The previous policy does not relate to container managment. But, further, expecting for object put operation deny is OK 3. The case `check restrict create container in custom ns`: you need to not forget to add namespace kapusta to frostfs-id contract for this owner ```bash frostfs-adm morph frostfsid create-namespace --namespace kapusta --rpc-endpoint='http://morph-chain.frostfs.devenv:30333' --alphabet-wallets='frostfs-dev-env/services/ir'; frostfs-adm morph frostfsid create-subject --namespace kapusta --subject-key "022bb4041c50d607ff871dec7e4cd7778388e0ea6849d84ccbd9aa8f32e16a8131" --subject-name subj1 --rpc-endpoint='http://morph-chain.frostfs.devenv:30333' --alphabet-wallets='frostfs-dev-env/services/ir'; ``` Also you need to register `ns, kapusta.ns` in NNS contract to create container with such domain params. The rule works 4. `check restrict create container in root ns and can create in another ns`: You need to expect an error for ```go _, err = createContainer(ctx, p, ownerID, "test3", "kapusta.ns") // make sure kapusta.nns is registered to nns require.Error(t, err) ``` because you left previous rule for the namespace kapusta As soon as the PR will be merged, container service will scan a namespace for ownerID from frostfs-id contract (Put, List method). Otherwise, a namespace is read from container's Zone property for those methods that suppose a container does exist (Get, SetExtendedACL etc.)
dkirillov commented 2024-01-31 06:39:03 +00:00
Poster

The first and the second cases both has typo

cnrID, err := createContainer(ctx, p, ownerID, "", "")
require.Error(t, err)

should be

cnrID, err := createContainer(ctx, p, ownerID, "", "")
require.NoError(t, err)

(I'll update description)

The first and the second cases both has typo ```golang cnrID, err := createContainer(ctx, p, ownerID, "", "") require.Error(t, err) ``` should be ```golang cnrID, err := createContainer(ctx, p, ownerID, "", "") require.NoError(t, err) ``` (I'll update description)
dkirillov commented 2024-01-31 08:44:04 +00:00
Poster

As soon as the PR will be merged, container service will scan a namespace for ownerID from frostfs-id contract (Put, List method). Otherwise, a namespace is read from container's Zone property

I suppose we should forbid container creation if its owner from namespace1 but trying to create container ins zone namespace2.ns. Otherwise he can bypass (current behavior with PR) restriction rule that forbids creation any container in ns namespace2 (case 4 above)

> As soon as the PR will be merged, container service will scan a namespace for ownerID from frostfs-id contract (Put, List method). Otherwise, a namespace is read from container's Zone property I suppose we should forbid container creation if its owner from `namespace1` but trying to create container ins zone `namespace2.ns`. Otherwise he can bypass (current behavior with [PR](https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/940)) restriction rule that forbids creation any container in ns `namespace2` (case 4 above)
dkirillov commented 2024-01-31 08:49:12 +00:00
Poster

You to expect an error for ... because you left previous rule for the namespace kapusta

Whoops, fixed test code in issue

> You to expect an error for ... because you left previous rule for the namespace kapusta Whoops, fixed test code in issue
👍 1
dkirillov commented 2024-02-02 09:37:31 +00:00
Poster

Actually, for object service (at least for Send) we don't take into account namespace at all

Is there any RP that resolve problem with namespace in object service ?

> Actually, for object service (at least for Send) we [don't take into account namespace at all](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L458-L463) Is there any RP that resolve problem with namespace in object service ?
fyrchik commented 2024-02-02 11:20:13 +00:00
Owner

Closed via #940
cc @aarifullin

Closed via https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/940 cc @aarifullin
fyrchik commented 2024-02-02 11:22:50 +00:00
Owner

@dkirillov It seems this one #903

@dkirillov It seems this one https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/903
aarifullin commented 2024-02-02 11:45:09 +00:00
Collaborator

Actually, for object service (at least for Send) we don't take into account namespace at all

Is there any RP that resolve problem with namespace in object service ?

It does for

But I really forgot to introduce it for other methods (by requestContext)
I'll make PR

> > Actually, for object service (at least for Send) we [don't take into account namespace at all](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L458-L463) > > Is there any RP that resolve problem with namespace in object service ? > It does for - [Get](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L113-L119) - [Range](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L139-L141) - [Search](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/1fe7736d921a06411f8076cedb8edf68519aca62/pkg/services/object/acl/v2/service.go#L162-L164) But I really forgot to introduce it for other methods (by requestContext) I'll make PR
dkirillov commented 2024-02-02 11:48:29 +00:00
Poster

I'll make PR

Can we reopen this task then?

> I'll make PR Can we reopen this task then?
👍 1
fyrchik reopened this issue 2024-02-02 11:55:02 +00:00
aarifullin commented 2024-02-02 11:56:26 +00:00
Collaborator

I'll make PR

Can we reopen this task then?

#952

> > I'll make PR > > Can we reopen this task then? https://git.frostfs.info/TrueCloudLab/frostfs-node/pulls/952
dkirillov commented 2024-02-02 12:46:10 +00:00
Poster

@aarifullin @fyrchik

Sorry, but here (and for LIst the same)
we have to write hex.EncodeToString(pk.Bytes()) instead of pk.String()

@aarifullin @fyrchik Sorry, but [here](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/befbaf9d56e734b224e3aed0b118daa96865ad48/pkg/services/container/ape.go#L181) (and for LIst the same) we have to write `hex.EncodeToString(pk.Bytes())` instead of `pk.String()`
👀 1
fyrchik reopened this issue 2024-02-02 14:47:06 +00:00
fyrchik self-assigned this 2024-02-02 14:47:43 +00:00
aarifullin was unassigned by fyrchik 2024-02-02 14:47:43 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
support/v0.38
master
support/v0.37
support/v0.36
support/v0.34
support/v0.30
support/v0.27
v0.39.0
v0.38.6
v0.38.5
v0.38.4
v0.38.3
v0.38.2
v0.38.1
v0.38.0
v0.38.0-rc.2
v0.38.0-rc.1
v0.37.0
v0.37.0-rc.1
v0.36.0
v0.34.0
Labels
Clear labels   
P0

Highest   
P1

High   
P2

Medium   
P3

Low   
badger

Badger DB related issues   
frostfs-adm
   
frostfs-cli
   
frostfs-ir
   
frostfs-lens
   
frostfs-node
   
good first issue

Good for newcomers   
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff   
blocked

The issue or PR is blocked by something   
bug

Something is not working   
config

Configuration format update or breaking change   
discussion

Talking about something in order to reach a decision or to exchange ideas   
documentation

Documentation related   
duplicate

This issue or pull request already exists   
enhancement

New feature   
go

Language features adoption   
help wanted

Extra attention is needed   
internal
   
invalid

Something is wrong   
kludge

This is a kludge and we know it   
observability

Related to metrics, tracing and logs   
perfomance

Perfomance related   
question

More information is needed   
refactoring

Refactoring   
wontfix

This won't be fixed
No Label
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No Milestone
v0.38.0
Assignees
Clear assignees
No Assignees
fyrchik
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#934
There is no content yet.
Delete Branch "%!s(<nil>)"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?