Consider ACL filter optimizations #279

New issue

Open

opened 2025-12-28 17:36:50 +00:00 by sami · 2 comments

sami commented

2025-12-28 17:36:50 +00:00

Owner

Originally created by @alexvanin on GitHub (Jul 5, 2022).

Part one: simplify `eacl` -> `ast` decoding by using `system` filters

Use system filters to define grantees and permissions, like

SYSTEM: permission STRING_EQUAL FULL_CONTROL
SYSTEM: group STRING_EQUAL AllUsers

These values may be used during encoding and decoding of rules

Part two: reduce number of records in `eacl`

Some EACL records may not produce behaviour changes, but they required for decoding ast. We can try to remove such records and replace then by storing extra information in system filters of EACL records.

Originally created by @alexvanin on GitHub (Jul 5, 2022). ## Part one: simplify `eacl` -> `ast` decoding by using `system` filters Use system filters to define grantees and permissions, like ``` SYSTEM: permission STRING_EQUAL FULL_CONTROL SYSTEM: group STRING_EQUAL AllUsers ``` These values may be used during encoding and decoding of rules ## Part two: reduce number of records in `eacl` Some EACL records may not produce behaviour changes, but they required for decoding `ast`. We can try to remove such records and replace then by storing extra information in system filters of EACL records.

sami added the

enhancement

performance

labels

2025-12-28 17:36:50 +00:00

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@alexvanin commented on GitHub (Jul 21, 2022):

In https://github.com/nspcc-dev/neofs-s3-gw/pull/605 a tried to fix some issues but acl parse order kicked me in the guts. Here I briefly describe what I tried.

bucketACLToTable upgrade

I want to use AST to provide correct bucket ACL, so I come up with something like this

func bucketACLToTable(acp *AccessControlPolicy, resInfo *resourceInfo) (*eacl.Table, error) {
	if !resInfo.IsBucket() {
		return nil, fmt.Errorf("allowed only bucket acl")
	}

	ownerKey, err := keys.NewPublicKeyFromString(acp.Owner.ID)
	if err != nil {
		return nil, fmt.Errorf("public key from string: %w", err)
	}

	r := astResource{
		resourceInfo: *resInfo,
		Operations:   make([]*astOperation, 0, len(acp.AccessControlList)),
	}

	// first loop
	for _, grant := range acp.AccessControlList {
		if !isValidGrant(grant) {
			return nil, stderrors.New("unsupported grantee")
		}
		getRecord, err := getRecordFunction(grant.Grantee)
		if err != nil {
			return nil, fmt.Errorf("record func from grantee: %w", err)
		}
		for _, op := range permissionToOperations(grant.Permission) {
			rec := getRecord(op)
			r.Operations = addToList(r.Operations, *rec, rec.Targets()[0])
		}
	}

	// second loop 
	for _, op := range fullOps {
		rec := getOthersRecord(op, eacl.ActionDeny)
		r.Operations = addToList(r.Operations, *rec, rec.Targets()[0])
	}

	// third loop
	for _, op := range fullOps {
		rec := getAllowRecord(op, ownerKey)
		r.Operations = addToList(r.Operations, *rec, rec.Targets()[0])
	}

	return astToTable(&ast{Resources: []*astResource{&r}})
}

To make it work, remove mandatory full control grant from nspcc-dev/neofs-s3-gw@7ba7e7dc4d/api/handler/acl.go (L468-L475)

It works very well when creating buckets with canned ACL without grants. However when grant appears, it turns out that DENY record is going to be placed above grant rule, which is incorrect.

This happens because DENY rules appear on the second loop after processing grants. If they go before grants, then there will be useless DENY rules in eACL.

Public object table record

After #605 private objects have correct eACL field order: owner rules above others rules.

ALLOW GET          [ SERVICE:Resource MATCH_TYPE_UNSPECIFIED mid400/foo/versioned-object1:HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS, SERVICE:GroupLength MATCH_TYPE_UNSPECIFIED 10 ] [ SYSTEM:{} ]
ALLOW GETRANGEHASH [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} 
ALLOW GETRANGE     [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} 
ALLOW SEARCH       [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} 
ALLOW HEAD         [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} 
ALLOW GET          [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} 
DENY  GETRANGEHASH [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ OTHERS:{} ]
DENY  GETRANGE     [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ OTHERS:{} ]
DENY  SEARCH       [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ OTHERS:{} ]
DENY  HEAD         [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ OTHERS:{} ]
DENY  GET          [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ]                                                                                [ OTHERS:{} ]

However public objects produce reversed order. It doesn't affect access control, but it looks odd.

