-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(azure): add vault parallelization in keyvault service
#9876
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
Open
HugoPBrito
wants to merge
6
commits into
master
Choose a base branch
from
PROWLER-737-clone-scans-running-for-more-than-24-hours-in-production-sdk-root-cause
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+241
−127
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
4baf8e8
feat: add vault parallelization
HugoPBrito 75f370d
fix: tests
HugoPBrito be47cec
Merge branch 'master' of https://github.com/prowler-cloud/prowler int…
HugoPBrito 1bd0744
chore: add to changelog
HugoPBrito 825f329
chore: remove test terraform file from repo
HugoPBrito 3e297bf
chore: remove debug logging from keyvault service
HugoPBrito File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,22 +5,21 @@ | |
| class keyvault_rbac_secret_expiration_set(Check): | ||
| def execute(self) -> Check_Report_Azure: | ||
| findings = [] | ||
|
|
||
| for subscription, key_vaults in keyvault_client.key_vaults.items(): | ||
| for keyvault in key_vaults: | ||
| if keyvault.properties.enable_rbac_authorization and keyvault.secrets: | ||
| report = Check_Report_Azure( | ||
| metadata=self.metadata(), resource=keyvault | ||
| ) | ||
| report.subscription = subscription | ||
| report.status = "PASS" | ||
| report.status_extended = f"Keyvault {keyvault.name} from subscription {subscription} has all the secrets with expiration date set." | ||
| has_secret_without_expiration = False | ||
| for secret in keyvault.secrets: | ||
| report = Check_Report_Azure( | ||
| metadata=self.metadata(), resource=secret | ||
| ) | ||
| report.subscription = subscription | ||
| if not secret.attributes.expires and secret.enabled: | ||
| report.status = "FAIL" | ||
| report.status_extended = f"Keyvault {keyvault.name} from subscription {subscription} has the secret {secret.name} without expiration date set." | ||
| has_secret_without_expiration = True | ||
| findings.append(report) | ||
| if not has_secret_without_expiration: | ||
| report.status_extended = f"Secret '{secret.name}' in KeyVault '{keyvault.name}' does not have expiration date set." | ||
| else: | ||
| report.status = "PASS" | ||
| report.status_extended = f"Secret '{secret.name}' in KeyVault '{keyvault.name}' has expiration date set." | ||
| findings.append(report) | ||
|
|
||
| return findings | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| from concurrent.futures import ThreadPoolExecutor | ||
| from dataclasses import dataclass | ||
| from datetime import datetime | ||
| from typing import List, Optional, Union | ||
|
|
@@ -16,103 +17,149 @@ | |
| class KeyVault(AzureService): | ||
| def __init__(self, provider: AzureProvider): | ||
| super().__init__(KeyVaultManagementClient, provider) | ||
| # TODO: review this credentials assignment | ||
| self.key_vaults = self._get_key_vaults(provider) | ||
| self._provider = provider | ||
| self.key_vaults = self._get_key_vaults() | ||
|
|
||
| def _get_key_vaults(self, provider): | ||
| def _get_key_vaults(self): | ||
| """ | ||
| Get all KeyVaults with parallel processing. | ||
|
|
||
| Optimizations: | ||
| 1. Uses list_by_subscription() for full Vault objects | ||
| 2. Processes vaults in parallel using __threading_call__ | ||
| 3. Each vault's keys/secrets/monitor fetched in parallel | ||
| """ | ||
| logger.info("KeyVault - Getting key_vaults...") | ||
| key_vaults = {} | ||
|
|
||
| for subscription, client in self.clients.items(): | ||
| try: | ||
| key_vaults.update({subscription: []}) | ||
| key_vaults_list = client.vaults.list() | ||
| for keyvault in key_vaults_list: | ||
| resource_group = keyvault.id.split("/")[4] | ||
| keyvault_name = keyvault.name | ||
| keyvault_properties = client.vaults.get( | ||
| resource_group, keyvault_name | ||
| ).properties | ||
| keys = self._get_keys( | ||
| subscription, resource_group, keyvault_name, provider | ||
| ) | ||
| secrets = self._get_secrets( | ||
| subscription, resource_group, keyvault_name | ||
| ) | ||
| key_vaults[subscription].append( | ||
| KeyVaultInfo( | ||
| id=getattr(keyvault, "id", ""), | ||
| name=getattr(keyvault, "name", ""), | ||
| location=getattr(keyvault, "location", ""), | ||
| resource_group=resource_group, | ||
| properties=VaultProperties( | ||
| tenant_id=getattr(keyvault_properties, "tenant_id", ""), | ||
| enable_rbac_authorization=getattr( | ||
| keyvault_properties, | ||
| "enable_rbac_authorization", | ||
| False, | ||
| ), | ||
| private_endpoint_connections=[ | ||
| PrivateEndpointConnection(id=conn.id) | ||
| for conn in ( | ||
| getattr( | ||
| keyvault_properties, | ||
| "private_endpoint_connections", | ||
| [], | ||
| ) | ||
| or [] | ||
| ) | ||
| ], | ||
| enable_soft_delete=getattr( | ||
| keyvault_properties, "enable_soft_delete", False | ||
| ), | ||
| enable_purge_protection=getattr( | ||
| keyvault_properties, | ||
| "enable_purge_protection", | ||
| False, | ||
| ), | ||
| public_network_access_disabled=( | ||
| getattr( | ||
| keyvault_properties, | ||
| "public_network_access", | ||
| "Enabled", | ||
| ) | ||
| == "Disabled" | ||
| ), | ||
| ), | ||
| keys=keys, | ||
| secrets=secrets, | ||
| monitor_diagnostic_settings=self._get_vault_monitor_settings( | ||
| keyvault_name, resource_group, subscription | ||
| ), | ||
| ) | ||
| ) | ||
| key_vaults[subscription] = [] | ||
| vaults_list = list(client.vaults.list_by_subscription()) | ||
|
|
||
| if not vaults_list: | ||
| continue | ||
|
|
||
| # Prepare items for parallel processing | ||
| items = [ | ||
| {"subscription": subscription, "keyvault": vault} | ||
| for vault in vaults_list | ||
| ] | ||
|
|
||
| # Process all KeyVaults in parallel | ||
| results = self.__threading_call__(self._process_single_keyvault, items) | ||
| key_vaults[subscription] = results | ||
|
|
||
| except Exception as error: | ||
| logger.error( | ||
| f"Subscription name: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" | ||
| ) | ||
|
|
||
| return key_vaults | ||
|
|
||
| def _get_keys(self, subscription, resource_group, keyvault_name, provider): | ||
| logger.info(f"KeyVault - Getting keys for {keyvault_name}...") | ||
| def _process_single_keyvault(self, item: dict) -> Optional["KeyVaultInfo"]: | ||
| """Process a single KeyVault in parallel.""" | ||
| subscription = item["subscription"] | ||
| keyvault = item["keyvault"] | ||
|
|
||
| try: | ||
| resource_group = keyvault.id.split("/")[4] | ||
| keyvault_name = keyvault.name | ||
| keyvault_properties = keyvault.properties | ||
|
|
||
| # Fetch keys, secrets, and monitor in parallel | ||
| with ThreadPoolExecutor(max_workers=3) as executor: | ||
| keys_future = executor.submit( | ||
| self._get_keys, subscription, resource_group, keyvault_name | ||
| ) | ||
| secrets_future = executor.submit( | ||
| self._get_secrets, subscription, resource_group, keyvault_name | ||
| ) | ||
| monitor_future = executor.submit( | ||
| self._get_vault_monitor_settings, | ||
| keyvault_name, | ||
| resource_group, | ||
| subscription, | ||
| ) | ||
|
|
||
| keys = keys_future.result() | ||
| secrets = secrets_future.result() | ||
| monitor_settings = monitor_future.result() | ||
|
|
||
| return KeyVaultInfo( | ||
| id=getattr(keyvault, "id", ""), | ||
| name=getattr(keyvault, "name", ""), | ||
| location=getattr(keyvault, "location", ""), | ||
| resource_group=resource_group, | ||
| properties=VaultProperties( | ||
| tenant_id=getattr(keyvault_properties, "tenant_id", ""), | ||
| enable_rbac_authorization=getattr( | ||
| keyvault_properties, | ||
| "enable_rbac_authorization", | ||
| False, | ||
| ), | ||
| private_endpoint_connections=[ | ||
| PrivateEndpointConnection(id=conn.id) | ||
| for conn in ( | ||
| getattr( | ||
| keyvault_properties, | ||
| "private_endpoint_connections", | ||
| [], | ||
| ) | ||
| or [] | ||
| ) | ||
| ], | ||
| enable_soft_delete=getattr( | ||
| keyvault_properties, "enable_soft_delete", False | ||
| ), | ||
| enable_purge_protection=getattr( | ||
| keyvault_properties, | ||
| "enable_purge_protection", | ||
| False, | ||
| ), | ||
| public_network_access_disabled=( | ||
| getattr( | ||
| keyvault_properties, | ||
| "public_network_access", | ||
| "Enabled", | ||
| ) | ||
| == "Disabled" | ||
| ), | ||
| ), | ||
| keys=keys, | ||
| secrets=secrets, | ||
| monitor_diagnostic_settings=monitor_settings, | ||
| ) | ||
|
|
||
| except Exception as error: | ||
| logger.error( | ||
| f"KeyVault {keyvault.name} in {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" | ||
| ) | ||
| return None | ||
|
|
||
| def _get_keys(self, subscription, resource_group, keyvault_name): | ||
| keys = [] | ||
| keys_dict = {} | ||
|
|
||
| try: | ||
| client = self.clients[subscription] | ||
| keys_list = client.keys.list(resource_group, keyvault_name) | ||
| for key in keys_list: | ||
| keys.append( | ||
| Key( | ||
| id=getattr(key, "id", ""), | ||
| name=getattr(key, "name", ""), | ||
| key_obj = Key( | ||
| id=getattr(key, "id", ""), | ||
| name=getattr(key, "name", ""), | ||
| enabled=getattr(key.attributes, "enabled", False), | ||
| location=getattr(key, "location", ""), | ||
| attributes=KeyAttributes( | ||
| enabled=getattr(key.attributes, "enabled", False), | ||
| location=getattr(key, "location", ""), | ||
| attributes=KeyAttributes( | ||
| enabled=getattr(key.attributes, "enabled", False), | ||
| created=getattr(key.attributes, "created", 0), | ||
| updated=getattr(key.attributes, "updated", 0), | ||
| expires=getattr(key.attributes, "expires", 0), | ||
| ), | ||
| ) | ||
| created=getattr(key.attributes, "created", 0), | ||
| updated=getattr(key.attributes, "updated", 0), | ||
| expires=getattr(key.attributes, "expires", 0), | ||
| ), | ||
| ) | ||
| keys.append(key_obj) | ||
| keys_dict[key_obj.name] = key_obj | ||
|
|
||
| except Exception as error: | ||
| logger.error( | ||
| f"Subscription name: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" | ||
|
|
@@ -121,31 +168,52 @@ def _get_keys(self, subscription, resource_group, keyvault_name, provider): | |
| try: | ||
| key_client = KeyClient( | ||
| vault_url=f"https://{keyvault_name}.vault.azure.net/", | ||
| # TODO: review the following line | ||
| credential=provider.session, | ||
| credential=self._provider.session, | ||
| ) | ||
| properties = key_client.list_properties_of_keys() | ||
| for prop in properties: | ||
| policy = key_client.get_key_rotation_policy(prop.name) | ||
| for key in keys: | ||
| if key.name == prop.name: | ||
| key.rotation_policy = KeyRotationPolicy( | ||
| properties = list(key_client.list_properties_of_keys()) | ||
|
|
||
| if properties: | ||
| items = [ | ||
| {"key_client": key_client, "prop": prop} for prop in properties | ||
| ] | ||
| rotation_results = self.__threading_call__( | ||
| self._get_single_rotation_policy, items | ||
| ) | ||
|
|
||
| for name, policy in rotation_results: | ||
| if policy and name in keys_dict: | ||
| keys_dict[name].rotation_policy = KeyRotationPolicy( | ||
| id=getattr(policy, "id", ""), | ||
| lifetime_actions=[ | ||
| KeyRotationLifetimeAction(action=action.action) | ||
| for action in getattr(policy, "lifetime_actions", []) | ||
| ], | ||
| ) | ||
|
|
||
| # TODO: handle different errors here since we are catching all HTTP Errors here | ||
| except HttpResponseError: | ||
| logger.warning( | ||
| f"Subscription name: {subscription} -- has no access policy configured for keyvault {keyvault_name}" | ||
| ) | ||
|
|
||
| return keys | ||
|
|
||
| def _get_single_rotation_policy(self, item: dict) -> tuple: | ||
| """Thread-safe rotation policy retrieval.""" | ||
| key_client = item["key_client"] | ||
| prop = item["prop"] | ||
|
|
||
| try: | ||
| policy = key_client.get_key_rotation_policy(prop.name) | ||
| return (prop.name, policy) | ||
| except HttpResponseError: | ||
| return (prop.name, None) | ||
| except Exception as error: | ||
| logger.warning( | ||
| f"KeyVault - Failed to get rotation policy for key {prop.name}: {error}" | ||
| ) | ||
| return (prop.name, None) | ||
|
|
||
| def _get_secrets(self, subscription, resource_group, keyvault_name): | ||
| logger.info(f"KeyVault - Getting secrets for {keyvault_name}...") | ||
| secrets = [] | ||
| try: | ||
| client = self.clients[subscription] | ||
|
|
@@ -177,12 +245,10 @@ def _get_secrets(self, subscription, resource_group, keyvault_name): | |
| logger.error( | ||
| f"Subscription name: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" | ||
| ) | ||
|
|
||
| return secrets | ||
|
|
||
| def _get_vault_monitor_settings(self, keyvault_name, resource_group, subscription): | ||
| logger.info( | ||
| f"KeyVault - Getting monitor diagnostics settings for {keyvault_name}..." | ||
| ) | ||
| monitor_diagnostics_settings = [] | ||
| try: | ||
| monitor_diagnostics_settings = monitor_client.diagnostic_settings_with_uri( | ||
|
|
@@ -192,8 +258,9 @@ def _get_vault_monitor_settings(self, keyvault_name, resource_group, subscriptio | |
| ) | ||
| except Exception as error: | ||
| logger.error( | ||
| f"Subscription name: {self.subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" | ||
| f"Subscription name: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" | ||
| ) | ||
|
|
||
| return monitor_diagnostics_settings | ||
|
|
||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.