>From 19a955cb4c9f0caeb99ff4de71d5a5e344932f53 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Fri, 17 Oct 2014 10:15:40 +0800 Subject: [PATCH 2/2] Log a hint if pg_ident.conf or pg_hba.conf changed since last reload Users often get tripped up by the fact that you have to reload the server config to have changes to pg_hba.conf take effect. To help them out, we now emit a HINT message when authentication fails and pg_hba.conf or pg_ident.conf are stale, telling them that they should check the server error log for details. In the server error log we report that pg_hba.conf or pg_ident.conf have changed since the last time the server configuration was reloaded, and that they should reload the config. No attempt is made to determine whether the change to the config files is relevant. This is done purely based on timestamp comparisons. If the change isn't related to the connection issue they're having then at worst they'll reload the config file and get the same error sans the HINT. --- src/backend/libpq/auth.c | 68 +++++++++++++++++++++++++++++++++++++++++++----- src/backend/libpq/hba.c | 8 ++++-- src/include/libpq/auth.h | 2 ++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index b1974d1..8f682e2 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -16,9 +16,11 @@ #include "postgres.h" #include +#include #include #include #include +#include #include #include "libpq/auth.h" @@ -30,6 +32,8 @@ #include "miscadmin.h" #include "replication/walsender.h" #include "storage/ipc.h" +#include "utils/guc.h" +#include "utils/timestamp.h" /*---------------------------------------------------------------- @@ -41,6 +45,8 @@ static void auth_failed(Port *port, int status, char *logdetail); static char *recv_password_packet(Port *port); static int recv_and_check_password_packet(Port *port, char **logdetail); +static int errhint_if_hba_conf_stale(void); + /*---------------------------------------------------------------- * Ident authentication @@ -282,12 +288,12 @@ auth_failed(Port *port, int status, char *logdetail) ereport(FATAL, (errcode(errcode_return), errmsg(errstr, port->user_name), - logdetail ? errdetail_log("%s", logdetail) : 0)); + logdetail ? errdetail_log("%s", logdetail) : 0, + errhint_if_hba_conf_stale())); /* doesn't return */ } - /* * Client authentication starts here. If there is an error, this * function does not return and the backend process is terminated. @@ -334,7 +340,8 @@ ClientAuthentication(Port *port) { ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), - errmsg("connection requires a valid client certificate"))); + errmsg("connection requires a valid client certificate"), + errhint_if_hba_conf_stale())); } #else @@ -378,12 +385,14 @@ ClientAuthentication(Port *port) (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("pg_hba.conf rejects replication connection for host \"%s\", user \"%s\", %s", hostinfo, port->user_name, - port->ssl_in_use ? _("SSL on") : _("SSL off")))); + port->ssl_in_use ? _("SSL on") : _("SSL off")), + errhint_if_hba_conf_stale())); #else ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("pg_hba.conf rejects replication connection for host \"%s\", user \"%s\"", - hostinfo, port->user_name))); + hostinfo, port->user_name), + errhint_if_hba_conf_stale())); #endif } else @@ -394,13 +403,15 @@ ClientAuthentication(Port *port) errmsg("pg_hba.conf rejects connection for host \"%s\", user \"%s\", database \"%s\", %s", hostinfo, port->user_name, port->database_name, - port->ssl_in_use ? _("SSL on") : _("SSL off")))); + port->ssl_in_use ? _("SSL on") : _("SSL off")), + errhint_if_hba_conf_stale())); #else ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("pg_hba.conf rejects connection for host \"%s\", user \"%s\", database \"%s\"", hostinfo, port->user_name, - port->database_name))); + port->database_name), + errhint_if_hba_conf_stale())); #endif } break; @@ -453,12 +464,14 @@ ClientAuthentication(Port *port) errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\", %s", hostinfo, port->user_name, port->ssl_in_use ? _("SSL on") : _("SSL off")), + errhint_if_hba_conf_stale(), HOSTNAME_LOOKUP_DETAIL(port))); #else ereport(FATAL, (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION), errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\"", hostinfo, port->user_name), + errhint_if_hba_conf_stale(), HOSTNAME_LOOKUP_DETAIL(port))); #endif } @@ -471,6 +484,7 @@ ClientAuthentication(Port *port) hostinfo, port->user_name, port->database_name, port->ssl_in_use ? _("SSL on") : _("SSL off")), + errhint_if_hba_conf_stale(), HOSTNAME_LOOKUP_DETAIL(port))); #else ereport(FATAL, @@ -478,6 +492,7 @@ ClientAuthentication(Port *port) errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\"", hostinfo, port->user_name, port->database_name), + errhint_if_hba_conf_stale(), HOSTNAME_LOOKUP_DETAIL(port))); #endif } @@ -684,6 +699,45 @@ recv_password_packet(Port *port) return buf.data; } +/* See errhint_if_hba_conf_stale */ +static int +errhint_if_cfg_stale(const char * cfgpath, const char * cfgfile) +{ + struct stat statbuf; + + if (stat(cfgpath, &statbuf) == 0) + { + if (difftime(statbuf.st_mtime, timestamptz_to_time_t(PgReloadTime)) > 0) + { + errhint("See the server error log for additional information."); + return errhint_log("%s has been changed since last server configuration reload. Reload the server configuration to apply the changes.", + cfgfile); + } + } + return 0; +} + +/* + * If pg_hba.conf has been modified since the last reload, emit a HINT + * to the client suggesting that they check the server error log, and + * a HINT to the server error log informing the user that pg_hba.conf + * has changed since the last reload. + */ +static int +errhint_if_hba_conf_stale() +{ + return errhint_if_cfg_stale(HbaFileName, "pg_hba.conf"); +} + +/* + * Like errhint_if_hba_conf_stale but for pg_ident.conf + */ +int +errhint_if_ident_conf_stale() +{ + return errhint_if_cfg_stale(IdentFileName, "pg_ident.conf"); +} + /*---------------------------------------------------------------- * MD5 authentication diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 84da823..3265a85 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -28,12 +28,14 @@ #include "catalog/pg_collation.h" #include "libpq/ip.h" #include "libpq/libpq.h" +#include "libpq/auth.h" #include "postmaster/postmaster.h" #include "regex/regex.h" #include "replication/walsender.h" #include "storage/fd.h" #include "utils/acl.h" #include "utils/guc.h" +#include "utils/timestamp.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -2106,7 +2108,8 @@ check_usermap(const char *usermap_name, } ereport(LOG, (errmsg("provided user name (%s) and authenticated user name (%s) do not match", - pg_role, auth_user))); + pg_role, auth_user), + errhint_if_ident_conf_stale())); return STATUS_ERROR; } else @@ -2126,7 +2129,8 @@ check_usermap(const char *usermap_name, { ereport(LOG, (errmsg("no match in usermap \"%s\" for user \"%s\" authenticated as \"%s\"", - usermap_name, pg_role, auth_user))); + usermap_name, pg_role, auth_user), + errhint_if_ident_conf_stale())); } return found_entry ? STATUS_OK : STATUS_ERROR; } diff --git a/src/include/libpq/auth.h b/src/include/libpq/auth.h index ace647a..cfc6d9c 100644 --- a/src/include/libpq/auth.h +++ b/src/include/libpq/auth.h @@ -22,6 +22,8 @@ extern char *pg_krb_realm; extern void ClientAuthentication(Port *port); +extern int errhint_if_ident_conf_stale(void); + /* Hook for plugins to get control in ClientAuthentication() */ typedef void (*ClientAuthentication_hook_type) (Port *, int); extern PGDLLIMPORT ClientAuthentication_hook_type ClientAuthentication_hook; -- 1.9.3