From 994a86e6ac3abb647d93bdaf0f42be76f42b83a8 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Tue, 8 Nov 2022 18:56:26 +0100 Subject: [PATCH] Introduce 'log_connection_messages' This patch removes 'log_connections' and 'log_disconnections' in favor of 'log_connection_messages', thereby introducing incompatibility Author: Sergey Dudoladov Reviewed-by: Justin Pryzby Discussion: https://www.postgresql.org/message-id/flat/CAA8Fd-qCB96uwfgMKrzfNs32mqqysi53yZFNVaRNJ6xDthZEgA%40mail.gmail.com --- doc/src/sgml/config.sgml | 89 +++++++++++++------ src/backend/commands/variable.c | 77 ++++++++++++++++ src/backend/libpq/auth.c | 5 +- src/backend/postmaster/postmaster.c | 5 +- src/backend/tcop/postgres.c | 11 ++- src/backend/utils/init/postinit.c | 2 +- src/backend/utils/misc/guc_tables.c | 30 +++---- src/backend/utils/misc/postgresql.conf.sample | 5 +- src/include/postgres.h | 9 ++ src/include/postmaster/postmaster.h | 1 - src/include/utils/guc_hooks.h | 2 + src/test/authentication/t/001_password.pl | 2 +- src/test/authentication/t/003_peer.pl | 2 +- src/test/kerberos/t/001_auth.pl | 2 +- src/test/ldap/t/001_auth.pl | 2 +- src/test/ldap/t/002_bindpasswd.pl | 2 +- src/test/recovery/t/013_crash_restart.pl | 2 +- src/test/recovery/t/022_crash_temp_files.pl | 2 +- src/test/recovery/t/032_relfilenode_reuse.pl | 2 +- src/test/ssl/t/SSL/Server.pm | 2 +- src/tools/ci/pg_ci_base.conf | 3 +- 21 files changed, 182 insertions(+), 75 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 1cf53c74ea..ccefe144c3 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -140,7 +140,7 @@ An example of what this file might look like is: # This is a comment -log_connections = yes +log_connection_messages = all log_destination = 'syslog' search_path = '"$user", public' shared_buffers = 128MB @@ -335,7 +335,7 @@ UPDATE pg_settings SET setting = reset_val WHERE name = 'configuration_parameter passed to the postgres command via the command-line parameter. For example, -postgres -c log_connections=yes -c log_destination='syslog' +postgres -c log_connection_messages=all -c log_destination='syslog' Settings provided in this way override those set via postgresql.conf or ALTER SYSTEM, @@ -7085,23 +7085,74 @@ local0.* /var/log/postgresql - - log_connections (boolean) + + log_connection_messages (string) - log_connections configuration parameter + log_connection_messages 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. Example: authorized,disconnected. + The default is the empty string, which means that nothing is logged. 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. + + Connection messages + + + + + + Name + Description + + + + + received + Logs receipt of a connection. At this point a connection has been + received, but no further work has been done: PostgreSQL is about to start + interacting with a client. + + + + authenticated + 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. + + + + + authorized + Logs successful completion of authorization. At this point the + connection has been fully established. + + + + + disconnected + Logs session termination. The log output provides information + similar to authorized, plus the duration of the session. + + + + + all + A convenience alias. If present, it must be the only value in the + list. + + + + +
+ Some client programs, like psql, attempt @@ -7113,26 +7164,6 @@ local0.* /var/log/postgresql - - log_disconnections (boolean) - - log_disconnections configuration parameter - - - - - Causes session terminations to be logged. The log output - provides information similar to log_connections, - plus the duration of the session. - 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. - - - - - log_duration (boolean) diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index bb0f5de4c2..b76fd6f27c 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -993,6 +993,74 @@ show_role(void) } +/* check_hook: validate new log_connection_messages value + * + * The implementation follows the check_log_destination hook. + */ +bool +check_log_connection_messages(char **newval, void **extra, GucSource source) +{ + char *rawname; + List *namelist; + ListCell *l; + int newlogconnect = 0; + int *myextra; + + if (pg_strcasecmp(*newval, "all") == 0) + { + newlogconnect |= LOG_CONNECTION_ALL; + myextra = (int *) guc_malloc(ERROR, sizeof(int)); + *myextra = newlogconnect; + *extra = (void *) myextra; + return true; + } + + /* Need a modifiable copy of string */ + rawname = pstrdup(*newval); + + /* Parse string into list of identifiers */ + /* We rely on SplitIdentifierString to downcase and strip whitespace */ + if (!SplitIdentifierString(rawname, ',', &namelist)) + { + /* syntax error in name list */ + GUC_check_errdetail("List syntax is invalid."); + pfree(rawname); + list_free(namelist); + return false; + } + + foreach(l, namelist) + { + char *stage = (char *) lfirst(l); + if (pg_strcasecmp(stage, "received") == 0) + newlogconnect |= LOG_CONNECTION_RECEIVED; + else if (pg_strcasecmp(stage, "authenticated") == 0) + newlogconnect |= LOG_CONNECTION_AUTHENTICATED; + else if (pg_strcasecmp(stage, "authorized") == 0) + newlogconnect |= LOG_CONNECTION_AUTHORIZED; + else if (pg_strcasecmp(stage, "disconnected") == 0) + newlogconnect |= LOG_CONNECTION_DISCONNECTED; + else { + GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE); + GUC_check_errmsg("invalid value '%s'", stage); + GUC_check_errdetail("Valid values to use in the list are 'all', 'received', 'authenticated', 'authorized', and 'disconnected'." + " If 'all' is present, it must be the only value in the list."); + pfree(rawname); + list_free(namelist); + return false; + } + } + + pfree(rawname); + list_free(namelist); + + myextra = (int *) guc_malloc(ERROR, sizeof(int)); + *myextra = newlogconnect; + *extra = (void *) myextra; + + return true; +} + /* * PATH VARIABLES * @@ -1015,6 +1083,15 @@ check_canonical_path(char **newval, void **extra, GucSource source) /* + * assign_log_connection_messages: GUC assign_hook for log_connection_messages + */ +void +assign_log_connection_messages(const char *newval, void *extra) +{ + Log_connection_messages = *((int *) extra); +} + + /* * MISCELLANEOUS */ diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 25b3a781cd..e0f8034fb3 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -327,7 +327,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_connection_messages contains the 'authenticated' value. * * Auth methods should call this routine exactly once, as soon as the user is * successfully authenticated, even if they have reasons to know that @@ -359,7 +360,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_connection_messages & LOG_CONNECTION_AUTHENTICATED) { ereport(LOG, errmsg("connection authenticated: identity=\"%s\" method=%s " diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index f92dbc2270..0710168ae3 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -235,7 +235,6 @@ int PreAuthDelay = 0; int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ -bool Log_connections = false; bool Db_user_namespace = false; bool enable_bonjour = false; @@ -4343,8 +4342,8 @@ BackendInitialize(Port *port) port->remote_host = strdup(remote_host); port->remote_port = strdup(remote_port); - /* And now we can issue the Log_connections message, if wanted */ - if (Log_connections) + /* And now we can issue the "connection received" message, if wanted */ + if (Log_connection_messages & LOG_CONNECTION_RECEIVED) { if (remote_port[0]) ereport(LOG, diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 470b734e9e..0c95fbc3bd 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -83,8 +83,8 @@ const char *debug_query_string; /* client-supplied query string */ /* Note: whereToSendOutput is initialized for the bootstrap/standalone case */ CommandDest whereToSendOutput = DestDebug; -/* flag for logging end of session */ -bool Log_disconnections = false; +/* flags for logging information about session state */ +int Log_connection_messages = 0; int log_statement = LOGSTMT_NONE; @@ -3596,8 +3596,7 @@ set_debug_options(int debug_flag, GucContext context, GucSource source) if (debug_flag >= 1 && context == PGC_POSTMASTER) { - SetConfigOption("log_connections", "true", context, source); - SetConfigOption("log_disconnections", "true", context, source); + SetConfigOption("log_connection_messages", "all", context, source); } if (debug_flag >= 2) SetConfigOption("log_statement", "all", context, source); @@ -4167,9 +4166,9 @@ PostgresMain(const char *dbname, const char *username) /* * Also set up handler to log session end; we have to wait till now to be - * sure Log_disconnections has its final value. + * sure Log_connection_messages has its final value. */ - if (IsUnderPostmaster && Log_disconnections) + if (IsUnderPostmaster && (Log_connection_messages & LOG_CONNECTION_DISCONNECTED)) on_proc_exit(log_disconnections, 0); pgstat_report_connect(MyDatabaseId); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 2f07ca7a0e..a780c19349 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -250,7 +250,7 @@ PerformAuthentication(Port *port) */ disable_timeout(STATEMENT_TIMEOUT, false); - if (Log_connections) + if (Log_connection_messages & LOG_CONNECTION_AUTHORIZED) { StringInfoData logmsg; diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index c5a95f5dcc..7abe27e602 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -87,7 +87,6 @@ #endif /* XXX these should appear in other modules' header files */ -extern bool Log_disconnections; extern int CommitDelay; extern int CommitSiblings; extern char *default_tablespace; @@ -585,6 +584,7 @@ static char *recovery_target_string; static char *recovery_target_xid_string; static char *recovery_target_name_string; static char *recovery_target_lsn_string; +static char *log_connection_messages_string; /* should be static, but commands/variable.c needs to get at this */ char *role_string; @@ -1182,24 +1182,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 - }, - { - {"log_disconnections", PGC_SU_BACKEND, LOGGING_WHAT, - gettext_noop("Logs end of a session, including duration."), - NULL - }, - &Log_disconnections, - false, - NULL, NULL, NULL - }, { {"log_replication_commands", PGC_SUSET, LOGGING_WHAT, gettext_noop("Logs each replication command."), @@ -4163,6 +4145,16 @@ struct config_string ConfigureNamesString[] = check_session_authorization, assign_session_authorization, NULL }, + { + {"log_connection_messages", PGC_SU_BACKEND, LOGGING_WHAT, + gettext_noop("Lists connection stages to log."), + NULL, + GUC_LIST_INPUT + }, + &log_connection_messages_string, + "", + check_log_connection_messages, assign_log_connection_messages, NULL + }, { {"log_destination", PGC_SIGHUP, LOGGING_WHERE, gettext_noop("Sets the destination for server log output."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index d06074b86f..23c6705cf2 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_connection_messages=all". Some parameters can be changed at run time # with the "SET" SQL command. # # Memory units: B = bytes Time units: us = microseconds @@ -553,8 +553,7 @@ # actions running at least this number # of milliseconds. #log_checkpoints = on -#log_connections = off -#log_disconnections = off +#log_connection_messages = '' #log_duration = off #log_error_verbosity = default # terse, default, or verbose messages #log_hostname = off diff --git a/src/include/postgres.h b/src/include/postgres.h index 8a028ff789..9f888f4aef 100644 --- a/src/include/postgres.h +++ b/src/include/postgres.h @@ -576,4 +576,13 @@ extern Datum Float8GetDatum(float8 X); #define NON_EXEC_STATIC static #endif +extern PGDLLIMPORT int Log_connection_messages; + +/* Bitmap for logging connection messages */ +#define LOG_CONNECTION_RECEIVED (1 << 0) +#define LOG_CONNECTION_AUTHENTICATED (1 << 1) +#define LOG_CONNECTION_AUTHORIZED (1 << 2) +#define LOG_CONNECTION_DISCONNECTED (1 << 3) +#define LOG_CONNECTION_ALL 0xFFFF + #endif /* POSTGRES_H */ diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index 3b3889c58c..e1ad7e1c50 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -25,7 +25,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/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index aeb3663071..9690c13ab6 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -67,6 +67,8 @@ extern bool check_locale_numeric(char **newval, void **extra, GucSource source); extern void assign_locale_numeric(const char *newval, void *extra); extern bool check_locale_time(char **newval, void **extra, GucSource source); extern void assign_locale_time(const char *newval, void *extra); +extern bool check_log_connection_messages(char **newval, void **extra, GucSource source); +extern void assign_log_connection_messages(const char *newval, void *extra); extern bool check_log_destination(char **newval, void **extra, GucSource source); extern void assign_log_destination(const char *newval, void *extra); diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index a2fde1408b..30581ea5d7 100644 --- a/src/test/authentication/t/001_password.pl +++ b/src/test/authentication/t/001_password.pl @@ -63,7 +63,7 @@ sub test_conn # Initialize primary node my $node = PostgreSQL::Test::Cluster->new('primary'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; # Create 3 roles with different password methods for each one. The same diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl index a6be651ea7..4ad97da2f4 100644 --- a/src/test/authentication/t/003_peer.pl +++ b/src/test/authentication/t/003_peer.pl @@ -82,7 +82,7 @@ sub find_in_log my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; # Set pg_hba.conf with the peer authentication. diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index d610ce63ab..8b1b87e554 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -180,7 +180,7 @@ $node->append_conf( 'postgresql.conf', qq{ listen_addresses = '$hostaddr' krb_server_keyfile = '$keytab' -log_connections = on +log_connection_messages = all lc_messages = 'C' }); $node->start; diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index f3ed806ec2..6e6dbb830a 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -48,7 +48,7 @@ note "setting up PostgreSQL instance"; my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; $node->safe_psql('postgres', 'CREATE USER test0;'); diff --git a/src/test/ldap/t/002_bindpasswd.pl b/src/test/ldap/t/002_bindpasswd.pl index bcd4aa2b74..e274dbf928 100644 --- a/src/test/ldap/t/002_bindpasswd.pl +++ b/src/test/ldap/t/002_bindpasswd.pl @@ -44,7 +44,7 @@ note "setting up PostgreSQL instance"; my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connection_messages = all\n"); $node->start; $node->safe_psql('postgres', 'CREATE USER test0;'); diff --git a/src/test/recovery/t/013_crash_restart.pl b/src/test/recovery/t/013_crash_restart.pl index 92e7b367df..ff0bb3daf0 100644 --- a/src/test/recovery/t/013_crash_restart.pl +++ b/src/test/recovery/t/013_crash_restart.pl @@ -27,7 +27,7 @@ $node->start(); $node->safe_psql( 'postgres', q[ALTER SYSTEM SET restart_after_crash = 1; - ALTER SYSTEM SET log_connections = 1; + ALTER SYSTEM SET log_connection_messages = 'all'; SELECT pg_reload_conf();]); # Run psql, keeping session alive, so we have an alive backend to kill. diff --git a/src/test/recovery/t/022_crash_temp_files.pl b/src/test/recovery/t/022_crash_temp_files.pl index 03c8efdfb5..2a466da25b 100644 --- a/src/test/recovery/t/022_crash_temp_files.pl +++ b/src/test/recovery/t/022_crash_temp_files.pl @@ -26,7 +26,7 @@ $node->start(); $node->safe_psql( 'postgres', q[ALTER SYSTEM SET remove_temp_files_after_crash = on; - ALTER SYSTEM SET log_connections = 1; + ALTER SYSTEM SET log_connection_messages = 'all'; ALTER SYSTEM SET work_mem = '64kB'; ALTER SYSTEM SET restart_after_crash = on; SELECT pg_reload_conf();]); diff --git a/src/test/recovery/t/032_relfilenode_reuse.pl b/src/test/recovery/t/032_relfilenode_reuse.pl index 92ec510037..f4ac72ef62 100644 --- a/src/test/recovery/t/032_relfilenode_reuse.pl +++ b/src/test/recovery/t/032_relfilenode_reuse.pl @@ -11,7 +11,7 @@ $node_primary->init(allows_streaming => 1); $node_primary->append_conf( 'postgresql.conf', q[ allow_in_place_tablespaces = true -log_connections=on +log_connection_messages=all # to avoid "repairing" corruption full_page_writes=off log_min_messages=debug2 diff --git a/src/test/ssl/t/SSL/Server.pm b/src/test/ssl/t/SSL/Server.pm index b6344b936a..6cd5a46921 100644 --- a/src/test/ssl/t/SSL/Server.pm +++ b/src/test/ssl/t/SSL/Server.pm @@ -193,7 +193,7 @@ sub configure_test_server_for_ssl # enable logging etc. open my $conf, '>>', "$pgdata/postgresql.conf"; print $conf "fsync=off\n"; - print $conf "log_connections=on\n"; + print $conf "log_connection_messages=all\n"; print $conf "log_hostname=on\n"; print $conf "listen_addresses='$serverhost'\n"; print $conf "log_statement=all\n"; diff --git a/src/tools/ci/pg_ci_base.conf b/src/tools/ci/pg_ci_base.conf index d8faa9c26c..7596d515aa 100644 --- a/src/tools/ci/pg_ci_base.conf +++ b/src/tools/ci/pg_ci_base.conf @@ -8,7 +8,6 @@ max_prepared_transactions = 10 # Settings that make logs more useful log_autovacuum_min_duration = 0 log_checkpoints = true -log_connections = true -log_disconnections = true +log_connection_messages = all log_line_prefix = '%m [%p][%b] %q[%a][%v:%x] ' log_lock_waits = true -- 2.34.1