[sec] Match ${aws:username} exactly instead of prefix match (#7791)

This PR fixes a security issue where an IAM user based
on his policy is granted more privileges than restricted
by the users IAM policy.

This is due to an issue of prefix based Matcher() function
which was incorrectly matching prefix based on resource
prefixes instead of exact match.
This commit is contained in:
Harshavardhana
2019-06-15 11:27:17 -07:00
committed by kannappanr
parent 7b8beecc81
commit 99bf4d0c42
5 changed files with 23 additions and 25 deletions

View File

@@ -19,6 +19,7 @@ package iampolicy
import (
"encoding/json"
"fmt"
"path"
"strings"
"github.com/minio/minio/pkg/policy/condition"
@@ -56,7 +57,7 @@ func (r Resource) Match(resource string, conditionValues map[string][]string) bo
pattern = strings.Replace(pattern, key.VarName(), rvalues[0], -1)
}
}
if strings.HasPrefix(resource, pattern) {
if path.Clean(resource) == pattern {
return true
}
return wildcard.Match(pattern, resource)

View File

@@ -18,6 +18,7 @@ package iampolicy
import (
"encoding/json"
"fmt"
"reflect"
"testing"
)
@@ -120,15 +121,17 @@ func TestResourceMatch(t *testing.T) {
{NewResource("*", "*"), "mybucket", false},
{NewResource("mybucket", "*"), "mybucket10/myobject", false},
{NewResource("mybucket?0", "/2010/photos/*"), "mybucket0/2010/photos/1.jpg", false},
{NewResource("mybucket", ""), "mybucket/myobject", true},
{NewResource("mybucket", ""), "mybucket/myobject", false},
}
for i, testCase := range testCases {
result := testCase.resource.Match(testCase.objectName, nil)
if result != testCase.expectedResult {
t.Fatalf("case %v: expected: %v, got: %v", i+1, testCase.expectedResult, result)
}
testCase := testCase
t.Run(fmt.Sprintf("Test%d", i+1), func(t *testing.T) {
result := testCase.resource.Match(testCase.objectName, nil)
if result != testCase.expectedResult {
t.Errorf("case %v: expected: %v, got: %v", i+1, testCase.expectedResult, result)
}
})
}
}

View File

@@ -18,6 +18,7 @@ package iampolicy
import (
"encoding/json"
"fmt"
"reflect"
"testing"
)
@@ -174,16 +175,18 @@ func TestResourceSetMatch(t *testing.T) {
{NewResourceSet(NewResource("", "*")), "mybucket/myobject", false},
{NewResourceSet(NewResource("*", "*")), "mybucket", false},
{NewResourceSet(NewResource("mybucket", "*")), "mybucket10/myobject", false},
{NewResourceSet(NewResource("mybucket", "")), "mybucket/myobject", true},
{NewResourceSet(NewResource("mybucket", "")), "mybucket/myobject", false},
{NewResourceSet(), "mybucket/myobject", false},
}
for i, testCase := range testCases {
result := testCase.resourceSet.Match(testCase.resource, nil)
if result != testCase.expectedResult {
t.Fatalf("case %v: expected: %v, got: %v", i+1, testCase.expectedResult, result)
}
testCase := testCase
t.Run(fmt.Sprintf("Test%d", i+1), func(t *testing.T) {
result := testCase.resourceSet.Match(testCase.resource, nil)
if result != testCase.expectedResult {
t.Errorf("case %v: expected: %v, got: %v", i+1, testCase.expectedResult, result)
}
})
}
}