From 6178535e0be8e25e4209f42d52c8402b6d1e4724 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Thu, 27 Feb 2025 17:32:17 -0500 Subject: [PATCH v7 1/2] Make log_connections a list Instead of a boolean, make log_connections a list of the possible log connections events to log. all and none mimic current behavior of on and off. This is in response to feedback that log_connections logs too many messages. --- doc/src/sgml/config.sgml | 25 ++++-- src/backend/libpq/auth.c | 5 +- src/backend/postmaster/postmaster.c | 76 ++++++++++++++++++- src/backend/tcop/backend_startup.c | 2 +- src/backend/utils/init/postinit.c | 2 +- src/backend/utils/misc/guc_tables.c | 20 ++--- src/backend/utils/misc/postgresql.conf.sample | 4 +- src/include/postmaster/postmaster.h | 10 ++- src/include/utils/guc_hooks.h | 2 + .../libpq/t/005_negotiate_encryption.pl | 2 +- src/test/authentication/t/001_password.pl | 2 +- src/test/authentication/t/003_peer.pl | 2 +- src/test/authentication/t/005_sspi.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 +- .../t/001_mutated_bindpasswd.pl | 2 +- .../modules/oauth_validator/t/001_server.pl | 2 +- .../modules/oauth_validator/t/002_client.pl | 2 +- .../postmaster/t/002_connection_limits.pl | 2 +- src/test/postmaster/t/003_start_stop.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/recovery/t/037_invalid_database.pl | 2 +- src/test/ssl/t/SSL/Server.pm | 2 +- src/tools/ci/pg_ci_base.conf | 2 +- src/tools/pgindent/typedefs.list | 1 + 28 files changed, 141 insertions(+), 42 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e55700f35b8..c927efd22d1 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_connections = 'all' log_destination = 'syslog' search_path = '"$user", public' shared_buffers = 128MB @@ -337,7 +337,7 @@ UPDATE pg_settings SET setting = reset_val WHERE name = 'configuration_parameter command-line parameter, or its equivalent variation. For example, -postgres -c log_connections=yes --log-destination='syslog' +postgres -c log_connections='all' --log-destination='syslog' Settings provided in this way override those set via postgresql.conf or ALTER SYSTEM, @@ -7315,20 +7315,31 @@ 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 each attempted connection to the server to be logged, as well as + successful completion of both client authentication (if necessary) and + authorization. + + + + May be set to none to disable this logging, + all to enable all connection log message types, or a + comma-separated list of the specific connection log message types to + emit. The valid options are received, + authenticated, and authorized. + + + 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. + The default is 'none'. diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 81e2f8184e3..b4cc8cb4dd1 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -349,7 +349,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 +633,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 5dd3b6a4fd4..aef63793aa7 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -118,6 +118,7 @@ #include "utils/pidfile.h" #include "utils/timestamp.h" #include "utils/varlena.h" +#include "utils/guc_hooks.h" #ifdef EXEC_BACKEND #include "common/file_utils.h" @@ -237,7 +238,8 @@ int PreAuthDelay = 0; int AuthenticationTimeout = 60; bool log_hostname; /* for ps display and logging */ -bool Log_connections = false; +char *log_connections_string = NULL; +int log_connections = 0; bool enable_bonjour = false; char *bonjour_name; @@ -1517,6 +1519,78 @@ checkControlFile(void) FreeFile(fp); } +bool +check_log_connections(char **newval, void **extra, GucSource source) +{ + int flags = 0; + char *rawstring = NULL; + List *elemlist; + ListCell *l; + + char *effval = *newval; + + /* + * log_connections can be "all" or "none" or a list of comma separated + * options + */ + if (pg_strcasecmp(*newval, "none") == 0) + effval = ""; + else if (pg_strcasecmp(*newval, "all") == 0) + effval = "received, authenticated, authorized"; + + /* Need a modifiable copy of string */ + rawstring = pstrdup(effval); + + if (!SplitGUCList(rawstring, ',', &elemlist)) + { + GUC_check_errdetail("Invalid list syntax in parameter \"%s\".", + "log_connections"); + goto error; + } + + foreach(l, elemlist) + { + char *item = (char *) lfirst(l); + + if (pg_strcasecmp(item, "received") == 0) + flags |= LOG_CONNECTION_RECEIVED; + else if (pg_strcasecmp(item, "authenticated") == 0) + flags |= LOG_CONNECTION_AUTHENTICATED; + else if (pg_strcasecmp(item, "authorized") == 0) + flags |= LOG_CONNECTION_AUTHORIZED; + else if (pg_strcasecmp(item, "none") == 0 || pg_strcasecmp(item, "all") == 0) + { + GUC_check_errdetail("Cannot specify \"%s\" in a list of other log_connections options.", item); + goto error; + } + else + { + GUC_check_errdetail("Invalid option \"%s\".", item); + goto error; + } + } + + pfree(rawstring); + list_free(elemlist); + + /* Save the flags in *extra, for use by assign_log_connections */ + *extra = guc_malloc(ERROR, sizeof(int)); + *((int *) *extra) = flags; + + return true; + +error: + pfree(rawstring); + list_free(elemlist); + return false; +} + +void +assign_log_connections(const char *newval, void *extra) +{ + log_connections = *((int *) extra); +} + /* * Determine how long should we let ServerLoop sleep, in milliseconds. * diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c index c70746fa562..2c208c25356 100644 --- a/src/backend/tcop/backend_startup.c +++ b/src/backend/tcop/backend_startup.c @@ -202,7 +202,7 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac) port->remote_port = MemoryContextStrdup(TopMemoryContext, remote_port); /* And now we can issue the Log_connections message, if wanted */ - if (Log_connections) + if (log_connections & LOG_CONNECTION_RECEIVED) { if (remote_port[0]) ereport(LOG, diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 318600d6d02..8811f2ba3e6 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -251,7 +251,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..7dfa11810a8 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."), @@ -4850,6 +4841,17 @@ struct config_string ConfigureNamesString[] = check_debug_io_direct, assign_debug_io_direct, NULL }, + { + {"log_connections", PGC_SU_BACKEND, LOGGING_WHAT, + gettext_noop("Logs information about successful connection."), + NULL, + GUC_LIST_INPUT + }, + &log_connections_string, + "none", + check_log_connections, assign_log_connections, NULL + }, + { {"synchronized_standby_slots", PGC_SIGHUP, REPLICATION_PRIMARY, gettext_noop("Lists streaming replication standby server replication slot " diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 5362ff80519..4f54719dd9e 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 = 'none' #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..adc693a6b2b 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -63,7 +63,8 @@ extern PGDLLIMPORT char *ListenAddresses; extern PGDLLIMPORT bool ClientAuthInProgress; extern PGDLLIMPORT int PreAuthDelay; extern PGDLLIMPORT int AuthenticationTimeout; -extern PGDLLIMPORT bool Log_connections; +extern PGDLLIMPORT int log_connections; +extern PGDLLIMPORT char *log_connections_string; extern PGDLLIMPORT bool log_hostname; extern PGDLLIMPORT bool enable_bonjour; extern PGDLLIMPORT char *bonjour_name; @@ -143,4 +144,11 @@ typedef enum DispatchOption extern DispatchOption parse_dispatch_option(const char *name); +typedef enum ConnectionLogType +{ + LOG_CONNECTION_RECEIVED, + LOG_CONNECTION_AUTHENTICATED, + LOG_CONNECTION_AUTHORIZED, +} ConnectionLogType; + #endif /* _POSTMASTER_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/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl index c834fa5149a..cb56e09d50b 100644 --- a/src/interfaces/libpq/t/005_negotiate_encryption.pl +++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl @@ -107,7 +107,7 @@ $node->append_conf( listen_addresses = '$hostaddr' # Capturing the EVENTS that occur during tests requires these settings -log_connections = on +log_connections = 'all' log_disconnections = on trace_connection_negotiation = on lc_messages = 'C' diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl index 4ce22ccbdf2..72c187fbdc0 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_connections = 'all'\n"); $node->start; # could fail in FIPS mode diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl index 69ba73bd2b9..8e1b0477d6b 100644 --- a/src/test/authentication/t/003_peer.pl +++ b/src/test/authentication/t/003_peer.pl @@ -71,7 +71,7 @@ sub test_role my $node = PostgreSQL::Test::Cluster->new('node'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connections = 'all'\n"); $node->start; # Set pg_hba.conf with the peer authentication. diff --git a/src/test/authentication/t/005_sspi.pl b/src/test/authentication/t/005_sspi.pl index b480b702590..f9b61babcd7 100644 --- a/src/test/authentication/t/005_sspi.pl +++ b/src/test/authentication/t/005_sspi.pl @@ -18,7 +18,7 @@ if (!$windows_os || $use_unix_sockets) # 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_connections = 'all'\n"); $node->start; my $huge_pages_status = diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index 6748b109dec..afc3130e720 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -65,7 +65,7 @@ $node->append_conf( 'postgresql.conf', qq{ listen_addresses = '$hostaddr' krb_server_keyfile = '$krb->{keytab}' -log_connections = on +log_connections = '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 352b0fc1fa7..b6ee386679d 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -47,7 +47,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_connections = '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 f8beba2b279..2acf96aeb6d 100644 --- a/src/test/ldap/t/002_bindpasswd.pl +++ b/src/test/ldap/t/002_bindpasswd.pl @@ -43,7 +43,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_connections = 'all'\n"); $node->start; $node->safe_psql('postgres', 'CREATE USER test0;'); diff --git a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl index 9b062e1c800..4591b5568a8 100644 --- a/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl +++ b/src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl @@ -42,7 +42,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_connections = 'all'\n"); $node->append_conf('postgresql.conf', "shared_preload_libraries = 'ldap_password_func'"); $node->start; diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index 6fa59fbeb25..27829680439 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -43,7 +43,7 @@ if ($ENV{with_python} ne 'yes') my $node = PostgreSQL::Test::Cluster->new('primary'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connections = 'all'\n"); $node->append_conf('postgresql.conf', "oauth_validator_libraries = 'validator'\n"); $node->start; diff --git a/src/test/modules/oauth_validator/t/002_client.pl b/src/test/modules/oauth_validator/t/002_client.pl index ab83258d736..aa944ba5f2f 100644 --- a/src/test/modules/oauth_validator/t/002_client.pl +++ b/src/test/modules/oauth_validator/t/002_client.pl @@ -26,7 +26,7 @@ if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\boauth\b/) my $node = PostgreSQL::Test::Cluster->new('primary'); $node->init; -$node->append_conf('postgresql.conf', "log_connections = on\n"); +$node->append_conf('postgresql.conf', "log_connections = 'all'\n"); $node->append_conf('postgresql.conf', "oauth_validator_libraries = 'validator'\n"); $node->start; diff --git a/src/test/postmaster/t/002_connection_limits.pl b/src/test/postmaster/t/002_connection_limits.pl index 8cfa6e0ced5..a221501c206 100644 --- a/src/test/postmaster/t/002_connection_limits.pl +++ b/src/test/postmaster/t/002_connection_limits.pl @@ -19,7 +19,7 @@ $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->append_conf('postgresql.conf', "log_connections = 'all'"); $node->start; $node->safe_psql( diff --git a/src/test/postmaster/t/003_start_stop.pl b/src/test/postmaster/t/003_start_stop.pl index 036b296f72b..08cd7a4857e 100644 --- a/src/test/postmaster/t/003_start_stop.pl +++ b/src/test/postmaster/t/003_start_stop.pl @@ -29,7 +29,7 @@ $node->append_conf('postgresql.conf', "max_connections = 5"); $node->append_conf('postgresql.conf', "max_wal_senders = 0"); $node->append_conf('postgresql.conf', "autovacuum_max_workers = 1"); $node->append_conf('postgresql.conf', "max_worker_processes = 1"); -$node->append_conf('postgresql.conf', "log_connections = on"); +$node->append_conf('postgresql.conf', "log_connections = 'all'"); $node->append_conf('postgresql.conf', "log_min_messages = debug2"); $node->append_conf('postgresql.conf', "authentication_timeout = '$authentication_timeout s'"); diff --git a/src/test/recovery/t/013_crash_restart.pl b/src/test/recovery/t/013_crash_restart.pl index cd848918d00..2b6ec94cd75 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_connections = '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 483a416723f..5d4b695c193 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_connections = '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 ddb7223b337..388a50add7a 100644 --- a/src/test/recovery/t/032_relfilenode_reuse.pl +++ b/src/test/recovery/t/032_relfilenode_reuse.pl @@ -14,7 +14,7 @@ $node_primary->init(allows_streaming => 1); $node_primary->append_conf( 'postgresql.conf', q[ allow_in_place_tablespaces = true -log_connections=on +log_connections='all' # to avoid "repairing" corruption full_page_writes=off log_min_messages=debug2 diff --git a/src/test/recovery/t/037_invalid_database.pl b/src/test/recovery/t/037_invalid_database.pl index bdf39397397..ea62650b767 100644 --- a/src/test/recovery/t/037_invalid_database.pl +++ b/src/test/recovery/t/037_invalid_database.pl @@ -15,7 +15,7 @@ $node->append_conf( autovacuum = off max_prepared_transactions=5 log_min_duration_statement=0 -log_connections=on +log_connections='all' log_disconnections=on )); diff --git a/src/test/ssl/t/SSL/Server.pm b/src/test/ssl/t/SSL/Server.pm index 447469d8937..7bc45ee7611 100644 --- a/src/test/ssl/t/SSL/Server.pm +++ b/src/test/ssl/t/SSL/Server.pm @@ -200,7 +200,7 @@ sub configure_test_server_for_ssl $node->append_conf( 'postgresql.conf', <