v14-0001-Modularize-log_connections-output.patch

From 46d416c95943ff79762c551443e6de1f64f771e0 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed, 5 Mar 2025 18:02:45 -0500
Subject: [PATCH v14 1/2] Modularize log_connections output

Convert the boolean log_connections GUC into a list GUC comprised of the
connection aspects to log.

This gives users more control over the volume and kind of connection
logging.

The current log_connections options are 'receipt', 'authentication', and
'authorization'. The empty string disables all connection logging. 'all'
enables all available connection logging.

For backwards compatibility, the most common values for the
log_connections boolean are still supported (on, off, 1, 0, true,
false, yes, no).

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/flat/CAAKRu_b_smAHK0ZjrnL5GRxnAVWujEXQWpLXYzGbmpcZd3nLYw%40mail.gmail.com
---
 doc/src/sgml/config.sgml                      |  77 ++++++++-
 src/backend/libpq/auth.c                      |   9 +-
 src/backend/postmaster/postmaster.c           |   1 -
 src/backend/tcop/backend_startup.c            | 161 +++++++++++++++++-
 src/backend/utils/init/postinit.c             |   3 +-
 src/backend/utils/misc/guc_tables.c           |  21 ++-
 src/backend/utils/misc/postgresql.conf.sample |   8 +-
 src/include/postmaster/postmaster.h           |   1 -
 src/include/tcop/backend_startup.h            |  29 ++++
 src/include/utils/guc_hooks.h                 |   2 +
 src/test/authentication/t/001_password.pl     |  38 +++++
 src/tools/pgindent/typedefs.list              |   1 +
 12 files changed, 326 insertions(+), 25 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d2fa5f7d1a9..d939d99d6e8 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7315,20 +7315,87 @@ local0.*    /var/log/postgresql
      </varlistentry>
 
      <varlistentry id="guc-log-connections" xreflabel="log_connections">
-      <term><varname>log_connections</varname> (<type>boolean</type>)
+      <term><varname>log_connections</varname> (<type>string</type>)
       <indexterm>
        <primary><varname>log_connections</varname> configuration parameter</primary>
       </indexterm>
       </term>
       <listitem>
        <para>
-        Causes each attempted connection to the server to be logged,
-        as well as successful completion of both client authentication (if
-        necessary) and authorization.
+        Causes aspects of each connection attempt to the server to be
+        logged. The default is the empty string, <literal>''</literal>,
+        which disables all connection logging. The following options may
+        be specified alone or in a comma-separated list:
+       </para>
+
+       <table id="log-connections-options">
+        <title>Log Connection Options</title>
+        <tgroup cols="2">
+         <colspec colname="col1" colwidth="1*"/>
+         <colspec colname="col2" colwidth="2*"/>
+         <thead>
+          <row>
+           <entry>Name</entry>
+           <entry>Description</entry>
+          </row>
+         </thead>
+         <tbody>
+          <row>
+           <entry><literal>receipt</literal></entry>
+           <entry>Logs receipt of a connection.</entry>
+          </row>
+
+          <row>
+           <entry><literal>authentication</literal></entry>
+           <entry>
+            Logs the original identity used by an authentication method
+            to identify a user. In most cases, the identity string
+            matches the PostgreSQL username, but some third-party
+            authentication methods may alter the original user
+            identifier before the server stores it. Failed
+            authentication is always logged regardless of the value of
+            this setting.
+           </entry>
+          </row>
+
+          <row>
+           <entry><literal>authorization</literal></entry>
+           <entry>
+            Logs successful completion of authorization. At this point
+            the connection has been established but the backend is not
+            yet fully set up. The log message includes the authorized
+            username as well as the database name and application name,
+            if applicable.
+           </entry>
+          </row>
+
+          <row>
+           <entry><literal>all</literal></entry>
+           <entry>
+            A convenience alias equivalent to specifying all options. If
+            <literal>all</literal> is specified in a list of other
+            options, all connection aspects will be logged.
+           </entry>
+          </row>
+
+         </tbody>
+        </tgroup>
+       </table>
+
+       <para>
+        For the purposes of backwards compatibility, <literal>on</literal>,
+        <literal>off</literal>, <literal>true</literal>,
+        <literal>false</literal>, <literal>yes</literal>,
+        <literal>no</literal>, <literal>1</literal>, and <literal>0</literal>
+        are still supported. The positive values are equivalent to specifying
+        the <literal>receipt</literal>, <literal>authentication</literal>, and
+        <literal>authorization</literal> options.
+       </para>
+
+       <para>
         Only superusers and users with the appropriate <literal>SET</literal>
         privilege can change this parameter at session start,
         and it cannot be changed at all within a session.
