[#36] iam: Keep s3/iam prefixes in resources

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
This commit is contained in:
Denis Kirillov 2023-12-19 10:35:14 +03:00 committed by Alexey Vanin
parent ec39d8371a
commit 3128352693
5 changed files with 130 additions and 161 deletions

View file

@ -244,50 +244,38 @@ func parsePrincipalAsIAMUser(principal string) (account string, user string, err
return account, user, nil return account, user, nil
} }
func parseResourceAsS3ARN(resource string) (bucket string, object string, err error) { func validateResource(resource string) error {
if resource == Wildcard { if resource == Wildcard {
return Wildcard, Wildcard, nil return nil
} }
if !strings.HasPrefix(resource, s3ResourcePrefix) { if !strings.HasPrefix(resource, s3ResourcePrefix) && !strings.HasPrefix(resource, arnIAMPrefix) {
return "", "", ErrInvalidResourceFormat return ErrInvalidResourceFormat
} }
// iam arn format arn:aws:s3:::<bucket-name>/<object-name> index := strings.IndexByte(resource, Wildcard[0])
s3Resource := strings.TrimPrefix(resource, s3ResourcePrefix) if index != -1 && index != utf8.RuneCountInString(resource)-1 {
sepIndex := strings.Index(s3Resource, "/") return ErrInvalidResourceFormat
if sepIndex < 0 {
return s3Resource, "", nil
} }
bucket = s3Resource[:sepIndex] return nil
object = s3Resource[sepIndex+1:]
if len(object) == 0 {
return bucket, Wildcard, nil
}
if bucket == Wildcard && object != Wildcard {
return "", "", ErrInvalidResourceFormat
}
return bucket, object, nil
} }
func parseActionAsS3Action(action string) (string, error) { func validateAction(action string) error {
if action == Wildcard { if action == Wildcard {
return Wildcard, nil return nil
} }
if !strings.HasPrefix(action, s3ActionPrefix) && !strings.HasPrefix(action, iamActionPrefix) { if !strings.HasPrefix(action, s3ActionPrefix) && !strings.HasPrefix(action, iamActionPrefix) {
return "", ErrInvalidActionFormat return ErrInvalidActionFormat
} }
index := strings.IndexByte(action, Wildcard[0]) index := strings.IndexByte(action, Wildcard[0])
if index != -1 && index != utf8.RuneCountInString(action)-1 { if index != -1 && index != utf8.RuneCountInString(action)-1 {
return "", ErrInvalidActionFormat return ErrInvalidActionFormat
} }
return action, nil return nil
} }
func splitGroupedConditions(groupedConditions []GroupedConditions) [][]chain.Condition { func splitGroupedConditions(groupedConditions []GroupedConditions) [][]chain.Condition {

View file

@ -161,30 +161,50 @@ func formNativeResourceNamesAndConditions(names []string, resolver NativeResolve
var combined []string var combined []string
for i := range names { for _, resource := range names {
bkt, obj, err := parseResourceAsS3ARN(names[i]) if err := validateResource(resource); err != nil {
if err != nil {
return nil, err return nil, err
} }
if bkt == Wildcard { if resource == Wildcard {
res = res[:0] res = res[:0]
return append(res, GroupedResources{Names: []string{native.ResourceFormatAllObjects}}), nil return append(res, GroupedResources{Names: []string{native.ResourceFormatAllObjects}}), nil
} }
if !strings.HasPrefix(resource, s3ResourcePrefix) {
continue
}
var bkt, obj string
s3Resource := strings.TrimPrefix(resource, s3ResourcePrefix)
if s3Resource == Wildcard {
res = res[:0]
return append(res, GroupedResources{Names: []string{native.ResourceFormatAllObjects}}), nil
}
if sepIndex := strings.Index(s3Resource, "/"); sepIndex < 0 {
bkt = s3Resource
} else {
bkt = s3Resource[:sepIndex]
obj = s3Resource[sepIndex+1:]
if len(obj) == 0 {
obj = Wildcard
}
}
cnrID, err := resolver.GetBucketCID(bkt) cnrID, err := resolver.GetBucketCID(bkt)
if err != nil { if err != nil {
return nil, err return nil, err
} }
resource := fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID) nativeResource := fmt.Sprintf(native.ResourceFormatRootContainerObjects, cnrID)
if obj == Wildcard || obj == "" { if obj == Wildcard || obj == "" {
combined = append(combined, resource) combined = append(combined, nativeResource)
continue continue
} }
res = append(res, GroupedResources{ res = append(res, GroupedResources{
Names: []string{resource}, Names: []string{nativeResource},
Conditions: []chain.Condition{ Conditions: []chain.Condition{
{ {
Op: chain.CondStringLike, Op: chain.CondStringLike,
@ -233,14 +253,23 @@ func formPrincipalKey(principal string, resolver NativeResolver) (string, error)
func formNativeActionNames(names []string) ([]string, error) { func formNativeActionNames(names []string) ([]string, error) {
res := make([]string, 0, len(names)) res := make([]string, 0, len(names))
for i := range names { for _, action := range names {
action, err := parseActionAsS3Action(names[i]) if err := validateAction(action); err != nil {
if err != nil {
return nil, err return nil, err
} }
if action == Wildcard || strings.TrimPrefix(action, s3ActionPrefix) == Wildcard {
if action == Wildcard {
return []string{Wildcard}, nil return []string{Wildcard}, nil
} }
if !strings.HasPrefix(action, s3ActionPrefix) {
continue
}
if strings.TrimPrefix(action, s3ActionPrefix) == Wildcard {
return []string{Wildcard}, nil
}
res = append(res, actionToOpMap[action]...) res = append(res, actionToOpMap[action]...)
} }

View file

@ -21,19 +21,17 @@ func ConvertToS3Chain(p Policy, resolver S3Resolver) (*chain.Chain, error) {
for _, statement := range p.Statement { for _, statement := range p.Statement {
status := formStatus(statement) status := formStatus(statement)
action, actionInverted := statement.action() actions, actionInverted := statement.action()
s3Actions, err := formS3ActionNames(action) if err := validateS3ActionNames(actions); err != nil {
if err != nil {
return nil, err return nil, err
} }
ruleAction := chain.Actions{Inverted: actionInverted, Names: s3Actions} ruleAction := chain.Actions{Inverted: actionInverted, Names: actions}
resource, resourceInverted := statement.resource() resources, resourceInverted := statement.resource()
s3Resources, err := formS3ResourceNames(resource) if err := validateS3ResourceNames(resources); err != nil {
if err != nil {
return nil, err return nil, err
} }
ruleResource := chain.Resources{Inverted: resourceInverted, Names: s3Resources} ruleResource := chain.Resources{Inverted: resourceInverted, Names: resources}
groupedConditions, err := convertToS3ChainCondition(statement.Conditions, resolver) groupedConditions, err := convertToS3ChainCondition(statement.Conditions, resolver)
if err != nil { if err != nil {
@ -141,33 +139,22 @@ func formPrincipalOwner(principal string, resolver S3Resolver) (string, error) {
return address, nil return address, nil
} }
func formS3ResourceNames(names []string) ([]string, error) { func validateS3ResourceNames(names []string) error {
res := make([]string, len(names))
for i := range names { for i := range names {
bkt, obj, err := parseResourceAsS3ARN(names[i]) if err := validateResource(names[i]); err != nil {
if err != nil { return err
return nil, err }
} }
if bkt == Wildcard || obj == "" { return nil
res[i] = bkt
continue
}
res[i] = bkt + "/" + obj
}
return res, nil
} }
func formS3ActionNames(names []string) ([]string, error) { func validateS3ActionNames(names []string) error {
var err error
res := make([]string, len(names))
for i := range names { for i := range names {
if res[i], err = parseActionAsS3Action(names[i]); err != nil { if err := validateAction(names[i]); err != nil {
return nil, err return err
} }
} }
return res, nil return nil
} }

View file

@ -69,7 +69,7 @@ func TestConverters(t *testing.T) {
principal := "arn:aws:iam::" + namespace + ":user/" + userName principal := "arn:aws:iam::" + namespace + ":user/" + userName
bktName := "DOC-EXAMPLE-BUCKET" bktName := "DOC-EXAMPLE-BUCKET"
objName := "object-name" objName := "object-name"
resource := bktName + "/*" resource := fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName)
s3action := "s3:PutObject" s3action := "s3:PutObject"
mockResolver := newMockUserResolver([]string{user}, []string{bktName}) mockResolver := newMockUserResolver([]string{user}, []string{bktName})
@ -83,7 +83,7 @@ func TestConverters(t *testing.T) {
}, },
Effect: AllowEffect, Effect: AllowEffect,
Action: []string{"s3:PutObject"}, Action: []string{"s3:PutObject"},
Resource: []string{"arn:aws:s3:::" + resource}, Resource: []string{resource},
Conditions: map[string]Condition{ Conditions: map[string]Condition{
CondStringEquals: { CondStringEquals: {
"s3:RequestObjectTag/Department": {"Finance"}, "s3:RequestObjectTag/Department": {"Finance"},
@ -128,7 +128,7 @@ func TestConverters(t *testing.T) {
}, },
Effect: AllowEffect, Effect: AllowEffect,
Action: []string{"s3:PutObject"}, Action: []string{"s3:PutObject"},
Resource: []string{"arn:aws:s3:::" + resource}, Resource: []string{resource},
}}, }},
} }
@ -162,7 +162,7 @@ func TestConverters(t *testing.T) {
}, },
Effect: DenyEffect, Effect: DenyEffect,
NotAction: []string{"s3:PutObject"}, NotAction: []string{"s3:PutObject"},
NotResource: []string{"arn:aws:s3:::" + resource}, NotResource: []string{resource},
}}, }},
} }
@ -196,7 +196,7 @@ func TestConverters(t *testing.T) {
}, },
Effect: DenyEffect, Effect: DenyEffect,
NotAction: []string{"s3:GetObject"}, NotAction: []string{"s3:GetObject"},
NotResource: []string{"arn:aws:s3:::" + bktName + "/" + objName}, NotResource: []string{fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName, objName)},
}}, }},
} }
@ -892,9 +892,9 @@ func TestComplexS3Conditions(t *testing.T) {
principal2 := "arn:aws:iam::" + namespace + ":user/" + userName2 principal2 := "arn:aws:iam::" + namespace + ":user/" + userName2
bktName1, bktName2, bktName3 := "bktName", "bktName2", "bktName3" bktName1, bktName2, bktName3 := "bktName", "bktName2", "bktName3"
objName1 := "objName1" objName1 := "objName1"
resource1 := bktName1 + "/" + objName1 resource1 := fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName1, objName1)
resource2 := bktName2 + "/*" resource2 := fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName2)
resource3 := bktName3 + "/*" resource3 := fmt.Sprintf(s3.ResourceFormatS3BucketObjects, bktName3)
action := "s3:PutObject" action := "s3:PutObject"
key1, key2 := "key1", "key2" key1, key2 := "key1", "key2"
@ -910,7 +910,7 @@ func TestComplexS3Conditions(t *testing.T) {
}, },
Effect: DenyEffect, Effect: DenyEffect,
Action: []string{action}, Action: []string{action},
Resource: []string{"arn:aws:s3:::" + resource1, "arn:aws:s3:::" + resource2, "arn:aws:s3:::" + resource3}, Resource: []string{resource1, resource2, resource3},
Conditions: map[string]Condition{ Conditions: map[string]Condition{
CondStringEquals: {key1: {val0, val1}}, CondStringEquals: {key1: {val0, val1}},
CondStringLike: {key2: {val2}}, CondStringLike: {key2: {val2}},
@ -1001,7 +1001,7 @@ func TestComplexS3Conditions(t *testing.T) {
{ {
name: "bucket resource3, all conditions matched", name: "bucket resource3, all conditions matched",
action: action, action: action,
resource: bktName3 + "/some-obj", resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName3, "some-obj"),
requestMap: map[string]string{ requestMap: map[string]string{
s3.PropertyKeyOwner: mockResolver.users[user1], s3.PropertyKeyOwner: mockResolver.users[user1],
key1: val0, key1: val0,
@ -1012,7 +1012,7 @@ func TestComplexS3Conditions(t *testing.T) {
{ {
name: "bucket resource, user condition mismatched", name: "bucket resource, user condition mismatched",
action: action, action: action,
resource: bktName2 + "/some-obj", resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "some-obj"),
requestMap: map[string]string{ requestMap: map[string]string{
key1: val0, key1: val0,
key2: val2, key2: val2,
@ -1022,7 +1022,7 @@ func TestComplexS3Conditions(t *testing.T) {
{ {
name: "bucket resource, key2 condition mismatched", name: "bucket resource, key2 condition mismatched",
action: action, action: action,
resource: bktName3 + "/some-obj", resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName3, "some-obj"),
requestMap: map[string]string{ requestMap: map[string]string{
s3.PropertyKeyOwner: mockResolver.users[user1], s3.PropertyKeyOwner: mockResolver.users[user1],
key1: val0, key1: val0,
@ -1033,7 +1033,7 @@ func TestComplexS3Conditions(t *testing.T) {
{ {
name: "bucket resource, key1 condition mismatched", name: "bucket resource, key1 condition mismatched",
action: action, action: action,
resource: bktName2 + "/some-obj", resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "some-obj"),
requestMap: map[string]string{ requestMap: map[string]string{
s3.PropertyKeyOwner: mockResolver.users[user1], s3.PropertyKeyOwner: mockResolver.users[user1],
key2: val2, key2: val2,
@ -1054,7 +1054,7 @@ func TestComplexS3Conditions(t *testing.T) {
{ {
name: "bucket/object resource, resource mismatched", name: "bucket/object resource, resource mismatched",
action: action, action: action,
resource: bktName1 + "/some-obj", resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName1, "some-obj"),
requestMap: map[string]string{ requestMap: map[string]string{
s3.PropertyKeyOwner: mockResolver.users[user1], s3.PropertyKeyOwner: mockResolver.users[user1],
key1: val0, key1: val0,
@ -1097,7 +1097,7 @@ func TestComplexS3Conditions(t *testing.T) {
{ {
name: "resource mismatched", name: "resource mismatched",
action: action, action: action,
resource: "some-bkt/some-obj", resource: fmt.Sprintf(s3.ResourceFormatS3BucketObject, "some-bkt", "some-obj"),
requestMap: map[string]string{ requestMap: map[string]string{
s3.PropertyKeyOwner: mockResolver.users[user1], s3.PropertyKeyOwner: mockResolver.users[user1],
key1: val0, key1: val0,
@ -1120,7 +1120,7 @@ func TestS3BucketResource(t *testing.T) {
bktName1, bktName2 := "bucket1", "bucket2" bktName1, bktName2 := "bucket1", "bucket2"
chainName := chain.Name("name") chainName := chain.Name("name")
mockResolver := newMockUserResolver([]string{}, []string{bktName1}) mockResolver := newMockUserResolver([]string{}, []string{})
p := Policy{ p := Policy{
Version: "2012-10-17", Version: "2012-10-17",
@ -1129,13 +1129,13 @@ func TestS3BucketResource(t *testing.T) {
Principal: map[PrincipalType][]string{Wildcard: nil}, Principal: map[PrincipalType][]string{Wildcard: nil},
Effect: DenyEffect, Effect: DenyEffect,
Action: []string{"s3:HeadBucket"}, Action: []string{"s3:HeadBucket"},
Resource: []string{"arn:aws:s3:::" + bktName1}, Resource: []string{fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName1)},
}, },
{ {
Principal: map[PrincipalType][]string{Wildcard: nil}, Principal: map[PrincipalType][]string{Wildcard: nil},
Effect: AllowEffect, Effect: AllowEffect,
Action: []string{"*"}, Action: []string{"*"},
Resource: []string{"arn:aws:s3:::*"}, Resource: []string{s3.ResourceFormatS3All},
}, },
}, },
} }
@ -1148,19 +1148,19 @@ func TestS3BucketResource(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
// check we match just "bucket1" resource // check we match just "bucket1" resource
req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(bktName1, nil), nil) req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName1), nil), nil)
status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, chain.AccessDenied.String(), status.String()) require.Equal(t, chain.AccessDenied.String(), status.String())
// check we match just "bucket2" resource // check we match just "bucket2" resource
req = testutil.NewRequest("s3:HeadBucket", testutil.NewResource(bktName2, nil), nil) req = testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName2), nil), nil)
status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, chain.Allow.String(), status.String()) require.Equal(t, chain.Allow.String(), status.String())
// check we also match "bucket2/object" resource // check we also match "bucket2/object" resource
req = testutil.NewRequest("s3:PutObject", testutil.NewResource(bktName2+"/object", nil), nil) req = testutil.NewRequest("s3:PutObject", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "object"), nil), nil)
status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req) status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, chain.Allow.String(), status.String()) require.Equal(t, chain.Allow.String(), status.String())
@ -1183,57 +1183,45 @@ func TestWildcardConverters(t *testing.T) {
func TestActionParsing(t *testing.T) { func TestActionParsing(t *testing.T) {
for _, tc := range []struct { for _, tc := range []struct {
action string action string
expected string
err bool err bool
}{ }{
{ {
action: "withoutPrefix", action: "withoutPrefix",
expected: "",
err: true, err: true,
}, },
{ {
action: "s3:*Object", action: "s3:*Object",
expected: "",
err: true, err: true,
}, },
{ {
action: "*", action: "*",
expected: "*",
}, },
{ {
action: "s3:PutObject", action: "s3:PutObject",
expected: "s3:PutObject",
}, },
{ {
action: "s3:Put*", action: "s3:Put*",
expected: "s3:Put*",
}, },
{ {
action: "s3:*", action: "s3:*",
expected: "s3:*",
}, },
{ {
action: "s3:", action: "s3:",
expected: "s3:",
}, },
{ {
action: "iam:ListAccessKeys", action: "iam:ListAccessKeys",
expected: "iam:ListAccessKeys",
}, },
{ {
action: "iam:*", action: "iam:*",
expected: "iam:*",
}, },
} { } {
t.Run("", func(t *testing.T) { t.Run("", func(t *testing.T) {
actual, err := parseActionAsS3Action(tc.action) err := validateAction(tc.action)
if tc.err { if tc.err {
require.Error(t, err) require.Error(t, err)
return } else {
}
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, tc.expected, actual) }
}) })
} }
} }
@ -1304,54 +1292,23 @@ func TestPrincipalParsing(t *testing.T) {
func TestResourceParsing(t *testing.T) { func TestResourceParsing(t *testing.T) {
for _, tc := range []struct { for _, tc := range []struct {
resource string resource string
expectedBucket string
expectedObject string
err bool err bool
}{ }{
{ {resource: "withoutPrefixAnd", err: true},
resource: "withoutPrefixAnd", {resource: "arn:aws:s3:::*/obj", err: true},
err: true, {resource: "arn:aws:s3:::bkt/*"},
}, {resource: "arn:aws:s3:::bkt"},
{ {resource: "arn:aws:s3:::bkt/"},
resource: "arn:aws:s3:::*/obj", {resource: "arn:aws:s3:::*"},
err: true, {resource: "*"},
},
{
resource: "arn:aws:s3:::bkt/*",
expectedBucket: "bkt",
expectedObject: "*",
},
{
resource: "arn:aws:s3:::bkt",
expectedBucket: "bkt",
expectedObject: "",
},
{
resource: "arn:aws:s3:::bkt/",
expectedBucket: "bkt",
expectedObject: "*",
},
{
resource: "arn:aws:s3:::*",
expectedBucket: "*",
expectedObject: "",
},
{
resource: "*",
expectedBucket: "*",
expectedObject: "*",
},
} { } {
t.Run("", func(t *testing.T) { t.Run("", func(t *testing.T) {
bkt, obj, err := parseResourceAsS3ARN(tc.resource) err := validateResource(tc.resource)
if tc.err { if tc.err {
require.Error(t, err) require.Error(t, err)
return } else {
}
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, tc.expectedBucket, bkt) }
require.Equal(t, tc.expectedObject, obj)
}) })
} }
} }

View file

@ -6,4 +6,12 @@ const (
PropertyKeyDelimiter = "s3:delimiter" PropertyKeyDelimiter = "s3:delimiter"
PropertyKeyPrefix = "s3:prefix" PropertyKeyPrefix = "s3:prefix"
PropertyKeyVersionID = "s3:VersionId" PropertyKeyVersionID = "s3:VersionId"
ResourceFormatS3All = "arn:aws:s3:::*"
ResourceFormatS3Bucket = "arn:aws:s3:::%s"
ResourceFormatS3BucketObjects = "arn:aws:s3:::%s/*"
ResourceFormatS3BucketObject = "arn:aws:s3:::%s/%s"
ResourceFormatIAMNamespaceUser = "arn:aws:iam::%s:user/%s"
ResourceFormatIAMNamespaceGroup = "arn:aws:iam::%s:group/%s"
) )