Merge pull request #1577 from versity/sis/overwrite-locked-objects

feat: prevents locked objects overwrite with CopyObject and CompleteMultipartUpload

s3api/controllers/object-post.go

@@ -325,6 +325,15 @@ func (c S3ApiController) CompleteMultipartUpload(ctx *fiber.Ctx) (*Response, err
 
 	ifMatch, ifNoneMatch := utils.ParsePreconditionMatchHeaders(ctx)
 
+	err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []types.ObjectIdentifier{{Key: &key}}, true, isBucketPublic, c.be, true)
+	if err != nil {
+		return &Response{
+			MetaOpts: &MetaOptions{
+				BucketOwner: parsedAcl.Owner,
+			},
+		}, err
+	}
+
 	res, versid, err := c.be.CompleteMultipartUpload(ctx.Context(),
 		&s3.CompleteMultipartUploadInput{
 			Bucket:   &bucket,

s3api/controllers/object-post_test.go

@@ -479,13 +479,30 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) {
 				err: s3err.GetInvalidChecksumHeaderErr("x-amz-checksum-type"),
 			},
 		},
+		{
+			name: "object is locked",
+			input: testInput{
+				locals:       defaultLocals,
+				body:         validMpBody,
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLocked),
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+					},
+				},
+				err: s3err.GetAPIError(s3err.ErrObjectLocked),
+			},
+		},
 		{
 			name: "backend returns error",
 			input: testInput{
-				locals: defaultLocals,
-				body:   validMpBody,
-				beErr:  s3err.GetAPIError(s3err.ErrNoSuchBucket),
-				beRes:  s3response.CompleteMultipartUploadResult{},
+				locals:       defaultLocals,
+				body:         validMpBody,
+				beErr:        s3err.GetAPIError(s3err.ErrNoSuchBucket),
+				beRes:        s3response.CompleteMultipartUploadResult{},
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
 			},
 			output: testOutput{
 				response: &Response{
@@ -514,6 +531,7 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) {
 				headers: map[string]string{
 					"X-Amz-Mp-Object-Size": "3",
 				},
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
 			},
 			output: testOutput{
 				response: &Response{
@@ -542,6 +560,12 @@ func TestS3ApiController_CompleteMultipartUpload(t *testing.T) {
 				GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
 					return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
 				},
+				GetObjectLockConfigurationFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
+					return nil, tt.input.extraMockErr
+				},
+				GetBucketVersioningFunc: func(contextMoqParam context.Context, bucket string) (s3response.GetBucketVersioningOutput, error) {
+					return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNotImplemented)
+				},
 			}
 
 			ctrl := S3ApiController{

s3api/controllers/object-put.go

@@ -531,6 +531,15 @@ func (c S3ApiController) CopyObject(ctx *fiber.Ctx) (*Response, error) {
 
 	preconditionHdrs := utils.ParsePreconditionHeaders(ctx, utils.WithCopySource())
 
+	err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []types.ObjectIdentifier{{Key: &key}}, true, false, c.be, true)
+	if err != nil {
+		return &Response{
+			MetaOpts: &MetaOptions{
+				BucketOwner: parsedAcl.Owner,
+			},
+		}, err
+	}
+
 	res, err := c.be.CopyObject(ctx.Context(),
 		s3response.CopyObjectInput{
 			Bucket:                      &bucket,

s3api/controllers/object-put_test.go

@@ -891,6 +891,24 @@ func TestS3ApiController_CopyObject(t *testing.T) {
 				err: s3err.GetAPIError(s3err.ErrObjectLockInvalidHeaders),
 			},
 		},
+		{
+			name: "object is locked",
+			input: testInput{
+				locals: defaultLocals,
+				headers: map[string]string{
+					"X-Amz-Copy-Source": "bucket/object",
+				},
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLocked),
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+					},
+				},
+				err: s3err.GetAPIError(s3err.ErrObjectLocked),
+			},
+		},
 		{
 			name: "backend returns error",
 			input: testInput{
@@ -900,6 +918,7 @@ func TestS3ApiController_CopyObject(t *testing.T) {
 				headers: map[string]string{
 					"X-Amz-Copy-Source": "bucket/object",
 				},
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
 			},
 			output: testOutput{
 				response: &Response{
@@ -930,6 +949,7 @@ func TestS3ApiController_CopyObject(t *testing.T) {
 						ETag: utils.GetStringPtr("ETag"),
 					},
 				},
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
 			},
 			output: testOutput{
 				response: &Response{
@@ -959,6 +979,12 @@ func TestS3ApiController_CopyObject(t *testing.T) {
 				GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
 					return nil, s3err.GetAPIError(s3err.ErrAccessDenied)
 				},
+				GetBucketVersioningFunc: func(contextMoqParam context.Context, bucket string) (s3response.GetBucketVersioningOutput, error) {
+					return s3response.GetBucketVersioningOutput{}, s3err.GetAPIError(s3err.ErrNotImplemented)
+				},
+				GetObjectLockConfigurationFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) {
+					return nil, tt.input.extraMockErr
+				},
 			}
 
 			ctrl := S3ApiController{

tests/integration/group-tests.go

@@ -714,7 +714,12 @@ func TestWORMProtection(ts *TestState) {
 	ts.Run(WORMProtection_bucket_object_lock_governance_bypass_delete_multiple)
 	ts.Run(WORMProtection_object_lock_retention_compliance_locked)
 	ts.Run(WORMProtection_object_lock_retention_governance_locked)
-	ts.Run(WORMProtection_object_lock_retention_governance_bypass_overwrite)
+	ts.Run(WORMProtection_object_lock_retention_governance_bypass_overwrite_put)
+	ts.Run(WORMProtection_object_lock_retention_governance_bypass_overwrite_copy)
+	ts.Run(WORMProtection_object_lock_retention_governance_bypass_overwrite_mp)
+	ts.Run(WORMProtection_unable_to_overwrite_locked_object_put)
+	ts.Run(WORMProtection_unable_to_overwrite_locked_object_copy)
+	ts.Run(WORMProtection_unable_to_overwrite_locked_object_mp)
 	ts.Run(WORMProtection_object_lock_retention_governance_bypass_delete)
 	ts.Run(WORMProtection_object_lock_retention_governance_bypass_delete_mul)
 	ts.Run(WORMProtection_object_lock_legal_hold_locked)
@@ -1522,7 +1527,12 @@ func GetIntTests() IntTests {
 		"WORMProtection_bucket_object_lock_governance_bypass_delete_multiple":     WORMProtection_bucket_object_lock_governance_bypass_delete_multiple,
 		"WORMProtection_object_lock_retention_compliance_locked":                  WORMProtection_object_lock_retention_compliance_locked,
 		"WORMProtection_object_lock_retention_governance_locked":                  WORMProtection_object_lock_retention_governance_locked,
-		"WORMProtection_object_lock_retention_governance_bypass_overwrite":        WORMProtection_object_lock_retention_governance_bypass_overwrite,
+		"WORMProtection_object_lock_retention_governance_bypass_overwrite_put":    WORMProtection_object_lock_retention_governance_bypass_overwrite_put,
+		"WORMProtection_object_lock_retention_governance_bypass_overwrite_copy":   WORMProtection_object_lock_retention_governance_bypass_overwrite_copy,
+		"WORMProtection_object_lock_retention_governance_bypass_overwrite_mp":     WORMProtection_object_lock_retention_governance_bypass_overwrite_mp,
+		"WORMProtection_unable_to_overwrite_locked_object_put":                    WORMProtection_unable_to_overwrite_locked_object_put,
+		"WORMProtection_unable_to_overwrite_locked_object_copy":                   WORMProtection_unable_to_overwrite_locked_object_copy,
+		"WORMProtection_unable_to_overwrite_locked_object_mp":                     WORMProtection_unable_to_overwrite_locked_object_mp,
 		"WORMProtection_object_lock_retention_governance_bypass_delete":           WORMProtection_object_lock_retention_governance_bypass_delete,
 		"WORMProtection_object_lock_retention_governance_bypass_delete_mul":       WORMProtection_object_lock_retention_governance_bypass_delete_mul,
 		"WORMProtection_object_lock_legal_hold_locked":                            WORMProtection_object_lock_legal_hold_locked,

tests/integration/tests.go

@@ -17881,8 +17881,8 @@ func WORMProtection_object_lock_retention_governance_locked(s *S3Conf) error {
 	}, withLock())
 }
 
-func WORMProtection_object_lock_retention_governance_bypass_overwrite(s *S3Conf) error {
-	testName := "WORMProtection_object_lock_retention_governance_bypass_overwrite"
+func WORMProtection_object_lock_retention_governance_bypass_overwrite_put(s *S3Conf) error {
+	testName := "WORMProtection_object_lock_retention_governance_bypass_overwrite_put"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
 		object := "my-obj"
 
@@ -17891,24 +17891,14 @@ func WORMProtection_object_lock_retention_governance_bypass_overwrite(s *S3Conf)
 			return err
 		}
 
-		date := time.Now().Add(time.Hour * 3)
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
-			Bucket: &bucket,
-			Key:    &object,
-			Retention: &types.ObjectLockRetention{
-				Mode:            types.ObjectLockRetentionModeGovernance,
-				RetainUntilDate: &date,
-			},
-		})
-		cancel()
+		err = lockObject(s3client, objectLockModeGovernance, bucket, object, "")
 		if err != nil {
 			return err
 		}
 
-		policy := genPolicyDoc("Allow", `"*"`, `["s3:BypassGovernanceRetention"]`, fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket))
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, s.awsID), `["s3:BypassGovernanceRetention"]`, fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket))
 
