Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Removed S3 bucket policy chaining because it's not required aft… #161

Conversation

sergeyshevch
Copy link

@sergeyshevch sergeyshevch commented May 22, 2022

…er new AWS provider was released

Description

hashicorp/terraform-provider-aws#7628 was resolved in AWS Provider v3.67.0 so we can remove this policy chaining

Motivation and Context

Currently if we run tfsec linter on code that use this module with custom S3 Bucket policy, it will report that aws-s3-specify-public-access-block is not defined and other errors. If we remove this hack it will works correctly

tfsec output
Result #1 HIGH No public access block so not blocking public acls 
────────────────────────────────────────────────────────────────────────────────
  terraform-aws-modules/s3-bucket/aws/Users/sergeyshevchpers/job/terraform-poc/modules/cur/.terraform/modules/s3_bucket/main.tf:14-40
   via main.tf:3-30 (module.s3_bucket)
────────────────────────────────────────────────────────────────────────────────
   14    resource "aws_s3_bucket" "this" {
   15      count = local.create_bucket ? 1 : 0
   16    
   17      bucket        = var.bucket
   18      bucket_prefix = var.bucket_prefix
   19    
   20      force_destroy       = var.force_destroy
   21      object_lock_enabled = var.object_lock_enabled
   22      tags                = var.tags
   23    
   24      lifecycle {
   25        ignore_changes = [
   26          acceleration_status,
   27          acl,
   28          grant,
   29          cors_rule,
   30          lifecycle_rule,
   31          logging,
   32          object_lock_configuration,
   33          replication_configuration,
   34          request_payer,
   35          server_side_encryption_configuration,
   36          versioning,
   37          website
   38        ]
   39      }
   40    }
────────────────────────────────────────────────────────────────────────────────
          ID aws-s3-block-public-acls
      Impact PUT calls with public ACLs specified can make objects public
  Resolution Enable blocking any PUT calls with a public ACL specified

  More Information
  - https://aquasecurity.github.io/tfsec/v1.21.2/checks/aws/s3/block-public-acls/
  - https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block#block_public_acls
────────────────────────────────────────────────────────────────────────────────


Result #2 HIGH No public access block so not blocking public policies 
────────────────────────────────────────────────────────────────────────────────
  terraform-aws-modules/s3-bucket/aws/Users/sergeyshevchpers/job/terraform-poc/modules/cur/.terraform/modules/s3_bucket/main.tf:14-40
   via main.tf:3-30 (module.s3_bucket)
────────────────────────────────────────────────────────────────────────────────
   14    resource "aws_s3_bucket" "this" {
   15      count = local.create_bucket ? 1 : 0
   16    
   17      bucket        = var.bucket
   18      bucket_prefix = var.bucket_prefix
   19    
   20      force_destroy       = var.force_destroy
   21      object_lock_enabled = var.object_lock_enabled
   22      tags                = var.tags
   23    
   24      lifecycle {
   25        ignore_changes = [
   26          acceleration_status,
   27          acl,
   28          grant,
   29          cors_rule,
   30          lifecycle_rule,
   31          logging,
   32          object_lock_configuration,
   33          replication_configuration,
   34          request_payer,
   35          server_side_encryption_configuration,
   36          versioning,
   37          website
   38        ]
   39      }
   40    }
────────────────────────────────────────────────────────────────────────────────
          ID aws-s3-block-public-policy
      Impact Users could put a policy that allows public access
  Resolution Prevent policies that allow public access being PUT

  More Information
  - https://aquasecurity.github.io/tfsec/v1.21.2/checks/aws/s3/block-public-policy/
  - https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block#block_public_policy
────────────────────────────────────────────────────────────────────────────────


Result #3 HIGH No public access block so not ignoring public acls 
────────────────────────────────────────────────────────────────────────────────
  terraform-aws-modules/s3-bucket/aws/Users/sergeyshevchpers/job/terraform-poc/modules/cur/.terraform/modules/s3_bucket/main.tf:14-40
   via main.tf:3-30 (module.s3_bucket)
────────────────────────────────────────────────────────────────────────────────
   14    resource "aws_s3_bucket" "this" {
   15      count = local.create_bucket ? 1 : 0
   16    
   17      bucket        = var.bucket
   18      bucket_prefix = var.bucket_prefix
   19    
   20      force_destroy       = var.force_destroy
   21      object_lock_enabled = var.object_lock_enabled
   22      tags                = var.tags
   23    
   24      lifecycle {
   25        ignore_changes = [
   26          acceleration_status,
   27          acl,
   28          grant,
   29          cors_rule,
   30          lifecycle_rule,
   31          logging,
   32          object_lock_configuration,
   33          replication_configuration,
   34          request_payer,
   35          server_side_encryption_configuration,
   36          versioning,
   37          website
   38        ]
   39      }
   40    }
