From cca0cb5307ca625b9688e378fbd669ed977ab763 Mon Sep 17 00:00:00 2001
From: Andrew <61951628+andrew-aiken@users.noreply.github.com>
Date: Thu, 4 Jan 2024 15:13:31 -0500
Subject: [PATCH] Refactor/cloud breach s3 (#214)

* Fixes #198
---
 .github/pull_request_template.md              |   5 +
 .../cloud_breach_s3/terraform/data_sources.tf |   8 +-
 scenarios/cloud_breach_s3/terraform/ec2.tf    | 191 +++++++++---------
 scenarios/cloud_breach_s3/terraform/locals.tf |  15 ++
 .../cloud_breach_s3/terraform/outputs.tf      |   8 +-
 .../cloud_breach_s3/terraform/provider.tf     |   6 +-
 scenarios/cloud_breach_s3/terraform/s3.tf     |  74 ++-----
 .../cloud_breach_s3/terraform/variables.tf    |  39 +++-
 .../cloud_breach_s3/terraform/versions.tf     |  10 +
 scenarios/cloud_breach_s3/terraform/vpc.tf    |  80 ++++----
 10 files changed, 224 insertions(+), 212 deletions(-)
 create mode 100644 .github/pull_request_template.md
 create mode 100644 scenarios/cloud_breach_s3/terraform/locals.tf
 create mode 100644 scenarios/cloud_breach_s3/terraform/versions.tf

diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
new file mode 100644
index 00000000..bd3b4675
--- /dev/null
+++ b/.github/pull_request_template.md
@@ -0,0 +1,5 @@
+#### Overview of Changes
+- What was changed
+
+#### Testing
+Was this tested with different Terraform versions?
diff --git a/scenarios/cloud_breach_s3/terraform/data_sources.tf b/scenarios/cloud_breach_s3/terraform/data_sources.tf
index e06e5681..0c69f6f7 100644
--- a/scenarios/cloud_breach_s3/terraform/data_sources.tf
+++ b/scenarios/cloud_breach_s3/terraform/data_sources.tf
@@ -1,8 +1,2 @@
 #AWS Account Id
-data "aws_caller_identity" "aws-account-id" {
-
-}
-#S3 Full Access Policy
-data "aws_iam_policy" "s3-full-access" {
-  arn = "arn:aws:iam::aws:policy/AmazonS3FullAccess"
-}
\ No newline at end of file
+data "aws_caller_identity" "aws-account-id" {}
diff --git a/scenarios/cloud_breach_s3/terraform/ec2.tf b/scenarios/cloud_breach_s3/terraform/ec2.tf
index b064fa52..b8dfacf5 100644
--- a/scenarios/cloud_breach_s3/terraform/ec2.tf
+++ b/scenarios/cloud_breach_s3/terraform/ec2.tf
@@ -1,122 +1,129 @@
 #IAM Role
 resource "aws_iam_role" "cg-banking-WAF-Role" {
   name = "cg-banking-WAF-Role-${var.cgid}"
-  assume_role_policy = <<EOF
-{
-  "Version": "2012-10-17",
-  "Statement": [
+  assume_role_policy = jsonencode(
     {
-      "Action": "sts:AssumeRole",
-      "Principal": {
-        "Service": "ec2.amazonaws.com"
-      },
-      "Effect": "Allow",
-      "Sid": ""
+      Version = "2012-10-17",
+      Statement = [
+        {
+          Action = "sts:AssumeRole"
+          Principal = {
+            Service = "ec2.amazonaws.com"
+          }
+          Effect = "Allow"
+          Sid    = ""
+        }
+      ]
     }
+  )
+
+  managed_policy_arns = [
+    "arn:aws:iam::aws:policy/AmazonS3FullAccess"
   ]
-}
-EOF
-  tags = {
-      Name = "cg-banking-WAF-Role-${var.cgid}"
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
-  }
-}
 
-#IAM Role Policy Attachment
-resource "aws_iam_role_policy_attachment" "cg-banking-WAF-Role-policy-attachment-s3" {
-  role = "${aws_iam_role.cg-banking-WAF-Role.name}"
-  policy_arn = "${data.aws_iam_policy.s3-full-access.arn}"
+  tags = merge(local.default_tags, {
+    Name = "cg-banking-WAF-Role-${var.cgid}"
+  })
 }
 
+
 #IAM Instance Profile
 resource "aws_iam_instance_profile" "cg-ec2-instance-profile" {
   name = "cg-ec2-instance-profile-${var.cgid}"
-  role = "${aws_iam_role.cg-banking-WAF-Role.name}"
+  role = aws_iam_role.cg-banking-WAF-Role.name
 }
 
 #Security Groups
 resource "aws_security_group" "cg-ec2-ssh-security-group" {
-  name = "cg-ec2-ssh-${var.cgid}"
+  name        = "cg-ec2-ssh-${var.cgid}"
   description = "CloudGoat ${var.cgid} Security Group for EC2 Instance over SSH"
-  vpc_id = "${aws_vpc.cg-vpc.id}"
+  vpc_id      = aws_vpc.cg-vpc.id
+
   ingress {
-      from_port = 22
-      to_port = 22
-      protocol = "tcp"
-      cidr_blocks = var.cg_whitelist
+    from_port   = 22
+    to_port     = 22
+    protocol    = "tcp"
+    cidr_blocks = var.cg_whitelist
   }
+
   egress {
-      from_port = 0
-      to_port = 0
-      protocol = "-1"
-      cidr_blocks = [
-          "0.0.0.0/0"
-      ]
+    from_port = 0
+    to_port   = 0
+    protocol  = "-1"
+    cidr_blocks = [
+      "0.0.0.0/0"
+    ]
   }
-  tags = {
+
+  tags = merge(local.default_tags, {
     Name = "cg-ec2-ssh-${var.cgid}"
-    Stack = "${var.stack-name}"
-    Scenario = "${var.scenario-name}"
-  }
+  })
 }
+
 resource "aws_security_group" "cg-ec2-http-security-group" {
-  name = "cg-ec2-http-${var.cgid}"
+  name        = "cg-ec2-http-${var.cgid}"
   description = "CloudGoat ${var.cgid} Security Group for EC2 Instance over HTTP"
-  vpc_id = "${aws_vpc.cg-vpc.id}"
+  vpc_id      = aws_vpc.cg-vpc.id
+
   ingress {
-      from_port = 80
-      to_port = 80
-      protocol = "tcp"
-      cidr_blocks = var.cg_whitelist
+    from_port   = 80
+    to_port     = 80
+    protocol    = "tcp"
+    cidr_blocks = var.cg_whitelist
   }
+
   egress {
-      from_port = 0
-      to_port = 0
-      protocol = "-1"
-      cidr_blocks = [
-          "0.0.0.0/0"
-      ]
+    from_port = 0
+    to_port   = 0
+    protocol  = "-1"
+    cidr_blocks = [
+      "0.0.0.0/0"
+    ]
   }
-  tags = {
+
+  tags = merge(local.default_tags, {
     Name = "cg-ec2-http-${var.cgid}"
-    Stack = "${var.stack-name}"
-    Scenario = "${var.scenario-name}"
-  }
+  })
 }
+
 #AWS Key Pair
 resource "aws_key_pair" "cg-ec2-key-pair" {
-  key_name = "cg-ec2-key-pair-${var.cgid}"
-  public_key = "${file(var.ssh-public-key-for-ec2)}"
+  key_name   = "cg-ec2-key-pair-${var.cgid}"
+  public_key = file(var.ssh-public-key-for-ec2)
 }
+
 #EC2 Instance
 resource "aws_instance" "ec2-vulnerable-proxy-server" {
-    ami = "ami-0a313d6098716f372"
-    instance_type = "t2.micro"
-    iam_instance_profile = "${aws_iam_instance_profile.cg-ec2-instance-profile.name}"
-    subnet_id = "${aws_subnet.cg-public-subnet-1.id}"
-    associate_public_ip_address = true
-    vpc_security_group_ids = [
-        "${aws_security_group.cg-ec2-ssh-security-group.id}",
-        "${aws_security_group.cg-ec2-http-security-group.id}"
-    ]
-    key_name = "${aws_key_pair.cg-ec2-key-pair.key_name}"
-    root_block_device {
-        volume_type = "gp2"
-        volume_size = 8
-        delete_on_termination = true
-    }
-    provisioner "file" {
-      source = "../assets/proxy.com"
-      destination = "/home/ubuntu/proxy.com"
-      connection {
-        type = "ssh"
-        user = "ubuntu"
-        private_key = "${file(var.ssh-private-key-for-ec2)}"
-        host = self.public_ip
-      }
+  ami                         = "ami-0a313d6098716f372"
+  instance_type               = "t2.micro"
+  iam_instance_profile        = aws_iam_instance_profile.cg-ec2-instance-profile.name
+  subnet_id                   = aws_subnet.cg-public-subnet-1.id
+  associate_public_ip_address = true
+
+  vpc_security_group_ids = [
+    aws_security_group.cg-ec2-ssh-security-group.id,
+    aws_security_group.cg-ec2-http-security-group.id
+  ]
+
+  key_name = aws_key_pair.cg-ec2-key-pair.key_name
+  root_block_device {
+    volume_type           = "gp2"
+    volume_size           = 8
+    delete_on_termination = true
+  }
+
+  provisioner "file" {
+    source      = "../assets/proxy.com"
+    destination = "/home/ubuntu/proxy.com"
+    connection {
+      type        = "ssh"
+      user        = "ubuntu"
+      private_key = file(var.ssh-private-key-for-ec2)
+      host        = self.public_ip
     }
-    user_data = <<-EOF
+  }
+
+  user_data = <<-EOF
         #!/bin/bash
         apt-get update
         apt-get install -y nginx
@@ -125,14 +132,12 @@ resource "aws_instance" "ec2-vulnerable-proxy-server" {
         rm /etc/nginx/sites-enabled/default
         systemctl restart nginx
         EOF
-    volume_tags = {
-        Name = "CloudGoat ${var.cgid} EC2 Instance Root Device"
-        Stack = "${var.stack-name}"
-        Scenario = "${var.scenario-name}"
-    }
-    tags = {
-        Name = "ec2-vulnerable-proxy-server-${var.cgid}"
-        Stack = "${var.stack-name}"
-        Scenario = "${var.scenario-name}"
-    }
-}
\ No newline at end of file
+
+  volume_tags = merge(local.default_tags, {
+    Name = "CloudGoat ${var.cgid} EC2 Instance Root Device"
+  })
+
+  tags = merge(local.default_tags, {
+    Name = "ec2-vulnerable-proxy-server-${var.cgid}"
+  })
+}
diff --git a/scenarios/cloud_breach_s3/terraform/locals.tf b/scenarios/cloud_breach_s3/terraform/locals.tf
new file mode 100644
index 00000000..116d1ca9
--- /dev/null
+++ b/scenarios/cloud_breach_s3/terraform/locals.tf
@@ -0,0 +1,15 @@
+locals {
+  s3_bucket_suffix = replace(var.cgid, "/[^a-z0-9-.]/", "-")
+
+  s3_objects = [
+    "cardholder_data_primary.csv",
+    "cardholder_data_secondary.csv",
+    "cardholders_corporate.csv",
+    "goat.png"
+  ]
+
+  default_tags = {
+    Stack    = var.stack-name
+    Scenario = var.scenario-name
+  }
+}
diff --git a/scenarios/cloud_breach_s3/terraform/outputs.tf b/scenarios/cloud_breach_s3/terraform/outputs.tf
index 7ddfe788..13ce73eb 100644
--- a/scenarios/cloud_breach_s3/terraform/outputs.tf
+++ b/scenarios/cloud_breach_s3/terraform/outputs.tf
@@ -1,8 +1,8 @@
-
 #Required: Always output the AWS Account ID
 output "cloudgoat_output_aws_account_id" {
-  value = "${data.aws_caller_identity.aws-account-id.account_id}"
+  value = data.aws_caller_identity.aws-account-id.account_id
 }
+
 output "cloudgoat_output_target_ec2_server_ip" {
-  value = "${aws_instance.ec2-vulnerable-proxy-server.public_ip}"
-}
\ No newline at end of file
+  value = aws_instance.ec2-vulnerable-proxy-server.public_ip
+}
diff --git a/scenarios/cloud_breach_s3/terraform/provider.tf b/scenarios/cloud_breach_s3/terraform/provider.tf
index 2851ee7d..1e9ce4cf 100644
--- a/scenarios/cloud_breach_s3/terraform/provider.tf
+++ b/scenarios/cloud_breach_s3/terraform/provider.tf
@@ -1,4 +1,4 @@
 provider "aws" {
-  profile = "${var.profile}"
-  region = "${var.region}"
-}
\ No newline at end of file
+  profile = var.profile
+  region  = var.region
+}
diff --git a/scenarios/cloud_breach_s3/terraform/s3.tf b/scenarios/cloud_breach_s3/terraform/s3.tf
index cd0bd30b..112d4d04 100644
--- a/scenarios/cloud_breach_s3/terraform/s3.tf
+++ b/scenarios/cloud_breach_s3/terraform/s3.tf
@@ -1,63 +1,23 @@
 #Secret S3 Bucket
-locals {
-  # Ensure the bucket suffix doesn't contain invalid characters
-  # "Bucket names can consist only of lowercase letters, numbers, dots (.), and hyphens (-)."
-  # (per https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html) 
-  bucket_suffix = replace(var.cgid, "/[^a-z0-9-.]/", "-")
-}
-
 resource "aws_s3_bucket" "cg-cardholder-data-bucket" {
-  bucket = "cg-cardholder-data-bucket-${local.bucket_suffix}"
+  bucket        = "cg-cardholder-data-bucket-${local.s3_bucket_suffix}"
   force_destroy = true
-  tags = {
-      Name = "cg-cardholder-data-bucket-${local.bucket_suffix}"
-      Description = "CloudGoat ${var.cgid} S3 Bucket used for storing sensitive cardholder data."
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
-  }
-}
-resource "aws_s3_bucket_object" "cardholder-data-primary" {
-  bucket = "${aws_s3_bucket.cg-cardholder-data-bucket.id}"
-  key = "cardholder_data_primary.csv"
-  source = "../assets/cardholder_data_primary.csv"
-  tags = {
-    Name = "cardholder-data-${var.cgid}"
-    Stack = "${var.stack-name}"
-    Scenario = "${var.scenario-name}"
-  }
-}
-resource "aws_s3_bucket_object" "cardholder-data-secondary" {
-  bucket = "${aws_s3_bucket.cg-cardholder-data-bucket.id}"
-  key = "cardholder_data_secondary.csv"
-  source = "../assets/cardholder_data_secondary.csv"
-  tags = {
-    Name = "cardholder-data-${var.cgid}"
-    Stack = "${var.stack-name}"
-    Scenario = "${var.scenario-name}"
-  }
-}
-resource "aws_s3_bucket_object" "cardholder-data-corporate" {
-  bucket = "${aws_s3_bucket.cg-cardholder-data-bucket.id}"
-  key = "cardholders_corporate.csv"
-  source = "../assets/cardholders_corporate.csv"
-  tags = {
-    Name = "cardholder-data-${var.cgid}"
-    Stack = "${var.stack-name}"
-    Scenario = "${var.scenario-name}"
-  }
-}
-resource "aws_s3_bucket_object" "goat" {
-  bucket = "${aws_s3_bucket.cg-cardholder-data-bucket.id}"
-  key = "goat.png"
-  source = "../assets/goat.png"
-  tags = {
-    Name = "cardholder-data-${var.cgid}"
-    Stack = "${var.stack-name}"
-    Scenario = "${var.scenario-name}"
-  }
+
+  tags = merge(local.default_tags, {
+    Name        = "cg-cardholder-data-bucket-${local.s3_bucket_suffix}"
+    Description = "CloudGoat ${var.cgid} S3 Bucket used for storing sensitive cardholder data."
+  })
 }
 
-resource "aws_s3_bucket_acl" "cardholder-data-bucket-acl" {
+# S3 Objects Uploaded
+resource "aws_s3_object" "s3-objects" {
+  for_each = toset(local.s3_objects)
+
   bucket = aws_s3_bucket.cg-cardholder-data-bucket.id
-  acl    = "private"
-}
\ No newline at end of file
+  key    = each.key
+  source = "../assets/${each.key}"
+
+  tags = merge(local.default_tags, {
+    Name = "cardholder-data-${var.cgid}"
+  })
+}
diff --git a/scenarios/cloud_breach_s3/terraform/variables.tf b/scenarios/cloud_breach_s3/terraform/variables.tf
index cabe3141..6812a519 100644
--- a/scenarios/cloud_breach_s3/terraform/variables.tf
+++ b/scenarios/cloud_breach_s3/terraform/variables.tf
@@ -1,32 +1,53 @@
 #Required: AWS Profile
 variable "profile" {
-
+  description = "The AWS profile to use."
+  type        = string
 }
+
 #Required: AWS Region
 variable "region" {
-  default = "us-east-1"
+  description = "The AWS region to deploy resources to."
+  default     = "us-east-1"
+  type        = string
 }
+
 #Required: CGID Variable for unique naming
 variable "cgid" {
-
+  description = "CGID variable for unique naming."
+  type        = string
 }
+
 #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"]
+  type        = list(any)
 }
