On 1/19/18 4:43 PM, Peter Eisentraut wrote:
> On 1/19/18 14:07, David Steele wrote:
>> I have yet to add tests for pg_rewindwal and pg_upgrade. pg_rewindwal
>> doesn't *have* any tests as far as I can tell and pg_upgrade has tests
>> in a shell script -- it's not clear how I would extend it or reuse the
>> Perl code for perm testing.
>>
>> Does anyone have suggestions on tests for pg_resetwal and pg_upgrade?
>> Should I start from scratch?
>
> A test suite for pg_resetwal would be nice.
Agreed.
> TAP tests for pg_upgrade will create problems with the build farm.
> There was a recent thread about that.
OK, that being the case I have piggy-backed on the current pg_upgrade
tests in the same way that --wal-segsize did.
There are now three patches:
1) 01-pgresetwal-test
Adds a *very* basic test suite for pg_resetwal. I was able to make this
utility core dump (floating point errors) pretty easily with empty or
malformed pg_control files so I focused on WAL reset functionality plus
the basic help/command tests that every utility has.
2) 02-mkdir
Converts mkdir() calls to MakeDirectoryDefaultPerm() if the original
call used default permissions.
3) 03-group
Allow group access on PGDATA. This includes backend changes, utility
changes (initdb, pg_ctl, pg_upgrade, etc.) and tests for each utility.
Regards,
--
-David
da...@pgmasters.net
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;
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index e42b828edf..9814ac6d3e 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..746b8c121b 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 (MakeDirectoryDefaultPerm(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 (MakeDirectoryDefaultPerm(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 (MakeDirectoryDefaultPerm(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 (MakeDirectoryDefaultPerm(dir) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could
not create directory \"%s\": %m",
@@ -279,7 +279,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)
@@ -574,7 +575,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 +600,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 +611,7 @@ create_tablespace_directories(const char *location, const
Oid tablespaceoid)
* The creation of the version directory prevents more than one
tablespace
* in a single location.
*/
- if (mkdir(location_with_version_dir, S_IRWXU) < 0)
+ if (MakeDirectoryDefaultPerm(location_with_version_dir) < 0)
{
if (errno == EEXIST)
ereport(ERROR,
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index f3ddf828bb..e13a2ae8c4 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4495,7 +4495,7 @@ internal_forkexec(int argc, char *argv[], Port *port)
* As in OpenTemporaryFileInTablespace, try to make the
temp-file
* directory
*/
- mkdir(PG_TEMP_FILES_DIR, S_IRWXU);
+ MakeDirectoryDefaultPerm(PG_TEMP_FILES_DIR);
fp = AllocateFile(tmpfilename, PG_BINARY_W);
if (!fp)
diff --git a/src/backend/postmaster/syslogger.c
b/src/backend/postmaster/syslogger.c
index f70eea37df..ae23941f59 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -41,6 +41,7 @@
#include "postmaster/postmaster.h"
#include "postmaster/syslogger.h"
#include "storage/dsm.h"
+#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/latch.h"
#include "storage/pg_shmem.h"
@@ -322,7 +323,7 @@ SysLoggerMain(int argc, char *argv[])
/*
* Also, create new directory if not present;
ignore errors
*/
- mkdir(Log_directory, S_IRWXU);
+ MakeDirectoryDefaultPerm(Log_directory);
}
if (strcmp(Log_filename, currentLogFilename) != 0)
{
@@ -564,7 +565,7 @@ SysLogger_Start(void)
/*
* Create log directory if not present; ignore errors
*/
- mkdir(Log_directory, S_IRWXU);
+ MakeDirectoryDefaultPerm(Log_directory);
/*
* The initial logfile is created right in the postmaster, to verify
that
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index fc9ef22b0b..3a98360b50 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1166,13 +1166,14 @@ CreateSlotOnDisk(ReplicationSlot *slot)
* It's just barely possible that some previous effort to create or
drop a
* slot with this name left a temp directory lying around. If that seems
* to be the case, try to remove it. If the rmtree() fails, we'll error
- * out at the mkdir() below, so we don't bother checking success.
+ * out at the MakeDirectoryDefaultPerm() below, so we don't bother
checking
+ * success.
*/
if (stat(tmppath, &st) == 0 && S_ISDIR(st.st_mode))
rmtree(tmppath, true);
/* Create and fsync the temporary slot directory. */
- if (mkdir(tmppath, S_IRWXU) < 0)
+ if (MakeDirectoryDefaultPerm(tmppath) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/backend/storage/file/copydir.c
b/src/backend/storage/file/copydir.c
index ca6342db0d..ff980dc1f5 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -41,7 +41,7 @@ copydir(char *fromdir, char *todir, bool recurse)
char fromfile[MAXPGPATH * 2];
char tofile[MAXPGPATH * 2];
- if (mkdir(todir, S_IRWXU) != 0)
+ if (MakeDirectoryDefaultPerm(todir) != 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\":
%m", todir)));
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 71516a9a5a..9aeca8a04d 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1435,7 +1435,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 +1445,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 +1602,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);
@@ -3545,3 +3545,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 2de35efbd4..687df55444 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -132,6 +132,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/include/storage/fd.h b/src/include/storage/fd.h
index db5ca16679..909d7202e9 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -57,6 +57,10 @@ extern PGDLLIMPORT int max_files_per_process;
*/
extern int max_safe_fds;
+/*
+ * Default mode for directories created with MakeDirectoryDefaultPerm().
+ */
+#define PG_DIR_MODE_DEFAULT (S_IRWXU)
/*
* prototypes for functions in fd.c
@@ -111,6 +115,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/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 d162acb2e8..1c6fbf2920 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>
@@ -2236,6 +2239,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/commands/tablespace.c
b/src/backend/commands/tablespace.c
index 746b8c121b..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"
@@ -566,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,
@@ -575,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, PG_DIR_MODE_DEFAULT) != 0)
+ current_umask = umask(0);
+ umask(current_umask);
+
+ if (chmod(location, PG_DIR_MODE_DEFAULT & ~current_umask) != 0)
{
if (errno == ENOENT)
ereport(ERROR,
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index e13a2ae8c4..65f528147a 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);
@@ -1524,25 +1526,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/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 9aeca8a04d..21c2456e63 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.
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 2efd3b75b1..56356d75c5 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -64,6 +64,7 @@
#include "catalog/pg_authid.h"
#include "catalog/pg_class.h"
#include "catalog/pg_collation.h"
+#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/restricted_token.h"
#include "common/username.h"
@@ -144,6 +145,7 @@ static bool data_checksums = false;
static char *xlog_dir = NULL;
static char *str_wal_segment_size_mb = NULL;
static int wal_segment_size_mb;
+static mode_t file_mode_mask = PG_MODE_MASK_DEFAULT;
/* internal vars */
@@ -1170,12 +1172,6 @@ setup_config(void)
snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data);
writefile(path, conflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
/*
* create the automatic configuration file to store the configuration
@@ -1190,12 +1186,6 @@ setup_config(void)
sprintf(path, "%s/postgresql.auto.conf", pg_data);
writefile(path, autoconflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
free(conflines);
@@ -1277,12 +1267,6 @@ setup_config(void)
snprintf(path, sizeof(path), "%s/pg_hba.conf", pg_data);
writefile(path, conflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
free(conflines);
@@ -1293,12 +1277,6 @@ setup_config(void)
snprintf(path, sizeof(path), "%s/pg_ident.conf", pg_data);
writefile(path, conflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
free(conflines);
@@ -2311,6 +2289,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"
@@ -2692,7 +2671,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 +2689,7 @@ create_data_directory(void)
pg_data);
fflush(stdout);
- if (chmod(pg_data, S_IRWXU) != 0)
+ if (chmod(pg_data, PG_DIR_MODE_DEFAULT &
~file_mode_mask) != 0)
{
fprintf(stderr, _("%s: could not change
permissions of directory \"%s\": %s\n"),
progname, pg_data,
strerror(errno));
@@ -2778,7 +2757,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 +2775,7 @@ create_xlog_or_symlink(void)
xlog_dir);
fflush(stdout);
- if (chmod(xlog_dir, S_IRWXU) != 0)
+ if (chmod(xlog_dir, PG_DIR_MODE_DEFAULT &
~file_mode_mask) != 0)
{
fprintf(stderr, _("%s: could not change
permissions of directory \"%s\": %s\n"),
progname, xlog_dir,
strerror(errno));
@@ -2846,7 +2825,7 @@ create_xlog_or_symlink(void)
else
{
/* Without -X option, just make the subdirectory normally */
- if (mkdir(subdirloc, S_IRWXU) < 0)
+ if (mkdir(subdirloc, PG_DIR_MODE_DEFAULT) < 0)
{
fprintf(stderr, _("%s: could not create directory
\"%s\": %s\n"),
progname, subdirloc, strerror(errno));
@@ -2882,8 +2861,6 @@ initialize_data_directory(void)
setup_signals();
- umask(S_IRWXG | S_IRWXO);
-
create_data_directory();
create_xlog_or_symlink();
@@ -2902,7 +2879,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));
@@ -3016,6 +2993,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 +3035,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)
{
@@ -3151,6 +3129,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"),
@@ -3159,6 +3140,8 @@ main(int argc, char *argv[])
}
}
+ /* Set the file mode creation mask */
+ umask(file_mode_mask);
/*
* Non-option argument specifies data directory as long as it wasn't
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index c0cfa6e92c..f767f07ed5 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 => 15;
+use Test::More tests => 18;
my $tempdir = TestLib::tempdir;
my $xlogdir = "$tempdir/pgxlog";
@@ -45,6 +47,17 @@ mkdir $datadir;
command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ],
'successful creation');
+
+ ok(check_pg_data_perm($datadir, 0));
}
command_ok([ 'initdb', '-S', $datadir ], 'sync only');
command_fails([ 'initdb', $datadir ], 'existing data directory');
+
+# 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_pg_data_perm($datadir_group, 1));
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 9bc830b085..bb70e625fd 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -25,6 +25,7 @@
#include "catalog/pg_control.h"
#include "common/controldata_utils.h"
+#include "common/file_perm.h"
#include "getopt_long.h"
#include "utils/pidfile.h"
@@ -2170,7 +2171,8 @@ main(int argc, char **argv)
*/
argv0 = argv[0];
- umask(S_IRWXG | S_IRWXO);
+ /* Set restrictive mode mask until PGDATA permissions are checked */
+ umask(PG_MODE_MASK_DEFAULT);
/* support --help and --version even if invoked as root */
if (argc > 1)
@@ -2405,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 5da4746cb4..b04f54e734 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 => 19;
+use Test::More tests => 25;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -57,10 +59,37 @@ 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" ],
+
+# Log file should exist and have no group permissions
+ok(-f $logFileName);
+ok(check_pg_data_perm("$tempdir/data", 0));
+
+# 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";
+
+command_ok(
+ [ 'chmod', "-R", 'g+rX', "$tempdir/data" ],
+ 'add group perms to PGDATA');
+
+command_ok(
+ [ 'pg_ctl', 'start', '-D', "$tempdir/data", '-l', $logFileName ],
+ 'start server to check group permissions');
+
+ok(-f $logFileName);
+ok(check_pg_data_perm("$tempdir/data", 1));
+
+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 a132cf2e9f..6a4dc502c4 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 mask based on PGDATA permissions */
+ umask(DataDirectoryMask(DataDir));
+
/* 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..184e2ea3a8 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 => 16;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -36,11 +36,18 @@ is_deeply(
[sort(qw(. .. archive_status 000000010000000000000002))],
'WAL recreated');
+ok(check_pg_data_perm($pgdata, 0), 'check PGDATA permissions');
+
$node->start;
-# Reset to specific WAL segment
+# Reset to specific WAL segment. Also enable group access to make sure files
+# and directories are created with group permissions.
$node->stop;
+command_ok(
+ ['chmod', "-R", 'g+rX', "$pgdata"],
+ 'add group perms to PGDATA');
+
$node->command_ok(
['pg_resetwal', '-l', '000000070000000700000007', '-D', $pgdata],
'set to specific WAL');
@@ -50,4 +57,6 @@ is_deeply(
[sort(qw(. .. archive_status 000000070000000700000007))],
'WAL recreated');
+ok(check_pg_data_perm($pgdata, 1), 'check PGDATA permissions');
+
$node->start;
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 42fd577f21..5b310f208b 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -115,10 +115,12 @@ sub check_query
sub setup_cluster
{
my $extra_name = shift;
+ my $group_access = shift;
# 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,
+ has_group_access => defined($group_access) ? $group_access : 0);
# Set wal_keep_segments to prevent WAL segment recycling after enforced
# checkpoints in the tests.
$node_master->append_conf('postgresql.conf', qq(
@@ -236,6 +238,9 @@ sub run_pg_rewind
"$tmp_folder/master-postgresql.conf.tmp",
"$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
my $port_standby = $node_standby->port;
$node_master->append_conf(
@@ -254,6 +259,9 @@ recovery_target_timeline='latest'
# Clean up after the test. Stop both servers, if they're still running.
sub clean_rewind_test
{
+ ok (check_pg_data_perm(
+ $node_master->data_dir(), $node_master->group_access()));
+
$node_master->teardown_node if defined $node_master;
$node_standby->teardown_node if defined $node_standby;
}
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index 705383d184..d51914e901 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -18,6 +18,7 @@
#include <fcntl.h>
#include <unistd.h>
+#include "common/file_perm.h"
#include "file_ops.h"
#include "filemap.h"
#include "logging.h"
@@ -58,7 +59,7 @@ open_target_file(const char *path, bool trunc)
mode = O_WRONLY | O_CREAT | PG_BINARY;
if (trunc)
mode |= O_TRUNC;
- dstfd = open(dstpath, mode, 0600);
+ dstfd = open(dstpath, mode, PG_FILE_MODE_DEFAULT);
if (dstfd < 0)
pg_fatal("could not open target file \"%s\": %s\n",
dstpath, strerror(errno));
@@ -190,7 +191,7 @@ truncate_target_file(const char *path, off_t newsize)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
- fd = open(dstpath, O_WRONLY, 0);
+ fd = open(dstpath, O_WRONLY, PG_FILE_MODE_DEFAULT);
if (fd < 0)
pg_fatal("could not open file \"%s\" for truncation: %s\n",
dstpath, strerror(errno));
@@ -211,7 +212,7 @@ create_target_dir(const char *path)
return;
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
- if (mkdir(dstpath, S_IRWXU) != 0)
+ if (mkdir(dstpath, PG_DIR_MODE_DEFAULT) != 0)
pg_fatal("could not create directory \"%s\": %s\n",
dstpath, strerror(errno));
}
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 72ab2f8d5e..11153e5f2f 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 mask based on PGDATA permissions */
+ umask(DataDirectoryMask(datadir_target));
+
/*
* 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..205bdd77ef 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;
@@ -9,7 +9,7 @@ sub run_test
{
my $test_mode = shift;
- RewindTest::setup_cluster($test_mode);
+ RewindTest::setup_cluster($test_mode, 1);
RewindTest::start_master();
# Create a test table and insert a row in master.
diff --git a/src/bin/pg_rewind/t/002_databases.pl
b/src/bin/pg_rewind/t/002_databases.pl
index 37cdd712f3..0e0140b96f 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 6;
use RewindTest;
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl
b/src/bin/pg_rewind/t/003_extrafiles.pl
index 2433a4aab6..28933da343 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -3,7 +3,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 6;
use File::Find;
@@ -14,6 +14,8 @@ sub run_test
{
my $test_mode = shift;
+ umask(0077);
+
RewindTest::setup_cluster($test_mode);
RewindTest::start_master();
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index feadaa6a0f..70dd061915 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -14,7 +14,7 @@ if ($windows_os)
}
else
{
- plan tests => 4;
+ plan tests => 6;
}
use RewindTest;
diff --git a/src/bin/pg_rewind/t/005_same_timeline.pl
b/src/bin/pg_rewind/t/005_same_timeline.pl
index 0e334ee191..2fbca4ad7c 100644
--- a/src/bin/pg_rewind/t/005_same_timeline.pl
+++ b/src/bin/pg_rewind/t/005_same_timeline.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 1;
+use Test::More tests => 2;
use RewindTest;
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 8d4f254f9f..44d63f451f 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -442,7 +442,7 @@ create_script_for_cluster_analyze(char
**analyze_script_file_name)
*analyze_script_file_name = psprintf("%sanalyze_new_cluster.%s",
SCRIPT_PREFIX, SCRIPT_EXT);
- if ((script = fopen_priv(*analyze_script_file_name, "w")) == NULL)
+ if ((script = fopen(*analyze_script_file_name, "w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
*analyze_script_file_name, strerror(errno));
@@ -570,7 +570,7 @@ create_script_for_old_cluster_deletion(char
**deletion_script_file_name)
prep_status("Creating script to delete old cluster");
- if ((script = fopen_priv(*deletion_script_file_name, "w")) == NULL)
+ if ((script = fopen(*deletion_script_file_name, "w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
*deletion_script_file_name, strerror(errno));
@@ -834,7 +834,7 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo
*cluster)
for (rowno = 0; rowno < ntups; rowno++)
{
found = true;
- if (script == NULL && (script = fopen_priv(output_path,
"w")) == NULL)
+ if (script == NULL && (script = fopen(output_path,
"w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
output_path, strerror(errno));
if (!db_used)
@@ -937,7 +937,7 @@ check_for_reg_data_type_usage(ClusterInfo *cluster)
for (rowno = 0; rowno < ntups; rowno++)
{
found = true;
- if (script == NULL && (script = fopen_priv(output_path,
"w")) == NULL)
+ if (script == NULL && (script = fopen(output_path,
"w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
output_path, strerror(errno));
if (!db_used)
@@ -1028,7 +1028,7 @@ check_for_jsonb_9_4_usage(ClusterInfo *cluster)
for (rowno = 0; rowno < ntups; rowno++)
{
found = true;
- if (script == NULL && (script = fopen_priv(output_path,
"w")) == NULL)
+ if (script == NULL && (script = fopen(output_path,
"w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
output_path, strerror(errno));
if (!db_used)
diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index 8a662e9865..def22c6521 100644
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -18,7 +18,6 @@ void
generate_old_dump(void)
{
int dbnum;
- mode_t old_umask;
prep_status("Creating dump of global objects");
@@ -33,13 +32,6 @@ generate_old_dump(void)
prep_status("Creating dump of database schemas\n");
- /*
- * Set umask for this function, all functions it calls, and all
- * subprocesses/threads it creates. We can't use fopen_priv() as
Windows
- * uses threads and umask is process-global.
- */
- old_umask = umask(S_IRWXG | S_IRWXO);
-
/* create per-db dump files */
for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
{
@@ -74,8 +66,6 @@ generate_old_dump(void)
while (reap_child(true) == true)
;
- umask(old_umask);
-
end_progress_output();
check_ok();
}
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index f88e3d558f..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));
@@ -314,18 +315,3 @@ win32_pghardlink(const char *src, const char *dst)
return 0;
}
#endif
-
-
-/* fopen() file with no group/other permissions */
-FILE *
-fopen_priv(const char *path, const char *mode)
-{
- mode_t old_umask = umask(S_IRWXG | S_IRWXO);
- FILE *fp;
-
- fp = fopen(path, mode);
-
- umask(old_umask); /* we assume this can't change
errno */
-
- return fp;
-}
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index d61fa38c92..70d8cf2902 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -249,7 +249,7 @@ check_loadable_libraries(void)
{
found = true;
- if (script == NULL && (script = fopen_priv(output_path,
"w")) == NULL)
+ if (script == NULL && (script = fopen(output_path,
"w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
output_path, strerror(errno));
fprintf(script, _("could not load library \"%s\": %s"),
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 9dbc9225a6..ef84f44672 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -97,7 +97,7 @@ parseCommandLine(int argc, char *argv[])
if (os_user_effective_id == 0)
pg_fatal("%s: cannot be run as root\n", os_info.progname);
- if ((log_opts.internal = fopen_priv(INTERNAL_LOG_FILE, "a")) == NULL)
+ if ((log_opts.internal = fopen(INTERNAL_LOG_FILE, "a")) == NULL)
pg_fatal("could not write to log file \"%s\"\n",
INTERNAL_LOG_FILE);
while ((option = getopt_long(argc, argv, "d:D:b:B:cj:ko:O:p:P:rU:v",
@@ -213,7 +213,7 @@ parseCommandLine(int argc, char *argv[])
/* label start of upgrade in logfiles */
for (filename = output_files; *filename != NULL; filename++)
{
- if ((fp = fopen_priv(*filename, "a")) == NULL)
+ if ((fp = fopen(*filename, "a")) == NULL)
pg_fatal("could not write to log file \"%s\"\n",
*filename);
/* Start with newline because we might be appending to a file.
*/
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index a67e484a85..98e8977190 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"
@@ -95,6 +96,9 @@ main(int argc, char **argv)
check_cluster_compatibility(live_check);
+ /* Set mask based on PGDATA permissions */
+ umask(DataDirectoryMask(new_cluster.pgdata));
+
check_and_dump_old_cluster(live_check);
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 67f874b4f1..b8a1a18e36 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -374,7 +374,6 @@ void linkFile(const char *src, const char *dst,
void rewriteVisibilityMap(const char *fromfile, const char *tofile,
const char *schemaName, const char
*relName);
void check_hard_link(void);
-FILE *fopen_priv(const char *path, const char *mode);
/* function.c */
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 39983abea1..24631a91c6 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -20,9 +20,9 @@ unset MAKELEVEL
# Run a given "initdb" binary and overlay the regression testing
# authentication configuration.
standard_initdb() {
- # To increase coverage of non-standard segment size without
- # increase test runtime, run these tests with a lower setting.
- "$1" -N --wal-segsize 1
+ # To increase coverage of non-standard segment size and group access
+ # without increasing test runtime, run these tests with a custom
setting.
+ "$1" -N --wal-segsize 1 -g
if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
then
cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -230,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 640 | wc -l) -ne 0 ]; then
+ echo "files in PGDATA with permission != 640";
+ exit 1;
+fi
+
+if [ $(find ${PGDATA} -type d ! -perm 750 | wc -l) -ne 0 ]; then
+ echo "directories in PGDATA with permission != 750";
+ exit 1;
+fi
+
pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
case $testhost in
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 76e9d65537..205f772fc5 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -53,7 +53,7 @@ new_9_0_populate_pg_largeobject_metadata(ClusterInfo
*cluster, bool check_mode)
{
PQExpBufferData connectbuf;
- if (script == NULL && (script =
fopen_priv(output_path, "w")) == NULL)
+ if (script == NULL && (script =
fopen(output_path, "w")) == NULL)
pg_fatal("could not open file \"%s\":
%s\n", output_path,
strerror(errno));
@@ -152,7 +152,7 @@ old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster)
for (rowno = 0; rowno < ntups; rowno++)
{
found = true;
- if (script == NULL && (script = fopen_priv(output_path,
"w")) == NULL)
+ if (script == NULL && (script = fopen(output_path,
"w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
output_path,
strerror(errno));
if (!db_used)
@@ -253,7 +253,7 @@ old_9_6_check_for_unknown_data_type_usage(ClusterInfo
*cluster)
for (rowno = 0; rowno < ntups; rowno++)
{
found = true;
- if (script == NULL && (script = fopen_priv(output_path,
"w")) == NULL)
+ if (script == NULL && (script = fopen(output_path,
"w")) == NULL)
pg_fatal("could not open file \"%s\": %s\n",
output_path,
strerror(errno));
if (!db_used)
@@ -335,7 +335,7 @@ old_9_6_invalidate_hash_indexes(ClusterInfo *cluster, bool
check_mode)
found = true;
if (!check_mode)
{
- if (script == NULL && (script =
fopen_priv(output_path, "w")) == NULL)
+ if (script == NULL && (script =
fopen(output_path, "w")) == NULL)
pg_fatal("could not open file \"%s\":
%s\n", output_path,
strerror(errno));
if (!db_used)
diff --git a/src/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..2be76913cd
--- /dev/null
+++ b/src/common/file_perm.c
@@ -0,0 +1,50 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres_fe.h"
+
+#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.
+ */
+#ifdef FRONTEND
+
+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;
+}
+
+#endif /* FRONTEND */
diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h
new file mode 100644
index 0000000000..b94fdaf08a
--- /dev/null
+++ b/src/include/common/file_perm.h
@@ -0,0 +1,55 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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.
+ *
+ * This module is located in common so the backend can use the 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)
+
+/*
+ * 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_DIR_MODE_DEFAULT (S_IRWXU | S_IRGRP | S_IXGRP)
+
+#ifdef FRONTEND
+
+/*
+ * Determine the mask to use when writing to PGDATA
+ */
+mode_t DataDirectoryMask(const char *dataDir);
+
+#endif /* FRONTEND */
+
+
+#endif /* FILE_PERM_H */
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 909d7202e9..09c3533251 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -57,10 +57,6 @@ extern PGDLLIMPORT int max_files_per_process;
*/
extern int max_safe_fds;
-/*
- * Default mode for directories created with MakeDirectoryDefaultPerm().
- */
-#define PG_DIR_MODE_DEFAULT (S_IRWXU)
/*
* prototypes for functions in fd.c
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 1d5ac4ee35..6d02377294 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -268,6 +268,20 @@ sub connstr
=pod
+=item $node->group_access()
+
+Does the data dir allow group access?
+
+=cut
+
+sub group_access
+{
+ my ($self) = @_;
+ return $self->{_group_access};
+}
+
+=pod
+
=item $node->data_dir()
Returns the path to the data directory. postgresql.conf and pg_hba.conf are
@@ -405,10 +419,16 @@ sub init
$params{allows_streaming} = 0 unless defined $params{allows_streaming};
$params{has_archiving} = 0 unless defined $params{has_archiving};
+ $params{has_group_access} = 0 unless defined $params{has_group_access};
mkdir $self->backup_dir;
mkdir $self->archive_dir;
+ if ($params{has_group_access})
+ {
+ push(@{$params{extra}}, '-g');
+ }
+
TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
@{ $params{extra} });
TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
@@ -459,8 +479,12 @@ sub init
}
close $conf;
+ chmod($params{has_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};
+ $self->{_group_access} = 1 if $params{has_group_access};
}
=pod
@@ -483,6 +507,9 @@ sub append_conf
my $conffile = $self->data_dir . '/' . $filename;
TestLib::append_to_file($conffile, $str . "\n");
+
+ chmod($self->group_access() ? 0640 : 0600, $conffile)
+ or die("unable to set permissions for $conffile");
}
=pod
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index fdd427608b..c4da3140c1 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -12,8 +12,10 @@ use warnings;
use Config;
use Exporter 'import';
+use Fcntl qw(:mode);
use File::Basename;
use File::Spec;
+use File::stat qw(stat);
use File::Temp ();
use IPC::Run;
use SimpleTee;
@@ -26,6 +28,7 @@ our @EXPORT = qw(
slurp_dir
slurp_file
append_to_file
+ check_pg_data_perm
check_pg_config
system_or_bail
system_log
@@ -222,6 +225,96 @@ sub append_to_file
close $fh;
}
+# Ensure all permissions in the pg_data directory are correct. When
allow_group
+# is true then permissions should be dir = 0750, file = 0640. When allow_group
+# is false then permissions should be dir = 0700, file = 0600.
+sub check_pg_data_perm
+{
+ my ($dir, $allow_group, $level) = @_;
+
+ # Expected permission
+ my $expected_file_perm = $allow_group ? 0640 : 0600;
+ my $expected_dir_perm = $allow_group ? 0750 : 0700;
+
+ # First level is 0
+ $level = defined($level) ? $level : 0;
+
+ # Get all entries in the dir
+ opendir(my $dir_handle, $dir)
+ or die("unable to open $dir");
+
+ my @dir_entry = grep(!/^(\.\.)$/i, readdir($dir_handle));
+ close($dir_handle);
+
+ @dir_entry != 0
+ or die("unable to read $dir");
+
+ # Check each entry
+ foreach my $entry (@dir_entry)
+ {
+ my $entry_path = $entry eq '.' ? $dir : "$dir/$entry";
+ my $entry_stat = stat($entry_path);
+
+ defined($entry_stat)
+ or die("unable to stat $entry_path");
+
+ my $entry_mode = S_IMODE($entry_stat->mode);
+
+ # Is this a file?
+ if (S_ISREG($entry_stat->mode))
+ {
+ # postmaster.pid file must be 600
+ if ($level == 0 && $entry eq 'postmaster.pid')
+ {
+ if ($entry_mode != 0600)
+ {
+ print(*STDERR, "$entry_path mode must be 0600\n");
+ return 0;
+ }
+ }
+ else
+ {
+ if ($entry_mode != $expected_file_perm)
+ {
+ print(*STDERR,
+ sprintf("$entry_path mode must be %04o\n",
+ $expected_file_perm));
+ return 0;
+ }
+ }
+ }
+ # Else a directory?
+ elsif (S_ISDIR($entry_stat->mode))
+ {
+ # Only need to check the dir once
+ if ($entry eq '.')
+ {
+ if ($entry_mode != $expected_dir_perm)
+ {
+ print(*STDERR,
+ sprintf("$entry_path mode must be %04o\n",
+ $expected_dir_perm));
+ return 0;
+ }
+ }
+ else
+ {
+ if (!check_pg_data_perm($entry_path, $allow_group, $level + 1))
+ {
+ return 0;
+ }
+ }
+ }
+ # Else something we can't handle
+ else
+ {
+ die "unknown file type for $entry_path";
+ }
+ }
+
+ return 1;
+}
+
# 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.