────────────────────────────────────────────────────────────────────────────────
          ID aws-s3-ignore-public-acls
      Impact PUT calls with public ACLs specified can make objects public
  Resolution Enable ignoring the application of public ACLs in PUT calls

  More Information
  - https://aquasecurity.github.io/tfsec/v1.21.2/checks/aws/s3/ignore-public-acls/
  - https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block#ignore_public_acls
────────────────────────────────────────────────────────────────────────────────


Result #4 HIGH No public access block so not restricting public buckets 
────────────────────────────────────────────────────────────────────────────────
  terraform-aws-modules/s3-bucket/aws/Users/sergeyshevchpers/job/terraform-poc/modules/cur/.terraform/modules/s3_bucket/main.tf:14-40
   via main.tf:3-30 (module.s3_bucket)
────────────────────────────────────────────────────────────────────────────────
   14    resource "aws_s3_bucket" "this" {
   15      count = local.create_bucket ? 1 : 0
   16    
   17      bucket        = var.bucket
   18      bucket_prefix = var.bucket_prefix
   19    
   20      force_destroy       = var.force_destroy
   21      object_lock_enabled = var.object_lock_enabled
   22      tags                = var.tags
   23    
   24      lifecycle {
   25        ignore_changes = [
   26          acceleration_status,
   27          acl,
   28          grant,
   29          cors_rule,
   30          lifecycle_rule,
   31          logging,
   32          object_lock_configuration,
   33          replication_configuration,
   34          request_payer,
   35          server_side_encryption_configuration,
   36          versioning,
   37          website
   38        ]
   39      }
   40    }
────────────────────────────────────────────────────────────────────────────────
          ID aws-s3-no-public-buckets
      Impact Public buckets can be accessed by anyone
  Resolution Limit the access to public buckets to only the owner or AWS Services (eg; CloudFront)

  More Information
  - https://aquasecurity.github.io/tfsec/v1.21.2/checks/aws/s3/no-public-buckets/
  - https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block#restrict_public_buckets¡
────────────────────────────────────────────────────────────────────────────────


Result #5 LOW Bucket does not have a corresponding public access block. 
────────────────────────────────────────────────────────────────────────────────
  terraform-aws-modules/s3-bucket/aws/Users/sergeyshevchpers/job/terraform-poc/modules/cur/.terraform/modules/s3_bucket/main.tf:14-40
   via main.tf:3-30 (module.s3_bucket)
────────────────────────────────────────────────────────────────────────────────
   14    resource "aws_s3_bucket" "this" {
   15      count = local.create_bucket ? 1 : 0
   16    
   17      bucket        = var.bucket
   18      bucket_prefix = var.bucket_prefix
   19    
   20      force_destroy       = var.force_destroy
   21      object_lock_enabled = var.object_lock_enabled
   22      tags                = var.tags
   23    
   24      lifecycle {
   25        ignore_changes = [
   26          acceleration_status,
   27          acl,
   28          grant,
   29          cors_rule,
   30          lifecycle_rule,
   31          logging,
   32          object_lock_configuration,
   33          replication_configuration,
   34          request_payer,
   35          server_side_encryption_configuration,
   36          versioning,
   37          website
   38        ]
   39      }
   40    }
────────────────────────────────────────────────────────────────────────────────
          ID aws-s3-specify-public-access-block
      Impact Public access policies may be applied to sensitive data buckets
  Resolution Define a aws_s3_bucket_public_access_block for the given bucket to control public access policies

  More Information
  - https://aquasecurity.github.io/tfsec/v1.21.2/checks/aws/s3/specify-public-access-block/
  - https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block#bucket
────────────────────────────────────────────────────────────────────────────────


  timings
  ──────────────────────────────────────────
  disk i/o             107.749µs
  parsing              17.73971ms
  adaptation           134.708µs
  checks               6.306625ms
  total                24.288792ms

  counts
  ──────────────────────────────────────────
  modules downloaded   0
  modules processed    2
  blocks processed     80
  files read           6

  results
  ──────────────────────────────────────────
  passed               2
  ignored              3
  critical             0
  high                 4
  medium               0
  low                  1

  2 passed, 3 ignored, 5 potential problem(s) detected.

Breaking Changes

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

@antonbabenko
Copy link
Member

It is pretty hard to guess how tfsec came to such a conclusion (maybe it is right, but it is rather outside of the scope of the module).

Could please run terraform plan and terraform apply in example/complete with both versions of providers (older and newer) to prove that this PR actually fixes the wrong behavior (aka "a bug") and that we don't need such workaround anymore?

@sergeyshevch
Copy link
Author

@antonbabenko Sure. I will run it in a few days

@github-actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Jul 19, 2022
@github-actions
Copy link

This PR was automatically closed because of stale in 10 days

@github-actions github-actions bot closed this Jul 30, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants