From 46d416c95943ff79762c551443e6de1f64f771e0 Mon Sep 17 00:00:00 2001 From: Melanie Plageman 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 Reviewed-by: Bertrand Drouvot Reviewed-by: Fujii Masao Reviewed-by: Daniel Gustafsson 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 - 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 aspects of each connection attempt to the server to be + logged. The default is the empty string, '', + which disables all connection logging. The following options may + be specified alone or in a comma-separated list: + + + + Log Connection Options + + + + + + Name + Description + + + + + receipt + Logs receipt of a connection. + + + + authentication + + 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. + + + + + authorization + + 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. + + + + + all + + A convenience alias equivalent to specifying all options. If + all is specified in a list of other + options, all connection aspects will be logged. + + + + + +
+ + + For the purposes of backwards compatibility, on, + off, true, + false, yes, + no, 1, and 0 + are still supported. The positive values are equivalent to specifying + the receipt, authentication, and + authorization options. + + + 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..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