-
Notifications
You must be signed in to change notification settings - Fork 2
eli-385 finessing github permissions #283
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
Changes from all commits
73324b6
a7cb932
99f5572
38e2133
e03ad5d
6279ee2
9626a17
b2d9a0b
7c1c332
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -163,6 +163,8 @@ resource "aws_iam_policy" "s3_management" { | |
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-truststore/*", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-truststore-access-logs", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-truststore-access-logs/*", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-eli-splunk-backup", | ||
| "arn:aws:s3:::*eligibility-signposting-api-${var.environment}-eli-splunk-backup/*" | ||
| ] | ||
| } | ||
| ] | ||
|
|
@@ -196,20 +198,62 @@ resource "aws_iam_policy" "api_infrastructure" { | |
| # ACM for certs | ||
| "acm:DescribeCertificate", | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. acm needs * level resource as it's an 'account level' thing. |
||
| "acm:GetCertificate", | ||
| "acm:ListCertificates", | ||
| # S3 for mTLS truststore | ||
| "s3:GetObject", | ||
| # CloudWatch Logs for logging | ||
| "logs:CreateLogGroup", | ||
| "logs:CreateLogStream", | ||
| "logs:PutLogEvents", | ||
| # IAM PassRole for logging role association (if needed) | ||
| "iam:PassRole" | ||
| "acm:ListCertificates" | ||
|
|
||
| ], | ||
| Resource = "*" | ||
| #checkov:skip=CKV_AWS_289: Actions require wildcard resource | ||
| }, | ||
| { | ||
| Effect = "Allow", | ||
| Action = [ | ||
| # CloudWatch Logs creation and management | ||
| "logs:CreateLogGroup", | ||
| "logs:CreateLogStream", | ||
| "logs:PutLogEvents" | ||
| ], | ||
| Resource = [ | ||
| # VPC Flow Logs | ||
| "arn:aws:logs:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:log-group:/aws/vpc/*", | ||
| # Lambda function logs | ||
| "arn:aws:logs:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:log-group:/aws/lambda/*", | ||
| # API Gateway logs | ||
| "arn:aws:logs:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:log-group:/aws/apigateway/*", | ||
| # Kinesis Firehose logs | ||
| "arn:aws:logs:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:log-group:/aws/kinesisfirehose/*" | ||
| ] | ||
| }, | ||
| { | ||
| Effect = "Allow", | ||
| Action = [ | ||
| # IAM PassRole for specific service roles only | ||
| "iam:PassRole" | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This specifically addresses the issue around passrole, limiting it to just the roles we want to deploy (and allow AWS services to assume) |
||
| ], | ||
| Resource = [ | ||
| # Lambda execution roles | ||
| "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/eligibility_lambda-role*", | ||
| # API Gateway CloudWatch logging role | ||
| "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/*-api-gateway-*-role", | ||
| # VPC Flow Logs role | ||
| "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/vpc-flow-logs-role*", | ||
| # EventBridge to Firehose role | ||
| "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/eventbridge-firehose-role*", | ||
| # Kinesis Firehose S3 backup roles | ||
| "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/*firehose*role*", | ||
| "arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/splunk-firehose-assume-role*" | ||
| ], | ||
| Condition = { | ||
| StringEquals = { | ||
| "iam:PassedToService" = [ | ||
| "lambda.amazonaws.com", | ||
| "apigateway.amazonaws.com", | ||
| "vpc-flow-logs.amazonaws.com", | ||
| "events.amazonaws.com", | ||
| "firehose.amazonaws.com" | ||
| ] | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| Effect = "Allow", | ||
| Action = [ | ||
|
|
@@ -299,24 +343,22 @@ resource "aws_iam_policy" "kms_creation" { | |
| { | ||
| Effect = "Allow", | ||
| Action = [ | ||
| # Key creation and listing actions require wildcard resource | ||
| "kms:CreateKey", | ||
| "kms:DescribeKey", | ||
| "kms:CreateAlias", | ||
| "kms:List*", | ||
| "kms:ListAliases", | ||
| "kms:Decrypt", | ||
| "kms:Encrypt", | ||
| "kms:ReEncrypt*", | ||
| "kms:ListAliases" | ||
| ], | ||
| Resource = "*" | ||
| }, | ||
| { | ||
| Effect = "Allow", | ||
| Action = [ | ||
| # Key management actions on account-specific keys only | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IAM customer managed policies should not allow decryption actions on all KMS keys Restricting here to only those created in the account. Deployment role needs to be able to do this to access assets... |
||
| "kms:DescribeKey", | ||
| "kms:Describe*", | ||
| "kms:GetKeyPolicy*", | ||
| "kms:GetKeyRotationStatus", | ||
| "kms:Decrypt*", | ||
| "kms:DeleteAlias", | ||
| "kms:UpdateKeyDescription", | ||
| "kms:CreateGrant", | ||
|
|
@@ -325,8 +367,9 @@ resource "aws_iam_policy" "kms_creation" { | |
| "kms:ScheduleKeyDeletion", | ||
| "kms:PutKeyPolicy", | ||
| "kms:Encrypt", | ||
| "kms:TagResource", | ||
| "kms:GenerateDataKey", | ||
| "kms:Decrypt", | ||
| "kms:ReEncrypt*", | ||
| "kms:GenerateDataKey" | ||
| ], | ||
| Resource = [ | ||
| "arn:aws:kms:${var.default_aws_region}:${data.aws_caller_identity.current.account_id}:key/*", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the permissions boundary used by assumed roles (e.g. lambda, cloudwatch --> kinesis etc.) so the permissions boundary itself needs to restrict to only those actions and resources we'd expect those roles to use.