-        The default is <literal>off</literal>.
        </para>
 
        <note>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 81e2f8184e3..e18683c47e7 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -38,6 +38,7 @@
 #include "postmaster/postmaster.h"
 #include "replication/walsender.h"
 #include "storage/ipc.h"
+#include "tcop/backend_startup.h"
 #include "utils/memutils.h"
 
 /*----------------------------------------------------------------
@@ -317,7 +318,8 @@ auth_failed(Port *port, int status, const char *logdetail)
 /*
  * Sets the authenticated identity for the current user.  The provided string
  * will be stored into MyClientConnectionInfo, alongside the current HBA
- * method in use.  The ID will be logged if log_connections is enabled.
+ * method in use.  The ID will be logged if log_connections has the
+ * 'authentication' option specified.
  *
  * Auth methods should call this routine exactly once, as soon as the user is
  * successfully authenticated, even if they have reasons to know that
@@ -349,7 +351,7 @@ set_authn_id(Port *port, const char *id)
 	MyClientConnectionInfo.authn_id = MemoryContextStrdup(TopMemoryContext, id);
 	MyClientConnectionInfo.auth_method = port->hba->auth_method;
 
-	if (Log_connections)
+	if (log_connections & LOG_CONNECTION_AUTHENTICATION)
 	{
 		ereport(LOG,
 				errmsg("connection authenticated: identity=\"%s\" method=%s "
@@ -633,7 +635,8 @@ ClientAuthentication(Port *port)
 #endif
 	}
 
-	if (Log_connections && status == STATUS_OK &&
+	if ((log_connections & LOG_CONNECTION_AUTHENTICATION) &&
+		status == STATUS_OK &&
 		!MyClientConnectionInfo.authn_id)
 	{
 		/*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index d2a7a7add6f..b4f34c57a1b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -237,7 +237,6 @@ int			PreAuthDelay = 0;
 int			AuthenticationTimeout = 60;
 
 bool		log_hostname;		/* for ps display and logging */
-bool		Log_connections = false;
 
 bool		enable_bonjour = false;
 char	   *bonjour_name;
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index c70746fa562..962b6e60002 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -34,13 +34,17 @@
 #include "tcop/backend_startup.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
+#include "utils/guc_hooks.h"
 #include "utils/injection_point.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
+#include "utils/varlena.h"
 
 /* GUCs */
 bool		Trace_connection_negotiation = false;
+uint32		log_connections = 0;
+char	   *log_connections_string = NULL;
 
 static void BackendInitialize(ClientSocket *client_sock, CAC_state cac);
 static int	ProcessSSLStartup(Port *port);
@@ -48,6 +52,7 @@ static int	ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
 static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
 static void process_startup_packet_die(SIGNAL_ARGS);
 static void StartupPacketTimeoutHandler(void);
