Skip to content

fix(kms): a denied ListKeys leaves an empty inventory, so KMS checks report nothing and CMK checks report an unverified FAIL #12711

Description

@lydiavilchez

Context

KMS._list_keys catches any collection failure, logs it and returns, leaving self.keys as it was:

def _list_keys(self, regional_client):
    try:
        ...
        self.keys.append(Key(id=..., arn=..., region=...))
    except Exception as error:
        logger.error(...)

KMS._describe_key does the same, leaving manager, state, origin, spec and multi_region as None on a key that is otherwise present in the inventory.

Nothing downstream can tell those failures apart from a legitimately empty result. For every consumer of kms_client.keys, "this account has no KMS keys" and "the keys could not be listed" are the same empty list, and "this key is AWS-managed" and "this key could not be described" are the same manager is None.

The service already applies the opposite treatment to key policies, so the pattern is established one field away:

class Key(BaseModel):
    ...
    # Populated by _get_key_policy on API failure. Distinguishes "policy not
    # applicable" (None + no error) from "policy could not be fetched" (None +
    # error class name). Checks that make security assertions from the policy
    # should emit MANUAL when this is set, not silently skip the key.
    policy_fetch_error: Optional[str] = None

Consumers of kms_client.keys today:

  • kms_cmk_are_used
  • kms_cmk_not_deleted_unintentionally
  • kms_cmk_not_multi_region
  • kms_cmk_rotation_enabled
  • kms_key_not_publicly_accessible
  • kms_key_enclave_attestation_not_enforced
  • kms_key_enclave_attestation_no_deployment_binding
  • kms_key_enclave_attestation_unknown_image
  • kms_key_enclave_attestation_bypassable_path
  • kms_key_enclave_attestation_pcr_mismatch
  • kms_key_enclave_debug_attestation_detected
  • kafka_cluster_encryption_at_rest_uses_cmk

Steps to Reproduce

Deny kms:ListKeys for the audit role, or simulate it, and run any check that reads the key inventory:

error = ClientError(
    {"Error": {"Code": "AccessDenied", "Message": "not authorized"}}, "ListKeys"
)
# regional_client.get_paginator raises the error above
kms = KMS(provider)

kms_client.keys = kms.keys
kms_cmk_rotation_enabled().execute()

Actual Result

kms.keys after AccessDenied on ListKeys: []
findings from kms_cmk_rotation_enabled: 0

Zero findings. The KMS section of the report is empty and indistinguishable from an account with no keys, so the scan looks clean while nothing was inspected.

The cross-service consumers degrade differently. kafka_cluster_encryption_at_rest_uses_cmk matches its cluster key against the empty inventory and reports:

FAIL  Kafka cluster 'x' does not have encryption at rest enabled with a CMK.

which is a claim it never had the evidence to make.

Expected Result

A region whose keys could not be listed produces a MANUAL finding naming the region and the error, and a key whose manager could not be resolved produces a MANUAL finding rather than a FAIL or a silent skip. Nothing that was not inspected is reported as inspected.

Possible fixes

Follow what the Bedrock service already does.

  1. A regional error map for the listing, mirroring bedrock_client.custom_models_scan_errors:
self.keys_scan_errors = {}          # region -> error code
...
except ClientError as error:
    self.keys_scan_errors[regional_client.region] = error.response["Error"]["Code"]
  1. Per-key detail tracking, mirroring detail_retrieved on the Bedrock models and the existing Key.policy_fetch_error:
class Key(BaseModel):
    ...
    detail_retrieved: bool = False   # set by _describe_key
  1. Update the consumers above to emit MANUAL instead of skipping a region or asserting an unverified FAIL. bedrock_custom_model_encrypted_with_cmk is the reference implementation: it emits one MANUAL per region that could not be listed, before evaluating any resource.

A cross-service check can work around part of this on its own. ecr_repository_encrypted_with_cmk (#12618, PR #12665) distinguishes "the key is not in the inventory" from "the key is in the inventory and is AWS-managed" and reports MANUAL for the first, without touching the KMS service. That covers the false FAIL, but it cannot name the region or the cause, and it does nothing for the 11 checks inside the KMS service, which only ever see an empty list.

Impact

Affects any account audited with a scoped role, which is the recommended setup. A single denied permission silently removes the whole KMS surface from the report, and the person reading the findings has no signal that it happened: the error only reaches logger.error and never makes it to the report, the exported outputs or the UI.

Found while reviewing #12665.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugprovider/awsIssues/PRs related with the AWS providerseverity/mediumResults in some unexpected or undesired behavior.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions