diff --git a/dbutils.c b/dbutils.c index 31be15f9..50560b36 100644 --- a/dbutils.c +++ b/dbutils.c @@ -25,55 +25,6 @@ static void _populate_node_record(PGresult *res, t_node_info *node_info, int row static bool _create_update_node_record(PGconn *conn, char *action, t_node_info *node_info); static bool _create_event_record(PGconn *conn, t_configuration_options *options, int node_id, char *event, bool successful, char *details, t_event_info *event_info); -/* =================== */ -/* extension functions */ -/* =================== */ - -t_extension_status -get_repmgr_extension_status(PGconn *conn) -{ - PQExpBufferData query; - PGresult *res; - - /* TODO: check version */ - - initPQExpBuffer(&query); - - appendPQExpBuffer(&query, - " SELECT ae.name, e.extname " - " FROM pg_catalog.pg_available_extensions ae " - "LEFT JOIN pg_catalog.pg_extension e " - " ON e.extname=ae.name " - " WHERE ae.name='repmgr' "); - - res = PQexec(conn, query.data); - - termPQExpBuffer(&query); - - if (PQresultStatus(res) != PGRES_TUPLES_OK) - { - log_error(_("unable to execute extension query:\n %s"), - PQerrorMessage(conn)); - PQclear(res); - - return REPMGR_UNKNOWN; - } - - /* 1. Check extension is actually available */ - - if (PQntuples(res) == 0) - { - return REPMGR_UNAVAILABLE; - } - - /* 2. Check if extension installed */ - if (PQgetisnull(res, 0, 1) == 0) - { - return REPMGR_INSTALLED; - } - - return REPMGR_AVAILABLE; -} /* ==================== */ /* Connection functions */ @@ -241,6 +192,28 @@ establish_db_connection_by_params(const char *keywords[], const char *values[], return conn; } + +bool +is_superuser_connection(PGconn *conn, t_connection_user *userinfo) +{ + char *current_user; + const char *superuser_status; + bool is_superuser; + + current_user = PQuser(conn); + superuser_status = PQparameterStatus(conn, "is_superuser"); + is_superuser = (strcmp(superuser_status, "on") == 0) ? true : false; + + if (userinfo != NULL) + { + strncpy(userinfo->username, current_user, MAXLEN); + userinfo->is_superuser = is_superuser; + } + + return is_superuser; +} + + /* =============================== */ /* conninfo manipulation functions */ /* =============================== */ @@ -1000,6 +973,58 @@ bool atobool(const char *value) : false; } + +/* =================== */ +/* extension functions */ +/* =================== */ + +t_extension_status +get_repmgr_extension_status(PGconn *conn) +{ + PQExpBufferData query; + PGresult *res; + + /* TODO: check version */ + + initPQExpBuffer(&query); + + appendPQExpBuffer(&query, + " SELECT ae.name, e.extname " + " FROM pg_catalog.pg_available_extensions ae " + "LEFT JOIN pg_catalog.pg_extension e " + " ON e.extname=ae.name " + " WHERE ae.name='repmgr' "); + + res = PQexec(conn, query.data); + + termPQExpBuffer(&query); + + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + log_error(_("unable to execute extension query:\n %s"), + PQerrorMessage(conn)); + PQclear(res); + + return REPMGR_UNKNOWN; + } + + /* 1. Check extension is actually available */ + + if (PQntuples(res) == 0) + { + return REPMGR_UNAVAILABLE; + } + + /* 2. Check if extension installed */ + if (PQgetisnull(res, 0, 1) == 0) + { + return REPMGR_INSTALLED; + } + + return REPMGR_AVAILABLE; +} + + /* ===================== */ /* Node record functions */ /* ===================== */ @@ -1811,5 +1836,3 @@ stop_backup(PGconn *conn, char *last_wal_segment, int server_version_num) return true; } - - diff --git a/dbutils.h b/dbutils.h index b303ee63..e4aa6602 100644 --- a/dbutils.h +++ b/dbutils.h @@ -114,6 +114,12 @@ typedef struct s_replication_slot } t_replication_slot; +typedef struct s_connection_user +{ + char username[MAXLEN]; + bool is_superuser; +} t_connection_user; + /* connection functions */ PGconn *establish_db_connection(const char *conninfo, const bool exit_on_error); @@ -126,8 +132,7 @@ PGconn *establish_db_connection_by_params(const char *keywords[], const char *values[], const bool exit_on_error); -/* extension functions */ -t_extension_status get_repmgr_extension_status(PGconn *conn); +bool is_superuser_connection(PGconn *conn, t_connection_user *userinfo); /* conninfo manipulation functions */ bool get_conninfo_value(const char *conninfo, const char *keyword, char *output); @@ -161,6 +166,8 @@ int is_standby(PGconn *conn); PGconn *get_master_connection(PGconn *standby_conn, int *master_id, char *master_conninfo_out); int get_master_node_id(PGconn *conn); +/* extension functions */ +t_extension_status get_repmgr_extension_status(PGconn *conn); /* result functions */ bool atobool(const char *value); diff --git a/repmgr-action-master.c b/repmgr-action-master.c index a9374038..2f5e8d91 100644 --- a/repmgr-action-master.c +++ b/repmgr-action-master.c @@ -189,8 +189,16 @@ do_master_register(void) commit_transaction(conn); PQfinish(conn); - log_notice(_("master node record (id: %i) %s"), - config_file_options.node_id, - record_found ? "updated" : "registered"); + if (record_found) + { + log_notice(_("master node record (id: %i) updated"), + config_file_options.node_id); + } + else + { + log_notice(_("master node record (id: %i) registered"), + config_file_options.node_id); + } + return; } diff --git a/repmgr-action-standby.c b/repmgr-action-standby.c index 0bc18a9a..86738815 100644 --- a/repmgr-action-standby.c +++ b/repmgr-action-standby.c @@ -111,9 +111,6 @@ do_standby_clone(void) * to the node we're cloning from) to write to recovery.conf */ - /* - * detecting the cloning mode - */ mode = get_standby_clone_mode(); /* @@ -131,9 +128,9 @@ do_standby_clone(void) } /* - * If dest_dir (-D/--pgdata) was provided, this will become the new data - * directory (otherwise repmgr will default to using the same directory - * path as on the source host). + * If a data directory (-D/--pgdata) was provided, use that, otherwise + * repmgr will default to using the same directory path as on the source + * host. * * Note that barman mode requires -D/--pgdata. * @@ -144,7 +141,7 @@ do_standby_clone(void) if (runtime_options.data_dir[0]) { local_data_directory_provided = true; - log_notice(_("destination directory '%s' provided"), + log_notice(_("destination directory \"%s\" provided"), runtime_options.data_dir); } else if (mode == barman) @@ -449,6 +446,9 @@ check_barman_config(void) static void check_source_server() { + PGconn *superuser_conn = NULL; + PGconn *privileged_conn = NULL; + char cluster_size[MAXLEN]; t_node_info node_record = T_NODE_INFO_INITIALIZER; int query_result; @@ -548,13 +548,20 @@ check_source_server() } /* Fetch the source's data directory */ - if (get_pg_setting(source_conn, "data_directory", upstream_data_directory) == false) + get_superuser_connection(&source_conn, &superuser_conn, &privileged_conn); + + if (get_pg_setting(privileged_conn, "data_directory", upstream_data_directory) == false) { log_error(_("unable to retrieve source node's data directory")); log_hint(_("STANDBY CLONE must be run as a database superuser")); PQfinish(source_conn); + if(superuser_conn != NULL) + PQfinish(superuser_conn); + exit(ERR_BAD_CONFIG); } + if(superuser_conn != NULL) + PQfinish(superuser_conn); /* * If no target data directory was explicitly provided, we'll default to @@ -588,8 +595,6 @@ check_source_server() * Copy the source connection so that we have some default values, * particularly stuff like passwords extracted from PGPASSFILE; * these will be overridden from the upstream conninfo, if provided. - * - * XXX only allow passwords if --use-conninfo-password */ conn_to_param_list(source_conn, &recovery_conninfo); @@ -723,10 +728,11 @@ check_source_server_via_barman() static void initialise_direct_clone(void) { - PGresult *res; - int i; - + PGconn *superuser_conn = NULL; + PGconn *privileged_conn = NULL; + PGresult *res; PQExpBufferData query; + int i; /* * Check the destination data directory can be used @@ -735,7 +741,7 @@ initialise_direct_clone(void) if (!create_pg_dir(local_data_directory, runtime_options.force)) { - log_error(_("unable to use directory %s"), + log_error(_("unable to use directory \"%s\""), local_data_directory); log_hint(_("use -F/--force to force this directory to be overwritten")); exit(ERR_BAD_CONFIG); @@ -772,7 +778,7 @@ initialise_direct_clone(void) appendPQExpBuffer(&query, "SELECT spcname " " FROM pg_catalog.pg_tablespace " - " WHERE pg_tablespace_location(oid) = '%s'", + " WHERE pg_catalog.pg_tablespace_location(oid) = '%s'", cell->old_dir); res = PQexec(source_conn, query.data); @@ -801,16 +807,23 @@ initialise_direct_clone(void) /* * Obtain configuration file locations + * * We'll check to see whether the configuration files are in the data * directory - if not we'll have to copy them via SSH, if copying * requested. * + * This will require superuser permissions, so we'll attempt to connect + * as -S/--superuser (if provided), otherwise check the current connection + * user has superuser rights. + * * XXX: if configuration files are symlinks to targets outside the data * directory, they won't be copied by pg_basebackup, but we can't tell * this from the below query; we'll probably need to add a check for their * presence and if missing force copy by SSH */ + get_superuser_connection(&source_conn, &superuser_conn, &privileged_conn); + initPQExpBuffer(&query); appendPQExpBuffer(&query, @@ -820,22 +833,26 @@ initialise_direct_clone(void) " WHERE name = 'data_directory' " " ) " " SELECT DISTINCT(sourcefile), " - " regexp_replace(sourcefile, '^.*\\/', '') AS filename, " + " pg_catalog.regexp_replace(sourcefile, '^.*\\/', '') AS filename, " " sourcefile ~ ('^' || dd.data_directory) AS in_data_dir " " FROM dd, pg_catalog.pg_settings ps " " WHERE sourcefile IS NOT NULL " " ORDER BY 1 "); log_debug("standby clone: %s", query.data); - res = PQexec(source_conn, query.data); + res = PQexec(privileged_conn, query.data); termPQExpBuffer(&query); if (PQresultStatus(res) != PGRES_TUPLES_OK) { log_error(_("unable to retrieve configuration file locations:\n %s"), - PQerrorMessage(source_conn)); + PQerrorMessage(privileged_conn)); PQclear(res); PQfinish(source_conn); + + if (superuser_conn != NULL) + PQfinish(superuser_conn); + exit(ERR_BAD_CONFIG); } @@ -873,15 +890,19 @@ initialise_direct_clone(void) " ORDER BY 1 "); log_debug("standby clone: %s", query.data); - res = PQexec(source_conn, query.data); + res = PQexec(privileged_conn, query.data); termPQExpBuffer(&query); if (PQresultStatus(res) != PGRES_TUPLES_OK) { log_error(_("unable to retrieve configuration file locations:\n %s"), - PQerrorMessage(source_conn)); + PQerrorMessage(privileged_conn)); PQclear(res); PQfinish(source_conn); + + if (superuser_conn != NULL) + PQfinish(superuser_conn); + exit(ERR_BAD_CONFIG); } @@ -909,7 +930,7 @@ initialise_direct_clone(void) PQExpBufferData event_details; initPQExpBuffer(&event_details); - if (create_replication_slot(source_conn, repmgr_slot_name, server_version_num, &event_details) == false) + if (create_replication_slot(privileged_conn, repmgr_slot_name, server_version_num, &event_details) == false) { log_error("%s", event_details.data); @@ -921,15 +942,24 @@ initialise_direct_clone(void) event_details.data); PQfinish(source_conn); + + if (superuser_conn != NULL) + PQfinish(superuser_conn); + exit(ERR_DB_QUERY); } termPQExpBuffer(&event_details); - log_notice(_("replication slot '%s' created on upstream node (node_id: %i)"), + log_notice(_("replication slot \"%s\" created on upstream node (node_id: %i)"), repmgr_slot_name, upstream_node_id); } + + if (superuser_conn != NULL) + PQfinish(superuser_conn); + + return; } diff --git a/repmgr-client-global.h b/repmgr-client-global.h index 4a21bec8..d69ba44e 100644 --- a/repmgr-client-global.h +++ b/repmgr-client-global.h @@ -125,4 +125,5 @@ extern char * make_pg_path(char *file); extern bool create_recovery_file(const char *data_dir, t_conninfo_param_list *recovery_conninfo); +extern void get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privileged_conn); #endif diff --git a/repmgr-client.c b/repmgr-client.c index 1fe39f85..aebe5b7a 100644 --- a/repmgr-client.c +++ b/repmgr-client.c @@ -988,7 +988,7 @@ do_help(void) * user if not a superuser. * * Note: - * This should be the only place where superuser rights are required. + * This is one of two places where superuser rights are required. * We should also consider possible scenarious where a non-superuser * has sufficient privileges to install the extension. */ @@ -1000,9 +1000,9 @@ create_repmgr_extension(PGconn *conn) PGresult *res; t_extension_status extension_status; - char *current_user; - const char *superuser_status; - bool is_superuser; + + t_connection_user userinfo; + bool is_superuser = false; PGconn *superuser_conn = NULL; PGconn *schema_create_conn = NULL; @@ -1020,7 +1020,7 @@ create_repmgr_extension(PGconn *conn) case REPMGR_INSTALLED: /* TODO: check version */ - log_info(_("extension \"repmgr\" already installed")); + log_info(_("\"repmgr\" extension is already installed")); return true; case REPMGR_AVAILABLE: @@ -1029,48 +1029,11 @@ create_repmgr_extension(PGconn *conn) } + /* 3. Attempt to get a superuser connection */ - /* 3. Check if repmgr user is superuser, if not connect as superuser */ - current_user = PQuser(conn); - superuser_status = PQparameterStatus(conn, "is_superuser"); + is_superuser = is_superuser_connection(conn, &userinfo); - is_superuser = (strcmp(superuser_status, "on") == 0) ? true : false; - - if (is_superuser == false) - { - if (runtime_options.superuser[0] == '\0') - { - log_error(_("\"%s\" is not a superuser and no superuser name supplied"), current_user); - log_hint(_("supply a valid superuser name with -S/--superuser")); - return false; - } - - superuser_conn = establish_db_connection_as_user(config_file_options.conninfo, - runtime_options.superuser, - false); - - if (PQstatus(superuser_conn) != CONNECTION_OK) - { - log_error(_("unable to establish superuser connection as \"%s\""), - runtime_options.superuser); - return false; - } - - /* check provided superuser really is superuser */ - superuser_status = PQparameterStatus(superuser_conn, "is_superuser"); - if (strcmp(superuser_status, "off") == 0) - { - log_error(_("\"%s\" is not a superuser"), runtime_options.superuser); - PQfinish(superuser_conn); - return false; - } - - schema_create_conn = superuser_conn; - } - else - { - schema_create_conn = conn; - } + get_superuser_connection(&conn, &superuser_conn, &schema_create_conn); /* 4. Create extension */ initPQExpBuffer(&query); @@ -1089,7 +1052,7 @@ create_repmgr_extension(PGconn *conn) log_hint(_("check that the provided user has sufficient privileges for CREATE EXTENSION")); PQclear(res); - if (superuser_conn != 0) + if (superuser_conn != NULL) PQfinish(superuser_conn); return false; } @@ -1103,14 +1066,14 @@ create_repmgr_extension(PGconn *conn) appendPQExpBuffer(&query, "GRANT USAGE ON SCHEMA repmgr TO %s", - current_user); + userinfo.username); res = PQexec(schema_create_conn, query.data); termPQExpBuffer(&query); if (PQresultStatus(res) != PGRES_COMMAND_OK) { log_error(_("unable to grant usage on \"repmgr\" extension to %s:\n %s"), - current_user, + userinfo.username, PQerrorMessage(schema_create_conn)); PQclear(res); @@ -1123,7 +1086,7 @@ create_repmgr_extension(PGconn *conn) initPQExpBuffer(&query); appendPQExpBuffer(&query, "GRANT ALL ON ALL TABLES IN SCHEMA repmgr TO %s", - current_user); + userinfo.username); res = PQexec(schema_create_conn, query.data); termPQExpBuffer(&query); @@ -1131,18 +1094,18 @@ create_repmgr_extension(PGconn *conn) if (PQresultStatus(res) != PGRES_COMMAND_OK) { log_error(_("unable to grant permission on tables on \"repmgr\" extension to %s:\n %s"), - current_user, + userinfo.username, PQerrorMessage(schema_create_conn)); PQclear(res); - if (superuser_conn != 0) + if (superuser_conn != NULL) PQfinish(superuser_conn); return false; } } - if (superuser_conn != 0) + if (superuser_conn != NULL) PQfinish(superuser_conn); log_notice(_("\"repmgr\" extension successfully installed")); @@ -1287,6 +1250,57 @@ local_command(const char *command, PQExpBufferData *outputbuf) } +void +get_superuser_connection(PGconn **conn, PGconn **superuser_conn, PGconn **privileged_conn) +{ + t_connection_user userinfo; + bool is_superuser; + + is_superuser = is_superuser_connection(*conn, &userinfo); + + if (is_superuser == true) + { + *privileged_conn = *conn; + + return; + } + + // XXX largely duplicatied from create_repmgr_extension() + if (runtime_options.superuser[0] == '\0') + { + log_error(_("\"%s\" is not a superuser and no superuser name supplied"), userinfo.username); + log_hint(_("supply a valid superuser name with -S/--superuser")); + PQfinish(*conn); + exit(ERR_BAD_CONFIG); + } + + *superuser_conn = establish_db_connection_as_user(config_file_options.conninfo, + runtime_options.superuser, + false); + + if (PQstatus(*superuser_conn) != CONNECTION_OK) + { + log_error(_("unable to establish superuser connection as \"%s\""), + runtime_options.superuser); + + PQfinish(*conn); + exit(ERR_BAD_CONFIG); + } + + /* check provided superuser really is superuser */ + if (!is_superuser_connection(*superuser_conn, NULL)) + { + log_error(_("\"%s\" is not a superuser"), runtime_options.superuser); + PQfinish(*superuser_conn); + PQfinish(*conn); + exit(ERR_BAD_CONFIG); + } + + *privileged_conn = *superuser_conn; + return; +} + + /* * check_upstream_config() *