From f06b1c598254f8adb2b7f51d6a7685618a7fb121 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 3 Mar 2021 09:44:46 +0100 Subject: [PATCH] pg_upgrade: Check version of target cluster binaries This expands the binary validation in pg_upgrade with a version check per binary to ensure that the target cluster installation only contains binaries from the target version. In order to reduce duplication, validate_exec is exported from port.h and the local copy in pg_upgrade is removed. Author: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us --- src/bin/pg_upgrade/exec.c | 90 ++++++++++++++++----------------------- src/common/exec.c | 3 +- src/include/port.h | 1 + 3 files changed, 39 insertions(+), 55 deletions(-) diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index 43a4565c2e..4d77543d6f 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -11,12 +11,13 @@ #include +#include "common/string.h" #include "pg_upgrade.h" static void check_data_dir(ClusterInfo *cluster); static void check_bin_dir(ClusterInfo *cluster); static void get_bin_version(ClusterInfo *cluster); -static void validate_exec(const char *dir, const char *cmdName); +static void check_exec(const char *dir, const char *program); #ifdef WIN32 static int win32_check_directory_write_permissions(void); @@ -375,9 +376,9 @@ check_bin_dir(ClusterInfo *cluster) report_status(PG_FATAL, "\"%s\" is not a directory\n", cluster->bindir); - validate_exec(cluster->bindir, "postgres"); - validate_exec(cluster->bindir, "pg_controldata"); - validate_exec(cluster->bindir, "pg_ctl"); + check_exec(cluster->bindir, "postgres"); + check_exec(cluster->bindir, "pg_controldata"); + check_exec(cluster->bindir, "pg_ctl"); /* * Fetch the binary version after checking for the existence of pg_ctl. @@ -388,9 +389,9 @@ check_bin_dir(ClusterInfo *cluster) /* pg_resetxlog has been renamed to pg_resetwal in version 10 */ if (GET_MAJOR_VERSION(cluster->bin_version) <= 906) - validate_exec(cluster->bindir, "pg_resetxlog"); + check_exec(cluster->bindir, "pg_resetxlog"); else - validate_exec(cluster->bindir, "pg_resetwal"); + check_exec(cluster->bindir, "pg_resetwal"); if (cluster == &new_cluster) { @@ -399,63 +400,46 @@ check_bin_dir(ClusterInfo *cluster) * pg_dumpall are used to dump the old cluster, but must be of the * target version. */ - validate_exec(cluster->bindir, "initdb"); - validate_exec(cluster->bindir, "pg_dump"); - validate_exec(cluster->bindir, "pg_dumpall"); - validate_exec(cluster->bindir, "pg_restore"); - validate_exec(cluster->bindir, "psql"); - validate_exec(cluster->bindir, "vacuumdb"); + check_exec(cluster->bindir, "initdb"); + check_exec(cluster->bindir, "pg_dump"); + check_exec(cluster->bindir, "pg_dumpall"); + check_exec(cluster->bindir, "pg_restore"); + check_exec(cluster->bindir, "psql"); + check_exec(cluster->bindir, "vacuumdb"); } } - -/* - * validate_exec() - * - * validate "path" as an executable file - */ static void -validate_exec(const char *dir, const char *cmdName) +check_exec(const char *dir, const char *program) { - char path[MAXPGPATH]; - struct stat buf; + char path[MAXPGPATH]; + char line[MAXPGPATH]; + char cmd[MAXPGPATH]; + char versionstr[128]; + int ret; - snprintf(path, sizeof(path), "%s/%s", dir, cmdName); + snprintf(path, sizeof(path), "%s/%s", dir, program); -#ifdef WIN32 - /* Windows requires a .exe suffix for stat() */ - if (strlen(path) <= strlen(EXE_EXT) || - pg_strcasecmp(path + strlen(path) - strlen(EXE_EXT), EXE_EXT) != 0) - strlcat(path, EXE_EXT, sizeof(path)); -#endif + ret = validate_exec(path); - /* - * Ensure that the file exists and is a regular file. - */ - if (stat(path, &buf) < 0) - pg_fatal("check for \"%s\" failed: %s\n", - path, strerror(errno)); - else if (!S_ISREG(buf.st_mode)) + if (ret == -1) pg_fatal("check for \"%s\" failed: not a regular file\n", path); - - /* - * Ensure that the file is both executable and readable (required for - * dynamic loading). - */ -#ifndef WIN32 - if (access(path, R_OK) != 0) -#else - if ((buf.st_mode & S_IRUSR) == 0) -#endif - pg_fatal("check for \"%s\" failed: cannot read file (permission denied)\n", - path); - -#ifndef WIN32 - if (access(path, X_OK) != 0) -#else - if ((buf.st_mode & S_IXUSR) == 0) -#endif + else if (ret == -2) pg_fatal("check for \"%s\" failed: cannot execute (permission denied)\n", path); + + snprintf(cmd, sizeof(cmd), "\"%s\" -V", path); + + if (!pipe_read_line(cmd, line, sizeof(line))) + pg_fatal("check for \"%s\" failed: cannot execute\n", + path); + + pg_strip_crlf(line); + + snprintf(versionstr, sizeof(versionstr), "%s (PostgreSQL) " PG_VERSION, program); + + if (strcmp(line, versionstr) != 0) + pg_fatal("check for \"%s\" failed: incorrect version: found \"%s\", expected \"%s\"\n", + path, line, versionstr); } diff --git a/src/common/exec.c b/src/common/exec.c index 6cbc820042..66c3aa6acc 100644 --- a/src/common/exec.c +++ b/src/common/exec.c @@ -49,7 +49,6 @@ #define getcwd(cwd,len) GetCurrentDirectory(len, cwd) #endif -static int validate_exec(const char *path); static int resolve_symlinks(char *path); #ifdef WIN32 @@ -63,7 +62,7 @@ static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser); * -1 if the regular file "path" does not exist or cannot be executed. * -2 if the file is otherwise valid but cannot be read. */ -static int +int validate_exec(const char *path) { struct stat buf; diff --git a/src/include/port.h b/src/include/port.h index 6486db9fdd..227ef4b148 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -125,6 +125,7 @@ extern void pgfnames_cleanup(char **filenames); extern void set_pglocale_pgservice(const char *argv0, const char *app); /* Portable way to find and execute binaries (in exec.c) */ +extern int validate_exec(const char *path); extern int find_my_exec(const char *argv0, char *retpath); extern int find_other_exec(const char *argv0, const char *target, const char *versionstr, char *retpath);