Re: PATCH: Configurable file mode mask

Sat, 07 Apr 2018 10:10:04 -0700 

Hi Stephen,

On 4/6/18 10:22 PM, Stephen Frost wrote:
> 
> * David Steele (da...@pgmasters.net) wrote:
>> On 4/6/18 6:04 PM, David Steele wrote:
>>> On 4/6/18 3:02 PM, Stephen Frost wrote:
>>>>
>>>> - Further discussion in the commit messages
>>>
>>> Agreed, these need some more work.  I'm happy to do that but I'll need a
>>> bit more time.  Have a look at the new patches and I'll work on some
>>> better messages.
>>
>> I'm sure you'll want to reword some things, but I think these commit
>> messages capture the essential changes for each patch.
> 
> Thanks much.  I've taken (most) of these, adjusting a few bits here and
> there.
> 
> I've been back over the patch again, mostly improving the commit
> messages, comments, and docs.  I also looked over the code and tests
> again and they're looking pretty good to me, so I'll be looking to
> commit this tomorrow afternoon or so US/Eastern.

OK, one last review.  I did't make any code changes, but I improved some
comments, added documentation and fixed a test.

01) Refactor dir/file permissions

* Small comment update, replace "such cases-" with "such cases --".

02) Allow group access on PGDATA

* Add data_directory_mode guc to "Preset Options" documentation.
* Add a section to the documentation that discusses changing the
permissions of an existing cluster.
* Improved comment on checkControlFile().
* Comment wordsmithing for SetDataDirectoryCreatePerm() and
RetrieveDataDirCreatePerm().
* Fixed ordering of -g option in initdb documentation.
* Fixed a new test that was "broken" by 032429701e477.  It was broken
before but the rmtrees added in 032429701e477 exposed the problem.

Attached are the v19 patches.

Thanks!
-- 
-David
da...@pgmasters.net

From d97495e81bb59beb03941398d7746a22b55d48ec Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Sat, 7 Apr 2018 09:49:15 -0400
Subject: [PATCH 1/2] Refactor dir/file permissions

Consolidate directory and file create permissions for tools which work
with the PG data directory by adding a new module (common/file_perm.c)
that contains variables (pg_file_create_mode, pg_dir_create_mode) and
constants to initialize them (0600 for files and 0700 for directories).

Convert mkdir() calls in the backend to MakePGDirectory() if the
original call used default permissions (always the case for regular PG
directories).

Add tests to make sure permissions in PGDATA are set correctly by the
tools which modify the PG data directory.

Authors: David Steele <da...@pgmasters.net>,
         Adam Brightwell <adam.brightw...@crunchydata.com>
