s3: CopyObject to disallow invalid dest object names (#19110)

By not doing so, objects can risk being in a wrong erasure set if the
destination object name contains e.g. '//'
This commit is contained in:
Anis Eleuch 2024-02-22 19:05:17 +01:00 committed by GitHub
parent 8c53a4405a
commit fa68efb1e7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 73 additions and 52 deletions

View File

@ -1196,6 +1196,13 @@ func (z *erasureServerPools) DeleteObjects(ctx context.Context, bucket string, o
}
func (z *erasureServerPools) CopyObject(ctx context.Context, srcBucket, srcObject, dstBucket, dstObject string, srcInfo ObjectInfo, srcOpts, dstOpts ObjectOptions) (objInfo ObjectInfo, err error) {
if err := checkCopyObjArgs(ctx, srcBucket, srcObject); err != nil {
return ObjectInfo{}, err
}
if err := checkCopyObjArgs(ctx, dstBucket, dstObject); err != nil {
return ObjectInfo{}, err
}
srcObject = encodeDirObject(srcObject)
dstObject = encodeDirObject(dstObject)
@ -1574,7 +1581,7 @@ func (z *erasureServerPools) ListMultipartUploads(ctx context.Context, bucket, p
// Initiate a new multipart upload on a hashedSet based on object name.
func (z *erasureServerPools) NewMultipartUpload(ctx context.Context, bucket, object string, opts ObjectOptions) (*NewMultipartUploadResult, error) {
if err := checkNewMultipartArgs(ctx, bucket, object, z); err != nil {
if err := checkNewMultipartArgs(ctx, bucket, object); err != nil {
return nil, err
}
@ -1621,7 +1628,7 @@ func (z *erasureServerPools) NewMultipartUpload(ctx context.Context, bucket, obj
// Copies a part of an object from source hashedSet to destination hashedSet.
func (z *erasureServerPools) CopyObjectPart(ctx context.Context, srcBucket, srcObject, destBucket, destObject string, uploadID string, partID int, startOffset int64, length int64, srcInfo ObjectInfo, srcOpts, dstOpts ObjectOptions) (PartInfo, error) {
if err := checkNewMultipartArgs(ctx, srcBucket, srcObject, z); err != nil {
if err := checkNewMultipartArgs(ctx, srcBucket, srcObject); err != nil {
return PartInfo{}, err
}
@ -1631,7 +1638,7 @@ func (z *erasureServerPools) CopyObjectPart(ctx context.Context, srcBucket, srcO
// PutObjectPart - writes part of an object to hashedSet based on the object name.
func (z *erasureServerPools) PutObjectPart(ctx context.Context, bucket, object, uploadID string, partID int, data *PutObjReader, opts ObjectOptions) (PartInfo, error) {
if err := checkPutObjectPartArgs(ctx, bucket, object, uploadID, z); err != nil {
if err := checkPutObjectPartArgs(ctx, bucket, object, uploadID); err != nil {
return PartInfo{}, err
}
@ -1663,7 +1670,7 @@ func (z *erasureServerPools) PutObjectPart(ctx context.Context, bucket, object,
}
func (z *erasureServerPools) GetMultipartInfo(ctx context.Context, bucket, object, uploadID string, opts ObjectOptions) (MultipartInfo, error) {
if err := checkListPartsArgs(ctx, bucket, object, uploadID, z); err != nil {
if err := checkListPartsArgs(ctx, bucket, object, uploadID); err != nil {
return MultipartInfo{}, err
}
@ -1694,7 +1701,7 @@ func (z *erasureServerPools) GetMultipartInfo(ctx context.Context, bucket, objec
// ListObjectParts - lists all uploaded parts to an object in hashedSet.
func (z *erasureServerPools) ListObjectParts(ctx context.Context, bucket, object, uploadID string, partNumberMarker int, maxParts int, opts ObjectOptions) (ListPartsInfo, error) {
if err := checkListPartsArgs(ctx, bucket, object, uploadID, z); err != nil {
if err := checkListPartsArgs(ctx, bucket, object, uploadID); err != nil {
return ListPartsInfo{}, err
}
@ -1723,7 +1730,7 @@ func (z *erasureServerPools) ListObjectParts(ctx context.Context, bucket, object
// Aborts an in-progress multipart operation on hashedSet based on the object name.
func (z *erasureServerPools) AbortMultipartUpload(ctx context.Context, bucket, object, uploadID string, opts ObjectOptions) error {
if err := checkAbortMultipartArgs(ctx, bucket, object, uploadID, z); err != nil {
if err := checkAbortMultipartArgs(ctx, bucket, object, uploadID); err != nil {
return err
}
@ -1754,7 +1761,7 @@ func (z *erasureServerPools) AbortMultipartUpload(ctx context.Context, bucket, o
// CompleteMultipartUpload - completes a pending multipart transaction, on hashedSet based on object name.
func (z *erasureServerPools) CompleteMultipartUpload(ctx context.Context, bucket, object, uploadID string, uploadedParts []CompletePart, opts ObjectOptions) (objInfo ObjectInfo, err error) {
if err = checkCompleteMultipartArgs(ctx, bucket, object, uploadID, z); err != nil {
if err = checkCompleteMultipartArgs(ctx, bucket, object, uploadID); err != nil {
return objInfo, err
}

View File

@ -27,6 +27,11 @@ import (
"github.com/minio/minio/internal/logger"
)
// Checks on CopyObject arguments, bucket and object.
func checkCopyObjArgs(ctx context.Context, bucket, object string) error {
return checkBucketAndObjectNames(ctx, bucket, object)
}
// Checks on GetObject arguments, bucket and object.
func checkGetObjArgs(ctx context.Context, bucket, object string) error {
return checkBucketAndObjectNames(ctx, bucket, object)
@ -106,42 +111,42 @@ func checkListMultipartArgs(ctx context.Context, bucket, prefix, keyMarker, uplo
}
// Checks for NewMultipartUpload arguments validity, also validates if bucket exists.
func checkNewMultipartArgs(ctx context.Context, bucket, object string, obj ObjectLayer) error {
return checkObjectArgs(ctx, bucket, object, obj)
func checkNewMultipartArgs(ctx context.Context, bucket, object string) error {
return checkObjectArgs(ctx, bucket, object)
}
func checkMultipartObjectArgs(ctx context.Context, bucket, object, uploadID string, obj ObjectLayer) error {
func checkMultipartObjectArgs(ctx context.Context, bucket, object, uploadID string) error {
_, err := base64.RawURLEncoding.DecodeString(uploadID)
if err != nil {
return MalformedUploadID{
UploadID: uploadID,
}
}
return checkObjectArgs(ctx, bucket, object, obj)
return checkObjectArgs(ctx, bucket, object)
}
// Checks for PutObjectPart arguments validity, also validates if bucket exists.
func checkPutObjectPartArgs(ctx context.Context, bucket, object, uploadID string, obj ObjectLayer) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID, obj)
func checkPutObjectPartArgs(ctx context.Context, bucket, object, uploadID string) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID)
}
// Checks for ListParts arguments validity, also validates if bucket exists.
func checkListPartsArgs(ctx context.Context, bucket, object, uploadID string, obj ObjectLayer) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID, obj)
func checkListPartsArgs(ctx context.Context, bucket, object, uploadID string) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID)
}
// Checks for CompleteMultipartUpload arguments validity, also validates if bucket exists.
func checkCompleteMultipartArgs(ctx context.Context, bucket, object, uploadID string, obj ObjectLayer) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID, obj)
func checkCompleteMultipartArgs(ctx context.Context, bucket, object, uploadID string) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID)
}
// Checks for AbortMultipartUpload arguments validity, also validates if bucket exists.
func checkAbortMultipartArgs(ctx context.Context, bucket, object, uploadID string, obj ObjectLayer) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID, obj)
func checkAbortMultipartArgs(ctx context.Context, bucket, object, uploadID string) error {
return checkMultipartObjectArgs(ctx, bucket, object, uploadID)
}
// Checks Object arguments validity.
func checkObjectArgs(ctx context.Context, bucket, object string, obj ObjectLayer) error {
func checkObjectArgs(ctx context.Context, bucket, object string) error {
// Verify if bucket is valid.
if !isMinioMetaBucketName(bucket) && s3utils.CheckValidBucketNameStrict(bucket) != nil {
return BucketNameInvalid{Bucket: bucket}

View File

@ -2225,10 +2225,19 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusBadRequest,
},
// Test case - 3.
// Test case with new object name is same as object to be copied.
// Test case with invalid source object.
3: {
bucketName: bucketName,
newObjectName: "dir//newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
accessKey: credentials.AccessKey,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusBadRequest,
},
// Test case - 4.
// Test case with new object name is same as object to be copied.
4: {
bucketName: bucketName,
newObjectName: objectName,
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2239,10 +2248,10 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusBadRequest,
},
// Test case - 4.
// Test case - 5.
// Test case with new object name is same as object to be copied.
// But source copy is without leading slash
4: {
5: {
bucketName: bucketName,
newObjectName: objectName,
copySourceSame: true,
@ -2253,10 +2262,10 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusBadRequest,
},
// Test case - 5.
// Test case - 6.
// Test case with new object name is same as object to be copied
// but metadata is updated.
5: {
6: {
bucketName: bucketName,
newObjectName: objectName,
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2270,9 +2279,9 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusOK,
},
// Test case - 6.
// Test case - 7.
// Test case with invalid metadata-directive.
6: {
7: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2286,10 +2295,10 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusBadRequest,
},
// Test case - 7.
// Test case - 8.
// Test case with new object name is same as object to be copied
// fail with BadRequest.
7: {
8: {
bucketName: bucketName,
newObjectName: objectName,
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2304,11 +2313,11 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusBadRequest,
},
// Test case - 8.
// Test case - 9.
// Test case with non-existent source file.
// Case for the purpose of failing `api.ObjectAPI.GetObjectInfo`.
// Expecting the response status code to http.StatusNotFound (404).
8: {
9: {
bucketName: bucketName,
newObjectName: objectName,
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + "non-existent-object"),
@ -2318,11 +2327,11 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusNotFound,
},
// Test case - 9.
// Test case - 10.
// Test case with non-existent source file.
// Case for the purpose of failing `api.ObjectAPI.PutObject`.
// Expecting the response status code to http.StatusNotFound (404).
9: {
10: {
bucketName: "non-existent-destination-bucket",
newObjectName: objectName,
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2332,9 +2341,9 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusNotFound,
},
// Test case - 10.
// Test case - 11.
// Case with invalid AccessKey.
10: {
11: {
bucketName: bucketName,
newObjectName: objectName,
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2343,8 +2352,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
expectedRespStatus: http.StatusForbidden,
},
// Test case - 11, copy metadata from newObject1 with satisfying modified header.
11: {
// Test case - 12, copy metadata from newObject1 with satisfying modified header.
12: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2353,8 +2362,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusOK,
},
// Test case - 12, copy metadata from newObject1 with unsatisfying modified header.
12: {
// Test case - 13, copy metadata from newObject1 with unsatisfying modified header.
13: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2363,8 +2372,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusPreconditionFailed,
},
// Test case - 13, copy metadata from newObject1 with wrong modified header format
13: {
// Test case - 14, copy metadata from newObject1 with wrong modified header format
14: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2373,8 +2382,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusOK,
},
// Test case - 14, copy metadata from newObject1 with satisfying unmodified header.
14: {
// Test case - 15, copy metadata from newObject1 with satisfying unmodified header.
15: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2383,8 +2392,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusOK,
},
// Test case - 15, copy metadata from newObject1 with unsatisfying unmodified header.
15: {
// Test case - 16, copy metadata from newObject1 with unsatisfying unmodified header.
16: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2393,8 +2402,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusPreconditionFailed,
},
// Test case - 16, copy metadata from newObject1 with incorrect unmodified header format.
16: {
// Test case - 17, copy metadata from newObject1 with incorrect unmodified header format.
17: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator + bucketName + SlashSeparator + objectName),
@ -2403,8 +2412,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusOK,
},
// Test case - 17, copy metadata from newObject1 with null versionId
17: {
// Test case - 18, copy metadata from newObject1 with null versionId
18: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator+bucketName+SlashSeparator+objectName) + "?versionId=null",
@ -2412,8 +2421,8 @@ func testAPICopyObjectHandler(obj ObjectLayer, instanceType, bucketName string,
secretKey: credentials.SecretKey,
expectedRespStatus: http.StatusOK,
},
// Test case - 18, copy metadata from newObject1 with non null versionId
18: {
// Test case - 19, copy metadata from newObject1 with non null versionId
19: {
bucketName: bucketName,
newObjectName: "newObject1",
copySourceHeader: url.QueryEscape(SlashSeparator+bucketName+SlashSeparator+objectName) + "?versionId=17",