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

Filter out opted out regions in config check #437

Merged
merged 4 commits into from
Jul 7, 2022
Merged

Filter out opted out regions in config check #437

merged 4 commits into from
Jul 7, 2022

Conversation

yorinasub17
Copy link
Contributor

This PR updates the "AWS Config Enabled in all regions" query to filter out regions that are opted out. Opted out regions can not be accessed in any way so missing config resources in those regions should not be reported as an error.

@rajlearner17 rajlearner17 changed the base branch from main to release/v0.38 July 6, 2022 12:14
@misraved
Copy link
Contributor

misraved commented Jul 6, 2022

Thanks @yorinasub17 for using Steampipe 👍 . Great to see your contribution here 👍.

You are absolutely right about the control reporting an error state, however, instead of filtering out the regions which have not been opted in, could we skip them in state and add a reason for opted-out regions?

You could refer https://github.com/turbot/steampipe-mod-aws-compliance/blob/main/query/securityhub/securityhub_enabled.sql for more information.

@yorinasub17
Copy link
Contributor Author

however, instead of filtering out the regions which have not been opted in, could we skip them in state and add a reason for opted-out regions?

Sounds good! Updated in 547faad and checked that it works as intended (account ID masked):

%~> aws-vault exec logs -- steampipe check control.cis_v140_3_5 --workspace-chdir .

+ 3.5 Ensure AWS Config is enabled in all regions ........................................................................................ 1 / 22 [==========]
  |
  ALARM: ap-northeast-3 IncludeGlobalResourceTypes disabled, AllSupported disabled, Recording disabled and LastStatus is not SUCC… ap-northeast-3 000000000000
  OK   : sa-east-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ sa-east-1 000000000000
  OK   : eu-west-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ eu-west-1 000000000000
  OK   : ap-south-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .......... ap-south-1 000000000000
  OK   : us-west-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ us-west-2 000000000000
  OK   : us-east-1 IncludeGlobalResourceTypes enabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............. us-east-1 000000000000
  OK   : ap-southeast-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-southeast-2 000000000000
  OK   : ap-northeast-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-northeast-1 000000000000
  OK   : us-east-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ us-east-2 000000000000
  OK   : ap-southeast-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-southeast-1 000000000000
  OK   : ap-northeast-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-northeast-2 000000000000
  OK   : eu-north-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .......... eu-north-1 000000000000
  OK   : us-west-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ us-west-1 000000000000
  OK   : eu-west-3 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ eu-west-3 000000000000
  OK   : ca-central-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ...... ca-central-1 000000000000
  OK   : eu-central-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ...... eu-central-1 000000000000
  OK   : eu-west-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ eu-west-2 000000000000
  SKIP : Region is opted out. ......................................................................................................... ap-east-1 000000000000
  SKIP : Region is opted out. ........................................................................................................ af-south-1 000000000000
  SKIP : Region is opted out. ........................................................................................................ eu-south-1 000000000000
  SKIP : Region is opted out. .................................................................................................... ap-southeast-3 000000000000
  SKIP : Region is opted out. ........................................................................................................ me-south-1 000000000000

Summary

OK ........................................................................................................................................... 16 [========  ]
SKIP .......................................................................................................................................... 5 [===       ]
INFO .......................................................................................................................................... 0 [          ]
ALARM ......................................................................................................................................... 1 [=         ]
ERROR ......................................................................................................................................... 0 [          ]

TOTAL .................................................................................................................................... 1 / 22 [==========]

Copy link
Contributor

@cbruno10 cbruno10 left a comment

Choose a reason for hiding this comment

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

@yorinasub17 Please see comment/questions, thanks!

case
when status ->> 'LastStatus' = 'SUCCESS' then ' and LastStatus is SUCCESS.'
else ' and LastStatus is not SUCCESS.'
when a.opt_in_status = 'not-opted-in' then 'Region is opted out.'
Copy link
Contributor

@cbruno10 cbruno10 Jul 6, 2022

Choose a reason for hiding this comment

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

