From 05cb06921589bc9422d2235008b67bfa08968f35 Mon Sep 17 00:00:00 2001 From: Jakub Wartak Date: Fri, 21 Aug 2026 10:02:37 +0200 Subject: [PATCH vIDEAv1 2/5] Add PASSWORD command to set a role's password without logging it PQchangePassword() and psql's \password already avoid sending a clear text password by encrypting it on the client, but they still embed the resulting verifier in an ALTER USER ... PASSWORD statement, where it can be captured by statement logging. More generally there was no way to set a password such that neither the secret nor its hash could reach the server log. Add a new command, "PASSWORD $1, $2", whose first argument is the target role and second is the new password. Like CALL it is parse-analyzed so its arguments become bind parameters, and it insists that both arguments be external ($n) parameters: it can therefore only be used through the extended query protocol, where the values travel in a Bind message and never appear in the statement text. Using it via the simple protocol, or with literal arguments, is rejected. While such a command is processed, the protocol handlers suppress statement and parameter logging (log_statement, log_min_duration_statement, log_duration, log_parameter_max_length[_on_error], log_min_messages and log_error_verbosity), restoring them afterwards and, on error, from PostgresMain()'s recovery path. This keeps the password out of the log even under maximally verbose logging. Execution delegates to AlterRole(), so the command shares its privilege model exactly: an unprivileged role may change its own password, while changing another role's still requires CREATEROLE plus ADMIN OPTION. It also reuses password encryption (already-hashed passwords pass through unchanged), the check_password_hook and empty-password handling. PQchangePassword() now issues this command via PQexecParams(), still encrypting on the client, so \password benefits automatically. Co-Authored-By: Claude Opus 4.8 (1M context) --- doc/src/sgml/ref/allfiles.sgml | 1 + doc/src/sgml/ref/password.sgml | 131 ++++++++++++++++ doc/src/sgml/reference.sgml | 1 + src/backend/commands/user.c | 98 ++++++++++++ src/backend/parser/analyze.c | 61 ++++++++ src/backend/parser/gram.y | 27 +++- src/backend/tcop/postgres.c | 141 ++++++++++++++++++ src/backend/tcop/utility.c | 13 ++ src/include/commands/user.h | 3 + src/include/nodes/parsenodes.h | 22 +++ src/include/tcop/cmdtaglist.h | 1 + src/interfaces/libpq/fe-auth.c | 67 ++++----- src/test/modules/test_misc/meson.build | 1 + .../test_misc/t/015_password_command.pl | 121 +++++++++++++++ 14 files changed, 645 insertions(+), 43 deletions(-) create mode 100644 doc/src/sgml/ref/password.sgml create mode 100644 src/test/modules/test_misc/t/015_password_command.pl diff --git a/doc/src/sgml/ref/allfiles.sgml b/doc/src/sgml/ref/allfiles.sgml index e1a56c36221..c7fe6f72f7c 100644 --- a/doc/src/sgml/ref/allfiles.sgml +++ b/doc/src/sgml/ref/allfiles.sgml @@ -164,6 +164,7 @@ Complete list of usable sgml source files in this directory. + diff --git a/doc/src/sgml/ref/password.sgml b/doc/src/sgml/ref/password.sgml new file mode 100644 index 00000000000..458ae2353d4 --- /dev/null +++ b/doc/src/sgml/ref/password.sgml @@ -0,0 +1,131 @@ + + + + + PASSWORD + + + + PASSWORD + 7 + SQL - Language Statements + + + + PASSWORD + set a role's password without exposing it in the server log + + + + +PASSWORD $1, $2 + + + + + Description + + + PASSWORD changes the password of a database role. + The first parameter ($1) is the name of the target + role and the second parameter ($2) is the new + password. + + + + Unlike ALTER ROLE ... PASSWORD, + the command accepts its arguments only as bind + parameters supplied through the + extended query protocol; + it cannot be issued through the simple query protocol, and its arguments + cannot be written as literals or other expressions. Because the password + therefore never appears in the statement text (it travels only in a + Bind message), and because the server suppresses + statement and parameter logging while the command runs, the password is + kept out of the server log, pg_stat_activity, + and similar facilities. + + + + The new password is processed exactly as for + ALTER ROLE ... PASSWORD: a clear-text password is + encrypted according to the + setting (subject to + ), while an + already-encrypted password (for example a SCRAM verifier computed on the + client) is stored unchanged. Client applications that want to change a + password should generally use + PQchangePassword, + which encrypts the password on the client and then issues this command; + psql's + \password + does this for you. + + + + + Parameters + + + + $1 + + + The name of the role whose password is to be changed, supplied as a + bind parameter. + + + + + + $2 + + + The new password (clear text, or an already-encrypted password), + supplied as a bind parameter. + + + + + + + + Notes + + + An unprivileged role may change its own password (that is, when + $1 is the current user). Changing another role's + password requires the CREATEROLE attribute together + with ADMIN OPTION on the target role, the same rule + that ALTER ROLE enforces. + + + + Issuing the command through the simple query protocol, or with the + arguments written as anything other than the bind parameters + $1 and $2, results in an error. + + + + + Compatibility + + + The PASSWORD command is a + PostgreSQL extension. + + + + + See Also + + + + + + + + diff --git a/doc/src/sgml/reference.sgml b/doc/src/sgml/reference.sgml index 674ac17e82c..8ecdc9ceb75 100644 --- a/doc/src/sgml/reference.sgml +++ b/doc/src/sgml/reference.sgml @@ -192,6 +192,7 @@ &merge; &move; ¬ify; + &password; &prepare; &prepareTransaction; &reassignOwned; diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 04b270c08a8..cc4b2a087e8 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -30,8 +30,10 @@ #include "commands/defrem.h" #include "commands/seclabel.h" #include "commands/user.h" +#include "executor/executor.h" #include "libpq/crypt.h" #include "miscadmin.h" +#include "nodes/makefuncs.h" #include "port/pg_bitutils.h" #include "storage/lmgr.h" #include "utils/acl.h" @@ -1000,6 +1002,102 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt) } +/* + * ExecPasswordStmt + * + * Executes the PASSWORD command (PASSWORD $1, $2). The two arguments are + * bind parameters (guaranteed to be external Params by parse analysis); we + * evaluate them from the supplied ParamListInfo to obtain the target role + * name and the new password, then delegate to AlterRole(). + * + * Delegating to AlterRole() means this command shares ALTER ROLE's privilege + * model exactly: an unprivileged role may change its own password, while + * changing another role's password still requires CREATEROLE plus ADMIN + * OPTION. It also reuses the password-checking hook, the server-side + * encryption of clear-text passwords (already-hashed passwords such as the + * SCRAM verifier sent by psql's \password are passed through unchanged), and + * the empty-password handling. + * + * The value of the command is that the secret is transmitted only in a Bind + * message and never appears in the SQL text; the protocol handlers in + * postgres.c additionally suppress parameter/statement logging for the + * duration of the command so it cannot leak into the server log. + */ +void +ExecPasswordStmt(ParseState *pstate, PasswordStmt *stmt, ParamListInfo params) +{ + EState *estate; + ExprContext *econtext; + ExprState *user_exprstate; + ExprState *pass_exprstate; + Datum user_datum; + Datum pass_datum; + bool user_isnull; + bool pass_isnull; + char *username; + char *password; + RoleSpec *role; + AlterRoleStmt *alterstmt; + + /* + * The command only makes sense through the extended query protocol, where + * a Bind message supplies the two parameter values. Parse analysis has + * already required both arguments to be external Params, but guard here + * too so we never dereference a missing parameter list. + */ + if (params == NULL || params->numParams < 2) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("PASSWORD requires parameters supplied through the extended query protocol"))); + + /* Evaluate the two argument expressions against the bound parameters. */ + estate = CreateExecutorState(); + estate->es_param_list_info = params; + econtext = CreateExprContext(estate); + + user_exprstate = ExecPrepareExpr((Expr *) stmt->user, estate); + user_datum = ExecEvalExprSwitchContext(user_exprstate, econtext, + &user_isnull); + + pass_exprstate = ExecPrepareExpr((Expr *) stmt->password, estate); + pass_datum = ExecEvalExprSwitchContext(pass_exprstate, econtext, + &pass_isnull); + + if (user_isnull) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("role name for PASSWORD must not be null"))); + if (pass_isnull) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("password for PASSWORD must not be null"))); + + /* Copy the values out of the (soon to be freed) executor context. */ + username = TextDatumGetCString(user_datum); + password = TextDatumGetCString(pass_datum); + + /* + * Build and execute the equivalent of + * ALTER ROLE PASSWORD + */ + role = makeNode(RoleSpec); + role->roletype = ROLESPEC_CSTRING; + role->rolename = username; + role->location = stmt->location; + + alterstmt = makeNode(AlterRoleStmt); + alterstmt->role = role; + alterstmt->action = 0; /* not adding or dropping members */ + alterstmt->options = list_make1(makeDefElem("password", + (Node *) makeString(password), + stmt->location)); + + (void) AlterRole(pstate, alterstmt); + + FreeExecutorState(estate); +} + + /* * ALTER ROLE ... SET */ diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 263d1b6e1cc..e3cb9b41954 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -106,6 +106,8 @@ static Query *transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt); static Query *transformCallStmt(ParseState *pstate, CallStmt *stmt); +static Query *transformPasswordStmt(ParseState *pstate, + PasswordStmt *stmt); static void transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, bool pushedDown); #ifdef DEBUG_NODE_TESTS_ENABLED @@ -432,6 +434,11 @@ transformStmt(ParseState *pstate, Node *parseTree) (CallStmt *) parseTree); break; + case T_PasswordStmt: + result = transformPasswordStmt(pstate, + (PasswordStmt *) parseTree); + break; + default: /* @@ -493,6 +500,7 @@ stmt_requires_parse_analysis(RawStmt *parseTree) case T_ExplainStmt: case T_CreateTableAsStmt: case T_CallStmt: + case T_PasswordStmt: result = true; break; @@ -3705,6 +3713,59 @@ transformCallStmt(ParseState *pstate, CallStmt *stmt) return result; } +/* + * transformPasswordStmt - + * transforms a PASSWORD statement (PASSWORD $1, $2) + * + * Like CALL, this is a utility statement that we nonetheless run through parse + * analysis, so that its two argument expressions become resolvable Param nodes + * that the executor can fill in from the Bind message. We insist that both + * arguments are external ($n) parameters: that is what forces the command to + * be used through the extended query protocol (where a Bind supplies the + * values) and guarantees the secret never appears in the statement text. + */ +static Query * +transformPasswordStmt(ParseState *pstate, PasswordStmt *stmt) +{ + Query *result; + Node *user; + Node *password; + + user = transformExpr(pstate, stmt->user, EXPR_KIND_CALL_ARGUMENT); + user = coerce_to_specific_type(pstate, user, TEXTOID, "PASSWORD"); + + password = transformExpr(pstate, stmt->password, EXPR_KIND_CALL_ARGUMENT); + password = coerce_to_specific_type(pstate, password, TEXTOID, "PASSWORD"); + + /* + * Require both arguments to be bind parameters supplied via the extended + * query protocol. A simple-protocol query has no way to supply parameter + * values, and inlining the password as a literal would defeat the purpose + * of the command (keeping the secret out of the statement text and the + * logs), so reject anything that is not a plain external Param. + */ + if (!IsA(user, Param) || ((Param *) user)->paramkind != PARAM_EXTERN || + !IsA(password, Param) || ((Param *) password)->paramkind != PARAM_EXTERN) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("PASSWORD requires its arguments to be supplied as bind parameters"), + errhint("Use the extended query protocol and pass the user name and password as $1 and $2."), + parser_errposition(pstate, stmt->location))); + + assign_expr_collations(pstate, user); + assign_expr_collations(pstate, password); + + stmt->user = user; + stmt->password = password; + + /* represent the command as a utility Query */ + result = makeNode(Query); + result->commandType = CMD_UTILITY; + result->utilityStmt = (Node *) stmt; + + return result; +} + /* * Produce a string representation of a LockClauseStrength value. * This should only be applied to valid values (not LCS_NONE). diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 17035fb4d15..7a0367f0aaf 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -287,7 +287,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); AlterCompositeTypeStmt AlterUserMappingStmt AlterRoleStmt AlterRoleSetStmt AlterPolicyStmt AlterStatsStmt AlterDefaultPrivilegesStmt DefACLAction - AnalyzeStmt CallStmt ClosePortalStmt CommentStmt + AnalyzeStmt CallStmt ClosePortalStmt CommentStmt PasswordStmt ConstraintsSetStmt CopyStmt CreateAsStmt CreateCastStmt CreateDomainStmt CreateExtensionStmt CreateGroupStmt CreateOpClassStmt CreateOpFamilyStmt AlterOpFamilyStmt CreatePLangStmt @@ -1152,6 +1152,7 @@ stmt: | LockStmt | MergeStmt | NotifyStmt + | PasswordStmt | PrepareStmt | ReassignOwnedStmt | ReindexStmt @@ -1258,6 +1259,30 @@ CallStmt: CALL func_application } ; +/***************************************************************************** + * + * PASSWORD $1, $2 + * + * Set a role's password. The first argument is the target role name and the + * second is the new password; both are ordinary expressions but are required + * (during parse analysis) to be bind parameters, so that the secret is only + * ever supplied through a Bind message of the extended query protocol and + * never appears in the statement text. + * + *****************************************************************************/ + +PasswordStmt: + PASSWORD a_expr ',' a_expr + { + PasswordStmt *n = makeNode(PasswordStmt); + + n->user = $2; + n->password = $4; + n->location = @1; + $$ = (Node *) n; + } + ; + /***************************************************************************** * * Create a new Postgres DBMS role diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index b6bdfe213fe..bf90bed9383 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -183,6 +183,8 @@ static int SocketBackend(StringInfo inBuf); static int ReadCommand(StringInfo inBuf); static void forbidden_in_wal_sender(char firstchar); static bool check_log_statement(List *stmt_list); +static void disarm_secret_logging(void); +static void restore_secret_logging(void); static char *truncate_query_log(const char *query); static int errdetail_execute(List *raw_parsetree_list); static int errdetail_params(ParamListInfo params); @@ -1082,6 +1084,28 @@ exec_simple_query(const char *query_string) */ parsetree_list = pg_parse_query(query_string); + /* + * The PASSWORD command may only be used through the extended query + * protocol, where its arguments arrive as bind parameters. Reject it + * here before any statement logging can copy the (possibly secret-bearing) + * query text into the log; disarm logging first, and hide the statement + * text from the error report, so nothing leaks even in this failure path. + */ + foreach(parsetree_item, parsetree_list) + { + RawStmt *pstmt = lfirst_node(RawStmt, parsetree_item); + + if (IsA(pstmt->stmt, PasswordStmt)) + { + disarm_secret_logging(); + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("PASSWORD can only be executed through the extended query protocol"), + errhint("Send \"PASSWORD $1, $2\" and supply the user name and password as bind parameters."), + errhidestmt(true))); + } + } + /* Log immediately if dictated by log_statement */ if (check_log_statement(parsetree_list)) { @@ -1714,6 +1738,15 @@ exec_bind_message(StringInfo input_message) */ debug_query_string = psrc->query_string; + /* + * The PASSWORD command receives its secret in this Bind message. Disarm + * parameter and statement logging now, before any logging can occur, so + * that the password cannot be written to the server log. It is restored + * at the end of the message (and, on error, from PostgresMain()). + */ + if (psrc->commandTag == CMDTAG_PASSWORD) + disarm_secret_logging(); + pgstat_report_activity(STATE_RUNNING, psrc->query_string); foreach(lc, psrc->query_list) @@ -2139,6 +2172,9 @@ exec_bind_message(StringInfo input_message) valgrind_report_error_query(debug_query_string); + /* Restore any logging settings suppressed for a PASSWORD command. */ + restore_secret_logging(); + debug_query_string = NULL; } @@ -2212,6 +2248,15 @@ exec_execute_message(const char *portal_name, long max_rows) */ debug_query_string = sourceText; + /* + * A PASSWORD command's secret was supplied as a bind parameter; disarm + * parameter and statement logging before any logging can occur so it + * cannot reach the server log. Restored at the end of the message (and, + * on error, from PostgresMain()). + */ + if (portal->commandTag == CMDTAG_PASSWORD) + disarm_secret_logging(); + pgstat_report_activity(STATE_RUNNING, sourceText); foreach(lc, portal->stmts) @@ -2425,9 +2470,96 @@ exec_execute_message(const char *portal_name, long max_rows) valgrind_report_error_query(debug_query_string); + /* Restore any logging settings suppressed for a PASSWORD command. */ + restore_secret_logging(); + debug_query_string = NULL; } +/* + * Support for keeping a role's password out of the server log. + * + * The PASSWORD command (see gram.y / user.c) carries the secret only in a Bind + * message of the extended query protocol. Several logging facilities could + * otherwise copy that value into the log: parameter logging for statement and + * duration logs (log_parameter_max_length), parameter logging in the error + * context callback (log_parameter_max_length_on_error), and, in the simple + * protocol rejection path, statement-text logging. While such a command is + * being processed we override the relevant logging GUCs and restore them as + * soon as the message has been handled. In effect, for the command only: + * + * log_statement = none + * log_duration = off + * log_min_duration_statement = -1 + * log_min_messages = error + * log_error_verbosity = terse + * log_parameter_max_length = 0 + * log_parameter_max_length_on_error = 0 + * + * The guard is a single process-global: it is armed just before any logging + * could occur and disarmed when the message completes. Because an error while + * armed would otherwise longjmp past the disarm, restore_secret_logging() is + * also called from PostgresMain()'s error-recovery path; it is a no-op when the + * guard is not armed. + */ +typedef struct SecretLogGuard +{ + bool active; + int save_log_statement; + bool save_log_duration; + int save_log_min_duration_statement; + int save_log_min_messages; + int save_log_error_verbosity; + int save_log_parameter_max_length; + int save_log_parameter_max_length_on_error; +} SecretLogGuard; + +static SecretLogGuard secret_log_guard = {0}; + +static void +disarm_secret_logging(void) +{ + /* If somehow already armed, keep the originally-saved values. */ + if (secret_log_guard.active) + return; + + secret_log_guard.save_log_statement = log_statement; + secret_log_guard.save_log_duration = log_duration; + secret_log_guard.save_log_min_duration_statement = log_min_duration_statement; + secret_log_guard.save_log_min_messages = log_min_messages[MyBackendType]; + secret_log_guard.save_log_error_verbosity = Log_error_verbosity; + secret_log_guard.save_log_parameter_max_length = log_parameter_max_length; + secret_log_guard.save_log_parameter_max_length_on_error = + log_parameter_max_length_on_error; + secret_log_guard.active = true; + + log_statement = LOGSTMT_NONE; + log_duration = false; + log_min_duration_statement = -1; + log_min_messages[MyBackendType] = ERROR; + Log_error_verbosity = PGERROR_TERSE; + log_parameter_max_length = 0; + log_parameter_max_length_on_error = 0; +} + +static void +restore_secret_logging(void) +{ + if (!secret_log_guard.active) + return; + + log_statement = secret_log_guard.save_log_statement; + log_duration = secret_log_guard.save_log_duration; + log_min_duration_statement = secret_log_guard.save_log_min_duration_statement; + log_min_messages[MyBackendType] = secret_log_guard.save_log_min_messages; + Log_error_verbosity = secret_log_guard.save_log_error_verbosity; + log_parameter_max_length = secret_log_guard.save_log_parameter_max_length; + log_parameter_max_length_on_error = + secret_log_guard.save_log_parameter_max_length_on_error; + + secret_log_guard.active = false; +} + /* * check_log_statement * Determine whether command should be logged because of log_statement @@ -4611,6 +4743,15 @@ PostgresMain(const char *dbname, const char *username) /* Report the error to the client and/or server log */ EmitErrorReport(); + /* + * Restore any logging settings that were suppressed for a PASSWORD + * command, in case the error longjmp'd past the normal restore point. + * Done after EmitErrorReport() so the failing command's own error is + * still reported with the suppressed (secret-free) settings. No-op if + * the guard was not armed. + */ + restore_secret_logging(); + /* * If Valgrind noticed something during the erroneous query, print the * query string, assuming we have one. diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 4d33fcb5e9d..6d4099b3c51 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -208,6 +208,7 @@ ClassifyUtilityCommandAsReadOnly(Node *parsetree) case T_GrantStmt: case T_ImportForeignSchemaStmt: case T_IndexStmt: + case T_PasswordStmt: case T_ReassignOwnedStmt: case T_RefreshMatViewStmt: case T_RenameStmt: @@ -859,6 +860,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, ExecuteCallStmt(castNode(CallStmt, parsetree), params, isAtomicContext, dest); break; + case T_PasswordStmt: + ExecPasswordStmt(pstate, castNode(PasswordStmt, parsetree), params); + break; + case T_VacuumStmt: ExecVacuum(pstate, (VacuumStmt *) parsetree, isTopLevel); break; @@ -2900,6 +2905,10 @@ CreateCommandTag(Node *parsetree) tag = CMDTAG_CALL; break; + case T_PasswordStmt: + tag = CMDTAG_PASSWORD; + break; + case T_VacuumStmt: if (((VacuumStmt *) parsetree)->is_vacuumcmd) tag = CMDTAG_VACUUM; @@ -3563,6 +3572,10 @@ GetCommandLogLevel(Node *parsetree) lev = LOGSTMT_ALL; break; + case T_PasswordStmt: + lev = LOGSTMT_DDL; + break; + case T_RepackStmt: lev = LOGSTMT_DDL; break; diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 97dcb93791b..5ffd999aff4 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -13,6 +13,7 @@ #include "catalog/objectaddress.h" #include "libpq/crypt.h" +#include "nodes/params.h" #include "nodes/parsenodes.h" #include "parser/parse_node.h" #include "utils/guc.h" @@ -28,6 +29,8 @@ extern PGDLLIMPORT check_password_hook_type check_password_hook; extern Oid CreateRole(ParseState *pstate, CreateRoleStmt *stmt); extern Oid AlterRole(ParseState *pstate, AlterRoleStmt *stmt); +extern void ExecPasswordStmt(ParseState *pstate, PasswordStmt *stmt, + ParamListInfo params); extern Oid AlterRoleSet(AlterRoleSetStmt *stmt); extern void DropRole(DropRoleStmt *stmt); extern void GrantRole(ParseState *pstate, GrantRoleStmt *stmt); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 8a9df884276..6724d01274d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3793,6 +3793,28 @@ typedef struct CallStmt List *outargs; } CallStmt; +/* ---------------------- + * PASSWORD Statement + * + * Sets a role's password. Unlike ALTER ROLE ... PASSWORD, this command is + * parse-analyzed like an optimizable statement so that its two arguments can + * be supplied as bind parameters through the extended query protocol. Both + * "user" and "password" must be external ($n) parameters; the command is + * therefore usable only via the extended protocol, never via a simple query. + * Keeping the secret out of the SQL text (it travels only in a Bind message) + * lets the server scrub it from the logs. + * ---------------------- + */ +typedef struct PasswordStmt +{ + NodeTag type; + /* target role name expression (a text-typed $n parameter) */ + Node *user pg_node_attr(query_jumble_ignore); + /* new password expression (a text-typed $n parameter) */ + Node *password pg_node_attr(query_jumble_ignore); + ParseLoc location; /* token location, or -1 if unknown */ +} PasswordStmt; + typedef struct CallContext { pg_node_attr(nodetag_only) /* this is not a member of parse trees */ diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h index befae5f6b4f..8a54c20cd66 100644 --- a/src/include/tcop/cmdtaglist.h +++ b/src/include/tcop/cmdtaglist.h @@ -193,6 +193,7 @@ PG_CMDTAG(CMDTAG_LOGIN, "LOGIN", true, false, false) PG_CMDTAG(CMDTAG_MERGE, "MERGE", false, false, true) PG_CMDTAG(CMDTAG_MOVE, "MOVE", false, false, true) PG_CMDTAG(CMDTAG_NOTIFY, "NOTIFY", false, false, false) +PG_CMDTAG(CMDTAG_PASSWORD, "PASSWORD", false, false, false) PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false) PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false) PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false) diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index e3bddf91203..7f1ddadfabe 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -1517,12 +1517,19 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user, * on the implementation specific details with respect to how the * server changes passwords. * + * The change is applied with the server's PASSWORD command, sending the + * target user name and the (already encrypted) password as bind parameters + * through the extended query protocol. This keeps both values out of the + * statement text; combined with the server suppressing parameter logging for + * this command, the password never reaches the server log, pg_stat_activity, + * etc. + * * Arguments are a connection object, the SQL name of the target user, * and the cleartext password. * - * Return value is the PGresult of the executed ALTER USER statement - * or NULL if we never get there. The caller is responsible to PQclear() - * the returned PGresult. + * Return value is the PGresult of the executed PASSWORD command or NULL if we + * never get there. The caller is responsible to PQclear() the returned + * PGresult. * * PQresultStatus() should be called to check the return value for errors, * and PQerrorMessage() used to get more information about such errors. @@ -1532,55 +1539,31 @@ PQchangePassword(PGconn *conn, const char *user, const char *passwd) { char *encrypted_password = PQencryptPasswordConn(conn, passwd, user, NULL); + PGresult *res; + const char *paramValues[2]; if (!encrypted_password) { /* PQencryptPasswordConn() already registered the error */ return NULL; } - else - { - char *fmtpw = PQescapeLiteral(conn, encrypted_password, - strlen(encrypted_password)); - - /* no longer needed, so clean up now */ - PQfreemem(encrypted_password); - - if (!fmtpw) - { - /* PQescapeLiteral() already registered the error */ - return NULL; - } - else - { - char *fmtuser = PQescapeIdentifier(conn, user, strlen(user)); - if (!fmtuser) - { - /* PQescapeIdentifier() already registered the error */ - PQfreemem(fmtpw); - return NULL; - } - else - { - PQExpBufferData buf; - PGresult *res; - - initPQExpBuffer(&buf); - printfPQExpBuffer(&buf, "ALTER USER %s PASSWORD %s", - fmtuser, fmtpw); + /* + * Pass the user name and the encrypted password as parameters $1 and $2 + * of the PASSWORD command. PQexecParams uses the extended query protocol, + * so the values travel only in the Bind message and are never part of the + * query text. + */ + paramValues[0] = user; + paramValues[1] = encrypted_password; - res = PQexec(conn, buf.data); + res = PQexecParams(conn, "PASSWORD $1, $2", 2, + NULL, paramValues, NULL, NULL, 0); - /* clean up */ - termPQExpBuffer(&buf); - PQfreemem(fmtuser); - PQfreemem(fmtpw); + /* no longer needed, so clean up now */ + PQfreemem(encrypted_password); - return res; - } - } - } + return res; } PQauthDataHook_type PQauthDataHook = PQdefaultAuthDataHook; diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build index ee290698b31..5d11594a516 100644 --- a/src/test/modules/test_misc/meson.build +++ b/src/test/modules/test_misc/meson.build @@ -23,6 +23,7 @@ tests += { 't/012_ddlutils.pl', 't/013_temp_obj_multisession.pl', 't/014_log_statement_max_length.pl', + 't/015_password_command.pl', ], # The injection points are cluster-wide, so disable installcheck 'runningcheck': false, diff --git a/src/test/modules/test_misc/t/015_password_command.pl b/src/test/modules/test_misc/t/015_password_command.pl new file mode 100644 index 00000000000..f38f81face3 --- /dev/null +++ b/src/test/modules/test_misc/t/015_password_command.pl @@ -0,0 +1,121 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Tests for the PASSWORD command (PASSWORD $1, $2). +# +# The command sets a role's password using bind parameters supplied through the +# extended query protocol. The point of the command is that the secret travels +# only in a Bind message and is scrubbed from the server log, so these tests +# turn logging up to the maximum and assert that the password never appears in +# the log, in addition to checking the command's behaviour and privilege model. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init; + +# Log everything we can, so that any leak of the password would be captured. +$node->append_conf( + 'postgresql.conf', qq{ +log_statement = 'all' +log_min_duration_statement = 0 +log_duration = on +log_parameter_max_length = -1 +log_parameter_max_length_on_error = -1 +log_min_messages = 'debug1' +log_error_verbosity = 'verbose' +# Allow the command to accept a clear-text password so we can prove the secret +# is kept out of the log even in that case. +cleartext_passwords_action = 'warn' +}); +$node->start; + +$node->safe_psql('postgres', + 'CREATE ROLE regress_pw_alice LOGIN; ' + . 'CREATE ROLE regress_pw_bob LOGIN;'); + +# A distinctive secret we can grep for in the log. +my $secret = 'p4ssw0rd_SECRET_TOKEN_do_not_log'; + +# Helper: does the current pg_authid entry hold a SCRAM verifier? +sub password_is_scram +{ + my ($role) = @_; + return $node->safe_psql('postgres', + "SELECT rolpassword LIKE 'SCRAM-SHA-256\$%' FROM pg_authid WHERE rolname = '$role';" + ); +} + +# +# 1. Extended query protocol: superuser sets alice's password. +# +note "extended protocol sets the password without logging it"; +my $log_offset = -s $node->logfile; +my ($ret, $out, $err) = $node->psql('postgres', + "PASSWORD \$1, \$2 \\bind regress_pw_alice $secret \\g"); +is($ret, 0, "PASSWORD via extended protocol succeeds"); +is(password_is_scram('regress_pw_alice'), + 't', "alice's password was stored as a SCRAM verifier"); + +# The heart of the feature: the secret must not be in the log, even though we +# are logging statements, durations, and parameters at maximum verbosity. +ok(!$node->log_contains(qr/\Q$secret\E/, $log_offset), + "password does not appear in the server log"); + +# The command itself should still be visible in the log (just without the +# secret), confirming logging is otherwise active. +ok($node->log_contains(qr/PASSWORD \$1, \$2/, $log_offset), + "the PASSWORD statement text is still logged"); + +# The clear-text warning must be suppressed from the log for this command. +ok( !$node->log_contains(qr/using a clear text password/, $log_offset), + "clear-text warning is kept out of the log for PASSWORD"); + +# +# 2. Simple query protocol is rejected, and no secret leaks in that path. +# +note "simple query protocol is rejected"; +$log_offset = -s $node->logfile; +($ret, $out, $err) = $node->psql('postgres', "PASSWORD \$1, \$2;"); +isnt($ret, 0, "PASSWORD via simple protocol fails"); +like( + $err, + qr/PASSWORD can only be executed through the extended query protocol/, + "simple protocol gives the expected error"); + +# A literal password in a simple query is also rejected -- and must not be +# written to the log, even in the error path. +$log_offset = -s $node->logfile; +($ret, $out, $err) = + $node->psql('postgres', "PASSWORD 'regress_pw_alice', '$secret';"); +isnt($ret, 0, "PASSWORD with literal arguments fails"); +ok( !$node->log_contains(qr/\Q$secret\E/, $log_offset), + "literal password in a rejected simple query is not logged"); + +# +# 3. Privilege model (inherited from ALTER ROLE via delegation). +# +note "privilege model"; +# An unprivileged role may change its own password. +$log_offset = -s $node->logfile; +($ret, $out, $err) = $node->psql( + 'postgres', + "PASSWORD \$1, \$2 \\bind regress_pw_alice ${secret}_self \\g", + extra_params => [ '-U', 'regress_pw_alice' ]); +is($ret, 0, "unprivileged role can change its own password"); +ok(!$node->log_contains(qr/\Q$secret\E/, $log_offset), + "self-service password change is not logged"); + +# But not another role's password. +($ret, $out, $err) = $node->psql( + 'postgres', + "PASSWORD \$1, \$2 \\bind regress_pw_bob ${secret}_evil \\g", + extra_params => [ '-U', 'regress_pw_alice' ]); +isnt($ret, 0, "unprivileged role cannot change another role's password"); +like($err, qr/permission denied to alter role/, + "changing another role's password is denied"); + +done_testing(); -- 2.43.0