Hi Michael,

On 3/7/18 8:51 PM, Michael Paquier wrote:
> On Wed, Mar 07, 2018 at 10:56:32AM -0500, David Steele wrote:
>> On 3/6/18 10:04 PM, Michael Paquier wrote:
>>> Seems like you forgot to re-add the chmod calls in initdb.c.
>>
>> Hmmm, I thought we were talking about moving the position of umask().
>>
>> I don't think the chmod() calls are needed because umask() is being set.
>>  The tests show that the config files have the proper permissions after
>> inidb, so this just looks like redundant code to me.
> 
> Let's discuss that on a separate thread then, there could be something
> we are missing, but I agree that those should not be needed.  Looking at
> the code history, those calls have been around since the beginning of
> PG-times.

Done.

>> Another way to do this would be to make the function generic and
>> stipulate that the postmaster must be shut down before running the
>> function.  We could check postmaster.pid permissions as a separate
>> test.
> 
> Yeah, that looks like a sensitive approach as this could be run
> post-initdb or after taking a backup.  This way we would avoid other
> similar behaviors in the future...  Still postmaster.pid is an
> exception.

Done.

>>> sub clean_rewind_test
>>> {
>>> +   ok (check_pg_data_perm($node_master->data_dir(), 0));
>>> +
>>>     $node_master->teardown_node  if defined $node_master;
>>> I have also a pending patch for pg_rewind which adds read-only files in
>>> the data folders of a new test, so this would cause this part to blow
>>> up.  Testing that for all the test sets does not bring additional value
>>> as well, and doing it at cleanup phase is also confusing.  So could you
>>> move that check into only one test's script?  You could remove the umask
>>> call in 003_extrafiles.pl as well this way, and reduce the patch diffs.
>>
>> I think I would rather have a way to skip the permission test rather
>> than disable it for most of the tests.  pg_rewind does more writes into
>> PGDATA that anything other than a backend.  Good coverage seems like a
>> plus.
> 
> All the tests basically run the same scenario, with minimal variations,
> so let's do the test once in the test touching the highest amount of
> files and call it good.

OK, test 001 is used to check default mode and 002 is used to check
group mode.  The rest are left as-is.  Does that work for you?

> I have begun to read through patch 3.
> 
> -    if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
> +    if (stat_buf.st_mode & PG_MODE_MASK_ALLOW_GROUP)
>          ereport(FATAL,
>              (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> -             errmsg("data directory \"%s\" has group or world access",
> +             errmsg("data directory \"%s\" has invalid permissions",
>                      DataDir),
> -             errdetail("Permissions should be u=rwx (0700).")));
> +             errdetail("Permissions should be u=rwx (0700) or u=rwx,g=rx 
> (0750).")));
> Hm.  This relaxes the checks and concerns me a lot.  What if users want
> to keep strict permission all the time and rely on the existing check to
> be sure that this gets never changed post-initdb?  For such users, we
> may want to track if cluster has been initialized with group access, in
> which case tracking that in the control file would be more adapted. 
> Then the startup checks should use this configuration.  Another idea
> would be a startup option.  So, I cannot believe that all users would
> like to see such checks relaxed.  Some of my users would surely complain
> about such sanity checks relaxed unconditionally, so making this
> configurable would be fine, and the current approach is not acceptable
> in my opinion.

How about a GUC that enforces one mode or the other on startup?  Default
would be 700.  The GUC can be set automatically by initdb based on the
-g option.  We had this GUC originally, but since the front-end tools
can't read it we abandoned it.  Seems like it would be good as an
enforcing mechanism, though.

Thanks,
-- 
-David
da...@pgmasters.net
From 19d827b9d9c0285556e977d3962619deb84c3c0e Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Mon, 5 Mar 2018 14:33:10 -0500
Subject: [PATCH 1/3] pg_resetwal tests.

Adds a very basic test suite for pg_resetwal.
---
 src/bin/pg_resetwal/.gitignore     |  1 +
 src/bin/pg_resetwal/Makefile       |  6 +++++
 src/bin/pg_resetwal/t/001_basic.pl | 53 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 src/bin/pg_resetwal/t/001_basic.pl

diff --git a/src/bin/pg_resetwal/.gitignore b/src/bin/pg_resetwal/.gitignore
index 236abb4323..a950255fd7 100644
--- a/src/bin/pg_resetwal/.gitignore
+++ b/src/bin/pg_resetwal/.gitignore
@@ -1 +1,2 @@
 /pg_resetwal
+/tmp_check
diff --git a/src/bin/pg_resetwal/Makefile b/src/bin/pg_resetwal/Makefile
index 5ab7ad33e0..13c9ca6279 100644
--- a/src/bin/pg_resetwal/Makefile
+++ b/src/bin/pg_resetwal/Makefile
@@ -33,3 +33,9 @@ uninstall:
 
 clean distclean maintainer-clean:
        rm -f pg_resetwal$(X) $(OBJS)
+
+check:
+       $(prove_check)
+
+installcheck:
+       $(prove_installcheck)
diff --git a/src/bin/pg_resetwal/t/001_basic.pl 
b/src/bin/pg_resetwal/t/001_basic.pl
new file mode 100644
index 0000000000..234bd70303
--- /dev/null
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -0,0 +1,53 @@
+use strict;
+use warnings;
+
+use Config;
+use PostgresNode;
+use TestLib;
+use Test::More tests => 13;
+
+my $tempdir       = TestLib::tempdir;
+my $tempdir_short = TestLib::tempdir_short;
+
+program_help_ok('pg_resetwal');
+program_version_ok('pg_resetwal');
+program_options_handling_ok('pg_resetwal');
+
+# Initialize node without replication settings
+my $node = get_new_node('main');
+my $pgdata = $node->data_dir;
+my $pgwal = "$pgdata/pg_wal";
+$node->init;
+$node->start;
+
+# Remove WAL from pg_wal and make sure it gets rebuilt
+$node->stop;
+
+unlink("$pgwal/000000010000000000000001") == 1
+       or die("unable to remove 000000010000000000000001");
+
+is_deeply(
+       [sort(slurp_dir($pgwal))], [sort(qw(. .. archive_status))], 'no WAL');
+
+$node->command_ok(['pg_resetwal', '-D', $pgdata], 'recreate pg_wal');
+
+is_deeply(
+       [sort(slurp_dir($pgwal))],
+       [sort(qw(. .. archive_status 000000010000000000000002))],
+       'WAL recreated');
+
+$node->start;
+
+# Reset to specific WAL segment
+$node->stop;
+
+$node->command_ok(
+       ['pg_resetwal', '-l', '000000070000000700000007', '-D', $pgdata],
+       'set to specific WAL');
+
+is_deeply(
+       [sort(slurp_dir($pgwal))],
+       [sort(qw(. .. archive_status 000000070000000700000007))],
+       'WAL recreated');
+
+$node->start;
-- 
2.14.3 (Apple Git-98)
From aa244efad465533225442b817f303ce81a76dd79 Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Fri, 9 Mar 2018 13:44:36 -0500
Subject: [PATCH 2/3] Refactor file permissions in backend/frontend

Adds a new header (file_perm.h) and makes all front-end utilities use
the new constants for setting umask. Converts mkdir() calls in the
backend to MakeDirectoryDefaultPerm() if the original
call used default permissions. Adds tests to make sure permissions in
PGDATA are set correctly by the front-end tools.
---
 src/backend/access/transam/xlog.c   |  2 +-
 src/backend/commands/tablespace.c   | 22 +++++++----
 src/backend/postmaster/postmaster.c |  2 +-
 src/backend/postmaster/syslogger.c  |  5 ++-
 src/backend/replication/slot.c      |  5 ++-
 src/backend/storage/file/copydir.c  |  2 +-
 src/backend/storage/file/fd.c       | 32 ++++++++++------
 src/backend/storage/ipc/ipc.c       |  4 ++
 src/bin/initdb/initdb.c             | 24 ++++++------
 src/bin/initdb/t/001_initdb.pl      |  4 +-
 src/bin/pg_ctl/pg_ctl.c             |  3 +-
 src/bin/pg_ctl/t/001_start_stop.pl  | 13 +++++--
 src/bin/pg_resetwal/pg_resetwal.c   |  8 +++-
 src/bin/pg_resetwal/t/001_basic.pl  |  4 +-
 src/bin/pg_rewind/RewindTest.pm     |  3 ++
 src/bin/pg_rewind/file_ops.c        |  7 ++--
 src/bin/pg_rewind/pg_rewind.c       |  4 ++
 src/bin/pg_rewind/t/001_basic.pl    |  3 +-
 src/bin/pg_upgrade/file.c           |  5 ++-
 src/bin/pg_upgrade/pg_upgrade.c     |  3 +-
 src/bin/pg_upgrade/test.sh          | 11 ++++++
 src/include/common/file_perm.h      | 32 ++++++++++++++++
 src/include/storage/fd.h            |  3 ++
 src/test/perl/PostgresNode.pm       |  3 ++
 src/test/perl/TestLib.pm            | 73 +++++++++++++++++++++++++++++++++++++
 25 files changed, 223 insertions(+), 54 deletions(-)
 create mode 100644 src/include/common/file_perm.h

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

This includes backend changes, utility changes (initdb, pg_ctl, pg_upgrade, 
etc.) and tests for each utility.
---
 doc/src/sgml/backup.sgml             |  6 ++++-
 doc/src/sgml/ref/initdb.sgml         | 19 ++++++++++++++
 doc/src/sgml/runtime.sgml            | 14 ++++++++++-
 src/backend/postmaster/postmaster.c  | 29 +++++++++++++---------
 src/bin/initdb/initdb.c              | 22 +++++++++++------
 src/bin/initdb/t/001_initdb.pl       | 13 +++++++++-
 src/bin/pg_ctl/pg_ctl.c              |  4 +++
 src/bin/pg_ctl/t/001_start_stop.pl   | 24 +++++++++++++++++-
 src/bin/pg_resetwal/pg_resetwal.c    |  4 +--
 src/bin/pg_resetwal/t/001_basic.pl   |  9 +++++--
 src/bin/pg_rewind/RewindTest.pm      |  9 ++++---
 src/bin/pg_rewind/pg_rewind.c        |  4 +--
 src/bin/pg_rewind/t/002_databases.pl |  6 +++--
 src/bin/pg_upgrade/pg_upgrade.c      |  5 +++-
 src/bin/pg_upgrade/test.sh           | 14 +++++------
 src/common/Makefile                  |  2 +-
 src/common/file_perm.c               | 48 ++++++++++++++++++++++++++++++++++++
 src/include/common/file_perm.h       | 31 +++++++++++++++++++----
 src/test/perl/PostgresNode.pm        | 27 +++++++++++++++++++-
 src/test/perl/TestLib.pm             | 25 +++++++++++++++++++
 src/tools/msvc/Mkvcbuild.pm          |  2 +-
 21 files changed, 267 insertions(+), 50 deletions(-)
 create mode 100644 src/common/file_perm.c

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 9d8e69056f..a9912eb418 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1095,7 +1095,11 @@ SELECT pg_stop_backup();
     and <filename>postmaster.opts</filename>, which record information
     about the running <application>postmaster</application>, not about the
     <application>postmaster</application> which will eventually use this 
backup.
-    (These files can confuse <application>pg_ctl</application>.)
+    (These files can confuse <application>pg_ctl</application>.)  If group read
+    access is enabled on the data directory and an unprivileged user in the
+    <productname>PostgreSQL</productname> group is performing the backup, then
+    <filename>postmaster.pid</filename> will not be readable and must be
+    excluded.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 585665f161..5f57b933b9 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,
@@ -301,6 +309,17 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <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>
       <term><option>-W</option></term>
       <term><option>--pwprompt</option></term>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 587b430527..40ce3d9813 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -137,7 +137,10 @@ 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 <productname>PostgreSQL</productname> group to take a backup of
+   the cluster data or perform other operations that only require read access.
   </para>
 
   <para>
@@ -2194,6 +2197,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/postmaster/postmaster.c 
b/src/backend/postmaster/postmaster.c
index 956960fd4b..9ce5535327 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"
@@ -587,7 +588,8 @@ PostmasterMain(int argc, char *argv[])
        IsPostmasterEnvironment = true;
 
        /*
-        * for security, no dir or file created can be group or other accessible
+        * By default, no dir or file created can be group or other accessible. 
 This
+        * may be modified later depending in the permissions of the data 
directory.
         */
        umask(S_IRWXG | S_IRWXO);
 
@@ -1521,25 +1523,30 @@ checkDataDir(void)
 #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.
+        * Check if the directory has correct permissions.  If not, reject.
         *
         * 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))
+       if (stat_buf.st_mode & PG_MODE_MASK_ALLOW_GROUP)
                ereport(FATAL,
                                
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                errmsg("data directory \"%s\" has group or 
world access",
+                                errmsg("data directory \"%s\" has invalid 
permissions",
                                                DataDir),
-                                errdetail("Permissions should be u=rwx 
(0700).")));
+                                errdetail("Permissions should be u=rwx (0700) 
or u=rwx,g=rx (0750).")));
+#endif
+
+       /*
+        * Reset the file mode creation mask based on the mode of the data
+        * directory.
+        *
+        * Suppress when on Windows, because there may not be proper support
+        * for Unix-y file permissions.
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       umask(PG_MODE_MASK_DEFAULT & ~stat_buf.st_mode);
 #endif
 
        /* Look for PG_VERSION before looking for pg_control */
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 278e56bf95..f77b58bf74 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -145,6 +145,7 @@ static bool data_checksums = false;
 static char *xlog_dir = NULL;
 static char *str_wal_segment_size_mb = NULL;
 static int     wal_segment_size_mb;
+static mode_t file_mode_mask = PG_MODE_MASK_DEFAULT;
 
 
 /* internal vars */
@@ -1171,7 +1172,7 @@ setup_config(void)
        snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data);
 
        writefile(path, conflines);
-       if (chmod(path, PG_FILE_MODE_DEFAULT) != 0)
+       if (chmod(path, PG_FILE_MODE_DEFAULT & ~file_mode_mask) != 0)
        {
                fprintf(stderr, _("%s: could not change permissions of \"%s\": 
%s\n"),
                                progname, path, strerror(errno));
@@ -1191,7 +1192,7 @@ setup_config(void)
        sprintf(path, "%s/postgresql.auto.conf", pg_data);
 
        writefile(path, autoconflines);
-       if (chmod(path, PG_FILE_MODE_DEFAULT) != 0)
+       if (chmod(path, PG_FILE_MODE_DEFAULT & ~file_mode_mask) != 0)
        {
                fprintf(stderr, _("%s: could not change permissions of \"%s\": 
%s\n"),
                                progname, path, strerror(errno));
@@ -1278,7 +1279,7 @@ setup_config(void)
        snprintf(path, sizeof(path), "%s/pg_hba.conf", pg_data);
 
        writefile(path, conflines);
-       if (chmod(path, PG_FILE_MODE_DEFAULT) != 0)
+       if (chmod(path, PG_FILE_MODE_DEFAULT & ~file_mode_mask) != 0)
        {
                fprintf(stderr, _("%s: could not change permissions of \"%s\": 
%s\n"),
                                progname, path, strerror(errno));
@@ -1294,7 +1295,7 @@ setup_config(void)
        snprintf(path, sizeof(path), "%s/pg_ident.conf", pg_data);
 
        writefile(path, conflines);
-       if (chmod(path, PG_FILE_MODE_DEFAULT) != 0)
+       if (chmod(path, PG_FILE_MODE_DEFAULT & ~file_mode_mask) != 0)
        {
                fprintf(stderr, _("%s: could not change permissions of \"%s\": 
%s\n"),
                                progname, path, strerror(errno));
@@ -2312,6 +2313,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"
@@ -2711,7 +2713,7 @@ create_data_directory(void)
                                   pg_data);
                        fflush(stdout);
 
-                       if (chmod(pg_data, PG_DIR_MODE_DEFAULT) != 0)
+                       if (chmod(pg_data, PG_DIR_MODE_DEFAULT & 
~file_mode_mask) != 0)
                        {
                                fprintf(stderr, _("%s: could not change 
permissions of directory \"%s\": %s\n"),
                                                progname, pg_data, 
strerror(errno));
@@ -2797,7 +2799,7 @@ create_xlog_or_symlink(void)
                                           xlog_dir);
                                fflush(stdout);
 
-                               if (chmod(xlog_dir, PG_DIR_MODE_DEFAULT) != 0)
+                               if (chmod(xlog_dir, PG_DIR_MODE_DEFAULT & 
~file_mode_mask) != 0)
                                {
                                        fprintf(stderr, _("%s: could not change 
permissions of directory \"%s\": %s\n"),
                                                        progname, xlog_dir, 
strerror(errno));
@@ -2884,7 +2886,7 @@ initialize_data_directory(void)
        setup_signals();
 
        /* Set the file mode creation mask */
-       umask(PG_MODE_MASK_DEFAULT);
+       umask(file_mode_mask);
 
        create_data_directory();
 
@@ -3018,6 +3020,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 +3062,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 +3156,9 @@ main(int argc, char *argv[])
                        case 12:
                                str_wal_segment_size_mb = pg_strdup(optarg);
                                break;
+                       case 'g':
+                               file_mode_mask = PG_MODE_MASK_ALLOW_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 3331560445..f08fc6ea8f 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";
@@ -50,3 +52,12 @@ mkdir $datadir;
 }
 command_ok([ 'initdb', '-S', $datadir ], 'sync only');
 command_fails([ 'initdb', $datadir ], 'existing data directory');
+
+# 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));
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 1eef7179cc..bb70e625fd 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -2171,6 +2171,7 @@ main(int argc, char **argv)
         */
        argv0 = argv[0];
 
+       /* Set restrictive mode mask until PGDATA permissions are checked */
        umask(PG_MODE_MASK_DEFAULT);
 
        /* support --help and --version even if invoked as root */
@@ -2406,6 +2407,9 @@ 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 */
+               umask(DataDirectoryMask(pg_data));
        }
 
        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 3d82abe696..7318c27c85 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 => 20;
+use Test::More tests => 24;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -68,4 +70,24 @@ command_ok(
 ok(-f $logFileName);
 ok(check_mode_recursive("$tempdir/data", 0700, 0600));
 
+# Log file for second perm test
+$logFileName = "$tempdir/data/perm-test-640.log";
+
+# Change the data dir mode so log file will be created with group read
+# privileges on the next start
+system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data";
+
+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, ['postmaster.pid']));
+
+command_ok(
+       [ 'pg_ctl', 'restart', '-D', "$tempdir/data", '-l', $logFileName ],
+       'pg_ctl restart with server running');
+
 system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data";
diff --git a/src/bin/pg_resetwal/pg_resetwal.c 
b/src/bin/pg_resetwal/pg_resetwal.c
index 47abd41fc3..6a4dc502c4 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -328,8 +328,8 @@ main(int argc, char *argv[])
                exit(1);
        }
 
-       /* Set dir/file mode mask */
-       umask(PG_MODE_MASK_DEFAULT);
+       /* Set mask based on PGDATA permissions */
+       umask(DataDirectoryMask(DataDir));
 
        /* Check that data directory matches our server version */
        CheckDataVersion();
diff --git a/src/bin/pg_resetwal/t/001_basic.pl 
b/src/bin/pg_resetwal/t/001_basic.pl
index 8f3f2990f3..8736130731 100644
--- a/src/bin/pg_resetwal/t/001_basic.pl
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 14;
+use Test::More tests => 15;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -40,9 +40,12 @@ ok(check_mode_recursive($pgdata, 0700, 0600), 'check PGDATA 
permissions');
 
 $node->start;
 
-# Reset to specific WAL segment
+# Reset to specific WAL segment.  Also enable group access to make sure files
+# and directories are created with group permissions.
 $node->stop;
 
+chmod_recursive($pgdata, 0750, 0640);
+
 $node->command_ok(
        ['pg_resetwal', '-l', '000000070000000700000007', '-D', $pgdata],
        'set to specific WAL');
@@ -52,4 +55,6 @@ is_deeply(
        [sort(qw(. .. archive_status 000000070000000700000007))],
        'WAL recreated');
 
+ok(check_mode_recursive($pgdata, 0750, 0640), 'check PGDATA permissions');
+
 $node->start;
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 1751de1a35..9dab6bd7e8 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 die("unable to set permissions for 
$master_pgdata/postgresql.conf");
 
        # Plug-in rewound node to the now-promoted standby node
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 3d179e2c7d..11153e5f2f 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -186,8 +186,8 @@ main(int argc, char **argv)
                exit(1);
        }
 
-       /* Set dir/file mode mask */
-       umask(PG_MODE_MASK_DEFAULT);
+       /* Set mask based on PGDATA permissions */
+       umask(DataDirectoryMask(datadir_target));
 
        /*
         * Don't allow pg_rewind to be run as root, to avoid overwriting the
diff --git a/src/bin/pg_rewind/t/002_databases.pl 
b/src/bin/pg_rewind/t/002_databases.pl
index 37cdd712f3..570fa5cee0 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,8 @@ template1
 ),
                'database names');
 
+       ok (check_mode_recursive(
+               $node_master->data_dir(), 0750, 0640, ['postmaster.pid']));
        RewindTest::clean_rewind_test();
 }
 
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 59df6fd88f..54f54fb0a4 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_DEFAULT);
 
        parseCommandLine(argc, argv);
@@ -100,6 +100,9 @@ main(int argc, char **argv)
 
        check_cluster_compatibility(live_check);
 
+       /* Set mask based on new PGDATA permissions */
+       umask(DataDirectoryMask(new_cluster.pgdata));
+
        check_and_dump_old_cluster(live_check);
 
 
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 4e8db4aaf4..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"
@@ -231,13 +231,13 @@ standard_initdb 'initdb'
 pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" 
-B "$bindir" -p "$PGPORT" -P "$PGPORT"
 
 # make sure all directories and files have group permissions
-if [ $(find ${PGDATA} -type f ! -perm 600 | wc -l) -ne 0 ]; then
-       echo "files in PGDATA with permission != 600";
+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/Makefile b/src/common/Makefile
index 80e78d72fe..504375bef4 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -51,7 +51,7 @@ else
 OBJS_COMMON += sha2.o
 endif
 
-OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o file_utils.o restricted_token.o
+OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o file_perm.o file_utils.o 
restricted_token.o
 
 OBJS_SRV = $(OBJS_COMMON:%.o=%_srv.o)
 
diff --git a/src/common/file_perm.c b/src/common/file_perm.c
new file mode 100644
index 0000000000..f01d43725e
--- /dev/null
+++ b/src/common/file_perm.c
@@ -0,0 +1,48 @@
+/*-------------------------------------------------------------------------
+ *
+ * File and directory permission routines
+ *
+ * Group read/execute may optional be enabled on PGDATA so any frontend tools
+ * That write into PGDATA must know what mask to set and the permissions to
+ * use for creating files and directories.
+ *
+ *
+ * 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
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifndef FRONTEND
+#error "This file is not expected to be compiled for backend code"
+#endif
+
+#include <sys/stat.h>
+
+#include "common/file_perm.h"
+
+/*
+ * Determine the mask to use when writing to PGDATA.
+ *
+ * Errors are not handled here and should be checked by the frontend
+ * application.
+ */
+mode_t
+DataDirectoryMask(const char *dataDir)
+{
+       struct stat statBuf;
+
+       /*
+        * If an error occurs getting the mode then return the more restrictive 
mask.
+        * It is the reponsibility of the frontend application to generate an 
error.
+        */
+       if (stat(dataDir, &statBuf) != 0)
+               return PG_MODE_MASK_DEFAULT;
+
+       /*
+        * Construct the mask that the caller should pass to umask().
+        */
+       return PG_MODE_MASK_DEFAULT & ~statBuf.st_mode;
+}
diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h
index 3c6da0e000..35dcbe6108 100644
--- a/src/include/common/file_perm.h
+++ b/src/include/common/file_perm.h
@@ -1,6 +1,10 @@
 /*-------------------------------------------------------------------------
  *
- * File and directory permission constants
+ * File and directory permission constants and routines
+ *
+ * Group read/execute may optional be enabled on PGDATA so any frontend tools
+ * That write into PGDATA must know what mask to set and the permissions to
+ * use for creating files and directories.
  *
  *
  * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
@@ -20,13 +24,30 @@
 #define PG_MODE_MASK_DEFAULT           (S_IRWXG | S_IRWXO)
 
 /*
- * Default mode for created files.
+ * Optional mode mask for data directory permissions that allows group
+ * read/execute.
+ */
+#define PG_MODE_MASK_ALLOW_GROUP       (S_IWGRP | S_IRWXO)
+
+/*
+ * Default mode for created files, unless something else is specified using
+ * the *Perm() function variants.
+ */
+#define PG_FILE_MODE_DEFAULT           (S_IRUSR | S_IWUSR | S_IRGRP)
+
+/*
+ * Default mode for directories created with MakeDirectoryDefaultPerm().
  */
-#define PG_FILE_MODE_DEFAULT           (S_IRUSR | S_IWUSR)
+#define PG_DIR_MODE_DEFAULT                    (S_IRWXU | S_IRGRP | S_IXGRP)
+
+#ifdef FRONTEND
 
 /*
- * Default mode for directories.
+ * Determine the mask to use when writing to PGDATA
  */
-#define PG_DIR_MODE_DEFAULT                    S_IRWXU
+mode_t DataDirectoryMask(const char *dataDir);
+
+#endif                                                 /* FRONTEND */
+
 
 #endif                                                 /* FILE_PERM_H */
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 9de8dc9041..93ac24e261 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -30,6 +30,7 @@ our @EXPORT = qw(
   slurp_file
   append_to_file
   check_mode_recursive
+  chmod_recursive
   check_pg_config
   system_or_bail
   system_log
@@ -295,6 +296,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.
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 72976f44d8..78d4934ec6 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -125,7 +125,7 @@ sub mkvcbuild
        }
 
        our @pgcommonfrontendfiles = (
-               @pgcommonallfiles, qw(fe_memutils.c file_utils.c
+               @pgcommonallfiles, qw(fe_memutils.c file_perm.c file_utils.c
                  restricted_token.c));
 
        our @pgcommonbkndfiles = @pgcommonallfiles;
-- 
2.14.3 (Apple Git-98)

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to