+
 #SSH Public Key
 variable "ssh-public-key-for-ec2" {
-  default = "../cloudgoat.pub"
+  description = "Where to store the public key"
+  default     = "../cloudgoat.pub"
+  type        = string
 }
+
 #SSH Private Key
 variable "ssh-private-key-for-ec2" {
-  default = "../cloudgoat"
+  description = "Where to store the private key"
+  default     = "../cloudgoat"
+  type        = string
 }
+
 #Stack Name
 variable "stack-name" {
-  default = "CloudGoat"
+  description = "Name of the stack."
+  default     = "CloudGoat"
+  type        = string
 }
+
 #Scenario Name
 variable "scenario-name" {
-  default = "cloud-breach-s3"
-}
\ No newline at end of file
+  description = "Name of the scenario."
+  default     = "cloud-breach-s3"
+  type        = string
+}
diff --git a/scenarios/cloud_breach_s3/terraform/versions.tf b/scenarios/cloud_breach_s3/terraform/versions.tf
new file mode 100644
index 00000000..85b208f2
--- /dev/null
+++ b/scenarios/cloud_breach_s3/terraform/versions.tf
@@ -0,0 +1,10 @@
+terraform {
+  required_version = ">= 1.2.0"
+
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = "~> 4.16"
+    }
+  }
+}
diff --git a/scenarios/cloud_breach_s3/terraform/vpc.tf b/scenarios/cloud_breach_s3/terraform/vpc.tf
index a561d209..e927ee9d 100644
--- a/scenarios/cloud_breach_s3/terraform/vpc.tf
+++ b/scenarios/cloud_breach_s3/terraform/vpc.tf
@@ -1,61 +1,63 @@
 resource "aws_vpc" "cg-vpc" {
-  cidr_block = "10.10.0.0/16"
+  cidr_block           = "10.10.0.0/16"
   enable_dns_hostnames = true
-  tags = {
-      Name = "CloudGoat ${var.cgid} VPC"
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
-  }
+
+  tags = merge(local.default_tags, {
+    Name = "CloudGoat ${var.cgid} VPC"
+  })
 }
