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 Terminator support for KMS and allow KMS in the CI account #106

Merged
merged 5 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
93 changes: 63 additions & 30 deletions aws/policy/security-services.yaml
Original file line number Diff line number Diff line change
@@ -1,21 +1,7 @@
Version: '2012-10-17'
Statement:
- Sid: AllowAssumeRoleTests
Effect: Allow
Action:
- iam:CreateRole
- iam:DeleteRole
- iam:ListInstanceProfilesForRole
- iam:GetInstanceProfile
- iam:CreateInstanceProfile
- iam:DeleteInstanceProfile
- iam:AddRoleToInstanceProfile
- iam:RemoveRoleFromInstanceProfile
- sts:AssumeRole
Resource:
- 'arn:aws:iam::{{ aws_account_id }}:instance-profile/ansible-test-*'
- 'arn:aws:iam::{{ aws_account_id }}:role/ansible-test-*'
- Sid: AllowAssumeRoleTestsAttachAndDetachRole

- Sid: AllowAssumeRoleTestsAttachAndDetachPolicy
Effect: Allow
Action:
- iam:AttachRolePolicy
Expand All @@ -32,7 +18,16 @@ Statement:
- 'arn:aws:iam::aws:policy/service-role/AmazonEC2ContainerServiceRole'
- 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole'

- Sid: AllowGlobalUnrestrictedResourceActionsWhichIncurNoFees
# Legacy - We need to backport ansible-collections/community.aws/63 or
# wait until community.aws drops CI support for Ansible 2.9
Copy link
Collaborator

Choose a reason for hiding this comment

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

@tremble We don't currently have a sunset timeline for 2.9 support, would you mind opening that backport when 63 merges? It'll ultimately be up to core to accept but I'd rather not have extraneous roles hanging around forever if we can avoid it.

- Sid: AllowPassRole
Effect: Allow
Action:
- iam:PassRole
Resource:
- 'arn:aws:iam::{{ aws_account_id }}:role/ansible_lambda_role'

- Sid: AllowRegionalUnrestrictedResourceActionsWhichIncurNoFees
Effect: Allow
Action:
- iam:ListAccountAliases
Expand Down Expand Up @@ -117,7 +112,19 @@ Statement:
aws:RequestedRegion:
- '{{ aws_region }}'

- Sid: AllowGlobalResourceRestrictedActionsWhichIncurNoFees
- Sid: AllowRegionalUnrestrictedResourceActionsWhichIncurFees
Effect: Allow
Action:
- kms:CancelKeyDeletion
- kms:CreateKey
- kms:GenerateRandom
Resource: "*"
tremble marked this conversation as resolved.
Show resolved Hide resolved
Condition:
StringEquals:
aws:RequestedRegion:
- '{{ aws_region }}'

- Sid: AllowGlobalUnrestrictedResourceActionsWhichIncurNoFees
Effect: Allow
Action:
- iam:GetRole
Expand All @@ -127,27 +134,53 @@ Statement:
- iam:ListRoleTags
- iam:TagRole
- iam:UntagRole
Resource:
- 'arn:aws:iam::{{ aws_account_id }}:role/*'
- Sid: AllowPassRole
Effect: Allow
Action:
- iam:PassRole
Resource:
- 'arn:aws:iam::{{ aws_account_id }}:role/ansible-test-*'
# Legacy - We need to backport ansible-collections/community.aws/63 or
# wait until community.aws drops CI support for Ansible 2.9
- 'arn:aws:iam::{{ aws_account_id }}:role/ansible_lambda_role'
- kms:CreateAlias
- kms:CreateGrant
- kms:DeleteAlias
- kms:DescribeKey
- kms:DisableKey
- kms:DisableKeyRotation
- kms:EnableKey
- kms:EnableKeyRotation
- kms:GetKeyPolicy
- kms:GetKeyRotationStatus
- kms:GetPublicKey
- kms:ListAliases
- kms:ListGrants
- kms:ListKeyPolicies
- kms:ListKeys
- kms:ListResourceTags
- kms:ListRetirableGrants
- kms:PutKeyPolicy
- kms:RetireGrant
- kms:ScheduleKeyDeletion
- kms:TagResource
- kms:UntagResource
- kms:UpdateGrant
- kms:UpdateKeyDescription
Resource: "*"

