Skip to content

Commit

Permalink
[DPE-5116] Add pgAudit (#688)
Browse files Browse the repository at this point in the history
* Add pgAudit

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Fix extensions enablement on database request

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Make pgAudit work and add integration test

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Split integration tests

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Fix integration test

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Add unit tests

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Enable pgAudit by default

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Fix test_no_password_exposed_on_logs

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

* Enable/disable the plugin at the unit test

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

---------

Signed-off-by: Marcelo Henrique Neppel <[email protected]>
  • Loading branch information
marceloneppel authored Sep 19, 2024
1 parent e70eb0e commit f882a87
Show file tree
Hide file tree
Showing 14 changed files with 223 additions and 22 deletions.
4 changes: 4 additions & 0 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ options:
default: false
type: boolean
description: Enable timescaledb extension
plugin_audit_enable:
default: true
type: boolean
description: Enable pgAudit extension
profile:
description: |
Profile representing the scope of deployment, and used to tune resource allocation.
Expand Down
22 changes: 21 additions & 1 deletion lib/charms/postgresql_k8s/v0/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 34
LIBPATCH = 35

INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"

Expand Down Expand Up @@ -114,6 +114,25 @@ def __init__(
self.database = database
self.system_users = system_users

def _configure_pgaudit(self, enable: bool) -> None:
connection = None
try:
connection = self._connect_to_database()
connection.autocommit = True
with connection.cursor() as cursor:
if enable:
cursor.execute("ALTER SYSTEM SET pgaudit.log = 'ROLE,DDL,MISC,MISC_SET';")
cursor.execute("ALTER SYSTEM SET pgaudit.log_client TO off;")
cursor.execute("ALTER SYSTEM SET pgaudit.log_parameter TO off")
else:
cursor.execute("ALTER SYSTEM RESET pgaudit.log;")
cursor.execute("ALTER SYSTEM RESET pgaudit.log_client;")
cursor.execute("ALTER SYSTEM RESET pgaudit.log_parameter;")
cursor.execute("SELECT pg_reload_conf();")
finally:
if connection is not None:
connection.close()

def _connect_to_database(
self, database: str = None, database_host: str = None
) -> psycopg2.extensions.connection:
Expand Down Expand Up @@ -325,6 +344,7 @@ def enable_disable_extensions(self, extensions: Dict[str, bool], database: str =
if enable
else f"DROP EXTENSION IF EXISTS {extension};"
)
self._configure_pgaudit(ordered_extensions.get("pgaudit", False))
except psycopg2.errors.UniqueViolation:
pass
except psycopg2.errors.DependentObjectsStillExist:
Expand Down
22 changes: 18 additions & 4 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,15 @@
MONITORING_USER,
PATRONI_PASSWORD_KEY,
PEER,
PLUGIN_OVERRIDES,
POSTGRES_LOG_FILES,
REPLICATION_PASSWORD_KEY,
REPLICATION_USER,
REWIND_PASSWORD_KEY,
SECRET_DELETED_LABEL,
SECRET_INTERNAL_LABEL,
SECRET_KEY_OVERRIDES,
SPI_MODULE,
SYSTEM_USERS,
TLS_CA_FILE,
TLS_CERT_FILE,
Expand Down Expand Up @@ -663,8 +665,6 @@ def enable_disable_extensions(self, database: str = None) -> None:
if self._patroni.get_primary() is None:
logger.debug("Early exit enable_disable_extensions: standby cluster")
return
spi_module = ["refint", "autoinc", "insert_username", "moddatetime"]
plugins_exception = {"uuid_ossp": '"uuid-ossp"'}
original_status = self.unit.status
extensions = {}
# collect extensions
Expand All @@ -674,10 +674,10 @@ def enable_disable_extensions(self, database: str = None) -> None:
# Enable or disable the plugin/extension.
extension = "_".join(plugin.split("_")[1:-1])
if extension == "spi":
for ext in spi_module:
for ext in SPI_MODULE:
extensions[ext] = enable
continue
extension = plugins_exception.get(extension, extension)
extension = PLUGIN_OVERRIDES.get(extension, extension)
if self._check_extension_dependencies(extension, enable):
self.unit.status = BlockedStatus(EXTENSIONS_DEPENDENCY_MESSAGE)
return
Expand Down Expand Up @@ -2132,6 +2132,20 @@ def log_pitr_last_transaction_time(self) -> None:
else:
logger.error("Can't tell last completed transaction time")

def get_plugins(self) -> List[str]:
"""Return a list of installed plugins."""
plugins = [
"_".join(plugin.split("_")[1:-1])
for plugin in self.config.plugin_keys()
if self.config[plugin]
]
plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins]
if "spi" in plugins:
plugins.remove("spi")
for ext in SPI_MODULE:
plugins.append(ext)
return plugins


if __name__ == "__main__":
main(PostgresqlOperatorCharm, use_juju_for_storage=True)
1 change: 1 addition & 0 deletions src/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class CharmConfig(BaseConfigModel):
optimizer_join_collapse_limit: Optional[int]
profile: str
profile_limit_memory: Optional[int]
plugin_audit_enable: bool
plugin_citext_enable: bool
plugin_debversion_enable: bool
plugin_hstore_enable: bool
Expand Down
3 changes: 3 additions & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@

SECRET_KEY_OVERRIDES = {"ca": "cauth"}
BACKUP_TYPE_OVERRIDES = {"full": "full", "differential": "diff", "incremental": "incr"}
PLUGIN_OVERRIDES = {"audit": "pgaudit", "uuid_ossp": '"uuid-ossp"'}

SPI_MODULE = ["refint", "autoinc", "insert_username", "moddatetime"]

TRACING_RELATION_NAME = "tracing"
TRACING_PROTOCOL = "otlp_http"
Expand Down
12 changes: 6 additions & 6 deletions src/relations/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
from ops.model import ActiveStatus, BlockedStatus, Relation, Unit
from pgconnstr import ConnectionString

from constants import ALL_LEGACY_RELATIONS, DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE
from constants import (
ALL_LEGACY_RELATIONS,
DATABASE_PORT,
ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE,
)
from utils import new_password

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -176,11 +180,7 @@ def set_up_relation(self, relation: Relation) -> bool:
user = f"relation_id_{relation.id}"
password = unit_relation_databag.get("password", new_password())
self.charm.postgresql.create_user(user, password, self.admin)
plugins = [
"_".join(plugin.split("_")[1:-1])
for plugin in self.charm.config.plugin_keys()
if self.charm.config[plugin]
]
plugins = self.charm.get_plugins()

self.charm.postgresql.create_database(
database, user, plugins=plugins, client_relations=self.charm.client_relations
Expand Down
11 changes: 5 additions & 6 deletions src/relations/postgresql_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
from ops.framework import Object
from ops.model import ActiveStatus, BlockedStatus, Relation

from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE
from constants import (
DATABASE_PORT,
ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE,
)
from utils import new_password

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -87,11 +90,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
user = f"relation_id_{event.relation.id}"
password = new_password()
self.charm.postgresql.create_user(user, password, extra_user_roles=extra_user_roles)
plugins = [
"_".join(plugin.split("_")[1:-1])
for plugin in self.charm.config.plugin_keys()
if self.charm.config[plugin]
]
plugins = self.charm.get_plugins()

self.charm.postgresql.create_database(
database, user, plugins=plugins, client_relations=self.charm.client_relations
Expand Down
2 changes: 1 addition & 1 deletion templates/patroni.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ bootstrap:
log_truncate_on_rotation: 'on'
logging_collector: 'on'
wal_level: logical
shared_preload_libraries: 'timescaledb'
shared_preload_libraries: 'timescaledb,pgaudit'
{%- if pg_parameters %}
{%- for key, value in pg_parameters.items() %}
{{key}}: {{value}}
Expand Down
97 changes: 97 additions & 0 deletions tests/integration/test_audit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env python3
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.
import asyncio
import logging

import psycopg2 as psycopg2
import pytest as pytest
from pytest_operator.plugin import OpsTest
from tenacity import Retrying, stop_after_delay, wait_fixed

from .helpers import (
APPLICATION_NAME,
DATABASE_APP_NAME,
build_and_deploy,
run_command_on_unit,
)
from .new_relations.helpers import build_connection_string

logger = logging.getLogger(__name__)

RELATION_ENDPOINT = "database"


@pytest.mark.group(1)
@pytest.mark.abort_on_fail
async def test_audit_plugin(ops_test: OpsTest) -> None:
"""Test the audit plugin."""
await asyncio.gather(build_and_deploy(ops_test, 1), ops_test.model.deploy(APPLICATION_NAME))
await ops_test.model.relate(f"{APPLICATION_NAME}:{RELATION_ENDPOINT}", DATABASE_APP_NAME)
async with ops_test.fast_forward():
await ops_test.model.wait_for_idle(
apps=[APPLICATION_NAME, DATABASE_APP_NAME], status="active"
)

logger.info("Checking that the audit plugin is enabled")
connection_string = await build_connection_string(
ops_test, APPLICATION_NAME, RELATION_ENDPOINT
)
connection = None
try:
connection = psycopg2.connect(connection_string)
with connection.cursor() as cursor:
cursor.execute("CREATE TABLE test2(value TEXT);")
cursor.execute("GRANT SELECT ON test2 TO PUBLIC;")
cursor.execute("SET TIME ZONE 'Europe/Rome';")
finally:
if connection is not None:
connection.close()
unit_name = f"{DATABASE_APP_NAME}/0"
for attempt in Retrying(stop=stop_after_delay(90), wait=wait_fixed(10), reraise=True):
with attempt:
try:
logs = await run_command_on_unit(
ops_test,
unit_name,
"grep AUDIT /var/log/postgresql/postgresql-*.log",
)
assert "MISC,BEGIN,,,BEGIN" in logs
assert (
"DDL,CREATE TABLE,TABLE,public.test2,CREATE TABLE test2(value TEXT);" in logs
)
assert "ROLE,GRANT,TABLE,,GRANT SELECT ON test2 TO PUBLIC;" in logs
assert "MISC,SET,,,SET TIME ZONE 'Europe/Rome';" in logs
except Exception:
assert False, "Audit logs were not found when the plugin is enabled."

logger.info("Disabling the audit plugin")
await ops_test.model.applications[DATABASE_APP_NAME].set_config({
"plugin_audit_enable": "False"
})
await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active")

logger.info("Removing the previous logs")
await run_command_on_unit(ops_test, unit_name, "rm /var/log/postgresql/postgresql-*.log")

logger.info("Checking that the audit plugin is disabled")
try:
connection = psycopg2.connect(connection_string)
with connection.cursor() as cursor:
cursor.execute("CREATE TABLE test1(value TEXT);")
cursor.execute("GRANT SELECT ON test1 TO PUBLIC;")
cursor.execute("SET TIME ZONE 'Europe/Rome';")
finally:
if connection is not None:
connection.close()
try:
logs = await run_command_on_unit(
ops_test,
unit_name,
"grep AUDIT /var/log/postgresql/postgresql-*.log",
)
except Exception:
pass
else:
logger.info(f"Logs: {logs}")
assert False, "Audit logs were found when the plugin is disabled."
7 changes: 6 additions & 1 deletion tests/integration/test_password_rotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Copyright 2022 Canonical Ltd.
# See LICENSE file for licensing details.
import json
import re
import time

import psycopg2
Expand Down Expand Up @@ -180,4 +181,8 @@ async def test_no_password_exposed_on_logs(ops_test: OpsTest) -> None:
)
except Exception:
continue
assert len(logs) == 0, f"Sensitive information detected on {unit.name} logs"
regex = re.compile("(PASSWORD )(?!<REDACTED>)")
logs_without_false_positives = regex.findall(logs)
assert (
len(logs_without_false_positives) == 0
), f"Sensitive information detected on {unit.name} logs"
31 changes: 31 additions & 0 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1778,3 +1778,34 @@ def test_create_pgdata(harness):
"postgres:postgres",
"/var/lib/postgresql/data",
])


