From 60752c2e12fa2132c24d71ff030cefe4b2b6c502 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 27 Mar 2018 15:47:15 -0400 Subject: [PATCH 1/2] 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 | 18 ++++--- src/backend/postmaster/postmaster.c | 5 +- src/backend/postmaster/syslogger.c | 5 +- src/backend/replication/basebackup.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/backend/utils/init/miscinit.c | 5 +- src/bin/initdb/initdb.c | 24 ++++----- src/bin/initdb/t/001_initdb.pl | 4 +- src/bin/pg_basebackup/pg_basebackup.c | 7 +-- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 9 +++- src/bin/pg_basebackup/t/020_pg_receivewal.pl | 9 +++- src/bin/pg_basebackup/walmethods.c | 6 ++- src/bin/pg_ctl/pg_ctl.c | 4 +- src/bin/pg_ctl/t/001_start_stop.pl | 13 +++-- src/bin/pg_dump/pg_backup_directory.c | 3 +- src/bin/pg_resetwal/pg_resetwal.c | 8 ++- src/bin/pg_resetwal/t/001_basic.pl | 5 +- src/bin/pg_rewind/RewindTest.pm | 4 ++ src/bin/pg_rewind/file_ops.c | 7 +-- src/bin/pg_rewind/pg_rewind.c | 4 ++ src/bin/pg_rewind/t/001_basic.pl | 3 +- 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 | 73 ++++++++++++++++++++++++++++ 32 files changed, 256 insertions(+), 67 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 cb9c2a29cb..95c0fda4e3 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..f7e1818350 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) @@ -574,7 +576,7 @@ 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) + if (chmod(location, PG_DIR_MODE_DEFAULT) != 0) { if (errno == ENOENT) ereport(ERROR, @@ -599,7 +601,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 +612,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 660f3185e6..868fba8cac 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -97,6 +97,7 @@ #include "access/xlog.h" #include "bootstrap/bootstrap.h" #include "catalog/pg_control.h" +#include "common/file_perm.h" #include "common/ip.h" #include "lib/ilist.h" #include "libpq/auth.h" @@ -589,7 +590,7 @@ PostmasterMain(int argc, char *argv[]) /* * for security, no dir or file created can be group or other accessible */ - umask(S_IRWXG | S_IRWXO); + umask(PG_MODE_MASK_DEFAULT); /* * Initialize random(3) so we don't get the same values in every run. @@ -4492,7 +4493,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/basebackup.c b/src/backend/replication/basebackup.c index 654d0832da..e4a80edb8a 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -19,6 +19,7 @@ #include "access/xlog_internal.h" /* for pg_start/stop_backup */ #include "catalog/catalog.h" #include "catalog/pg_type.h" +#include "common/file_perm.h" #include "lib/stringinfo.h" #include "libpq/libpq.h" #include "libpq/pqformat.h" @@ -875,7 +876,7 @@ sendFileWithContent(const char *filename, const char *content) statbuf.st_gid = getegid(); #endif statbuf.st_mtime = time(NULL); - statbuf.st_mode = S_IRUSR | S_IWUSR; + statbuf.st_mode = PG_FILE_MODE_DEFAULT; statbuf.st_size = len; _tarWriteHeader(filename, NULL, &statbuf, false); @@ -1394,7 +1395,7 @@ _tarWriteDir(const char *pathbuf, int basepathlen, struct stat *statbuf, #else if (pgwin32_is_junction(pathbuf)) #endif - statbuf->st_mode = S_IFDIR | S_IRWXU; + statbuf->st_mode = S_IFDIR | PG_DIR_MODE_DEFAULT; return _tarWriteHeader(pathbuf + basepathlen + 1, NULL, statbuf, sizeonly); } 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 d30a725f90..40f754afb6 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. @@ -1434,7 +1429,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; @@ -1444,14 +1439,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", @@ -1601,11 +1596,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); @@ -3554,3 +3549,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/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 87ed7d3f71..82da4b9f27 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -32,6 +32,7 @@ #include "access/htup_details.h" #include "catalog/pg_authid.h" +#include "common/file_perm.h" #include "libpq/libpq.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -831,7 +832,7 @@ CreateLockFile(const char *filename, bool amPostmaster, * Think not to make the file protection weaker than 0600. See * comments below. */ - fd = open(filename, O_RDWR | O_CREAT | O_EXCL, 0600); + fd = open(filename, O_RDWR | O_CREAT | O_EXCL, PG_FILE_MODE_DEFAULT); if (fd >= 0) break; /* Success; exit the retry loop */ @@ -848,7 +849,7 @@ CreateLockFile(const char *filename, bool amPostmaster, * Read the file to get the old owner's PID. Note race condition * here: file might have been deleted since we tried to create it. */ - fd = open(filename, O_RDONLY, 0600); + fd = open(filename, O_RDONLY, PG_FILE_MODE_DEFAULT); if (fd < 0) { if (errno == ENOENT) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 78990f5a27..0b69eded81 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" @@ -1170,7 +1171,7 @@ setup_config(void) snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data); writefile(path, conflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) + if (chmod(path, PG_FILE_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); @@ -1190,7 +1191,7 @@ setup_config(void) sprintf(path, "%s/postgresql.auto.conf", pg_data); writefile(path, autoconflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) + if (chmod(path, PG_FILE_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); @@ -1277,7 +1278,7 @@ setup_config(void) snprintf(path, sizeof(path), "%s/pg_hba.conf", pg_data); writefile(path, conflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) + if (chmod(path, PG_FILE_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); @@ -1293,7 +1294,7 @@ setup_config(void) snprintf(path, sizeof(path), "%s/pg_ident.conf", pg_data); writefile(path, conflines); - if (chmod(path, S_IRUSR | S_IWUSR) != 0) + if (chmod(path, PG_FILE_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), progname, path, strerror(errno)); @@ -2692,7 +2693,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 +2711,7 @@ create_data_directory(void) pg_data); fflush(stdout); - if (chmod(pg_data, S_IRWXU) != 0) + if (chmod(pg_data, PG_DIR_MODE_DEFAULT) != 0) { fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"), progname, pg_data, strerror(errno)); @@ -2778,7 +2779,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 +2797,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) != 0) { fprintf(stderr, _("%s: could not change permissions of directory \"%s\": %s\n"), progname, xlog_dir, strerror(errno)); @@ -2846,7 +2847,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 +2883,8 @@ initialize_data_directory(void) setup_signals(); - umask(S_IRWXG | S_IRWXO); + /* Set dir/file mode mask */ + umask(PG_MODE_MASK_DEFAULT); create_data_directory(); @@ -2902,7 +2904,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..3331560445 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_mode_recursive($datadir, 0700, 0600)); } command_ok([ 'initdb', '-S', $datadir ], 'sync only'); command_fails([ 'initdb', $datadir ], 'existing data directory'); diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 1b32592063..3d6e47008a 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -27,6 +27,7 @@ #endif #include "access/xlog_internal.h" +#include "common/file_perm.h" #include "common/file_utils.h" #include "common/string.h" #include "fe_utils/string_utils.h" @@ -624,7 +625,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal"); - if (pg_mkdir_p(statusdir, S_IRWXU) != 0 && errno != EEXIST) + if (pg_mkdir_p(statusdir, PG_DIR_MODE_DEFAULT) != 0 && errno != EEXIST) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), @@ -680,7 +681,7 @@ verify_dir_is_empty_or_create(char *dirname, bool *created, bool *found) /* * Does not exist, so create */ - if (pg_mkdir_p(dirname, S_IRWXU) == -1) + if (pg_mkdir_p(dirname, PG_DIR_MODE_DEFAULT) == -1) { fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"), @@ -1436,7 +1437,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum) * Directory */ filename[strlen(filename) - 1] = '\0'; /* Remove trailing slash */ - if (mkdir(filename, S_IRWXU) != 0) + if (mkdir(filename, PG_DIR_MODE_DEFAULT) != 0) { /* * When streaming WAL, pg_wal (or pg_xlog for pre-9.6 diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 32d21ce644..8d9f9c21f4 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -5,7 +5,7 @@ use Config; use File::Basename qw(basename dirname); use PostgresNode; use TestLib; -use Test::More tests => 93; +use Test::More tests => 94; program_help_ok('pg_basebackup'); program_version_ok('pg_basebackup'); @@ -15,6 +15,9 @@ my $tempdir = TestLib::tempdir; my $node = get_new_node('main'); +# Set umask so no test directories or files are created with group permissions +umask (0077); + # Initialize node without replication settings $node->init; $node->start; @@ -93,6 +96,10 @@ $node->command_ok([ 'pg_basebackup', '-D', "$tempdir/backup", '-X', 'none' ], 'pg_basebackup runs'); ok(-f "$tempdir/backup/PG_VERSION", 'backup was created'); +# Group access should not be enabled on backup +ok(check_mode_recursive("$tempdir/backup", 0700, 0600), + "check backup dir permissions"); + # Only archive_status directory should be copied in pg_wal/. is_deeply( [ sort(slurp_dir("$tempdir/backup/pg_wal/")) ], diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl index 64e3a35a87..d155d5b711 100644 --- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl +++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl @@ -2,12 +2,15 @@ use strict; use warnings; use TestLib; use PostgresNode; -use Test::More tests => 18; +use Test::More tests => 19; program_help_ok('pg_receivewal'); program_version_ok('pg_receivewal'); program_options_handling_ok('pg_receivewal'); +# Set umask so no test directories or files are created with group permissions +umask (0077); + my $primary = get_new_node('primary'); $primary->init(allows_streaming => 1); $primary->start; @@ -56,3 +59,7 @@ $primary->command_ok( [ 'pg_receivewal', '-D', $stream_dir, '--verbose', '--endpos', $nextlsn, '--synchronous', '--no-loop' ], 'streaming some WAL with --synchronous'); + +# Group access should not be enabled on WAL files +ok(check_mode_recursive($stream_dir, 0700, 0600), + "check stream dir permissions"); diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c index b4558a0184..aa3bb35d92 100644 --- a/src/bin/pg_basebackup/walmethods.c +++ b/src/bin/pg_basebackup/walmethods.c @@ -22,6 +22,7 @@ #endif #include "pgtar.h" +#include "common/file_perm.h" #include "common/file_utils.h" #include "receivelog.h" @@ -89,7 +90,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_ * does not do any system calls to fsync() to make changes permanent on * disk. */ - fd = open(tmppath, O_WRONLY | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR); + fd = open(tmppath, O_WRONLY | O_CREAT | PG_BINARY, PG_FILE_MODE_DEFAULT); if (fd < 0) return NULL; @@ -534,7 +535,8 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_ * We open the tar file only when we first try to write to it. */ tar_data->fd = open(tar_data->tarfilename, - O_WRONLY | O_CREAT | PG_BINARY, S_IRUSR | S_IWUSR); + O_WRONLY | O_CREAT | PG_BINARY, + PG_FILE_MODE_DEFAULT); if (tar_data->fd < 0) return NULL; diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 9bc830b085..e83a7179ea 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,8 @@ main(int argc, char **argv) */ argv0 = argv[0]; - umask(S_IRWXG | S_IRWXO); + /* Set dir/file mode mask */ + 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..3d82abe696 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_mode_recursive("$tempdir/data", 0700, 0600)); system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data"; diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c index 4aabb40f59..6fcd72a0f2 100644 --- a/src/bin/pg_dump/pg_backup_directory.c +++ b/src/bin/pg_dump/pg_backup_directory.c @@ -37,6 +37,7 @@ #include "compress_io.h" #include "parallel.h" #include "pg_backup_utils.h" +#include "common/file_perm.h" #include "common/file_utils.h" #include @@ -192,7 +193,7 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) } } - if (!is_empty && mkdir(ctx->directory, 0700) < 0) + if (!is_empty && mkdir(ctx->directory, PG_DIR_MODE_DEFAULT) < 0) exit_horribly(modulename, "could not create directory \"%s\": %s\n", ctx->directory, strerror(errno)); } diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index eba7c5fdee..49670a0d29 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" @@ -362,6 +363,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(); @@ -967,7 +971,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"), @@ -1249,7 +1253,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 1b157cb555..7a68a00638 100644 --- a/src/bin/pg_resetwal/t/001_basic.pl +++ b/src/bin/pg_resetwal/t/001_basic.pl @@ -3,7 +3,7 @@ use warnings; use PostgresNode; use TestLib; -use Test::More tests => 11; +use Test::More tests => 12; program_help_ok('pg_resetwal'); program_version_ok('pg_resetwal'); @@ -15,3 +15,6 @@ $node->init; command_like([ 'pg_resetwal', '-n', $node->data_dir ], qr/checkpoint/, 'pg_resetwal -n produces output'); + +ok(check_mode_recursive( + $node->data_dir, 0700, 0600), 'check PGDATA permissions'); diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm index 00b5b42dd7..7b632c7dcd 100644 --- a/src/bin/pg_rewind/RewindTest.pm +++ b/src/bin/pg_rewind/RewindTest.pm @@ -237,6 +237,10 @@ sub run_pg_rewind "$tmp_folder/master-postgresql.conf.tmp", "$master_pgdata/postgresql.conf"); + chmod(0600, "$master_pgdata/postgresql.conf") + or BAIL_OUT( + "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( 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..559dc53602 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; @@ -86,6 +86,7 @@ in master, before promotion ), 'tail-copy'); + ok (check_mode_recursive($node_master->data_dir(), 0700, 0600)); RewindTest::clean_rewind_test(); } 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 e49b42ce86..340dbd4d3b 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 b6862688d4..93610e4bc4 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -13,8 +13,11 @@ use warnings; use Config; use Cwd; 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; @@ -27,6 +30,7 @@ our @EXPORT = qw( slurp_dir slurp_file append_to_file + check_mode_recursive check_pg_config system_or_bail system_log @@ -240,6 +244,75 @@ sub append_to_file close $fh; } +# Check that all file/dir modes in a directory match the expected values, +# ignoring the mode of any specified files. +sub check_mode_recursive +{ + my ($dir, $expected_dir_mode, $expected_file_mode, $ignore_list) = @_; + + # Result defaults to true + my $result = 1; + + find + ( + {follow_fast => 1, + wanted => + sub + { + my $file_stat = stat($File::Find::name); + + # Is file in the ignore list? + foreach my $ignore ($ignore_list ? @{$ignore_list} : []) + { + if ("$dir/$ignore" eq $File::Find::name) + { + return; + } + } + + 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)) + { + if ($file_mode != $expected_file_mode) + { + print(*STDERR, + sprintf("$File::Find::name mode must be %04o\n", + $expected_file_mode)); + + $result = 0; + return; + } + } + # Else a directory? + elsif (S_ISDIR($file_stat->mode)) + { + if ($file_mode != $expected_dir_mode) + { + print(*STDERR, + sprintf("$File::Find::name mode must be %04o\n", + $expected_dir_mode)); + + $result = 0; + return; + } + } + # Else something we can't handle + else + { + die "unknown file type for $File::Find::name"; + } + }}, + $dir + ); + + 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)