Suggested change
when a.opt_in_status = 'not-opted-in' then 'Region is opted out.'
when a.opt_in_status = 'not-opted-in' then a.region || ' disabled.'

For instance: ap-southeast-3 disabled.

I think we should include the region name in the reason to keep all of the reasons consistent. I also wonder if we should use the term disabled instead of opted out? The column/property uses the terms opt-in and opt out, but in all AWS documentation, they use enabled and disabled, e.g.,

@yorinasub17 Do you think the term disabled vs. opt out be confusing, or would it convey the same message?

Copy link
Contributor

@cbruno10 cbruno10 Jul 6, 2022

Choose a reason for hiding this comment

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

Or if this is more clear:

Suggested change
when a.opt_in_status = 'not-opted-in' then 'Region is opted out.'
when a.opt_in_status = 'not-opted-in' then a.region || ' region is disabled.'

For instance: ap-southeast-3 region is disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should include the region name in the reason to keep all of the reasons consistent.

Sounds good to me.

I also wonder if we should use the term disabled instead of opted out?

That's a good question! I actually used disabled originally, and switched to opted out to be consistent with the parameter, but you are right that most of the AWS documentation and UI use the enable/disable terminology, so probably does make sense to be consistent there.

I accepted the second suggestion!

Here is what the output looks like now:

%~> aws-vault exec logs -- steampipe check control.cis_v140_3_5 --workspace-chdir .

+ 3.5 Ensure AWS Config is enabled in all regions ........................................................................................ 1 / 22 [==========]
  |
  ALARM: ap-northeast-3 IncludeGlobalResourceTypes disabled, AllSupported disabled, Recording disabled and LastStatus is not SUCC… ap-northeast-3 000000000000
  OK   : us-west-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ us-west-1 000000000000
  OK   : ap-southeast-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-southeast-2 000000000000
  OK   : eu-west-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ eu-west-1 000000000000
  OK   : ap-southeast-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-southeast-1 000000000000
  OK   : us-east-1 IncludeGlobalResourceTypes enabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............. us-east-1 000000000000
  OK   : us-east-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ us-east-2 000000000000
  OK   : eu-north-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .......... eu-north-1 000000000000
  OK   : us-west-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ us-west-2 000000000000
  OK   : ca-central-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ...... ca-central-1 000000000000
  OK   : eu-west-3 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ eu-west-3 000000000000
  OK   : eu-central-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ...... eu-central-1 000000000000
  OK   : ap-northeast-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-northeast-2 000000000000
  OK   : sa-east-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ sa-east-1 000000000000
  OK   : ap-northeast-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .. ap-northeast-1 000000000000
  OK   : eu-west-2 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. ............ eu-west-2 000000000000
  OK   : ap-south-1 IncludeGlobalResourceTypes disabled, AllSupported enabled, Recording enabled and LastStatus is SUCCESS. .......... ap-south-1 000000000000
  SKIP : ap-southeast-3 region is disabled. ...................................................................................... ap-southeast-3 000000000000
  SKIP : eu-south-1 region is disabled. .............................................................................................. eu-south-1 000000000000
  SKIP : ap-east-1 region is disabled. ................................................................................................ ap-east-1 000000000000
  SKIP : me-south-1 region is disabled. .............................................................................................. me-south-1 000000000000
  SKIP : af-south-1 region is disabled. .............................................................................................. af-south-1 000000000000

Summary

OK ........................................................................................................................................... 16 [========  ]
SKIP .......................................................................................................................................... 5 [===       ]
INFO .......................................................................................................................................... 0 [          ]
ALARM ......................................................................................................................................... 1 [=         ]
ERROR ......................................................................................................................................... 0 [          ]

TOTAL .................................................................................................................................... 1 / 22 [==========]

@cbruno10 cbruno10 merged commit aded5eb into turbot:release/v0.38 Jul 7, 2022
@cbruno10
Copy link
Contributor

cbruno10 commented Jul 7, 2022

@yorinasub17 Thanks again for the PR!

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