def test_get_plugins(harness):
with patch("charm.PostgresqlOperatorCharm._on_config_changed"):
# Test when the charm has no plugins enabled.
assert harness.charm.get_plugins() == ["pgaudit"]

# Test when the charm has some plugins enabled.
harness.update_config({
"plugin_audit_enable": True,
"plugin_citext_enable": True,
"plugin_spi_enable": True,
})
assert harness.charm.get_plugins() == [
"pgaudit",
"citext",
"refint",
"autoinc",
"insert_username",
"moddatetime",
]

# Test when the charm has the pgAudit plugin disabled.
harness.update_config({"plugin_audit_enable": False})
assert harness.charm.get_plugins() == [
"citext",
"refint",
"autoinc",
"insert_username",
"moddatetime",
]
4 changes: 2 additions & 2 deletions tests/unit/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def test_set_up_relation(harness):
user = f"relation_id_{rel_id}"
postgresql_mock.create_user.assert_called_once_with(user, "test-password", False)
postgresql_mock.create_database.assert_called_once_with(
DATABASE, user, plugins=[], client_relations=[relation]
DATABASE, user, plugins=["pgaudit"], client_relations=[relation]
)
assert postgresql_mock.get_postgresql_version.call_count == 1
_update_unit_status.assert_called_once()
Expand Down Expand Up @@ -279,7 +279,7 @@ def test_set_up_relation(harness):
assert harness.charm.legacy_db_relation.set_up_relation(relation)
postgresql_mock.create_user.assert_called_once_with(user, "test-password", False)
postgresql_mock.create_database.assert_called_once_with(
DATABASE, user, plugins=[], client_relations=[relation]
DATABASE, user, plugins=["pgaudit"], client_relations=[relation]
)
assert postgresql_mock.get_postgresql_version.call_count == 1
_update_unit_status.assert_called_once()
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/test_postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,30 @@ def test_build_postgresql_parameters(harness):
parameters = harness.charm.postgresql.build_postgresql_parameters(config_options, 1000000000)
assert "shared_buffers" not in parameters
assert "effective_cache_size" not in parameters