-		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err = s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
 			Bucket: &bucket,
 			Policy: &policy,
@@ -17928,6 +17918,239 @@ func WORMProtection_object_lock_retention_governance_bypass_overwrite(s *S3Conf)
 	}, withLock())
 }
 
+func WORMProtection_object_lock_retention_governance_bypass_overwrite_mp(s *S3Conf) error {
+	testName := "WORMProtection_object_lock_retention_governance_bypass_overwrite_mp"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		object := "my-obj"
+
+		_, err := putObjects(s3client, []string{object}, bucket)
+		if err != nil {
+			return err
+		}
+
+		err = lockObject(s3client, objectLockModeGovernance, bucket, object, "")
+		if err != nil {
+			return err
+		}
+
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, s.awsID), `["s3:BypassGovernanceRetention"]`, fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket))
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+			Bucket: &bucket,
+			Policy: &policy,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		// overwrite the locked object with a new object with mp
+		mp, err := createMp(s3client, bucket, object)
+		if err != nil {
+			return err
+		}
+
+		dataLen := int64(10)
+
+		parts, _, err := uploadParts(s3client, dataLen, 1, bucket, object, *mp.UploadId)
+		if err != nil {
+			return err
+		}
+		part := parts[0]
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{
+			Bucket: &bucket,
+			Key:    &object,
+			MultipartUpload: &types.CompletedMultipartUpload{
+				Parts: []types.CompletedPart{
+					{
+						ETag:              part.ETag,
+						PartNumber:        part.PartNumber,
+						ChecksumCRC64NVME: part.ChecksumCRC64NVME,
+					},
+				},
+			},
+			UploadId: mp.UploadId,
+		})
+		cancel()
+		return err
+	}, withLock())
+}
+
+func WORMProtection_object_lock_retention_governance_bypass_overwrite_copy(s *S3Conf) error {
+	testName := "WORMProtection_object_lock_retention_governance_bypass_overwrite_copy"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		object := "my-obj"
+
+		_, err := putObjects(s3client, []string{object}, bucket)
+		if err != nil {
+			return err
+		}
+
+		err = lockObject(s3client, objectLockModeGovernance, bucket, object, "")
+		if err != nil {
+			return err
+		}
+
+		policy := genPolicyDoc("Allow", fmt.Sprintf(`"%s"`, s.awsID), `["s3:BypassGovernanceRetention"]`, fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket))
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
+			Bucket: &bucket,
+			Policy: &policy,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		srcObj := "source-object"
+		_, err = putObjects(s3client, []string{srcObj}, bucket)
+		if err != nil {
+			return err
+		}
+
+		// overwrite the locked object with a new object with CopyObject
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{
+			Bucket:     &bucket,
+			Key:        &object,
+			CopySource: getPtr(fmt.Sprintf("%s/%s", bucket, srcObj)),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+		return err
+	}, withLock())
+}
+
+func WORMProtection_unable_to_overwrite_locked_object_put(s *S3Conf) error {
+	testName := "WORMProtection_unable_to_overwrite_locked_object_put"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		object := "my-obj"
+		_, err := putObjects(s3client, []string{object}, bucket)
+		if err != nil {
+			return err
+		}
+
+		err = lockObject(s3client, objectLockModeLegalHold, bucket, object, "")
+		if err != nil {
+			return err
+		}
+
+		_, err = putObjects(s3client, []string{object}, bucket)
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil {
+			return err
+		}
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:                object,
+				removeOnlyLeglHold: true,
+			},
+		})
+	}, withLock())
+}
+
+func WORMProtection_unable_to_overwrite_locked_object_copy(s *S3Conf) error {
+	testName := "WORMProtection_unable_to_overwrite_locked_object_copy"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		object := "my-obj"
+
+		_, err := putObjects(s3client, []string{object}, bucket)
+		if err != nil {
+			return err
+		}
+
+		err = lockObject(s3client, objectLockModeLegalHold, bucket, object, "")
+		if err != nil {
+			return err
+		}
+
+		srcObj := "source-object"
+		_, err = putObjects(s3client, []string{srcObj}, bucket)
+		if err != nil {
+			return err
+		}
+
+		// overwrite the locked object with a new object with CopyObject
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.CopyObject(ctx, &s3.CopyObjectInput{
+			Bucket:     &bucket,
+			Key:        &object,
+			CopySource: getPtr(fmt.Sprintf("%s/%s", bucket, srcObj)),
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil {
+			return err
+		}
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:                object,
+				removeOnlyLeglHold: true,
+			},
+		})
+	}, withLock())
+}
+
+func WORMProtection_unable_to_overwrite_locked_object_mp(s *S3Conf) error {
+	testName := "WORMProtection_unable_to_overwrite_locked_object_mp"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		object := "my-obj"
+
+		_, err := putObjects(s3client, []string{object}, bucket)
+		if err != nil {
+			return err
+		}
+
+		err = lockObject(s3client, objectLockModeLegalHold, bucket, object, "")
+		if err != nil {
+			return err
+		}
+
+		mp, err := createMp(s3client, bucket, object)
+		if err != nil {
+			return err
+		}
+
+		dataLen := int64(10)
+
+		parts, _, err := uploadParts(s3client, dataLen, 1, bucket, object, *mp.UploadId)
+		if err != nil {
+			return err
+		}
+		part := parts[0]
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{
+			Bucket: &bucket,
+			Key:    &object,
+			MultipartUpload: &types.CompletedMultipartUpload{
+				Parts: []types.CompletedPart{
+					{
+						ETag:              part.ETag,
+						PartNumber:        part.PartNumber,
+						ChecksumCRC64NVME: part.ChecksumCRC64NVME,
+					},
+				},
+			},
+			UploadId: mp.UploadId,
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil {
+			return err
+		}
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:                object,
+				removeOnlyLeglHold: true,
+			},
+		})
+	}, withLock())
+}
+
 func WORMProtection_object_lock_retention_governance_bypass_delete(s *S3Conf) error {
 	testName := "WORMProtection_object_lock_retention_governance_bypass_delete"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {

tests/integration/utils.go

@@ -1913,3 +1913,47 @@ func cleanupLockedObjects(client *s3.Client, bucket string, objs []objToDelete)
 	// Wait for all goroutines to finish, return any error encountered
 	return eg.Wait()
 }
+
+type objectLockMode string
+
+const (
+	objectLockModeLegalHold  = "legal-hold"
+	objectLockModeGovernance = "governance"
+	objectLockModeCompliance = "compliance"
+)
+
+func lockObject(client *s3.Client, mode objectLockMode, bucket, object, versionId string) error {
+	ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+	defer cancel()
+	var m types.ObjectLockRetentionMode
+	switch mode {
+	case objectLockModeLegalHold:
+		_, err := client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
+			Bucket:    &bucket,
+			Key:       &object,
+			VersionId: getPtr(versionId),
+			LegalHold: &types.ObjectLockLegalHold{
+				Status: types.ObjectLockLegalHoldStatusOn,
+			},
+		})
+		return err
+	case objectLockModeCompliance:
+		m = types.ObjectLockRetentionModeCompliance
+	case objectLockModeGovernance:
+		m = types.ObjectLockRetentionModeGovernance
+	default:
+		return fmt.Errorf("invalid object lock mode: %s", mode)
+	}
+
+	date := time.Now().Add(time.Hour * 3)
+	_, err := client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
+		Bucket:    &bucket,
+		Key:       &object,
+		VersionId: getPtr(versionId),
+		Retention: &types.ObjectLockRetention{
+			Mode:            m,
+			RetainUntilDate: &date,
+		},
+	})
+	return err
+}