Fix md5_password_warnings for role and database settings

MD5 authentication warnings are queued during authentication, before
startup options and role/database settings have been applied. The code
checked md5_password_warnings at queue time, so settings such as
ALTER ROLE ... SET md5_password_warnings = off did not suppress the
warning, even though the established session showed the GUC as off.

Keep the connection-warning infrastructure generic by allowing each
queued warning to carry an optional filter callback. Evaluate that
callback when warnings are emitted, after startup options and
role/database settings have been processed.

Use this for MD5 authentication warnings, while leaving password
expiration warnings unchanged. Add test coverage for an MD5-authenticated
role with md5_password_warnings disabled.

Author: Chao Li <lic@highgo.com>
Reviewed-by: Japin Li <japinli@hotmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/AE46E42D-5966-4D76-9E64-95EAB01B9FB5@gmail.com

src/backend/libpq/crypt.c

@@ -32,6 +32,8 @@ int			password_expiration_warning_threshold = 604800;
 /* Enables deprecation warnings for MD5 passwords. */
 bool		md5_password_warnings = true;
 
+static bool md5_password_warning_enabled(void);
+
 /*
  * Fetch stored password for a user, for authentication.
  *
@@ -137,7 +139,7 @@ get_role_password(const char *role, const char **logdetail)
 				detail = psprintf(_("The password for role \"%s\" will expire in less than 1 minute."),
 								  role);
 
-			StoreConnectionWarning(warning, detail);
+			StoreConnectionWarning(warning, detail, NULL);
 
 			MemoryContextSwitchTo(oldcontext);
 		}
@@ -296,22 +298,19 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
 	if (strlen(client_pass) == strlen(crypt_pwd) &&
 		timingsafe_bcmp(client_pass, crypt_pwd, strlen(crypt_pwd)) == 0)
 	{
+		MemoryContext oldcontext;
+		char	   *warning;
+		char	   *detail;
+
 		retval = STATUS_OK;
 
-		if (md5_password_warnings)
-		{
-			MemoryContext oldcontext;
-			char	   *warning;
-			char	   *detail;
+		oldcontext = MemoryContextSwitchTo(TopMemoryContext);
 
-			oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+		warning = pstrdup(_("authenticated with an MD5-encrypted password"));
+		detail = pstrdup(_("MD5 password support is deprecated and will be removed in a future release of PostgreSQL."));
+		StoreConnectionWarning(warning, detail, md5_password_warning_enabled);
 
-			warning = pstrdup(_("authenticated with an MD5-encrypted password"));
-			detail = pstrdup(_("MD5 password support is deprecated and will be removed in a future release of PostgreSQL."));
-			StoreConnectionWarning(warning, detail);
-
-			MemoryContextSwitchTo(oldcontext);
-		}
+		MemoryContextSwitchTo(oldcontext);
 	}
 	else
 	{
@@ -323,6 +322,12 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
 	return retval;
 }
 
+static bool
+md5_password_warning_enabled(void)
+{
+	return md5_password_warnings;
+}
+
 /*
  * Check given password for given user, and return STATUS_OK or STATUS_ERROR.
  *

src/backend/utils/init/postinit.c

@@ -74,9 +74,15 @@
 /* has this backend called EmitConnectionWarnings()? */
 static bool ConnectionWarningsEmitted;
 
-/* content of warnings to send via EmitConnectionWarnings() */
-static List *ConnectionWarningMessages;
-static List *ConnectionWarningDetails;
+typedef struct ConnectionWarning
+{
+	char	   *message;
+	char	   *detail;
+	ConnectionWarningFilter filter;
+} ConnectionWarning;
+
+/* warnings to send via EmitConnectionWarnings() */
+static List *ConnectionWarnings;
 
 static HeapTuple GetDatabaseTuple(const char *dbname);
 static HeapTuple GetDatabaseTupleByOid(Oid dboid);
@@ -1499,15 +1505,19 @@ ThereIsAtLeastOneRole(void)
 
 /*
  * Stores a warning message to be sent later via EmitConnectionWarnings().
- * Both msg and detail must be non-NULL.
+ * Both msg and detail must be non-NULL.  If filter is non-NULL, it is called
+ * just before the warning is emitted, after startup and role/database settings
+ * have been applied.
  *
- * NB: Caller should ensure the strings are allocated in a long-lived context
- * like TopMemoryContext.
+ * NB: Caller should ensure the strings are palloc'd in a long-lived context
+ * like TopMemoryContext.  This function takes ownership of the strings, which
+ * will be pfree'd in EmitConnectionWarnings().
  */
 void
