From 6aa2fc95c00d2313ee88059c861f957223a7c96c Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 26 Sep 2016 14:28:35 -0700 Subject: [PATCH] Revert "bucket: refactor policies and fix bugs related to enforcing policies. (#2766)" This reverts commit ca5ca8332bdef662bc6bcb3e421013139c42954e. --- cmd/bucket-handlers.go | 15 +++-- cmd/bucket-handlers_test.go | 6 ++ cmd/bucket-policy-handlers.go | 6 ++ cmd/bucket-policy-handlers_test.go | 6 ++ cmd/bucket-policy.go | 100 +++++++++++++++++++++++++++++ cmd/controller-handlers.go | 4 +- cmd/routers.go | 4 ++ cmd/storage-errors.go | 3 + cmd/test-utils_test.go | 1 - cmd/web-handlers.go | 18 +++--- 10 files changed, 144 insertions(+), 19 deletions(-) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index 17cd94763..d63975e91 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -31,9 +31,8 @@ import ( // http://docs.aws.amazon.com/AmazonS3/latest/dev/using-with-s3-actions.html // Enforces bucket policies for a bucket for a given tatusaction. func enforceBucketPolicy(bucket string, action string, reqURL *url.URL) (s3Error APIErrorCode) { - // Fetch bucket policy, if policy is not set return access denied. - policy, err := readBucketPolicy(bucket, newObjectLayerFn()) - if err != nil { + // Verify if bucket actually exists + if err := isBucketExist(bucket, newObjectLayerFn()); err != nil { err = errorCause(err) switch err.(type) { case BucketNameInvalid: @@ -42,16 +41,18 @@ func enforceBucketPolicy(bucket string, action string, reqURL *url.URL) (s3Error case BucketNotFound: // For no bucket found we return NoSuchBucket instead. return ErrNoSuchBucket - case BucketPolicyNotFound: - // For no bucket policy found, return AccessDenied, since - // anonymous requests are not allowed without bucket policies. - return ErrAccessDenied } errorIf(err, "Unable to read bucket policy.") // Return internal error for any other errors so that we can investigate. return ErrInternalError } + // Fetch bucket policy, if policy is not set return access denied. + policy := globalBucketPolicies.GetBucketPolicy(bucket) + if policy == nil { + return ErrAccessDenied + } + // Construct resource in 'arn:aws:s3:::examplebucket/object' format. resource := AWSResourcePrefix + strings.TrimSuffix(strings.TrimPrefix(reqURL.Path, "/"), "/") diff --git a/cmd/bucket-handlers_test.go b/cmd/bucket-handlers_test.go index bc993bac2..60b693ac8 100644 --- a/cmd/bucket-handlers_test.go +++ b/cmd/bucket-handlers_test.go @@ -30,6 +30,8 @@ func TestGetBucketLocationHandler(t *testing.T) { } func testGetBucketLocationHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { + initBucketPolicies(obj) + // get random bucket name. bucketName := getRandomBucketName() // Create bucket. @@ -128,6 +130,8 @@ func TestHeadBucketHandler(t *testing.T) { } func testHeadBucketHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { + initBucketPolicies(obj) + // get random bucket name. bucketName := getRandomBucketName() // Create bucket. @@ -203,6 +207,8 @@ func TestListMultipartUploadsHandler(t *testing.T) { // testListMultipartUploadsHandler - Tests validate listing of multipart uploads. func testListMultipartUploadsHandler(obj ObjectLayer, instanceType string, t TestErrHandler) { + initBucketPolicies(obj) + // get random bucket name. bucketName := getRandomBucketName() diff --git a/cmd/bucket-policy-handlers.go b/cmd/bucket-policy-handlers.go index 65207c66d..e6a5b826e 100644 --- a/cmd/bucket-policy-handlers.go +++ b/cmd/bucket-policy-handlers.go @@ -196,6 +196,9 @@ func (api objectAPIHandlers) PutBucketPolicyHandler(w http.ResponseWriter, r *ht return } + // Set the bucket policy in memory. + globalBucketPolicies.SetBucketPolicy(bucket, policy) + // Success. writeSuccessNoContent(w) } @@ -239,6 +242,9 @@ func (api objectAPIHandlers) DeleteBucketPolicyHandler(w http.ResponseWriter, r return } + // Remove bucket policy. + globalBucketPolicies.RemoveBucketPolicy(bucket) + // Success. writeSuccessNoContent(w) } diff --git a/cmd/bucket-policy-handlers_test.go b/cmd/bucket-policy-handlers_test.go index 27e5517de..cc2b75c45 100644 --- a/cmd/bucket-policy-handlers_test.go +++ b/cmd/bucket-policy-handlers_test.go @@ -247,6 +247,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. @@ -311,6 +313,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. @@ -419,6 +423,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/cmd/bucket-policy.go b/cmd/bucket-policy.go index 96fb7ea00..f510ebabb 100644 --- a/cmd/bucket-policy.go +++ b/cmd/bucket-policy.go @@ -18,10 +18,110 @@ package cmd import ( "bytes" + "errors" "io" "path" + "sync" ) +// 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() + errorIf(err, "Unable to list buckets.") + err = errorCause(err) + if err != nil { + return nil, err + } + + policies = make(map[string]*bucketPolicy) + var pErrs []error + // Loads bucket policy. + for _, bucket := range buckets { + policy, pErr := readBucketPolicy(bucket.Name, objAPI) + if pErr != nil { + switch pErr.(type) { + case BucketPolicyNotFound: + continue + } + pErrs = append(pErrs, pErr) + // Continue to load other bucket policies if possible. + continue + } + policies[bucket.Name] = policy + } + + // Look for any errors occurred while reading bucket policies. + for _, pErr := range pErrs { + if pErr != nil { + return policies, pErr + } + } + + // 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 + } + + // Populate global bucket collection. + globalBucketPolicies = &bucketPolicies{ + rwMutex: &sync.RWMutex{}, + bucketPolicyConfigs: policies, + } + + // Success. + return nil +} + // getOldBucketsConfigPath - get old buckets config path. (Only used for migrating old bucket policies) func getOldBucketsConfigPath() (string, error) { configPath, err := getConfigPath() diff --git a/cmd/controller-handlers.go b/cmd/controller-handlers.go index 9d6ca9654..b20c1f2d5 100644 --- a/cmd/controller-handlers.go +++ b/cmd/controller-handlers.go @@ -99,7 +99,7 @@ type HealObjectArgs struct { // HealObjectReply - reply by HealObject RPC. type HealObjectReply struct{} -// HealObject - heals an object, returns nil error upon success. +// HealObject - heal the object. func (c *controllerAPIHandlers) HealObjectHandler(args *HealObjectArgs, reply *GenericReply) error { objAPI := c.ObjectAPI() if objAPI == nil { @@ -111,7 +111,7 @@ func (c *controllerAPIHandlers) HealObjectHandler(args *HealObjectArgs, reply *G return objAPI.HealObject(args.Bucket, args.Object) } -// HealDiskMetadataHandler - heals disks metadata, returns nil error upon success. +// HealObject - heal the object. func (c *controllerAPIHandlers) HealDiskMetadataHandler(args *GenericArgs, reply *GenericReply) error { objAPI := c.ObjectAPI() if objAPI == nil { diff --git a/cmd/routers.go b/cmd/routers.go index f55dfde2e..43b1a9478 100644 --- a/cmd/routers.go +++ b/cmd/routers.go @@ -73,6 +73,10 @@ func newObjectLayer(disks, ignoredDisks []string) (ObjectLayer, error) { err = initEventNotifier(objAPI) fatalIf(err, "Unable to initialize event notification.") + // Initialize and load bucket policies. + err = initBucketPolicies(objAPI) + fatalIf(err, "Unable to load all bucket policies.") + // Success. return objAPI, nil } diff --git a/cmd/storage-errors.go b/cmd/storage-errors.go index 787febf25..8e6fb65f0 100644 --- a/cmd/storage-errors.go +++ b/cmd/storage-errors.go @@ -59,3 +59,6 @@ var errVolumeAccessDenied = errors.New("volume access denied") // errVolumeAccessDenied - cannot access file, insufficient permissions. var errFileAccessDenied = errors.New("file access denied") + +// errVolumeBusy - remote disk is not connected to yet. +var errVolumeBusy = errors.New("volume is busy") diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index d3ed4f5b6..eeed3ce3d 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1298,7 +1298,6 @@ func initTestAPIEndPoints(objLayer ObjectLayer, apiFunctions []string) http.Hand // initialize a new mux router. // goriilla/mux is the library used to register all the routes and handle them. muxRouter := router.NewRouter() - // All object storage operations are registered as HTTP handlers on `objectAPIHandlers`. // When the handlers get a HTTP request they use the underlyting ObjectLayer to perform operations. objLayerMutex.Lock() diff --git a/cmd/web-handlers.go b/cmd/web-handlers.go index 723d0a6b8..917153013 100644 --- a/cmd/web-handlers.go +++ b/cmd/web-handlers.go @@ -128,7 +128,7 @@ func (web *webAPIHandlers) StorageInfo(r *http.Request, args *GenericArgs, reply reply.UIVersion = miniobrowser.UIVersion objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again., please try again."} + return &json2.Error{Message: "Server not initialized"} } reply.StorageInfo = objectAPI.StorageInfo() return nil @@ -147,7 +147,7 @@ func (web *webAPIHandlers) MakeBucket(r *http.Request, args *MakeBucketArgs, rep reply.UIVersion = miniobrowser.UIVersion objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again."} + return &json2.Error{Message: "Server not initialized"} } if err := objectAPI.MakeBucket(args.BucketName); err != nil { return &json2.Error{Message: err.Error()} @@ -176,7 +176,7 @@ func (web *webAPIHandlers) ListBuckets(r *http.Request, args *WebGenericArgs, re } objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again."} + return &json2.Error{Message: "Server not initialized"} } buckets, err := objectAPI.ListBuckets() if err != nil { @@ -228,7 +228,7 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r for { objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again."} + return &json2.Error{Message: "Server not initialized"} } lo, err := objectAPI.ListObjects(args.BucketName, args.Prefix, marker, "/", 1000) if err != nil { @@ -270,7 +270,7 @@ func (web *webAPIHandlers) RemoveObject(r *http.Request, args *RemoveObjectArgs, reply.UIVersion = miniobrowser.UIVersion objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again."} + return &json2.Error{Message: "Server not initialized"} } if err := objectAPI.DeleteObject(args.BucketName, args.ObjectName); err != nil { return &json2.Error{Message: err.Error()} @@ -413,7 +413,7 @@ func (web *webAPIHandlers) Upload(w http.ResponseWriter, r *http.Request) { objectAPI := web.ObjectAPI() if objectAPI == nil { - writeWebErrorResponse(w, errors.New("Server not initialized, please try again.")) + writeWebErrorResponse(w, errors.New("Server not initialized")) return } if _, err := objectAPI.PutObject(bucket, object, -1, r.Body, metadata); err != nil { @@ -469,7 +469,7 @@ func (web *webAPIHandlers) Download(w http.ResponseWriter, r *http.Request) { objectAPI := web.ObjectAPI() if objectAPI == nil { - writeWebErrorResponse(w, errors.New("Server not initialized, please try again.")) + writeWebErrorResponse(w, errors.New("Server not initialized")) return } objInfo, err := objectAPI.GetObjectInfo(bucket, object) @@ -568,7 +568,7 @@ func (web *webAPIHandlers) GetBucketPolicy(r *http.Request, args *GetBucketPolic objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again."} + return &json2.Error{Message: "Server not initialized"} } policyInfo, err := readBucketAccessPolicy(objectAPI, args.BucketName) if err != nil { @@ -638,7 +638,7 @@ func (web *webAPIHandlers) SetBucketPolicy(r *http.Request, args *SetBucketPolic } objectAPI := web.ObjectAPI() if objectAPI == nil { - return &json2.Error{Message: "Server not initialized, please try again."} + return &json2.Error{Message: "Server not initialized"} } bucketP := policy.BucketPolicy(args.Policy)