From e28ebc3fdef3d689084fcefae5e0068c9784604c Mon Sep 17 00:00:00 2001 From: Jonathan Buch Date: Wed, 30 Mar 2022 12:47:08 +0200 Subject: [PATCH 1/5] bucket, remove unused function for finding canned policies --- minio/resource_minio_s3_bucket.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/minio/resource_minio_s3_bucket.go b/minio/resource_minio_s3_bucket.go index 1800e465..c86c85ec 100644 --- a/minio/resource_minio_s3_bucket.go +++ b/minio/resource_minio_s3_bucket.go @@ -243,16 +243,6 @@ func minioSetBucketACL(ctx context.Context, bucketConfig *S3MinioBucket) diag.Di return nil } -func findValuePolicies(ctx context.Context, bucketConfig *S3MinioBucket) bool { - policies, _ := bucketConfig.MinioAdmin.ListCannedPolicies(ctx) - for key := range policies { - if key == bucketConfig.MinioACL { - return true - } - } - return false -} - func exportPolicyString(policyStruct BucketPolicy, bucketName string) string { policyJSON, err := json.Marshal(policyStruct) if err != nil { From edc6607480f08da3ef85a66efe311bb09f150642 Mon Sep 17 00:00:00 2001 From: Jonathan Buch Date: Wed, 30 Mar 2022 12:52:28 +0200 Subject: [PATCH 2/5] bucket import, remove unused function mapping names --- minio/import_minio_s3_buckets.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/minio/import_minio_s3_buckets.go b/minio/import_minio_s3_buckets.go index 3b9932fb..e0a7f9b7 100644 --- a/minio/import_minio_s3_buckets.go +++ b/minio/import_minio_s3_buckets.go @@ -6,7 +6,6 @@ import ( awspolicy "github.com/hashicorp/awspolicyequivalence" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/minio/minio-go/v7/pkg/policy" ) func resourceMinioS3BucketImportState( @@ -53,20 +52,3 @@ func policyToACLName(bucketConfig *S3MinioBucket, pol string) string { return "private" } - -func policyNameToACLBucket(policyName string) string { - - policyMapping := map[string]string{ - policy.BucketPolicyReadOnly: "public-read", - policy.BucketPolicyWriteOnly: "public-write", - policy.BucketPolicyReadWrite: "public-read-write", - string(policy.BucketPolicyNone): "private", - "": "private", - } - - x, ok := policyMapping[policyName] - if !ok { - return "custom" - } - return x -} From 6641ae0e38524a7d9fc454afe444349de2e99f41 Mon Sep 17 00:00:00 2001 From: Jonathan Buch Date: Wed, 30 Mar 2022 13:05:39 +0200 Subject: [PATCH 3/5] bucket test, prepare for more tests with different acls --- minio/resource_minio_s3_bucket_test.go | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/minio/resource_minio_s3_bucket_test.go b/minio/resource_minio_s3_bucket_test.go index 51aa7f9d..9fbf814b 100644 --- a/minio/resource_minio_s3_bucket_test.go +++ b/minio/resource_minio_s3_bucket_test.go @@ -125,8 +125,8 @@ func TestAccMinioS3Bucket_generatedName(t *testing.T) { func TestAccMinioS3Bucket_UpdateAcl(t *testing.T) { ri := fmt.Sprintf("tf-test-bucket-%d", acctest.RandInt()) - preConfig := fmt.Sprintf(testAccMinioS3BucketConfigWithACL, ri) - postConfig := fmt.Sprintf(testAccMinioS3BucketConfigWithACLUpdate, ri) + preConfig := testAccMinioS3BucketConfigWithACL(ri, "public-read") + postConfig := testAccMinioS3BucketConfigWithACL(ri, "public") resourceName := "minio_s3_bucket.bucket" resource.ParallelTest(t, resource.TestCase{ @@ -153,7 +153,7 @@ func TestAccMinioS3Bucket_UpdateAcl(t *testing.T) { { ResourceName: resourceName, Config: postConfig, - Check: testAccCheckMinioS3BucketACLInState(resourceName, "private"), + Check: testAccCheckMinioS3BucketACLInState(resourceName, "public"), }, { ResourceName: resourceName, @@ -410,19 +410,14 @@ resource "minio_s3_bucket" "bucket" { `, randInt) } -var testAccMinioS3BucketConfigWithACL = ` +func testAccMinioS3BucketConfigWithACL(name, acl string) string { + return fmt.Sprintf(` resource "minio_s3_bucket" "bucket" { bucket = "%s" - acl = "public-read" + acl = "%s" } -` - -var testAccMinioS3BucketConfigWithACLUpdate = ` -resource "minio_s3_bucket" "bucket" { - bucket = "%s" - acl = "private" +`, name, acl) } -` func testAccMinioS3BucketConfigForceDestroy(bucketName string) string { return fmt.Sprintf(` From d3fd210dc14d07859a9ff822f00896658e5185fe Mon Sep 17 00:00:00 2001 From: Jonathan Buch Date: Wed, 30 Mar 2022 13:06:59 +0200 Subject: [PATCH 4/5] bucket, make sure bucket policy private does not create publicly readable bucket --- minio/import_minio_s3_buckets.go | 3 +-- minio/public_read_policy.go | 24 ------------------------ minio/resource_minio_s3_bucket.go | 6 +++--- 3 files changed, 4 insertions(+), 29 deletions(-) delete mode 100644 minio/public_read_policy.go diff --git a/minio/import_minio_s3_buckets.go b/minio/import_minio_s3_buckets.go index e0a7f9b7..ce6786ff 100644 --- a/minio/import_minio_s3_buckets.go +++ b/minio/import_minio_s3_buckets.go @@ -37,8 +37,7 @@ func resourceMinioS3BucketImportState( func policyToACLName(bucketConfig *S3MinioBucket, pol string) string { defaultPolicies := map[string]string{ - "private": exportPolicyString(ReadOnlyPolicy(bucketConfig), bucketConfig.MinioBucket), - "public-read": exportPolicyString(PublicReadPolicy(bucketConfig), bucketConfig.MinioBucket), + "public-read": exportPolicyString(ReadOnlyPolicy(bucketConfig), bucketConfig.MinioBucket), "public-write": exportPolicyString(WriteOnlyPolicy(bucketConfig), bucketConfig.MinioBucket), "public-read-write": exportPolicyString(ReadWritePolicy(bucketConfig), bucketConfig.MinioBucket), "public": exportPolicyString(PublicPolicy(bucketConfig), bucketConfig.MinioBucket), diff --git a/minio/public_read_policy.go b/minio/public_read_policy.go deleted file mode 100644 index cbc51d16..00000000 --- a/minio/public_read_policy.go +++ /dev/null @@ -1,24 +0,0 @@ -package minio - -import ( - "fmt" - "github.com/minio/minio-go/v7/pkg/policy" - - "github.com/minio/minio-go/v7/pkg/set" -) - -// PublicReadPolicy returns policy where everyone can read objects -func PublicReadPolicy(bucket *S3MinioBucket) BucketPolicy { - return BucketPolicy{ - Version: "2012-10-17", - Statements: []policy.Statement{ - { - Sid: "AllowAllS3Actions", - Effect: "Allow", - Principal: policy.User{AWS: set.CreateStringSet("*")}, - Actions: readListMyObjectActions, - Resources: set.CreateStringSet([]string{fmt.Sprintf("%s%s", awsResourcePrefix, bucket.MinioBucket), fmt.Sprintf("%s%s/*", awsResourcePrefix, bucket.MinioBucket)}...), - }, - }, - } -} diff --git a/minio/resource_minio_s3_bucket.go b/minio/resource_minio_s3_bucket.go index c86c85ec..55662c97 100644 --- a/minio/resource_minio_s3_bucket.go +++ b/minio/resource_minio_s3_bucket.go @@ -220,9 +220,9 @@ func minioDeleteBucket(ctx context.Context, d *schema.ResourceData, meta interfa func minioSetBucketACL(ctx context.Context, bucketConfig *S3MinioBucket) diag.Diagnostics { defaultPolicies := map[string]string{ - "private": exportPolicyString(ReadOnlyPolicy(bucketConfig), bucketConfig.MinioBucket), + "private": "", "public-write": exportPolicyString(WriteOnlyPolicy(bucketConfig), bucketConfig.MinioBucket), - "public-read": exportPolicyString(PublicReadPolicy(bucketConfig), bucketConfig.MinioBucket), + "public-read": exportPolicyString(ReadOnlyPolicy(bucketConfig), bucketConfig.MinioBucket), "public-read-write": exportPolicyString(ReadWritePolicy(bucketConfig), bucketConfig.MinioBucket), "public": exportPolicyString(PublicPolicy(bucketConfig), bucketConfig.MinioBucket), } @@ -233,7 +233,7 @@ func minioSetBucketACL(ctx context.Context, bucketConfig *S3MinioBucket) diag.Di return NewResourceError("Unsupported ACL", bucketConfig.MinioACL, errors.New("(valid acl: private, public-write, public-read, public-read-write, public)")) } - if policyString != "none" { + if policyString != "" { if err := bucketConfig.MinioClient.SetBucketPolicy(ctx, bucketConfig.MinioBucket, policyString); err != nil { log.Printf("%s", NewResourceErrorStr("Unable to set bucket policy", bucketConfig.MinioBucket, err)) return NewResourceError("Unable to set bucket policy", bucketConfig.MinioBucket, err) From e3ff89ac3dca8bd18bc523b0cab912c15cde61b4 Mon Sep 17 00:00:00 2001 From: Jonathan Buch Date: Wed, 30 Mar 2022 13:47:35 +0200 Subject: [PATCH 5/5] bucket, add test to make sure private buckets cannot be read anonymously --- minio/resource_minio_s3_bucket_test.go | 38 ++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/minio/resource_minio_s3_bucket_test.go b/minio/resource_minio_s3_bucket_test.go index 9fbf814b..b9d8bb62 100644 --- a/minio/resource_minio_s3_bucket_test.go +++ b/minio/resource_minio_s3_bucket_test.go @@ -3,6 +3,8 @@ package minio import ( "context" "fmt" + "net/http" + "os" "regexp" "strings" "testing" @@ -208,6 +210,29 @@ func TestAccMinioS3Bucket_forceDestroy(t *testing.T) { }) } +func TestAccMinioS3Bucket_PrivateBucketUnreadable(t *testing.T) { + ri := fmt.Sprintf("tf-test-bucket-%d", acctest.RandInt()) + preConfig := testAccMinioS3BucketConfigWithACL(ri, "private") + resourceName := "minio_s3_bucket.bucket" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviders, + CheckDestroy: testAccCheckMinioS3BucketDestroy, + Steps: []resource.TestStep{ + { + Config: preConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckMinioS3BucketExists(resourceName), + resource.TestCheckResourceAttr( + resourceName, "acl", "private"), + testAccCheckBucketNotReadableAnonymously(resourceName), + ), + }, + }, + }) +} + func TestMinioS3BucketName(t *testing.T) { validDNSNames := []string{ "foobar", @@ -449,3 +474,16 @@ resource "minio_s3_bucket" "test" { bucket_prefix = "tf-test-" } ` + +func testAccCheckBucketNotReadableAnonymously(bucket string) resource.TestCheckFunc { + return func(s *terraform.State) error { + resp, err := http.Get("http://" + os.Getenv("MINIO_ENDPOINT") + "/" + bucket) + if err != nil { + return err + } + if resp.StatusCode != 403 { + return fmt.Errorf("Should not be able to list buckets") + } + return nil + } +}