Resource matching fix to overcome issues with regular expression based match (#1476)

master
karthic rao 9 years ago committed by Harshavardhana
parent a5959789d5
commit 82113b747c
  1. 2
      bucket-handlers.go
  2. 38
      bucket-policy-handlers.go
  3. 78
      bucket-policy-handlers_test.go

@ -54,7 +54,7 @@ func enforceBucketPolicy(action string, bucket string, reqURL *url.URL) (s3Error
return ErrAccessDenied
}
// Construct resource in 'arn:aws:s3:::examplebucket' format.
// Construct resource in 'arn:aws:s3:::examplebucket/object' format.
resource := AWSResourcePrefix + strings.TrimPrefix(reqURL.Path, "/")
// Get conditions for policy verification.

@ -77,15 +77,43 @@ func bucketPolicyActionMatch(action string, statement policyStatement) bool {
// Verify if given resource matches with policy statement.
func bucketPolicyResourceMatch(resource string, statement policyStatement) bool {
for _, presource := range statement.Resources {
matched, err := regexp.MatchString(presource, strings.TrimPrefix(resource, "/"))
fatalIf(err, "Invalid pattern, please verify the pattern string.", nil)
// For any path matches, we return quickly and the let the caller continue.
if matched {
match := func(pattern, subj string) bool {
if pattern == "" {
return subj == pattern
}
if pattern == "*" {
return true
}
parts := strings.Split(pattern, "*")
if len(parts) == 1 {
return subj == pattern
}
tGlob := strings.HasSuffix(pattern, "*")
end := len(parts) - 1
if !strings.HasPrefix(subj, parts[0]) {
return false
}
for i := 1; i < end; i++ {
if !strings.Contains(subj, parts[i]) {
return false
}
idx := strings.Index(subj, parts[i]) + len(parts[i])
subj = subj[idx:]
}
return tGlob || strings.HasSuffix(subj, parts[end])
}
for _, presource := range statement.Resources {
// the resource rule for object could contain "*" wild card.
// the requested object can be given access based on the already set bucket policy if
// the match is successful.
// More info: http://docs.aws.amazon.com/AmazonS3/latest/dev/s3-arn-format.html .
if matched := match(presource, resource); !matched {
return false
}
}
return true
}
// Verify if given condition matches with policy statement.

@ -0,0 +1,78 @@
/*
* Minio Cloud Storage, (C) 2015, 2016 Minio, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package main
import (
"fmt"
"testing"
)
// Tests validate Bucket policy resource matcher.
func TestBucketPolicyResourceMatch(t *testing.T) {
// generates\ statement with given resource..
generateStatement := func(resource string) policyStatement {
statement := policyStatement{}
statement.Resources = []string{resource}
return statement
}
// generates resource prefix.
generateResource := func(bucketName, objectName string) string {
return AWSResourcePrefix + bucketName + "/" + objectName
}
testCases := []struct {
resourceToMatch string
statement policyStatement
expectedResourceMatch bool
}{
// Test case 1-4.
// Policy with resource ending with bucket/* allows access to all objects inside the given bucket.
{generateResource("minio-bucket", ""), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/*")), true},
{generateResource("minio-bucket", ""), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/*")), true},
{generateResource("minio-bucket", ""), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/*")), true},
{generateResource("minio-bucket", ""), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/*")), true},
// Test case - 5.
// Policy with resource ending with bucket/oo* should not allow access to bucket/output.txt.
{generateResource("minio-bucket", "output.txt"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/oo*")), false},
// Test case - 6.
// Policy with resource ending with bucket/oo* should allow access to bucket/ootput.txt.
{generateResource("minio-bucket", "ootput.txt"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/oo*")), true},
// Test case - 7.
// Policy with resource ending with bucket/oo* allows access to all subfolders starting with "oo" inside given bucket.
{generateResource("minio-bucket", "oop-bucket/my-file"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/oo*")), true},
// Test case - 8.
{generateResource("minio-bucket", "Asia/India/1.pjg"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/Asia/Japan/*")), false},
// Test case - 9.
{generateResource("minio-bucket", "Asia/India/1.pjg"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix, "minio-bucket"+"/Asia/Japan/*")), false},
// Test case - 10.
// Proves that the name space is flat.
{generateResource("minio-bucket", "Africa/Bihar/India/design_info.doc/Bihar"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix,
"minio-bucket"+"/*/India/*/Bihar")), true},
// Test case - 11.
// Proves that the name space is flat.
{generateResource("minio-bucket", "Asia/China/India/States/Bihar/output.txt"), generateStatement(fmt.Sprintf("%s%s", AWSResourcePrefix,
"minio-bucket"+"/*/India/*/Bihar/*")), true},
}
for i, testCase := range testCases {
actualResourceMatch := bucketPolicyResourceMatch(testCase.resourceToMatch, testCase.statement)
if testCase.expectedResourceMatch != actualResourceMatch {
t.Errorf("Test %d: Expected Resource match to be `%v`, but instead found it to be `%v`", i+1, testCase.expectedResourceMatch, actualResourceMatch)
}
}
}
Loading…
Cancel
Save