fix(oci): false positive for kms key rotation check (#10450)

2026-07-23 20:42:02 +00:00 · 2026-03-25 11:09:02 +01:00
parent 4d1f7626f9
commit c752811666
5 changed files with 174 additions and 157 deletions
@@ -17,6 +17,7 @@ All notable changes to the **Prowler SDK** are documented in this file.

 - Oracle Cloud `events_rule_idp_group_mapping_changes` now recognizes the CIS 3.1 `add/remove` event names to avoid false positives [(#10416)](https://github.com/prowler-cloud/prowler/pull/10416)
 - Oracle Cloud password policy checks now exclude immutable system-managed policies (`SimplePasswordPolicy`, `StandardPasswordPolicy`) to avoid false positives [(#10453)](https://github.com/prowler-cloud/prowler/pull/10453)
+- Oracle Cloud `kms_key_rotation_enabled` now checks current key version age to avoid false positives on vaults without auto-rotation support [(#10450)](https://github.com/prowler-cloud/prowler/pull/10450)

 ---

@@ -9,7 +9,7 @@
  "Severity": "high",
  "ResourceType": "Key",
  "ResourceGroup": "security",
-  "Description": "**OCI KMS customer-managed keys** configured for **automatic rotation** or with a rotation interval set to `<= 365` days.",
+  "Description": "**OCI KMS customer-managed keys** configured for **automatic rotation**, with a rotation interval set to `<= 365` days, or **manually rotated** within the last 365 days. Some vault types do not support auto-rotation, so manual rotation is accepted as an alternative.",
  "Risk": "Without regular rotation, a compromised key can be used longer to decrypt data at rest and backups or to forge signatures. This erodes **confidentiality** and **integrity**, increases the blast radius, and complicates incident response due to broad reuse of the same key version.",
  "RelatedUrl": "",
  "AdditionalURLs": [
@@ -20,11 +20,11 @@
    "Code": {
      "CLI": "oci kms management key update --key-id <example_resource_id> --endpoint <example_management_endpoint> --is-auto-rotation-enabled true --auto-key-rotation-details '{\"rotationIntervalInDays\": 365}'",
      "NativeIaC": "",
-      "Other": "1. In OCI Console, go to Identity & Security > Vault\n2. Open the vault, then under Resources select Master Encryption Keys\n3. Click the target key name\n4. Click Edit auto-rotation settings\n5. Enable Auto rotation and set Rotation interval to 365 days (or less)\n6. Click Update",
+      "Other": "1. In OCI Console, go to Identity & Security > Vault\n2. Open the vault, then under Resources select Master Encryption Keys\n3. Click the target key name\n4. For vaults that support auto-rotation: Click Edit auto-rotation settings, enable Auto rotation, set Rotation interval to 365 days (or less), and click Update\n5. For vaults that do not support auto-rotation (e.g., External or Virtual Private vaults): Click 'Rotate Key' to manually rotate the key version at least once every 365 days",
      "Terraform": "```hcl\nresource \"oci_kms_key\" \"<example_resource_name>\" {\n  compartment_id      = \"<example_resource_id>\"\n  display_name        = \"<example_resource_name>\"\n  management_endpoint = \"<example_management_endpoint>\"\n\n  key_shape {\n    algorithm = \"AES\"\n    length    = 16\n  }\n\n  is_auto_rotation_enabled = true  # Critical: enables auto rotation\n  auto_key_rotation_details {\n    rotation_interval_in_days = 365  # Critical: interval <= 365 days\n  }\n}\n```"
    },
    "Recommendation": {
-      "Text": "Enable **automatic key rotation** and set an interval `<= 365` days (*shorter for sensitive data*). Apply **least privilege** and **separation of duties** for key administration. Monitor rotation status, retire old key versions, and ensure applications handle key versioning to prevent outages.",
+      "Text": "Enable **automatic key rotation** and set an interval `<= 365` days (*shorter for sensitive data*). For vault types that do not support auto-rotation (e.g., External or Virtual Private vaults), **manually rotate** the key at least once every 365 days. Apply **least privilege** and **separation of duties** for key administration. Monitor rotation status, retire old key versions, and ensure applications handle key versioning to prevent outages.",
      "Url": "https://hub.prowler.com/check/kms_key_rotation_enabled"
    }
  },
@@ -1,5 +1,7 @@
 """Check Ensure customer created Customer Managed Key (CMK) is rotated at least annually."""

+from datetime import datetime, timedelta, timezone
+
 from prowler.lib.check.models import Check, Check_Report_OCI
 from prowler.providers.oraclecloud.services.kms.kms_client import kms_client

@@ -21,16 +23,32 @@ class kms_key_rotation_enabled(Check):
                compartment_id=key.compartment_id,
            )

-            # Check if auto-rotation is enabled OR if rotation interval is set and <= 365 days
-            if key.is_auto_rotation_enabled or (
-                key.rotation_interval_in_days is not None
-                and key.rotation_interval_in_days <= 365
+            now = datetime.now(timezone.utc)
+            max_age = timedelta(days=365)
+
+            manually_rotated_recently = (
+                key.current_key_version_time_created is not None
+                and (now - key.current_key_version_time_created) <= max_age
+            )
+
+            if (
+                key.is_auto_rotation_enabled
+                or (
+                    key.rotation_interval_in_days is not None
+                    and key.rotation_interval_in_days <= 365
+                )
+                or manually_rotated_recently
            ):
                report.status = "PASS"
-                report.status_extended = f"KMS key '{key.name}' has rotation enabled (auto-rotation: {key.is_auto_rotation_enabled}, interval: {key.rotation_interval_in_days} days)."
+                if key.is_auto_rotation_enabled:
+                    report.status_extended = f"KMS key {key.name} has auto-rotation enabled with interval of {key.rotation_interval_in_days} days."
+                elif manually_rotated_recently:
+                    report.status_extended = f"KMS key {key.name} was manually rotated within the last 365 days."
+                else:
+                    report.status_extended = f"KMS key {key.name} has rotation interval set to {key.rotation_interval_in_days} days."
            else:
                report.status = "FAIL"
-                report.status_extended = f"KMS key '{key.name}' does not have rotation enabled or rotation interval exceeds 365 days."
+                report.status_extended = f"KMS key {key.name} has not been rotated within the last 365 days and does not have auto-rotation enabled."

            findings.append(report)

@@ -1,5 +1,6 @@
 """OCI Kms Service Module."""

+from datetime import datetime
 from typing import Optional

 import oci
@@ -78,6 +79,25 @@ class Kms(OCIService):
                                        key_id=key_summary.id
                                    ).data

+                                    # Fetch current key version to get its creation time
+                                    current_key_version_time_created = None
+                                    if (
+                                        hasattr(key_details, "current_key_version")
+                                        and key_details.current_key_version
+                                    ):
+                                        try:
+                                            key_version = kms_management_client.get_key_version(
+                                                key_id=key_details.id,
+                                                key_version_id=key_details.current_key_version,
+                                            ).data
+                                            current_key_version_time_created = (
+                                                key_version.time_created
+                                            )
+                                        except Exception as version_error:
+                                            logger.warning(
+                                                f"Could not fetch key version for {key_details.id}: {version_error}"
+                                            )
+
                                    self.keys.append(
                                        Key(
                                            id=key_details.id,
@@ -110,6 +130,7 @@ class Kms(OCIService):
                                                )
                                                else None
                                            ),
+                                            current_key_version_time_created=current_key_version_time_created,
                                        )
                                    )
                except Exception as error:
@@ -134,3 +155,4 @@ class Key(BaseModel):
    lifecycle_state: str
    is_auto_rotation_enabled: bool = False
    rotation_interval_in_days: Optional[int] = None
+    current_key_version_time_created: Optional[datetime] = None
@@ -1,5 +1,7 @@
+from datetime import datetime, timedelta, timezone
 from unittest import mock

+from prowler.providers.oraclecloud.services.kms.kms_service import Key
 from tests.providers.oraclecloud.oci_fixtures import (
    OCI_COMPARTMENT_ID,
    OCI_REGION,
@@ -7,36 +9,17 @@ from tests.providers.oraclecloud.oci_fixtures import (
    set_mocked_oraclecloud_provider,
 )

+KEY_ID = "ocid1.key.oc1.iad.aaaaaaaexample"
+KEY_NAME = "test-key"
+

 class Test_kms_key_rotation_enabled:
-    def test_no_resources(self):
-        """kms_key_rotation_enabled: No resources to check"""
+    def test_no_keys(self):
+        """No keys → empty findings."""
        kms_client = mock.MagicMock()
        kms_client.audited_compartments = {OCI_COMPARTMENT_ID: mock.MagicMock()}
        kms_client.audited_tenancy = OCI_TENANCY_ID
-
-        # Mock empty collections
-        kms_client.rules = []
-        kms_client.topics = []
-        kms_client.subscriptions = []
-        kms_client.users = []
-        kms_client.groups = []
-        kms_client.policies = []
-        kms_client.compartments = []
-        kms_client.instances = []
-        kms_client.volumes = []
-        kms_client.boot_volumes = []
-        kms_client.buckets = []
        kms_client.keys = []
-        kms_client.file_systems = []
-        kms_client.databases = []
-        kms_client.security_lists = []
-        kms_client.security_groups = []
-        kms_client.subnets = []
-        kms_client.vcns = []
-        kms_client.configuration = None
-        kms_client.active_non_root_compartments = []
-        kms_client.password_policy = None

        with (
            mock.patch(
@@ -55,49 +38,24 @@ class Test_kms_key_rotation_enabled:
            check = kms_key_rotation_enabled()
            result = check.execute()

-            # Verify result is a list (empty or with findings)
-            assert isinstance(result, list)
+            assert result == []

-    def test_resource_compliant(self):
-        """kms_key_rotation_enabled: Resource passes the check (PASS)"""
+    def test_key_with_auto_rotation_enabled(self):
+        """Key with auto-rotation enabled → PASS."""
        kms_client = mock.MagicMock()
        kms_client.audited_compartments = {OCI_COMPARTMENT_ID: mock.MagicMock()}
        kms_client.audited_tenancy = OCI_TENANCY_ID
-
-        # Mock a compliant resource
-        resource = mock.MagicMock()
-        resource.id = "ocid1.resource.oc1.iad.aaaaaaaexample"
-        resource.name = "compliant-resource"
-        resource.region = OCI_REGION
-        resource.compartment_id = OCI_COMPARTMENT_ID
-        resource.lifecycle_state = "ACTIVE"
-        resource.tags = {"Environment": "Production"}
-
-        # Set attributes that make the resource compliant
-        resource.versioning = "Enabled"
-        resource.is_auto_rotation_enabled = True
-        resource.rotation_interval_in_days = 90
-        resource.public_access_type = "NoPublicAccess"
-        resource.logging_enabled = True
-        resource.kms_key_id = "ocid1.key.oc1.iad.aaaaaaaexample"
-        resource.in_transit_encryption = "ENABLED"
-        resource.is_secure_boot_enabled = True
-        resource.legacy_endpoint_disabled = True
-        resource.is_legacy_imds_endpoint_disabled = True
-
-        # Mock client with compliant resource
-        kms_client.buckets = [resource]
-        kms_client.keys = [resource]
-        kms_client.volumes = [resource]
-        kms_client.boot_volumes = [resource]
-        kms_client.instances = [resource]
-        kms_client.file_systems = [resource]
-        kms_client.databases = [resource]
-        kms_client.security_lists = []
-        kms_client.security_groups = []
-        kms_client.rules = []
-        kms_client.configuration = resource
-        kms_client.users = []
+        kms_client.keys = [
+            Key(
+                id=KEY_ID,
+                name=KEY_NAME,
+                compartment_id=OCI_COMPARTMENT_ID,
+                region=OCI_REGION,
+                lifecycle_state="ENABLED",
+                is_auto_rotation_enabled=True,
+                rotation_interval_in_days=90,
+            )
+        ]

        with (
            mock.patch(
@@ -116,73 +74,32 @@ class Test_kms_key_rotation_enabled:
            check = kms_key_rotation_enabled()
            result = check.execute()

-            assert isinstance(result, list)
+            assert len(result) == 1
+            assert result[0].status == "PASS"
+            assert "auto-rotation enabled" in result[0].status_extended
+            assert result[0].resource_id == KEY_ID
+            assert result[0].resource_name == KEY_NAME
+            assert result[0].region == OCI_REGION
+            assert result[0].compartment_id == OCI_COMPARTMENT_ID

-            # If results exist, verify PASS findings
-            if len(result) > 0:
-                # Find PASS results
-                pass_results = [r for r in result if r.status == "PASS"]
-
-                if pass_results:
-                    # Detailed assertions on first PASS result
-                    assert pass_results[0].status == "PASS"
-                    assert pass_results[0].status_extended is not None
-                    assert len(pass_results[0].status_extended) > 0
-
-                    # Verify resource identification
-                    assert pass_results[0].resource_id is not None
-                    assert pass_results[0].resource_name is not None
-                    assert pass_results[0].region is not None
-                    assert pass_results[0].compartment_id is not None
-
-                    # Verify metadata
-                    assert pass_results[0].check_metadata.Provider == "oraclecloud"
-                    assert (
-                        pass_results[0].check_metadata.CheckID
-                        == "kms_key_rotation_enabled"
-                    )
-                    assert pass_results[0].check_metadata.ServiceName == "kms"
-
-    def test_resource_non_compliant(self):
-        """kms_key_rotation_enabled: Resource fails the check (FAIL)"""
+    def test_key_manually_rotated_within_365_days(self):
+        """Key manually rotated within last 365 days (no auto-rotation) → PASS."""
        kms_client = mock.MagicMock()
        kms_client.audited_compartments = {OCI_COMPARTMENT_ID: mock.MagicMock()}
        kms_client.audited_tenancy = OCI_TENANCY_ID
-
-        # Mock a non-compliant resource
-        resource = mock.MagicMock()
-        resource.id = "ocid1.resource.oc1.iad.bbbbbbbexample"
-        resource.name = "non-compliant-resource"
-        resource.region = OCI_REGION
-        resource.compartment_id = OCI_COMPARTMENT_ID
-        resource.lifecycle_state = "ACTIVE"
-        resource.tags = {"Environment": "Development"}
-
-        # Set attributes that make the resource non-compliant
-        resource.versioning = "Disabled"
-        resource.is_auto_rotation_enabled = False
-        resource.rotation_interval_in_days = None
-        resource.public_access_type = "ObjectRead"
-        resource.logging_enabled = False
-        resource.kms_key_id = None
-        resource.in_transit_encryption = "DISABLED"
-        resource.is_secure_boot_enabled = False
-        resource.legacy_endpoint_disabled = False
-        resource.is_legacy_imds_endpoint_disabled = False
-
-        # Mock client with non-compliant resource
-        kms_client.buckets = [resource]
-        kms_client.keys = [resource]
-        kms_client.volumes = [resource]
-        kms_client.boot_volumes = [resource]
-        kms_client.instances = [resource]
-        kms_client.file_systems = [resource]
-        kms_client.databases = [resource]
-        kms_client.security_lists = []
-        kms_client.security_groups = []
-        kms_client.rules = []
-        kms_client.configuration = resource
-        kms_client.users = []
+        kms_client.keys = [
+            Key(
+                id=KEY_ID,
+                name=KEY_NAME,
+                compartment_id=OCI_COMPARTMENT_ID,
+                region=OCI_REGION,
+                lifecycle_state="ENABLED",
+                is_auto_rotation_enabled=False,
+                rotation_interval_in_days=None,
+                current_key_version_time_created=datetime.now(timezone.utc)
+                - timedelta(days=100),
+            )
+        ]

        with (
            mock.patch(
@@ -201,29 +118,88 @@ class Test_kms_key_rotation_enabled:
            check = kms_key_rotation_enabled()
            result = check.execute()

-            assert isinstance(result, list)
+            assert len(result) == 1
+            assert result[0].status == "PASS"
+            assert "manually rotated" in result[0].status_extended
+            assert result[0].resource_id == KEY_ID

-            # Verify FAIL findings exist
-            if len(result) > 0:
-                # Find FAIL results
-                fail_results = [r for r in result if r.status == "FAIL"]
+    def test_key_manually_rotated_over_365_days_ago(self):
+        """Key manually rotated more than 365 days ago (no auto-rotation) → FAIL."""
+        kms_client = mock.MagicMock()
+        kms_client.audited_compartments = {OCI_COMPARTMENT_ID: mock.MagicMock()}
+        kms_client.audited_tenancy = OCI_TENANCY_ID
+        kms_client.keys = [
+            Key(
+                id=KEY_ID,
+                name=KEY_NAME,
+                compartment_id=OCI_COMPARTMENT_ID,
+                region=OCI_REGION,
+                lifecycle_state="ENABLED",
+                is_auto_rotation_enabled=False,
+                rotation_interval_in_days=None,
+                current_key_version_time_created=datetime.now(timezone.utc)
+                - timedelta(days=400),
+            )
+        ]

-                if fail_results:
-                    # Detailed assertions on first FAIL result
-                    assert fail_results[0].status == "FAIL"
-                    assert fail_results[0].status_extended is not None
-                    assert len(fail_results[0].status_extended) > 0
+        with (
+            mock.patch(
+                "prowler.providers.common.provider.Provider.get_global_provider",
+                return_value=set_mocked_oraclecloud_provider(),
+            ),
+            mock.patch(
+                "prowler.providers.oraclecloud.services.kms.kms_key_rotation_enabled.kms_key_rotation_enabled.kms_client",
+                new=kms_client,
+            ),
+        ):
+            from prowler.providers.oraclecloud.services.kms.kms_key_rotation_enabled.kms_key_rotation_enabled import (
+                kms_key_rotation_enabled,
+            )

-                    # Verify resource identification
-                    assert fail_results[0].resource_id is not None
-                    assert fail_results[0].resource_name is not None
-                    assert fail_results[0].region is not None
-                    assert fail_results[0].compartment_id is not None
+            check = kms_key_rotation_enabled()
+            result = check.execute()

-                    # Verify metadata
-                    assert fail_results[0].check_metadata.Provider == "oraclecloud"
-                    assert (
-                        fail_results[0].check_metadata.CheckID
-                        == "kms_key_rotation_enabled"
-                    )
-                    assert fail_results[0].check_metadata.ServiceName == "kms"
+            assert len(result) == 1
+            assert result[0].status == "FAIL"
+            assert "not been rotated" in result[0].status_extended
+            assert result[0].resource_id == KEY_ID
+
+    def test_key_no_rotation_at_all(self):
+        """Key with no auto-rotation and no version info → FAIL."""
+        kms_client = mock.MagicMock()
+        kms_client.audited_compartments = {OCI_COMPARTMENT_ID: mock.MagicMock()}
+        kms_client.audited_tenancy = OCI_TENANCY_ID
+        kms_client.keys = [
+            Key(
+                id=KEY_ID,
+                name=KEY_NAME,
+                compartment_id=OCI_COMPARTMENT_ID,
+                region=OCI_REGION,
+                lifecycle_state="ENABLED",
+                is_auto_rotation_enabled=False,
+                rotation_interval_in_days=None,
+                current_key_version_time_created=None,
+            )
+        ]
+
+        with (
+            mock.patch(
+                "prowler.providers.common.provider.Provider.get_global_provider",
+                return_value=set_mocked_oraclecloud_provider(),
+            ),
+            mock.patch(
+                "prowler.providers.oraclecloud.services.kms.kms_key_rotation_enabled.kms_key_rotation_enabled.kms_client",
+                new=kms_client,
+            ),
+        ):
+            from prowler.providers.oraclecloud.services.kms.kms_key_rotation_enabled.kms_key_rotation_enabled import (
+                kms_key_rotation_enabled,
+            )
+
+            check = kms_key_rotation_enabled()
+            result = check.execute()
+
+            assert len(result) == 1
+            assert result[0].status == "FAIL"
+            assert "not been rotated" in result[0].status_extended
+            assert result[0].resource_id == KEY_ID