From ec39d8371adf6b8f1779df994473563c0641a9db Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Mon, 18 Dec 2023 17:00:31 +0300 Subject: [PATCH] [#36] iam: Support iam actions Signed-off-by: Denis Kirillov --- iam/converter.go | 12 +++--- iam/converter_native.go | 13 ++++--- iam/converter_test.go | 86 ++++++++++++++++++++++++++++++++--------- 3 files changed, 80 insertions(+), 31 deletions(-) diff --git a/iam/converter.go b/iam/converter.go index 4749526..d7cd4a0 100644 --- a/iam/converter.go +++ b/iam/converter.go @@ -56,6 +56,7 @@ const ( arnIAMPrefix = "arn:aws:iam::" s3ResourcePrefix = "arn:aws:s3:::" s3ActionPrefix = "s3:" + iamActionPrefix = "iam:" ) var ( @@ -277,19 +278,16 @@ func parseActionAsS3Action(action string) (string, error) { return Wildcard, nil } - if !strings.HasPrefix(action, s3ActionPrefix) { + if !strings.HasPrefix(action, s3ActionPrefix) && !strings.HasPrefix(action, iamActionPrefix) { return "", ErrInvalidActionFormat } - // iam arn format :s3: - s3Action := strings.TrimPrefix(action, s3ActionPrefix) - - index := strings.IndexByte(s3Action, Wildcard[0]) - if index != -1 && index != utf8.RuneCountInString(s3Action)-1 { + index := strings.IndexByte(action, Wildcard[0]) + if index != -1 && index != utf8.RuneCountInString(action)-1 { return "", ErrInvalidActionFormat } - return s3Action, nil + return action, nil } func splitGroupedConditions(groupedConditions []GroupedConditions) [][]chain.Condition { diff --git a/iam/converter_native.go b/iam/converter_native.go index 7ddcd99..8e6c4a1 100644 --- a/iam/converter_native.go +++ b/iam/converter_native.go @@ -3,6 +3,7 @@ package iam import ( "errors" "fmt" + "strings" "git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain" "git.frostfs.info/TrueCloudLab/policy-engine/schema/native" @@ -22,11 +23,11 @@ var actionToOpMap = map[string][]string{ } const ( - supportedS3ActionDeleteObject = "DeleteObject" - supportedS3ActionGetObject = "GetObject" - supportedS3ActionHeadObject = "HeadObject" - supportedS3ActionPutObject = "PutObject" - supportedS3ActionListBucket = "ListBucket" + supportedS3ActionDeleteObject = "s3:DeleteObject" + supportedS3ActionGetObject = "s3:GetObject" + supportedS3ActionHeadObject = "s3:HeadObject" + supportedS3ActionPutObject = "s3:PutObject" + supportedS3ActionListBucket = "s3:ListBucket" ) type NativeResolver interface { @@ -237,7 +238,7 @@ func formNativeActionNames(names []string) ([]string, error) { if err != nil { return nil, err } - if action == Wildcard { + if action == Wildcard || strings.TrimPrefix(action, s3ActionPrefix) == Wildcard { return []string{Wildcard}, nil } res = append(res, actionToOpMap[action]...) diff --git a/iam/converter_test.go b/iam/converter_test.go index f73106d..b5836fd 100644 --- a/iam/converter_test.go +++ b/iam/converter_test.go @@ -70,7 +70,7 @@ func TestConverters(t *testing.T) { bktName := "DOC-EXAMPLE-BUCKET" objName := "object-name" resource := bktName + "/*" - action := "PutObject" + s3action := "s3:PutObject" mockResolver := newMockUserResolver([]string{user}, []string{bktName}) @@ -95,7 +95,7 @@ func TestConverters(t *testing.T) { expected := &chain.Chain{Rules: []chain.Rule{ { Status: chain.Allow, - Actions: chain.Actions{Names: []string{action}}, + Actions: chain.Actions{Names: []string{s3action}}, Resources: chain.Resources{Names: []string{resource}}, Condition: []chain.Condition{ { @@ -135,7 +135,7 @@ func TestConverters(t *testing.T) { expected := &chain.Chain{Rules: []chain.Rule{ { Status: chain.Allow, - Actions: chain.Actions{Names: []string{action}}, + Actions: chain.Actions{Names: []string{native.MethodPutObject}}, Resources: chain.Resources{Names: []string{fmt.Sprintf(native.ResourceFormatRootContainerObjects, mockResolver.buckets[bktName])}}, Condition: []chain.Condition{ { @@ -169,7 +169,7 @@ func TestConverters(t *testing.T) { expected := &chain.Chain{Rules: []chain.Rule{ { Status: chain.AccessDenied, - Actions: chain.Actions{Inverted: true, Names: []string{action}}, + Actions: chain.Actions{Inverted: true, Names: []string{s3action}}, Resources: chain.Resources{Inverted: true, Names: []string{resource}}, Condition: []chain.Condition{ { @@ -203,7 +203,7 @@ func TestConverters(t *testing.T) { expected := &chain.Chain{Rules: []chain.Rule{ { Status: chain.AccessDenied, - Actions: chain.Actions{Inverted: true, Names: actionToOpMap["GetObject"]}, + Actions: chain.Actions{Inverted: true, Names: actionToOpMap["s3:GetObject"]}, Resources: chain.Resources{Inverted: true, Names: []string{ fmt.Sprintf(native.ResourceFormatRootContainerObjects, mockResolver.buckets[bktName]), }}, @@ -278,6 +278,49 @@ func TestConverters(t *testing.T) { _, err := ConvertToNativeChain(p, mockResolver) require.Error(t, err) }) + + t.Run("valid mixed iam/s3 actions", func(t *testing.T) { + p := Policy{ + Statement: []Statement{{ + Principal: map[PrincipalType][]string{AWSPrincipalType: {principal}}, + Effect: AllowEffect, + Action: []string{"s3:PutObject", "iam:*"}, + Resource: []string{"*"}, + }}, + } + + s3Expected := &chain.Chain{Rules: []chain.Rule{{ + Status: chain.Allow, + Actions: chain.Actions{Names: []string{"s3:PutObject", "iam:*"}}, + Resources: chain.Resources{Names: []string{"*"}}, + Condition: []chain.Condition{{ + Op: chain.CondStringEquals, + Object: chain.ObjectRequest, + Key: s3.PropertyKeyOwner, + Value: mockResolver.users[user], + }}, + }}} + + s3Chain, err := ConvertToS3Chain(p, mockResolver) + require.NoError(t, err) + require.Equal(t, s3Expected, s3Chain) + + nativeExpected := &chain.Chain{Rules: []chain.Rule{{ + Status: chain.Allow, + Actions: chain.Actions{Names: []string{native.MethodPutObject}}, + Resources: chain.Resources{Names: []string{native.ResourceFormatAllObjects}}, + Condition: []chain.Condition{{ + Op: chain.CondStringEquals, + Object: chain.ObjectRequest, + Key: native.PropertyKeyActorPublicKey, + Value: mockResolver.users[user], + }}, + }}} + + nativeChain, err := ConvertToNativeChain(p, mockResolver) + require.NoError(t, err) + require.Equal(t, nativeExpected, nativeChain) + }) } func TestConvertToChainCondition(t *testing.T) { @@ -566,7 +609,7 @@ func TestComplexNativeConditions(t *testing.T) { } expectedStatus := chain.AccessDenied - expectedActions := chain.Actions{Names: actionToOpMap[action]} + expectedActions := chain.Actions{Names: actionToOpMap["s3:"+action]} expectedResource1 := chain.Resources{Names: []string{nativeResource1}} expectedResource23 := chain.Resources{Names: []string{nativeResource2, nativeResource3}} @@ -852,7 +895,7 @@ func TestComplexS3Conditions(t *testing.T) { resource1 := bktName1 + "/" + objName1 resource2 := bktName2 + "/*" resource3 := bktName3 + "/*" - action := "PutObject" + action := "s3:PutObject" key1, key2 := "key1", "key2" val0, val1, val2 := "val0", "val1", "val2" @@ -866,7 +909,7 @@ func TestComplexS3Conditions(t *testing.T) { AWSPrincipalType: {principal1, principal2}, }, Effect: DenyEffect, - Action: []string{"s3:" + action}, + Action: []string{action}, Resource: []string{"arn:aws:s3:::" + resource1, "arn:aws:s3:::" + resource2, "arn:aws:s3:::" + resource3}, Conditions: map[string]Condition{ CondStringEquals: {key1: {val0, val1}}, @@ -876,7 +919,7 @@ func TestComplexS3Conditions(t *testing.T) { } expectedStatus := chain.AccessDenied - expectedActions := chain.Actions{Names: actionToOpMap[action]} + expectedActions := chain.Actions{Names: []string{action}} expectedResources := chain.Resources{Names: []string{resource1, resource2, resource3}} user1Condition := chain.Condition{Op: chain.CondStringEquals, Object: chain.ObjectRequest, Key: s3.PropertyKeyOwner, Value: mockResolver.users[user1]} @@ -1105,19 +1148,19 @@ func TestS3BucketResource(t *testing.T) { require.NoError(t, err) // check we match just "bucket1" resource - req := testutil.NewRequest("HeadBucket", testutil.NewResource(bktName1, nil), nil) + req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(bktName1, nil), nil) status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) require.NoError(t, err) require.Equal(t, chain.AccessDenied.String(), status.String()) // check we match just "bucket2" resource - req = testutil.NewRequest("HeadBucket", testutil.NewResource(bktName2, nil), nil) + req = testutil.NewRequest("s3:HeadBucket", testutil.NewResource(bktName2, nil), nil) status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) require.NoError(t, err) require.Equal(t, chain.Allow.String(), status.String()) // check we also match "bucket2/object" resource - req = testutil.NewRequest("PutObject", testutil.NewResource(bktName2+"/object", nil), nil) + req = testutil.NewRequest("s3:PutObject", testutil.NewResource(bktName2+"/object", nil), nil) status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) require.NoError(t, err) require.Equal(t, chain.Allow.String(), status.String()) @@ -1159,20 +1202,27 @@ func TestActionParsing(t *testing.T) { }, { action: "s3:PutObject", - expected: "PutObject", + expected: "s3:PutObject", }, { action: "s3:Put*", - expected: "Put*", + expected: "s3:Put*", }, { action: "s3:*", - expected: "*", + expected: "s3:*", }, { - action: "s3:", - - expected: "", + action: "s3:", + expected: "s3:", + }, + { + action: "iam:ListAccessKeys", + expected: "iam:ListAccessKeys", + }, + { + action: "iam:*", + expected: "iam:*", }, } { t.Run("", func(t *testing.T) {