From faa0519cf11650eee58da1f94127fb381c23b85c Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 4 Aug 2026 10:59:45 +1000 Subject: [PATCH] fix: the RDS master user is no longer a PostgreSQL superuser (#687) --- docs/COMMANDS.md | 1 + .../rds-quickstart/README.md | 2 +- .../rds-quickstart/main.tf | 2 +- scripts/images/rds-postgres/rds-init | 40 ++++++++++++++--- scripts/images/rds-postgres/rds-init_test.sh | 43 +++++++++++++++++-- spinifex/handlers/rds/engine_postgres.go | 4 +- spinifex/handlers/rds/engine_test.go | 5 +++ tests/e2e/rds/connect_test.go | 32 ++++++++++++++ 8 files changed, 117 insertions(+), 12 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 2d664381e..9e2a390a2 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -918,6 +918,7 @@ Each DB instance is one dedicated system-owned VM running the engine directly, l - **Instance classes:** `db.t3.micro`, `db.t3.small`, `db.t3.medium`, `db.t3.large`, `db.m5.large`, `db.m5.xlarge` — a naming facade over the platform's EC2 sizing table. Any other class is rejected at create. - **Storage:** gp3 only, 20–65536 GiB, always encrypted with the cluster key. Grow-only, and a grow is **stop/start with downtime** — the volume cannot be resized while attached. - **TLS:** offered, not enforced. The engine serves a per-instance certificate signed by the cluster CA (the same `ca.pem` baked into AMIs), with the ENI IP and the DNS name both in the SAN set, so `sslmode=verify-full` works by name or by address. Clients opt in; a deployment with no cluster CA starts the engine without TLS. +- **Master user:** administrative but **not a PostgreSQL superuser**, as on AWS. It is created with `CREATEDB` and `CREATEROLE`, owns the initial database, and holds the `rds_superuser` group role (`pg_monitor`, `pg_signal_backend`, `pg_checkpoint`) with `ADMIN OPTION`, so it can grant the same set to roles it creates. It cannot `COPY ... FROM/TO PROGRAM`, read or write server-side files, run `ALTER SYSTEM`, or install **untrusted** extensions — those need the cluster superuser, which no customer credential is. Trusted extensions install normally in a database the master owns. `postgres`, `rdsadmin` and `rds_superuser` are reserved and rejected as `--master-username`. - **Backups:** daily COW snapshots of the data volume inside `PreferredBackupWindow`. Retention defaults to 7 days and is capped at 7; `0` disables automated backups. Point-in-time recovery is not implemented. - **Availability:** single-AZ. An instance whose host is lost is reported `failed` with a reason in `StatusInfos`; recovery is operator-driven. Engine crashes are restarted in-guest and VM crashes on a live host are restarted by `ec2-health-restart`. diff --git a/docs/terraform-workbooks/rds-quickstart/README.md b/docs/terraform-workbooks/rds-quickstart/README.md index 74164c527..e9975b19d 100644 --- a/docs/terraform-workbooks/rds-quickstart/README.md +++ b/docs/terraform-workbooks/rds-quickstart/README.md @@ -122,7 +122,7 @@ psql "sslmode=verify-full sslrootcert=/home/ubuntu/ca.pem" \ | `db_instance_class` | `db.t3.micro` | DB class. One of the curated `db.*` subset. | | `engine_version` | `18` | PostgreSQL major. `18` is the only version served. | | `db_name` | `appdb` | Database created at bootstrap. | -| `db_username` | `appuser` | Master user. `postgres`, `rdsadmin` and `pg_*` are reserved. | +| `db_username` | `appuser` | Master user. `postgres`, `rdsadmin`, `rds_superuser` and `pg_*` are reserved. | | `db_password` | `QuickstartS3cret1` | Master password — override it. No `/`, `"`, `@` or spaces. | | `allocated_storage` | `20` | Data-volume GiB. Grow-only, and a grow is stop/start. | diff --git a/docs/terraform-workbooks/rds-quickstart/main.tf b/docs/terraform-workbooks/rds-quickstart/main.tf index 48d805411..9cf7f1070 100644 --- a/docs/terraform-workbooks/rds-quickstart/main.tf +++ b/docs/terraform-workbooks/rds-quickstart/main.tf @@ -105,7 +105,7 @@ variable "db_name" { variable "db_username" { type = string default = "appuser" - description = "Master user. postgres, rdsadmin and pg_* are reserved by the engine." + description = "Master user. postgres, rdsadmin, rds_superuser and pg_* are reserved by the engine." } # No '/', '"', '@' or spaces: the characters the API rejects because they break a diff --git a/scripts/images/rds-postgres/rds-init b/scripts/images/rds-postgres/rds-init index 26c319b1b..c52bce80e 100644 --- a/scripts/images/rds-postgres/rds-init +++ b/scripts/images/rds-postgres/rds-init @@ -49,6 +49,10 @@ BOOTSTRAP_DIR="${RDS_BOOTSTRAP_DIR:-/run/rds-init}" MOUNTS_FILE="${RDS_MOUNTS_FILE:-/proc/mounts}" +# The group role the master's administrative privileges hang off. Named as on +# AWS, so a customer's `GRANT rds_superuser TO ...` works here too. +MASTER_GROUP_ROLE="rds_superuser" + log() { echo "[rds-init] $*"; } fail() { echo "[rds-init] ERROR: $*" >&2; exit 1; } @@ -85,6 +89,17 @@ load_handoff() { RDS_PORT="${RDS_PORT:-5432}" [ -n "${RDS_MASTER_USERNAME}" ] || fail "bootstrap config has no RDS_MASTER_USERNAME" + + # The master role is created NOSUPERUSER, so a master named after the + # bootstrap superuser would leave the cluster without one and strip rds-agent + # of the privileged SQL it reaches over peer auth — unrepairable, since the + # datadir bootstraps once. The control plane reserves both names already. + case "${RDS_MASTER_USERNAME}" in + "${PG_USER}" | "${MASTER_GROUP_ROLE}") + fail "RDS_MASTER_USERNAME ${RDS_MASTER_USERNAME} is reserved by the platform; the control plane should have rejected it" + ;; + esac + case "${RDS_PORT}" in '' | *[!0-9]*) fail "bootstrap config has a non-numeric RDS_PORT: ${RDS_PORT}" ;; esac @@ -264,6 +279,7 @@ psql_bootstrap() { RDS_MASTER_USERNAME="${RDS_MASTER_USERNAME}" \ RDS_MASTER_PASSWORD="${RDS_MASTER_PASSWORD}" \ RDS_DB_NAME="${RDS_DB_NAME}" \ + RDS_GROUP_ROLE="${MASTER_GROUP_ROLE}" \ "${PG_BIN}/psql" --no-psqlrc --quiet --no-align --tuples-only \ -v ON_ERROR_STOP=1 \ -h "${BOOTSTRAP_DIR}" -p "${RDS_PORT}" -U "${PG_USER}" -d postgres -f - @@ -291,18 +307,32 @@ bootstrap_master() { fi } -# initdb may already have made it the bootstrap superuser, when the master -# username is "postgres". +# The master role is administrative but deliberately not a PostgreSQL SUPERUSER, +# as on AWS: a superuser reaches outside the database entirely — COPY FROM +# PROGRAM, pg_read_file/pg_write_file, untrusted languages — so master +# credentials would be a shell in the DB VM rather than a database privilege. +# The cluster's own superuser stays the bootstrap role, which only rds-init and +# rds-agent reach, over the unix socket under peer auth. apply_master_role() { - log "setting the master password for role ${RDS_MASTER_USERNAME}" + log "creating the master role ${RDS_MASTER_USERNAME}" + # PG16 hardened CREATEROLE: it can neither grant itself SUPERUSER nor take + # membership in a predefined role it does not administer, so CREATEROLE here + # is not a route back to the privileges above. psql_bootstrap <<'SQL' \getenv master RDS_MASTER_USERNAME \getenv password RDS_MASTER_PASSWORD +\getenv grp RDS_GROUP_ROLE +SELECT NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = :'grp') AS create_group \gset +\if :create_group +CREATE ROLE :"grp" NOLOGIN; +\endif +GRANT pg_monitor, pg_signal_backend, pg_checkpoint TO :"grp"; SELECT NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = :'master') AS create_role \gset \if :create_role -CREATE ROLE :"master" WITH LOGIN SUPERUSER; +CREATE ROLE :"master" WITH LOGIN NOSUPERUSER CREATEDB CREATEROLE; \endif -ALTER ROLE :"master" WITH LOGIN SUPERUSER PASSWORD :'password'; +ALTER ROLE :"master" WITH LOGIN NOSUPERUSER CREATEDB CREATEROLE PASSWORD :'password'; +GRANT :"grp" TO :"master" WITH ADMIN OPTION; SQL } diff --git a/scripts/images/rds-postgres/rds-init_test.sh b/scripts/images/rds-postgres/rds-init_test.sh index ea4592caa..6992555d7 100644 --- a/scripts/images/rds-postgres/rds-init_test.sh +++ b/scripts/images/rds-postgres/rds-init_test.sh @@ -111,7 +111,7 @@ EOF cat > "${STUBBIN}/psql" <<'EOF' #!/bin/sh { - echo "--- psql $* [master=${RDS_MASTER_USERNAME:-} password=${RDS_MASTER_PASSWORD:-} db=${RDS_DB_NAME:-}]" + echo "--- psql $* [master=${RDS_MASTER_USERNAME:-} password=${RDS_MASTER_PASSWORD:-} db=${RDS_DB_NAME:-} grp=${RDS_GROUP_ROLE:-}]" cat } >> "${PSQL_CALLS}" # ON_ERROR_STOP makes a real psql exit non-zero on any SQL failure; the stub @@ -140,11 +140,12 @@ export INITDB_CALLS PGCTL_CALLS PSQL_CALLS # write_handoff : lay down the bootstrap.env rds-agent # would have written. An empty password omits the field, as `attach` does. +# MASTER_USER overrides the master role name, for the reserved-name cases. write_handoff() { mkdir -p "${HANDOFF}" { echo "RDS_MODE=$1" - echo "RDS_MASTER_USERNAME=mulgamaster" + echo "RDS_MASTER_USERNAME=${MASTER_USER:-mulgamaster}" [ -n "$2" ] && echo "RDS_MASTER_PASSWORD=$2" [ -n "$3" ] && echo "RDS_DB_NAME=$3" echo "RDS_PORT=6543" @@ -171,7 +172,7 @@ reset_state() { : > "${INITDB_CALLS}" : > "${PGCTL_CALLS}" : > "${PSQL_CALLS}" - unset INITDB_FAIL PG_HBA_AS_DIR PSQL_FAIL LS_FAIL RDS_ALLOW_LOCAL_DATADIR || true + unset INITDB_FAIL PG_HBA_AS_DIR PSQL_FAIL LS_FAIL RDS_ALLOW_LOCAL_DATADIR MASTER_USER || true } # run: invoke rds-init with every path knob pointed into the temp dir. @@ -213,12 +214,31 @@ if run_ok "initialize"; then && pass "initialize: ssl enabled" || fail "initialize: ssl not enabled" grep -q 'shared_buffers = 128MB' "${PGDATA}/conf.d/10-rds-parameters.conf" \ && pass "initialize: resolved parameters installed" || fail "initialize: no parameter include" - grep -q 'ALTER ROLE :"master" WITH LOGIN SUPERUSER PASSWORD' "${PSQL_CALLS}" \ + grep -q 'ALTER ROLE :"master" WITH LOGIN NOSUPERUSER CREATEDB CREATEROLE PASSWORD' "${PSQL_CALLS}" \ && pass "initialize: master password applied" || fail "initialize: no ALTER ROLE" grep -q 'password=s3cr3t' "${PSQL_CALLS}" \ && pass "initialize: password passed through the environment" || fail "initialize: password not in psql env" grep -q 'CREATE DATABASE' "${PSQL_CALLS}" \ && pass "initialize: initial database created" || fail "initialize: no CREATE DATABASE" + + # The master role is administrative but not a PostgreSQL superuser: a + # superuser reaches outside the database (COPY FROM PROGRAM, pg_read_file, + # untrusted languages), so master credentials would be a shell in the DB VM. + grep -qE '(^|[^O])SUPERUSER' "${PSQL_CALLS}" \ + && fail "initialize: bootstrap SQL still grants SUPERUSER" \ + || pass "initialize: master role is not a superuser" + grep -q 'GRANT :"grp" TO :"master" WITH ADMIN OPTION' "${PSQL_CALLS}" \ + && pass "initialize: master holds the administrative group role" \ + || fail "initialize: master never granted the group role" + grep -q 'grp=rds_superuser' "${PSQL_CALLS}" \ + && pass "initialize: the group role is named as on AWS" || fail "initialize: group role name not delivered" + grep -q 'GRANT pg_monitor, pg_signal_backend, pg_checkpoint TO :"grp"' "${PSQL_CALLS}" \ + && pass "initialize: monitoring/signal/checkpoint granted" || fail "initialize: group role has no privileges" + + # The three predefined roles that hand back exactly what SUPERUSER gave away. + grep -qE 'pg_(read|write)_server_files|pg_execute_server_program' "${PSQL_CALLS}" \ + && fail "initialize: granted a role that restores file or program access" \ + || pass "initialize: no server-file or program-execution roles granted" grep -q "listen_addresses=''" "${PGCTL_CALLS}" \ && pass "initialize: bootstrap server is socket-only" || fail "initialize: bootstrap server listened on TCP" grep -q 'stop' "${PGCTL_CALLS}" \ @@ -400,6 +420,21 @@ reset_state rm -rf "${HANDOFF}" run_fails "no-handoff" +# --- Case 9: a master username the platform reserves --- +# The master role is created NOSUPERUSER, so one named after the bootstrap +# superuser would leave the cluster without a superuser at all and strip +# rds-agent of the privileged SQL it needs — on a datadir that bootstraps once. +for reserved in postgres rds_superuser; do + reset_state + MASTER_USER="${reserved}" + write_handoff initialize 's3cr3t' appdb + run_fails "reserved-master-${reserved}" + [ -s "${INITDB_CALLS}" ] \ + && fail "reserved-master-${reserved}: initdb ran before the name was refused" \ + || pass "reserved-master-${reserved}: refused before initdb" + unset MASTER_USER +done + if [ "${FAILS}" -eq 0 ]; then echo "PASS: all rds-init cases" exit 0 diff --git a/spinifex/handlers/rds/engine_postgres.go b/spinifex/handlers/rds/engine_postgres.go index 8c11944bb..147716d2f 100644 --- a/spinifex/handlers/rds/engine_postgres.go +++ b/spinifex/handlers/rds/engine_postgres.go @@ -8,6 +8,8 @@ var enginePostgres = Engine{ DefaultPort: 5432, // rdsadmin is the management role AWS reserves; postgres is the cluster // superuser initdb creates, which the master role must not collide with. - reservedUsernames: []string{"rdsadmin", "postgres"}, + // rds_superuser is the group role the bootstrap grants the master its + // administrative privileges through. + reservedUsernames: []string{"rdsadmin", "postgres", "rds_superuser"}, reservedUsernamePrefixes: []string{"pg_"}, } diff --git a/spinifex/handlers/rds/engine_test.go b/spinifex/handlers/rds/engine_test.go index 8579fae15..1d86131b6 100644 --- a/spinifex/handlers/rds/engine_test.go +++ b/spinifex/handlers/rds/engine_test.go @@ -57,6 +57,11 @@ func TestEngineValidateMasterUsername(t *testing.T) { "too long": strings.Repeat("a", maxMasterUsernameLen+1), "reserved": "rdsadmin", "reserved cased": "RDSAdmin", + // The cluster superuser and the group role the master's administrative + // privileges come through. Both exist by the time the master is created, + // so a collision would surface as a failed bootstrap inside the guest. + "cluster superuser": "postgres", + "group role": "rds_superuser", // postgres reserves the pg_ prefix for its own internal roles, so a master // role taking one would collide inside the engine, not at the API. "reserved prefix": "pg_backup", diff --git a/tests/e2e/rds/connect_test.go b/tests/e2e/rds/connect_test.go index 22e6de3a5..b2427b1d2 100644 --- a/tests/e2e/rds/connect_test.go +++ b/tests/e2e/rds/connect_test.go @@ -149,6 +149,38 @@ func TestConnectivity(t *testing.T) { }, sgSettleTimeout, 5*time.Second) }) + // D21: the master user is administrative but not a PostgreSQL superuser. This + // is the only leg that can prove it — the bootstrap unit test sees the SQL + // that was sent, not what the engine then refuses. + t.Run("TheMasterUserIsAdministrativeButNotASuperuser", func(t *testing.T) { + out := harness.PSQL(t, client, byIP, "SELECT rolsuper FROM pg_roles WHERE rolname = current_user;") + assert.Equal(t, "f", strings.TrimSpace(out), + "the master user must not be a PostgreSQL superuser") + + // A superuser master is command execution as the postgres OS user inside + // the DB VM, which is what makes this the one assertion that matters. + out, err := harness.TryPSQL(client, byIP, + "CREATE TEMP TABLE rce(l text); COPY rce FROM PROGRAM 'id';") + require.Error(t, err, "COPY FROM PROGRAM must be refused: %s", out) + // Matched on the phrase the engine's own refusal carries, rather than on + // the privilege it names: PG16 reworded that half of the message. + assert.Contains(t, out, "external program", + "the engine must refuse the program COPY, not fail for some other reason") + + // The administrative capability a customer is actually owed, on the same + // connection: create a database, create a role, install a trusted extension. + harness.PSQL(t, client, byIP, "CREATE DATABASE e2e_master_admin;") + t.Cleanup(func() { harness.PSQL(t, client, byIP, "DROP DATABASE IF EXISTS e2e_master_admin;") }) + + harness.PSQL(t, client, byIP, "CREATE ROLE e2e_master_role LOGIN PASSWORD 'e2eR0leSecret1';") + t.Cleanup(func() { harness.PSQL(t, client, byIP, "DROP ROLE IF EXISTS e2e_master_role;") }) + + harness.PSQL(t, client, byIP, "CREATE EXTENSION IF NOT EXISTS pg_trgm;") + out = harness.PSQL(t, client, byIP, "SELECT extname FROM pg_extension WHERE extname = 'pg_trgm';") + assert.Equal(t, "pg_trgm", strings.TrimSpace(out), + "the master user must be able to install a trusted extension in a database it owns") + }) + // Last: it retires the credential every subtest above connects with. D8 keeps // no cleartext password anywhere, so the only proof the rotation reached the // engine is that the new one authenticates and the old one does not.