From 99bf4d0c429f04dbd013ba98840d07b759ae1702 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 15 Jun 2019 11:27:17 -0700 Subject: [PATCH] [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. --- go.mod | 3 +-- go.sum | 12 ++---------- pkg/iam/policy/resource.go | 3 ++- pkg/iam/policy/resource_test.go | 15 +++++++++------ pkg/iam/policy/resourceset_test.go | 15 +++++++++------ 5 files changed, 23 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index 904ffcad7..8f62b5466 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/djherbis/atime v1.0.0 github.com/dnaeon/go-vcr v1.0.1 // indirect github.com/dustin/go-humanize v1.0.0 + github.com/eapache/go-resiliency v1.2.0 // indirect github.com/eclipse/paho.mqtt.golang v1.1.2-0.20190322152051-20337d8c3947 github.com/elazarl/go-bindata-assetfs v1.0.0 github.com/fatih/color v1.7.0 @@ -89,10 +90,8 @@ require ( go.etcd.io/bbolt v1.3.3 // indirect go.uber.org/atomic v1.3.2 golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8 - golang.org/x/lint v0.0.0-20190409202823-959b441ac422 // indirect golang.org/x/net v0.0.0-20190611141213-3f473d35a33a // indirect golang.org/x/sys v0.0.0-20190610200419-93c9922d18ae - golang.org/x/tools v0.0.0-20190612232758-d4e310b4a8a5 // indirect google.golang.org/api v0.4.0 gopkg.in/Shopify/sarama.v1 v1.20.0 gopkg.in/olivere/elastic.v5 v5.0.80 diff --git a/go.sum b/go.sum index 844ca25c4..c233c57a0 100644 --- a/go.sum +++ b/go.sum @@ -128,6 +128,8 @@ github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25Kn github.com/eapache/go-resiliency v0.0.0-20160104191539-b86b1ec0dd42/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs= github.com/eapache/go-resiliency v1.1.0 h1:1NtRmCAqadE2FN4ZcN6g90TP3uk8cg9rn9eNK2197aU= github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs= +github.com/eapache/go-resiliency v1.2.0 h1:v7g92e/KSN71Rq7vSThKaWIq68fL4YHvWyiUKorFR1Q= +github.com/eapache/go-resiliency v1.2.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs= github.com/eapache/go-xerial-snappy v0.0.0-20160609142408-bb955e01b934/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU= github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 h1:YEetp8/yCZMuEPMUDHG0CW/brkkEp8mzqk2+ODEitlw= github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU= @@ -433,8 +435,6 @@ github.com/minio/minio-go v0.0.0-20190327203652-5325257a208f h1:u+iNxfkLrfyWp7Kx github.com/minio/minio-go v0.0.0-20190327203652-5325257a208f/go.mod h1:/haSOWG8hQNx2+JOfLJ9GKp61EAmgPwRVw/Sac0NzaM= github.com/minio/minio-go/v6 v6.0.26 h1:nHLr1A+sJBv/sQu6zc5BrHLFAStCXxloC+jmZp4FtW0= github.com/minio/minio-go/v6 v6.0.26/go.mod h1:vaNT59cWULS37E+E9zkuN/BVnKHyXtVGS+b04Boc66Y= -github.com/minio/minio-go/v6 v6.0.28 h1:RIsoxMaljP2euGbu2r0gBC0UNn70l2gzHjifU6DhE0c= -github.com/minio/minio-go/v6 v6.0.28/go.mod h1:vaNT59cWULS37E+E9zkuN/BVnKHyXtVGS+b04Boc66Y= github.com/minio/minio-go/v6 v6.0.29 h1:p4YPxK1beY13reFJjCE5QwCnXUMT9D5sV5wl0BSy5Xo= github.com/minio/minio-go/v6 v6.0.29/go.mod h1:vaNT59cWULS37E+E9zkuN/BVnKHyXtVGS+b04Boc66Y= github.com/minio/parquet-go v0.0.0-20190318185229-9d767baf1679 h1:OMKaN/82sBHUZPvjYNBFituHExa1OGY63eACDGtetKs= @@ -679,8 +679,6 @@ golang.org/x/crypto v0.0.0-20190325154230-a5d413f7728c/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190426145343-a29dc8fdc734/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f h1:R423Cnkcp5JABoeemiGEPlt9tHXFfw5kvc0yqlxRPWo= golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5 h1:58fnuSXlxZmFdJyvtTFVmVhcMLU6v5fEb/ok4wyqtNU= -golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8 h1:1wopBVtVdWnn03fZelqdXTqk7U7zPQCb+T4rbU9ZEoU= golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -691,8 +689,6 @@ golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvx golang.org/x/lint v0.0.0-20190301231843-5614ed5bae6f/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 h1:XQyxROzUlZH+WIQwySDgnISgOivlhjIEwaQaJEJrrN0= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= -golang.org/x/lint v0.0.0-20190409202823-959b441ac422 h1:QzoH/1pFpZguR8NrRHLcO6jKqfv2zpuSqZLgdm7ZmjI= -golang.org/x/lint v0.0.0-20190409202823-959b441ac422/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -715,8 +711,6 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190424112056-4829fb13d2c6/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190522155817-f3200d17e092 h1:4QSRKanuywn15aTZvI/mIDEgPQpswuFndXpOj3rKEco= golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.0.0-20190607181551-461777fb6f67 h1:rJJxsykSlULwd2P2+pg/rtnwN2FrWp4IuCxOSyS0V00= -golang.org/x/net v0.0.0-20190607181551-461777fb6f67/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190611141213-3f473d35a33a h1:+KkCgOMgnKSgenxTBoiwkMqTiouMIy/3o8RLdmSbGoY= golang.org/x/net v0.0.0-20190611141213-3f473d35a33a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180603041954-1e0a3fa8ba9a/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -781,8 +775,6 @@ golang.org/x/tools v0.0.0-20190312170243-e65039ee4138/go.mod h1:LCzVGOaR6xXOjkQ3 golang.org/x/tools v0.0.0-20190318200714-bb1270c20edf/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20190328211700-ab21143f2384 h1:TFlARGu6Czu1z7q93HTxcP1P+/ZFC/IKythI5RzrnRg= golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= -golang.org/x/tools v0.0.0-20190612232758-d4e310b4a8a5 h1:WfRBLVK37R+k1gUOKuZX8JtangyEXmuopHz5tazlZRo= -golang.org/x/tools v0.0.0-20190612232758-d4e310b4a8a5/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= google.golang.org/api v0.0.0-20180603000442-8e296ef26005/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.0.0-20180910000450-7ca32eb868bf/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= google.golang.org/api v0.0.0-20180916000451-19ff8768a5c0/go.mod h1:4mhQ8q/RsB7i+udVvVy5NUi08OU8ZlA0gRVgrF7VFY0= diff --git a/pkg/iam/policy/resource.go b/pkg/iam/policy/resource.go index 3ac5229e7..aa4446c70 100644 --- a/pkg/iam/policy/resource.go +++ b/pkg/iam/policy/resource.go @@ -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) diff --git a/pkg/iam/policy/resource_test.go b/pkg/iam/policy/resource_test.go index 5107af332..a6c7e6efd 100644 --- a/pkg/iam/policy/resource_test.go +++ b/pkg/iam/policy/resource_test.go @@ -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) + } + }) } } diff --git a/pkg/iam/policy/resourceset_test.go b/pkg/iam/policy/resourceset_test.go index d6e7b31ce..5395bdd61 100644 --- a/pkg/iam/policy/resourceset_test.go +++ b/pkg/iam/policy/resourceset_test.go @@ -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) + } + }) } }