diff --git a/CHANGELOG.md b/CHANGELOG.md index 45b831d..30984aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Security +- [#87] Rotate the superuser password once in `post-upgrade.sh` — it may have been generated by a `doguctl` older than v0.12.2 which used `math/rand` instead of `crypto/rand` +- [#87] Affected values are indistinguishable from safe ones, so every instance rotates regardless of its installed version, guarded by the new dogu config marker `password_rotated` +- [#87] A fresh installation sets the marker in `initAdmin` and never rotates ## [v14.23-1] - 2026-06-23 ### Changed diff --git a/batsTests/post-upgrade.bats b/batsTests/post-upgrade.bats index 5f48409..3542ba3 100755 --- a/batsTests/post-upgrade.bats +++ b/batsTests/post-upgrade.bats @@ -11,10 +11,13 @@ load '/workspace/target/bats_libs/bats-file/load.bash' setup() { doguctl="$(mock_create)" + psql="$(mock_create)" export doguctl + export psql export PATH="${BATS_TMPDIR}:${PATH}" ln -sf "${doguctl}" "${BATS_TMPDIR}/doguctl" + ln -sf "${psql}" "${BATS_TMPDIR}/psql" # Mocks für Systembefehle echo "#!/bin/bash" > "${BATS_TMPDIR}/chown" @@ -117,4 +120,96 @@ load_script_safely() { assert_success assert_line "Postgresql post-upgrade done" -} \ No newline at end of file +} + + +@test "rotateSuperuserPassword guard suppresses rotation via rotation flag" { + # arrange + load_script_safely + mock_set_output "${doguctl}" "true" + + # act + run rotateSuperuserPassword + + # assert + assert_success + assert_equal "$(mock_get_call_num "${doguctl}")" "1" + assert_line "Superuser password has already been rotated; skipping" +} + +@test "rotateSuperuserPassword() should rotate superuser password if not rotated before" { + # arrange doguctl + load_script_safely + mock_set_output "${doguctl}" "false" 1 + mock_set_output "${doguctl}" "testusername" 2 + mock_set_output "${doguctl}" "random123" 3 + + # arrange psql + local psql_mock_call_args + psql_mock_call_args="--variable=ON_ERROR_STOP=1" + psql_mock_call_args+=" --username=testusername" + psql_mock_call_args+=" --command=ALTER USER \"testusername\" WITH PASSWORD 'random123';" + + # act + run rotateSuperuserPassword + + # assert + assert_success + assert_equal "$(mock_get_call_args "${doguctl}" 1)" "config --default false password_rotated" + assert_line "Rotating superuser password..." + assert_equal "$(mock_get_call_args "${doguctl}" 2)" "config --default postgres user" + assert_equal "$(mock_get_call_args "${doguctl}" 3)" "random" + assert_equal "$(mock_get_call_args "${doguctl}" 4)" "config --encrypted password random123" + assert_equal "$(mock_get_call_args "${psql}" 1)" "${psql_mock_call_args}" + assert_equal "$(mock_get_call_args "${doguctl}" 5)" "config password_rotated true" + assert_line "Superuser password rotated" + + assert_equal "$(mock_get_call_num "${doguctl}")" "5" + assert_equal "$(mock_get_call_num "${psql}")" "1" +} + +@test "rotateSuperuserPassword() should not set the flag if the database update fails" { + # arrange + load_script_safely + mock_set_output "${doguctl}" "false" 1 + mock_set_output "${doguctl}" "testusername" 2 + mock_set_output "${doguctl}" "random123" 3 + mock_set_status "${psql}" 1 1 # psql fails + + # act + # run seems to disable errexit so the psql failure would be ingored and flag "password_rotated" set true + # Another subshell with errexit validates cancelation. + run bash -c "source '${BATS_TMPDIR}/post-upgrade-patched.sh'; set -o errexit; rotateSuperuserPassword" + + # assert + assert_failure + assert_line "Rotating superuser password..." + assert_equal "$(mock_get_call_args "${doguctl}" 4)" "config --encrypted password random123" + assert_equal "$(mock_get_call_num "${psql}")" "1" + + # password is set in config, flag "password_rotated" is NOT. + # Only four doguctl calls. + assert_equal "$(mock_get_call_num "${doguctl}")" "4" + refute_line "Superuser password rotated" +} + +@test "rotateSuperuserPassword() should not touch the database if writing the config fails" { + # arrange + load_script_safely + mock_set_output "${doguctl}" "false" 1 + mock_set_output "${doguctl}" "testusername" 2 + mock_set_output "${doguctl}" "random123" 3 + # the fourth call is the one writing the new password into the config + mock_set_status "${doguctl}" 1 4 + + # act + run bash -c "source '${BATS_TMPDIR}/post-upgrade-patched.sh'; set -o errexit; rotateSuperuserPassword" + + # assert + assert_failure + assert_line "Rotating superuser password..." + # If password could not be saved to config, "ALTER ... PASSWORD" should not be called + assert_equal "$(mock_get_call_num "${psql}")" "0" + assert_equal "$(mock_get_call_num "${doguctl}")" "4" + refute_line "Superuser password rotated" +} diff --git a/batsTests/startup.bats b/batsTests/startup.bats index 735d520..29f667d 100644 --- a/batsTests/startup.bats +++ b/batsTests/startup.bats @@ -70,3 +70,18 @@ Ziel Router Genmask Flags MSS Fenster irtt Iface assert_line '# container networks' assert_line "host all all 192.168.179.0/24 password" } + +@test "initAdmin() should set flag password_rotated true" { + # arrange + source /workspace/resources/startup.sh + mock_set_output "${doguctl}" "GEHEIM123" 2 + + # act + run initAdmin + + # assert + assert_success + assert_equal "$(mock_get_call_args "${doguctl}" 3)" "config -e password GEHEIM123" + assert_equal "$(mock_get_call_args "${doguctl}" 4)" "config password_rotated true" + assert_equal "$(mock_get_call_num "${doguctl}")" "4" +} \ No newline at end of file diff --git a/docs/development/upgrade_dogu_de.md b/docs/development/upgrade_dogu_de.md index 03647a7..6062eb5 100644 --- a/docs/development/upgrade_dogu_de.md +++ b/docs/development/upgrade_dogu_de.md @@ -33,12 +33,23 @@ Restore-Fall: Regulärer Migrationsfall: 1. Wenn kein Restore nötig ist und eine DB bereits initialisiert ist, startet `post-upgrade.sh` PostgreSQL temporär. -2. Danach werden Migrationsskripte aus `/docker-entrypoint-initdb.d` (aus `resources/migrations`) manuell ausgeführt. -3. PostgreSQL wird wieder gestoppt und `local_state` entfernt. +2. Das Superuser-Passwort wird einmalig rotiert (siehe unten). +3. Danach werden Migrationsskripte aus `/docker-entrypoint-initdb.d` (aus `resources/migrations`) manuell ausgeführt. +4. PostgreSQL wird wieder gestoppt und `local_state` entfernt. + +### Rotation des Superuser-Passworts (`rotateSuperuserPassword`) + +Vor `doguctl` v0.12.2 nutzte `doguctl random` Gos `math/rand` statt `crypto/rand`. Betroffene Werte sind nicht erkennbar, deshalb rotiert jede Instanz ohne Marker. + +1. Marker `password_rotated` in der Dogu-Config, kein Versionsvergleich. +2. Reihenfolge: Config, dann `ALTER USER`, dann Marker — eine abgebrochene Rotation wird wiederholt. +3. Läuft nach `startPostgresql`, weil `ALTER USER` eine laufende DB braucht. Über den Unix-Socket genügt `trust`, das alte Passwort wird nicht gebraucht. +4. Im Restore-Fall nicht erreicht - dort setzt `initAdmin` Passwort und Marker. ### Startup (`resources/startup.sh`) `startup.sh` wartet, solange `local_state=upgrading` gesetzt ist. +Bei leerem `PGDATA` (postgresql nicht installiert) setzt `initAdmin` zusätzlich `password_rotated=true`, da ein frisch erzeugtes Passwort sicher ist. Danach startet es `/usr/local/bin/docker-entrypoint.sh` mit den Dogu-spezifischen Parametern. Wichtig: diff --git a/docs/development/upgrade_dogu_en.md b/docs/development/upgrade_dogu_en.md index ec5924c..c9a6428 100644 --- a/docs/development/upgrade_dogu_en.md +++ b/docs/development/upgrade_dogu_en.md @@ -33,12 +33,23 @@ Restore case: Regular migration case: 1. If no restore is needed and a DB is already initialized, `post-upgrade.sh` starts PostgreSQL temporarily. -2. Then migration scripts from `/docker-entrypoint-initdb.d` (from `resources/migrations`) are executed manually. -3. PostgreSQL is stopped again and `local_state` is removed. +2. The superuser password is rotated once (see below). +3. Then migration scripts from `/docker-entrypoint-initdb.d` (from `resources/migrations`) are executed manually. +4. PostgreSQL is stopped again and `local_state` is removed. + +### Rotation of the superuser password (`rotateSuperuserPassword`) + +Before `doguctl` v0.12.2, `doguctl random` used Go's `math/rand` instead of `crypto/rand`. Affected values cannot be told apart from safe ones, so every instance without the marker rotates. + +1. Marker `password_rotated` in the Dogu config, no version comparison. +2. Order: config, then `ALTER USER`, then the marker — an aborted rotation is retried. +3. Runs after `startPostgresql`, because `ALTER USER` needs a running DB. On the Unix socket `trust` applies, so the old password is not needed. +4. Never reached in the restore case — there `initAdmin` sets password and marker. ### Startup (`resources/startup.sh`) `startup.sh` waits as long as `local_state=upgrading` is set. +If `PGDATA` is empty, `initAdmin` additionally sets `password_rotated=true`, because a freshly generated password is safe. After that, it starts `/usr/local/bin/docker-entrypoint.sh` with Dogu-specific parameters. Important: diff --git a/resources/post-upgrade.sh b/resources/post-upgrade.sh index 9fe3bad..47b53d2 100755 --- a/resources/post-upgrade.sh +++ b/resources/post-upgrade.sh @@ -9,6 +9,40 @@ source "/usr/local/bin/docker-entrypoint.sh" : "${PGDATA:?PGDATA is not set. Abort post-upgrade as the script needs the environment variable.}" +# rotateSuperuserPassword replaces the superuser password once, because it may have been generated by +# a doguctl older than v0.12.2 which used math/rand instead of crypto/rand. Affected values cannot be +# told apart from safe ones, so the rotation is unconditional and guarded by a config marker only. +function rotateSuperuserPassword() { + local alreadyRotated + alreadyRotated=$(doguctl config --default "false" "password_rotated") + + if [[ "${alreadyRotated}" == "true" ]]; then + echo "Superuser password has already been rotated; skipping" + return 0 + fi + + echo "Rotating superuser password..." + + local postgresUser + postgresUser=$(doguctl config --default "postgres" user) + + local newPassword + newPassword=$(doguctl random) + + # The config is written first: the new password must never exist in the database alone. + doguctl config --encrypted password "${newPassword}" + + psql \ + --variable=ON_ERROR_STOP=1 \ + --username="${postgresUser}" \ + --command="ALTER USER \"${postgresUser}\" WITH PASSWORD '${newPassword}';" + + # The marker is written last, so an aborted rotation is retried on the next upgrade. + doguctl config "password_rotated" "true" + + echo "Superuser password rotated" +} + function startPostgresql() { echo "starting postgresql temporary" docker_temp_server_start postgres @@ -89,6 +123,7 @@ function runPostUpgrade() { fi startPostgresql + rotateSuperuserPassword runMigrations stopPostgresql diff --git a/resources/startup.sh b/resources/startup.sh index 38673d8..7b963d6 100755 --- a/resources/startup.sh +++ b/resources/startup.sh @@ -34,6 +34,10 @@ function initAdmin() { # store the password encrypted doguctl config -e password "${postgres_psw}" + + # A freshly generated password is safe by definition, so the rotation marker is set right away and + # rotateSuperuserPassword in post-upgrade.sh never runs on this instance. + doguctl config "password_rotated" "true" } function mask2cidr() { diff --git a/resources/util.sh b/resources/util.sh index 679a999..07ecd57 100644 --- a/resources/util.sh +++ b/resources/util.sh @@ -58,4 +58,4 @@ function versionXLessOrEqualThanY() { fi return 1 -} \ No newline at end of file +}