-StoreConnectionWarning(char *msg, char *detail)
+StoreConnectionWarning(char *msg, char *detail, ConnectionWarningFilter filter)
 {
 	MemoryContext oldcontext;
+	ConnectionWarning *warning;
 
 	Assert(msg);
 	Assert(detail);
@@ -1517,8 +1527,11 @@ StoreConnectionWarning(char *msg, char *detail)
 
 	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
 
-	ConnectionWarningMessages = lappend(ConnectionWarningMessages, msg);
-	ConnectionWarningDetails = lappend(ConnectionWarningDetails, detail);
+	warning = palloc_object(ConnectionWarning);
+	warning->message = msg;
+	warning->detail = detail;
+	warning->filter = filter;
+	ConnectionWarnings = lappend(ConnectionWarnings, warning);
 
 	MemoryContextSwitchTo(oldcontext);
 }
@@ -1532,22 +1545,23 @@ StoreConnectionWarning(char *msg, char *detail)
 static void
 EmitConnectionWarnings(void)
 {
-	ListCell   *lc_msg;
-	ListCell   *lc_detail;
-
 	if (ConnectionWarningsEmitted)
 		elog(ERROR, "EmitConnectionWarnings() called more than once");
 	else
 		ConnectionWarningsEmitted = true;
 
-	forboth(lc_msg, ConnectionWarningMessages,
-			lc_detail, ConnectionWarningDetails)
+	foreach_ptr(ConnectionWarning, warning, ConnectionWarnings)
 	{
-		ereport(WARNING,
-				(errmsg("%s", (char *) lfirst(lc_msg)),
-				 errdetail("%s", (char *) lfirst(lc_detail))));
+		if (warning->filter == NULL || warning->filter())
+			ereport(WARNING,
+					(errmsg("%s", warning->message),
+					 errdetail("%s", warning->detail)));
+
+		pfree(warning->message);
+		pfree(warning->detail);
+		pfree(warning);
 	}
 
-	list_free_deep(ConnectionWarningMessages);
-	list_free_deep(ConnectionWarningDetails);
+	list_free(ConnectionWarnings);
+	ConnectionWarnings = NIL;
 }

src/include/miscadmin.h

@@ -516,7 +516,9 @@ extern void InitPostgres(const char *in_dbname, Oid dboid,
 						 uint32 flags,
 						 char *out_dbname);
 extern void BaseInit(void);
-extern void StoreConnectionWarning(char *msg, char *detail);
+typedef bool (*ConnectionWarningFilter) (void);
+extern void StoreConnectionWarning(char *msg, char *detail,
+								   ConnectionWarningFilter filter);
 
 /* in utils/init/miscinit.c */
 extern PGDLLIMPORT bool IgnoreSystemIndexes;

src/test/authentication/t/001_password.pl

@@ -157,6 +157,14 @@ is( $node->psql(
 	),
 	$md5_works ? 0 : 3,
 	'created user with md5 password');
+is( $node->psql(
+		'postgres',
+		"SET password_encryption='md5';
+		 CREATE ROLE md5_role_no_warnings LOGIN PASSWORD 'pass';
+		 ALTER ROLE md5_role_no_warnings SET md5_password_warnings = off;"
+	),
+	$md5_works ? 0 : 3,
+	'created user with md5 password and MD5 warnings disabled');
 # Set up a table for tests of SYSTEM_USER.
 $node->safe_psql(
 	'postgres',
@@ -504,6 +512,15 @@ SKIP:
 		expected_stderr => qr/authenticated with an MD5-encrypted password/,
 		log_like =>
 		  [qr/connection authenticated: identity="md5_role" method=md5/]);
+
+	$node->connect_ok(
+		'user=md5_role_no_warnings',
+		'md5 with warnings disabled',
+		sql => 'SHOW md5_password_warnings',
+		expected_stdout => qr/^off$/,
+		log_like => [
+			qr/connection authenticated: identity="md5_role_no_warnings" method=md5/
+		]);
 }
 
 # require_auth succeeds with SCRAM required.

src/tools/pgindent/typedefs.list

@@ -524,6 +524,7 @@ ConnStatusType
 ConnType
 ConnectionStateEnum
 ConnectionTiming
+ConnectionWarning
 ConsiderSplitContext
 Const
 ConstrCheck