Reviewed-By: Michael Paquier, with discussion amongst many others.
Discussion: 
https://postgr.es/m/ad346fe6-b23e-59f1-ecb7-0e08390ad629%40pgmasters.net
---
 src/backend/access/transam/xlog.c            |  2 +-
 src/backend/commands/tablespace.c            | 18 ++++---
 src/backend/postmaster/postmaster.c          |  7 +--
 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                | 51 +++++++++++++------
 src/backend/storage/ipc/dsm_impl.c           |  3 +-
 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               | 11 ++++-
 src/bin/pg_basebackup/pg_basebackup.c        |  9 ++--
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 14 +++++-
 src/bin/pg_basebackup/t/020_pg_receivewal.pl | 14 +++++-
 src/bin/pg_basebackup/walmethods.c           |  6 ++-
 src/bin/pg_ctl/pg_ctl.c                      |  4 +-
 src/bin/pg_ctl/t/001_start_stop.pl           | 18 ++++++-
 src/bin/pg_resetwal/pg_resetwal.c            |  5 +-
 src/bin/pg_resetwal/t/001_basic.pl           | 12 ++++-
 src/bin/pg_rewind/RewindTest.pm              |  4 ++
 src/bin/pg_rewind/file_ops.c                 |  7 +--
 src/bin/pg_rewind/t/001_basic.pl             | 11 ++++-
 src/bin/pg_upgrade/file.c                    |  5 +-
 src/bin/pg_upgrade/pg_upgrade.c              |  3 +-
 src/bin/pg_upgrade/test.sh                   | 11 +++++
 src/common/Makefile                          |  4 +-
 src/common/file_perm.c                       | 19 ++++++++
 src/include/common/file_perm.h               | 34 +++++++++++++
 src/include/storage/fd.h                     |  3 ++
 src/test/perl/PostgresNode.pm                |  3 ++
 src/test/perl/TestLib.pm                     | 73 ++++++++++++++++++++++++++++
 src/tools/msvc/Mkvcbuild.pm                  |  4 +-
 34 files changed, 330 insertions(+), 75 deletions(-)
 create mode 100644 src/common/file_perm.c
 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 813b2afaac..4a47395174 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4107,7 +4107,7 @@ ValidateXLOGDirectoryStructure(void)
        {
                ereport(LOG,
                                (errmsg("creating missing WAL directory 
\"%s\"", path)));
-               if (mkdir(path, S_IRWXU) < 0)
+               if (MakePGDirectory(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..cfc9fe468c 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 (MakePGDirectory(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 (MakePGDirectory(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 (MakePGDirectory(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 (MakePGDirectory(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 MakePGDirectory() 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_create_mode) != 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, MakePGDirectory() 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 (MakePGDirectory(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 3dfb87d701..10afecffb3 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_OWNER);
 
        /*
         * Initialize random(3) so we don't get the same values in every run.
@@ -4490,9 +4491,9 @@ internal_forkexec(int argc, char *argv[], Port *port)
        {
                /*
                 * As in OpenTemporaryFileInTablespace, try to make the 
temp-file
-                * directory
+                * directory, ignoring errors.
                 */
-               mkdir(PG_TEMP_FILES_DIR, S_IRWXU);
+               (void) MakePGDirectory(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..58b759f305 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);
+                               (void) MakePGDirectory(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);
+       (void) MakePGDirectory(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 8ba29453b9..babf85a6ea 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"
@@ -930,7 +931,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_create_mode;
        statbuf.st_size = len;
 
        _tarWriteHeader(filename, NULL, &statbuf, false);
@@ -1628,7 +1629,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_create_mode;
 
        return _tarWriteHeader(pathbuf + basepathlen + 1, NULL, statbuf, 
sizeonly);
 }
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index fc9ef22b0b..056628fe8e 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 MakePGDirectory() 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 (MakePGDirectory(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..4a0d23b11e 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 (MakePGDirectory(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..36eea9d11d 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.
@@ -937,7 +932,7 @@ set_max_safe_fds(void)
 int
 BasicOpenFile(const char *fileName, int fileFlags)
 {
-       return BasicOpenFilePerm(fileName, fileFlags, PG_FILE_MODE_DEFAULT);
+       return BasicOpenFilePerm(fileName, fileFlags, pg_file_create_mode);
 }
 
 /*
@@ -1356,7 +1351,7 @@ FileInvalidate(File file)
 File
 PathNameOpenFile(const char *fileName, int fileFlags)
 {
-       return PathNameOpenFilePerm(fileName, fileFlags, PG_FILE_MODE_DEFAULT);
+       return PathNameOpenFilePerm(fileName, fileFlags, pg_file_create_mode);
 }
 
 /*
@@ -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 (MakePGDirectory(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 (MakePGDirectory(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 (MakePGDirectory(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
-                * the second create attempt, instead.
+                * Don't check for an error from MakePGDirectory; 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);
+               (void) MakePGDirectory(tempdirpath);
 
                file = PathNameOpenFile(tempfilepath,
                                                                O_RDWR | 
O_CREAT | O_TRUNC | PG_BINARY);
@@ -2401,7 +2396,7 @@ TryAgain:
 int
 OpenTransientFile(const char *fileName, int fileFlags)
 {
-       return OpenTransientFilePerm(fileName, fileFlags, PG_FILE_MODE_DEFAULT);
+       return OpenTransientFilePerm(fileName, fileFlags, pg_file_create_mode);
 }
 
 /*
@@ -3554,3 +3549,27 @@ fsync_parent_path(const char *fname, int elevel)
 
        return 0;
 }
+
+/*
+ * Create a PostgreSQL data sub-directory
+ *
+ * The data directory itself, along with most other directories, are created at
+ * initdb-time, but we do have some occations where we create directories from
+ * the backend (CREATE TABLESPACE, for example).  In those cases, we want to
+ * make sure that those directories are created consistently.  Today, that 
means
+ * making sure that the created directory has the correct permissions, which is
+ * what pg_dir_create_mode tracks for us.
+ *
+ * Note that we also set the umask() based on what we understand the correct
+ * permissions to be (see file_perm.c).
+ *
+ * For permissions other than the default mkdir() can be used directly, but be
+ * sure to consider carefully such cases -- a directory with incorrect
+ * permissions in a PostgreSQL data directory could cause backups and other
+ * processes to fail.
+ */
+int
+MakePGDirectory(const char *directoryName)
+{
+       return mkdir(directoryName, pg_dir_create_mode);
+}
diff --git a/src/backend/storage/ipc/dsm_impl.c 
b/src/backend/storage/ipc/dsm_impl.c
index 67e76b98fe..2fca9fae51 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -60,6 +60,7 @@
 #ifdef HAVE_SYS_SHM_H
 #include <sys/shm.h>
 #endif
+#include "common/file_perm.h"
 #include "pgstat.h"
 
 #include "portability/mem.h"
@@ -285,7 +286,7 @@ dsm_impl_posix(dsm_op op, dsm_handle handle, Size 
request_size,
         * returning.
         */
        flags = O_RDWR | (op == DSM_OP_CREATE ? O_CREAT | O_EXCL : 0);
-       if ((fd = shm_open(name, flags, 0600)) == -1)
+       if ((fd = shm_open(name, flags, PG_FILE_MODE_OWNER)) == -1)
        {
                if (errno != EEXIST)
                        ereport(elevel,
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index fc0a9c0756..53f7c1e77e 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 MakePGDirectory() since we aren't 
making a
+                * PG directory here.
+                */
                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..f8f08f3f88 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_create_mode);
                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_create_mode);
                if (fd < 0)
                {
                        if (errno == ENOENT)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 78990f5a27..3765548a24 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_create_mode) != 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_create_mode) != 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_create_mode) != 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_create_mode) != 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_create_mode) != 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_create_mode) != 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_create_mode) != 
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_create_mode) != 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_create_mode) < 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_OWNER);
 
        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_create_mode) < 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..9dfb2ed96f 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,15 @@ mkdir $datadir;
 
        command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ],
                'successful creation');
+
+       # Permissions on PGDATA should be default
+       SKIP:
+       {
+               skip "unix-style permissions not supported on Windows", 1 if 
($windows_os);
+
+               ok(check_mode_recursive($datadir, 0700, 0600),
+                  "check PGDATA permissions");
+       }
 }
 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 8610fe8a1a..32b41e313c 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"
@@ -629,7 +630,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_create_mode) != 0 && errno != 
EEXIST)
                {
                        fprintf(stderr,
                                        _("%s: could not create directory 
\"%s\": %s\n"),
@@ -685,7 +686,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_create_mode) == -1)
                        {
                                fprintf(stderr,
                                                _("%s: could not create 
directory \"%s\": %s\n"),
@@ -1129,7 +1130,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 
                                tarCreateHeader(header, "recovery.conf", NULL,
                                                                
recoveryconfcontents->len,
-                                                               0600, 04000, 
02000,
+                                                               
pg_file_create_mode, 04000, 02000,
                                                                time(NULL));
 
                                padding = ((recoveryconfcontents->len + 511) & 
~511) - recoveryconfcontents->len;
@@ -1441,7 +1442,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_create_mode) 
!= 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 afb392dbb3..ac5bb99f1b 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -6,7 +6,7 @@ use File::Basename qw(basename dirname);
 use File::Path qw(rmtree);
 use PostgresNode;
 use TestLib;
-use Test::More tests => 104;
+use Test::More tests => 105;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -16,6 +16,9 @@ my $tempdir = TestLib::tempdir;
 
 my $node = get_new_node('main');
 
+# Set umask so test directories and files are created with default permissions
+umask(0077);
+
 # Initialize node without replication settings
 $node->init(extra => [ '--data-checksums' ]);
 $node->start;
@@ -94,6 +97,15 @@ $node->command_ok([ 'pg_basebackup', '-D', 
"$tempdir/backup", '-X', 'none' ],
        'pg_basebackup runs');
 ok(-f "$tempdir/backup/PG_VERSION", 'backup was created');
 
+# Permissions on backup should be default
+SKIP:
+{
+       skip "unix-style permissions not supported on Windows", 1 if 
($windows_os);
+
+       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..19c106d9f5 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 test directories and files are created with default permissions
+umask(0077);
+
 my $primary = get_new_node('primary');
 $primary->init(allows_streaming => 1);
 $primary->start;
@@ -56,3 +59,12 @@ $primary->command_ok(
        [   'pg_receivewal', '-D',     $stream_dir,     '--verbose',
                '--endpos',      $nextlsn, '--synchronous', '--no-loop' ],
        'streaming some WAL with --synchronous');
+
+# Permissions on WAL files should be default
+SKIP:
+{
+       skip "unix-style permissions not supported on Windows", 1 if 
($windows_os);
+
+       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..267a40debb 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_create_mode);
        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_create_mode);
                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..5ede385e6a 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_OWNER);
 
        /* 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..067a084c87 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 => 21;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -57,9 +57,23 @@ 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 default permission test.  The permissions won't be checked on
+# Windows but we still want to do the restart 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');
+
+# Permissions on log file should be default
+SKIP:
+{
+       skip "unix-style permissions not supported on Windows", 2 if 
($windows_os);
+
+       ok(-f $logFileName);
+       ok(check_mode_recursive("$tempdir/data", 0700, 0600));
+}
+
 command_ok([ 'pg_ctl', 'restart', '-D', "$tempdir/data" ],
        'pg_ctl restart with server running');
 
diff --git a/src/bin/pg_resetwal/pg_resetwal.c 
b/src/bin/pg_resetwal/pg_resetwal.c
index eba7c5fdee..bdf71886ee 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"
@@ -967,7 +968,7 @@ RewriteControlFile(void)
 
        fd = open(XLOG_CONTROL_FILE,
                          O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
-                         S_IRUSR | S_IWUSR);
+                         pg_file_create_mode);
        if (fd < 0)
        {
                fprintf(stderr, _("%s: could not create pg_control file: %s\n"),
@@ -1249,7 +1250,7 @@ WriteEmptyXLOG(void)
        unlink(path);
 
        fd = open(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
-                         S_IRUSR | S_IWUSR);
+                         pg_file_create_mode);
        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..0d6ab20073 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,13 @@ $node->init;
 command_like([ 'pg_resetwal', '-n', $node->data_dir ],
                         qr/checkpoint/,
                         'pg_resetwal -n produces output');
+
+
+# Permissions on PGDATA should be default
+SKIP:
+{
+       skip "unix-style permissions not supported on Windows", 1 if 
($windows_os);
+
+       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 f491ed7f5c..94bcc13ae8 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"
@@ -57,7 +58,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_create_mode);
        if (dstfd < 0)
                pg_fatal("could not open target file \"%s\": %s\n",
                                 dstpath, strerror(errno));
@@ -198,7 +199,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_create_mode);
        if (fd < 0)
                pg_fatal("could not open file \"%s\" for truncation: %s\n",
                                 dstpath, strerror(errno));
@@ -219,7 +220,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_create_mode) != 0)
                pg_fatal("could not create directory \"%s\": %s\n",
                                 dstpath, strerror(errno));
 }
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 736f34eae3..1b0f823b0c 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,15 @@ in master, before promotion
 ),
                'tail-copy');
 
+       # Permissions on PGDATA should be default
+       SKIP:
+       {
+               skip "unix-style permissions not supported on Windows", 1 if 
($windows_os);
+
+               ok(check_mode_recursive($node_master->data_dir(), 0700, 0600),
+                       'check PGDATA permissions');
+       }
+
        RewindTest::clean_rewind_test();
 }
 
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index f38bfacf02..f68211aa20 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_create_mode)) < 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_create_mode)) < 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..1d35188143 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_OWNER);
 
        parseCommandLine(argc, argv);
 
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 39983abea1..574639d47e 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 correct 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/common/Makefile b/src/common/Makefile
index 873fbb6438..e9e75867f3 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -40,8 +40,8 @@ override CPPFLAGS += -DVAL_LIBS="\"$(LIBS)\""
 override CPPFLAGS := -DFRONTEND $(CPPFLAGS)
 LIBS += $(PTHREAD_LIBS)
 
-OBJS_COMMON = base64.o config_info.o controldata_utils.o exec.o ip.o \
-       keywords.o md5.o pg_lzcompress.o pgfnames.o psprintf.o relpath.o \
+OBJS_COMMON = base64.o config_info.o controldata_utils.o exec.o file_perm.o \
+       ip.o keywords.o md5.o pg_lzcompress.o pgfnames.o psprintf.o relpath.o \
        rmtree.o saslprep.o scram-common.o string.o unicode_norm.o \
        username.o wait_error.o
 
diff --git a/src/common/file_perm.c b/src/common/file_perm.c
new file mode 100644
index 0000000000..fdfbb9a44c
--- /dev/null
+++ b/src/common/file_perm.c
@@ -0,0 +1,19 @@
+/*-------------------------------------------------------------------------
+ *
+ * File and directory permission routines
+ *
+ *
+ * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/common/file_perm.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include <sys/stat.h>
+
+#include "common/file_perm.h"
+
+/* Modes for creating directories and files in the data directory */
+int pg_dir_create_mode = PG_DIR_MODE_OWNER;
+int pg_file_create_mode = PG_FILE_MODE_OWNER;
diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h
new file mode 100644
index 0000000000..37631a7191
--- /dev/null
+++ b/src/include/common/file_perm.h
@@ -0,0 +1,34 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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
+
+/*
+ * Mode mask for data directory permissions that only allows the owner to
+ * read/write directories and files.
+ *
+ * This is the default.
+ */
+#define PG_MODE_MASK_OWNER                 (S_IRWXG | S_IRWXO)
+
+/* Default mode for creating directories */
+#define PG_DIR_MODE_OWNER                      S_IRWXU
+
+/* Default mode for creating files */
+#define PG_FILE_MODE_OWNER                 (S_IRUSR | S_IWUSR)
+
+/* Modes for creating directories and files in the data directory */
+extern int pg_dir_create_mode;
+extern int pg_file_create_mode;
+
+#endif                                                 /* FILE_PERM_H */
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index e49b42ce86..785d9058e9 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 MakePGDirectory(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.
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 41d720880a..1d3ed6b0b1 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -111,8 +111,8 @@ sub mkvcbuild
        }
 
        our @pgcommonallfiles = qw(
-         base64.c config_info.c controldata_utils.c exec.c ip.c keywords.c
-         md5.c pg_lzcompress.c pgfnames.c psprintf.c relpath.c rmtree.c
+         base64.c config_info.c controldata_utils.c exec.c file_perm.c ip.c
+         keywords.c md5.c pg_lzcompress.c pgfnames.c psprintf.c relpath.c 
rmtree.c
          saslprep.c scram-common.c string.c unicode_norm.c username.c
          wait_error.c);
 
-- 
2.14.3 (Apple Git-98)


From 8ff0e8fe63395efe1ec0f9e13780110629632279 Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Sat, 7 Apr 2018 11:26:20 -0400
Subject: [PATCH 2/2] Allow group access on PGDATA

Allow the cluster to be optionally init'd with read access for the
group.

This means a relatively non-privileged user can perform a backup of the
cluster without requiring write privileges, which enhances security.

The mode of PGDATA is used to determine whether group permissions are
enabled for directory and file creates.  This method was chosen as it's
simple and works well for the various utilities that write into PGDATA.

Changing the mode of PGDATA manually will not automatically change the
mode of all the files contained therein.  If the user would like to
enable group access on an existing cluster then changing the mode of all
the existing files will be required.  Note that pg_upgrade will
automatically change the mode of all migrated files if the new cluster
is init'd with the -g option.

Tests are included for the backend and all the utilities which operate
on the PG data directory to ensure that the correct mode is set based on
the data directory permissions.

Author: David Steele <da...@pgmasters.net>
Reviewed-By: Michael Paquier, with discussion amongst many others.
Discussion: 
https://postgr.es/m/ad346fe6-b23e-59f1-ecb7-0e08390ad629%40pgmasters.net
---
 doc/src/sgml/config.sgml                     |  17 ++++
 doc/src/sgml/ref/initdb.sgml                 |  19 +++++
 doc/src/sgml/ref/pg_basebackup.sgml          |   6 ++
 doc/src/sgml/ref/pg_receivewal.sgml          |   6 ++
 doc/src/sgml/ref/pg_recvlogical.sgml         |  11 +++
 doc/src/sgml/runtime.sgml                    |  26 +++++-
 src/backend/bootstrap/bootstrap.c            |  12 +--
 src/backend/postmaster/postmaster.c          |  87 ++++----------------
 src/backend/tcop/postgres.c                  |   3 +-
 src/backend/utils/init/globals.c             |   9 +++
 src/backend/utils/init/miscinit.c            | 115 ++++++++++++++++++++++++---
 src/backend/utils/misc/guc.c                 |  25 ++++++
 src/bin/initdb/initdb.c                      |  24 +++++-
 src/bin/initdb/t/001_initdb.pl               |  20 ++++-
 src/bin/pg_basebackup/pg_basebackup.c        |  22 +++--
 src/bin/pg_basebackup/pg_receivewal.c        |   7 ++
 src/bin/pg_basebackup/pg_recvlogical.c       |   7 ++
 src/bin/pg_basebackup/streamutil.c           |  76 ++++++++++++++++++
 src/bin/pg_basebackup/t/010_pg_basebackup.pl |  21 ++++-
 src/bin/pg_ctl/pg_ctl.c                      |  12 ++-
 src/bin/pg_ctl/t/001_start_stop.pl           |  25 +++++-
 src/bin/pg_resetwal/pg_resetwal.c            |  10 +++
 src/bin/pg_rewind/RewindTest.pm              |   9 ++-
 src/bin/pg_rewind/pg_rewind.c                |  11 +++
 src/bin/pg_rewind/t/002_databases.pl         |  13 ++-
 src/bin/pg_upgrade/pg_upgrade.c              |  12 ++-
 src/bin/pg_upgrade/test.sh                   |  16 ++--
 src/common/file_perm.c                       |  72 ++++++++++++++++-
 src/include/common/file_perm.h               |  24 +++++-
 src/include/miscadmin.h                      |   2 +
 src/test/perl/PostgresNode.pm                |  27 ++++++-
 src/test/perl/TestLib.pm                     |  25 ++++++
 32 files changed, 644 insertions(+), 127 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a189a8efc3..5d5f2d23c4 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8144,6 +8144,23 @@ dynamic_library_path = 
'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-data-directory-mode" 
xreflabel="data_directory_mode">
+      <term><varname>data_directory_mode</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>data_directory_mode</varname> configuration 
parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        On Unix systems this parameter reports the permissions of the data
+        directory defined by (<xref linkend="guc-data-directory"/>) at startup.
+        (On Microsoft Windows this parameter will always display
+        <literal>0700</literal>). See
+        <xref linkend="app-initdb-allow-group-access"/> for more information.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-debug-assertions" xreflabel="debug_assertions">
       <term><varname>debug_assertions</varname> (<type>boolean</type>)
       <indexterm>
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 826dd91f72..10a8a86a03 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -76,6 +76,14 @@ PostgreSQL documentation
    to do so.)
   </para>
 
