Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor/cloud breach s3 #214

Merged

Conversation

andrew-aiken
Copy link
Contributor

@andrew-aiken andrew-aiken commented Aug 30, 2023

Overview of Changes

Testing

Tested with version 1.2.0 & 1.5.6 of Terraform

@jdearmas jdearmas self-assigned this Sep 5, 2023
Copy link
Contributor Author

@andrew-aiken andrew-aiken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments to the PR to show changes


}
#S3 Full Access Policy
data "aws_iam_policy" "s3-full-access" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant

"Sid": ""
Version = "2012-10-17",
Statement = [
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsonencode


#IAM Role Policy Attachment
resource "aws_iam_role_policy_attachment" "cg-banking-WAF-Role-policy-attachment-s3" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant

from_port = 22
to_port = 22
protocol = "tcp"
cidr_blocks = var.cg_whitelist
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

subnet_id = aws_subnet.cg-public-subnet-1.id
associate_public_ip_address = true

vpc_security_group_ids = [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting

default_tags = {
Stack = var.stack-name
Scenario = var.scenario-name
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locals to combine default tags & other variables

bucket = "${aws_s3_bucket.cg-cardholder-data-bucket.id}"
key = "cardholder_data_primary.csv"
source = "../assets/cardholder_data_primary.csv"
tags = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update aws_s3_bucket_object -> aws_s3_object

Combined all objects into a single looped resources

@@ -1,32 +1,53 @@
#Required: AWS Profile
variable "profile" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add type and description

version = "~> 4.16"
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add minimum TF versions (linting)

@@ -1,61 +1,63 @@
resource "aws_vpc" "cg-vpc" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting

#Required: User's Public IP Address(es)
variable "cg_whitelist" {
default = "../whitelist.txt"
description = "User's public IP address, pulled from the file ../whitelist.txt"
default = ["0.0.0.0/0"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All IPs by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could switch this to something else (0.0.0.0/32) Its more to switch to a default value that would be valid (list(string))

@andrew-aiken andrew-aiken requested a review from jdearmas November 5, 2023 17:18
@jdearmas jdearmas merged commit cca0cb5 into RhinoSecurityLabs:master Jan 4, 2024
@andrew-aiken andrew-aiken deleted the refactor/cloud-breach-s3 branch January 4, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants