Skip to content

Commit

Permalink
fix: ACL issue (#167)
Browse files Browse the repository at this point in the history
removing ACL block since by default it's private
Adding aws_s3_bucket_ownership_controls to block ACL modifications

Keeping this change
https://github.com/sysdiglabs/terraform-aws-secure-for-cloud/pull/164/files
since AWS has not yet released the feature so public block access is not
enabled by default.
<!--
Thank you for your contribution!

## Testing your PR

You can pinpoint the pr changes as terraform module source with
following format

```
source                    = "github.com/sysdiglabs/terraform-aws-secure-for-cloud//examples/organizational?ref=<BRANCH_NAME>"
```


## General recommendations
Check contribution guidelines at
https://github.com/sysdiglabs/terraform-aws-secure-for-cloud/blob/master/CONTRIBUTE.md#contribution-checklist

For a cleaner PR make sure you follow these recommendations:
- Review modified files and delete small changes that were not intended
and maybe slip the commit.
- Use Pull Request Drafts for visibility on Work-In-Progress branches
and use them on daily mob/pairing for team review
- Unless an external revision is desired, in order to validate or gather
some feedback, you are free to merge as long as **validation checks are
green-lighted**

## Checklist

- [ ] If `test/fixtures/*/main.tf` files are modified, update:
    - [ ] the snippets in the README.md file under root folder.
- [ ] the snippets in the README.md file for the corresponding example.
- [ ] If `examples` folder are modified, update:
    - [ ] README.md file with pertinent changes.
- [ ] `test/fixtures/*/main.tf` in case the snippet needs modifications.
- [ ] If any architectural change has been made, update the diagrams.

-->
  • Loading branch information
hayk99 committed Apr 27, 2023
1 parent bfcfc60 commit 5305746
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 16 deletions.
2 changes: 1 addition & 1 deletion modules/infrastructure/cloudtrail/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ No modules.
| [aws_kms_alias.kms](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_alias) | resource |
| [aws_kms_key.cloudtrail_kms](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_key) | resource |
| [aws_s3_bucket.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket) | resource |
| [aws_s3_bucket_acl.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_acl) | resource |
| [aws_s3_bucket_lifecycle_configuration.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_lifecycle_configuration) | resource |
| [aws_s3_bucket_ownership_controls.owner_enforced](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_ownership_controls) | resource |
| [aws_s3_bucket_policy.cloudtrail_s3](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_policy) | resource |
| [aws_s3_bucket_public_access_block.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |
| [aws_s3_bucket_server_side_encryption_configuration.cloudtrail](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |
Expand Down
14 changes: 8 additions & 6 deletions modules/infrastructure/cloudtrail/s3.tf
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "cloudtrail" {
}
}

resource "aws_s3_bucket_ownership_controls" "owner_enforced" {
bucket = aws_s3_bucket.cloudtrail.id

rule {
object_ownership = "BucketOwnerEnforced"
}
}

resource "aws_s3_bucket_lifecycle_configuration" "cloudtrail" {
bucket = aws_s3_bucket.cloudtrail.id

Expand All @@ -34,12 +42,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "cloudtrail" {
}


resource "aws_s3_bucket_acl" "cloudtrail" {
bucket = aws_s3_bucket.cloudtrail.id
acl = "private"
}


# --------------------------
# iam, acl
# -------------------------
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/organizational/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ variable "sysdig_secure_for_cloud_member_account_id" {
variable "name" {
type = string
description = "Name is the prefix used in the resources will be created"
default = "sfctest-org-ecs"
default = "sfc-test-org-ecs"
}

variable "region" {
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/single-account-apprunner/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ variable "sysdig_secure_api_token" {
variable "name" {
type = string
description = "Name to be assigned to all child resources. A suffix may be added internally when required. Use default value unless you need to install multiple instances"
default = "sfctest-single-app"
default = "sfc-test-single-app"
}

variable "sysdig_secure_url" {
Expand Down
2 changes: 1 addition & 1 deletion use-cases/multiple-accounts-k8s-threat.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ module "cloudtrail_s3_sns_sqs" {

4. Kubernetes Multi-Account **AWS Permissions** to be able to handle S3/SQS operations

Helm Cloud-Connector chart requires specific AWS credentials to be passed by parameter, a new user + access key will
Helm Cloud-Connector chart requires specific AWS credentials to be passed by parameter, a new user + access key will
be created within account, to be able to fetch the events in the S3 bucket (1) or several S3 buckets (2)
<br/><br/>
WIP.
Expand Down
12 changes: 6 additions & 6 deletions use-cases/org-three-way-ecs.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ This accountID will be required in the `SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_I

#### 3.2 (Optional) S3 and Sysdig Workload are in different accounts

If `SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_ID` is different to the account where the S3 is located, we need to allow
If `SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_ID` is different to the account where the S3 is located, we need to allow
cross-account access through a role.

Permission setup for SysdigSecureForCloud-S3AccessRole
Expand All @@ -129,13 +129,13 @@ Permission setup for SysdigSecureForCloud-S3AccessRole

#### 3.3 Cloudtrail S3 ingestion through Event-Forward

When Cloudtrail-SNS is not available, or the Cloudtrail-S3 events are in an account different to the management
When Cloudtrail-SNS is not available, or the Cloudtrail-S3 events are in an account different to the management
account, we will rely on a S3 Event Forwarder, to allow the workload to ingest events more easily.

Secure for Cloud requires an SQS queue from which it can ingest events, and this will provide
Secure for Cloud requires an SQS queue from which it can ingest events, and this will provide
`CLOUDTRAIL_S3_SNS_SQS_ARN` and `CLOUDTRAIL_S3_SNS_SQS_URL` for later installation.

We provide a module to create this
We provide a module to create this
[Cloudtrail S3 bucket event-forwarder into an SNS>SQS](https://github.com/sysdiglabs/terraform-aws-secure-for-cloud/tree/master/modules/infrastructure/cloudtrail_s3-sns-sqs)
but you can do it manually too.

Expand Down Expand Up @@ -167,7 +167,7 @@ Inspect `terraform state list` to gather these two values, `CLOUDTRAIL_S3_SNS_SQ

#### 4. Launch Terraform Manifest

Let's create the Terraform manifest module parametrization, based on `examples/organizational`.
Let's create the Terraform manifest module parametrization, based on `examples/organizational`.
<br/>Get detailed explanation of each variable bellow.

```terraform
Expand Down Expand Up @@ -221,7 +221,7 @@ module "sysdig-sfc" {
existing_cloudtrail_config={
cloudtrail_s3_sns_sqs_arn = "<CLOUDTRAIL_S3_SNS_SQS_ARN>"
cloudtrail_s3_sns_sqs_url = "<CLOUDTRAIL_S3_SNS_SQS_URL>"
# optional, only if CLOUDTRAIL_S3 and SYSDIG_SECURE_FOR_CLOUD_MEMBER_ACCOUNT_ID are in different accounts
cloudtrail_s3_role_arn = "<CLOUDTRAIL_S3_ROLE_ARN>"
}
Expand Down

0 comments on commit 5305746

Please sign in to comment.