diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 99f3691dc6..bddc07792c 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -13,6 +13,10 @@ All notable changes to the **Prowler SDK** are documented in this file. - CIS Microsoft 365 Foundations Benchmark v7.0.0 compliance framework for the M365 provider [(#11699)](https://github.com/prowler-cloud/prowler/pull/11699) - `waf_regional_webacl_logging_enabled` check for AWS provider, verifying that each AWS WAF Classic Regional Web ACL has logging enabled to a Kinesis Data Firehose stream [(#11539)](https://github.com/prowler-cloud/prowler/pull/11539) +### 🐞 Fixed + +- Azure PostgreSQL flexible server collection no longer drops the remaining servers in a subscription when one server fails to collect; the `connection_throttle.enable` parameter (removed in PostgreSQL 16+) is treated as absent only when the Azure SDK reports it as not found, so unexpected lookup failures are not silently reported as throttling disabled [(#11595)](https://github.com/prowler-cloud/prowler/pull/11595) + --- ## [5.31.1] (Prowler v5.31.1) diff --git a/prowler/providers/azure/services/postgresql/postgresql_service.py b/prowler/providers/azure/services/postgresql/postgresql_service.py index 2048299bf1..681c57a32c 100644 --- a/prowler/providers/azure/services/postgresql/postgresql_service.py +++ b/prowler/providers/azure/services/postgresql/postgresql_service.py @@ -1,6 +1,7 @@ from dataclasses import dataclass from typing import Optional +from azure.core.exceptions import ResourceNotFoundError from azure.mgmt.postgresqlflexibleservers import PostgreSQLManagementClient from prowler.lib.logger import logger @@ -21,62 +22,70 @@ def _get_flexible_servers(self): flexible_servers.update({subscription: []}) flexible_servers_list = client.servers.list() for postgresql_server in flexible_servers_list: - resource_group = self._get_resource_group(postgresql_server.id) - # Fetch full server object once to extract multiple properties - server_details = client.servers.get( - resource_group, postgresql_server.name - ) - require_secure_transport = self._get_require_secure_transport( - subscription, resource_group, postgresql_server.name - ) - active_directory_auth = self._extract_active_directory_auth( - server_details - ) - entra_id_admins = self._get_entra_id_admins( - subscription, resource_group, postgresql_server.name - ) - log_checkpoints = self._get_log_checkpoints( - subscription, resource_group, postgresql_server.name - ) - log_disconnections = self._get_log_disconnections( - subscription, resource_group, postgresql_server.name - ) - log_connections = self._get_log_connections( - subscription, resource_group, postgresql_server.name - ) - connection_throttling = self._get_connection_throttling( - subscription, resource_group, postgresql_server.name - ) - log_retention_days = self._get_log_retention_days( - subscription, resource_group, postgresql_server.name - ) - firewall = self._get_firewall( - subscription, resource_group, postgresql_server.name - ) - location = server_details.location - backup = getattr(server_details, "backup", None) - ha = getattr(server_details, "high_availability", None) - flexible_servers[subscription].append( - Server( - id=postgresql_server.id, - name=postgresql_server.name, - resource_group=resource_group, - location=location, - require_secure_transport=require_secure_transport, - active_directory_auth=active_directory_auth, - entra_id_admins=entra_id_admins, - log_checkpoints=log_checkpoints, - log_connections=log_connections, - log_disconnections=log_disconnections, - connection_throttling=connection_throttling, - log_retention_days=log_retention_days, - firewall=firewall, - geo_redundant_backup=getattr( - backup, "geo_redundant_backup", None - ), - high_availability_mode=getattr(ha, "mode", None), + # Isolate each server: a failure collecting one server must + # not abort collection of the remaining servers in the + # subscription. + try: + resource_group = self._get_resource_group(postgresql_server.id) + # Fetch full server object once to extract multiple properties + server_details = client.servers.get( + resource_group, postgresql_server.name + ) + require_secure_transport = self._get_require_secure_transport( + subscription, resource_group, postgresql_server.name + ) + active_directory_auth = self._extract_active_directory_auth( + server_details + ) + entra_id_admins = self._get_entra_id_admins( + subscription, resource_group, postgresql_server.name + ) + log_checkpoints = self._get_log_checkpoints( + subscription, resource_group, postgresql_server.name + ) + log_disconnections = self._get_log_disconnections( + subscription, resource_group, postgresql_server.name + ) + log_connections = self._get_log_connections( + subscription, resource_group, postgresql_server.name + ) + connection_throttling = self._get_connection_throttling( + subscription, resource_group, postgresql_server.name + ) + log_retention_days = self._get_log_retention_days( + subscription, resource_group, postgresql_server.name + ) + firewall = self._get_firewall( + subscription, resource_group, postgresql_server.name + ) + location = server_details.location + backup = getattr(server_details, "backup", None) + ha = getattr(server_details, "high_availability", None) + flexible_servers[subscription].append( + Server( + id=postgresql_server.id, + name=postgresql_server.name, + resource_group=resource_group, + location=location, + require_secure_transport=require_secure_transport, + active_directory_auth=active_directory_auth, + entra_id_admins=entra_id_admins, + log_checkpoints=log_checkpoints, + log_connections=log_connections, + log_disconnections=log_disconnections, + connection_throttling=connection_throttling, + log_retention_days=log_retention_days, + firewall=firewall, + geo_redundant_backup=getattr( + backup, "geo_redundant_backup", None + ), + high_availability_mode=getattr(ha, "mode", None), + ) + ) + except Exception as error: + logger.error( + f"Subscription ID: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - ) except Exception as error: logger.error( f"Subscription ID: {subscription} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" @@ -166,26 +175,43 @@ def _get_entra_id_admins(self, subscription, resource_group_name, server_name): logger.error(f"Error getting Entra ID admins for {server_name}: {e}") return [] - def _get_connection_throttling(self, subscription, resouce_group_name, server_name): + def _get_connection_throttling( + self, subscription: str, resouce_group_name: str, server_name: str + ) -> Optional[str]: + """Get the ``connection_throttle.enable`` setting for a flexible server. + + The ``connection_throttle.enable`` server parameter was removed in + PostgreSQL 16+, so it no longer exists on newer flexible servers. When + the parameter is genuinely absent the Azure SDK raises + ``ResourceNotFoundError`` (error code ``ConfigurationNotExists``); that + case is treated as "not enabled" and ``None`` is returned so collection + of the server can continue. + + Any other error (permissions, throttling, transient SDK failures) is + intentionally left to propagate: returning ``None`` for those would make + the downstream check report the server as having connection throttling + disabled, silently turning a collection failure into a security finding. + + Args: + subscription: Azure subscription identifier. + resouce_group_name: Resource group containing the server. + server_name: PostgreSQL flexible server name. + + Returns: + The uppercased throttling value, or ``None`` when the parameter does + not exist on the server. + + Raises: + ResourceNotFoundError is handled; any other exception propagates to + the caller so it can be surfaced as a collection failure. + """ client = self.clients[subscription] try: connection_throttling = client.configurations.get( resouce_group_name, server_name, "connection_throttle.enable" ) return connection_throttling.value.upper() - except Exception as error: - message = str(error).lower() - if "connection_throttle.enable" in message and ( - "not exist" in message or "not found" in message - ): - # The "connection_throttle.enable" parameter does not exist on - # newer PostgreSQL versions (e.g. v18); this is expected. - return None - # Any other failure is a genuine problem: surface it, but still - # degrade gracefully instead of aborting the subscription inventory. - logger.error( - f"Error getting connection throttling for {server_name}: {error}" - ) + except ResourceNotFoundError: return None def _get_log_retention_days(self, subscription, resouce_group_name, server_name): diff --git a/tests/providers/azure/services/postgresql/postgresql_service_test.py b/tests/providers/azure/services/postgresql/postgresql_service_test.py index c0b8bca6c4..f372de8844 100644 --- a/tests/providers/azure/services/postgresql/postgresql_service_test.py +++ b/tests/providers/azure/services/postgresql/postgresql_service_test.py @@ -1,5 +1,8 @@ from unittest.mock import MagicMock, patch +import pytest +from azure.core.exceptions import HttpResponseError, ResourceNotFoundError + from prowler.providers.azure.services.postgresql.postgresql_service import ( EntraIdAdmin, Firewall, @@ -116,12 +119,13 @@ def test_get_connection_throttling(self): ) def test_get_connection_throttling_missing_parameter_returns_none(self): - # PostgreSQL v18 removed the "connection_throttle.enable" parameter; the - # service must degrade gracefully (quiet None) instead of raising and + # PostgreSQL v18 removed the "connection_throttle.enable" parameter; when + # it is genuinely absent the Azure SDK raises ResourceNotFoundError, and + # the service treats that as "not enabled" (quiet None) instead of # aborting the whole subscription's server inventory. postgresql = PostgreSQL(set_mocked_azure_provider()) mock_client = MagicMock() - mock_client.configurations.get.side_effect = Exception( + mock_client.configurations.get.side_effect = ResourceNotFoundError( "The configuration 'connection_throttle.enable' does not exist for " "server version 18." ) @@ -135,23 +139,22 @@ def test_get_connection_throttling_missing_parameter_returns_none(self): assert result is None mock_logger.error.assert_not_called() - def test_get_connection_throttling_unexpected_error_logs_error(self): + def test_get_connection_throttling_unexpected_error_propagates(self): # Any other failure (permissions, throttling, transient API errors) must - # still be logged as an error, while keeping the scan resilient (None). + # NOT be swallowed into None: that would make the downstream check report + # the server as having throttling disabled, hiding a collection failure + # as a security finding. The error propagates so the per-server handler + # in _get_flexible_servers can record it as a collection failure. postgresql = PostgreSQL(set_mocked_azure_provider()) mock_client = MagicMock() - mock_client.configurations.get.side_effect = Exception( - "Some unexpected failure" + mock_client.configurations.get.side_effect = HttpResponseError( + "(AuthorizationFailed) permission denied" ) postgresql.clients[AZURE_SUBSCRIPTION_ID] = mock_client - with patch( - "prowler.providers.azure.services.postgresql.postgresql_service.logger" - ) as mock_logger: - result = postgresql._get_connection_throttling( + with pytest.raises(HttpResponseError): + postgresql._get_connection_throttling( AZURE_SUBSCRIPTION_ID, "resource_group", "server_name" ) - assert result is None - mock_logger.error.assert_called_once() def test_get_log_retention_days(self): postgesql = PostgreSQL(set_mocked_azure_provider()) @@ -238,3 +241,126 @@ def test_get_firewall(self): postgesql.flexible_servers[AZURE_SUBSCRIPTION_ID][0].firewall[0].end_ip == "end_ip" ) + + +def _make_server(name): + server = MagicMock() + server.id = ( + f"/subscriptions/{AZURE_SUBSCRIPTION_ID}/resourceGroups/rg/providers/" + f"Microsoft.DBforPostgreSQL/flexibleServers/{name}" + ) + server.name = name + return server + + +class Test_PostgreSQL_Service_Resilience: + """Collecting one flexible server must never abort collection of the rest of + the subscription (regression: a missing/failing per-server configuration + lookup silently dropped every remaining server).""" + + def _build_service_with_client(self, mock_client): + # Skip the real network call during construction, then run the real + # collection against the mocked management client. + with patch.object(PostgreSQL, "_get_flexible_servers", return_value={}): + postgresql = PostgreSQL(set_mocked_azure_provider()) + postgresql.clients = {AZURE_SUBSCRIPTION_ID: mock_client} + return postgresql + + def test_missing_connection_throttle_config_still_collects_server(self): + # The "connection_throttle.enable" parameter was removed in PostgreSQL + # 16+, so the lookup raises ConfigurationNotExists on newer servers. + dev = _make_server("dev") + prd = _make_server("prd") + + mock_client = MagicMock() + mock_client.servers.list.return_value = [dev, prd] + server_details = MagicMock() + server_details.location = "westeurope" + mock_client.servers.get.return_value = server_details + mock_client.administrators.list_by_server.return_value = [] + mock_client.firewall_rules.list_by_server.return_value = [] + + def configurations_get(resource_group, server_name, key): + if key == "connection_throttle.enable" and server_name == "prd": + # Azure raises ResourceNotFoundError (ConfigurationNotExists) + # when the parameter does not exist on the server. + raise ResourceNotFoundError( + "(ConfigurationNotExists) The configuration " + "'connection_throttle.enable' does not exist for prd server " + "version 18." + ) + return MagicMock(value="ON") + + mock_client.configurations.get.side_effect = configurations_get + + postgresql = self._build_service_with_client(mock_client) + servers = postgresql._get_flexible_servers() + + names = sorted(server.name for server in servers[AZURE_SUBSCRIPTION_ID]) + assert names == ["dev", "prd"] + prd_server = next(s for s in servers[AZURE_SUBSCRIPTION_ID] if s.name == "prd") + assert prd_server.connection_throttling is None + dev_server = next(s for s in servers[AZURE_SUBSCRIPTION_ID] if s.name == "dev") + assert dev_server.connection_throttling == "ON" + + def test_unexpected_throttling_error_is_not_silently_collected(self): + # An unexpected failure reading "connection_throttle.enable" (e.g. a + # permission, throttling, or transient SDK error) must NOT be turned + # into connection_throttling=None: that would make the downstream check + # report the server as having throttling disabled, hiding a collection + # failure as a security finding. Only ResourceNotFoundError (the + # parameter genuinely missing) is treated as "not enabled"; anything + # else isolates to that server, which is dropped rather than fabricated. + ok = _make_server("ok") + denied = _make_server("denied") + + mock_client = MagicMock() + mock_client.servers.list.return_value = [ok, denied] + server_details = MagicMock() + server_details.location = "westeurope" + mock_client.servers.get.return_value = server_details + mock_client.administrators.list_by_server.return_value = [] + mock_client.firewall_rules.list_by_server.return_value = [] + + def configurations_get(resource_group, server_name, key): + if key == "connection_throttle.enable" and server_name == "denied": + raise HttpResponseError("(AuthorizationFailed) permission denied") + return MagicMock(value="ON") + + mock_client.configurations.get.side_effect = configurations_get + + postgresql = self._build_service_with_client(mock_client) + servers = postgresql._get_flexible_servers() + + collected = servers[AZURE_SUBSCRIPTION_ID] + # The server whose throttling lookup failed unexpectedly is dropped, + # not collected with a fabricated connection_throttling=None. + assert [server.name for server in collected] == ["ok"] + assert all(server.connection_throttling is not None for server in collected) + + def test_one_server_hard_failure_does_not_drop_others(self): + # A failure unrelated to a guarded getter (here, fetching the server + # details) must isolate to that server, not the whole subscription. + ok = _make_server("ok") + broken = _make_server("broken") + + mock_client = MagicMock() + mock_client.servers.list.return_value = [broken, ok] + mock_client.administrators.list_by_server.return_value = [] + mock_client.firewall_rules.list_by_server.return_value = [] + mock_client.configurations.get.return_value = MagicMock(value="ON") + + def servers_get(resource_group, server_name): + if server_name == "broken": + raise Exception("boom: transient failure fetching server details") + details = MagicMock() + details.location = "westeurope" + return details + + mock_client.servers.get.side_effect = servers_get + + postgresql = self._build_service_with_client(mock_client) + servers = postgresql._get_flexible_servers() + + names = [server.name for server in servers[AZURE_SUBSCRIPTION_ID]] + assert names == ["ok"]