- Sid: AllowACMRestrictable
- Sid: AllowResourceRestrictedActionsWhichIncurNoFees
Effect: Allow
Action:
- acm:DescribeCertificate
- acm:GetCertificate
- acm:AddTagsToCertificate
- acm:DeleteCertificate
- iam:AddRoleToInstanceProfile
- iam:CreateInstanceProfile
- iam:CreateRole
- iam:DeleteInstanceProfile
- iam:DeleteRole
- iam:GetInstanceProfile
- iam:ListInstanceProfilesForRole
- iam:PassRole
- iam:RemoveRoleFromInstanceProfile
- sts:AssumeRole
Resource:
- 'arn:aws:acm:{{ aws_region }}:{{ aws_account_id }}:certificate/*'
- 'arn:aws:iam::{{ aws_account_id }}:instance-profile/ansible-test-*'
- 'arn:aws:iam::{{ aws_account_id }}:role/ansible-test-*'

# This allows AWS Services to autmatically create their Default Service Linked Roles
# These have fixed policies and can only be assumed by the service itself.
Expand Down
53 changes: 53 additions & 0 deletions aws/terminator/security_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,56 @@ def name(self):

def terminate(self):
self.client.delete_certificate(CertificateArn=self.id)


class KMSKey(Terminator):
@staticmethod
def create(credentials):
def get_paginated_keys(client):
return client.get_paginator('list_keys').paginate().build_full_result()['Keys']

def get_key_details(client, key):
metadata = client.describe_key(KeyId=key['KeyId'])['KeyMetadata']
_aliases = client.list_aliases(KeyId=key['KeyId'])['Aliases']
aliases = []
for alias in _aliases:
aliases.append(alias['AliasName'])
metadata['Aliases'] = aliases
return metadata

def get_detailed_keys(client):
detailed_keys = []
for key in get_paginated_keys(client):
metadata = get_key_details(client, key)
if metadata:
detailed_keys.append(metadata)
return detailed_keys

return Terminator._create(credentials, KMSKey, 'kms', get_detailed_keys)

@property
def ignore(self):
# The key is already in a 'pending deletion' state, and doesn't need
# anything more done to it.
if self.instance['KeyState'] == 'PendingDeletion':
return True
# Don't try deleting the AWS managed keys (they're not charged for)
for alias in self.instance['Aliases']:
if alias.startswith('alias/aws/'):
return True
return False

@property
def created_time(self):
return self.instance['CreationDate']

@property
def id(self):
return self.instance['KeyId']

@property
def name(self):
return self.instance['Aliases']

def terminate(self):
self.client.schedule_key_deletion(KeyId=self.id, PendingWindowInDays=7)
27 changes: 0 additions & 27 deletions hacking/aws_config/test_policies/security-services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,33 +28,6 @@ Statement:
- iam:ListServerCertificates
- iam:UpdateServerCertificate
- iam:UploadServerCertificate
# Because KMS CMKs are assigned a random UUID we can't apply Resource
# restrictions at the IAM policy level
- kms:CancelKeyDeletion
- kms:CreateAlias
- kms:CreateGrant
- kms:CreateKey
- kms:DeleteAlias
- kms:DescribeKey
- kms:DisableKey
- kms:EnableKey
- kms:GenerateRandom
- kms:GetKeyPolicy
- kms:GetKeyRotationStatus
- kms:GetPublicKey
- kms:ListAliases
- kms:ListGrants
- kms:ListKeyPolicies
- kms:ListKeys
- kms:ListResourceTags
- kms:ListRetirableGrants
- kms:PutKeyPolicy
- kms:RetireGrant
- kms:ScheduleKeyDeletion
- kms:TagResource
- kms:UntagResource
- kms:UpdateGrant
- kms:UpdateKeyDescription
- waf:DeleteLoggingConfiguration
- waf:DeletePermissionPolicy
- waf:GetLoggingConfiguration
Expand Down