+
 #Internet Gateway
 resource "aws_internet_gateway" "cg-internet-gateway" {
-  vpc_id = "${aws_vpc.cg-vpc.id}"
-  tags = {
-      Name = "CloudGoat ${var.cgid} Internet Gateway"
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
-  }
+  vpc_id = aws_vpc.cg-vpc.id
+
+  tags = merge(local.default_tags, {
+    Name = "CloudGoat ${var.cgid} Internet Gateway"
+  })
 }
+
 #Public Subnets
 resource "aws_subnet" "cg-public-subnet-1" {
   availability_zone = "${var.region}a"
-  cidr_block = "10.10.10.0/24"
-  vpc_id = "${aws_vpc.cg-vpc.id}"
-  tags = {
-      Name = "CloudGoat ${var.cgid} Public Subnet #1"
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
-  }
+  cidr_block        = "10.10.10.0/24"
+  vpc_id            = aws_vpc.cg-vpc.id
+
+  tags = merge(local.default_tags, {
+    Name = "CloudGoat ${var.cgid} Public Subnet #1"
+  })
 }
+
 resource "aws_subnet" "cg-public-subnet-2" {
   availability_zone = "${var.region}b"
-  cidr_block = "10.10.20.0/24"
-  vpc_id = "${aws_vpc.cg-vpc.id}"
-  tags = {
-      Name = "CloudGoat ${var.cgid} Public Subnet #2"
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
-  }
+  cidr_block        = "10.10.20.0/24"
+  vpc_id            = aws_vpc.cg-vpc.id
+
+  tags = merge(local.default_tags, {
+    Name = "CloudGoat ${var.cgid} Public Subnet #2"
+  })
 }
