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

Adds support for monitoring IAM access #15

Merged
merged 2 commits into from
Nov 17, 2020

Conversation

fernandogoncalves-me
Copy link
Contributor

This PR adds the capability of monitoring the activity of the root user and any other IAM User or IAM Role.

The main purpose of this change is to monitor when users with privileged access interact with our accounts. The CloudWatch Rule pattern will look for both Console and API activity.

For the core accounts, the root user will be automatically monitored and all activities will be reported to the topic LandingZone-MonitorIAMAccess in the audit account.

For the avm created accounts, the monitoring is optional and can be enabled by passing the ARN of SNS Topic that should receive notifications. If enabled, the root user of the account will also be automatically monitored.

CHANGELOG.md Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Terraform Security Scan Failed

Show Output
Problem 1

  [AWS016][ERROR] Resource 'aws_sns_topic.monitor_iam_access' defines an unencrypted SNS topic.
  /github/workspace/audit.tf:48-50

      45 |   }
      46 | }
      47 | 
      48 | resource "aws_sns_topic" "monitor_iam_access" {
      49 |   name = "LandingZone-MonitorIAMAccess"
      50 | }
      51 | 
      52 | resource "aws_sns_topic_policy" "monitor_iam_access" {
      53 |   arn    = aws_sns_topic.monitor_iam_access.arn

  See https://tfsec.dev/docs/aws/AWS016/ for more information.

  disk i/o             6.746145ms
  parsing HCL          57.102µs
  evaluating values    7.38436ms
  running checks       959.721µs
  files loaded         12

1 potential problems detected.

Copy link
Member

@shoekstra shoekstra left a comment

Choose a reason for hiding this comment

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

Looks really great @fgoncalves-io, have a minor q regarding casing in an SNS topic name but otherwise ready to go I think!


data "aws_sns_topic" "all_config_notifications" {
provider = aws.audit
name = "aws-controltower-AllConfigNotifications"
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to capitalise certain letters shouldn't we do it the whole string, e.g.

Suggested change
name = "aws-controltower-AllConfigNotifications"
name = "AWS-ControlTower-AllConfigNotifications"

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how AWS creates the Control Tower SNS Topics...I also think it's a bit strange but I believe there isn't much we can do...

Screenshot 2020-11-17 at 09 21 57

Copy link
Member

Choose a reason for hiding this comment

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

Ah fair enough, if it's inline with other resources then all is good 👍

@fernandogoncalves-me fernandogoncalves-me merged commit d63db8a into master Nov 17, 2020
@fernandogoncalves-me fernandogoncalves-me deleted the monitor_iam_access branch November 17, 2020 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants