group-access-v7-02-file-perm.patch

text/plain

Filename: group-access-v7-02-file-perm.patch
Type: text/plain
Part: 1
Message: Re: PATCH: Configurable file mode mask

Patch

Format: unified
Series: patch v7
File+
src/backend/access/transam/xlog.c 1 1
src/backend/commands/tablespace.c 14 8
src/backend/postmaster/postmaster.c 1 1
src/backend/postmaster/syslogger.c 3 2
src/backend/replication/slot.c 3 2
src/backend/storage/file/copydir.c 1 1
src/backend/storage/file/fd.c 20 12
src/backend/storage/ipc/ipc.c 4 0
src/bin/initdb/initdb.c 10 32
src/bin/initdb/t/001_initdb.pl 3 1
src/bin/pg_ctl/pg_ctl.c 2 1
src/bin/pg_ctl/t/001_start_stop.pl 9 4
src/bin/pg_resetwal/pg_resetwal.c 6 2
src/bin/pg_resetwal/t/001_basic.pl 3 1
src/bin/pg_rewind/file_ops.c 4 3
src/bin/pg_rewind/pg_rewind.c 4 0
src/bin/pg_rewind/RewindTest.pm 5 0
src/bin/pg_rewind/t/001_basic.pl 1 1
src/bin/pg_rewind/t/002_databases.pl 1 1
src/bin/pg_rewind/t/003_extrafiles.pl 3 1
src/bin/pg_rewind/t/004_pg_xlog_symlink.pl 1 1
src/bin/pg_rewind/t/005_same_timeline.pl 1 1
src/bin/pg_upgrade/file.c 3 2
src/bin/pg_upgrade/pg_upgrade.c 2 1
src/bin/pg_upgrade/test.sh 11 0
src/bin/pg_upgrade/version.c 4 4
src/include/common/file_perm.h 32 0
src/include/storage/fd.h 3 0
src/test/perl/PostgresNode.pm 3 0
src/test/perl/TestLib.pm 81 0
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/<dboid>/<relid>_<fork>.<nnn>'.  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..4ef06c2349 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,8 +2860,6 @@ initialize_data_directory(void)
 
 	setup_signals();
 
-	umask(S_IRWXG | S_IRWXO);
-
 	create_data_directory();
 
 	create_xlog_or_symlink();
@@ -2902,7 +2878,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));
@@ -3159,6 +3135,8 @@ main(int argc, char *argv[])
 		}
 	}
 
+	/* Set the file mode creation mask */
+	umask(file_mode_mask);
 
 	/*
 	 * Non-option argument specifies data directory as long as it wasn't
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..b18a7f954c 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 <fcntl.h>
 #include <unistd.h>
 
+#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..28933da343 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,8 @@ sub run_test
 {
 	my $test_mode = shift;
 
+	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/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 76e9d65537..205f772fc5 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -53,7 +53,7 @@ new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster, bool check_mode)
 			{
 				PQExpBufferData connectbuf;
 
-				if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				if (script == NULL && (script = fopen(output_path, "w")) == NULL)
 					pg_fatal("could not open file \"%s\": %s\n", output_path,
 							 strerror(errno));
 
@@ -152,7 +152,7 @@ old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster)
 		for (rowno = 0; rowno < ntups; rowno++)
 		{
 			found = true;
-			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+			if (script == NULL && (script = fopen(output_path, "w")) == NULL)
 				pg_fatal("could not open file \"%s\": %s\n", output_path,
 						 strerror(errno));
 			if (!db_used)
@@ -253,7 +253,7 @@ old_9_6_check_for_unknown_data_type_usage(ClusterInfo *cluster)
 		for (rowno = 0; rowno < ntups; rowno++)
 		{
 			found = true;
-			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+			if (script == NULL && (script = fopen(output_path, "w")) == NULL)
 				pg_fatal("could not open file \"%s\": %s\n", output_path,
 						 strerror(errno));
 			if (!db_used)
@@ -335,7 +335,7 @@ old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool check_mode)
 			found = true;
 			if (!check_mode)
 			{
-				if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				if (script == NULL && (script = fopen(output_path, "w")) == NULL)
 					pg_fatal("could not open file \"%s\": %s\n", output_path,
 							 strerror(errno));
 				if (!db_used)
diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h
new file mode 100644
index 0000000000..5b823cbd39
--- /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 | S_IRGRP)
+
+/*
+ * Default mode for directories.
+ */
+#define PG_DIR_MODE_DEFAULT			(S_IRWXU | S_IRGRP | S_IXGRP)
+
+#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..acccecc5de 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,83 @@ sub append_to_file
 	close $fh;
 }
 
+# Ensure all permissions in the pg_data directory are correct.  When allow_group
+# is true then permissions should be dir = 0750, file = 0640.  When allow_group
+# is false then permissions should be dir = 0700, file = 0600.
+sub check_pg_data_perm
+{
+    my ($pgdata, $allow_group) = @_;
+
+	# Result defaults to true
+	my $result = 1;
+
+	# Expected permission
+	my $expected_file_perm = $allow_group ? 0640 : 0600;
+	my $expected_dir_perm = $allow_group ? 0750 : 0700;
+
+	find
+	(
+		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.