From ad78e84ccd7c114fa1719d15de267e31295570ac Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Mon, 20 Jul 2026 12:41:14 -0400 Subject: [PATCH 1/3] Prevent PostgreSQL from leaking plaintext passwords to postgres.log PostgreSQL's log_min_error_statement defaults to 'error', so whenever a CREATE/ALTER ROLE ... PASSWORD statement errored, the full statement text -- including the plaintext password -- was written to /opt/so/log/postgres/postgres.log. The role-provisioning paths (init-db.sh for so_postgres, so-telegraf-postgres for per-minion telegraf roles) both dispatch such DDL, the latter on every state.apply. - init-db.sh / so-telegraf-postgres: SET log_min_error_statement = panic before the password-bearing DDL so an error no longer emits the STATEMENT line. The ERROR message itself (no password) still logs, preserving debuggability. - logrotate: add a postgres stanza (daily, keep 14, copytruncate, compress) so postgres.log is rotated like every other service and leaked content can't persist indefinitely. copytruncate is required because the container holds the log open via redirected stderr. - soup: scrub any already-logged PASSWORD lines from postgres.log during post_to_3.2.0, rewriting in place to preserve the inode postgres is writing to. --- salt/logrotate/defaults.yaml | 10 +++++++ salt/logrotate/soc_logrotate.yaml | 7 +++++ salt/manager/tools/sbin/soup | 27 +++++++++++++++++++ salt/postgres/files/init-db.sh | 4 +++ salt/postgres/tools/sbin/so-telegraf-postgres | 4 +++ 5 files changed, 52 insertions(+) diff --git a/salt/logrotate/defaults.yaml b/salt/logrotate/defaults.yaml index 2261bb4f7..e37193d0d 100644 --- a/salt/logrotate/defaults.yaml +++ b/salt/logrotate/defaults.yaml @@ -150,6 +150,16 @@ logrotate: - extension .log - dateext - dateyesterday + /opt/so/log/postgres/*_x_log: + - daily + - rotate 14 + - missingok + - copytruncate + - compress + - create + - extension .log + - dateext + - dateyesterday /opt/so/log/telegraf/*_x_log: - daily - rotate 14 diff --git a/salt/logrotate/soc_logrotate.yaml b/salt/logrotate/soc_logrotate.yaml index f407ab48d..f329ed623 100644 --- a/salt/logrotate/soc_logrotate.yaml +++ b/salt/logrotate/soc_logrotate.yaml @@ -91,6 +91,13 @@ logrotate: multiline: True global: True forcedType: "[]string" + "/opt/so/log/postgres/*_x_log": + description: List of logrotate options for this file. + title: /opt/so/log/postgres/*.log + advanced: True + multiline: True + global: True + forcedType: "[]string" "/opt/so/log/telegraf/*_x_log": description: List of logrotate options for this file. title: /opt/so/log/telegraf/*.log diff --git a/salt/manager/tools/sbin/soup b/salt/manager/tools/sbin/soup index f5b40310e..8def509e1 100755 --- a/salt/manager/tools/sbin/soup +++ b/salt/manager/tools/sbin/soup @@ -905,6 +905,30 @@ bootstrap_so_soc_database() { docker restart so-soc } +scrub_postgres_log_passwords() { + # Before 3.2 PostgreSQL could log the full CREATE/ALTER ROLE ... PASSWORD + # statement -- with the plaintext password -- to postgres.log whenever such a + # DDL errored, because log_min_error_statement defaulted to 'error'. The leak + # is fixed going forward (the provisioning scripts now SET log_min_error_statement + # = panic), but any already-logged secret persists in the on-disk log. Purge it + # here. Only the manager runs so-postgres, so this is a local scrub. + local log=/opt/so/log/postgres/postgres.log + [[ -f "$log" ]] || return 0 + if ! grep -qai "PASSWORD '" "$log" 2>/dev/null; then + echo "No leaked passwords found in $log." + return 0 + fi + echo "Removing leaked password statements from $log." + local tmp + tmp=$(mktemp) + # Drop every line carrying a PASSWORD literal, then rewrite in place with + # `cat >` so the file keeps its inode -- postgres holds it open via redirected + # stderr and must keep logging to the same file. + grep -avi "PASSWORD '" "$log" > "$tmp" 2>/dev/null || true + cat "$tmp" > "$log" + rm -f "$tmp" +} + # Existing grids should keep ILM unless an admin explicitly opts in to DLM. pin_elasticsearch_data_retention_method() { local elasticsearch_file=/opt/so/saltstack/local/pillar/elasticsearch/soc_elasticsearch.sls @@ -991,6 +1015,9 @@ post_to_3.2.0() { bootstrap_so_soc_database + # Purge any plaintext passwords a pre-3.2 postgres logged on DDL errors. + scrub_postgres_log_passwords + # Generate 9.3.7 elastic agent installers echo "Regenerating Elastic Agent Installers" /sbin/so-elastic-agent-gen-installers diff --git a/salt/postgres/files/init-db.sh b/salt/postgres/files/init-db.sh index d12bc4c9b..d1f1e375f 100644 --- a/salt/postgres/files/init-db.sh +++ b/salt/postgres/files/init-db.sh @@ -8,6 +8,10 @@ if [ -z "${SO_POSTGRES_PASS:-}" ] && [ -n "${SO_POSTGRES_PASS_FILE:-}" ] && [ -r SO_POSTGRES_PASS="$(< "$SO_POSTGRES_PASS_FILE")" fi psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL + -- Shield the plaintext password below from postgres.log if this DDL errors: + -- log_min_error_statement defaults to 'error', which would append a STATEMENT line + -- containing the full CREATE/ALTER ROLE ... PASSWORD text. panic suppresses that. + SET log_min_error_statement = panic; DO \$\$ BEGIN IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '${SO_POSTGRES_USER}') THEN diff --git a/salt/postgres/tools/sbin/so-telegraf-postgres b/salt/postgres/tools/sbin/so-telegraf-postgres index ef7c3f9e6..e6cc69fa6 100644 --- a/salt/postgres/tools/sbin/so-telegraf-postgres +++ b/salt/postgres/tools/sbin/so-telegraf-postgres @@ -71,6 +71,10 @@ EOSQL -v role_user="$ROLE_USER" \ -v role_pass="$ROLE_PASS" \ -U postgres -d so_telegraf <<'EOSQL' +-- Shield the plaintext password below from postgres.log if this DDL errors: +-- log_min_error_statement defaults to 'error', which would append a STATEMENT line +-- containing the full CREATE/ALTER ROLE ... PASSWORD text. panic suppresses that. +SET log_min_error_statement = panic; SELECT format( CASE WHEN EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = :'role_user') THEN 'ALTER ROLE %I WITH LOGIN PASSWORD %L' From 011749ad09d383879e212e7624424130601e01b1 Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Mon, 20 Jul 2026 15:04:58 -0400 Subject: [PATCH 2/3] Self-heal PostgreSQL SOC database bootstrap init-db.sh only runs on a fresh PGDATA (docker-entrypoint-initdb.d), so a cluster left partially initialized -- e.g. the container restarted mid first-init by a watch trigger -- never recovered: the so_postgres role existed but its schema grants and the so_telegraf database were missing, breaking SOC with "permission denied for schema public". - init-db.sh: make the role upsert race-safe. Try CREATE ROLE and fall back to ALTER on duplicate_object/unique_violation instead of IF NOT EXISTS, so a concurrent creator no longer aborts the script (set -e + ON_ERROR_STOP=1) before the grants and so_telegraf creation run. The whole script is now idempotent and safe to re-run. - postgres/enabled.sls: move postgres_wait_ready here (was telegraf-gated in telegraf_users.sls) and add postgres_bootstrap_soc_db, which re-runs init-db.sh every highstate so a partially-initialized cluster self-heals. - telegraf_users.sls: drop its now-duplicate postgres_wait_ready definition; it resolves from postgres.enabled via the existing include. - soup: remove bootstrap_so_soc_database and its post_to_3.2.0 call. The highstates soup runs before postupgrade now reconcile the SOC DB, making the one-shot bootstrap redundant. --- salt/manager/tools/sbin/soup | 29 +++-------------------------- salt/postgres/enabled.sls | 20 ++++++++++++++++++++ salt/postgres/files/init-db.sh | 14 ++++++++++---- salt/postgres/telegraf_users.sls | 15 ++++----------- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/salt/manager/tools/sbin/soup b/salt/manager/tools/sbin/soup index 8def509e1..eb63a1d60 100755 --- a/salt/manager/tools/sbin/soup +++ b/salt/manager/tools/sbin/soup @@ -880,31 +880,6 @@ recollate_postgres() { echo "" } -bootstrap_so_soc_database() { - # init-db.sh is mounted into so-postgres at /docker-entrypoint-initdb.d/init-db.sh - # and runs automatically only on a fresh data directory. Hosts upgrading from - # 3.1.0 already have /nsm/postgres populated, so the so_soc bootstrap block - # added in 3.2 never fires. Re-run the script explicitly; it's idempotent. - echo "Bootstrapping database via init-db.sh." - # The postgres image has no USER directive, so `docker exec` defaults to - # root, and the container env intentionally omits POSTGRES_USER (the upstream - # entrypoint defaults it transiently during first-init only). Recreate both - # so psql inside init-db.sh resolves the connect user correctly. - local exec_cmd="docker exec -u postgres -e POSTGRES_USER=postgres so-postgres bash /docker-entrypoint-initdb.d/init-db.sh" - if ! /usr/sbin/so-postgres-wait; then - FINAL_MESSAGE_QUEUE+=("WARNING: so-postgres was not ready during the 3.2.0 upgrade; the so_soc database may not have been bootstrapped. Re-run manually: $exec_cmd") - return 0 - fi - if ! $exec_cmd; then - FINAL_MESSAGE_QUEUE+=("WARNING: init-db.sh failed inside so-postgres during the 3.2.0 upgrade; the database may not have been bootstrapped. Re-run manually: $exec_cmd") - return 0 - fi - echo "Database bootstrap complete." - - echo "Restarting so-soc container to pick up database changes" - docker restart so-soc -} - scrub_postgres_log_passwords() { # Before 3.2 PostgreSQL could log the full CREATE/ALTER ROLE ... PASSWORD # statement -- with the plaintext password -- to postgres.log whenever such a @@ -1013,7 +988,9 @@ post_to_3.2.0() { # Recollate due to image OS rebase recollate_postgres - bootstrap_so_soc_database + # The SOC database (so_postgres role, schema/db grants, so_telegraf) is + # bootstrapped idempotently by the postgres.enabled highstate that runs + # before this postupgrade step, so no explicit bootstrap call is needed here. # Purge any plaintext passwords a pre-3.2 postgres logged on DDL errors. scrub_postgres_log_passwords diff --git a/salt/postgres/enabled.sls b/salt/postgres/enabled.sls index b0eadd205..ad6ff7819 100644 --- a/salt/postgres/enabled.sls +++ b/salt/postgres/enabled.sls @@ -85,6 +85,26 @@ so-postgres: - x509: postgres_crt - x509: postgres_key +postgres_wait_ready: + cmd.run: + - name: /usr/sbin/so-postgres-wait + - require: + - docker_container: so-postgres + - file: postgres_sbin + +# Re-run the SOC database bootstrap on every highstate so a cluster left +# partially initialized -- e.g. the container was restarted mid first-init by a +# watch trigger, so docker-entrypoint-initdb.d never completed -- self-heals: +# the so_postgres role, its schema/database grants, and the so_telegraf database +# are all reconciled idempotently. init-db.sh is idempotent and race-safe. +# POSTGRES_USER is injected because the container env intentionally omits it. +postgres_bootstrap_soc_db: + cmd.run: + - name: docker exec -u postgres -e POSTGRES_USER=postgres so-postgres bash /docker-entrypoint-initdb.d/init-db.sh + - require: + - cmd: postgres_wait_ready + - file: postgresinitdb + delete_so-postgres_so-status.disabled: file.uncomment: - name: /opt/so/conf/so-status/so-status.conf diff --git a/salt/postgres/files/init-db.sh b/salt/postgres/files/init-db.sh index d1f1e375f..540980469 100644 --- a/salt/postgres/files/init-db.sh +++ b/salt/postgres/files/init-db.sh @@ -12,13 +12,19 @@ psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-E -- log_min_error_statement defaults to 'error', which would append a STATEMENT line -- containing the full CREATE/ALTER ROLE ... PASSWORD text. panic suppresses that. SET log_min_error_statement = panic; + -- Race-safe upsert: try CREATE, and if the role was created concurrently + -- (another session re-entering init) fall back to ALTER. Catching the + -- exception -- rather than an IF NOT EXISTS check -- avoids a TOCTOU window + -- where CREATE ROLE would abort the whole script with a duplicate-key error + -- and skip the grants below. Both SQLSTATEs are covered: duplicate_object + -- (role already exists) and unique_violation (racy pg_authid index insert). DO \$\$ BEGIN - IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '${SO_POSTGRES_USER}') THEN + BEGIN EXECUTE format('CREATE ROLE %I WITH LOGIN PASSWORD %L', '${SO_POSTGRES_USER}', '${SO_POSTGRES_PASS}'); - ELSE - EXECUTE format('ALTER ROLE %I WITH PASSWORD %L', '${SO_POSTGRES_USER}', '${SO_POSTGRES_PASS}'); - END IF; + EXCEPTION WHEN duplicate_object OR unique_violation THEN + EXECUTE format('ALTER ROLE %I WITH LOGIN PASSWORD %L', '${SO_POSTGRES_USER}', '${SO_POSTGRES_PASS}'); + END; END \$\$; GRANT ALL ON SCHEMA public TO "$SO_POSTGRES_USER"; diff --git a/salt/postgres/telegraf_users.sls b/salt/postgres/telegraf_users.sls index 5e3566a95..62c33b892 100644 --- a/salt/postgres/telegraf_users.sls +++ b/salt/postgres/telegraf_users.sls @@ -8,23 +8,16 @@ {% from 'vars/globals.map.jinja' import GLOBALS %} {% from 'telegraf/map.jinja' import TELEGRAFMERGED %} -{# postgres_wait_ready below requires `docker_container: so-postgres`, which is - declared in postgres.enabled. Include it here so state.apply postgres.telegraf_users - on its own (e.g. from orch.deploy_newnode) still has that ID in scope. Salt - de-duplicates the circular include. #} +{# postgres_wait_ready and the so-postgres container are declared in + postgres.enabled; include it so the require references below resolve and so + state.apply postgres.telegraf_users on its own (e.g. from orch.deploy_newnode) + still has those IDs in scope. Salt de-duplicates the circular include. #} include: - postgres.enabled {% set TG_OUT = TELEGRAFMERGED.output | upper %} {% if TG_OUT in ['POSTGRES', 'BOTH'] %} -postgres_wait_ready: - cmd.run: - - name: /usr/sbin/so-postgres-wait - - require: - - docker_container: so-postgres - - file: postgres_sbin - # Ensure the shared Telegraf database exists. init-db.sh only runs on a # fresh data dir, so hosts upgraded onto an existing /nsm/postgres volume # would otherwise never get so_telegraf. From 387781c6292c96aec01b3f0a66b103a9a29d9057 Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Mon, 20 Jul 2026 15:19:02 -0400 Subject: [PATCH 3/3] Trim verbose comments in postgres provisioning changes --- salt/manager/tools/sbin/soup | 17 +++-------------- salt/postgres/enabled.sls | 9 +++------ salt/postgres/files/init-db.sh | 12 +++--------- salt/postgres/telegraf_users.sls | 6 ++---- salt/postgres/tools/sbin/so-telegraf-postgres | 4 +--- 5 files changed, 12 insertions(+), 36 deletions(-) diff --git a/salt/manager/tools/sbin/soup b/salt/manager/tools/sbin/soup index eb63a1d60..9f02eb32a 100755 --- a/salt/manager/tools/sbin/soup +++ b/salt/manager/tools/sbin/soup @@ -881,12 +881,7 @@ recollate_postgres() { } scrub_postgres_log_passwords() { - # Before 3.2 PostgreSQL could log the full CREATE/ALTER ROLE ... PASSWORD - # statement -- with the plaintext password -- to postgres.log whenever such a - # DDL errored, because log_min_error_statement defaulted to 'error'. The leak - # is fixed going forward (the provisioning scripts now SET log_min_error_statement - # = panic), but any already-logged secret persists in the on-disk log. Purge it - # here. Only the manager runs so-postgres, so this is a local scrub. + # Purge plaintext passwords a pre-3.2 postgres could log on DDL errors. local log=/opt/so/log/postgres/postgres.log [[ -f "$log" ]] || return 0 if ! grep -qai "PASSWORD '" "$log" 2>/dev/null; then @@ -896,9 +891,7 @@ scrub_postgres_log_passwords() { echo "Removing leaked password statements from $log." local tmp tmp=$(mktemp) - # Drop every line carrying a PASSWORD literal, then rewrite in place with - # `cat >` so the file keeps its inode -- postgres holds it open via redirected - # stderr and must keep logging to the same file. + # Rewrite in place (cat >) to keep the inode postgres is writing to. grep -avi "PASSWORD '" "$log" > "$tmp" 2>/dev/null || true cat "$tmp" > "$log" rm -f "$tmp" @@ -988,11 +981,7 @@ post_to_3.2.0() { # Recollate due to image OS rebase recollate_postgres - # The SOC database (so_postgres role, schema/db grants, so_telegraf) is - # bootstrapped idempotently by the postgres.enabled highstate that runs - # before this postupgrade step, so no explicit bootstrap call is needed here. - - # Purge any plaintext passwords a pre-3.2 postgres logged on DDL errors. + # SOC database bootstrap is handled by the postgres.enabled highstate. scrub_postgres_log_passwords # Generate 9.3.7 elastic agent installers diff --git a/salt/postgres/enabled.sls b/salt/postgres/enabled.sls index ad6ff7819..fa91d146a 100644 --- a/salt/postgres/enabled.sls +++ b/salt/postgres/enabled.sls @@ -92,12 +92,9 @@ postgres_wait_ready: - docker_container: so-postgres - file: postgres_sbin -# Re-run the SOC database bootstrap on every highstate so a cluster left -# partially initialized -- e.g. the container was restarted mid first-init by a -# watch trigger, so docker-entrypoint-initdb.d never completed -- self-heals: -# the so_postgres role, its schema/database grants, and the so_telegraf database -# are all reconciled idempotently. init-db.sh is idempotent and race-safe. -# POSTGRES_USER is injected because the container env intentionally omits it. +# Reconcile the SOC database (role, grants, so_telegraf) every highstate so a +# partially-initialized cluster self-heals. POSTGRES_USER is injected because +# the container env omits it. postgres_bootstrap_soc_db: cmd.run: - name: docker exec -u postgres -e POSTGRES_USER=postgres so-postgres bash /docker-entrypoint-initdb.d/init-db.sh diff --git a/salt/postgres/files/init-db.sh b/salt/postgres/files/init-db.sh index 540980469..4d65b0c97 100644 --- a/salt/postgres/files/init-db.sh +++ b/salt/postgres/files/init-db.sh @@ -8,16 +8,10 @@ if [ -z "${SO_POSTGRES_PASS:-}" ] && [ -n "${SO_POSTGRES_PASS_FILE:-}" ] && [ -r SO_POSTGRES_PASS="$(< "$SO_POSTGRES_PASS_FILE")" fi psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL - -- Shield the plaintext password below from postgres.log if this DDL errors: - -- log_min_error_statement defaults to 'error', which would append a STATEMENT line - -- containing the full CREATE/ALTER ROLE ... PASSWORD text. panic suppresses that. + -- Keep the password out of postgres.log if this DDL errors. SET log_min_error_statement = panic; - -- Race-safe upsert: try CREATE, and if the role was created concurrently - -- (another session re-entering init) fall back to ALTER. Catching the - -- exception -- rather than an IF NOT EXISTS check -- avoids a TOCTOU window - -- where CREATE ROLE would abort the whole script with a duplicate-key error - -- and skip the grants below. Both SQLSTATEs are covered: duplicate_object - -- (role already exists) and unique_violation (racy pg_authid index insert). + -- Idempotent, race-safe upsert: CREATE, falling back to ALTER if the role + -- already exists or is created concurrently. DO \$\$ BEGIN BEGIN diff --git a/salt/postgres/telegraf_users.sls b/salt/postgres/telegraf_users.sls index 62c33b892..4c63d40b0 100644 --- a/salt/postgres/telegraf_users.sls +++ b/salt/postgres/telegraf_users.sls @@ -8,10 +8,8 @@ {% from 'vars/globals.map.jinja' import GLOBALS %} {% from 'telegraf/map.jinja' import TELEGRAFMERGED %} -{# postgres_wait_ready and the so-postgres container are declared in - postgres.enabled; include it so the require references below resolve and so - state.apply postgres.telegraf_users on its own (e.g. from orch.deploy_newnode) - still has those IDs in scope. Salt de-duplicates the circular include. #} +{# postgres.enabled declares the so-postgres container and postgres_wait_ready + that the requires below reference. Salt de-duplicates the circular include. #} include: - postgres.enabled diff --git a/salt/postgres/tools/sbin/so-telegraf-postgres b/salt/postgres/tools/sbin/so-telegraf-postgres index e6cc69fa6..17e4da744 100644 --- a/salt/postgres/tools/sbin/so-telegraf-postgres +++ b/salt/postgres/tools/sbin/so-telegraf-postgres @@ -71,9 +71,7 @@ EOSQL -v role_user="$ROLE_USER" \ -v role_pass="$ROLE_PASS" \ -U postgres -d so_telegraf <<'EOSQL' --- Shield the plaintext password below from postgres.log if this DDL errors: --- log_min_error_statement defaults to 'error', which would append a STATEMENT line --- containing the full CREATE/ALTER ROLE ... PASSWORD text. panic suppresses that. +-- Keep the password out of postgres.log if this DDL errors. SET log_min_error_statement = panic; SELECT format( CASE WHEN EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = :'role_user')