diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go index 49eaa1d45..3e3bce3c7 100644 --- a/cmd/admin-handlers-users_test.go +++ b/cmd/admin-handlers-users_test.go @@ -1261,6 +1261,20 @@ func (c *check) mustListBuckets(ctx context.Context, client *minio.Client) { } } +func (c *check) mustNotDelete(ctx context.Context, client *minio.Client, bucket string, vid string) { + c.Helper() + + err := client.RemoveObject(ctx, bucket, "some-object", minio.RemoveObjectOptions{VersionID: vid}) + if err == nil { + c.Fatalf("user must not be allowed to delete") + } + + err = client.RemoveObject(ctx, bucket, "some-object", minio.RemoveObjectOptions{}) + if err != nil { + c.Fatal("user must be able to create delete marker") + } +} + func (c *check) mustDownload(ctx context.Context, client *minio.Client, bucket string) { c.Helper() rd, err := client.GetObject(ctx, bucket, "some-object", minio.GetObjectOptions{}) @@ -1272,6 +1286,19 @@ func (c *check) mustDownload(ctx context.Context, client *minio.Client, bucket s } } +func (c *check) mustUploadReturnVersions(ctx context.Context, client *minio.Client, bucket string) []string { + c.Helper() + versions := []string{} + for i := 0; i < 5; i++ { + ui, err := client.PutObject(ctx, bucket, "some-object", bytes.NewBuffer([]byte("stuff")), 5, minio.PutObjectOptions{}) + if err != nil { + c.Fatalf("upload did not succeed got %#v", err) + } + versions = append(versions, ui.VersionID) + } + return versions +} + func (c *check) mustUpload(ctx context.Context, client *minio.Client, bucket string) { c.Helper() _, err := client.PutObject(ctx, bucket, "some-object", bytes.NewBuffer([]byte("stuff")), 5, minio.PutObjectOptions{}) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 99418067c..07e5ebc86 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -300,6 +300,17 @@ func checkRequestAuthType(ctx context.Context, r *http.Request, action policy.Ac return s3Err } +// checkRequestAuthTypeWithVID is similar to checkRequestAuthType +// passes versionID additionally. +func checkRequestAuthTypeWithVID(ctx context.Context, r *http.Request, action policy.Action, bucketName, objectName, versionID string) (s3Err APIErrorCode) { + logger.GetReqInfo(ctx).BucketName = bucketName + logger.GetReqInfo(ctx).ObjectName = objectName + logger.GetReqInfo(ctx).VersionID = versionID + + _, _, s3Err = checkRequestAuthTypeCredential(ctx, r, action) + return s3Err +} + func authenticateRequest(ctx context.Context, r *http.Request, action policy.Action) (s3Err APIErrorCode) { if logger.GetReqInfo(ctx) == nil { logger.LogIf(ctx, errors.New("unexpected context.Context does not have a logger.ReqInfo"), logger.Minio) diff --git a/cmd/bucket-handlers.go b/cmd/bucket-handlers.go index d4f37ccc5..17cb612dd 100644 --- a/cmd/bucket-handlers.go +++ b/cmd/bucket-handlers.go @@ -495,7 +495,7 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter, vc, _ := globalBucketVersioningSys.Get(bucket) oss := make([]*objSweeper, len(deleteObjectsReq.Objects)) for index, object := range deleteObjectsReq.Objects { - if apiErrCode := checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName); apiErrCode != ErrNone { + if apiErrCode := checkRequestAuthTypeWithVID(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName, object.VersionID); apiErrCode != ErrNone { if apiErrCode == ErrSignatureDoesNotMatch || apiErrCode == ErrInvalidAccessKeyID { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(apiErrCode), r.URL) return diff --git a/cmd/sts-handlers_test.go b/cmd/sts-handlers_test.go index 0bfce0907..03871065b 100644 --- a/cmd/sts-handlers_test.go +++ b/cmd/sts-handlers_test.go @@ -36,6 +36,7 @@ func runAllIAMSTSTests(suite *TestSuiteIAM, c *check) { // The STS for root test needs to be the first one after setup. suite.TestSTSForRoot(c) suite.TestSTS(c) + suite.TestSTSWithDenyDeleteVersion(c) suite.TestSTSWithTags(c) suite.TestSTSWithGroupPolicy(c) suite.TearDownSuite(c) @@ -73,6 +74,101 @@ func TestIAMInternalIDPSTSServerSuite(t *testing.T) { } } +func (s *TestSuiteIAM) TestSTSWithDenyDeleteVersion(c *check) { + ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) + defer cancel() + + bucket := getRandomBucketName() + err := s.client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{ObjectLocking: true}) + if err != nil { + c.Fatalf("bucket creat error: %v", err) + } + + // Create policy, user and associate policy + policy := "mypolicy" + policyBytes := []byte(fmt.Sprintf(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "ObjectActionsRW", + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:PutObjectTagging", + "s3:AbortMultipartUpload", + "s3:DeleteObject", + "s3:GetObject", + "s3:GetObjectTagging", + "s3:GetObjectVersion", + "s3:ListMultipartUploadParts" + ], + "Resource": [ + "arn:aws:s3:::%s/*" + ] + }, + { + "Sid": "DenyDeleteVersionAction", + "Effect": "Deny", + "Action": [ + "s3:DeleteObjectVersion" + ], + "Resource": [ + "arn:aws:s3:::%s/*" + ] + } + ] + } +`, bucket, bucket)) + + err = s.adm.AddCannedPolicy(ctx, policy, policyBytes) + if err != nil { + c.Fatalf("policy add error: %v", err) + } + + accessKey, secretKey := mustGenerateCredentials(c) + err = s.adm.SetUser(ctx, accessKey, secretKey, madmin.AccountEnabled) + if err != nil { + c.Fatalf("Unable to set user: %v", err) + } + + err = s.adm.SetPolicy(ctx, policy, accessKey, false) + if err != nil { + c.Fatalf("Unable to set policy: %v", err) + } + + // confirm that the user is able to access the bucket + uClient := s.getUserClient(c, accessKey, secretKey, "") + versions := c.mustUploadReturnVersions(ctx, uClient, bucket) + c.mustNotDelete(ctx, uClient, bucket, versions[0]) + + assumeRole := cr.STSAssumeRole{ + Client: s.TestSuiteCommon.client, + STSEndpoint: s.endPoint, + Options: cr.STSAssumeRoleOptions{ + AccessKey: accessKey, + SecretKey: secretKey, + Location: "", + }, + } + + value, err := assumeRole.Retrieve() + if err != nil { + c.Fatalf("err calling assumeRole: %v", err) + } + + minioClient, err := minio.New(s.endpoint, &minio.Options{ + Creds: cr.NewStaticV4(value.AccessKeyID, value.SecretAccessKey, value.SessionToken), + Secure: s.secure, + Transport: s.TestSuiteCommon.client.Transport, + }) + if err != nil { + c.Fatalf("Error initializing client: %v", err) + } + + versions = c.mustUploadReturnVersions(ctx, minioClient, bucket) + c.mustNotDelete(ctx, minioClient, bucket, versions[0]) +} + func (s *TestSuiteIAM) TestSTSWithTags(c *check) { ctx, cancel := context.WithTimeout(context.Background(), testDefaultTimeout) defer cancel() @@ -171,9 +267,9 @@ func (s *TestSuiteIAM) TestSTSWithTags(c *check) { } // Validate sts creds can access the object - c.mustPutObjectWithTags(ctx, uClient, bucket, object) - c.mustGetObject(ctx, uClient, bucket, object) - c.mustHeadObject(ctx, uClient, bucket, object, 2) + c.mustPutObjectWithTags(ctx, minioClient, bucket, object) + c.mustGetObject(ctx, minioClient, bucket, object) + c.mustHeadObject(ctx, minioClient, bucket, object, 2) // Validate that the client can remove objects if err = minioClient.RemoveObjectTagging(ctx, bucket, object, minio.RemoveObjectTaggingOptions{}); err != nil {