feature/11-support_notprincipal #12

Merged
dkirillov merged 1 commit from dkirillov/policy-engine:feature/11-support_notprincipal into master 2024-09-04 19:51:23 +00:00
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
dstepanov-yadro marked this conversation as resolved Outdated

I didn't catch the thought. Please explain.

I didn't catch the thought. Please explain.

Only Principal can be *:

{
	"Statement": {
		"Principal": "*",
	}
}

The following json is invalid according to spec

{
	"Statement": {
		"NotPrincipal": "*",
	}
}
Only `Principal` can be `*`: ```json { "Statement": { "Principal": "*", } } ``` The following json is invalid according to [spec](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_notprincipal.html) ```json { "Statement": { "NotPrincipal": "*", } } ```
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 {
aarifullin marked this conversation as resolved Outdated

It is good that you intend to check mutual exclusion of these fields and this semantically correct but:

  1. I would insist on statement.Resource != nil check instead length checking. If these types weren't be slices or maps you would define a pointer to a type and use check for nil-ness
  2. Is it right that "one of" must surely defined? Are there situtation when neither "Resource" nor "NotResource" are defined?
It is good that you intend to check mutual exclusion of these fields and this semantically correct but: 1. I would insist on `statement.Resource != nil` check instead length checking. If these types weren't be slices or maps you would define a pointer to a type and use check for `nil`-ness 2. Is it right that "one of" must surely defined? Are there situtation when neither "Resource" nor "NotResource" are defined?
  1. Ok, I'll fix this
  2. Yes. No, there isn't such situation
1. Ok, I'll fix this 2. Yes. No, there isn't such situation
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)
})
}