+  <para>
+    For security reasons the new cluster created by <command>initdb</command>
+    will only be accessible by the cluster owner by default.  The
+    <option>--allow-group-access</option> option allows any user in the same
+    group as the cluster owner to read files in the cluster.  This is useful
+    for performing backups as a non-privileged user.
+  </para>
+
   <para>
    <command>initdb</command> initializes the database cluster's default
    locale and character set encoding. The character set encoding,
@@ -188,6 +196,17 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry id="app-initdb-allow-group-access" xreflabel="group access">
+      <term><option>-g</option></term>
+      <term><option>--allow-group-access</option></term>
+      <listitem>
+       <para>
+        Allows users in the same group as the cluster owner to read all cluster
+        files created by <command>initdb</command>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="app-initdb-data-checksums" xreflabel="data checksums">
       <term><option>-k</option></term>
       <term><option>--data-checksums</option></term>
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml 
b/doc/src/sgml/ref/pg_basebackup.sgml
index 95045669c9..fc1edf4864 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -737,6 +737,12 @@ PostgreSQL documentation
    or later.
   </para>
 
+  <para>
+   <application>pg_basebackup</application> will preserve group permissions in
+   both the <literal>plain</literal> and <literal>tar</literal> formats if 
group
+   permissions are enabled on the source cluster.
+  </para>
+
  </refsect1>
 
  <refsect1>
diff --git a/doc/src/sgml/ref/pg_receivewal.sgml 
b/doc/src/sgml/ref/pg_receivewal.sgml
index e3f2ce1fcb..a18ddd4bff 100644
--- a/doc/src/sgml/ref/pg_receivewal.sgml
+++ b/doc/src/sgml/ref/pg_receivewal.sgml
@@ -425,6 +425,12 @@ PostgreSQL documentation
    not keep up with fetching the WAL data.
   </para>
 
+  <para>
+   <application>pg_receivewal</application> will preserve group permissions on
+   the received WAL files if group permissions are enabled on the source
+   cluster.
+  </para>
+
  </refsect1>
 
  <refsect1>
diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml 
b/doc/src/sgml/ref/pg_recvlogical.sgml
index a79ca20084..141c5cddce 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -399,6 +399,17 @@ PostgreSQL documentation
   </para>
  </refsect1>
 