def test_configure_pgaudit(harness):
with patch(
"charms.postgresql_k8s.v0.postgresql.PostgreSQL._connect_to_database"
) as _connect_to_database:
# Test when pgAudit is enabled.
execute = (
_connect_to_database.return_value.cursor.return_value.__enter__.return_value.execute
)
harness.charm.postgresql._configure_pgaudit(True)
execute.assert_has_calls([
call("ALTER SYSTEM SET pgaudit.log = 'ROLE,DDL,MISC,MISC_SET';"),
call("ALTER SYSTEM SET pgaudit.log_client TO off;"),
call("ALTER SYSTEM SET pgaudit.log_parameter TO off"),
call("SELECT pg_reload_conf();"),
])

# Test when pgAudit is disabled.
execute.reset_mock()
harness.charm.postgresql._configure_pgaudit(False)
execute.assert_has_calls([
call("ALTER SYSTEM RESET pgaudit.log;"),
call("ALTER SYSTEM RESET pgaudit.log_client;"),
call("ALTER SYSTEM RESET pgaudit.log_parameter;"),
call("SELECT pg_reload_conf();"),
])
2 changes: 1 addition & 1 deletion tests/unit/test_postgresql_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def test_on_database_requested(harness):
database_relation = harness.model.get_relation(RELATION_NAME)
client_relations = [database_relation]
postgresql_mock.create_database.assert_called_once_with(
DATABASE, user, plugins=[], client_relations=client_relations
DATABASE, user, plugins=["pgaudit"], client_relations=client_relations
)
postgresql_mock.get_postgresql_version.assert_called_once()

Expand Down

0 comments on commit f882a87

Please sign in to comment.