From 09e597dcdd64b6616d2dd201c8cc481ecefe78f7 Mon Sep 17 00:00:00 2001 From: Ian Barwick Date: Thu, 12 Apr 2018 12:42:46 +0900 Subject: [PATCH] Fix superuser password handling When establishing a superuser connection, the connection parameters were being copied from the existing (non-superuser) connection, which in some circumstances can lead to that user's password being included in the copied parameter list. The password parameter, if set, will now always be removed, which will cause libpq to retrieve the correct one from the .pgpass file. Addresses GitHub #400. --- dbutils.c | 17 +++++++++++++++-- repmgr-client.c | 3 +++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/dbutils.c b/dbutils.c index d576bed6..c8f27edf 100644 --- a/dbutils.c +++ b/dbutils.c @@ -556,7 +556,7 @@ param_get(t_conninfo_param_list *param_list, const char *param) /* * Parse a conninfo string into a t_conninfo_param_list * - * See conn_to_param_list() to do the same for a PQconn + * See conn_to_param_list() to do the same for a PGconn * * "ignore_local_params": ignores those parameters specific * to a local installation, i.e. when parsing an upstream @@ -600,10 +600,19 @@ parse_conninfo_string(const char *conninfo_str, t_conninfo_param_list *param_lis return true; } + /* - * Parse a PQconn into a t_conninfo_param_list + * Parse a PGconn into a t_conninfo_param_list * * See parse_conninfo_string() to do the same for a conninfo string + * + * NOTE: the current use case for this is to take an active connection, + * replace the existing username (typically replacing it with the superuser + * or replication user name), and make a new connection as that user. + * If the "password" field is set, it will cause any connection made with + * these parameters to fail (unless of course the password happens to be the + * same). Therefore we remove the password altogether, and rely on it being + * available via .pgpass. */ void conn_to_param_list(PGconn *conn, t_conninfo_param_list *param_list) @@ -619,6 +628,10 @@ conn_to_param_list(PGconn *conn, t_conninfo_param_list *param_list) (option->val != NULL && option->val[0] == '\0')) continue; + /* Ignore "password" */ + if (strcmp(option->keyword, "password") == 0) + continue; + param_set(param_list, option->keyword, option->val); } diff --git a/repmgr-client.c b/repmgr-client.c index 454673e5..9b455029 100644 --- a/repmgr-client.c +++ b/repmgr-client.c @@ -2236,6 +2236,7 @@ get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privil log_error(_("no database connection available")); exit(ERR_INTERNAL); } + is_superuser = is_superuser_connection(*conn, &userinfo); if (is_superuser == true) @@ -2277,6 +2278,8 @@ get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privil exit(ERR_BAD_CONFIG); } + log_debug("established superuser connection as \"%s\"", runtime_options.superuser); + *privileged_conn = *superuser_conn; return; }