From bff3a74ef82df71cc3b0811ee39582a27a0577c0 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 6 Mar 2018 13:03:32 -0500 Subject: [PATCH 2/3] Refactor file permissions in backend/frontend Adds a new header (file_perm.h) and makes all front-end utilities use the new constants for setting umask. Converts mkdir() calls in the backend to MakeDirectoryDefaultPerm() if the original call used default permissions. Adds tests to make sure permissions in PGDATA are set correctly by the front-end tools. --- src/backend/access/transam/xlog.c | 2 +- src/backend/commands/tablespace.c | 22 +++++--- src/backend/postmaster/postmaster.c | 2 +- src/backend/postmaster/syslogger.c | 5 +- src/backend/replication/slot.c | 5 +- src/backend/storage/file/copydir.c | 2 +- src/backend/storage/file/fd.c | 32 +++++++----- src/backend/storage/ipc/ipc.c | 4 ++ src/bin/initdb/initdb.c | 41 ++++----------- src/bin/initdb/t/001_initdb.pl | 4 +- src/bin/pg_ctl/pg_ctl.c | 3 +- src/bin/pg_ctl/t/001_start_stop.pl | 13 +++-- src/bin/pg_resetwal/pg_resetwal.c | 8 ++- src/bin/pg_resetwal/t/001_basic.pl | 4 +- src/bin/pg_rewind/RewindTest.pm | 5 ++ src/bin/pg_rewind/file_ops.c | 7 +-- src/bin/pg_rewind/pg_rewind.c | 4 ++ src/bin/pg_rewind/t/001_basic.pl | 2 +- src/bin/pg_rewind/t/002_databases.pl | 2 +- src/bin/pg_rewind/t/003_extrafiles.pl | 5 +- src/bin/pg_rewind/t/004_pg_xlog_symlink.pl | 2 +- src/bin/pg_rewind/t/005_same_timeline.pl | 2 +- src/bin/pg_upgrade/file.c | 5 +- src/bin/pg_upgrade/pg_upgrade.c | 3 +- src/bin/pg_upgrade/test.sh | 11 ++++ src/include/common/file_perm.h | 32 ++++++++++++ src/include/storage/fd.h | 3 ++ src/test/perl/PostgresNode.pm | 3 ++ src/test/perl/TestLib.pm | 82 ++++++++++++++++++++++++++++++ 29 files changed, 237 insertions(+), 78 deletions(-) create mode 100644 src/include/common/file_perm.h diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 47a6c4d895..bf07bbd746 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -4086,7 +4086,7 @@ ValidateXLOGDirectoryStructure(void) { ereport(LOG, (errmsg("creating missing WAL directory \"%s\"", path))); - if (mkdir(path, S_IRWXU) < 0) + if (MakeDirectoryDefaultPerm(path) < 0) ereport(FATAL, (errmsg("could not create missing directory \"%s\": %m", path))); diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 5c450caa4e..c4ee987446 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -68,6 +68,7 @@ #include "commands/seclabel.h" #include "commands/tablecmds.h" #include "commands/tablespace.h" +#include "common/file_perm.h" #include "miscadmin.h" #include "postmaster/bgwriter.h" #include "storage/fd.h" @@ -151,7 +152,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo) else { /* Directory creation failed? */ - if (mkdir(dir, S_IRWXU) < 0) + if (MakeDirectoryDefaultPerm(dir) < 0) { char *parentdir; @@ -173,7 +174,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo) get_parent_directory(parentdir); get_parent_directory(parentdir); /* Can't create parent and it doesn't already exist? */ - if (mkdir(parentdir, S_IRWXU) < 0 && errno != EEXIST) + if (MakeDirectoryDefaultPerm(parentdir) < 0 && errno != EEXIST) ereport(ERROR, (errcode_for_file_access(), errmsg("could not create directory \"%s\": %m", @@ -184,7 +185,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo) parentdir = pstrdup(dir); get_parent_directory(parentdir); /* Can't create parent and it doesn't already exist? */ - if (mkdir(parentdir, S_IRWXU) < 0 && errno != EEXIST) + if (MakeDirectoryDefaultPerm(parentdir) < 0 && errno != EEXIST) ereport(ERROR, (errcode_for_file_access(), errmsg("could not create directory \"%s\": %m", @@ -192,7 +193,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool isRedo) pfree(parentdir); /* Create database directory */ - if (mkdir(dir, S_IRWXU) < 0) + if (MakeDirectoryDefaultPerm(dir) < 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not create directory \"%s\": %m", @@ -279,7 +280,8 @@ CreateTableSpace(CreateTableSpaceStmt *stmt) /* * Check that location isn't too long. Remember that we're going to append * 'PG_XXX//_.'. FYI, we never actually - * reference the whole path here, but mkdir() uses the first two parts. + * reference the whole path here, but MakeDirectoryDefaultPerm() uses the first + * two parts. */ if (strlen(location) + 1 + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 + OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1 + OIDCHARS > MAXPGPATH) @@ -565,6 +567,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid) char *linkloc; char *location_with_version_dir; struct stat st; + mode_t current_umask; /* Current umask value */ linkloc = psprintf("pg_tblspc/%u", tablespaceoid); location_with_version_dir = psprintf("%s/%s", location, @@ -574,7 +577,10 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid) * Attempt to coerce target directory to safe permissions. If this fails, * it doesn't exist or has the wrong owner. */ - if (chmod(location, S_IRWXU) != 0) + current_umask = umask(0); + umask(current_umask); + + if (chmod(location, PG_DIR_MODE_DEFAULT & ~current_umask) != 0) { if (errno == ENOENT) ereport(ERROR, @@ -599,7 +605,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid) if (stat(location_with_version_dir, &st) == 0 && S_ISDIR(st.st_mode)) { if (!rmtree(location_with_version_dir, true)) - /* If this failed, mkdir() below is going to error. */ + /* If this failed, MakeDirectoryDefaultPerm() below is going to error. */ ereport(WARNING, (errmsg("some useless files may be left behind in old database directory \"%s\"", location_with_version_dir))); @@ -610,7 +616,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid) * The creation of the version directory prevents more than one tablespace * in a single location. */ - if (mkdir(location_with_version_dir, S_IRWXU) < 0) + if (MakeDirectoryDefaultPerm(location_with_version_dir) < 0) { if (errno == EEXIST) ereport(ERROR, diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index f3ddf828bb..e13a2ae8c4 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -4495,7 +4495,7 @@ internal_forkexec(int argc, char *argv[], Port *port) * As in OpenTemporaryFileInTablespace, try to make the temp-file * directory */ - mkdir(PG_TEMP_FILES_DIR, S_IRWXU); + MakeDirectoryDefaultPerm(PG_TEMP_FILES_DIR); fp = AllocateFile(tmpfilename, PG_BINARY_W); if (!fp) diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c index f70eea37df..ae23941f59 100644 --- a/src/backend/postmaster/syslogger.c +++ b/src/backend/postmaster/syslogger.c @@ -41,6 +41,7 @@ #include "postmaster/postmaster.h" #include "postmaster/syslogger.h" #include "storage/dsm.h" +#include "storage/fd.h" #include "storage/ipc.h" #include "storage/latch.h" #include "storage/pg_shmem.h" @@ -322,7 +323,7 @@ SysLoggerMain(int argc, char *argv[]) /* * Also, create new directory if not present; ignore errors */ - mkdir(Log_directory, S_IRWXU); + MakeDirectoryDefaultPerm(Log_directory); } if (strcmp(Log_filename, currentLogFilename) != 0) { @@ -564,7 +565,7 @@ SysLogger_Start(void) /* * Create log directory if not present; ignore errors */ - mkdir(Log_directory, S_IRWXU); + MakeDirectoryDefaultPerm(Log_directory); /* * The initial logfile is created right in the postmaster, to verify that diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index fc9ef22b0b..3a98360b50 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -1166,13 +1166,14 @@ CreateSlotOnDisk(ReplicationSlot *slot) * It's just barely possible that some previous effort to create or drop a * slot with this name left a temp directory lying around. If that seems * to be the case, try to remove it. If the rmtree() fails, we'll error - * out at the mkdir() below, so we don't bother checking success. + * out at the MakeDirectoryDefaultPerm() below, so we don't bother checking + * success. */ if (stat(tmppath, &st) == 0 && S_ISDIR(st.st_mode)) rmtree(tmppath, true); /* Create and fsync the temporary slot directory. */ - if (mkdir(tmppath, S_IRWXU) < 0) + if (MakeDirectoryDefaultPerm(tmppath) < 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not create directory \"%s\": %m", diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c index ca6342db0d..ff980dc1f5 100644 --- a/src/backend/storage/file/copydir.c +++ b/src/backend/storage/file/copydir.c @@ -41,7 +41,7 @@ copydir(char *fromdir, char *todir, bool recurse) char fromfile[MAXPGPATH * 2]; char tofile[MAXPGPATH * 2]; - if (mkdir(todir, S_IRWXU) != 0) + if (MakeDirectoryDefaultPerm(todir) != 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not create directory \"%s\": %m", todir))); diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 2a18e94ff4..1491665375 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -84,6 +84,7 @@ #include "access/xlog.h" #include "catalog/catalog.h" #include "catalog/pg_tablespace.h" +#include "common/file_perm.h" #include "pgstat.h" #include "portability/mem.h" #include "storage/fd.h" @@ -124,12 +125,6 @@ */ #define FD_MINFREE 10 -/* - * Default mode for created files, unless something else is specified using - * the *Perm() function variants. - */ -#define PG_FILE_MODE_DEFAULT (S_IRUSR | S_IWUSR) - /* * A number of platforms allow individual processes to open many more files * than they can really support when *many* processes do the same thing. @@ -1435,7 +1430,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode) void PathNameCreateTemporaryDir(const char *basedir, const char *directory) { - if (mkdir(directory, S_IRWXU) < 0) + if (MakeDirectoryDefaultPerm(directory) < 0) { if (errno == EEXIST) return; @@ -1445,14 +1440,14 @@ PathNameCreateTemporaryDir(const char *basedir, const char *directory) * EEXIST to close a race against another process following the same * algorithm. */ - if (mkdir(basedir, S_IRWXU) < 0 && errno != EEXIST) + if (MakeDirectoryDefaultPerm(basedir) < 0 && errno != EEXIST) ereport(ERROR, (errcode_for_file_access(), errmsg("cannot create temporary directory \"%s\": %m", basedir))); /* Try again. */ - if (mkdir(directory, S_IRWXU) < 0 && errno != EEXIST) + if (MakeDirectoryDefaultPerm(directory) < 0 && errno != EEXIST) ereport(ERROR, (errcode_for_file_access(), errmsg("cannot create temporary subdirectory \"%s\": %m", @@ -1602,11 +1597,11 @@ OpenTemporaryFileInTablespace(Oid tblspcOid, bool rejectError) * We might need to create the tablespace's tempfile directory, if no * one has yet done so. * - * Don't check for error from mkdir; it could fail if someone else - * just did the same thing. If it doesn't work then we'll bomb out on + * Don't check error from MakeDirectoryDefaultPerm; it could fail if someone + * else just did the same thing. If it doesn't work then we'll bomb out on * the second create attempt, instead. */ - mkdir(tempdirpath, S_IRWXU); + MakeDirectoryDefaultPerm(tempdirpath); file = PathNameOpenFile(tempfilepath, O_RDWR | O_CREAT | O_TRUNC | PG_BINARY); @@ -3555,3 +3550,16 @@ fsync_parent_path(const char *fname, int elevel) return 0; } + +/* + * Create a directory using PG_DIR_MODE_DEFAULT for permissions + * + * Directories in PGDATA should normally have specific permissions -- when + * using this function the caller does not need to know what they should be. + * For permissions other than the default use mkdir() directly. + */ +int +MakeDirectoryDefaultPerm(const char *directoryName) +{ + return mkdir(directoryName, PG_DIR_MODE_DEFAULT); +} diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c index 726db7b7f1..466613e4af 100644 --- a/src/backend/storage/ipc/ipc.c +++ b/src/backend/storage/ipc/ipc.c @@ -137,6 +137,10 @@ proc_exit(int code) else snprintf(gprofDirName, 32, "gprof/%d", (int) getpid()); + /* + * Use mkdir() instead of MakeDirectoryDefaultPerm() here because non-default + * permissions are required. + */ mkdir("gprof", S_IRWXU | S_IRWXG | S_IRWXO); mkdir(gprofDirName, S_IRWXU | S_IRWXG | S_IRWXO); chdir(gprofDirName); diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 2efd3b75b1..f8420af2a9 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -64,6 +64,7 @@ #include "catalog/pg_authid.h" #include "catalog/pg_class.h" #include "catalog/pg_collation.h" +#include "common/file_perm.h" #include "common/file_utils.h" #include "common/restricted_token.h" #include "common/username.h" @@ -144,6 +145,7 @@ static bool data_checksums = false; static char *xlog_dir = NULL; static char *str_wal_segment_size_mb = NULL; static int wal_segment_size_mb; +static mode_t file_mode_mask = PG_MODE_MASK_DEFAULT; /* internal vars */ @@ -1170,12 +1172,6 @@ setup_config(void) snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data); writefile(path, conflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) - { - fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), - progname, path, strerror(errno)); - exit_nicely(); - } /* * create the automatic configuration file to store the configuration @@ -1190,12 +1186,6 @@ setup_config(void) sprintf(path, "%s/postgresql.auto.conf", pg_data); writefile(path, autoconflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) - { - fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), - progname, path, strerror(errno)); - exit_nicely(); - } free(conflines); @@ -1277,12 +1267,6 @@ setup_config(void) snprintf(path, sizeof(path), "%s/pg_hba.conf", pg_data); writefile(path, conflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) - { - fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), - progname, path, strerror(errno)); - exit_nicely(); - } free(conflines); @@ -1293,12 +1277,6 @@ setup_config(void) snprintf(path, sizeof(path), "%s/pg_ident.conf", pg_data); writefile(path, conflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) - { - fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), - progname, path, strerror(errno)); - exit_nicely(); - } free(conflines); @@ -2692,7 +2670,7 @@ create_data_directory(void) pg_data); fflush(stdout); - if (pg_mkdir_p(pg_data, S_IRWXU) != 0) + if (pg_mkdir_p(pg_data, PG_DIR_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); @@ -2710,7 +2688,7 @@ create_data_directory(void) pg_data); fflush(stdout); - if (chmod(pg_data, S_IRWXU) != 0) + if (chmod(pg_data, PG_DIR_MODE_DEFAULT & ~file_mode_mask) != 0) { fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); @@ -2778,7 +2756,7 @@ create_xlog_or_symlink(void) xlog_dir); fflush(stdout); - if (pg_mkdir_p(xlog_dir, S_IRWXU) != 0) + if (pg_mkdir_p(xlog_dir, PG_DIR_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, xlog_dir, strerror(errno)); @@ -2796,7 +2774,7 @@ create_xlog_or_symlink(void) xlog_dir); fflush(stdout); - if (chmod(xlog_dir, S_IRWXU) != 0) + if (chmod(xlog_dir, PG_DIR_MODE_DEFAULT & ~file_mode_mask) != 0) { fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"), progname, xlog_dir, strerror(errno)); @@ -2846,7 +2824,7 @@ create_xlog_or_symlink(void) else { /* Without -X option, just make the subdirectory normally */ - if (mkdir(subdirloc, S_IRWXU) < 0) + if (mkdir(subdirloc, PG_DIR_MODE_DEFAULT) < 0) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, subdirloc, strerror(errno)); @@ -2882,7 +2860,8 @@ initialize_data_directory(void) setup_signals(); - umask(S_IRWXG | S_IRWXO); + /* Set the file mode creation mask */ + umask(file_mode_mask); create_data_directory(); @@ -2902,7 +2881,7 @@ initialize_data_directory(void) * The parent directory already exists, so we only need mkdir() not * pg_mkdir_p() here, which avoids some failure modes; cf bug #13853. */ - if (mkdir(path, S_IRWXU) < 0) + if (mkdir(path, PG_DIR_MODE_DEFAULT) < 0) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), progname, path, strerror(errno)); diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl index c0cfa6e92c..561608f1d8 100644 --- a/src/bin/initdb/t/001_initdb.pl +++ b/src/bin/initdb/t/001_initdb.pl @@ -6,7 +6,7 @@ use strict; use warnings; use PostgresNode; use TestLib; -use Test::More tests => 15; +use Test::More tests => 16; my $tempdir = TestLib::tempdir; my $xlogdir = "$tempdir/pgxlog"; @@ -45,6 +45,8 @@ mkdir $datadir; command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ], 'successful creation'); + + ok(check_pg_data_perm($datadir, 0)); } command_ok([ 'initdb', '-S', $datadir ], 'sync only'); command_fails([ 'initdb', $datadir ], 'existing data directory'); diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 9bc830b085..1eef7179cc 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -25,6 +25,7 @@ #include "catalog/pg_control.h" #include "common/controldata_utils.h" +#include "common/file_perm.h" #include "getopt_long.h" #include "utils/pidfile.h" @@ -2170,7 +2171,7 @@ main(int argc, char **argv) */ argv0 = argv[0]; - umask(S_IRWXG | S_IRWXO); + umask(PG_MODE_MASK_DEFAULT); /* support --help and --version even if invoked as root */ if (argc > 1) diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl index 5da4746cb4..909bf4d465 100644 --- a/src/bin/pg_ctl/t/001_start_stop.pl +++ b/src/bin/pg_ctl/t/001_start_stop.pl @@ -4,7 +4,7 @@ use warnings; use Config; use PostgresNode; use TestLib; -use Test::More tests => 19; +use Test::More tests => 20; my $tempdir = TestLib::tempdir; my $tempdir_short = TestLib::tempdir_short; @@ -57,10 +57,15 @@ command_ok([ 'pg_ctl', 'stop', '-D', "$tempdir/data" ], 'pg_ctl stop'); command_fails([ 'pg_ctl', 'stop', '-D', "$tempdir/data" ], 'second pg_ctl stop fails'); +# Log file for first perm test +my $logFileName = "$tempdir/data/perm-test-600.log"; + command_ok( - [ 'pg_ctl', 'restart', '-D', "$tempdir/data" ], + [ 'pg_ctl', 'restart', '-D', "$tempdir/data", '-l', $logFileName ], 'pg_ctl restart with server not running'); -command_ok([ 'pg_ctl', 'restart', '-D', "$tempdir/data" ], - 'pg_ctl restart with server running'); + +# Log file should exist and have no group permissions +ok(-f $logFileName); +ok(check_pg_data_perm("$tempdir/data", 0)); system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data"; diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index a132cf2e9f..47abd41fc3 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -52,6 +52,7 @@ #include "catalog/catversion.h" #include "catalog/pg_control.h" #include "common/fe_memutils.h" +#include "common/file_perm.h" #include "common/restricted_token.h" #include "storage/large_object.h" #include "pg_getopt.h" @@ -327,6 +328,9 @@ main(int argc, char *argv[]) exit(1); } + /* Set dir/file mode mask */ + umask(PG_MODE_MASK_DEFAULT); + /* Check that data directory matches our server version */ CheckDataVersion(); @@ -919,7 +923,7 @@ RewriteControlFile(void) fd = open(XLOG_CONTROL_FILE, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, - S_IRUSR | S_IWUSR); + PG_FILE_MODE_DEFAULT); if (fd < 0) { fprintf(stderr, _("%s: could not create pg_control file: %s\n"), @@ -1201,7 +1205,7 @@ WriteEmptyXLOG(void) unlink(path); fd = open(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, - S_IRUSR | S_IWUSR); + PG_FILE_MODE_DEFAULT); if (fd < 0) { fprintf(stderr, _("%s: could not open file \"%s\": %s\n"), diff --git a/src/bin/pg_resetwal/t/001_basic.pl b/src/bin/pg_resetwal/t/001_basic.pl index 234bd70303..db0c2a630a 100644 --- a/src/bin/pg_resetwal/t/001_basic.pl +++ b/src/bin/pg_resetwal/t/001_basic.pl @@ -4,7 +4,7 @@ use warnings; use Config; use PostgresNode; use TestLib; -use Test::More tests => 13; +use Test::More tests => 14; my $tempdir = TestLib::tempdir; my $tempdir_short = TestLib::tempdir_short; @@ -36,6 +36,8 @@ is_deeply( [sort(qw(. .. archive_status 000000010000000000000002))], 'WAL recreated'); +ok(check_pg_data_perm($pgdata, 0), 'check PGDATA permissions'); + $node->start; # Reset to specific WAL segment diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm index 00b5b42dd7..cbf4910ffc 100644 --- a/src/bin/pg_rewind/RewindTest.pm +++ b/src/bin/pg_rewind/RewindTest.pm @@ -237,6 +237,9 @@ sub run_pg_rewind "$tmp_folder/master-postgresql.conf.tmp", "$master_pgdata/postgresql.conf"); + chmod(0600, "$master_pgdata/postgresql.conf") + or die("unable to set permissions for $master_pgdata/postgresql.conf"); + # Plug-in rewound node to the now-promoted standby node my $port_standby = $node_standby->port; $node_master->append_conf( @@ -255,6 +258,8 @@ recovery_target_timeline='latest' # Clean up after the test. Stop both servers, if they're still running. sub clean_rewind_test { + ok (check_pg_data_perm($node_master->data_dir(), 0)); + $node_master->teardown_node if defined $node_master; $node_standby->teardown_node if defined $node_standby; } diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c index 705383d184..d51914e901 100644 --- a/src/bin/pg_rewind/file_ops.c +++ b/src/bin/pg_rewind/file_ops.c @@ -18,6 +18,7 @@ #include #include +#include "common/file_perm.h" #include "file_ops.h" #include "filemap.h" #include "logging.h" @@ -58,7 +59,7 @@ open_target_file(const char *path, bool trunc) mode = O_WRONLY | O_CREAT | PG_BINARY; if (trunc) mode |= O_TRUNC; - dstfd = open(dstpath, mode, 0600); + dstfd = open(dstpath, mode, PG_FILE_MODE_DEFAULT); if (dstfd < 0) pg_fatal("could not open target file \"%s\": %s\n", dstpath, strerror(errno)); @@ -190,7 +191,7 @@ truncate_target_file(const char *path, off_t newsize) snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); - fd = open(dstpath, O_WRONLY, 0); + fd = open(dstpath, O_WRONLY, PG_FILE_MODE_DEFAULT); if (fd < 0) pg_fatal("could not open file \"%s\" for truncation: %s\n", dstpath, strerror(errno)); @@ -211,7 +212,7 @@ create_target_dir(const char *path) return; snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path); - if (mkdir(dstpath, S_IRWXU) != 0) + if (mkdir(dstpath, PG_DIR_MODE_DEFAULT) != 0) pg_fatal("could not create directory \"%s\": %s\n", dstpath, strerror(errno)); } diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 72ab2f8d5e..3d179e2c7d 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -24,6 +24,7 @@ #include "access/xlog_internal.h" #include "catalog/catversion.h" #include "catalog/pg_control.h" +#include "common/file_perm.h" #include "common/restricted_token.h" #include "getopt_long.h" #include "storage/bufpage.h" @@ -185,6 +186,9 @@ main(int argc, char **argv) exit(1); } + /* Set dir/file mode mask */ + umask(PG_MODE_MASK_DEFAULT); + /* * Don't allow pg_rewind to be run as root, to avoid overwriting the * ownership of files in the data directory. We need only check for root diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl index 736f34eae3..e7fc200fc0 100644 --- a/src/bin/pg_rewind/t/001_basic.pl +++ b/src/bin/pg_rewind/t/001_basic.pl @@ -1,7 +1,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 8; +use Test::More tests => 10; use RewindTest; diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl index 37cdd712f3..0e0140b96f 100644 --- a/src/bin/pg_rewind/t/002_databases.pl +++ b/src/bin/pg_rewind/t/002_databases.pl @@ -1,7 +1,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 4; +use Test::More tests => 6; use RewindTest; diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl index 2433a4aab6..e8e8465c01 100644 --- a/src/bin/pg_rewind/t/003_extrafiles.pl +++ b/src/bin/pg_rewind/t/003_extrafiles.pl @@ -3,7 +3,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 4; +use Test::More tests => 6; use File::Find; @@ -14,6 +14,9 @@ sub run_test { my $test_mode = shift; + # Ensure that all files are created with default data dir permissions + umask(0077); + RewindTest::setup_cluster($test_mode); RewindTest::start_master(); diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl index feadaa6a0f..70dd061915 100644 --- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl +++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl @@ -14,7 +14,7 @@ if ($windows_os) } else { - plan tests => 4; + plan tests => 6; } use RewindTest; diff --git a/src/bin/pg_rewind/t/005_same_timeline.pl b/src/bin/pg_rewind/t/005_same_timeline.pl index 0e334ee191..2fbca4ad7c 100644 --- a/src/bin/pg_rewind/t/005_same_timeline.pl +++ b/src/bin/pg_rewind/t/005_same_timeline.pl @@ -1,7 +1,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 1; +use Test::More tests => 2; use RewindTest; diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c index f38bfacf02..595d43ee31 100644 --- a/src/bin/pg_upgrade/file.c +++ b/src/bin/pg_upgrade/file.c @@ -10,6 +10,7 @@ #include "postgres_fe.h" #include "access/visibilitymap.h" +#include "common/file_perm.h" #include "pg_upgrade.h" #include "storage/bufpage.h" #include "storage/checksum.h" @@ -44,7 +45,7 @@ copyFile(const char *src, const char *dst, schemaName, relName, src, strerror(errno)); if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, - S_IRUSR | S_IWUSR)) < 0) + PG_FILE_MODE_DEFAULT)) < 0) pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s\n", schemaName, relName, dst, strerror(errno)); @@ -151,7 +152,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile, schemaName, relName, fromfile, strerror(errno)); if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY, - S_IRUSR | S_IWUSR)) < 0) + PG_FILE_MODE_DEFAULT)) < 0) pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s\n", schemaName, relName, tofile, strerror(errno)); diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index d12412799f..59df6fd88f 100644 --- a/src/bin/pg_upgrade/pg_upgrade.c +++ b/src/bin/pg_upgrade/pg_upgrade.c @@ -38,6 +38,7 @@ #include "pg_upgrade.h" #include "catalog/pg_class.h" +#include "common/file_perm.h" #include "common/restricted_token.h" #include "fe_utils/string_utils.h" @@ -79,7 +80,7 @@ main(int argc, char **argv) set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_upgrade")); /* Ensure that all files created by pg_upgrade are non-world-readable */ - umask(S_IRWXG | S_IRWXO); + umask(PG_MODE_MASK_DEFAULT); parseCommandLine(argc, argv); diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh index 39983abea1..4e8db4aaf4 100644 --- a/src/bin/pg_upgrade/test.sh +++ b/src/bin/pg_upgrade/test.sh @@ -230,6 +230,17 @@ standard_initdb 'initdb' pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B "$bindir" -p "$PGPORT" -P "$PGPORT" +# make sure all directories and files have group permissions +if [ $(find ${PGDATA} -type f ! -perm 600 | wc -l) -ne 0 ]; then + echo "files in PGDATA with permission != 600"; + exit 1; +fi + +if [ $(find ${PGDATA} -type d ! -perm 700 | wc -l) -ne 0 ]; then + echo "directories in PGDATA with permission != 700"; + exit 1; +fi + pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w case $testhost in diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h new file mode 100644 index 0000000000..3c6da0e000 --- /dev/null +++ b/src/include/common/file_perm.h @@ -0,0 +1,32 @@ +/*------------------------------------------------------------------------- + * + * File and directory permission constants + * + * + * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/common/file_perm.h + * + *------------------------------------------------------------------------- + */ +#ifndef FILE_PERM_H +#define FILE_PERM_H + +/* + * Default mode mask for data directory permissions that does not allow + * group execute/read. + */ +#define PG_MODE_MASK_DEFAULT (S_IRWXG | S_IRWXO) + +/* + * Default mode for created files. + */ +#define PG_FILE_MODE_DEFAULT (S_IRUSR | S_IWUSR) + +/* + * Default mode for directories. + */ +#define PG_DIR_MODE_DEFAULT S_IRWXU + +#endif /* FILE_PERM_H */ diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 4244e7b1fd..33c636471b 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -112,6 +112,9 @@ extern int CloseTransientFile(int fd); extern int BasicOpenFile(const char *fileName, int fileFlags); extern int BasicOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode); + /* Make a directory with default permissions */ +extern int MakeDirectoryDefaultPerm(const char *directoryName); + /* Miscellaneous support routines */ extern void InitFileAccess(void); extern void set_max_safe_fds(void); diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 80188315f1..76e571b98c 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -484,6 +484,9 @@ sub append_conf my $conffile = $self->data_dir . '/' . $filename; TestLib::append_to_file($conffile, $str . "\n"); + + chmod(0600, $conffile) + or die("unable to set permissions for $conffile"); } =pod diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index fdd427608b..4a1f50cc1f 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -12,8 +12,11 @@ use warnings; use Config; use Exporter 'import'; +use Fcntl qw(:mode); use File::Basename; +use File::Find; use File::Spec; +use File::stat qw(stat); use File::Temp (); use IPC::Run; use SimpleTee; @@ -26,6 +29,7 @@ our @EXPORT = qw( slurp_dir slurp_file append_to_file + check_pg_data_perm check_pg_config system_or_bail system_log @@ -222,6 +226,84 @@ sub append_to_file close $fh; } +# Ensure all permissions in the pg_data directory are correct. Permissions +# should be dir = 0700, file = 0600. +sub check_pg_data_perm +{ + my ($pgdata) = @_; + + # Result defaults to true + my $result = 1; + + # Expected permission + my $expected_file_perm = 0600; + my $expected_dir_perm = 0700; + + find + ( + {follow_fast => 1, + wanted => + sub + { + my $file_stat = stat($File::Find::name); + + defined($file_stat) + or die("unable to stat $File::Find::name"); + + my $file_mode = S_IMODE($file_stat->mode); + + # Is this a file? + if (S_ISREG($file_stat->mode)) + { + # postmaster.pid file must be 600 + if ($File::Find::name =~ /\/postmaster\.pid$/) + { + if ($file_mode != 0600) + { + print(*STDERR, "$File::Find::name mode must be 0600\n"); + + $result = 0; + return + } + } + else + { + if ($file_mode != $expected_file_perm) + { + print(*STDERR, + sprintf("$File::Find::name mode must be %04o\n", + $expected_file_perm)); + + $result = 0; + return + } + } + } + # Else a directory? + elsif (S_ISDIR($file_stat->mode)) + { + if ($file_mode != $expected_dir_perm) + { + print(*STDERR, + sprintf("$File::Find::name mode must be %04o\n", + $expected_dir_perm)); + + $result = 0; + return + } + } + # Else something we can't handle + else + { + die "unknown file type for $File::Find::name"; + } + }}, + $pgdata + ); + + return $result; +} + # Check presence of a given regexp within pg_config.h for the installation # where tests are running, returning a match status result depending on # that. -- 2.14.3 (Apple Git-98)