-
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
eli-385 finessing github permissions #283
Conversation
…emove-wildcard-resource-and-passrole-permissions
| effect = "Allow" | ||
|
|
||
| actions = [ | ||
| "acm:*", |
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.
| sid = "RestrictRegion" | ||
| effect = "Allow" | ||
|
|
||
| actions = [ |
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 deployment role, so a wider range of services need to be accessed - we try here to restrict actions to only those deployments would need. As the permissions boundary doesn't actually grant permissions, we retain the * resource so that the role itself can set specific resource permissions.
| # API Gateway domain and deployment | ||
| "apigateway:*", | ||
| # ACM for certs | ||
| "acm:DescribeCertificate", |
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.
acm needs * level resource as it's an 'account level' thing.
…-and-passrole-permissions
…-and-passrole-permissions
…-and-passrole-permissions
| Effect = "Allow", | ||
| Action = [ | ||
| # IAM PassRole for specific service roles only | ||
| "iam:PassRole" |
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 specifically addresses the issue around passrole, limiting it to just the roles we want to deploy (and allow AWS services to assume)
| { | ||
| Effect = "Allow", | ||
| Action = [ | ||
| # Key management actions on account-specific keys only |
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.
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...
…-and-passrole-permissions
Description
Context
A few things are flagged up in both our and the pen test security reviews for the github actions role:
IAM customer managed policies should not allow decryption actions on all KMS keys
IAM customer managed policies that you create should not allow wildcard actions for services
IAM - Policies Allows “PassRole” Action For Any Resource
We can address these concerns by further tightening up the Github deployment role and permission boundary policies.
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.