+ <refsect1>
+  <title>Notes</title>
+
+  <para>
+   <application>pg_recvlogical</application> will preserve group permissions on
+   the received WAL files if group permissions are enabled on the source
+   cluster.
+  </para>
+
+ </refsect1>
+
  <refsect1>
   <title>Examples</title>
 
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 587b430527..330e38a29e 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -137,7 +137,22 @@ postgres$ <userinput>initdb -D 
/usr/local/pgsql/data</userinput>
    database, it is essential that it be secured from unauthorized
    access. <command>initdb</command> therefore revokes access
    permissions from everyone but the
-   <productname>PostgreSQL</productname> user.
+   <productname>PostgreSQL</productname> user, and optionally, group.
+   Group access, when enabled, is read-only.  This allows an unprivileged
+   user in the same group as the cluster owner to take a backup of the
+   cluster data or perform other operations that only require read access.
+  </para>
+
+  <para>
+   Note that enabling or disabling group access on an existing cluster requires
+   the cluster to be shut down and the appropriate mode to be set on all
+   directories and files before restarting
+   <productname>PostgreSQL</productname>.  Otherwise, a mix of modes might
+   exist in the data directory.  For clusters that allow access only by the
+   owner, the appropriate modes are <literal>0700</literal> for directories
+   and <literal>0600</literal> for files.  For clusters that also allow
+   reads by the group, the appropriate modes are <literal>0750</literal>
+   for directories and <literal>0640</literal> for files.
   </para>
 
   <para>
@@ -2194,6 +2209,15 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
    member of the group that has access to those certificate and key files.
   </para>
 
+  <para>
+    If the data directory allows group read access then certificate files may
+    need to be located outside of the data directory in order to conform to the
+    security requirements outlined above.  Generally, group access is enabled
+    to allow an unprivileged user to backup the database, and in that case the
+    backup software will not be able to read the certificate files and will
+    likely error.
+  </para>
+
   <para>
    If the private key is protected with a passphrase, the
    server will prompt for the passphrase and will not start until it has
diff --git a/src/backend/bootstrap/bootstrap.c 
b/src/backend/bootstrap/bootstrap.c
index 1430894ad2..d6da743ddd 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -349,13 +349,15 @@ AuxiliaryProcessMain(int argc, char *argv[])
                        proc_exit(1);
        }
 
-       /* Validate we have been given a reasonable-looking DataDir */
-       Assert(DataDir);
-       ValidatePgVersion(DataDir);
-
-       /* Change into DataDir (if under postmaster, should be done already) */
+       /*
+        * Validate we have been given a reasonable-looking DataDir and change
+        * into it (if under postmaster, should be done already).
+        */
        if (!IsUnderPostmaster)
+       {
+               checkDataDir();
                ChangeToDataDir();
+       }
 
        /* If standalone, create lockfile for data directory */
        if (!IsUnderPostmaster)
diff --git a/src/backend/postmaster/postmaster.c 
b/src/backend/postmaster/postmaster.c
index 10afecffb3..aa1fcf0bdd 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -391,7 +391,7 @@ static DNSServiceRef bonjour_sdref = NULL;
 static void CloseServerPorts(int status, Datum arg);
 static void unlink_external_pid_file(int status, Datum arg);
 static void getInstallationPaths(const char *argv0);
-static void checkDataDir(void);
+static void checkControlFile(void);
 static Port *ConnCreate(int serverFd);
 static void ConnFree(Port *port);
 static void reset_shared(int port);
@@ -588,7 +588,12 @@ PostmasterMain(int argc, char *argv[])
        IsPostmasterEnvironment = true;
 
        /*
-        * for security, no dir or file created can be group or other accessible
+        * We should not be creating any files or directories before we check 
the
+        * data directory (see checkDataDir()), but just in case set the umask 
to
+        * the most restrictive (onwer-only) permissions.
+        *
+        * checkDataDir() will reset the umask based on the data directory
+        * permissions.
         */
        umask(PG_MODE_MASK_OWNER);
 
@@ -877,6 +882,9 @@ PostmasterMain(int argc, char *argv[])
        /* Verify that DataDir looks reasonable */
        checkDataDir();
 
+       /* Check that pg_control exists */
+       checkControlFile();
+
        /* And switch working directory into it */
        ChangeToDataDir();
 
@@ -1469,82 +1477,17 @@ getInstallationPaths(const char *argv0)
         */
 }
 
-
 /*
- * Validate the proposed data directory
+ * Check that pg_control exists in the correct location in the data directory.
+ *
+ * No attempt is made to validate the contents of pg_control here.  This is
+ * just a sanity check to see if we are looking at a real data directory.
  */
 static void
