TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 100 Pull requests 4 Projects Releases 1 Activity Actions

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
Member
Copy link

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
Member
Copy link

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
Author
Member
Copy link

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
Author
Member
Copy link

@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
Member
Copy link

@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
Member
Copy link

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
Author
Member
Copy link

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
Author
Member
Copy link

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
Author
Member
Copy link

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
Author
Member
Copy link

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
Copy link

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
Copy link

@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
Member
Copy link

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
Author
Member
Copy link

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
Member
Copy link

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
Author
Member
Copy link

@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
master
support/v0.42
support/v0.38
support/v0.37
support/v0.36
support/v0.34
support/v0.30
support/v0.27
v0.44.0-rc.13
v0.44.0-rc.12
v0.44.0-rc.11
v0.42.16
v0.44.0-rc.10
v0.44.0-rc.9
v0.44.0-rc.8
v0.44.0-rc.7
v0.44.0-rc.6
v0.44.0-rc.5
v0.44.0-rc.4
v0.44.0-rc.3
v0.44.0-rc.2
v0.44.0-rc.1
v0.43.2
v0.43.1
v0.43.0
v0.42.15
v0.42.14
v0.42.13
v0.42.12
v0.42.11
v0.42.10
v0.42.9
v0.42.8
v0.42.7
v0.42.6
v0.42.5
v0.42.4
v0.42.3
v0.42.2
v0.42.1
v0.42.0
v0.42.0-rc.9
v0.42.0-rc.8
v0.42.0-rc.7
v0.38.9
v0.42.0-rc.6
v0.42.0-rc.5
v0.42.0-rc.4
v0.42.0-rc.3
v0.42.0-rc.2
v0.42.0-rc.1
v0.38.8
v0.41.0
v0.38.7
v0.40.0
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
v0.22.1
v0.22.0
v0.21.1
v0.21.0
v0.20.0
v0.19.0
v0.18.0
v0.17.0
v0.16.0
v0.15.0
v0.14.3
v0.14.2
v0.14.1
v0.14.0
v0.14.0-rc.1
v0.13.2
v0.13.1
v0.13.0
v0.13.0-rc.1
v0.12.1
v0.12.0
v0.12.0-rc3
v0.12.0-rc2
v0.12.0-rc1
v0.11.0
v0.10.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 labels
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
Projects
Clear projects
No items
No project
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
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?