mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-07-24 13:01:56 +00:00
fix(azure/postgresql): isolate per-server collection failures (#11595)
Co-authored-by: Hugo P.Brito <hugopbrit@gmail.com>
This commit is contained in:
@@ -14,6 +14,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)
|
||||
|
||||
@@ -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 @@ class PostgreSQL(AzureService):
|
||||
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 @@ class PostgreSQL(AzureService):
|
||||
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):
|
||||
|
||||
@@ -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 @@ class Test_SqlServer_Service:
|
||||
)
|
||||
|
||||
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 @@ class Test_SqlServer_Service:
|
||||
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 @@ class Test_SqlServer_Service:
|
||||
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"]
|
||||
|
||||
Reference in New Issue
Block a user