Hi Robert,
Thanks for looking at the patches.
On 12/31/17 1:27 PM, Robert Haas wrote:
On Thu, Dec 28, 2017 at 2:36 PM, David Steele <da...@pgmasters.net> wrote:
Attached is a new patch set that should address various concerns raised in
this thread.
1) group-access-v3-01-mkdir.patch
Abstracts all mkdir calls in the backend into a MakeDirectory() function
implemented in fd.c. This did not get committed in September as part of
0c5803b450e but I still think it has value. However, I have kept it
separate to reduce noise in the second patch. The mkdir() calls could also
be modified to use PG_DIR_MODE_DEFAULT with equivalent results.
+/*
+ * Default mode for created directories, unless something else is specified
+ * using the MakeDirectoryPerm() function variant.
+ */
+#define PG_DIR_MODE_DEFAULT (S_IRWXU)
+#define PG_MODE_MASK_DEFAULT (S_IRWXG | S_IRWXO)
There's only one comment here, but there are two constants that do
different things.
Looks like a copy pasto - I didn't mean PG_MODE_MASK_DEFAULT to be in
patch 01 at all. Removed.
Also, this hunk gets removed again by 02, which is probably OK, but 02
adds the replacement stuff in a different part of the file, which
seems not as good.
Agreed. I have moved that.
> I think MakeDirectory() is a good wrapper, but isn't
MakeDirectoryPerm() sort of silly?
There's one place in the backend (storage/ipc/ipc.c) that sets
non-default directory permissions. This function is intended to support
that and any extensions that need to set custom perms.
2) group-access-v3-02-group.patch
This is a "GUC-less" implementation of group read access that depends on the
mode of the $PGDATA directory to determine which mode to use for subsequent
writes. The initdb option is preserved to allow group access to be enabled
when the cluster is initialized.
Only two modes are allowed (700, 750) and the error message on startup is
hard-coded to address translation concerns.
+ umask(~(stat_buf.st_mode & PG_DIR_MODE_DEFAULT));
Hmm, I wonder if this is going to be 100% portable. Maybe some
obscure platform won't like an argument with all the high bits set.
Sure - I have masked this with 0777 to clear any high bits. Sound OK?
Also, why should we break what's now one #if into two? That seems
like it's mildly more complicated for no gain.
There were already two #ifdef blocks so I added a third. I think we
should leave it as three or combine them all into one. There are no
runtime performance implications so I'm agnostic.
+ (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.
Is that actually the behavior we want?
I think it is. Comments in the code are pretty specific about not
changing the permissions on postmaster.pid and I don't see any reason to
mess with it. Copying postmaster.pid as part of a backup is not a good
idea anyway.
New patches attached.
Thanks!
--
-David
da...@pgmasters.net
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 3e9a12dacd..cc6e46f7c4 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 (MakeDirectory(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 d574e4dd00..a0cafedd9b 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -151,7 +151,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool
isRedo)
else
{
/* Directory creation failed? */
- if (mkdir(dir, S_IRWXU) < 0)
+ if (MakeDirectory(dir) < 0)
{
char *parentdir;
@@ -173,7 +173,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 (MakeDirectory(parentdir) < 0 &&
errno != EEXIST)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could
not create directory \"%s\": %m",
@@ -184,7 +184,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 (MakeDirectory(parentdir) < 0 &&
errno != EEXIST)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could
not create directory \"%s\": %m",
@@ -192,7 +192,7 @@ TablespaceCreateDbspace(Oid spcNode, Oid dbNode, bool
isRedo)
pfree(parentdir);
/* Create database directory */
- if (mkdir(dir, S_IRWXU) < 0)
+ if (MakeDirectory(dir) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could
not create directory \"%s\": %m",
@@ -279,7 +279,7 @@ 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 MakeDirectory() uses the first
two parts.
*/
if (strlen(location) + 1 + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 +
OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1 + OIDCHARS >
MAXPGPATH)
@@ -574,7 +574,7 @@ create_tablespace_directories(const char *location, const
Oid tablespaceoid)
* Attempt to coerce target directory to safe permissions. If this
fails,
* it doesn't exist or has the wrong owner.
*/
- if (chmod(location, S_IRWXU) != 0)
+ if (chmod(location, PG_DIR_MODE_DEFAULT) != 0)
{
if (errno == ENOENT)
ereport(ERROR,
@@ -599,7 +599,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, MakeDirectory() 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 +610,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 (MakeDirectory(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 17c7f7e78f..aeda54f00b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4495,7 +4495,7 @@ internal_forkexec(int argc, char *argv[], Port *port)
* As in OpenTemporaryFileInTablespace, try to make the
temp-file
* directory
*/
- mkdir(PG_TEMP_FILES_DIR, S_IRWXU);
+ MakeDirectory(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 aeb117796d..ffb7540355 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);
+ MakeDirectory(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);
+ MakeDirectory(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 0d27b6f39e..6d10699c2c 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1166,13 +1166,13 @@ 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 MakeDirectory() 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 (MakeDirectory(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 d169e9c8bb..784400a285 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 (MakeDirectory(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 f449ee5c51..342401b8dd 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1434,7 +1434,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 (MakeDirectory(directory) < 0)
{
if (errno == EEXIST)
return;
@@ -1444,14 +1444,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 (MakeDirectory(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 (MakeDirectory(directory) < 0 && errno != EEXIST)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("cannot create temporary
subdirectory \"%s\": %m",
@@ -1601,11 +1601,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
+ * Don't check error from MakeDirectory; 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);
+ MakeDirectory(tempdirpath);
file = PathNameOpenFile(tempfilepath,
O_RDWR |
O_CREAT | O_TRUNC | PG_BINARY);
@@ -3533,3 +3533,22 @@ fsync_parent_path(const char *fname, int elevel)
return 0;
}
+
+/*
+ * Create a directory with MakeDirectoryPerm() and pass PG_DIR_MODE_DEFAULT to
+ * the directoryMode parameter.
+ */
+int
+MakeDirectory(const char *directoryName)
+{
+ return MakeDirectoryPerm(directoryName, PG_DIR_MODE_DEFAULT);
+}
+
+/*
+ * Create a directory with the specified mode.
+ */
+int
+MakeDirectoryPerm(const char *directoryName, mode_t directoryMode)
+{
+ return mkdir(directoryName, directoryMode);
+}
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index dfb47e7c39..f3c9d9f2d3 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -132,8 +132,8 @@ proc_exit(int code)
else
snprintf(gprofDirName, 32, "gprof/%d", (int) getpid());
- mkdir("gprof", S_IRWXU | S_IRWXG | S_IRWXO);
- mkdir(gprofDirName, S_IRWXU | S_IRWXG | S_IRWXO);
+ MakeDirectoryPerm("gprof", S_IRWXU | S_IRWXG | S_IRWXO);
+ MakeDirectoryPerm(gprofDirName, S_IRWXU | S_IRWXG | S_IRWXO);
chdir(gprofDirName);
}
#endif
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index dc2eb35f06..bc2d7bc063 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -57,6 +57,11 @@ extern PGDLLIMPORT int max_files_per_process;
*/
extern int max_safe_fds;
+/*
+ * Default mode for created directories, unless something else is specified
+ * using the MakeDirectoryPerm() function variant.
+ */
+#define PG_DIR_MODE_DEFAULT (S_IRWXU)
/*
* prototypes for functions in fd.c
@@ -111,6 +116,10 @@ extern int CloseTransientFile(int fd);
extern int BasicOpenFile(const char *fileName, int fileFlags);
extern int BasicOpenFilePerm(const char *fileName, int fileFlags, mode_t
fileMode);
+ /* Operations to make directories */
+extern int MakeDirectory(const char *directoryName);
+extern int MakeDirectoryPerm(const char *directoryName, mode_t directoryMode);
+
/* Miscellaneous support routines */
extern void InitFileAccess(void);
extern void set_max_safe_fds(void);
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
@@ -77,6 +77,14 @@ PostgreSQL documentation
</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,
collation order (<literal>LC_COLLATE</literal>) and character set classes
@@ -302,6 +310,17 @@ PostgreSQL documentation
</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>
<listitem>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index a2ebd3e21c..8f602dc705 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>
@@ -2237,6 +2240,15 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433
</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
been entered.
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index aeda54f00b..b780b70e53 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -587,7 +587,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);
@@ -1524,25 +1525,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",
- DataDir),
- errdetail("Permissions should be u=rwx
(0700).")));
+ errmsg("data directory \"%s\" has invalid
permissions",
+ DataDir),
+ 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(~(stat_buf.st_mode & PG_DIR_MODE_DEFAULT) & 0777);
#endif
/* Look for PG_VERSION before looking for pg_control */
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 342401b8dd..52a4a31ce1 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -125,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.
* This GUC parameter lets the DBA limit max_safe_fds to something less than
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ddc850db1c..afc94c7f94 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -113,6 +113,12 @@ static const char *const auth_methods_local[] = {
NULL
};
+/* Default file mode creation mask */
+#define PG_MODE_MASK_DEFAULT (S_IRWXG | S_IRWXO)
+
+/* File mode mask that allows group access */
+#define PG_MODE_MASK_ALLOW_GROUP (S_IWGRP | S_IRWXO)
+
/*
* these values are passed in by makefile defines
*/
@@ -144,7 +150,13 @@ 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;
+
+/* File mode to use with chmod on files */
+#define PG_FILE_MODE ((S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
S_IWOTH) & ~file_mode_mask)
+/* File mode to use with chmod on directories */
+#define PG_DIR_MODE ((S_IRWXU | S_IRWXG | S_IRWXO) & ~file_mode_mask)
/* internal vars */
static const char *progname;
@@ -1170,7 +1182,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) != 0)
{
fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
progname, path, strerror(errno));
@@ -1190,7 +1202,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) != 0)
{
fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
progname, path, strerror(errno));
@@ -1277,7 +1289,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) != 0)
{
fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
progname, path, strerror(errno));
@@ -1293,7 +1305,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) != 0)
{
fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
progname, path, strerror(errno));
@@ -2311,6 +2323,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 set file mode creation mask to
0027\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"
@@ -2692,7 +2705,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) != 0)
{
fprintf(stderr, _("%s: could not create
directory \"%s\": %s\n"),
progname, pg_data,
strerror(errno));
@@ -2710,7 +2723,7 @@ create_data_directory(void)
pg_data);
fflush(stdout);
- if (chmod(pg_data, S_IRWXU) != 0)
+ if (chmod(pg_data, PG_DIR_MODE) != 0)
{
fprintf(stderr, _("%s: could not change
permissions of directory \"%s\": %s\n"),
progname, pg_data,
strerror(errno));
@@ -2778,7 +2791,8 @@ create_xlog_or_symlink(void)
xlog_dir);
fflush(stdout);
- if (pg_mkdir_p(xlog_dir, S_IRWXU) != 0)
+ printf("MASK: %04o\n", file_mode_mask);
+ if (pg_mkdir_p(xlog_dir, PG_DIR_MODE) != 0)
{
fprintf(stderr, _("%s: could not create
directory \"%s\": %s\n"),
progname, xlog_dir,
strerror(errno));
@@ -2796,7 +2810,8 @@ create_xlog_or_symlink(void)
xlog_dir);
fflush(stdout);
- if (chmod(xlog_dir, S_IRWXU) != 0)
+ printf("MASK: %04o\n", file_mode_mask);
+ if (chmod(xlog_dir, PG_DIR_MODE) != 0)
{
fprintf(stderr, _("%s: could not change
permissions of directory \"%s\": %s\n"),
progname, xlog_dir,
strerror(errno));
@@ -2846,7 +2861,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) < 0)
{
fprintf(stderr, _("%s: could not create directory
\"%s\": %s\n"),
progname, subdirloc, strerror(errno));
@@ -2882,7 +2897,9 @@ initialize_data_directory(void)
setup_signals();
- umask(S_IRWXG | S_IRWXO);
+ /* Set the file mode creation mask. */
+ umask(file_mode_mask);
+ printf(_("Initializing with file mode creation mask: %04o\n"),
file_mode_mask);
create_data_directory();
@@ -2902,7 +2919,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) < 0)
{
fprintf(stderr, _("%s: could not create directory
\"%s\": %s\n"),
progname, path, strerror(errno));
@@ -3016,6 +3033,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}
};
@@ -3057,7 +3075,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)
{
@@ -3150,6 +3168,8 @@ main(int argc, char *argv[])
break;
case 12:
str_wal_segment_size_mb = pg_strdup(optarg);
+ case 'g':
+ file_mode_mask = PG_MODE_MASK_ALLOW_GROUP;
break;
default:
/* getopt_long already emitted a complaint */
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index bc2d7bc063..5258584cd3 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -58,10 +58,28 @@ extern PGDLLIMPORT int max_files_per_process;
extern int max_safe_fds;
/*
+ * Default mode mask for data directory permissions that does not allow
+ * group execute/read.
+ */
+#define PG_MODE_MASK_DEFAULT (S_IRWXG | S_IRWXO)
+
+/*
+ * 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 created directories, unless something else is specified
* using the MakeDirectoryPerm() function variant.
*/
-#define PG_DIR_MODE_DEFAULT (S_IRWXU)
+#define PG_DIR_MODE_DEFAULT (S_IRWXU | S_IRGRP | S_IXGRP)
/*
* prototypes for functions in fd.c
