Skip to content

Commit a67adf2

Browse files
eddalmond1eddalmond1
authored
Merge pull request #230 from NHSDigital/bugfix/eja-eli-327-fix-flipflopping-permissions
Bugfix/eja eli 327 fix flipflopping permissions
2 parents 6ef25eb + 4e58423 commit a67adf2

6 files changed

Lines changed: 90 additions & 63 deletions

File tree

‎infrastructure/modules/kinesis_firehose/kms.tf‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ data "aws_iam_policy_document" "firehose_kms_key_policy" {
4545
resources = ["*"]
4646
}
4747

48-
# Your existing statements below...
4948
statement {
5049
sid = "AllowFirehoseAccess"
5150
effect = "Allow"
@@ -110,5 +109,3 @@ data "aws_iam_policy_document" "firehose_kms_key_policy" {
110109
resources = [aws_kms_key.firehose_cmk.arn]
111110
}
112111
}
113-
114-

‎infrastructure/modules/s3/kms.tf‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
resource "aws_kms_key" "storage_bucket_cmk" {
2+
#checkov:skip=CKV2_AWS_64: KMS key policy is defined in api-layer iam_policies.tf
23
description = "${terraform.workspace == "default" ? "" : "${terraform.workspace}-"}${var.bucket_name} Master Key"
34
deletion_window_in_days = 14
45
is_enabled = true
@@ -14,21 +15,3 @@ resource "aws_kms_alias" "storage_bucket_cmk" {
1415
name = "alias/${terraform.workspace == "default" ? "" : "${terraform.workspace}-"}${var.bucket_name}-cmk"
1516
target_key_id = aws_kms_key.storage_bucket_cmk.key_id
1617
}
17-
18-
resource "aws_kms_key_policy" "storage_bucket_cmk" {
19-
key_id = aws_kms_key.storage_bucket_cmk.id
20-
policy = data.aws_iam_policy_document.storage_bucket_cmk.json
21-
}
22-
23-
data "aws_iam_policy_document" "storage_bucket_cmk" {
24-
statement {
25-
sid = "Enable IAM User Permissions for s3 buckets"
26-
effect = "Allow"
27-
principals {
28-
type = "AWS"
29-
identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"]
30-
}
31-
actions = ["kms:*"]
32-
resources = [aws_kms_key.storage_bucket_cmk.arn]
33-
}
34-
}

‎infrastructure/modules/s3/outputs.tf‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,7 @@ output "storage_bucket_id" {
2121
output "storage_bucket_kms_key_arn" {
2222
value = aws_kms_key.storage_bucket_cmk.arn
2323
}
24+
25+
output "storage_bucket_kms_key_id" {
26+
value = aws_kms_key.storage_bucket_cmk.id
27+
}

‎infrastructure/modules/s3/s3.tf‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,39 +14,6 @@ resource "aws_s3_bucket_versioning" "storage_bucket_versioning_config" {
1414
}
1515
}
1616

17-
# ensure only secure transport is allowed
18-
19-
resource "aws_s3_bucket_policy" "storage_bucket" {
20-
bucket = aws_s3_bucket.storage_bucket.id
21-
policy = data.aws_iam_policy_document.storage_s3_bucket_policy.json
22-
}
23-
24-
data "aws_iam_policy_document" "storage_s3_bucket_policy" {
25-
statement {
26-
sid = "AllowSslRequestsOnly"
27-
actions = [
28-
"s3:*",
29-
]
30-
effect = "Deny"
31-
resources = [
32-
aws_s3_bucket.storage_bucket.arn,
33-
"${aws_s3_bucket.storage_bucket.arn}/*",
34-
]
35-
principals {
36-
type = "*"
37-
identifiers = ["*"]
38-
}
39-
condition {
40-
test = "Bool"
41-
values = [
42-
"false",
43-
]
44-
45-
variable = "aws:SecureTransport"
46-
}
47-
}
48-
}
49-
5017
# Block public access to the bucket
5118
resource "aws_s3_bucket_public_access_block" "storage_bucket_block_public_access" {
5219
bucket = aws_s3_bucket.storage_bucket.id

‎infrastructure/stacks/api-layer/iam_policies.tf‎

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,70 @@ data "aws_iam_policy_document" "s3_rules_bucket_policy" {
7171
}
7272
}
7373

74+
# ensure only secure transport is allowed
75+
76+
resource "aws_s3_bucket_policy" "rules_s3_bucket" {
77+
bucket = module.s3_rules_bucket.storage_bucket_id
78+
policy = data.aws_iam_policy_document.rules_s3_bucket_policy.json
79+
}
80+
81+
data "aws_iam_policy_document" "rules_s3_bucket_policy" {
82+
statement {
83+
sid = "AllowSslRequestsOnly"
84+
actions = [
85+
"s3:*",
86+
]
87+
effect = "Deny"
88+
resources = [
89+
module.s3_rules_bucket.storage_bucket_arn,
90+
"${module.s3_rules_bucket.storage_bucket_arn}/*",
91+
]
92+
principals {
93+
type = "*"
94+
identifiers = ["*"]
95+
}
96+
condition {
97+
test = "Bool"
98+
values = [
99+
"false",
100+
]
101+
102+
variable = "aws:SecureTransport"
103+
}
104+
}
105+
}
106+
107+
resource "aws_s3_bucket_policy" "audit_s3_bucket" {
108+
bucket = module.s3_audit_bucket.storage_bucket_id
109+
policy = data.aws_iam_policy_document.audit_s3_bucket_policy.json
110+
}
111+
112+
data "aws_iam_policy_document" "audit_s3_bucket_policy" {
113+
statement {
114+
sid = "AllowSslRequestsOnly"
115+
actions = [
116+
"s3:*",
117+
]
118+
effect = "Deny"
119+
resources = [
120+
module.s3_audit_bucket.storage_bucket_arn,
121+
"${module.s3_audit_bucket.storage_bucket_arn}/*",
122+
]
123+
principals {
124+
type = "*"
125+
identifiers = ["*"]
126+
}
127+
condition {
128+
test = "Bool"
129+
values = [
130+
"false",
131+
]
132+
133+
variable = "aws:SecureTransport"
134+
}
135+
}
136+
}
137+
74138
# Attach s3 read policy to Lambda role
75139
resource "aws_iam_role_policy" "lambda_s3_read_policy" {
76140
name = "S3ReadAccess"
@@ -216,7 +280,7 @@ data "aws_iam_policy_document" "s3_rules_kms_key_policy" {
216280
}
217281

218282
resource "aws_kms_key_policy" "s3_rules_kms_key" {
219-
key_id = module.s3_rules_bucket.storage_bucket_kms_key_arn
283+
key_id = module.s3_rules_bucket.storage_bucket_kms_key_id
220284
policy = data.aws_iam_policy_document.s3_rules_kms_key_policy.json
221285
}
222286

@@ -235,7 +299,6 @@ data "aws_iam_policy_document" "s3_audit_kms_key_policy" {
235299
actions = ["kms:*"]
236300
resources = ["*"]
237301
}
238-
239302
statement {
240303
sid = "AllowLambdaFullWrite"
241304
effect = "Allow"
@@ -254,7 +317,7 @@ data "aws_iam_policy_document" "s3_audit_kms_key_policy" {
254317
}
255318

256319
resource "aws_kms_key_policy" "s3_audit_kms_key" {
257-
key_id = module.s3_audit_bucket.storage_bucket_kms_key_arn
320+
key_id = module.s3_audit_bucket.storage_bucket_kms_key_id
258321
policy = data.aws_iam_policy_document.s3_audit_kms_key_policy.json
259322
}
260323

@@ -277,9 +340,3 @@ resource "aws_iam_role_policy" "lambda_firehose_policy" {
277340
role = aws_iam_role.eligibility_lambda_role.id
278341
policy = data.aws_iam_policy_document.lambda_firehose_write_policy.json
279342
}
280-
281-
282-
283-
284-
285-

‎infrastructure/stacks/api-layer/truststore_s3_bucket.tf‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,25 @@ resource "aws_s3_bucket_policy" "truststore" {
1313
}
1414

1515
data "aws_iam_policy_document" "truststore_api_gateway" {
16+
# Deny non-SSL
17+
statement {
18+
sid = "AllowSslRequestsOnly"
19+
actions = ["s3:*"]
20+
effect = "Deny"
21+
resources = [
22+
module.s3_truststore_bucket.storage_bucket_arn,
23+
"${module.s3_truststore_bucket.storage_bucket_arn}/*"
24+
]
25+
principals {
26+
type = "*"
27+
identifiers = ["*"]
28+
}
29+
condition {
30+
test = "Bool"
31+
variable = "aws:SecureTransport"
32+
values = ["false"]
33+
}
34+
}
1635
statement {
1736
sid = "Enable S3 access permissions for API Gateway"
1837
effect = "Allow"

0 commit comments

Comments
 (0)