-checkDataDir(void)
+checkControlFile(void)
 {
        char            path[MAXPGPATH];
        FILE       *fp;
-       struct stat stat_buf;
-
-       Assert(DataDir);
-
-       if (stat(DataDir, &stat_buf) != 0)
-       {
-               if (errno == ENOENT)
-                       ereport(FATAL,
-                                       (errcode_for_file_access(),
-                                        errmsg("data directory \"%s\" does not 
exist",
-                                                       DataDir)));
-               else
-                       ereport(FATAL,
-                                       (errcode_for_file_access(),
-                                        errmsg("could not read permissions of 
directory \"%s\": %m",
-                                                       DataDir)));
-       }
-
-       /* eventual chdir would fail anyway, but let's test ... */
-       if (!S_ISDIR(stat_buf.st_mode))
-               ereport(FATAL,
-                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                errmsg("specified data directory \"%s\" is not 
a directory",
-                                               DataDir)));
-
-       /*
-        * Check that the directory belongs to my userid; if not, reject.
-        *
-        * This check is an essential part of the interlock that prevents two
-        * postmasters from starting in the same directory (see 
CreateLockFile()).
-        * Do not remove or weaken it.
-        *
-        * XXX can we safely enable this check on Windows?
-        */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-       if (stat_buf.st_uid != geteuid())
-               ereport(FATAL,
-                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                errmsg("data directory \"%s\" has wrong 
ownership",
-                                               DataDir),
-                                errhint("The server must be started by the 
user that owns the data directory.")));
-#endif
-
-       /*
-        * Check if the directory has group or world access.  If so, reject.
-        *
-        * It would be possible to allow weaker constraints (for example, allow
-        * group access) but we cannot make a general assumption that that is
-        * okay; for example there are platforms where nearly all users
-        * customarily belong to the same group.  Perhaps this test should be
-        * configurable.
-        *
-        * XXX temporarily suppress check when on Windows, because there may not
-        * be proper support for Unix-y file permissions.  Need to think of a
-        * reasonable check to apply on Windows.
-        */
-#if !defined(WIN32) && !defined(__CYGWIN__)
-       if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
-               ereport(FATAL,
-                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                errmsg("data directory \"%s\" has group or 
world access",
-                                               DataDir),
-                                errdetail("Permissions should be u=rwx 
(0700).")));
-#endif
-
-       /* Look for PG_VERSION before looking for pg_control */
-       ValidatePgVersion(DataDir);
 
        snprintf(path, sizeof(path), "%s/global/pg_control", DataDir);
 
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 7bdecc32ec..5095a4f686 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3731,8 +3731,7 @@ PostgresMain(int argc, char *argv[],
                 * Validate we have been given a reasonable-looking DataDir (if 
under
                 * postmaster, assume postmaster did this already).
                 */
-               Assert(DataDir);
-               ValidatePgVersion(DataDir);
+               checkDataDir();
 
                /* Change into DataDir (if under postmaster, was done already) 
*/
                ChangeToDataDir();
diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index c1f0441b08..0a3163398f 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -16,8 +16,11 @@
  *
  *-------------------------------------------------------------------------
  */
+#include <sys/stat.h>
+
 #include "postgres.h"
 
+#include "common/file_perm.h"
 #include "libpq/libpq-be.h"
 #include "libpq/pqcomm.h"
 #include "miscadmin.h"
@@ -59,6 +62,12 @@ struct Latch *MyLatch;
  */
 char      *DataDir = NULL;
 
+/*
+ * Mode of the data directory.  The default is 0700 but may it be changed in
+ * checkDataDir() to 0750 if the data directory actually has that mode.
+ */
+int                    data_directory_mode = PG_DIR_MODE_OWNER;
+
 char           OutputFileName[MAXPGPATH];      /* debugging output file */
 
 char           my_exec_path[MAXPGPATH];        /* full path to my executable */
diff --git a/src/backend/utils/init/miscinit.c 
b/src/backend/utils/init/miscinit.c
index f8f08f3f88..33ebcbb54c 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -88,6 +88,100 @@ SetDatabasePath(const char *path)
        DatabasePath = MemoryContextStrdup(TopMemoryContext, path);
 }
 
+/*
+ * Validate the proposed data directory.
+ *
+ * Also initialize file and directory create modes and mode mask.
+ */
+void
+checkDataDir(void)
+{
+       struct stat stat_buf;
+
+       Assert(DataDir);
+
+       if (stat(DataDir, &stat_buf) != 0)
+       {
+               if (errno == ENOENT)
+                       ereport(FATAL,
+                                       (errcode_for_file_access(),
+                                        errmsg("data directory \"%s\" does not 
exist",
+                                                       DataDir)));
+               else
+                       ereport(FATAL,
+                                       (errcode_for_file_access(),
+                                        errmsg("could not read permissions of 
directory \"%s\": %m",
+                                                       DataDir)));
+       }
+
+       /* eventual chdir would fail anyway, but let's test ... */
+       if (!S_ISDIR(stat_buf.st_mode))
+               ereport(FATAL,
+                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                errmsg("specified data directory \"%s\" is not 
a directory",
+                                               DataDir)));
+
+       /*
+        * Check that the directory belongs to my userid; if not, reject.
+        *
+        * This check is an essential part of the interlock that prevents two
+        * postmasters from starting in the same directory (see 
CreateLockFile()).
+        * Do not remove or weaken it.
+        *
+        * XXX can we safely enable this check on Windows?
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       if (stat_buf.st_uid != geteuid())
+               ereport(FATAL,
+                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                errmsg("data directory \"%s\" has wrong 
ownership",
+                                               DataDir),
+                                errhint("The server must be started by the 
user that owns the data directory.")));
+#endif
+
+       /*
+        * Check if the directory has correct permissions.  If not, reject.
+        *
+        * Only two possible modes are allowed, 0700 and 0750.  The latter mode
+        * indicates that group read/execute should be allowed on all newly 
created
+        * files and directories.
+        *
+        * XXX temporarily suppress check when on Windows, because there may not
+        * be proper support for Unix-y file permissions.  Need to think of a
+        * reasonable check to apply on Windows.
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       if (stat_buf.st_mode & PG_MODE_MASK_GROUP)
+               ereport(FATAL,
+                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                errmsg("data directory \"%s\" has invalid 
permissions",
+                                               DataDir),
+                                errdetail("Permissions should be u=rwx (0700) 
or u=rwx,g=rx (0750).")));
+#endif
+
+       /*
+        * Reset creation modes and mask based on the mode of the data 
directory.
+        *
+        * The mask was set earlier in startup to disallow group permissions on
+        * newly created files and directories.  However, if group read/execute 
are
+        * present on the data directory then modify the create modes and mask 
to
+        * allow group read/execute on newly created files and directories and 
set
+        * the data_directory_mode GUC.
+        *
+        * Suppress when on Windows, because there may not be proper support
+        * for Unix-y file permissions.
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       SetDataDirectoryCreatePerm(stat_buf.st_mode);
+
+       umask(pg_mode_mask);
+       data_directory_mode = pg_dir_create_mode;
+#endif
+
+       /* Check for PG_VERSION */
+       ValidatePgVersion(DataDir);
+}
+
 /*
  * Set data directory, but make sure it's an absolute path.  Use this,
  * never set DataDir directly.
@@ -829,7 +923,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
                /*
                 * Try to create the lock file --- O_EXCL makes this atomic.
                 *
-                * Think not to make the file protection weaker than 0600.  See
+                * Think not to make the file protection weaker than 0600/0640. 
 See
                 * comments below.
                 */
                fd = open(filename, O_RDWR | O_CREAT | O_EXCL, 
pg_file_create_mode);
@@ -899,17 +993,14 @@ CreateLockFile(const char *filename, bool amPostmaster,
                 * implies that the existing process has a different userid 
than we
                 * do, which means it cannot be a competing postmaster.  A 
postmaster
                 * cannot successfully attach to a data directory owned by a 
userid
-                * other than its own.  (This is now checked directly in
-                * checkDataDir(), but has been true for a long time because of 
the
-                * restriction that the data directory isn't group- or
-                * world-accessible.)  Also, since we create the lockfiles mode 
600,
-                * we'd have failed above if the lockfile belonged to another 
userid
-                * --- which means that whatever process kill() is reporting 
about
-                * isn't the one that made the lockfile.  (NOTE: this last
-                * consideration is the only one that keeps us from blowing 
away a
-                * Unix socket file belonging to an instance of Postgres being 
run by
-                * someone else, at least on machines where /tmp hasn't got a
-                * stickybit.)
+                * other than its own, as enforced in checkDataDir(). Also, 
since we
+                * create the lockfiles mode 0600/0640, we'd have failed above 
if the
+                * lockfile belonged to another userid --- which means that 
whatever
+                * process kill() is reporting about isn't the one that made the
+                * lockfile.  (NOTE: this last consideration is the only one 
that keeps
+                * us from blowing away a Unix socket file belonging to an 
instance of
+                * Postgres being run by someone else, at least on machines 
where /tmp
+                * hasn't got a stickybit.)
                 */
                if (other_pid != my_pid && other_pid != my_p_pid &&
                        other_pid != my_gp_pid)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 71c2b4eff1..8441837e0f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -194,6 +194,7 @@ static void assign_application_name(const char *newval, 
void *extra);
 static bool check_cluster_name(char **newval, void **extra, GucSource source);
 static const char *show_unix_socket_permissions(void);
 static const char *show_log_file_mode(void);
+static const char *show_data_directory_mode(void);
 
 /* Private functions in guc-file.l that need to be called from guc.c */
 static ConfigVariable *ProcessConfigFileInternal(GucContext context,
@@ -2044,6 +2045,21 @@ static struct config_int ConfigureNamesInt[] =
                NULL, NULL, show_log_file_mode
        },
 
+
+       {
+               {"data_directory_mode", PGC_INTERNAL, PRESET_OPTIONS,
+                       gettext_noop("Mode of the data directory."),
+                       gettext_noop("The parameter value is a numeric mode 
specification "
+                                                "in the form accepted by the 
chmod and umask system "
+                                                "calls. (To use the customary 
octal format the number "
+                                                "must start with a 0 
(zero).)"),
+                       GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+               },
+               &data_directory_mode,
+               0700, 0000, 0777,
+               NULL, NULL, show_data_directory_mode
+       },
+
        {
                {"work_mem", PGC_USERSET, RESOURCES_MEM,
                        gettext_noop("Sets the maximum memory to be used for 
query workspaces."),
@@ -10744,4 +10760,13 @@ show_log_file_mode(void)
        return buf;
 }
 
+static const char *
+show_data_directory_mode(void)
+{
+       static char buf[12];
+
+       snprintf(buf, sizeof(buf), "%04o", data_directory_mode);
+       return buf;
+}
+
 #include "guc-file.c"
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 3765548a24..ba7ced343f 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1168,6 +1168,19 @@ setup_config(void)
                                                                  
"password_encryption = scram-sha-256");
        }
 
+       /*
+        * If group access has been enabled for the cluster then it makes sense 
to
+        * ensure that the log files also allow group access.  Otherwise a 
backup
+        * from a user in the group would fail if the log files were not
+        * relocated.
+        */
+       if (pg_dir_create_mode == PG_DIR_MODE_GROUP)
+       {
+               conflines = replace_token(conflines,
+                                                                 
"#log_file_mode = 0600",
+                                                                 
"log_file_mode = 0640");
+       }
+
        snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data);
 
        writefile(path, conflines);
@@ -2312,6 +2325,7 @@ usage(const char *progname)
        printf(_("      --auth-local=METHOD   default authentication method for 
local-socket connections\n"));
        printf(_(" [-D, --pgdata=]DATADIR     location for this database 
cluster\n"));
        printf(_("  -E, --encoding=ENCODING   set default encoding for new 
databases\n"));
+       printf(_("  -g, --allow-group-access  allow group read/execute on data 
directory\n"));
        printf(_("      --locale=LOCALE       set default locale for new 
databases\n"));
        printf(_("      --lc-collate=, --lc-ctype=, --lc-messages=LOCALE\n"
                         "      --lc-monetary=, --lc-numeric=, 
--lc-time=LOCALE\n"
@@ -2883,8 +2897,8 @@ initialize_data_directory(void)
 
        setup_signals();
 
-       /* Set dir/file mode mask */
-       umask(PG_MODE_MASK_OWNER);
+       /* Set mask based on requested PGDATA permissions */
+       umask(pg_mode_mask);
 
        create_data_directory();
 
@@ -3018,6 +3032,7 @@ main(int argc, char *argv[])
                {"waldir", required_argument, NULL, 'X'},
                {"wal-segsize", required_argument, NULL, 12},
                {"data-checksums", no_argument, NULL, 'k'},
+               {"allow-group-access", no_argument, NULL, 'g'},
                {NULL, 0, NULL, 0}
        };
 
@@ -3059,7 +3074,7 @@ main(int argc, char *argv[])
 
        /* process command-line options */
 
-       while ((c = getopt_long(argc, argv, "dD:E:kL:nNU:WA:sST:X:", 
long_options, &option_index)) != -1)
+       while ((c = getopt_long(argc, argv, "dD:E:kL:nNU:WA:sST:X:g", 
long_options, &option_index)) != -1)
        {
                switch (c)
                {
@@ -3153,6 +3168,9 @@ main(int argc, char *argv[])
                        case 12:
                                str_wal_segment_size_mb = pg_strdup(optarg);
                                break;
+                       case 'g':
+                               SetDataDirectoryCreatePerm(PG_DIR_MODE_GROUP);
+                               break;
                        default:
                                /* getopt_long already emitted a complaint */
                                fprintf(stderr, _("Try \"%s --help\" for more 
information.\n"),
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 9dfb2ed96f..8609d2ecbc 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -4,9 +4,11 @@
 
 use strict;
 use warnings;
+use Fcntl ':mode';
+use File::stat qw{lstat};
 use PostgresNode;
 use TestLib;
-use Test::More tests => 16;
+use Test::More tests => 18;
 
 my $tempdir = TestLib::tempdir;
 my $xlogdir = "$tempdir/pgxlog";
@@ -57,3 +59,19 @@ mkdir $datadir;
 }
 command_ok([ 'initdb', '-S', $datadir ], 'sync only');
 command_fails([ 'initdb', $datadir ], 'existing data directory');
+
+# Check group access on PGDATA
+SKIP:
+{
+       skip "unix-style permissions not supported on Windows", 2 if 
($windows_os);
+
+       # Init a new db with group access
+       my $datadir_group = "$tempdir/data_group";
+
+       command_ok(
+               [ 'initdb', '-g', $datadir_group ],
+               'successful creation with group access');
+
+       ok(check_mode_recursive($datadir_group, 0750, 0640),
+               'check PGDATA permissions');
+}
diff --git a/src/bin/pg_basebackup/pg_basebackup.c 
b/src/bin/pg_basebackup/pg_basebackup.c
index 32b41e313c..8cbc902ca3 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2470,14 +2470,6 @@ main(int argc, char **argv)
        }
 #endif
 
-       /*
-        * Verify that the target directory exists, or create it. For plaintext
-        * backups, always require the directory. For tar backups, require it
-        * unless we are writing to stdout.
-        */
-       if (format == 'p' || strcmp(basedir, "-") != 0)
-               verify_dir_is_empty_or_create(basedir, &made_new_pgdata, 
&found_existing_pgdata);
-
        /* connection in replication mode to server */
        conn = GetConnection();
        if (!conn)
@@ -2486,6 +2478,20 @@ main(int argc, char **argv)
                exit(1);
        }
 
+       /*
+        * Set umask so that directories/files are created with the same
+        * permissions as directories/files in the source data directory.
+        */
+       umask(pg_mode_mask);
+
+       /*
+        * Verify that the target directory exists, or create it. For plaintext
+        * backups, always require the directory. For tar backups, require it
+        * unless we are writing to stdout.
+        */
+       if (format == 'p' || strcmp(basedir, "-") != 0)
+               verify_dir_is_empty_or_create(basedir, &made_new_pgdata, 
&found_existing_pgdata);
+
        /* determine remote server's xlog segment size */
        if (!RetrieveWalSegSize(conn))
                disconnect_and_exit(1);
diff --git a/src/bin/pg_basebackup/pg_receivewal.c 
b/src/bin/pg_basebackup/pg_receivewal.c
index b82e073b86..62b6c686f3 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -19,6 +19,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "common/file_perm.h"
 #include "libpq-fe.h"
 #include "access/xlog_internal.h"
 #include "getopt_long.h"
@@ -703,6 +704,12 @@ main(int argc, char **argv)
        if (!RunIdentifySystem(conn, NULL, NULL, NULL, &db_name))
                disconnect_and_exit(1);
 
+       /*
+        * Set umask so that directories/files are created with the same
+        * permissions as directories/files in the source data directory.
+        */
+       umask(pg_mode_mask);
+
        /* determine remote server's xlog segment size */
        if (!RetrieveWalSegSize(conn))
                disconnect_and_exit(1);
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c 
b/src/bin/pg_basebackup/pg_recvlogical.c
index 0866395944..6c272b8f78 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -23,6 +23,7 @@
 #include "streamutil.h"
 
 #include "access/xlog_internal.h"
+#include "common/file_perm.h"
 #include "common/fe_memutils.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
@@ -959,6 +960,12 @@ main(int argc, char **argv)
                disconnect_and_exit(1);
        }
 
+       /*
+        * Set umask so that directories/files are created with the same
+        * permissions as directories/files in the source data directory.
+        */
+       umask(pg_mode_mask);
+
        /* Drop a replication slot. */
        if (do_drop_slot)
        {
diff --git a/src/bin/pg_basebackup/streamutil.c 
b/src/bin/pg_basebackup/streamutil.c
index 1438f368ed..4fd536931b 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -23,6 +23,7 @@
 
 #include "access/xlog_internal.h"
 #include "common/fe_memutils.h"
+#include "common/file_perm.h"
 #include "datatype/timestamp.h"
 #include "fe_utils/connect.h"
 #include "port/pg_bswap.h"
@@ -32,9 +33,16 @@
 
 uint32         WalSegSz;
 
+static bool RetrieveDataDirCreatePerm(PGconn *conn);
+
 /* SHOW command for replication connection was introduced in version 10 */
 #define MINIMUM_VERSION_FOR_SHOW_CMD 100000
 
+/*
+ * Group access is supported from version 11.
+ */
+#define MINIMUM_VERSION_FOR_GROUP_ACCESS 110000
+
 const char *progname;
 char      *connection_string = NULL;
 char      *dbhost = NULL;
@@ -254,6 +262,16 @@ GetConnection(void)
                exit(1);
        }
 
+       /*
+        * Retrieve the source data directory mode and use it to construct a 
umask
+        * for creating directories and files.
+        */
+       if (!RetrieveDataDirCreatePerm(tmpconn))
+       {
+               PQfinish(tmpconn);
+               exit(1);
+       }
+
        return tmpconn;
 }
 
@@ -327,6 +345,64 @@ RetrieveWalSegSize(PGconn *conn)
        return true;
 }
 
+/*
+ * RetrieveDataDirCreatePerm
+ *
+ * This function is used to determine the privileges on the server's PG data
+ * directory and, based on that, set what the permissions will be for
+ * directories and files we create.
+ *
+ * PG11 added support for (optionally) group read/execute rights to be set on
+ * the data directory.  Prior to PG11, only the owner was allowed to have 
rights
+ * on the data directory.
+ */
+static bool
+RetrieveDataDirCreatePerm(PGconn *conn)
+{
+       PGresult   *res;
+       int                     data_directory_mode;
+
+       /* check connection existence */
+       Assert(conn != NULL);
+
+       /* for previous versions leave the default group access */
+       if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_GROUP_ACCESS)
+               return true;
+
+       res = PQexec(conn, "SHOW data_directory_mode");
+       if (PQresultStatus(res) != PGRES_TUPLES_OK)
+       {
+               fprintf(stderr, _("%s: could not send replication command 
\"%s\": %s\n"),
+                               progname, "SHOW data_directory_mode", 
PQerrorMessage(conn));
+
+               PQclear(res);
+               return false;
+       }
+       if (PQntuples(res) != 1 || PQnfields(res) < 1)
+       {
+               fprintf(stderr,
+                               _("%s: could not fetch group access flag: got 
%d rows and %d fields, expected %d rows and %d or more fields\n"),
+                               progname, PQntuples(res), PQnfields(res), 1, 1);
+
+               PQclear(res);
+               return false;
+       }
+
+       if (sscanf(PQgetvalue(res, 0, 0), "%o", &data_directory_mode) != 1)
+       {
+               fprintf(stderr, _("%s: group access flag could not be parsed: 
%s\n"),
+                               progname, PQgetvalue(res, 0, 0));
+
+               PQclear(res);
+               return false;
+       }
+
+       SetDataDirectoryCreatePerm(data_directory_mode);
+
+       PQclear(res);
+       return true;
+}
+
 /*
  * Run IDENTIFY_SYSTEM through a given connection and give back to caller
  * some result information if requested:
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl 
b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index ac5bb99f1b..f502a2e3c7 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -6,7 +6,7 @@ use File::Basename qw(basename dirname);
 use File::Path qw(rmtree);
 use PostgresNode;
 use TestLib;
-use Test::More tests => 105;
+use Test::More tests => 106;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -44,10 +44,17 @@ $node->command_fails(
 
 ok(!-d "$tempdir/backup", 'backup directory was cleaned up');
 
+# Create a backup directory that is not empty so the next commnd will fail
+# but leave the data directory behind
+mkdir("$tempdir/backup")
+       or BAIL_OUT("unable to create $tempdir/backup");
+append_to_file("$tempdir/backup/dir-not-empty.txt");
+
 $node->command_fails([ 'pg_basebackup', '-D', "$tempdir/backup", '-n' ],
        'failing run with no-clean option');
 
 ok(-d "$tempdir/backup", 'backup directory was created and left behind');
+rmtree("$tempdir/backup");
 
 open my $conf, '>>', "$pgdata/postgresql.conf";
 print $conf "max_replication_slots = 10\n";
@@ -200,11 +207,17 @@ unlink "$pgdata/$superlongname";
 # skip on Windows.
 SKIP:
 {
-       skip "symlinks not supported on Windows", 17 if ($windows_os);
+       skip "symlinks not supported on Windows", 18 if ($windows_os);
 
        # Move pg_replslot out of $pgdata and create a symlink to it.
        $node->stop;
 
+       # Set umask so test directories and files are created with group 
permissions
+       umask(0027);
+
+       # Enable group permissions on PGDATA
+       chmod_recursive("$pgdata", 0750, 0640);
+
        rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
          or BAIL_OUT "could not move $pgdata/pg_replslot";
        symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
@@ -275,6 +288,10 @@ SKIP:
                "tablespace symlink was updated");
        closedir $dh;
 
+       # Group access should be enabled on all backup files
+       ok(check_mode_recursive("$tempdir/backup1", 0750, 0640),
+          "check backup dir permissions");
+
        # Unlogged relation forks other than init should not be copied
        my ($tblspc1UnloggedBackupPath) = $tblspc1UnloggedPath =~ 
/[^\/]*\/[^\/]*\/[^\/]*$/g;
 
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 5ede385e6a..143021de05 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2171,7 +2171,7 @@ main(int argc, char **argv)
         */
        argv0 = argv[0];
 
-       /* Set dir/file mode mask */
+       /* Set restrictive mode mask until PGDATA permissions are checked */
        umask(PG_MODE_MASK_OWNER);
 
        /* support --help and --version even if invoked as root */
@@ -2407,6 +2407,16 @@ main(int argc, char **argv)
                snprintf(version_file, MAXPGPATH, "%s/PG_VERSION", pg_data);
                snprintf(pid_file, MAXPGPATH, "%s/postmaster.pid", pg_data);
                snprintf(backup_file, MAXPGPATH, "%s/backup_label", pg_data);
+
+               /*
+                * Set mask based on PGDATA permissions,
+                *
+                * Don't error here if the data directory cannot be stat'd. 
This is
+                * handled differently based on the command and we don't want to
+                * interfere with that logic.
+                */
+               if (GetDataDirectoryCreatePerm(pg_data))
+                       umask(pg_mode_mask);
        }
 
        switch (ctl_command)
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl 
b/src/bin/pg_ctl/t/001_start_stop.pl
index 067a084c87..2f9dfa7b81 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -2,9 +2,11 @@ use strict;
 use warnings;
 
 use Config;
+use Fcntl ':mode';
+use File::stat qw{lstat};
 use PostgresNode;
 use TestLib;
-use Test::More tests => 21;
+use Test::More tests => 24;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -74,6 +76,27 @@ SKIP:
        ok(check_mode_recursive("$tempdir/data", 0700, 0600));
 }
 
+# Log file for group access test
+$logFileName = "$tempdir/data/perm-test-640.log";
+
+SKIP:
+{
+       skip "group access not supported on Windows", 3 if ($windows_os);
+
+       system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data";
+
+       # Change the data dir mode so log file will be created with group read
+       # privileges on the next start
+       chmod_recursive("$tempdir/data", 0750, 0640);
+
+       command_ok(
+               [ 'pg_ctl', 'start', '-D', "$tempdir/data", '-l', $logFileName 
],
+               'start server to check group permissions');
+
+       ok(-f $logFileName);
+       ok(check_mode_recursive("$tempdir/data", 0750, 0640));
+}
+
 command_ok([ 'pg_ctl', 'restart', '-D', "$tempdir/data" ],
        'pg_ctl restart with server running');
 
diff --git a/src/bin/pg_resetwal/pg_resetwal.c 
b/src/bin/pg_resetwal/pg_resetwal.c
index bdf71886ee..8a0a805f1e 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -363,6 +363,16 @@ main(int argc, char *argv[])
                exit(1);
        }
 
+       /* Set mask based on PGDATA permissions */
+       if (!GetDataDirectoryCreatePerm(DataDir))
+       {
+               fprintf(stderr, _("%s: unable to read permissions from 
\"%s\"\n"),
+                               progname, DataDir);
+               exit(1);
+       }
+
+       umask(pg_mode_mask);
+
        /* Check that data directory matches our server version */
        CheckDataVersion();
 
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 7b632c7dcd..63d9bd517d 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -115,11 +115,13 @@ sub check_query
 
 sub setup_cluster
 {
-       my $extra_name = shift;
+       my $extra_name = shift;         # Used to differentiate clusters
+       my $extra = shift;                      # Extra params for initdb
 
        # Initialize master, data checksums are mandatory
        $node_master = get_new_node('master' . ($extra_name ? "_${extra_name}" 
: ''));
-       $node_master->init(allows_streaming => 1);
+       $node_master->init(
+               allows_streaming => 1, extra => $extra);
        # Set wal_keep_segments to prevent WAL segment recycling after enforced
        # checkpoints in the tests.
        $node_master->append_conf('postgresql.conf', qq(
@@ -237,7 +239,8 @@ sub run_pg_rewind
                "$tmp_folder/master-postgresql.conf.tmp",
                "$master_pgdata/postgresql.conf");
 
-       chmod(0600, "$master_pgdata/postgresql.conf")
+       chmod($node_master->group_access() ? 0640 : 0600,
+                 "$master_pgdata/postgresql.conf")
                or BAIL_OUT(
                        "unable to set permissions for 
$master_pgdata/postgresql.conf");
 
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 72ab2f8d5e..b9ea6a4c21 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,16 @@ main(int argc, char **argv)
                exit(1);
        }
 
+       /* Set mask based on PGDATA permissions */
+       if (!GetDataDirectoryCreatePerm(datadir_target))
+       {
+               fprintf(stderr, _("%s: unable to read permissions from 
\"%s\"\n"),
+                               progname, datadir_target);
+               exit(1);
+       }
+
+       umask(pg_mode_mask);
+
        /*
         * 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/002_databases.pl 
b/src/bin/pg_rewind/t/002_databases.pl
index 37cdd712f3..c364965d3a 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;
 
@@ -9,7 +9,7 @@ sub run_test
 {
        my $test_mode = shift;
 
-       RewindTest::setup_cluster($test_mode);
+       RewindTest::setup_cluster($test_mode, ['-g']);
        RewindTest::start_master();
 
        # Create a database in master.
@@ -42,6 +42,15 @@ template1
 ),
                'database names');
 
+       # Permissions on PGDATA should have group permissions
+       SKIP:
+       {
+               skip "unix-style permissions not supported on Windows", 1 if 
($windows_os);
+
+               ok(check_mode_recursive($node_master->data_dir(), 0750, 0640),
+                       'check PGDATA permissions');
+       }
+
        RewindTest::clean_rewind_test();
 }
 
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 1d35188143..cc8e8c94c5 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -79,7 +79,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 */
+       /* Set default restrictive mask until new cluster permissions are read 
*/
        umask(PG_MODE_MASK_OWNER);
 
        parseCommandLine(argc, argv);
@@ -100,6 +100,16 @@ main(int argc, char **argv)
 
        check_cluster_compatibility(live_check);
 
+       /* Set mask based on PGDATA permissions */
+       if (!GetDataDirectoryCreatePerm(new_cluster.pgdata))
+       {
+               pg_log(PG_FATAL, "unable to read permissions from \"%s\"\n",
+                          new_cluster.pgdata);
+               exit(1);
+       }
+
+       umask(pg_mode_mask);
+
        check_and_dump_old_cluster(live_check);
 
 
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 574639d47e..24631a91c6 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -20,9 +20,9 @@ unset MAKELEVEL
 # Run a given "initdb" binary and overlay the regression testing
 # authentication configuration.
 standard_initdb() {
-       # To increase coverage of non-standard segment size without
-       # increase test runtime, run these tests with a lower setting.
-       "$1" -N --wal-segsize 1
+       # To increase coverage of non-standard segment size and group access
+       # without increasing test runtime, run these tests with a custom 
setting.
+       "$1" -N --wal-segsize 1 -g
        if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
        then
                cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -230,14 +230,14 @@ 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 correct permissions
-if [ $(find ${PGDATA} -type f ! -perm 600 | wc -l) -ne 0 ]; then
-       echo "files in PGDATA with permission != 600";
+# make sure all directories and files have group permissions
+if [ $(find ${PGDATA} -type f ! -perm 640 | wc -l) -ne 0 ]; then
+       echo "files in PGDATA with permission != 640";
        exit 1;
 fi
 
-if [ $(find ${PGDATA} -type d ! -perm 700 | wc -l) -ne 0 ]; then
-       echo "directories in PGDATA with permission != 700";
+if [ $(find ${PGDATA} -type d ! -perm 750 | wc -l) -ne 0 ]; then
+       echo "directories in PGDATA with permission != 750";
        exit 1;
 fi
 
diff --git a/src/common/file_perm.c b/src/common/file_perm.c
index fdfbb9a44c..d640d6a1fd 100644
--- a/src/common/file_perm.c
+++ b/src/common/file_perm.c
@@ -12,8 +12,76 @@
  */
 #include <sys/stat.h>
 
+#include "c.h"
 #include "common/file_perm.h"
 
 /* Modes for creating directories and files in the data directory */
-int pg_dir_create_mode = PG_DIR_MODE_OWNER;
-int pg_file_create_mode = PG_FILE_MODE_OWNER;
+int                    pg_dir_create_mode = PG_DIR_MODE_OWNER;
+int                    pg_file_create_mode = PG_FILE_MODE_OWNER;
+
+/*
+ * Mode mask to pass to umask().  This is more of a preventative measure since
+ * all file/directory creates should be performed using the create modes above.
+ */
+int                    pg_mode_mask = PG_MODE_MASK_OWNER;
+
+/*
+ * Set create modes and mask to use when writing to PGDATA based on the data
+ * directory mode passed.  If group read/execute are present in the mode, then
+ * create modes and mask will be relaxed to allow group read/execute on all
+ * newly created files and directories.
+ */
+void
+SetDataDirectoryCreatePerm(int dataDirMode)
+{
+       /* If the data directory mode has group access */
+       if ((PG_DIR_MODE_GROUP & dataDirMode) == PG_DIR_MODE_GROUP)
+       {
+               pg_dir_create_mode = PG_DIR_MODE_GROUP;
+               pg_file_create_mode = PG_FILE_MODE_GROUP;
+               pg_mode_mask = PG_MODE_MASK_GROUP;
+       }
+       /* Else use default permissions */
+       else
+       {
+               pg_dir_create_mode = PG_DIR_MODE_OWNER;
+               pg_file_create_mode = PG_FILE_MODE_OWNER;
+               pg_mode_mask = PG_MODE_MASK_OWNER;
+       }
+}
+
+#ifdef FRONTEND
+
+/*
+ * Get the create modes and mask to use when writing to PGDATA by examining the
+ * mode of the PGDATA directory and calling SetDataDirectoryCreatePerm().
+ *
+ * Errors are not handled here and should be reported by the application when
+ * false is returned.
+ *
+ * Suppress when on Windows, because there may not be proper support for Unix-y
+ * file permissions.
+ */
+bool
+GetDataDirectoryCreatePerm(const char *dataDir)
+{
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       struct stat statBuf;
+
+       /*
+        * If an error occurs getting the mode then return false.  The caller is
+        * responsible for generating an error, if appropriate, indicating that 
we
+        * were unable to access the data directory.
+        */
+       if (stat(dataDir, &statBuf) == -1)
+               return false;
+
+       /* Set permissions */
+       SetDataDirectoryCreatePerm(statBuf.st_mode);
+
+       return true;
+#endif                                                 /* !defined(WIN32) && 
!defined(__CYGWIN__) */
+}
+
+
+#endif                                                 /* FRONTEND */
diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h
index 37631a7191..3090f78931 100644
--- a/src/include/common/file_perm.h
+++ b/src/include/common/file_perm.h
@@ -21,14 +21,34 @@
  */
 #define PG_MODE_MASK_OWNER                 (S_IRWXG | S_IRWXO)
 
+/*
+ * Mode mask for data directory permissions that also allows group 
read/execute.
+ */
+#define PG_MODE_MASK_GROUP                     (S_IWGRP | S_IRWXO)
+
 /* Default mode for creating directories */
 #define PG_DIR_MODE_OWNER                      S_IRWXU
 
+/* Mode for creating directories that allows group read/execute */
+#define PG_DIR_MODE_GROUP                      (S_IRWXU | S_IRGRP | S_IXGRP)
+
 /* Default mode for creating files */
 #define PG_FILE_MODE_OWNER                 (S_IRUSR | S_IWUSR)
 
+/* Mode for creating files that allows group read */
+#define PG_FILE_MODE_GROUP                     (S_IRUSR | S_IWUSR | S_IRGRP)
+
 /* Modes for creating directories and files in the data directory */
-extern int pg_dir_create_mode;
-extern int pg_file_create_mode;
+extern int     pg_dir_create_mode;
+extern int     pg_file_create_mode;
+
+/* Mode mask to pass to umask() */
+extern int     pg_mode_mask;
+
+/* Set permissions and mask based on the provided mode */
+extern void SetDataDirectoryCreatePerm(int dataDirMode);
+
+/* Set permissions and mask based on the mode of the data directory */
+extern bool GetDataDirectoryCreatePerm(const char *dataDir);
 
 #endif                                                 /* FILE_PERM_H */
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index b5ad841968..e167ee8fcb 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -153,6 +153,7 @@ extern PGDLLIMPORT bool IsBinaryUpgrade;
 extern PGDLLIMPORT bool ExitOnAnyError;
 
 extern PGDLLIMPORT char *DataDir;
+extern PGDLLIMPORT int data_directory_mode;
 
 extern PGDLLIMPORT int NBuffers;
 extern PGDLLIMPORT int MaxBackends;
@@ -323,6 +324,7 @@ extern void SetSessionAuthorization(Oid userid, bool 
is_superuser);
 extern Oid     GetCurrentRoleId(void);
 extern void SetCurrentRoleId(Oid roleid, bool is_superuser);
 
+extern void checkDataDir(void);
 extern void SetDataDir(const char *dir);
 extern void ChangeToDataDir(void);
 
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 76e571b98c..1bd91524d7 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -86,9 +86,11 @@ use Carp;
 use Config;
 use Cwd;
 use Exporter 'import';
+use Fcntl qw(:mode);
 use File::Basename;
 use File::Path qw(rmtree);
 use File::Spec;
+use File::stat qw(stat);
 use File::Temp ();
 use IPC::Run;
 use RecursiveCopy;
@@ -269,6 +271,26 @@ sub connstr
 
 =pod
 
+=item $node->group_access()
+
+Does the data dir allow group access?
+
+=cut
+
+sub group_access
+{
+       my ($self) = @_;
+
+       my $dir_stat = stat($self->data_dir);
+
+       defined($dir_stat)
+               or die('unable to stat ' . $self->data_dir);
+
+       return (S_IMODE($dir_stat->mode) == 0750);
+}
+
+=pod
+
 =item $node->data_dir()
 
 Returns the path to the data directory. postgresql.conf and pg_hba.conf are
@@ -460,6 +482,9 @@ sub init
        }
        close $conf;
 
+    chmod($self->group_access ? 0640 : 0600, "$pgdata/postgresql.conf")
+        or die("unable to set permissions for $pgdata/postgresql.conf");
+
        $self->set_replication_conf if $params{allows_streaming};
        $self->enable_archiving     if $params{has_archiving};
 }
@@ -485,7 +510,7 @@ sub append_conf
 
        TestLib::append_to_file($conffile, $str . "\n");
 
-    chmod(0600, $conffile)
+    chmod($self->group_access() ? 0640 : 0600, $conffile)
         or die("unable to set permissions for $conffile");
 }
 
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 93610e4bc4..8047404efd 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -31,6 +31,7 @@ our @EXPORT = qw(
   slurp_file
   append_to_file
   check_mode_recursive
+  chmod_recursive
   check_pg_config
   system_or_bail
   system_log
@@ -313,6 +314,30 @@ sub check_mode_recursive
        return $result;
 }
 
+# Change mode recursively on a directory
+sub chmod_recursive
+{
+       my ($dir, $dir_mode, $file_mode) = @_;
+
+       find
+       (
+               {follow_fast => 1,
+               wanted =>
+                       sub
+                       {
+                               my $file_stat = stat($File::Find::name);
+
+                               if (defined($file_stat))
+                               {
+                                       chmod(S_ISDIR($file_stat->mode) ? 
$dir_mode : $file_mode,
+                                                 $File::Find::name)
+                                               or die "unable to chmod 
$File::Find::name";
+                               }
+                       }},
+               $dir
+       );
+}
+
 # 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)

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to