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

Add KMS key creation and conditional policy for SNS + Other Services #49

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,16 @@ Before you begin using this Terraform module, ensure you meet the following prer
| asg\_enable\_instance\_refresh | Enables instance refresh for the GraphDB Auto scaling group. A refresh is started when any of the following Auto Scaling Group properties change: launch\_configuration, launch\_template, mixed\_instances\_policy | `bool` | `false` | no |
| asg\_instance\_refresh\_checkpoint\_delay | Number of seconds to wait after a checkpoint. | `number` | `3600` | no |
| graphdb\_enable\_userdata\_scripts\_on\_reboot | (Experimental) Modifies cloud-config to always run user data scripts on EC2 boot | `bool` | `false` | no |
| enable\_sns\_kms\_key | Enable Customer managed keys for encryption. If set to false it will use AWS managed key. | `bool` | `false` | no |
| sns\_cmk\_description | Description for the KMS key for the encryption of SNS | `string` | `"KMS CMK Key to encrypt SNS topics"` | no |
| sns\_key\_admin\_arn | ARN of the role or user granted administrative access to the SNS KMS key. | `string` | `""` | no |
| deletion\_window\_in\_days | The waiting period, specified in number of days for AWS to delete the KMS key(Between 7 and 30). | `number` | `30` | no |
| sns\_external\_kms\_key | ARN of the external KMS key that will be used for encryption of SNS topics | `string` | `""` | no |
| key\_spec | Specification of the Key. | `string` | `"SYMMETRIC_DEFAULT"` | no |
| key\_enabled | Specifies whether the key is enabled. | `bool` | `true` | no |
| rotation\_enabled | Specifies whether key rotation is enabled. | `bool` | `true` | no |
| cmk\_key\_alias | The alias for the CMK key. | `string` | `"sns-cmk-key"` | no |
| sns\_default\_kms\_key | ARN of the default KMS key that will be used for encryption of SNS topics | `string` | `"alias/aws/sns"` | no |
<!-- END_TF_DOCS -->

## Usage
Expand Down
27 changes: 19 additions & 8 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,24 @@ module "monitoring" {

count = var.deploy_monitoring ? 1 : 0

resource_name_prefix = var.resource_name_prefix
aws_region = var.aws_region
route53_availability_check_region = var.monitoring_route53_health_check_aws_region
cloudwatch_alarms_actions_enabled = var.monitoring_actions_enabled
sns_topic_endpoint = var.deploy_monitoring ? var.monitoring_sns_topic_endpoint : null
sns_endpoint_auto_confirms = var.monitoring_endpoint_auto_confirms
sns_protocol = var.monitoring_sns_protocol
resource_name_prefix = var.resource_name_prefix
aws_region = var.aws_region
route53_availability_check_region = var.monitoring_route53_health_check_aws_region
cloudwatch_alarms_actions_enabled = var.monitoring_actions_enabled
sns_topic_endpoint = var.deploy_monitoring ? var.monitoring_sns_topic_endpoint : null
sns_endpoint_auto_confirms = var.monitoring_endpoint_auto_confirms
sns_protocol = var.monitoring_sns_protocol
sns_cmk_description = var.sns_cmk_description
sns_key_admin_arn = var.sns_key_admin_arn
enable_sns_kms_key = var.enable_sns_kms_key
sns_external_kms_key = var.sns_external_kms_key
rotation_enabled = var.rotation_enabled
deletion_window_in_days = var.deletion_window_in_days
key_enabled = var.key_enabled
key_spec = var.key_spec
sns_default_kms_key = var.sns_default_kms_key
cmk_key_alias = var.cmk_key_alias

cloudwatch_log_group_retention_in_days = var.monitoring_log_group_retention_in_days
route53_availability_request_url = module.load_balancer.lb_dns_name
route53_availability_measure_latency = var.monitoring_route53_measure_latency
Expand Down Expand Up @@ -215,4 +226,4 @@ module "graphdb" {
asg_enable_instance_refresh = var.asg_enable_instance_refresh
asg_instance_refresh_checkpoint_delay = var.asg_instance_refresh_checkpoint_delay
graphdb_enable_userdata_scripts_on_reboot = var.graphdb_enable_userdata_scripts_on_reboot
}
}
2 changes: 1 addition & 1 deletion modules/monitoring/availability_tests.tf
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
resource "aws_sns_topic" "graphdb_route53_sns_topic" {
provider = aws.useast1
name = "${var.resource_name_prefix}-route53-sns-notifications"
kms_master_key_id = "alias/aws/sns"
kms_master_key_id = var.sns_external_kms_key != "" ? var.sns_external_kms_key : (var.enable_sns_kms_key ? aws_kms_key.sns_cmk[0].arn : var.sns_default_kms_key)
Copy link

Choose a reason for hiding this comment

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

I don't think that you need an additional variable for the default KMS key variable. Instead, use:

kms_master_key_id = var.sns_external_kms_key != "" ? var.sns_external_kms_key : (var.enable_sns_kms_key ? aws_kms_key.sns_cmk[0].arn : "alias/aws/sns")

}

