From d1bb8a5b2150ffc037a301440678585855f9f99f Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 10 Aug 2016 20:10:48 -0700 Subject: [PATCH] api: refactor the bucket policy reading and writing. (#2395) Policies are read once during server startup and subsequently managed through in memory map. In-memory map is updated as and when there are new changes coming in. --- bucket-handlers-listobjects.go | 4 +- bucket-handlers.go | 37 ++++------- bucket-policy-handlers.go | 31 ++++++--- bucket-policy-handlers_test.go | 76 +++------------------ bucket-policy-parser.go | 43 +++++++----- bucket-policy-parser_test.go | 113 ++++++++++++++++---------------- bucket-policy.go | 116 +++++++++++++++++++++++++++++---- object-handlers.go | 28 ++++---- routers.go | 4 ++ server_test.go | 78 ++++++++++++---------- 10 files changed, 298 insertions(+), 232 deletions(-) diff --git a/bucket-handlers-listobjects.go b/bucket-handlers-listobjects.go index 44974be30..d1e7275ea 100644 --- a/bucket-handlers-listobjects.go +++ b/bucket-handlers-listobjects.go @@ -71,7 +71,7 @@ func (api objectAPIHandlers) ListObjectsV2Handler(w http.ResponseWriter, r *http return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:ListBucket", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:ListBucket", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -130,7 +130,7 @@ func (api objectAPIHandlers) ListObjectsV1Handler(w http.ResponseWriter, r *http return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:ListBucket", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:ListBucket", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } diff --git a/bucket-handlers.go b/bucket-handlers.go index 626413055..fa24eef9f 100644 --- a/bucket-handlers.go +++ b/bucket-handlers.go @@ -28,25 +28,14 @@ import ( ) // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html -func enforceBucketPolicy(objAPI ObjectLayer, action string, bucket string, reqURL *url.URL) (s3Error APIErrorCode) { - // Read saved bucket policy. - policy, err := readBucketPolicy(bucket, objAPI) - if err != nil { - errorIf(err, "Unable read bucket policy.") - switch err.(type) { - case BucketNotFound: - return ErrNoSuchBucket - case BucketNameInvalid: - return ErrInvalidBucketName - default: - // For any other error just return AccessDenied. - return ErrAccessDenied - } - } - // Parse the saved policy. - bucketPolicy, err := parseBucketPolicy(policy) - if err != nil { - errorIf(err, "Unable to parse bucket policy.") +// Enforces bucket policies for a bucket for a given tatusaction. +func enforceBucketPolicy(bucket string, action string, reqURL *url.URL) (s3Error APIErrorCode) { + if !IsValidBucketName(bucket) { + return ErrInvalidBucketName + } + // Fetch bucket policy, if policy is not set return access denied. + policy := globalBucketPolicies.GetBucketPolicy(bucket) + if policy == nil { return ErrAccessDenied } @@ -60,7 +49,7 @@ func enforceBucketPolicy(objAPI ObjectLayer, action string, bucket string, reqUR } // Validate action, resource and conditions with current policy statements. - if !bucketPolicyEvalStatements(action, resource, conditions, bucketPolicy.Statements) { + if !bucketPolicyEvalStatements(action, resource, conditions, policy.Statements) { return ErrAccessDenied } return ErrNone @@ -80,7 +69,7 @@ func (api objectAPIHandlers) GetBucketLocationHandler(w http.ResponseWriter, r * return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:GetBucketLocation", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:GetBucketLocation", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -129,7 +118,7 @@ func (api objectAPIHandlers) ListMultipartUploadsHandler(w http.ResponseWriter, return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:ListBucketMultipartUploads", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:ListBucketMultipartUploads", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -207,7 +196,7 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:DeleteObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:DeleteObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -410,7 +399,7 @@ func (api objectAPIHandlers) HeadBucketHandler(w http.ResponseWriter, r *http.Re return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:ListBucket", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:ListBucket", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } diff --git a/bucket-policy-handlers.go b/bucket-policy-handlers.go index 1bc40ea6a..944e39d0d 100644 --- a/bucket-policy-handlers.go +++ b/bucket-policy-handlers.go @@ -18,6 +18,7 @@ package main import ( "bytes" + "fmt" "io" "io/ioutil" "net/http" @@ -166,15 +167,15 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht // Read access policy up to maxAccessPolicySize. // http://docs.aws.amazon.com/AmazonS3/latest/dev/access-policy-language-overview.html // bucket policies are limited to 20KB in size, using a limit reader. - bucketPolicyBuf, err := ioutil.ReadAll(io.LimitReader(r.Body, maxAccessPolicySize)) + policyBytes, err := ioutil.ReadAll(io.LimitReader(r.Body, maxAccessPolicySize)) if err != nil { - errorIf(err, "Unable to read bucket policy.") - writeErrorResponse(w, r, ErrInternalError, r.URL.Path) + errorIf(err, "Unable to read from client.") + writeErrorResponse(w, r, toAPIErrorCode(err), r.URL.Path) return } - // Parse bucket policy. - bucketPolicy, err := parseBucketPolicy(bucketPolicyBuf) + var policy = &bucketPolicy{} + err = parseBucketPolicy(bytes.NewReader(policyBytes), policy) if err != nil { errorIf(err, "Unable to parse bucket policy.") writeErrorResponse(w, r, ErrInvalidPolicyDocument, r.URL.Path) @@ -182,13 +183,13 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht } // Parse check bucket policy. - if s3Error := checkBucketPolicyResources(bucket, bucketPolicy); s3Error != ErrNone { + if s3Error := checkBucketPolicyResources(bucket, policy); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } // Save bucket policy. - if err := writeBucketPolicy(bucket, api.ObjectAPI, bucketPolicyBuf); err != nil { + if err = writeBucketPolicy(bucket, api.ObjectAPI, bytes.NewReader(policyBytes), int64(len(policyBytes))); err != nil { errorIf(err, "Unable to write bucket policy.") switch err.(type) { case BucketNameInvalid: @@ -198,6 +199,11 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht } return } + + // Set the bucket policy in memory. + globalBucketPolicies.SetBucketPolicy(bucket, policy) + + // Success. writeSuccessNoContent(w) } @@ -234,6 +240,11 @@ func (api objectAPIHandlers) DeleteBucketPolicyHandler(w http.ResponseWriter, r } return } + + // Remove bucket policy. + globalBucketPolicies.RemoveBucketPolicy(bucket) + + // Success. writeSuccessNoContent(w) } @@ -258,7 +269,7 @@ func (api objectAPIHandlers) GetBucketPolicyHandler(w http.ResponseWriter, r *ht } // Read bucket access policy. - p, err := readBucketPolicy(bucket, api.ObjectAPI) + policy, err := readBucketPolicy(bucket, api.ObjectAPI) if err != nil { errorIf(err, "Unable to read bucket policy.") switch err.(type) { @@ -271,5 +282,7 @@ func (api objectAPIHandlers) GetBucketPolicyHandler(w http.ResponseWriter, r *ht } return } - io.Copy(w, bytes.NewReader(p)) + + // Write to client. + fmt.Fprint(w, policy) } diff --git a/bucket-policy-handlers_test.go b/bucket-policy-handlers_test.go index bd6fb081f..156afc151 100644 --- a/bucket-policy-handlers_test.go +++ b/bucket-policy-handlers_test.go @@ -245,6 +245,8 @@ func TestPutBucketPolicyHandler(t *testing.T) { // testPutBucketPolicyHandler - Test for Bucket policy end point. // TODO: Add exhaustive cases with various combination of statement fields. func testPutBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { + initBucketPolicies(obj) + // get random bucket name. bucketName := getRandomBucketName() // Create bucket. @@ -267,40 +269,7 @@ func testPutBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrH credentials := serverConfig.GetCredential() // template for constructing HTTP request body for PUT bucket policy. - bucketPolicyTemplate := `{ - "Version": "2012-10-17", - "Statement": [ - { - "Action": [ - "s3:GetBucketLocation", - "s3:ListBucket" - ], - "Effect": "Allow", - "Principal": { - "AWS": [ - "*" - ] - }, - "Resource": [ - "arn:aws:s3:::%s" - ] - }, - { - "Action": [ - "s3:GetObject" - ], - "Effect": "Allow", - "Principal": { - "AWS": [ - "*" - ] - }, - "Resource": [ - "arn:aws:s3:::%s/this*" - ] - } - ] -}` + bucketPolicyTemplate := `{"Version":"2012-10-17","Statement":[{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetBucketLocation","s3:ListBucket"],"Resource":["arn:aws:s3:::%s"]},{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::%s/this*"]}]}` // test cases with sample input and expected output. testCases := []struct { @@ -342,6 +311,8 @@ func TestGetBucketPolicyHandler(t *testing.T) { // testGetBucketPolicyHandler - Test for end point which fetches the access policy json of the given bucket. // TODO: Add exhaustive cases with various combination of statement fields. func testGetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { + initBucketPolicies(obj) + // get random bucket name. bucketName := getRandomBucketName() // Create bucket. @@ -365,40 +336,7 @@ func testGetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrH credentials := serverConfig.GetCredential() // template for constructing HTTP request body for PUT bucket policy. - bucketPolicyTemplate := `{ - "Version": "2012-10-17", - "Statement": [ - { - "Action": [ - "s3:GetBucketLocation", - "s3:ListBucket" - ], - "Effect": "Allow", - "Principal": { - "AWS": [ - "*" - ] - }, - "Resource": [ - "arn:aws:s3:::%s" - ] - }, - { - "Action": [ - "s3:GetObject" - ], - "Effect": "Allow", - "Principal": { - "AWS": [ - "*" - ] - }, - "Resource": [ - "arn:aws:s3:::%s/this*" - ] - } - ] -}` + bucketPolicyTemplate := `{"Version":"2012-10-17","Statement":[{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetBucketLocation","s3:ListBucket"],"Resource":["arn:aws:s3:::%s"]},{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::%s/this*"]}]}` // Writing bucket policy before running test on GetBucketPolicy. putTestPolicies := []struct { @@ -483,6 +421,8 @@ func TestDeleteBucketPolicyHandler(t *testing.T) { // testDeleteBucketPolicyHandler - Test for Delete bucket policy end point. // TODO: Add exhaustive cases with various combination of statement fields. func testDeleteBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { + initBucketPolicies(obj) + // get random bucket name. bucketName := getRandomBucketName() // Create bucket. diff --git a/bucket-policy-parser.go b/bucket-policy-parser.go index d5bff1174..f47d22278 100644 --- a/bucket-policy-parser.go +++ b/bucket-policy-parser.go @@ -22,6 +22,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "path" "sort" "strings" @@ -71,15 +72,25 @@ type policyStatement struct { Principal policyUser `json:"Principal"` Actions []string `json:"Action"` Resources []string `json:"Resource"` - Conditions map[string]map[string]string `json:"Condition"` + Conditions map[string]map[string]string `json:"Condition,omitempty"` } -// BucketPolicy - minio policy collection -type BucketPolicy struct { +// bucketPolicy - collection of various bucket policy statements. +type bucketPolicy struct { Version string // date in 0000-00-00 format Statements []policyStatement `json:"Statement"` } +// Stringer implementation for the bucket policies. +func (b bucketPolicy) String() string { + bbytes, err := json.Marshal(&b) + if err != nil { + errorIf(err, "Unable to unmarshal bucket policy into JSON %#v", b) + return "" + } + return string(bbytes) +} + // supportedEffectMap - supported effects. var supportedEffectMap = map[string]struct{}{ "Allow": {}, @@ -213,7 +224,7 @@ func resourcePrefix(resource string) string { // checkBucketPolicyResources validates Resources in unmarshalled bucket policy structure. // First valation of Resources done for given set of Actions. // Later its validated for recursive Resources. -func checkBucketPolicyResources(bucket string, bucketPolicy BucketPolicy) APIErrorCode { +func checkBucketPolicyResources(bucket string, bucketPolicy *bucketPolicy) APIErrorCode { // Validate statements for special actions and collect resources // for others to validate nesting. var resourceMap = make(map[string]struct{}) @@ -268,44 +279,46 @@ func checkBucketPolicyResources(bucket string, bucketPolicy BucketPolicy) APIErr // parseBucketPolicy - parses and validates if bucket policy is of // proper JSON and follows allowed restrictions with policy standards. -func parseBucketPolicy(bucketPolicyBuf []byte) (policy BucketPolicy, err error) { - if err = json.Unmarshal(bucketPolicyBuf, &policy); err != nil { - return BucketPolicy{}, err +func parseBucketPolicy(bucketPolicyReader io.Reader, policy *bucketPolicy) (err error) { + // Parse bucket policy reader. + decoder := json.NewDecoder(bucketPolicyReader) + if err = decoder.Decode(&policy); err != nil { + return err } // Policy version cannot be empty. if len(policy.Version) == 0 { err = errors.New("Policy version cannot be empty.") - return BucketPolicy{}, err + return err } // Policy statements cannot be empty. if len(policy.Statements) == 0 { err = errors.New("Policy statement cannot be empty.") - return BucketPolicy{}, err + return err } // Loop through all policy statements and validate entries. for _, statement := range policy.Statements { // Statement effect should be valid. if err := isValidEffect(statement.Effect); err != nil { - return BucketPolicy{}, err + return err } // Statement principal should be supported format. if err := isValidPrincipals(statement.Principal.AWS); err != nil { - return BucketPolicy{}, err + return err } // Statement actions should be valid. if err := isValidActions(statement.Actions); err != nil { - return BucketPolicy{}, err + return err } // Statement resources should be valid. if err := isValidResources(statement.Resources); err != nil { - return BucketPolicy{}, err + return err } // Statement conditions should be valid. if err := isValidConditions(statement.Conditions); err != nil { - return BucketPolicy{}, err + return err } } @@ -326,5 +339,5 @@ func parseBucketPolicy(bucketPolicyBuf []byte) (policy BucketPolicy, err error) policy.Statements = append(denyStatements, allowStatements...) // Return successfully parsed policy structure. - return policy, nil + return nil } diff --git a/bucket-policy-parser_test.go b/bucket-policy-parser_test.go index 8a5d64f7e..350416027 100644 --- a/bucket-policy-parser_test.go +++ b/bucket-policy-parser_test.go @@ -16,10 +16,10 @@ package main import ( + "bytes" "encoding/json" "errors" "fmt" - "reflect" "testing" ) @@ -69,7 +69,7 @@ var ( } ) -// Obtain bucket statement for read-write BucketPolicy. +// Obtain bucket statement for read-write bucketPolicy. func getReadWriteObjectStatement(bucketName, objectPrefix string) policyStatement { objectResourceStatement := policyStatement{} objectResourceStatement.Effect = "Allow" @@ -79,7 +79,7 @@ func getReadWriteObjectStatement(bucketName, objectPrefix string) policyStatemen return objectResourceStatement } -// Obtain object statement for read-write BucketPolicy. +// Obtain object statement for read-write bucketPolicy. func getReadWriteBucketStatement(bucketName, objectPrefix string) policyStatement { bucketResourceStatement := policyStatement{} bucketResourceStatement.Effect = "Allow" @@ -89,7 +89,7 @@ func getReadWriteBucketStatement(bucketName, objectPrefix string) policyStatemen return bucketResourceStatement } -// Obtain statements for read-write BucketPolicy. +// Obtain statements for read-write bucketPolicy. func getReadWriteStatement(bucketName, objectPrefix string) []policyStatement { statements := []policyStatement{} // Save the read write policy. @@ -97,7 +97,7 @@ func getReadWriteStatement(bucketName, objectPrefix string) []policyStatement { return statements } -// Obtain bucket statement for read only BucketPolicy. +// Obtain bucket statement for read only bucketPolicy. func getReadOnlyBucketStatement(bucketName, objectPrefix string) policyStatement { bucketResourceStatement := policyStatement{} bucketResourceStatement.Effect = "Allow" @@ -107,7 +107,7 @@ func getReadOnlyBucketStatement(bucketName, objectPrefix string) policyStatement return bucketResourceStatement } -// Obtain object statement for read only BucketPolicy. +// Obtain object statement for read only bucketPolicy. func getReadOnlyObjectStatement(bucketName, objectPrefix string) policyStatement { objectResourceStatement := policyStatement{} objectResourceStatement.Effect = "Allow" @@ -117,7 +117,7 @@ func getReadOnlyObjectStatement(bucketName, objectPrefix string) policyStatement return objectResourceStatement } -// Obtain statements for read only BucketPolicy. +// Obtain statements for read only bucketPolicy. func getReadOnlyStatement(bucketName, objectPrefix string) []policyStatement { statements := []policyStatement{} // Save the read only policy. @@ -125,7 +125,7 @@ func getReadOnlyStatement(bucketName, objectPrefix string) []policyStatement { return statements } -// Obtain bucket statements for write only BucketPolicy. +// Obtain bucket statements for write only bucketPolicy. func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policyStatement { bucketResourceStatement := policyStatement{} @@ -136,7 +136,7 @@ func getWriteOnlyBucketStatement(bucketName, objectPrefix string) policyStatemen return bucketResourceStatement } -// Obtain object statements for write only BucketPolicy. +// Obtain object statements for write only bucketPolicy. func getWriteOnlyObjectStatement(bucketName, objectPrefix string) policyStatement { objectResourceStatement := policyStatement{} objectResourceStatement.Effect = "Allow" @@ -146,7 +146,7 @@ func getWriteOnlyObjectStatement(bucketName, objectPrefix string) policyStatemen return objectResourceStatement } -// Obtain statements for write only BucketPolicy. +// Obtain statements for write only bucketPolicy. func getWriteOnlyStatement(bucketName, objectPrefix string) []policyStatement { statements := []policyStatement{} // Write only policy. @@ -471,7 +471,7 @@ func TestIsValidConditions(t *testing.T) { } // Tests validate Policy Action and Resource fields. -func TestCheckBucketPolicyResources(t *testing.T) { +func TestCheckbucketPolicyResources(t *testing.T) { // constructing policy statement without invalidPrefixActions (check bucket-policy-parser.go). setValidPrefixActions := func(statements []policyStatement) []policyStatement { statements[0].Actions = []string{"s3:DeleteObject", "s3:PutObject"} @@ -491,27 +491,27 @@ func TestCheckBucketPolicyResources(t *testing.T) { return statements } - // List of BucketPolicy used for tests. - bucketAccessPolicies := []BucketPolicy{ - // BucketPolicy - 1. + // List of bucketPolicy used for tests. + bucketAccessPolicies := []bucketPolicy{ + // bucketPolicy - 1. // Contains valid read only policy statement. {Version: "1.0", Statements: getReadOnlyStatement("minio-bucket", "")}, - // BucketPolicy - 2. + // bucketPolicy - 2. // Contains valid read-write only policy statement. {Version: "1.0", Statements: getReadWriteStatement("minio-bucket", "Asia/")}, - // BucketPolicy - 3. + // bucketPolicy - 3. // Contains valid write only policy statement. {Version: "1.0", Statements: getWriteOnlyStatement("minio-bucket", "Asia/India/")}, - // BucketPolicy - 4. + // bucketPolicy - 4. // Contains invalidPrefixActions. // Since resourcePrefix is not to the bucket-name, it return ErrMalformedPolicy. {Version: "1.0", Statements: getReadOnlyStatement("minio-bucket-fail", "Asia/India/")}, - // BucketPolicy - 5. + // bucketPolicy - 5. // constructing policy statement without invalidPrefixActions (check bucket-policy-parser.go). // but bucket part of the resource is not equal to the bucket name. // this results in return of ErrMalformedPolicy. {Version: "1.0", Statements: setValidPrefixActions(getWriteOnlyStatement("minio-bucket-fail", "Asia/India/"))}, - // BucketPolicy - 6. + // bucketPolicy - 6. // contructing policy statement with recursive resources. // should result in ErrMalformedPolicy {Version: "1.0", Statements: setRecurseResource(setValidPrefixActions(getWriteOnlyStatement("minio-bucket", "")))}, @@ -523,7 +523,7 @@ func TestCheckBucketPolicyResources(t *testing.T) { } testCases := []struct { - inputPolicy BucketPolicy + inputPolicy bucketPolicy // expected results. apiErrCode APIErrorCode // Flag indicating whether the test should pass. @@ -554,7 +554,7 @@ func TestCheckBucketPolicyResources(t *testing.T) { {bucketAccessPolicies[6], ErrNone, true}, } for i, testCase := range testCases { - apiErrCode := checkBucketPolicyResources("minio-bucket", testCase.inputPolicy) + apiErrCode := checkBucketPolicyResources("minio-bucket", &testCase.inputPolicy) if apiErrCode != ErrNone && testCase.shouldPass { t.Errorf("Test %d: Expected to pass, but failed with Errocode %v", i+1, apiErrCode) } @@ -596,53 +596,53 @@ func TestParseBucketPolicy(t *testing.T) { statements[0].Resources = []string{"my-resource"} return statements } - // List of BucketPolicy used for test cases. - bucketAccesPolicies := []BucketPolicy{ - // BucketPolicy - 0. - // BucketPolicy statement empty. + // List of bucketPolicy used for test cases. + bucketAccesPolicies := []bucketPolicy{ + // bucketPolicy - 0. + // bucketPolicy statement empty. {Version: "1.0"}, - // BucketPolicy - 1. - // BucketPolicy version empty. + // bucketPolicy - 1. + // bucketPolicy version empty. {Version: "", Statements: []policyStatement{}}, - // BucketPolicy - 2. - // Readonly BucketPolicy. + // bucketPolicy - 2. + // Readonly bucketPolicy. {Version: "1.0", Statements: getReadOnlyStatement("minio-bucket", "")}, - // BucketPolicy - 3. + // bucketPolicy - 3. // Read-Write bucket policy. {Version: "1.0", Statements: getReadWriteStatement("minio-bucket", "Asia/")}, - // BucketPolicy - 4. + // bucketPolicy - 4. // Write only bucket policy. {Version: "1.0", Statements: getWriteOnlyStatement("minio-bucket", "Asia/India/")}, - // BucketPolicy - 5. - // BucketPolicy statement contains unsupported action. + // bucketPolicy - 5. + // bucketPolicy statement contains unsupported action. {Version: "1.0", Statements: setUnsupportedActions(getReadOnlyStatement("minio-bucket", ""))}, - // BucketPolicy - 6. - // BucketPolicy statement contains unsupported Effect. + // bucketPolicy - 6. + // bucketPolicy statement contains unsupported Effect. {Version: "1.0", Statements: setUnsupportedEffect(getReadWriteStatement("minio-bucket", "Asia/"))}, - // BucketPolicy - 7. - // BucketPolicy statement contains unsupported Principal. + // bucketPolicy - 7. + // bucketPolicy statement contains unsupported Principal. {Version: "1.0", Statements: setUnsupportedPrincipals(getWriteOnlyStatement("minio-bucket", "Asia/India/"))}, - // BucketPolicy - 8. - // BucketPolicy statement contains unsupported Resource. + // bucketPolicy - 8. + // bucketPolicy statement contains unsupported Resource. {Version: "1.0", Statements: setUnsupportedResources(getWriteOnlyStatement("minio-bucket", "Asia/India/"))}, } testCases := []struct { - inputPolicy BucketPolicy + inputPolicy bucketPolicy // expected results. - expectedPolicy BucketPolicy + expectedPolicy bucketPolicy err error // Flag indicating whether the test should pass. shouldPass bool }{ // Test case - 1. - // BucketPolicy statement empty. - {bucketAccesPolicies[0], BucketPolicy{}, errors.New("Policy statement cannot be empty."), false}, + // bucketPolicy statement empty. + {bucketAccesPolicies[0], bucketPolicy{}, errors.New("Policy statement cannot be empty."), false}, // Test case - 2. - // BucketPolicy version empty. - {bucketAccesPolicies[1], BucketPolicy{}, errors.New("Policy version cannot be empty."), false}, + // bucketPolicy version empty. + {bucketAccesPolicies[1], bucketPolicy{}, errors.New("Policy version cannot be empty."), false}, // Test case - 3. - // Readonly BucketPolicy. + // Readonly bucketPolicy. {bucketAccesPolicies[2], bucketAccesPolicies[2], nil, true}, // Test case - 4. // Read-Write bucket policy. @@ -651,25 +651,28 @@ func TestParseBucketPolicy(t *testing.T) { // Write only bucket policy. {bucketAccesPolicies[4], bucketAccesPolicies[4], nil, true}, // Test case - 6. - // BucketPolicy statement contains unsupported action. + // bucketPolicy statement contains unsupported action. {bucketAccesPolicies[5], bucketAccesPolicies[5], fmt.Errorf("Unsupported action found: ‘s3:DeleteEverything’, please validate your policy document."), false}, // Test case - 7. - // BucketPolicy statement contains unsupported Effect. + // bucketPolicy statement contains unsupported Effect. {bucketAccesPolicies[6], bucketAccesPolicies[6], fmt.Errorf("Unsupported Effect found: ‘DontAllow’, please validate your policy document."), false}, // Test case - 8. - // BucketPolicy statement contains unsupported Principal. + // bucketPolicy statement contains unsupported Principal. {bucketAccesPolicies[7], bucketAccesPolicies[7], fmt.Errorf("Unsupported principal style found: ‘User1111’, please validate your policy document."), false}, // Test case - 9. - // BucketPolicy statement contains unsupported Resource. + // bucketPolicy statement contains unsupported Resource. {bucketAccesPolicies[8], bucketAccesPolicies[8], fmt.Errorf("Unsupported resource style found: ‘my-resource’, please validate your policy document."), false}, } for i, testCase := range testCases { - inputPolicyBytes, e := json.Marshal(testCase.inputPolicy) - if e != nil { - t.Fatalf("Test %d: Couldn't Marshal bucket policy", i+1) + var buffer bytes.Buffer + encoder := json.NewEncoder(&buffer) + err := encoder.Encode(testCase.inputPolicy) + if err != nil { + t.Fatalf("Test %d: Couldn't Marshal bucket policy %s", i+1, err) } - actualAccessPolicy, err := parseBucketPolicy(inputPolicyBytes) + var actualAccessPolicy = &bucketPolicy{} + err = parseBucketPolicy(&buffer, actualAccessPolicy) if err != nil && testCase.shouldPass { t.Errorf("Test %d: Expected to pass, but failed with: %s", i+1, err.Error()) } @@ -684,7 +687,7 @@ func TestParseBucketPolicy(t *testing.T) { } // Test passes as expected, but the output values are verified for correctness here. if err == nil && testCase.shouldPass { - if !reflect.DeepEqual(testCase.expectedPolicy, actualAccessPolicy) { + if testCase.expectedPolicy.String() != actualAccessPolicy.String() { t.Errorf("Test %d: The expected statements from resource statement generator doesn't match the actual statements", i+1) } } diff --git a/bucket-policy.go b/bucket-policy.go index 7767fb002..315ae2fb2 100644 --- a/bucket-policy.go +++ b/bucket-policy.go @@ -18,25 +18,111 @@ package main import ( "bytes" - "path/filepath" + "errors" + "io" + "path" + "sync" ) -// getBucketsConfigPath - get buckets path. +// Variable represents bucket policies in memory. +var globalBucketPolicies *bucketPolicies + +// Global bucket policies list, policies are enforced on each bucket looking +// through the policies here. +type bucketPolicies struct { + rwMutex *sync.RWMutex + + // Collection of 'bucket' policies. + bucketPolicyConfigs map[string]*bucketPolicy +} + +// Fetch bucket policy for a given bucket. +func (bp bucketPolicies) GetBucketPolicy(bucket string) *bucketPolicy { + bp.rwMutex.RLock() + defer bp.rwMutex.RUnlock() + return bp.bucketPolicyConfigs[bucket] +} + +// Set a new bucket policy for a bucket, this operation will overwrite +// any previous bucketpolicies for the bucket. +func (bp *bucketPolicies) SetBucketPolicy(bucket string, policy *bucketPolicy) error { + bp.rwMutex.Lock() + defer bp.rwMutex.Unlock() + if policy == nil { + return errors.New("invalid argument") + } + bp.bucketPolicyConfigs[bucket] = policy + return nil +} + +// Remove bucket policy for a bucket, from in-memory map. +func (bp *bucketPolicies) RemoveBucketPolicy(bucket string) { + bp.rwMutex.Lock() + defer bp.rwMutex.Unlock() + delete(bp.bucketPolicyConfigs, bucket) +} + +// Loads all bucket policies from persistent layer. +func loadAllBucketPolicies(objAPI ObjectLayer) (policies map[string]*bucketPolicy, err error) { + // List buckets to proceed loading all notification configuration. + buckets, err := objAPI.ListBuckets() + if err != nil { + return nil, err + } + policies = make(map[string]*bucketPolicy) + // Loads bucket policy. + for _, bucket := range buckets { + var policy *bucketPolicy + policy, err = readBucketPolicy(bucket.Name, objAPI) + if err != nil { + switch err.(type) { + case BucketPolicyNotFound: + continue + } + return nil, err + } + policies[bucket.Name] = policy + } + + // Success. + return policies, nil + +} + +// Intialize all bucket policies. +func initBucketPolicies(objAPI ObjectLayer) error { + if objAPI == nil { + return errInvalidArgument + } + // Read all bucket policies. + policies, err := loadAllBucketPolicies(objAPI) + if err != nil { + return err + } + + globalBucketPolicies = &bucketPolicies{ + rwMutex: &sync.RWMutex{}, + bucketPolicyConfigs: policies, + } + return nil +} + +// getOldBucketsConfigPath - get old buckets config path. (Only used for migrating old bucket policies) func getOldBucketsConfigPath() (string, error) { configPath, err := getConfigPath() if err != nil { return "", err } - return filepath.Join(configPath, "buckets"), nil + return path.Join(configPath, "buckets"), nil } -// readBucketPolicy - read bucket policy. -func readBucketPolicy(bucket string, objAPI ObjectLayer) ([]byte, error) { +// readBucketPolicy - reads bucket policy for an input bucket, returns BucketPolicyNotFound +// if bucket policy is not found. This function also parses the bucket policy into an object. +func readBucketPolicy(bucket string, objAPI ObjectLayer) (*bucketPolicy, error) { // Verify bucket is valid. if !IsValidBucketName(bucket) { return nil, BucketNameInvalid{Bucket: bucket} } - policyPath := pathJoin(bucketConfigPrefix, bucket, policyJSON) objInfo, err := objAPI.GetObjectInfo(minioMetaBucket, policyPath) if err != nil { @@ -53,10 +139,18 @@ func readBucketPolicy(bucket string, objAPI ObjectLayer) ([]byte, error) { } return nil, err } - return buffer.Bytes(), nil + // Parse the saved policy. + var policy = &bucketPolicy{} + err = parseBucketPolicy(&buffer, policy) + if err != nil { + return nil, err + + } + return policy, nil } -// removeBucketPolicy - remove bucket policy. +// removeBucketPolicy - removes any previously written bucket policy. Returns BucketPolicyNotFound +// if no policies are found. func removeBucketPolicy(bucket string, objAPI ObjectLayer) error { // Verify bucket is valid. if !IsValidBucketName(bucket) { @@ -73,14 +167,14 @@ func removeBucketPolicy(bucket string, objAPI ObjectLayer) error { return nil } -// writeBucketPolicy - save bucket policy. -func writeBucketPolicy(bucket string, objAPI ObjectLayer, accessPolicyBytes []byte) error { +// writeBucketPolicy - save all bucket policies. +func writeBucketPolicy(bucket string, objAPI ObjectLayer, reader io.Reader, size int64) error { // Verify if bucket path legal if !IsValidBucketName(bucket) { return BucketNameInvalid{Bucket: bucket} } policyPath := pathJoin(bucketConfigPrefix, bucket, policyJSON) - _, err := objAPI.PutObject(minioMetaBucket, policyPath, int64(len(accessPolicyBytes)), bytes.NewReader(accessPolicyBytes), nil) + _, err := objAPI.PutObject(minioMetaBucket, policyPath, size, reader, nil) return err } diff --git a/object-handlers.go b/object-handlers.go index a84e3d87d..3a92a8955 100644 --- a/object-handlers.go +++ b/object-handlers.go @@ -54,13 +54,13 @@ func setGetRespHeaders(w http.ResponseWriter, reqParams url.Values) { // this is in keeping with the permissions sections of the docs of both: // HEAD Object: http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectHEAD.html // GET Object: http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html -func errAllowableObjectNotFound(objAPI ObjectLayer, bucket string, r *http.Request) APIErrorCode { +func errAllowableObjectNotFound(bucket string, r *http.Request) APIErrorCode { if getRequestAuthType(r) == authTypeAnonymous { //we care about the bucket as a whole, not a particular resource url := *r.URL url.Path = "/" + bucket - if s3Error := enforceBucketPolicy(objAPI, "s3:ListBucket", bucket, &url); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:ListBucket", &url); s3Error != ErrNone { return ErrAccessDenied } } @@ -91,7 +91,7 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:GetObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:GetObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -107,7 +107,7 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req errorIf(err, "Unable to fetch object info.") apiErr := toAPIErrorCode(err) if apiErr == ErrNoSuchKey { - apiErr = errAllowableObjectNotFound(api.ObjectAPI, bucket, r) + apiErr = errAllowableObjectNotFound(bucket, r) } writeErrorResponse(w, r, apiErr, r.URL.Path) return @@ -197,7 +197,7 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:GetObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:GetObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -213,7 +213,7 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re errorIf(err, "Unable to fetch object info.") apiErr := toAPIErrorCode(err) if apiErr == ErrNoSuchKey { - apiErr = errAllowableObjectNotFound(api.ObjectAPI, bucket, r) + apiErr = errAllowableObjectNotFound(bucket, r) } writeErrorResponse(w, r, apiErr, r.URL.Path) return @@ -247,7 +247,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:PutObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -426,7 +426,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:PutObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -492,7 +492,7 @@ func (api objectAPIHandlers) NewMultipartUploadHandler(w http.ResponseWriter, r return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:PutObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -583,7 +583,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:PutObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -627,7 +627,7 @@ func (api objectAPIHandlers) AbortMultipartUploadHandler(w http.ResponseWriter, return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:AbortMultipartUpload", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:AbortMultipartUpload", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -660,7 +660,7 @@ func (api objectAPIHandlers) ListObjectPartsHandler(w http.ResponseWriter, r *ht return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:ListMultipartUploadParts", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:ListMultipartUploadParts", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -711,7 +711,7 @@ func (api objectAPIHandlers) CompleteMultipartUploadHandler(w http.ResponseWrite return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/mpuAndPermissions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:PutObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:PutObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } @@ -832,7 +832,7 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http. return case authTypeAnonymous: // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html - if s3Error := enforceBucketPolicy(api.ObjectAPI, "s3:DeleteObject", bucket, r.URL); s3Error != ErrNone { + if s3Error := enforceBucketPolicy(bucket, "s3:DeleteObject", r.URL); s3Error != ErrNone { writeErrorResponse(w, r, s3Error, r.URL.Path) return } diff --git a/routers.go b/routers.go index b2e55f455..c7b4069db 100644 --- a/routers.go +++ b/routers.go @@ -70,6 +70,10 @@ func configureServerHandler(srvCmdConfig serverCmdConfig) http.Handler { err = initEventNotifier(objAPI) fatalIf(err, "Unable to initialize event notification queue") + // Initialize a new bucket policies. + err = initBucketPolicies(objAPI) + fatalIf(err, "Unable to load all bucket policies") + // Initialize router. mux := router.NewRouter() diff --git a/server_test.go b/server_test.go index e6f1da9c7..cd0e825da 100644 --- a/server_test.go +++ b/server_test.go @@ -161,40 +161,7 @@ func (s *TestSuiteCommon) TestBucketNotification(c *C) { // Deletes the policy and verifies the deletion by fetching it back. func (s *TestSuiteCommon) TestBucketPolicy(c *C) { // Sample bucket policy. - bucketPolicyBuf := `{ - "Version": "2012-10-17", - "Statement": [ - { - "Action": [ - "s3:GetBucketLocation", - "s3:ListBucket" - ], - "Effect": "Allow", - "Principal": { - "AWS": [ - "*" - ] - }, - "Resource": [ - "arn:aws:s3:::%s" - ] - }, - { - "Action": [ - "s3:GetObject" - ], - "Effect": "Allow", - "Principal": { - "AWS": [ - "*" - ] - }, - "Resource": [ - "arn:aws:s3:::%s/this*" - ] - } - ] -}` + bucketPolicyBuf := `{"Version":"2012-10-17","Statement":[{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetBucketLocation","s3:ListBucket"],"Resource":["arn:aws:s3:::%s"]},{"Sid":"","Effect":"Allow","Principal":{"AWS":["*"]},"Action":["s3:GetObject"],"Resource":["arn:aws:s3:::%s/this*"]}]}` // generate a random bucket Name. bucketName := getRandomBucketName() @@ -496,6 +463,49 @@ func (s *TestSuiteCommon) TestBucket(c *C) { c.Assert(response.StatusCode, Equals, http.StatusOK) } +// Tests get anonymous object. +func (s *TestSuiteCommon) TestObjectGetAnonymous(c *C) { + // generate a random bucket name. + bucketName := getRandomBucketName() + buffer := bytes.NewReader([]byte("hello world")) + // HTTP request to create the bucket. + request, err := newTestSignedRequest("PUT", getMakeBucketURL(s.endPoint, bucketName), + 0, nil, s.accessKey, s.secretKey) + c.Assert(err, IsNil) + + client := http.Client{} + // execute the make bucket http request. + response, err := client.Do(request) + c.Assert(err, IsNil) + // assert the response http status code. + c.Assert(response.StatusCode, Equals, http.StatusOK) + + objectName := "testObject" + // create HTTP request to upload the object. + request, err = newTestSignedRequest("PUT", getPutObjectURL(s.endPoint, bucketName, objectName), + int64(buffer.Len()), buffer, s.accessKey, s.secretKey) + c.Assert(err, IsNil) + + client = http.Client{} + // execute the HTTP request to upload the object. + response, err = client.Do(request) + c.Assert(err, IsNil) + // assert the HTTP response status code. + c.Assert(response.StatusCode, Equals, http.StatusOK) + + // initiate anonymous HTTP request to fetch the object which does not exist. We need to return AccessDenied. + response, err = http.Get(getGetObjectURL(s.endPoint, bucketName, objectName+".1")) + c.Assert(err, IsNil) + // assert the http response status code. + verifyError(c, response, "AccessDenied", "Access Denied.", http.StatusForbidden) + + // initiate anonymous HTTP request to fetch the object which does exist. We need to return AccessDenied. + response, err = http.Get(getGetObjectURL(s.endPoint, bucketName, objectName)) + c.Assert(err, IsNil) + // assert the http response status code. + verifyError(c, response, "AccessDenied", "Access Denied.", http.StatusForbidden) +} + // TestGetObject - Tests fetching of a small object after its insertion into the bucket. func (s *TestSuiteCommon) TestObjectGet(c *C) { // generate a random bucket name.