From 5a9507933ff8436b2eada2fb91303a6f2250c71f Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Mon, 11 Dec 2023 14:31:41 +0300 Subject: [PATCH 1/2] [#28] chain: Add S3 chain name Signed-off-by: Denis Kirillov --- pkg/chain/chain_names.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/chain/chain_names.go b/pkg/chain/chain_names.go index 8bb88c0..3e9ea5a 100644 --- a/pkg/chain/chain_names.go +++ b/pkg/chain/chain_names.go @@ -7,4 +7,7 @@ const ( // Ingress represents chains applied when crossing user/storage network boundary. // It is not applied when talking between nodes. Ingress Name = "ingress" + + // S3 represents chains applied when crossing user/s3 network boundary. + S3 Name = "s3" ) -- 2.45.2 From 85a462d23eb73df524e2595c85f94acafc12e18a Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Mon, 11 Dec 2023 14:43:47 +0300 Subject: [PATCH 2/2] [#28] iam: Fix converters Handle resource without object as bucket name instead of bucket with any object Signed-off-by: Denis Kirillov --- iam/converter.go | 2 +- iam/converter_native.go | 2 +- iam/converter_s3.go | 2 +- iam/converter_test.go | 55 +++++++++++++++++++++++++++++++++++++++-- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/iam/converter.go b/iam/converter.go index b288de3..4749526 100644 --- a/iam/converter.go +++ b/iam/converter.go @@ -256,7 +256,7 @@ func parseResourceAsS3ARN(resource string) (bucket string, object string, err er s3Resource := strings.TrimPrefix(resource, s3ResourcePrefix) sepIndex := strings.Index(s3Resource, "/") if sepIndex < 0 { - return s3Resource, Wildcard, nil + return s3Resource, "", nil } bucket = s3Resource[:sepIndex] diff --git a/iam/converter_native.go b/iam/converter_native.go index 4abfcdf..7ddcd99 100644 --- a/iam/converter_native.go +++ b/iam/converter_native.go @@ -177,7 +177,7 @@ func formNativeResourceNamesAndConditions(names []string, resolver NativeResolve } resource := fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID) - if obj == Wildcard { + if obj == Wildcard || obj == "" { combined = append(combined, resource) continue } diff --git a/iam/converter_s3.go b/iam/converter_s3.go index 094c3d3..5380097 100644 --- a/iam/converter_s3.go +++ b/iam/converter_s3.go @@ -149,7 +149,7 @@ func formS3ResourceNames(names []string) ([]string, error) { return nil, err } - if bkt == Wildcard { + if bkt == Wildcard || obj == "" { res[i] = bkt continue } diff --git a/iam/converter_test.go b/iam/converter_test.go index 7ec177d..f73106d 100644 --- a/iam/converter_test.go +++ b/iam/converter_test.go @@ -1072,6 +1072,57 @@ func TestComplexS3Conditions(t *testing.T) { } } +func TestS3BucketResource(t *testing.T) { + namespace := "ns" + bktName1, bktName2 := "bucket1", "bucket2" + chainName := chain.Name("name") + + mockResolver := newMockUserResolver([]string{}, []string{bktName1}) + + p := Policy{ + Version: "2012-10-17", + Statement: []Statement{ + { + Principal: map[PrincipalType][]string{Wildcard: nil}, + Effect: DenyEffect, + Action: []string{"s3:HeadBucket"}, + Resource: []string{"arn:aws:s3:::" + bktName1}, + }, + { + Principal: map[PrincipalType][]string{Wildcard: nil}, + Effect: AllowEffect, + Action: []string{"*"}, + Resource: []string{"arn:aws:s3:::*"}, + }, + }, + } + + s3Chain, err := ConvertToS3Chain(p, mockResolver) + require.NoError(t, err) + + s := inmemory.NewInMemory() + _, _, err = s.MorphRuleChainStorage().AddMorphRuleChain(chainName, engine.NamespaceTarget(namespace), s3Chain) + require.NoError(t, err) + + // check we match just "bucket1" resource + req := testutil.NewRequest("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) + 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) + status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) + require.NoError(t, err) + require.Equal(t, chain.Allow.String(), status.String()) +} + func TestWildcardConverters(t *testing.T) { policy := `{"Version":"2012-10-17","Statement":{"Effect":"Allow", "Principal": "*", "Action":"*","Resource":"*"}}` @@ -1223,7 +1274,7 @@ func TestResourceParsing(t *testing.T) { { resource: "arn:aws:s3:::bkt", expectedBucket: "bkt", - expectedObject: "*", + expectedObject: "", }, { resource: "arn:aws:s3:::bkt/", @@ -1233,7 +1284,7 @@ func TestResourceParsing(t *testing.T) { { resource: "arn:aws:s3:::*", expectedBucket: "*", - expectedObject: "*", + expectedObject: "", }, { resource: "*", -- 2.45.2