From a5c92e470d095d04844317cc700b435db2ace97f Mon Sep 17 00:00:00 2001 From: Jakub Wartak Date: Fri, 21 Aug 2026 11:45:52 +0200 Subject: [PATCH vIDEAv1 5/5] pg_dumpall: dump role passwords with the PASSWORD command Previously pg_dumpall emitted a role's password inline as part of ALTER ROLE ... PASSWORD 'verifier', which put the (already-encrypted) password into the SQL text where statement logging could capture it on restore. Instead, set passwords with the new PASSWORD command, passing the role name and the verifier as bind parameters: PASSWORD $1, $2 \bind 'role' 'verifier' \g The secret no longer appears in the SQL text, and because the server suppresses parameter logging for the PASSWORD command, it is not written to the server log during restore. The stored verifier still round-trips exactly (SCRAM and MD5 alike). \bind and \g are psql meta-commands, which psql's restricted mode (the \restrict key that wraps dump output) forbids, so the password commands are collected and emitted together in a single \unrestrict/\restrict block after all the roles have been created -- restricted mode is toggled once, not per role. As with the \connect handling, everything emitted while unrestricted is injection-safe: appendPsqlMetaBindArg() writes each role name and verifier as an escaped, single-quoted \bind argument that no value can break out of. A new TAP test checks that the passwords are dumped via the PASSWORD command (never as a literal), that a quoted role name is escaped, that the block is wrapped once, that the verifiers round-trip through a psql restore, and that they do not appear in the server log. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/bin/pg_dump/meson.build | 1 + src/bin/pg_dump/pg_dumpall.c | 83 +++++++++++- src/bin/pg_dump/t/011_pg_dumpall_passwords.pl | 121 ++++++++++++++++++ 3 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 src/bin/pg_dump/t/011_pg_dumpall_passwords.pl diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build index 79bd5036841..b0fdf60ef42 100644 --- a/src/bin/pg_dump/meson.build +++ b/src/bin/pg_dump/meson.build @@ -104,6 +104,7 @@ tests += { 't/005_pg_dump_filterfile.pl', 't/006_pg_dump_compress.pl', 't/010_dump_connstr.pl', + 't/011_pg_dumpall_passwords.pl', ], }, } diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index c53e77c2878..2c1d8d16ff0 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -59,6 +59,7 @@ static void help(void); static void dropRoles(PGconn *conn); static void dumpRoles(PGconn *conn); +static void appendPsqlMetaBindArg(PQExpBuffer buf, const char *val); static void dumpRoleMembership(PGconn *conn); static void dumpRoleGUCPrivs(PGconn *conn); static void dropTablespaces(PGconn *conn); @@ -815,6 +816,44 @@ dropRoles(PGconn *conn) fprintf(OPF, "\n\n"); } +/* + * Append a value as a single argument to a psql backslash meta-command + * (specifically \bind), single-quoted and escaped so that psql's OT_NORMAL + * argument scanner reproduces it exactly. Inside such single quotes psql + * treats backslash as an escape character and '' as a literal quote, so we + * double any single quote and backslash-escape backslashes, newlines and + * carriage returns (the latter two to keep the command on one line). + */ +static void +appendPsqlMetaBindArg(PQExpBuffer buf, const char *val) +{ + const char *p; + + appendPQExpBufferChar(buf, '\''); + for (p = val; *p; p++) + { + switch (*p) + { + case '\'': + appendPQExpBufferStr(buf, "''"); + break; + case '\\': + appendPQExpBufferStr(buf, "\\\\"); + break; + case '\n': + appendPQExpBufferStr(buf, "\\n"); + break; + case '\r': + appendPQExpBufferStr(buf, "\\r"); + break; + default: + appendPQExpBufferChar(buf, *p); + break; + } + } + appendPQExpBufferChar(buf, '\''); +} + /* * Dump roles */ @@ -822,6 +861,7 @@ static void dumpRoles(PGconn *conn) { PQExpBuffer buf = createPQExpBuffer(); + PQExpBuffer passwordbuf = createPQExpBuffer(); PGresult *res; int i_oid, i_rolname, @@ -952,18 +992,31 @@ dumpRoles(PGconn *conn) PQgetvalue(res, i, i_rolconnlimit)); - if (!PQgetisnull(res, i, i_rolpassword) && !no_role_passwords) - { - appendPQExpBufferStr(buf, " PASSWORD "); - appendStringLiteralConn(buf, PQgetvalue(res, i, i_rolpassword), conn); - } - if (!PQgetisnull(res, i, i_rolvaliduntil)) appendPQExpBuffer(buf, " VALID UNTIL '%s'", PQgetvalue(res, i, i_rolvaliduntil)); appendPQExpBufferStr(buf, ";\n"); + /* + * Collect a command to set the password, passing the role name and the + * (already-encrypted) password as bind parameters rather than embedding + * the password in the SQL text. This keeps the secret out of the SQL, + * and the server suppresses parameter logging for the command, so it + * cannot be captured by statement logging on restore. + * + * These are emitted together, after all the roles have been created; + * see below. + */ + if (!PQgetisnull(res, i, i_rolpassword) && !no_role_passwords) + { + appendPQExpBufferStr(passwordbuf, "PASSWORD $1, $2 \\bind "); + appendPsqlMetaBindArg(passwordbuf, rolename); + appendPQExpBufferChar(passwordbuf, ' '); + appendPsqlMetaBindArg(passwordbuf, PQgetvalue(res, i, i_rolpassword)); + appendPQExpBufferStr(passwordbuf, " \\g\n"); + } + if (!no_comments && !PQgetisnull(res, i, i_rolcomment)) { appendPQExpBuffer(buf, "COMMENT ON ROLE %s IS ", fmtId(rolename)); @@ -979,6 +1032,23 @@ dumpRoles(PGconn *conn) fprintf(OPF, "%s", buf->data); } + /* + * Now that all the roles exist, emit the commands that set their passwords, + * as collected above. \bind and \g are psql meta-commands, so (as for + * \connect) we must leave restricted mode around them; we do it just once + * for the whole block rather than per role. Everything between the + * \unrestrict and \restrict must be injection-safe: the role names and + * passwords were written as escaped, single-quoted \bind arguments by + * appendPsqlMetaBindArg(), which no value can break out of. + */ + if (passwordbuf->len > 0) + { + fprintf(OPF, "\n--\n-- Role passwords\n--\n\n"); + fprintf(OPF, "\\unrestrict %s\n", restrict_key); + fprintf(OPF, "%s", passwordbuf->data); + fprintf(OPF, "\\restrict %s\n", restrict_key); + } + /* * Dump configuration settings for roles after all roles have been dumped. * We do it this way because config settings for roles could mention the @@ -995,6 +1065,7 @@ dumpRoles(PGconn *conn) fprintf(OPF, "\n\n"); destroyPQExpBuffer(buf); + destroyPQExpBuffer(passwordbuf); } diff --git a/src/bin/pg_dump/t/011_pg_dumpall_passwords.pl b/src/bin/pg_dump/t/011_pg_dumpall_passwords.pl new file mode 100644 index 00000000000..edb85138106 --- /dev/null +++ b/src/bin/pg_dump/t/011_pg_dumpall_passwords.pl @@ -0,0 +1,121 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Verify that pg_dumpall emits role passwords using the PASSWORD command with +# the value passed as a bind parameter (PASSWORD $1, $2 \bind ... \g) rather +# than embedding it in an ALTER ROLE ... PASSWORD statement. This keeps the +# (already-encrypted) password out of the SQL text and, because the server +# suppresses parameter logging for the command, out of the server log on +# restore. The dump must still round-trip the stored verifier exactly. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $tempdir = PostgreSQL::Test::Utils::tempdir; + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +# Allow the server to hash the clear-text passwords we set up, and log +# everything so we can prove the restore does not write them to the log. +$node->append_conf( + 'postgresql.conf', qq{ +cleartext_passwords_action = 'allow' +log_statement = 'all' +log_min_duration_statement = 0 +}); +$node->start; + +# Roles with SCRAM and MD5 verifiers, including one whose name needs quoting, +# to exercise the escaping of the \bind arguments. +$node->safe_psql( + 'postgres', q{ +SET password_encryption = 'scram-sha-256'; +CREATE ROLE dump_scram LOGIN PASSWORD 'scram_pw'; +CREATE ROLE "dump_weird'name" LOGIN PASSWORD 'weird_pw'; +SET password_encryption = 'md5'; +CREATE ROLE dump_md5 LOGIN PASSWORD 'md5_pw'; +}); + +# SQL literal for a role name, for use in WHERE clauses. +sub roleq +{ + my ($r) = @_; + $r =~ s/'/''/g; + return "'$r'"; +} + +my @roles = ('dump_scram', 'dump_md5', "dump_weird'name"); + +# Capture the stored verifiers before dumping. +my %before; +foreach my $r (@roles) +{ + $before{$r} = $node->safe_psql('postgres', + "SELECT rolpassword FROM pg_authid WHERE rolname = " . roleq($r)); + ok($before{$r} ne '', "role $r has a stored password"); +} + +# Dump globals. +my $globals = "$tempdir/globals.sql"; +$node->command_ok( + [ + 'pg_dumpall', '--globals-only', + '--file' => $globals, + '--port' => $node->port, + '--host' => $node->host, + ], + 'pg_dumpall --globals-only'); + +my $dump = slurp_file($globals); + +# The password must be set with the PASSWORD command and a bind parameter, +# never embedded in the SQL text. +like($dump, qr/PASSWORD \$1, \$2 \\bind 'dump_scram' 'SCRAM-SHA-256/, + 'SCRAM password dumped via PASSWORD ... \\bind'); +like($dump, qr/PASSWORD \$1, \$2 \\bind 'dump_md5' 'md5/, + 'MD5 password dumped via PASSWORD ... \\bind'); +like($dump, qr/PASSWORD \$1, \$2 \\bind 'dump_weird''name' /, + 'quoted role name is escaped in the \\bind argument'); +unlike($dump, qr/PASSWORD '/, + 'no role password is embedded as a SQL literal'); + +# All PASSWORD commands are emitted together in a single \unrestrict/\restrict +# block after the roles are created, rather than toggling restricted mode once +# per role. +like( + $dump, + qr/\\unrestrict \S+\n(?:PASSWORD \$1, \$2 \\bind [^\n]*\\g\n)+\\restrict \S+/, + 'PASSWORD commands are grouped in one \\unrestrict/\\restrict block'); + +# Drop the roles and restore the dump through psql; the verifiers must come +# back byte-for-byte, and the restore must not fail. +$node->safe_psql('postgres', + 'DROP ROLE dump_scram; DROP ROLE dump_md5; ' + . 'DROP ROLE "dump_weird\'name";'); + +my $log_offset = -s $node->logfile; +$node->command_ok( + [ 'psql', $node->connstr('postgres'), '--file' => $globals ], + 'restore globals through psql'); + +foreach my $r (@roles) +{ + my $after = $node->safe_psql('postgres', + "SELECT rolpassword FROM pg_authid WHERE rolname = " . roleq($r)); + is($after, $before{$r}, "verifier for $r round-trips exactly"); +} + +# The whole point: none of the verifiers may appear in the server log emitted +# during the restore, even though log_statement = all. +my $log = slurp_file($node->logfile, $log_offset); +foreach my $r (@roles) +{ + ok(index($log, $before{$r}) == -1, + "verifier for $r is not written to the log during restore"); +} + +$node->stop; + +done_testing(); -- 2.43.0