resource "aws_sns_topic_subscription" "graphdb_route53_sns_topic_subscription" {
Expand Down
148 changes: 148 additions & 0 deletions modules/monitoring/cmk.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
data "aws_region" "current" {}

data "aws_caller_identity" "current" {}

# Creates/manages KMS CMK
resource "aws_kms_key" "sns_cmk" {
count = var.enable_sns_kms_key ? 1 : 0

description = var.sns_cmk_description
customer_master_key_spec = var.key_spec
is_enabled = var.key_enabled
enable_key_rotation = var.rotation_enabled
deletion_window_in_days = var.deletion_window_in_days
}

resource "aws_kms_key_policy" "sns_cmk_policy" {
count = var.enable_sns_kms_key ? 1 : 0
key_id = aws_kms_key.sns_cmk[0].id

policy = jsonencode({
"Version" : "2012-10-17",
"Id" : "kms-key-policy-access-control",
"Statement" : [
{
"Sid" : "Enable IAM User Permissions",
"Effect" : "Allow",
"Principal" : {
"AWS" : var.sns_key_admin_arn != "" ? var.sns_key_admin_arn : aws_iam_role.graphdb_sns_key_admin_role.arn
},
"Action" : [
"kms:CreateAlias",
"kms:CreateKey",
"kms:Encrypt",
"kms:Decrypt",
"kms:DeleteAlias",
"kms:DescribeKey",
"kms:GetKeyPolicy",
"kms:GetKeyRotationStatus",
"kms:ListAliases",
"kms:ListGrants",
"kms:ListKeyPolicies",
"kms:ListKeys",
"kms:PutKeyPolicy",
"kms:UpdateAlias",
"kms:EnableKeyRotation",
"kms:ListResourceTags",
"kms:ScheduleKeyDeletion",
"kms:DisableKeyRotation"
],
"Resource" : "arn:aws:kms:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:key/${aws_kms_key.sns_cmk[0].id}"
},
{
"Sid" : "Allow access for Key Administrators",
"Effect" : "Allow",
"Principal" : {
"AWS" : var.sns_key_admin_arn != "" ? var.sns_key_admin_arn : aws_iam_role.graphdb_sns_key_admin_role.arn
},
"Action" : [
"kms:CreateAlias",
"kms:CreateKey",
"kms:Encrypt",
"kms:Decrypt",
"kms:DeleteAlias",
"kms:DescribeKey",
"kms:GetKeyPolicy",
"kms:GetKeyRotationStatus",
"kms:ListAliases",
"kms:ListGrants",
"kms:ListKeyPolicies",
"kms:ListKeys",
"kms:PutKeyPolicy",
"kms:UpdateAlias",
"kms:EnableKeyRotation",
"kms:ListResourceTags",
"kms:ScheduleKeyDeletion",
"kms:DisableKeyRotation"
],
"Resource" : "arn:aws:kms:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:key/${aws_kms_key.sns_cmk[0].id}"
},
{
"Sid" : "Allow use of the key",
"Effect" : "Allow",
"Principal" : {
"AWS" : var.sns_key_admin_arn != "" ? var.sns_key_admin_arn : aws_iam_role.graphdb_sns_key_admin_role.arn
},
"Action" : [
"kms:Encrypt",
"kms:Decrypt",
"kms:ReEncrypt*",
"kms:GenerateDataKey*",
"kms:DescribeKey"
],
"Resource" : "arn:aws:kms:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:key/${aws_kms_key.sns_cmk[0].id}"
},
{
"Sid" : "Allow use of the key for SNS",
"Effect" : "Allow",
"Principal" : {
"Service" : [
"sns.amazonaws.com",
"ec2.amazonaws.com"
]
},
"Action" : [
"kms:GenerateDataKeyWithoutPlaintext",
"kms:DescribeKey"
],
"Resource" : "arn:aws:kms:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:key/${aws_kms_key.sns_cmk[0].id}"
},
{
"Sid" : "Allow the current user to manage key",
"Effect" : "Allow",
"Principal" : {
"AWS" : "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"
Copy link

Choose a reason for hiding this comment

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

Doesn't that work without specifying root?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the idea is that the Root account have access so if anything goes wrong like it did in the other account, that the root account can fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look like a good approach. Imagine deploying this in your department AWS account, something goes wrong and you need to request support from the root account holder. This could be incredibly slow and tedious procedure in some companies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, however we have had a problem last week with a Key that was created without a policy for some reason and it could not be reached, opened or deleted. That's why I have added this "safety net" just in case. It is only for the root account. I could remove it if you think it is bad, I don't think it will affect the rest of the module, however I have only put it in case it goes wrong agian.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was rather going in another direction, can't se allow only people with admin roles to access it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll research for another solution, test it and come back to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there isn't a default admin group/role that we can use, I can create it so that they can (if they want) provide admin group's ARN which will get the full access over the keys and if not provided it will be left empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see other options here @simeonzhekofff WDYT?

Copy link

Choose a reason for hiding this comment

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

We can have an admin group that should have the right to assume an IAM role that has permission to take actions on the key.

},
"Action" : [
"kms:CreateAlias",
"kms:CreateKey",
"kms:Encrypt",
"kms:Decrypt",
"kms:DeleteAlias",
"kms:DescribeKey",
"kms:GetKeyPolicy",
"kms:GetKeyRotationStatus",
"kms:ListAliases",
"kms:ListGrants",
"kms:ListKeyPolicies",
"kms:ListKeys",
"kms:PutKeyPolicy",
"kms:UpdateAlias",
"kms:EnableKeyRotation",
"kms:ListResourceTags",
"kms:ScheduleKeyDeletion",
"kms:DisableKeyRotation"
],
"Resource" : "arn:aws:kms:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:key/${aws_kms_key.sns_cmk[0].id}"
}
]
})
}

# Add an alias to the key
resource "aws_kms_alias" "sns_cmk_alias" {
count = var.enable_sns_kms_key ? 1 : 0

name = "alias/${var.cmk_key_alias}"
target_key_id = aws_kms_key.sns_cmk[0].key_id
}
79 changes: 79 additions & 0 deletions modules/monitoring/iam.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
data "aws_iam_policy_document" "graphdb_sns_key_admin_role_assume" {
statement {
effect = "Allow"

principals {
type = "AWS"
identifiers = [
"${data.aws_caller_identity.current.arn}"
]
}

actions = [
"sts:AssumeRole"
]
}

statement {
effect = "Allow"

principals {
type = "Service"
identifiers = [
"s3.amazonaws.com",
"ebs.amazonaws.com",
"sns.amazonaws.com",
"ssm.amazonaws.com"
]
}

actions = [
"sts:AssumeRole"
]
}
}

data "aws_iam_policy_document" "graphdb_sns_key_admin_role_permissions" {
statement {
effect = "Allow"

actions = [
"kms:CreateAlias",
"kms:CreateKey",
"kms:Encrypt",
"kms:Decrypt",
"kms:DeleteAlias",
"kms:DescribeKey",
"kms:GetKeyPolicy",
"kms:GetKeyRotationStatus",
"kms:ListAliases",

"kms:UpdateKeyDescription",
"kms:ListGrants",
"kms:ListKeyPolicies",
"kms:ListKeys",
"kms:PutKeyPolicy",
"kms:UpdateAlias",
"kms:EnableKeyRotation",
"kms:ListResourceTags",
"kms:ScheduleKeyDeletion",
"kms:DisableKeyRotation",
"tag:GetResources",
]

resources = [
"*"
]
}
}

resource "aws_iam_role_policy" "graphdb_sns_key_admin_role_permissions" {
name = "KMSPermissionsPolicy-SNS"
role = aws_iam_role.graphdb_sns_key_admin_role.name
policy = data.aws_iam_policy_document.graphdb_sns_key_admin_role_permissions.json
}

resource "aws_iam_role" "graphdb_sns_key_admin_role" {
name = "${var.resource_name_prefix}-sns-topic-role"
assume_role_policy = data.aws_iam_policy_document.graphdb_sns_key_admin_role_assume.json
}
9 changes: 9 additions & 0 deletions modules/monitoring/outputs.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
output "sns_cmk_arn" {
description = "ARN of the KMS CMK"
value = var.enable_sns_kms_key && length(aws_kms_key.sns_cmk) > 0 ? aws_kms_key.sns_cmk[0].arn : null
}

output "sns_cmk_alias_arn" {
description = "ARN of the CMK Alias"
value = var.enable_sns_kms_key && length(aws_kms_alias.sns_cmk_alias) > 0 ? aws_kms_alias.sns_cmk_alias[0].arn : null
}
2 changes: 1 addition & 1 deletion modules/monitoring/sns.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

kristianiliev1 marked this conversation as resolved.
Show resolved Hide resolved
resource "aws_sns_topic" "graphdb_sns_topic" {
name = "${var.resource_name_prefix}-graphdb-notifications"
kms_master_key_id = "alias/aws/sns"
kms_master_key_id = var.sns_external_kms_key != "" ? var.sns_external_kms_key : (var.enable_sns_kms_key ? aws_kms_key.sns_cmk[0].arn : var.sns_default_kms_key)
}

# SNS Topic subscription
Expand Down
54 changes: 54 additions & 0 deletions modules/monitoring/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,57 @@ variable "route53_availability_check_region" {
description = "Define route53 health check region"
type = string
}

kristianiliev1 marked this conversation as resolved.
Show resolved Hide resolved
# KMS Encryption for SNS topics:

variable "sns_cmk_description" {
description = "Description of the Key to be created"
type = string
}

variable "deletion_window_in_days" {
description = "The waiting period, specified in number of days for AWS to delete the KMS key(Between 7 and 30)."
type = number
}

variable "key_spec" {
Copy link
Contributor

Choose a reason for hiding this comment

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

add missing types

description = "Specification of the Key."
Copy link

Choose a reason for hiding this comment

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

This description doesn't give me info why it's needed and what's the use of it.

type = string
}

variable "key_enabled" {
description = "Specifies whether the key is enabled."
type = bool
}

variable "rotation_enabled" {
description = "Specifies whether key rotation is enabled."
type = bool
}

variable "cmk_key_alias" {
description = "The alias for the CMK key."
type = string
}

variable "enable_sns_kms_key" {
description = "Enable CMK for encryption. If false, use AWS managed key."
type = bool
}

variable "sns_key_admin_arn" {
description = "ARN of the role or user who will have administrative access to the SNS KMS key"
type = string
default = ""
}

variable "sns_external_kms_key" {
description = "ARN of the external KMS key that will be used for encryption of SNS topics"
type = string
default = ""
}

variable "sns_default_kms_key" {
description = "ARN of the default KMS key that will be used for encryption of SNS topics"
type = string
}
Loading