To fix that we can set owner related record as last statement. But I am not sure if the change in the order is safe.

func aclToPolicy(acl *AccessControlPolicy, resInfo *resourceInfo) (*bucketPolicy, error) {
	. . .
	results := make([]statement, 0, len(acl.AccessControlList)+1)
	. . .
	results = append(results, getAllowStatement(resInfo, acl.Owner.ID, aclFullControl))

	return &bucketPolicy{
		Statement: results,
		Bucket:    resInfo.Bucket,
	}, nil
}

@alexvanin commented on GitHub (Jul 21, 2022): In https://github.com/nspcc-dev/neofs-s3-gw/pull/605 a tried to fix some issues but acl parse order kicked me in the guts. Here I briefly describe what I tried. ## bucketACLToTable upgrade I want to use AST to provide correct bucket ACL, so I come up with something like this ```go func bucketACLToTable(acp *AccessControlPolicy, resInfo *resourceInfo) (*eacl.Table, error) { if !resInfo.IsBucket() { return nil, fmt.Errorf("allowed only bucket acl") } ownerKey, err := keys.NewPublicKeyFromString(acp.Owner.ID) if err != nil { return nil, fmt.Errorf("public key from string: %w", err) } r := astResource{ resourceInfo: *resInfo, Operations: make([]*astOperation, 0, len(acp.AccessControlList)), } // first loop for _, grant := range acp.AccessControlList { if !isValidGrant(grant) { return nil, stderrors.New("unsupported grantee") } getRecord, err := getRecordFunction(grant.Grantee) if err != nil { return nil, fmt.Errorf("record func from grantee: %w", err) } for _, op := range permissionToOperations(grant.Permission) { rec := getRecord(op) r.Operations = addToList(r.Operations, *rec, rec.Targets()[0]) } } // second loop for _, op := range fullOps { rec := getOthersRecord(op, eacl.ActionDeny) r.Operations = addToList(r.Operations, *rec, rec.Targets()[0]) } // third loop for _, op := range fullOps { rec := getAllowRecord(op, ownerKey) r.Operations = addToList(r.Operations, *rec, rec.Targets()[0]) } return astToTable(&ast{Resources: []*astResource{&r}}) } ``` To make it work, remove mandatory full control grant from https://github.com/nspcc-dev/neofs-s3-gw/blob/7ba7e7dc4db079209593cfa0d7f79b2e722afced/api/handler/acl.go#L468-L475 It works very well when creating buckets with canned ACL without grants. However when grant appears, it turns out that DENY record is going to be placed **above** grant rule, which is incorrect. This happens because DENY rules appear on the second loop after processing grants. If they go before grants, then there will be useless DENY rules in eACL. ## Public object table record After #605 private objects have correct eACL field order: owner rules above others rules. ``` ALLOW GET [ SERVICE:Resource MATCH_TYPE_UNSPECIFIED mid400/foo/versioned-object1:HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS, SERVICE:GroupLength MATCH_TYPE_UNSPECIFIED 10 ] [ SYSTEM:{} ] ALLOW GETRANGEHASH [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} ALLOW GETRANGE [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} ALLOW SEARCH [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} ALLOW HEAD [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} ALLOW GET [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ ROLE_UNSPECIFIED:{031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a} DENY GETRANGEHASH [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ OTHERS:{} ] DENY GETRANGE [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ OTHERS:{} ] DENY SEARCH [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ OTHERS:{} ] DENY HEAD [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ OTHERS:{} ] DENY GET [ OBJECT:$Object:objectID STRING_EQUAL HPVZPnTpkByWybYcPGUCcxyt3qpvgqo4gZkbvTSatJSS ] [ OTHERS:{} ] ``` However public objects produce reversed order. It doesn't affect access control, but it looks odd. To fix that we can set owner related record as last statement. But I am not sure if the change in the order is safe. ```go func aclToPolicy(acl *AccessControlPolicy, resInfo *resourceInfo) (*bucketPolicy, error) { . . . results := make([]statement, 0, len(acl.AccessControlList)+1) . . . results = append(results, getAllowStatement(resInfo, acl.Owner.ID, aclFullControl)) return &bucketPolicy{ Statement: results, Bucket: resInfo.Bucket, }, nil } ```

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@alexvanin commented on GitHub (Nov 3, 2022):

All optimization will make more sense after https://github.com/nspcc-dev/neofs-api/issues/241

@alexvanin commented on GitHub (Nov 3, 2022): All optimization will make more sense after https://github.com/nspcc-dev/neofs-api/issues/241