turbot / steampipe-plugin-aws

Use SQL to instantly query AWS resources across regions and accounts. Open source CLI. No DB required.
https://hub.steampipe.io/plugins/turbot/aws
Apache License 2.0
188 stars 98 forks source link

kms:GetKeyRotationStatus alerts when running compliance checks #790

Closed ecktom closed 2 years ago

ecktom commented 2 years ago

Based on the AWS CIS Benchmark, we've setup an alert for AccessDenied and UnauthorizedOperation events (https://github.com/turbot/steampipe-mod-aws-compliance/blob/main/query/cloudwatch/log_metric_filter_unauthorized_api.sql#L27).

When running the CIS 1.4 compliance check (https://github.com/turbot/steampipe-mod-aws-compliance/tree/main/cis_v140) this alert always triggers for kms:GetKeyRotationStatus.

This happens due to the check https://github.com/turbot/steampipe-mod-aws-compliance/blob/main/query/kms/kms_cmk_rotation_enabled.sql and the corresponding hydrate implementation within the plugin https://github.com/turbot/steampipe-plugin-aws/blob/main/aws/table_aws_kms_key.go#L269.

This Issue could be mitigated by just calling svc.GetKeyRotationStatus() for customer managed keys which of course need's another svc.DescribeKey() roundtrip first

bigdatasourav commented 2 years ago

@ecktom thank you for highlighting this with us. We are looking into the feasibility, will make the appropriate action.

ecktom commented 2 years ago

@bigdatasourav is there anything I can help with to speed this up? ;)

cbruno10 commented 2 years ago

Hey @ecktom , sorry for the late response on this, we've been doing some testing and discussing the best approach to this issue.

We ran several queries against our accounts which have a number of AWS and customer managed keys to see how many resulted in the errors you first mentioned, and found that only 1-2 in our accounts (namely aws/acm) would result in that error.

However, since almost all of the AWS managed keys (and customer managed keys) actually return the rotation status OK, we're reluctant to have the KMS key table avoid making the kms:GetKeyRotationStatus call for all AWS managed keys, since we'd lose data we get back from the API.

Are you seeing a lot of AWS managed keys throwing the errors you mentioned when Steampipe makes the kms:GetKeyRotationStatus call? If so, can you please share which AWS managed keys in particular are throwing the errors so we can cross-check with our results?

We'd like to be able to filter out AWS managed vs. customer managed keys at the API layer instead of having PostgreSQL filter it after we get all of the results, which would mean the query in the AWS Compliance mod would never attempt to make the API calls in the first place; however, the API doesn't seem to support passing in any parameters when making the kms:ListKeys call as per ListKeys. We've raised a request in the AWS SDK Go repo to see if there's any chance of them adding that, https://github.com/aws/aws-sdk/issues/284.

We also believe that the AWS managed keys shouldn't throw errors when trying to get their key rotation status, as we've only found a handful that result in these errors, so they seem like errors from AWS' end. We've raised a support request in one of our AWS accounts asking why these particular keys, e.g., aws/acm, throw errors, while a majority of the others don't.

We'll keep this thread updated with any progress or findings from the GitHub issue and AWS support request mentioned above.

In the meantime, if it's still a major blocker for you, our recommendation is to fork the AWS plugin repo, make the change you had suggested above (skipping the kms:GetKeyRotationStatus call for all AWS managed keys), and then build the AWS plugin locally using the make command (which will automatically replace the binary on your system). Afterward, you can run the AWS Compliance mod against that local version.

ecktom commented 2 years ago

@cbruno10 Many Thanks for coming back to this issue and your detailed explanations. Really appreciated.

I double checked our alerts and can confirm that all of the alerts I can see are somewhat related to aws/acm in multiple regions. I agree on your explanations here and would be glad if this can get fixed on API level.

It's not a major blocker but rather quiet annoying as our internal altering triggers whenever re run those compliance checks ;) But I understand that this one is not fully in your hands, so we'll just be patient and hope that AWS finds a quick solution.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

cbruno10 commented 2 years ago

Unfortunately, there's still no reply from the SDK team in aws/aws-sdk#284.

@rajlearner17 I couldn't find the support case we raised with AWS earlier about certain AWS managed keys that throw an error when attempting to get the rotation status for those, are you able to check that case's status if you recall which account we raised it in?

cbruno10 commented 2 years ago

@ecktom As a workaround, we've recently added the ignore_error_codes config arg as part of v0.60.0. I believe if you add this config arg with the AccessDenied and UnauthorizedOperation error codes included, any KMS key we can't describe will now be skipped instead of throwing an error.

I checked again, and still no response from the AWS SDK team in https://github.com/aws/aws-sdk/issues/284. Since we haven't received a response from the AWS SDK team and it doesn't look like they'll get around to it soon, I'm closing this issue as resolved, but if you still see any issues, feel free to re-open. Thanks!

ecktom commented 2 years ago

@cbruno10 Thanks for coming back to this. I don't think it could improve our situation, it's not really an error which get's thrown while checking the KMS keys. It's is raised by the CIS alarm (https://github.com/turbot/steampipe-mod-aws-compliance/blob/main/query/cloudwatch/log_metric_filter_unauthorized_api.sql#L27) which listens for any AccessDenied or UnauthorizedOperation actions. So if there is no way to prevent those calls at all, this alert obviously will keep triggering ;)