From 2fb563dc2f95ff50bf492340cd1e1d10aa45eb94 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 5 Mar 2025 18:02:45 -0500 Subject: [PATCH v11 1/3] Modularize log_connections output Convert the boolean log_connections GUC into a list GUC of the connection stages to log. The current log_connections options are 'received', 'authenticated', and 'authorized'. The empty string disables all connection logging. 'all' enables all available connection logging. This gives users more control over the volume of connection logging. For backwards compatibility, the most common values for the log_connections boolean are still supported ('on', 'off', 1, 0, true, false). This patch has much of the same behavior as [1] but was developed independently. [1] https://postgr.es/m/flat/CAA8Fd-qCB96uwfgMKrzfNs32mqqysi53yZFNVaRNJ6xDthZEgA%40mail.gmail.com Author: Melanie Plageman Reviewed-by: Bertrand Drouvot Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/flat/CAAKRu_b_smAHK0ZjrnL5GRxnAVWujEXQWpLXYzGbmpcZd3nLYw%40mail.gmail.com --- doc/src/sgml/config.sgml | 71 +++++++- src/backend/libpq/auth.c | 9 +- src/backend/postmaster/postmaster.c | 1 - src/backend/tcop/backend_startup.c | 152 +++++++++++++++++- src/backend/utils/init/postinit.c | 3 +- src/backend/utils/misc/guc_tables.c | 21 +-- src/backend/utils/misc/postgresql.conf.sample | 4 +- src/include/postmaster/postmaster.h | 1 - src/include/tcop/backend_startup.h | 29 ++++ src/include/utils/guc_hooks.h | 2 + .../postmaster/t/002_connection_limits.pl | 50 +++++- src/tools/pgindent/typedefs.list | 1 + 12 files changed, 319 insertions(+), 25 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index d2fa5f7d1a9..b97707a7ec2 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7315,20 +7315,81 @@ local0.* /var/log/postgresql - log_connections (boolean) + log_connections (string) log_connections configuration parameter - Causes each attempted connection to the server to be logged, - as well as successful completion of both client authentication (if - necessary) and authorization. + Causes the specified stages of each connection attempt to the server to + be logged. The default is the empty string, '', + which disables all connection logging. The following are the options + that may be specified: + + + + Log Connection Stages + + + + + + Name + Description + + + + + received + Logs receipt of a connection. + + + + authenticated + + Logs the original identity used by an authentication method to + identify a user. Failed authentication is always logged regardless + of the value of this setting. + + + + + authorized + + Logs successful completion of authorization. At this point the + connection has been fully established. + + + + + all + + A convenience alias. If specified, all must be + quoted because it is a PostgreSQL + keyword. If all is specified in a list of other + options, all connection logging will be enabled. + + + + + +
+ + + For the purposes of backwards compatibility, on, + off, true, + false, yes, + no, 1, and 0 + are still supported. The positive values emit logs for the + received, authenticated, and + authorized stages. + + + Only superusers and users with the appropriate SET privilege can change this parameter at session start, and it cannot be changed at all within a session. - The default is off. diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 81e2f8184e3..066bc5254ee 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 + * 'authenticated' 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_AUTHENTICATED) { 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_AUTHENTICATED) && + 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..84db47231f6 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; +int 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 int validate_log_connections_options(List *elemlist); /* * 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_RECEIVED) { if (remote_port[0]) ereport(LOG, @@ -924,3 +929,146 @@ StartupPacketTimeoutHandler(void) { _exit(1); } + +/* + * Helper for the log_connections GUC check hook. + * + * `elemlist` is the listified string input passed to check_log_connections(). + * check_log_connections() is responsible for cleaning up `elemlist`. + * + * validate_log_connections_options() returns the flags that should be stored + * in the log_connections GUC by the assign_hook or -1 if the input is not + * valid and we should error out. + */ +static int +validate_log_connections_options(List *elemlist) +{ + int flags = 0; + ListCell *l; + char *item; + + /* For backwards compatibility, we accept these tokens by themselves */ + static const struct config_enum_entry compat_options[] = { + {"off", LOG_CONNECTION_NONE}, + {"false", LOG_CONNECTION_NONE}, + {"no", LOG_CONNECTION_NONE}, + {"0", LOG_CONNECTION_NONE}, + {"on", LOG_CONNECTION_ON}, + {"true", LOG_CONNECTION_ON}, + {"yes", LOG_CONNECTION_ON}, + {"1", LOG_CONNECTION_ON}, + }; + + /* If an empty string was passed, we're done */ + if (list_length(elemlist) == 0) + return 0; + + /* + * 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 -1; + } + + return option.val; + } + + /* + * Now check the stage-specific options. The empty string was already + * handled. + */ + foreach(l, elemlist) + { + static const struct config_enum_entry options[] = { + {"received", LOG_CONNECTION_RECEIVED}, + {"authenticated", LOG_CONNECTION_AUTHENTICATED}, + {"authorized", LOG_CONNECTION_AUTHORIZED}, + {"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 -1; + +next: ; + } + + return flags; +} + + +/* + * GUC check hook for log_connections + */ +bool +check_log_connections(char **newval, void **extra, GucSource source) +{ + int flags; + char *rawstring; + List *elemlist; + + /* 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 */ + flags = validate_log_connections_options(elemlist); + + /* Time for cleanup */ + pfree(rawstring); + list_free(elemlist); + + /* We didn't succeed */ + if (flags == -1) + 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..dc6484bb092 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_AUTHORIZED) { StringInfoData logmsg; diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index ad25cbb39c5..71070465379 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 information about events during 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..aca610f4fad 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,7 +578,7 @@ # actions running at least this number # of milliseconds. #log_checkpoints = on -#log_connections = off +#log_connections = '' #log_disconnections = off #log_duration = off #log_error_verbosity = default # terse, default, or verbose messages 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..66d370c1e00 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 int 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. + * + * RECEIVED, AUTHENTICATED, and AUTHORIZED are different stages of connection + * establishment and setup where we may emit a log message. + * + * ALL is a convenience alias for when all stages should be logged. + * + * NONE is an alias for when no connection logging should be done. + * + * ON is backwards compatibility alias for the connection events that were + * logged in Postgres versions < 18. + */ +typedef enum LogConnectionOption +{ + LOG_CONNECTION_NONE = 0, + LOG_CONNECTION_RECEIVED = (1 << 0), + LOG_CONNECTION_AUTHENTICATED = (1 << 1), + LOG_CONNECTION_AUTHORIZED = (1 << 2), + LOG_CONNECTION_ON = + LOG_CONNECTION_RECEIVED | + LOG_CONNECTION_AUTHENTICATED | + LOG_CONNECTION_AUTHORIZED, + LOG_CONNECTION_ALL = + LOG_CONNECTION_ON, +} 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/postmaster/t/002_connection_limits.pl b/src/test/postmaster/t/002_connection_limits.pl index 8cfa6e0ced5..f19d6e73ac9 100644 --- a/src/test/postmaster/t/002_connection_limits.pl +++ b/src/test/postmaster/t/002_connection_limits.pl @@ -19,9 +19,57 @@ $node->init( $node->append_conf('postgresql.conf', "max_connections = 6"); $node->append_conf('postgresql.conf', "reserved_connections = 2"); $node->append_conf('postgresql.conf', "superuser_reserved_connections = 1"); -$node->append_conf('postgresql.conf', "log_connections = on"); $node->start; +# Before testing connection limits, test the behavior of log_connections + +$node->connect_ok("", + "no connection logging emitted when log_connections is the default (off)", + log_unlike => [ + qr/connection received/, + qr/connection authenticated/, + qr/connection authorized/, + ],); + +$node->safe_psql( + 'postgres', + q[ALTER SYSTEM SET log_connections = received,authorized; + SELECT pg_reload_conf();]); + +$node->connect_ok("", + "log_connections with subset of specified stages logs only those stages", + log_like => [ + qr/connection received/, + qr/connection authorized/, + ], + log_unlike => [ + qr/connection authenticated/, + ],); + +$node->safe_psql( + 'postgres', + q[ALTER SYSTEM SET log_connections = on; SELECT pg_reload_conf();]); + +$node->connect_ok("", + "log_connections 'on' works as expected for backwards compatibility", + log_like => [ + qr/connection received/, + qr/connection authenticated/, + qr/connection authorized/, + ],); + +$node->safe_psql( + 'postgres', + q[ALTER SYSTEM SET log_connections = 'all'; SELECT pg_reload_conf();]); + +$node->connect_ok("", + "all log connections messages printed when 'all' specified", + log_like => [ + qr/connection received/, + qr/connection authenticated/, + qr/connection authorized/, + ],); + $node->safe_psql( 'postgres', qq{ CREATE USER regress_regular LOGIN; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 9840060997f..29e52b64994 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1555,6 +1555,7 @@ LockTupleMode LockViewRecurse_context LockWaitPolicy LockingClause +LogConnectionOption LogOpts LogStmtLevel LogicalDecodeBeginCB -- 2.34.1