On Tue, Jun 03, 2025 at 09:43:59AM -0500, Nathan Bossart wrote: > On Tue, Jun 03, 2025 at 10:34:06AM -0400, Tom Lane wrote: >> If we really want to be in peoples' face about this, the thing >> to do is to print a warning every time they log in with an MD5 >> password. Also, to Michael's point, that really would be exactly >> the same place where the eventual "sorry, not supported anymore" >> message will be. > > I held off on this because I was worried it might be far too noisy. That > does seem like it has the best chance of getting folks' attention, though. > If it's too noisy, users can always turn off the warnings.
Here is a draft-grade patch that adds a WARNING upon successful authentication with an MD5 password. It's a little hacky because AFAICT we need to wait until well after authentication (for GUCs to be set up, etc.) before we actually emit the WARNING. When the time comes to remove MD5 password support completely, we'll need to do something like modify CheckMD5Auth() to always return STATUS_ERROR with an appropriate logdetail message. What do folks think about doing this? -- nathan
>From ccf33e5f93dd120a0975e7e0039c524ebcca2a3f Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Mon, 2 Jun 2025 10:10:03 -0500 Subject: [PATCH v2 1/2] pg_upgrade: Warn about roles with MD5 passwords. --- src/bin/pg_upgrade/check.c | 66 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 940fc77fc2e..9da2b5977e1 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -31,6 +31,7 @@ static void check_new_cluster_logical_replication_slots(void); static void check_new_cluster_subscription_configuration(void); static void check_old_cluster_for_valid_slots(void); static void check_old_cluster_subscription_state(void); +static void check_for_md5_passwords(ClusterInfo *cluster); /* * DataTypesUsageChecks - definitions of data type checks for the old cluster @@ -685,6 +686,12 @@ check_and_dump_old_cluster(void) if (GET_MAJOR_VERSION(old_cluster.major_version) <= 905) check_for_pg_role_prefix(&old_cluster); + /* + * MD5 password support is deprecated. Warn if any roles have MD5 + * passwords. + */ + check_for_md5_passwords(&old_cluster); + /* * While not a check option, we do this now because this is the only time * the old server is running. @@ -2272,3 +2279,62 @@ check_old_cluster_subscription_state(void) else check_ok(); } + +/* + * check_for_md5_passwords() + * + * As of v18, MD5 password support is marked as deprecated and to-be-removed in + * a future major release. + */ +static void +check_for_md5_passwords(ClusterInfo *cluster) +{ + PGresult *res; + PGconn *conn = connectToServer(cluster, "template1"); + int ntups; + int i_roloid; + int i_rolname; + FILE *script = NULL; + char output_path[MAXPGPATH]; + + prep_status("Checking for roles with MD5 passwords"); + + snprintf(output_path, sizeof(output_path), "%s/%s", + log_opts.basedir, + "roles_with_md5_passwords.txt"); + + res = executeQueryOrDie(conn, + "SELECT oid AS roloid, rolname " + "FROM pg_catalog.pg_authid " + "WHERE rolpassword ~ '^md5'"); + + ntups = PQntuples(res); + i_roloid = PQfnumber(res, "roloid"); + i_rolname = PQfnumber(res, "rolname"); + for (int rowno = 0; rowno < ntups; rowno++) + { + if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", output_path); + fprintf(script, "%s (oid=%s)\n", + PQgetvalue(res, rowno, i_rolname), + PQgetvalue(res, rowno, i_roloid)); + } + + PQclear(res); + + PQfinish(conn); + + if (script) + { + fclose(script); + report_status(PG_WARNING, "warning"); + pg_log(PG_WARNING, + "Your installation contains roles with MD5 passwords.\n" + "Support for MD5-encrypted passwords is deprecated and will be\n" + "removed in a future release of PostgreSQL. A list of roles\n" + "with MD5 passwords is in the file:\n" + " %s", output_path); + } + else + check_ok(); +} -- 2.39.5 (Apple Git-154)
>From d381840ddc1f80621955c252ede49ad0e0a38608 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Tue, 3 Jun 2025 11:52:03 -0500 Subject: [PATCH v2 2/2] [WIP] add warning upon authentication with MD5 password --- src/backend/libpq/crypt.c | 17 +++++++++++++++++ src/backend/utils/init/postinit.c | 3 +++ src/include/libpq/crypt.h | 1 + src/test/authentication/t/001_password.pl | 1 + 4 files changed, 22 insertions(+) diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c index f6b641e726e..90050dcfc12 100644 --- a/src/backend/libpq/crypt.c +++ b/src/backend/libpq/crypt.c @@ -27,6 +27,8 @@ /* Enables deprecation warnings for MD5 passwords. */ bool md5_password_warnings = true; +static bool used_md5_auth = false; + /* * Fetch stored password for a user, for authentication. * @@ -210,6 +212,8 @@ md5_crypt_verify(const char *role, const char *shadow_pass, Assert(md5_salt_len > 0); + used_md5_auth = true; + if (get_password_type(shadow_pass) != PASSWORD_TYPE_MD5) { /* incompatible password hash format. */ @@ -242,6 +246,19 @@ md5_crypt_verify(const char *role, const char *shadow_pass, return retval; } +void +warn_if_md5_auth(void) +{ + if (md5_password_warnings && used_md5_auth) + ereport(WARNING, + (errcode(ERRCODE_WARNING_DEPRECATED_FEATURE), + errmsg("authenticated with an MD5-encrypted password"), + errdetail("MD5 password support is deprecated and will be removed in a future release of PostgreSQL."), + errhint("Refer to the PostgreSQL documentation for details about migrating to another password type."))); + + used_md5_auth = false; +} + /* * Check given password for given user, and return STATUS_OK or STATUS_ERROR. * diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index c86ceefda94..b77c5967468 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -34,6 +34,7 @@ #include "catalog/pg_db_role_setting.h" #include "catalog/pg_tablespace.h" #include "libpq/auth.h" +#include "libpq/crypt.h" #include "libpq/libpq-be.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -1234,6 +1235,8 @@ InitPostgres(const char *in_dbname, Oid dboid, /* close the transaction we started above */ if (!bootstrap) CommitTransactionCommand(); + + warn_if_md5_auth(); } /* diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h index a1b4b363143..ca42210aaa3 100644 --- a/src/include/libpq/crypt.h +++ b/src/include/libpq/crypt.h @@ -53,6 +53,7 @@ extern char *get_role_password(const char *role, const char **logdetail); extern int md5_crypt_verify(const char *role, const char *shadow_pass, const char *client_pass, const uint8 *md5_salt, int md5_salt_len, const char **logdetail); +extern void warn_if_md5_auth(void); extern int plain_crypt_verify(const char *role, const char *shadow_pass, const char *client_pass, const char **logdetail); diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 37d96d95a1a..3d94b323c09 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -68,6 +68,7 @@ $node->init; $node->append_conf('postgresql.conf', "log_connections = on\n"); # Needed to allow connect_fails to inspect postmaster log: $node->append_conf('postgresql.conf', "log_min_messages = debug2"); +$node->append_conf('postgresql.conf', "md5_password_warnings = off"); $node->start; # Test behavior of log_connections GUC -- 2.39.5 (Apple Git-154)