+static bool validate_log_connections_options(List *elemlist, uint32 *flags);
 
 /*
  * Entry point for a new backend process.
@@ -201,8 +206,8 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
 	port->remote_host = MemoryContextStrdup(TopMemoryContext, remote_host);
 	port->remote_port = MemoryContextStrdup(TopMemoryContext, remote_port);
 
-	/* And now we can issue the Log_connections message, if wanted */
-	if (Log_connections)
+	/* And now we can log that the connection was received, if enabled */
+	if (log_connections & LOG_CONNECTION_RECEIPT)
 	{
 		if (remote_port[0])
 			ereport(LOG,
@@ -924,3 +929,155 @@ StartupPacketTimeoutHandler(void)
 {
 	_exit(1);
 }
+
+/*
+ * Helper for the log_connections GUC check hook.
+ *
+ * `elemlist` is a listified version of the string input passed to the
+ * log_connections GUC check hook, check_log_connections().
+ * check_log_connections() is responsible for cleaning up `elemlist`.
+ *
+ * validate_log_connections_options() returns false if an error was
+ * encountered and the GUC input could not be validated and true otherwise.
+ *
+ * `flags` returns the flags that should be stored in the log_connections GUC
+ * by its assign hook.
+ */
+static bool
+validate_log_connections_options(List *elemlist, uint32 *flags)
+{
+	ListCell   *l;
+	char	   *item;
+
+	/*
+	 * For backwards compatibility, we accept these tokens by themselves.
+	 *
+	 * Prior to PostgreSQL 18, log_connections was a boolean GUC that accepted
+	 * any unambiguous substring of 'true', 'false', 'yes', 'no', 'on', and
+	 * 'off'. Since log_connections became a list of strings in 18, we only
+	 * accept complete option strings.
+	 */
+	static const struct config_enum_entry compat_options[] = {
+		{"off", 0},
+		{"false", 0},
+		{"no", 0},
+		{"0", 0},
+		{"on", LOG_CONNECTION_ON},
+		{"true", LOG_CONNECTION_ON},
+		{"yes", LOG_CONNECTION_ON},
+		{"1", LOG_CONNECTION_ON},
+	};
+
+	*flags = 0;
+
+	/* If an empty string was passed, we're done */
+	if (list_length(elemlist) == 0)
+		return true;
+
+	/*
+	 * Now check for the backwards compatibility options. They must always be
+	 * specified on their own, so we error out if the first option is a
+	 * backwards compatibility option and other options are also specified.
+	 */
+	item = linitial(elemlist);
+
+	for (size_t i = 0; i < lengthof(compat_options); i++)
+	{
+		struct config_enum_entry option = compat_options[i];
+
+		if (pg_strcasecmp(item, option.name) != 0)
+			continue;
+
+		if (list_length(elemlist) > 1)
+		{
+			GUC_check_errdetail("Cannot specify log_connections option \"%s\" in a list with other options.",
+								item);
+			return false;
+		}
+
+		*flags = option.val;
+		return true;
+	}
+
+	/* Now check the aspect options. The empty string was already handled */
+	foreach(l, elemlist)
+	{
+		static const struct config_enum_entry options[] = {
+			{"receipt", LOG_CONNECTION_RECEIPT},
+			{"authentication", LOG_CONNECTION_AUTHENTICATION},
+			{"authorization", LOG_CONNECTION_AUTHORIZATION},
+			{"all", LOG_CONNECTION_ALL},
+		};
+
+		item = lfirst(l);
+		for (size_t i = 0; i < lengthof(options); i++)
+		{
+			struct config_enum_entry option = options[i];
+
+			if (pg_strcasecmp(item, option.name) == 0)
+			{
+				*flags |= option.val;
+				goto next;
+			}
+		}
+
+		GUC_check_errdetail("Invalid option \"%s\".", item);
+		return false;
+
+next:	;
+	}
+
+	return true;
+}
+
+
+/*
+ * GUC check hook for log_connections
+ */
+bool
+check_log_connections(char **newval, void **extra, GucSource source)
+{
+	uint32		flags;
+	char	   *rawstring;
+	List	   *elemlist;
+	bool		success;
+
+	/* Need a modifiable copy of string */
+	rawstring = pstrdup(*newval);
+
+	if (!SplitIdentifierString(rawstring, ',', &elemlist))
+	{
+		GUC_check_errdetail("Invalid list syntax in parameter \"log_connections\".");
+		pfree(rawstring);
+		list_free(elemlist);
+		return false;
+	}
+
+	/* Validation logic is all in the helper */
+	success = validate_log_connections_options(elemlist, &flags);
+
+	/* Time for cleanup */
+	pfree(rawstring);
+	list_free(elemlist);
+
+	if (!success)
+		return false;
+
+	/*
+	 * We succeeded, so allocate `extra` and save the flags there for use by
+	 * assign_log_connections().
+	 */
+	*extra = guc_malloc(ERROR, sizeof(int));
+	*((int *) *extra) = flags;
+
+	return true;
+}
+
+/*
+ * GUC assign hook for log_connections
+ */
+void
+assign_log_connections(const char *newval, void *extra)
+{
+	log_connections = *((int *) extra);
+}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index ee1a9d5d98b..6c207e17768 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -54,6 +54,7 @@
 #include "storage/sinvaladt.h"
 #include "storage/smgr.h"
 #include "storage/sync.h"
+#include "tcop/backend_startup.h"
 #include "tcop/tcopprot.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
@@ -252,7 +253,7 @@ PerformAuthentication(Port *port)
 	 */
 	disable_timeout(STATEMENT_TIMEOUT, false);
 
-	if (Log_connections)
+	if (log_connections & LOG_CONNECTION_AUTHORIZATION)
 	{
 		StringInfoData logmsg;
 
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index ad25cbb39c5..d95e6bb9e4b 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1219,15 +1219,6 @@ struct config_bool ConfigureNamesBool[] =
 		true,
 		NULL, NULL, NULL
 	},
-	{
-		{"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
-			gettext_noop("Logs each successful connection."),
-			NULL
-		},
-		&Log_connections,
-		false,
-		NULL, NULL, NULL
-	},
 	{
 		{"trace_connection_negotiation", PGC_POSTMASTER, DEVELOPER_OPTIONS,
 			gettext_noop("Logs details of pre-authentication connection handshake."),
@@ -4886,6 +4877,18 @@ struct config_string ConfigureNamesString[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"log_connections", PGC_SU_BACKEND, LOGGING_WHAT,
+			gettext_noop("Logs specified aspects of connection establishment."),
+			NULL,
+			GUC_LIST_INPUT
+		},
+		&log_connections_string,
+		"",
+		check_log_connections, assign_log_connections, NULL
+	},
+
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 2d1de9c37bd..c291c05d181 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -21,7 +21,7 @@
 # require a server shutdown and restart to take effect.
 #
 # Any parameter can also be given as a command-line option to the server, e.g.,
-# "postgres -c log_connections=on".  Some parameters can be changed at run time
+# "postgres -c log_connections=all".  Some parameters can be changed at run time
 # with the "SET" SQL command.
 #
 # Memory units:  B  = bytes            Time units:  us  = microseconds
@@ -578,9 +578,11 @@
 					# actions running at least this number
 					# of milliseconds.
 #log_checkpoints = on
-#log_connections = off
+#log_connections = '' # log aspects of connection setup
+          # options include receipt, authentication, authorization,
+          # and all to log all of these aspects
 #log_disconnections = off
-#log_duration = off
+#log_duration = off # log statement duration
 #log_error_verbosity = default		# terse, default, or verbose messages
 #log_hostname = off
 #log_line_prefix = '%m [%p] '		# special values:
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index b6a3f275a1b..aa786bfacf3 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -63,7 +63,6 @@ extern PGDLLIMPORT char *ListenAddresses;
 extern PGDLLIMPORT bool ClientAuthInProgress;
 extern PGDLLIMPORT int PreAuthDelay;
 extern PGDLLIMPORT int AuthenticationTimeout;
-extern PGDLLIMPORT bool Log_connections;
 extern PGDLLIMPORT bool log_hostname;
 extern PGDLLIMPORT bool enable_bonjour;
 extern PGDLLIMPORT char *bonjour_name;
diff --git a/src/include/tcop/backend_startup.h b/src/include/tcop/backend_startup.h
index 73285611203..e00851a004e 100644
--- a/src/include/tcop/backend_startup.h
+++ b/src/include/tcop/backend_startup.h
@@ -16,6 +16,8 @@
 
 /* GUCs */
 extern PGDLLIMPORT bool Trace_connection_negotiation;
+extern PGDLLIMPORT uint32 log_connections;
+extern PGDLLIMPORT char *log_connections_string;
 
 /*
  * CAC_state is passed from postmaster to the backend process, to indicate
@@ -39,6 +41,33 @@ typedef struct BackendStartupData
 	CAC_state	canAcceptConnections;
 } BackendStartupData;
 
+/*
+ * Granular control over which messages to log for the log_connections GUC.
+ *
+ * RECEIPT, AUTHENTICATION, and AUTHORIZATION are different aspects of
+ * connection establishment and backend setup for which we may emit a log
+ * message.
+ *
+ * ALL is a convenience alias equivalent to all of the above aspects.
+ *
+ * ON is backwards compatibility alias for the connection aspects that were
+ * logged in Postgres versions < 18.
+ */
+typedef enum LogConnectionOption
+{
+	LOG_CONNECTION_RECEIPT = (1 << 0),
+	LOG_CONNECTION_AUTHENTICATION = (1 << 1),
+	LOG_CONNECTION_AUTHORIZATION = (1 << 2),
+	LOG_CONNECTION_ON =
+		LOG_CONNECTION_RECEIPT |
+		LOG_CONNECTION_AUTHENTICATION |
+		LOG_CONNECTION_AUTHORIZATION,
+	LOG_CONNECTION_ALL =
+		LOG_CONNECTION_RECEIPT |
+		LOG_CONNECTION_AUTHENTICATION |
+		LOG_CONNECTION_AUTHORIZATION,
+} LogConnectionOption;
+
 extern void BackendMain(const void *startup_data, size_t startup_data_len) pg_attribute_noreturn();
 
 #endif							/* BACKEND_STARTUP_H */
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 951451a9765..9a0d8ec85c7 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -51,6 +51,8 @@ extern bool check_datestyle(char **newval, void **extra, GucSource source);
 extern void assign_datestyle(const char *newval, void *extra);
 extern bool check_debug_io_direct(char **newval, void **extra, GucSource source);
 extern void assign_debug_io_direct(const char *newval, void *extra);
+extern bool check_log_connections(char **newval, void **extra, GucSource source);
+extern void assign_log_connections(const char *newval, void *extra);
 extern bool check_default_table_access_method(char **newval, void **extra,
 											  GucSource source);
 extern bool check_default_tablespace(char **newval, void **extra,
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 4ce22ccbdf2..e307dee5c48 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -7,6 +7,8 @@
 # - MD5-encrypted
 # - SCRAM-encrypted
 # This test can only run with Unix-domain sockets.
+#
+# There's also a few tests of the log_connections GUC here.
 
 use strict;
 use warnings FATAL => 'all';
@@ -66,6 +68,42 @@ $node->init;
 $node->append_conf('postgresql.conf', "log_connections = on\n");
 $node->start;
 
+# Test behavior of log_connections GUC
+#
+# There wasn't another test file where these tests obviously fit, and we don't
+# want to incur the cost of spinning up a new cluster just to test one GUC.
+
+# Make a database for the log_connections tests to avoid test fragility if
+# other tests are added to this file in the future
+$node->safe_psql('postgres', "CREATE DATABASE test_log_connections");
+
+$node->safe_psql('test_log_connections',
+	q[ALTER SYSTEM SET log_connections = receipt,authorization;
+				   SELECT pg_reload_conf();]);
+
+$node->connect_ok('test_log_connections',
+	q(log_connections with subset of specified options logs only those aspects),
+	log_like => [
+		qr/connection received/,
+		qr/connection authorized: user=\S+ database=test_log_connections/,
+	],
+	log_unlike => [
+		qr/connection authenticated/,
+	],);
+
+$node->safe_psql('test_log_connections',
+	qq(ALTER SYSTEM SET log_connections = 'all'; SELECT pg_reload_conf();));
+
+$node->connect_ok('test_log_connections',
+	qq(log_connections 'all' logs all available connection aspects),
+	log_like => [
+		qr/connection received/,
+		qr/connection authenticated/,
+		qr/connection authorized: user=\S+ database=test_log_connections/,
+	],);
+
+# Authentication tests
+
 # could fail in FIPS mode
 my $md5_works = ($node->psql('postgres', "select md5('')") == 0);
 
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index dfe2690bdd3..9e7f58adb18 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1556,6 +1556,7 @@ LockTupleMode
 LockViewRecurse_context
 LockWaitPolicy
 LockingClause
+LogConnectionOption
 LogOpts
 LogStmtLevel
 LogicalDecodeBeginCB
-- 
2.34.1