Skip to content

Commit 04079ff

Browse files
author
Lucas Gameiro
authored
[DPE-4032] Exposed passwords on postgresql SQL queries logging (#495)
* add integration test * bump postgresql.py lib to 28
1 parent ec60939 commit 04079ff

File tree

2 files changed

+37
-11
lines changed

2 files changed

+37
-11
lines changed

lib/charms/postgresql_k8s/v0/postgresql.py

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Increment this PATCH version before using `charmcraft publish-lib` or reset
3838
# to 0 if you are raising the major API version
39-
LIBPATCH = 26
39+
LIBPATCH = 28
4040

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

@@ -111,20 +111,19 @@ def __init__(
111111
self.system_users = system_users
112112

113113
def _connect_to_database(
114-
self, database: str = None, connect_to_current_host: bool = False
114+
self, database: str = None, database_host: str = None
115115
) -> psycopg2.extensions.connection:
116116
"""Creates a connection to the database.
117117
118118
Args:
119119
database: database to connect to (defaults to the database
120120
provided when the object for this class was created).
121-
connect_to_current_host: whether to connect to the current host
122-
instead of the primary host.
121+
database_host: host to connect to instead of the primary host.
123122
124123
Returns:
125124
psycopg2 connection object.
126125
"""
127-
host = self.current_host if connect_to_current_host else self.primary_host
126+
host = database_host if database_host is not None else self.primary_host
128127
connection = psycopg2.connect(
129128
f"dbname='{database if database else self.database}' user='{self.user}' host='{host}'"
130129
f"password='{self.password}' connect_timeout=1"
@@ -231,7 +230,10 @@ def create_user(
231230
user_definition += f"WITH {'NOLOGIN' if user == 'admin' else 'LOGIN'}{' SUPERUSER' if admin else ''} ENCRYPTED PASSWORD '{password}'{'IN ROLE admin CREATEDB' if admin_role else ''}"
232231
if privileges:
233232
user_definition += f' {" ".join(privileges)}'
233+
cursor.execute(sql.SQL("BEGIN;"))
234+
cursor.execute(sql.SQL("SET LOCAL log_statement = 'none';"))
234235
cursor.execute(sql.SQL(f"{user_definition};").format(sql.Identifier(user)))
236+
cursor.execute(sql.SQL("COMMIT;"))
235237

236238
# Add extra user roles to the new user.
237239
if roles:
@@ -388,7 +390,7 @@ def get_postgresql_text_search_configs(self) -> Set[str]:
388390
Set of PostgreSQL text search configs.
389391
"""
390392
with self._connect_to_database(
391-
connect_to_current_host=True
393+
database_host=self.current_host
392394
) as connection, connection.cursor() as cursor:
393395
cursor.execute("SELECT CONCAT('pg_catalog.', cfgname) FROM pg_ts_config;")
394396
text_search_configs = cursor.fetchall()
@@ -401,7 +403,7 @@ def get_postgresql_timezones(self) -> Set[str]:
401403
Set of PostgreSQL timezones.
402404
"""
403405
with self._connect_to_database(
404-
connect_to_current_host=True
406+
database_host=self.current_host
405407
) as connection, connection.cursor() as cursor:
406408
cursor.execute("SELECT name FROM pg_timezone_names;")
407409
timezones = cursor.fetchall()
@@ -434,7 +436,7 @@ def is_tls_enabled(self, check_current_host: bool = False) -> bool:
434436
"""
435437
try:
436438
with self._connect_to_database(
437-
connect_to_current_host=check_current_host
439+
database_host=self.current_host if check_current_host else None
438440
) as connection, connection.cursor() as cursor:
439441
cursor.execute("SHOW ssl;")
440442
return "on" in cursor.fetchone()[0]
@@ -502,24 +504,32 @@ def set_up_database(self) -> None:
502504
if connection is not None:
503505
connection.close()
504506

505-
def update_user_password(self, username: str, password: str) -> None:
507+
def update_user_password(
508+
self, username: str, password: str, database_host: str = None
509+
) -> None:
506510
"""Update a user password.
507511
508512
Args:
509513
username: the user to update the password.
510514
password: the new password for the user.
515+
database_host: the host to connect to.
511516
512517
Raises:
513518
PostgreSQLUpdateUserPasswordError if the password couldn't be changed.
514519
"""
515520
connection = None
516521
try:
517-
with self._connect_to_database() as connection, connection.cursor() as cursor:
522+
with self._connect_to_database(
523+
database_host=database_host
524+
) as connection, connection.cursor() as cursor:
525+
cursor.execute(sql.SQL("BEGIN;"))
526+
cursor.execute(sql.SQL("SET LOCAL log_statement = 'none';"))
518527
cursor.execute(
519528
sql.SQL("ALTER USER {} WITH ENCRYPTED PASSWORD '" + password + "';").format(
520529
sql.Identifier(username)
521530
)
522531
)
532+
cursor.execute(sql.SQL("COMMIT;"))
523533
except psycopg2.Error as e:
524534
logger.error(f"Failed to update user password: {e}")
525535
raise PostgreSQLUpdateUserPasswordError()
@@ -610,7 +620,7 @@ def validate_date_style(self, date_style: str) -> bool:
610620
"""
611621
try:
612622
with self._connect_to_database(
613-
connect_to_current_host=True
623+
database_host=self.current_host
614624
) as connection, connection.cursor() as cursor:
615625
cursor.execute(
616626
sql.SQL(

tests/integration/test_password_rotation.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
get_primary,
2020
get_unit_address,
2121
restart_patroni,
22+
run_command_on_unit,
2223
set_password,
2324
)
2425

@@ -178,3 +179,18 @@ async def test_no_password_change_on_invalid_password(ops_test: OpsTest) -> None
178179
password2 = await get_password(ops_test, unit_name=leader, username="replication")
179180
# The password didn't change
180181
assert password1 == password2
182+
183+
184+
@pytest.mark.group(1)
185+
async def test_no_password_exposed_on_logs(ops_test: OpsTest) -> None:
186+
"""Test that passwords don't get exposed on postgresql logs."""
187+
for unit in ops_test.model.applications[APP_NAME].units:
188+
try:
189+
logs = await run_command_on_unit(
190+
ops_test,
191+
unit.name,
192+
"grep PASSWORD /var/snap/charmed-postgresql/common/var/log/postgresql/postgresql-*.log",
193+
)
194+
except Exception:
195+
continue
196+
assert len(logs) == 0, f"Sensitive information detected on {unit.name} logs"

0 commit comments

Comments
 (0)