+
 #Public Subnet Routing Table
 resource "aws_route_table" "cg-public-subnet-route-table" {
+  vpc_id = aws_vpc.cg-vpc.id
+
   route {
-      cidr_block = "0.0.0.0/0"
-      gateway_id = "${aws_internet_gateway.cg-internet-gateway.id}"
-  }
-  vpc_id = "${aws_vpc.cg-vpc.id}"
-  tags = {
-      Name = "CloudGoat ${var.cgid} Route Table for Public Subnet"
-      Stack = "${var.stack-name}"
-      Scenario = "${var.scenario-name}"
+    cidr_block = "0.0.0.0/0"
+    gateway_id = aws_internet_gateway.cg-internet-gateway.id
   }
+
+  tags = merge(local.default_tags, {
+    Name = "CloudGoat ${var.cgid} Route Table for Public Subnet"
+  })
 }
+
 #Public Subnets Routing Associations
 resource "aws_route_table_association" "cg-public-subnet-1-route-association" {
-  subnet_id = "${aws_subnet.cg-public-subnet-1.id}"
-  route_table_id = "${aws_route_table.cg-public-subnet-route-table.id}"
+  subnet_id      = aws_subnet.cg-public-subnet-1.id
+  route_table_id = aws_route_table.cg-public-subnet-route-table.id
 }
+
 resource "aws_route_table_association" "cg-public-subnet-2-route-association" {
-  subnet_id = "${aws_subnet.cg-public-subnet-2.id}"
-  route_table_id = "${aws_route_table.cg-public-subnet-route-table.id}"
-}
\ No newline at end of file
+  subnet_id      = aws_subnet.cg-public-subnet-2.id
+  route_table_id = aws_route_table.cg-public-subnet-route-table.id
+}