From 0a70bc24ac7086cffd0ac1f04b395ab8b09850d2 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 9 Jan 2020 19:29:57 -0800 Subject: [PATCH] Disallow only policy statements which are exactly same (#8785) --- pkg/iam/policy/actionset.go | 18 ++++++++++++++++++ pkg/iam/policy/policy.go | 8 +++----- pkg/iam/policy/resourceset.go | 18 ++++++++++++++++++ pkg/policy/actionset.go | 18 ++++++++++++++++++ pkg/policy/policy.go | 13 ++++++------- pkg/policy/principal.go | 5 +++++ pkg/policy/resourceset.go | 18 ++++++++++++++++++ 7 files changed, 86 insertions(+), 12 deletions(-) diff --git a/pkg/iam/policy/actionset.go b/pkg/iam/policy/actionset.go index a558fba07..fc09209e1 100644 --- a/pkg/iam/policy/actionset.go +++ b/pkg/iam/policy/actionset.go @@ -43,6 +43,24 @@ func (actionSet ActionSet) Match(action Action) bool { return false } +// Equals - checks whether given action set is equal to current action set or not. +func (actionSet ActionSet) Equals(sactionSet ActionSet) bool { + // If length of set is not equal to length of given set, the + // set is not equal to given set. + if len(actionSet) != len(sactionSet) { + return false + } + + // As both sets are equal in length, check each elements are equal. + for k := range actionSet { + if _, ok := sactionSet[k]; !ok { + return false + } + } + + return true +} + // Intersection - returns actions available in both ActionSet. func (actionSet ActionSet) Intersection(sset ActionSet) ActionSet { nset := NewActionSet() diff --git a/pkg/iam/policy/policy.go b/pkg/iam/policy/policy.go index a4ec9faea..9f679a1f6 100644 --- a/pkg/iam/policy/policy.go +++ b/pkg/iam/policy/policy.go @@ -110,13 +110,11 @@ func (iamp Policy) isValid() error { continue } - actions := iamp.Statements[i].Actions.Intersection(statement.Actions) - if len(actions) == 0 { + if !iamp.Statements[i].Actions.Equals(statement.Actions) { continue } - resources := iamp.Statements[i].Resources.Intersection(statement.Resources) - if len(resources) == 0 { + if !iamp.Statements[i].Resources.Equals(statement.Resources) { continue } @@ -125,7 +123,7 @@ func (iamp Policy) isValid() error { } return Errorf("duplicate actions %v, resources %v found in statements %v, %v", - actions, resources, iamp.Statements[i], statement) + statement.Actions, statement.Resources, iamp.Statements[i], statement) } } diff --git a/pkg/iam/policy/resourceset.go b/pkg/iam/policy/resourceset.go index 74725123c..b7eca66f9 100644 --- a/pkg/iam/policy/resourceset.go +++ b/pkg/iam/policy/resourceset.go @@ -54,6 +54,24 @@ func (resourceSet ResourceSet) Add(resource Resource) { resourceSet[resource] = struct{}{} } +// Equals - checks whether given resource set is equal to current resource set or not. +func (resourceSet ResourceSet) Equals(sresourceSet ResourceSet) bool { + // If length of set is not equal to length of given set, the + // set is not equal to given set. + if len(resourceSet) != len(sresourceSet) { + return false + } + + // As both sets are equal in length, check each elements are equal. + for k := range resourceSet { + if _, ok := sresourceSet[k]; !ok { + return false + } + } + + return true +} + // Intersection - returns resources available in both ResourceSet. func (resourceSet ResourceSet) Intersection(sset ResourceSet) ResourceSet { nset := NewResourceSet() diff --git a/pkg/policy/actionset.go b/pkg/policy/actionset.go index 8f21e29a3..1f4dd820b 100644 --- a/pkg/policy/actionset.go +++ b/pkg/policy/actionset.go @@ -38,6 +38,24 @@ func (actionSet ActionSet) Contains(action Action) bool { return found } +// Equals - checks whether given action set is equal to current action set or not. +func (actionSet ActionSet) Equals(sactionSet ActionSet) bool { + // If length of set is not equal to length of given set, the + // set is not equal to given set. + if len(actionSet) != len(sactionSet) { + return false + } + + // As both sets are equal in length, check each elements are equal. + for k := range actionSet { + if _, ok := sactionSet[k]; !ok { + return false + } + } + + return true +} + // Intersection - returns actions available in both ActionSet. func (actionSet ActionSet) Intersection(sset ActionSet) ActionSet { nset := NewActionSet() diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index 13931c299..cbdbbc081 100644 --- a/pkg/policy/policy.go +++ b/pkg/policy/policy.go @@ -92,18 +92,15 @@ func (policy Policy) isValid() error { continue } - principals := policy.Statements[i].Principal.Intersection(statement.Principal) - if principals.IsEmpty() { + if !policy.Statements[i].Principal.Equals(statement.Principal) { continue } - actions := policy.Statements[i].Actions.Intersection(statement.Actions) - if len(actions) == 0 { + if !policy.Statements[i].Actions.Equals(statement.Actions) { continue } - resources := policy.Statements[i].Resources.Intersection(statement.Resources) - if len(resources) == 0 { + if !policy.Statements[i].Resources.Equals(statement.Resources) { continue } @@ -112,7 +109,9 @@ func (policy Policy) isValid() error { } return Errorf("duplicate principal %v, actions %v, resouces %v found in statements %v, %v", - principals, actions, resources, policy.Statements[i], statement) + statement.Principal, statement.Actions, + statement.Resources, policy.Statements[i], + statement) } } diff --git a/pkg/policy/principal.go b/pkg/policy/principal.go index 9b4eb36eb..47d17e037 100644 --- a/pkg/policy/principal.go +++ b/pkg/policy/principal.go @@ -33,6 +33,11 @@ func (p Principal) IsValid() bool { return len(p.AWS) != 0 } +// Equals - returns true if principals are equal. +func (p Principal) Equals(pp Principal) bool { + return p.AWS.Equals(pp.AWS) +} + // Intersection - returns principals available in both Principal. func (p Principal) Intersection(principal Principal) set.StringSet { return p.AWS.Intersection(principal.AWS) diff --git a/pkg/policy/resourceset.go b/pkg/policy/resourceset.go index 88324b7b8..cab7dbbb8 100644 --- a/pkg/policy/resourceset.go +++ b/pkg/policy/resourceset.go @@ -54,6 +54,24 @@ func (resourceSet ResourceSet) Add(resource Resource) { resourceSet[resource] = struct{}{} } +// Equals - checks whether given resource set is equal to current resource set or not. +func (resourceSet ResourceSet) Equals(sresourceSet ResourceSet) bool { + // If length of set is not equal to length of given set, the + // set is not equal to given set. + if len(resourceSet) != len(sresourceSet) { + return false + } + + // As both sets are equal in length, check each elements are equal. + for k := range resourceSet { + if _, ok := sresourceSet[k]; !ok { + return false + } + } + + return true +} + // Intersection - returns resouces available in both ResourcsSet. func (resourceSet ResourceSet) Intersection(sset ResourceSet) ResourceSet { nset := NewResourceSet()