Re: Fix md5_password_warnings for role/database settings

Hi, Chao

On Wed, 10 Jun 2026 at 14:26, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> Hi,
>
> While testing “[bc60ee860] Warn upon successful MD5 password authentication”, I found a small issue.
>
> This feature emits a warning based on the existing GUC
> md5_password_warnings, but it queues the message in
> md5_crypt_verify(), before GUC values are loaded by
> process_startup_options() and process_settings(). As a result,
> settings loaded later during connection startup, such as startup
> options or ALTER ROLE/ALTER DATABASE settings, are not honored for
> this warning.
>
> Here is a repro:
>
> 1. Edit pg_hba.conf, add this line:
> ```
> local postgres md5_role md5
> ```
>
> 2. Setup in session 1:
> ```
> evantest=# set password_encryption='md5';
> SET
> evantest=# create role md5_role login password 'pass';
> WARNING: setting an MD5-encrypted password
> DETAIL: MD5 password support is deprecated and will be removed in a future release of PostgreSQL.
> HINT: Refer to the PostgreSQL documentation for details about migrating to another password type.
> CREATE ROLE
> evantest=#
> evantest=# alter role md5_role set md5_password_warnings =0;
> ALTER ROLE
> evantest=# select pg_reload_conf(); -- reload pg_hba.conf as I didn’t restart the server
> pg_reload_conf
> ----------------
> t
> (1 row)
> ```
>
> 3. Connect as md5_role:
> ```
> % PGPASSWORD=pass psql -d postgres -U md5_role -X -qAt -c “show md5_password_warnings"
> WARNING: authenticated with an MD5-encrypted password
> DETAIL: MD5 password support is deprecated and will be removed in a future release of PostgreSQL.
> off
> ```
>
> As we can see, although the role’s md5_password_warnings setting is off, the warning message is still shown.
>
> This feature uses the connection warning infrastructure introduced by 1d92e0c2cc, so fixing the problem requires enhancing that infrastructure.
>
> In the current implementation, there are two lists:
> ConnectionWarningMessages and ConnectionWarningDetails. The attached
> patch combines them into one list and adds a filter function to each
> list member, so the filter can be applied in
> EmitConnectionWarnings(). With this mechanism, the warning emitted
> upon successful MD5 authentication is checked against the final value
> of md5_password_warnings, while 1d92e0c2cc’s password expiration
> warning logic remains unchanged.
>

I'm in favor of this idea.

> See the attached patch for details.

A few comments:

1.
EmitConnectionWarnings(void)
{
- ListCell *lc_msg;
- ListCell *lc_detail;
+ ListCell *lc;

if (ConnectionWarningsEmitted)
elog(ERROR, "EmitConnectionWarnings() called more than once");
else
ConnectionWarningsEmitted = true;

- forboth(lc_msg, ConnectionWarningMessages,
- lc_detail, ConnectionWarningDetails)
+ foreach(lc, ConnectionWarnings)
{
- ereport(WARNING,
- (errmsg("%s", (char *) lfirst(lc_msg)),
- errdetail("%s", (char *) lfirst(lc_detail))));
+ ConnectionWarning *warning = lfirst(lc);
+

Perhaps we could use foreach_ptr(ConnectionWarning, warning, ConnectionWarnings)
to simplify the code.

2.
StoreConnectionWarning() states that the caller should ensure the strings are
allocated in a long-lived context. Since the two existing calls already use
TopMemoryContext, should the function always switch the memory context
internally?

>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.