From 41f9ab1c69093e91885d099a7f084f9f4130f740 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Fri, 21 Oct 2016 00:09:55 +0100 Subject: [PATCH] Translate storage access denied error to S3 Access Denied response (#3015) --- cmd/api-errors.go | 8 +++ cmd/object-api-getobject_test.go | 116 ++++++++++++++++++++++++++++++- cmd/object-api-multipart_test.go | 4 +- cmd/object-api-putobject_test.go | 2 +- cmd/object-errors.go | 14 ++++ cmd/posix-list-dir-nix.go | 3 + cmd/test-utils_test.go | 4 +- 7 files changed, 145 insertions(+), 6 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index e2520b10a..a1e98a7b3 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -64,6 +64,7 @@ const ( ErrInvalidCopySource ErrInvalidCopyDest ErrInvalidPolicyDocument + ErrInvalidObjectState ErrMalformedXML ErrMissingContentLength ErrMissingContentMD5 @@ -307,6 +308,11 @@ var errorCodeResponse = map[APIErrorCode]APIError{ Description: "The list of parts was not in ascending order. The parts list must be specified in order by part number.", HTTPStatusCode: http.StatusBadRequest, }, + ErrInvalidObjectState: { + Code: "InvalidObjectState", + Description: "The operation is not valid for the current state of the object.", + HTTPStatusCode: http.StatusForbidden, + }, ErrAuthorizationHeaderMalformed: { Code: "AuthorizationHeaderMalformed", Description: "The authorization header is malformed; the region is wrong; expecting 'us-east-1'.", @@ -587,6 +593,8 @@ func toAPIErrorCode(err error) (apiErr APIErrorCode) { apiErr = ErrIncompleteBody case ObjectExistsAsDirectory: apiErr = ErrObjectExistsAsDirectory + case PrefixAccessDenied: + apiErr = ErrAccessDenied case BucketNameInvalid: apiErr = ErrInvalidBucketName case BucketNotFound: diff --git a/cmd/object-api-getobject_test.go b/cmd/object-api-getobject_test.go index b83ecf000..6cb1ad7f3 100644 --- a/cmd/object-api-getobject_test.go +++ b/cmd/object-api-getobject_test.go @@ -20,6 +20,8 @@ import ( "bytes" "fmt" "io" + "os" + "runtime" "strings" "testing" ) @@ -175,9 +177,121 @@ func testGetObject(obj ObjectLayer, instanceType string, t TestErrHandler) { } } +// Wrapper for calling GetObject with permission denied expected +func TestGetObjectPermissionDenied(t *testing.T) { + // Windows doesn't support Chmod under golang + if runtime.GOOS != "windows" { + ExecObjectLayerDiskAlteredTest(t, testGetObjectPermissionDenied) + } +} + +// Test GetObject when we are allowed to access some dirs and objects +func testGetObjectPermissionDenied(obj ObjectLayer, instanceType string, disks []string, t *testing.T) { + // Setup for the tests. + bucketName := getRandomBucketName() + // create bucket. + err := obj.MakeBucket(bucketName) + // Stop the test if creation of the bucket fails. + if err != nil { + t.Fatalf("%s : %s", instanceType, err.Error()) + } + + bytesData := []struct { + byteData []byte + }{ + {generateBytesData(6 * 1024 * 1024)}, + } + // set of inputs for uploading the objects before tests for downloading is done. + putObjectInputs := []struct { + bucketName string + objectName string + contentLength int64 + textData []byte + metaData map[string]string + }{ + {bucketName, "test-object1", int64(len(bytesData[0].byteData)), bytesData[0].byteData, make(map[string]string)}, + {bucketName, "test-object2", int64(len(bytesData[0].byteData)), bytesData[0].byteData, make(map[string]string)}, + {bucketName, "dir/test-object3", int64(len(bytesData[0].byteData)), bytesData[0].byteData, make(map[string]string)}, + } + sha256sum := "" + // iterate through the above set of inputs and upkoad the object. + for i, input := range putObjectInputs { + // uploading the object. + _, err = obj.PutObject(input.bucketName, input.objectName, input.contentLength, bytes.NewBuffer(input.textData), input.metaData, sha256sum) + // if object upload fails stop the test. + if err != nil { + t.Fatalf("Put Object case %d: Error uploading object: %v", i+1, err) + } + } + + // set of empty buffers used to fill GetObject data. + buffers := []*bytes.Buffer{ + new(bytes.Buffer), + } + + // test cases with set of inputs + testCases := []struct { + bucketName string + objectName string + chmodPath string + startOffset int64 + length int64 + // data obtained/fetched from GetObject. + getObjectData *bytes.Buffer + // writer which governs the write into the `getObjectData`. + writer io.Writer + // flag indicating whether the test for given ase should pass. + shouldPass bool + // expected Result. + expectedData []byte + err error + }{ + // Test 1 - chmod 000 bucket/test-object1 + {bucketName, "test-object1", "test-object1", 0, int64(len(bytesData[0].byteData)), buffers[0], buffers[0], false, bytesData[0].byteData, PrefixAccessDenied{Bucket: bucketName, Object: "test-object1"}}, + // Test 2 - chmod 000 bucket/dir/ + {bucketName, "dir/test-object2", "dir", 0, int64(len(bytesData[0].byteData)), buffers[0], buffers[0], false, bytesData[0].byteData, PrefixAccessDenied{Bucket: bucketName, Object: "dir/test-object2"}}, + // Test 3 - chmod 000 bucket/ + {bucketName, "test-object3", "", 0, int64(len(bytesData[0].byteData)), buffers[0], buffers[0], false, bytesData[0].byteData, PrefixAccessDenied{Bucket: bucketName, Object: "test-object3"}}, + } + + for i, testCase := range testCases { + for _, d := range disks { + err = os.Chmod(d+"/"+testCase.bucketName+"/"+testCase.chmodPath, 0) + if err != nil { + t.Fatalf("Test %d, Unable to chmod: %v", i+1, err) + } + } + + err = obj.GetObject(testCase.bucketName, testCase.objectName, testCase.startOffset, testCase.length, testCase.writer) + if err != nil && testCase.shouldPass { + t.Errorf("Test %d: %s: Expected to pass, but failed with: %s", i+1, instanceType, err.Error()) + } + if err == nil && !testCase.shouldPass { + t.Errorf("Test %d: %s: Expected to fail with \"%s\", but passed instead.", i+1, instanceType, testCase.err.Error()) + } + // Failed as expected, but does it fail for the expected reason. + if err != nil && !testCase.shouldPass { + if !strings.Contains(err.Error(), testCase.err.Error()) { + t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but instead failed with error \"%s\" instead.", i+1, instanceType, testCase.err.Error(), err.Error()) + } + } + // Since there are cases for which GetObject fails, this is + // necessary. Test passes as expected, but the output values + // are verified for correctness here. + if err == nil && testCase.shouldPass { + if !bytes.Equal(testCase.expectedData, testCase.getObjectData.Bytes()) { + t.Errorf("Test %d: %s: Data Mismatch: Expected data and the fetched data from GetObject doesn't match.", i+1, instanceType) + } + // empty the buffer so that it can be used to further cases. + testCase.getObjectData.Reset() + } + } + +} + // Wrapper for calling GetObject tests for both XL multiple disks and single node setup. func TestGetObjectDiskNotFound(t *testing.T) { - ExecObjectLayerDiskNotFoundTest(t, testGetObjectDiskNotFound) + ExecObjectLayerDiskAlteredTest(t, testGetObjectDiskNotFound) } // ObjectLayer.GetObject is called with series of cases for valid and erroneous inputs and the result is validated. diff --git a/cmd/object-api-multipart_test.go b/cmd/object-api-multipart_test.go index 634d6e2d6..246f41087 100644 --- a/cmd/object-api-multipart_test.go +++ b/cmd/object-api-multipart_test.go @@ -160,7 +160,7 @@ func testObjectAPIIsUploadIDExists(obj ObjectLayer, instanceType string, t TestE // Wrapper for calling TestPutObjectPartDiskNotFound tests for both XL // write quorum. func TestPutObjectPartDiskNotFound(t *testing.T) { - ExecObjectLayerDiskNotFoundTest(t, testPutObjectPartDiskNotFound) + ExecObjectLayerDiskAlteredTest(t, testPutObjectPartDiskNotFound) } // testPutObjectPartDiskNotFound - Tests validate PutObjectPart behavior when disks go offline. @@ -1274,7 +1274,7 @@ func testListMultipartUploads(obj ObjectLayer, instanceType string, t TestErrHan // Wrapper for calling TestListObjectPartsDiskNotFound tests for both XL multiple disks and single node setup. func TestListObjectPartsDiskNotFound(t *testing.T) { - ExecObjectLayerDiskNotFoundTest(t, testListObjectPartsDiskNotFound) + ExecObjectLayerDiskAlteredTest(t, testListObjectPartsDiskNotFound) } // testListObjectParts - Tests validate listing of object parts when disks go offline. diff --git a/cmd/object-api-putobject_test.go b/cmd/object-api-putobject_test.go index ed33b85bb..8ca827be6 100644 --- a/cmd/object-api-putobject_test.go +++ b/cmd/object-api-putobject_test.go @@ -185,7 +185,7 @@ func testObjectAPIPutObject(obj ObjectLayer, instanceType string, t TestErrHandl // Wrapper for calling PutObject tests for both XL multiple disks case // when quorum is not available. func TestObjectAPIPutObjectDiskNotFound(t *testing.T) { - ExecObjectLayerDiskNotFoundTest(t, testObjectAPIPutObjectDiskNotFOund) + ExecObjectLayerDiskAlteredTest(t, testObjectAPIPutObjectDiskNotFOund) } // Tests validate correctness of PutObject. diff --git a/cmd/object-errors.go b/cmd/object-errors.go index 07590e694..4560e52ea 100644 --- a/cmd/object-errors.go +++ b/cmd/object-errors.go @@ -46,6 +46,13 @@ func toObjectErr(err error, params ...string) error { } case errDiskFull: err = StorageFull{} + case errFileAccessDenied: + if len(params) >= 2 { + err = PrefixAccessDenied{ + Bucket: params[0], + Object: params[1], + } + } case errIsNotRegular, errFileAccessDenied: if len(params) >= 2 { err = ObjectExistsAsDirectory{ @@ -145,6 +152,13 @@ func (e ObjectExistsAsDirectory) Error() string { return "Object exists on : " + e.Bucket + " as directory " + e.Object } +//PrefixAccessDenied object access is denied. +type PrefixAccessDenied GenericError + +func (e PrefixAccessDenied) Error() string { + return "Prefix access is denied: " + e.Bucket + "/" + e.Object +} + // BucketExists bucket exists. type BucketExists GenericError diff --git a/cmd/posix-list-dir-nix.go b/cmd/posix-list-dir-nix.go index f6af62a08..c58e0b6b5 100644 --- a/cmd/posix-list-dir-nix.go +++ b/cmd/posix-list-dir-nix.go @@ -115,6 +115,9 @@ func readDir(dirPath string) (entries []string, err error) { if os.IsNotExist(err) { return nil, errFileNotFound } + if os.IsPermission(err) { + return nil, errFileAccessDenied + } // File path cannot be verified since one of the parents is a file. if strings.Contains(err.Error(), "not a directory") { diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 8c114ddba..b5fad4a5c 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -1792,9 +1792,9 @@ func ExecObjectLayerTest(t TestErrHandler, objTest objTestType) { defer removeRoots(append(fsDirs, fsDir)) } -// ExecObjectLayerDiskNotFoundTest - executes object layer tests while deleting +// ExecObjectLayerDiskAlteredTest - executes object layer tests while altering // disks in between tests. Creates XL ObjectLayer instance and runs test for XL layer. -func ExecObjectLayerDiskNotFoundTest(t *testing.T, objTest objTestDiskNotFoundType) { +func ExecObjectLayerDiskAlteredTest(t *testing.T, objTest objTestDiskNotFoundType) { objLayer, fsDirs, err := prepareXL() if err != nil { t.Fatalf("Initialization of object layer failed for XL setup: %s", err)