[#11] iam: Support 'NotPrincipal', 'NotAction', 'NotResource'
All checks were successful
Tests and linters / Tests (1.21) (pull_request) Successful in 1m39s
Tests and linters / Tests (1.20) (pull_request) Successful in 2m11s
DCO action / DCO (pull_request) Successful in 2m31s
Tests and linters / Staticcheck (pull_request) Successful in 2m27s
Tests and linters / Tests with -race (pull_request) Successful in 2m35s
Tests and linters / Lint (pull_request) Successful in 3m55s

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>
This commit is contained in:
Denis Kirillov 2023-10-30 16:34:53 +03:00
parent 8d291039d8
commit 63ecf63a08
5 changed files with 499 additions and 67 deletions

View file

@ -1,7 +1,6 @@
package iam
import (
"errors"
"fmt"
"strconv"
"strings"
@ -11,8 +10,6 @@ import (
)
const (
FrostFSPrincipal = "FrostFS"
RequestOwnerProperty = "Owner"
)
@ -56,6 +53,10 @@ const (
)
func (p Policy) ToChain() (*policyengine.Chain, error) {
if err := p.Validate(GeneralPolicyType); err != nil {
return nil, err
}
var chain policyengine.Chain
for _, statement := range p.Statement {
@ -64,20 +65,21 @@ func (p Policy) ToChain() (*policyengine.Chain, error) {
status = policyengine.Allow
}
if len(statement.Principal) != 1 {
return nil, errors.New("currently supported exactly one principal type")
}
var principals []string
var op policyengine.ConditionType
if _, ok := statement.Principal[Wildcard]; ok {
statementPrincipal, inverted := statement.principal()
if _, ok := statementPrincipal[Wildcard]; ok { // this can be true only if 'inverted' false
principals = []string{Wildcard}
op = policyengine.CondStringLike
} else if frostfsPrincipals, ok := statement.Principal[FrostFSPrincipal]; ok {
principals = frostfsPrincipals
op = policyengine.CondStringEquals
} else {
return nil, errors.New("currently supported only FrostFS or all (wildcard) principals")
for _, principal := range statementPrincipal {
principals = append(principals, principal...)
}
op = policyengine.CondStringEquals
if inverted {
op = policyengine.CondStringNotEquals
}
}
var conditions []policyengine.Condition
@ -96,10 +98,16 @@ func (p Policy) ToChain() (*policyengine.Chain, error) {
}
conditions = append(conditions, conds...)
action, actionInverted := statement.action()
ruleAction := policyengine.Actions{Inverted: actionInverted, Names: action}
resource, resourceInverted := statement.resource()
ruleResource := policyengine.Resources{Inverted: resourceInverted, Names: resource}
r := policyengine.Rule{
Status: status,
Action: statement.Action,
Resource: statement.Resource,
Actions: ruleAction,
Resources: ruleResource,
Any: true,
Condition: conditions,
}

View file

@ -12,8 +12,8 @@ func TestConverters(t *testing.T) {
p := Policy{
Version: "2012-10-17",
Statement: []Statement{{
Principal: map[string][]string{
FrostFSPrincipal: {"arn:aws:iam::111122223333:user/JohnDoe"},
Principal: map[PrincipalType][]string{
AWSPrincipalType: {"arn:aws:iam::111122223333:user/JohnDoe"},
},
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -28,10 +28,10 @@ func TestConverters(t *testing.T) {
expected := &policyengine.Chain{Rules: []policyengine.Rule{
{
Status: policyengine.Allow,
Action: p.Statement[0].Action,
Resource: p.Statement[0].Resource,
Any: true,
Status: policyengine.Allow,
Actions: policyengine.Actions{Names: p.Statement[0].Action},
Resources: policyengine.Resources{Names: p.Statement[0].Resource},
Any: true,
Condition: []policyengine.Condition{
{
Op: policyengine.CondStringEquals,
@ -54,12 +54,47 @@ func TestConverters(t *testing.T) {
require.Equal(t, expected, chain)
})
t.Run("valid inverted policy", func(t *testing.T) {
p := Policy{
Version: "2012-10-17",
Statement: []Statement{{
NotPrincipal: map[PrincipalType][]string{
AWSPrincipalType: {"arn:aws:iam::111122223333:user/JohnDoe"},
},
Effect: DenyEffect,
NotAction: []string{"s3:PutObject"},
NotResource: []string{"arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"},
}},
}
expected := &policyengine.Chain{Rules: []policyengine.Rule{
{
Status: policyengine.AccessDenied,
Actions: policyengine.Actions{Inverted: true, Names: p.Statement[0].NotAction},
Resources: policyengine.Resources{Inverted: true, Names: p.Statement[0].NotResource},
Any: true,
Condition: []policyengine.Condition{
{
Op: policyengine.CondStringNotEquals,
Object: policyengine.ObjectRequest,
Key: RequestOwnerProperty,
Value: "arn:aws:iam::111122223333:user/JohnDoe",
},
},
},
}}
chain, err := p.ToChain()
require.NoError(t, err)
require.Equal(t, expected, chain)
})
t.Run("invalid policy (unsupported principal type)", func(t *testing.T) {
p := Policy{
Version: "2012-10-17",
Statement: []Statement{{
Principal: map[string][]string{
"AWS": {"arn:aws:iam::111122223333:user/JohnDoe"},
Principal: map[PrincipalType][]string{
"dummy": {"arn:aws:iam::111122223333:user/JohnDoe"},
},
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -71,14 +106,15 @@ func TestConverters(t *testing.T) {
require.Error(t, err)
})
t.Run("invalid policy (missing principal)", func(t *testing.T) {
t.Run("invalid policy (missing resource)", func(t *testing.T) {
p := Policy{
Version: "2012-10-17",
Statement: []Statement{{
Principal: map[string][]string{},
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{"arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"},
Principal: map[PrincipalType][]string{
AWSPrincipalType: {"arn:aws:iam::111122223333:user/JohnDoe"},
},
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
}},
}
@ -90,7 +126,7 @@ func TestConverters(t *testing.T) {
p := Policy{
Version: "2012-10-17",
Statement: []Statement{{
Principal: map[string][]string{"*": nil},
Principal: map[PrincipalType][]string{Wildcard: nil},
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{"arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"},
@ -120,10 +156,10 @@ func TestConverters(t *testing.T) {
expected := &policyengine.Chain{Rules: []policyengine.Rule{
{
Status: policyengine.Allow,
Action: p.Statement[0].Action,
Resource: p.Statement[0].Resource,
Any: true,
Status: policyengine.Allow,
Actions: policyengine.Actions{Names: p.Statement[0].Action},
Resources: policyengine.Resources{Names: p.Statement[0].Resource},
Any: true,
Condition: []policyengine.Condition{
{
Op: policyengine.CondStringLike,
@ -260,9 +296,9 @@ func TestConverters(t *testing.T) {
for i, rule := range chain.Rules {
expectedRule := expected.Rules[i]
require.Equal(t, expectedRule.Action, rule.Action)
require.Equal(t, expectedRule.Actions, rule.Actions)
require.Equal(t, expectedRule.Any, rule.Any)
require.Equal(t, expectedRule.Resource, rule.Resource)
require.Equal(t, expectedRule.Resources, rule.Resources)
require.Equal(t, expectedRule.Status, rule.Status)
require.ElementsMatch(t, expectedRule.Condition, rule.Condition)
}

View file

@ -3,11 +3,11 @@ package iam
import (
"encoding/json"
"errors"
"fmt"
)
type (
// Policy grammar https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_grammar.html
// Currently 'NotPrincipal', 'NotAction' and 'NotResource' are not supported (so cannot be unmarshalled).
Policy struct {
Version string `json:"Version,omitempty"`
ID string `json:"Id,omitempty"`
@ -17,15 +17,19 @@ type (
Statements []Statement
Statement struct {
SID string `json:"Sid,omitempty"`
Principal Principal `json:"Principal,omitempty"`
Effect Effect `json:"Effect"`
Action Action `json:"Action"`
Resource Resource `json:"Resource"`
Conditions Conditions `json:"Condition,omitempty"`
ID string `json:"Id,omitempty"`
SID string `json:"Sid,omitempty"`
Principal Principal `json:"Principal,omitempty"`
NotPrincipal Principal `json:"NotPrincipal,omitempty"`
Effect Effect `json:"Effect"`
Action Action `json:"Action,omitempty"`
NotAction Action `json:"NotAction,omitempty"`
Resource Resource `json:"Resource,omitempty"`
NotResource Resource `json:"NotResource,omitempty"`
Conditions Conditions `json:"Condition,omitempty"`
}
Principal map[string][]string
Principal map[PrincipalType][]string
Effect string
@ -36,6 +40,16 @@ type (
Conditions map[string]Condition
Condition map[string][]string
PolicyType int
PrincipalType string
)
const (
GeneralPolicyType PolicyType = iota
IdentityBasedPolicyType
ResourceBasedPolicyType
)
const Wildcard = "*"
@ -45,6 +59,22 @@ const (
DenyEffect Effect = "Deny"
)
func (e Effect) IsValid() bool {
return e == AllowEffect || e == DenyEffect
}
const (
AWSPrincipalType PrincipalType = "AWS"
FederatedPrincipalType PrincipalType = "Federated"
ServicePrincipalType PrincipalType = "Service"
CanonicalUserPrincipalType PrincipalType = "CanonicalUser"
)
func (p PrincipalType) IsValid() bool {
return p == AWSPrincipalType || p == FederatedPrincipalType ||
p == ServicePrincipalType || p == CanonicalUserPrincipalType
}
func (s *Statements) UnmarshalJSON(data []byte) error {
var list []Statement
if err := json.Unmarshal(data, &list); err == nil {
@ -75,7 +105,7 @@ func (p *Principal) UnmarshalJSON(data []byte) error {
return nil
}
m := make(map[string]interface{})
m := make(map[PrincipalType]any)
if err := json.Unmarshal(data, &m); err != nil {
return err
}
@ -87,7 +117,7 @@ func (p *Principal) UnmarshalJSON(data []byte) error {
continue
}
list, ok := val.([]interface{})
list, ok := val.([]any)
if !ok {
return errors.New("invalid principal format")
}
@ -144,7 +174,7 @@ func (r *Resource) UnmarshalJSON(data []byte) error {
func (c *Condition) UnmarshalJSON(data []byte) error {
*c = make(Condition)
m := make(map[string]interface{})
m := make(map[string]any)
if err := json.Unmarshal(data, &m); err != nil {
return err
}
@ -156,7 +186,7 @@ func (c *Condition) UnmarshalJSON(data []byte) error {
continue
}
list, ok := val.([]interface{})
list, ok := val.([]any)
if !ok {
return errors.New("invalid principal format")
}
@ -175,3 +205,110 @@ func (c *Condition) UnmarshalJSON(data []byte) error {
return nil
}
func (p Policy) Validate(typ PolicyType) error {
if err := p.validate(); err != nil {
return err
}
switch typ {
case IdentityBasedPolicyType:
return p.validateIdentityBased()
case ResourceBasedPolicyType:
return p.validateResourceBased()
default:
return nil
}
}
func (p Policy) validate() error {
for _, statement := range p.Statement {
if !statement.Effect.IsValid() {
return fmt.Errorf("unknown effect: '%s'", statement.Effect)
}
if len(statement.Action) != 0 && len(statement.NotAction) != 0 {
return errors.New("'Actions' and 'NotAction' are mutually exclusive")
}
if statement.Resource != nil && statement.NotResource != nil {
return errors.New("'Resources' and 'NotResource' are mutually exclusive")
}
if len(statement.Resource) == 0 && len(statement.NotResource) == 0 {
return errors.New("one of 'Resources'/'NotResource' must be provided")
}
if len(statement.Principal) != 0 && len(statement.NotPrincipal) != 0 {
return errors.New("'Principal' and 'NotPrincipal' are mutually exclusive")
}
if len(statement.NotPrincipal) != 0 && statement.Effect != DenyEffect {
return errors.New("using 'NotPrincipal' with effect 'Allow' is not supported")
}
principal, _ := statement.principal()
if err := principal.validate(); err != nil {
return err
}
}
return nil
}
func (p Policy) validateIdentityBased() error {
if len(p.ID) != 0 {
return errors.New("'Id' is not allowed for identity-based policy")
}
for _, statement := range p.Statement {
if len(statement.Principal) != 0 || len(statement.NotPrincipal) != 0 {
return errors.New("'Principal' and 'NotPrincipal' are not allowed for identity-based policy")
}
}
return nil
}
func (p Policy) validateResourceBased() error {
for _, statement := range p.Statement {
if len(statement.Principal) == 0 && len(statement.NotPrincipal) == 0 {
return errors.New("'Principal' or 'NotPrincipal' must be provided for resource-based policy")
}
}
return nil
}
func (s Statement) principal() (Principal, bool) {
if len(s.NotPrincipal) != 0 {
return s.NotPrincipal, true
}
return s.Principal, false
}
func (s Statement) action() (Action, bool) {
if len(s.NotAction) != 0 {
return s.NotAction, true
}
return s.Action, false
}
func (s Statement) resource() (Resource, bool) {
if len(s.NotResource) != 0 {
return s.NotResource, true
}
return s.Resource, false
}
func (p Principal) validate() error {
if _, ok := p[Wildcard]; ok && len(p) == 1 {
return nil
}
for key := range p {
if !key.IsValid() {
return fmt.Errorf("unknown principal type: '%s'", key)
}
}
return nil
}

View file

@ -31,7 +31,7 @@ func TestUnmarshalIAMPolicy(t *testing.T) {
ID: "PutObjPolicy",
Statement: []Statement{{
SID: "DenyObjectsThatAreNotSSEKMS",
Principal: map[string][]string{
Principal: map[PrincipalType][]string{
"*": nil,
},
Effect: DenyEffect,
@ -78,8 +78,8 @@ func TestUnmarshalIAMPolicy(t *testing.T) {
expected := Policy{
Version: "2012-10-17",
Statement: []Statement{{
Principal: map[string][]string{
"AWS": {"arn:aws:iam::111122223333:user/JohnDoe"},
Principal: map[PrincipalType][]string{
AWSPrincipalType: {"arn:aws:iam::111122223333:user/JohnDoe"},
},
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
@ -113,8 +113,8 @@ func TestUnmarshalIAMPolicy(t *testing.T) {
expected := Policy{
Statement: []Statement{{
Principal: map[string][]string{
"AWS": {"arn:aws:iam::111122223333:user/JohnDoe"},
Principal: map[PrincipalType][]string{
AWSPrincipalType: {"arn:aws:iam::111122223333:user/JohnDoe"},
},
}},
}
@ -170,4 +170,255 @@ func TestUnmarshalIAMPolicy(t *testing.T) {
require.NoError(t, err)
require.Equal(t, expected, p)
})
t.Run("'Not*' fields", func(t *testing.T) {
policy := `
{
"Id": "PutObjPolicy",
"Statement": [{
"NotPrincipal": {"AWS":["arn:aws:iam::111122223333:user/Alice"]},
"Effect": "Deny",
"NotAction": "s3:PutObject",
"NotResource": "arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"
}]
}`
expected := Policy{
ID: "PutObjPolicy",
Statement: []Statement{{
NotPrincipal: map[PrincipalType][]string{
AWSPrincipalType: {"arn:aws:iam::111122223333:user/Alice"},
},
Effect: DenyEffect,
NotAction: []string{"s3:PutObject"},
NotResource: []string{"arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"},
}},
}
var p Policy
err := json.Unmarshal([]byte(policy), &p)
require.NoError(t, err)
require.Equal(t, expected, p)
})
}
func TestValidatePolicies(t *testing.T) {
for _, tc := range []struct {
name string
policy Policy
typ PolicyType
isValid bool
}{
{
name: "valid permission boundaries",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
Resource: []string{Wildcard},
}},
},
typ: GeneralPolicyType,
isValid: true,
},
{
name: "general invalid effect",
policy: Policy{
Statement: []Statement{{
Effect: "dummy",
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
Resource: []string{Wildcard},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "general invalid principal block",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
Resource: []string{Wildcard},
Principal: map[PrincipalType][]string{Wildcard: nil},
NotPrincipal: map[PrincipalType][]string{Wildcard: nil},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "general invalid not principal",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
Resource: []string{Wildcard},
NotPrincipal: map[PrincipalType][]string{AWSPrincipalType: {"arn:aws:iam::111122223333:user/Alice"}},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "general invalid principal type",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
Resource: []string{Wildcard},
NotPrincipal: map[PrincipalType][]string{"dummy": {"arn:aws:iam::111122223333:user/Alice"}},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "general invalid action block",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:*", "cloudwatch:*", "ec2:*"},
NotAction: []string{"iam:*"},
Resource: []string{Wildcard},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "general invalid resource block",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Resource: []string{Wildcard},
NotResource: []string{"arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "invalid resource block",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Resource: []string{},
NotResource: []string{"arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"},
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "missing resource block",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
}},
},
typ: GeneralPolicyType,
isValid: false,
},
{
name: "identity based valid",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
}},
},
typ: IdentityBasedPolicyType,
isValid: true,
},
{
name: "identity based invalid because of id presence",
policy: Policy{
ID: "some-id",
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
}},
},
typ: IdentityBasedPolicyType,
isValid: false,
},
{
name: "identity based invalid because of principal presence",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
Principal: map[PrincipalType][]string{AWSPrincipalType: {"arn:aws:iam::111122223333:user/Alice"}},
}},
},
typ: IdentityBasedPolicyType,
isValid: false,
},
{
name: "identity based invalid because of not principal presence",
policy: Policy{
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
NotPrincipal: map[PrincipalType][]string{AWSPrincipalType: {"arn:aws:iam::111122223333:user/Alice"}},
}},
},
typ: IdentityBasedPolicyType,
isValid: false,
},
{
name: "resource based valid principal",
policy: Policy{
Statement: []Statement{{
Effect: DenyEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
Principal: map[PrincipalType][]string{AWSPrincipalType: {"arn:aws:iam::111122223333:user/Alice"}},
}},
},
typ: ResourceBasedPolicyType,
isValid: true,
},
{
name: "resource based valid not principal",
policy: Policy{
ID: "some-id",
Statement: []Statement{{
Effect: DenyEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
NotPrincipal: map[PrincipalType][]string{AWSPrincipalType: {"arn:aws:iam::111122223333:user/Alice"}},
}},
},
typ: ResourceBasedPolicyType,
isValid: true,
},
{
name: "resource based invalid missing principal",
policy: Policy{
ID: "some-id",
Statement: []Statement{{
Effect: AllowEffect,
Action: []string{"s3:PutObject"},
Resource: []string{Wildcard},
}},
},
typ: ResourceBasedPolicyType,
isValid: false,
},
} {
t.Run(tc.name, func(t *testing.T) {
err := tc.policy.Validate(tc.typ)
if tc.isValid {
require.NoError(t, err)
} else {
require.Error(t, err)
}
})
}
}

View file

@ -142,6 +142,22 @@ func TestInmemory(t *testing.T) {
require.Equal(t, AccessDenied, status)
require.True(t, ok)
})
t.Run("inverted rules", func(t *testing.T) {
req := newRequest("native::object::put", newResource(object, nil), nil)
status, ok = s.IsAllowed(Ingress, namespace2, req)
require.Equal(t, NoRuleFound, status)
require.False(t, ok)
req = newRequest("native::object::put", newResource("native::object::cba/def", nil), nil)
status, ok = s.IsAllowed(Ingress, namespace2, req)
require.Equal(t, AccessDenied, status)
require.True(t, ok)
req = newRequest("native::object::get", newResource("native::object::cba/def", nil), nil)
status, ok = s.IsAllowed(Ingress, namespace2, req)
require.Equal(t, NoRuleFound, status)
require.False(t, ok)
})
t.Run("good", func(t *testing.T) {
status, ok = s.IsAllowed(Ingress, namespace, reqGood)
require.Equal(t, NoRuleFound, status)
@ -174,20 +190,4 @@ func TestInmemory(t *testing.T) {
require.True(t, ok)
})
})
t.Run("inverted rules", func(t *testing.T) {
req := newRequest("native::object::put", newResource(object, nil), nil)
status, ok = s.IsAllowed(Ingress, namespace2, req)
require.Equal(t, NoRuleFound, status)
require.False(t, ok)
req = newRequest("native::object::put", newResource("native::object::cba/def", nil), nil)
status, ok = s.IsAllowed(Ingress, namespace2, req)
require.Equal(t, AccessDenied, status)
require.True(t, ok)
req = newRequest("native::object::get", newResource("native::object::cba/def", nil), nil)
status, ok = s.IsAllowed(Ingress, namespace2, req)
require.Equal(t, NoRuleFound, status)
require.False(t, ok)
})
}