On 2/28/18 2:28 AM, Michael Paquier wrote: > On Tue, Feb 27, 2018 at 03:52:32PM -0500, David Steele wrote: >> On 1/30/18 3:01 AM, Michael Paquier wrote: > >>> +command_ok( >>> + ['chmod', "-R", 'g+rX', "$pgdata"], >>> + 'add group perms to PGDATA'); >>> >>> That would blow up on Windows. You should replace that by perl's chmod >>> and look at its return result for sanity checks, likely with ($cnt != 0). >>> And let's not use icacls please... >> >> Fixed with a new function, add_pg_data_group_perm(). > > That's basically a recursive chmod, so chmod_recursive is more adapted? > I could imagine that this is useful as well for removing group > permissions, so the new mode could be specified as an argument.
The required package (File::chmod::Recursive) for chmod_recursive is not
in use anywhere else and was not installed when I installed build
dependencies.
I'm not sure what the protocol for introducing a new Perl module is? I
couldn't find packages for the major OSes. Are we OK with using CPAN?
>>> + if ($params{has_group_access})
>>> + {
>>> + push(@{$params{extra}}, '-g');
>>> + }
>>> No need to introduce a new parameter here, please use directly extra =>
>>> [ '-g' ] in the routines calling PostgresNode::init.
>>
>> Other code needs to know if group access is enabled on the node (see
>> RewindTest.pm) so it's not just a matter of passing the option.
>
> I don't quite understand here. I have no objection into extending
> setup_cluster() with a group_access argument so as the tests run both
> the group access and without it, but I'd rather keep the low-level API
> clean of anything fancy if we can use the facility which already
> exists.
I'm not sure what you mean by, "facility that already exists". I think
I implemented this the same as the allows_streaming and has_archiving
flags. The only difference is that this option must be set at initdb
time rather than in postgresql.conf.
Could you be more specific?
>> New patches are attached. The end result is almost identical to v6 but
>> code was moved from 03 to 02 as per Michael's suggestion.
>
> Thanks for the updated versions.
>
>> 1) 01-pgresetwal-test
>>
>> Adds a very basic test suite for pg_resetwal.
>
> Okay. This one looks fine to me. It could be passed down to a
> committer.
Agreed.
>> 2) 02-file-perm
>>
>> 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.
>
> I had a more simple (complicated?) thing in mind for the split, which
> would actually cause this patch to be split into two more:
> 1) Introduce file_perm.h and replace all the existing values for modes
> and masks to what the header introduces. This allows to check if the
> new header is not forgotten anywhere, especially for the frontends.
> 2) Introduce MakeDirectoryDefaultPerm, which switches the backend calls
> of mkdir() to the new API.
> 3) Add the grouping facilities, which updates the default masks and
> modes of file_perm.h.
>
> Grouping 1) and 2) together as you do makes sense as well I guess, as in
> my proposal the mkdir calls in the backend would be touched twice, still
> things get more incremental.
I think I prefer grouping 1 and 2. It produces less churn, as you say,
and I don't think MakeDirectoryDefaultPerm really needs its own patch.
Based on comments so far, nobody has an objection to it.
In theory, the first two patches could be applied just for refactoring
without adding group permissions at all. There are a lot of new tests
to make sure permissions are set correctly so it seems like a win right off.
>
> +/*
> + * Default mode for created files.
> + */
> +#define PG_FILE_MODE_DEFAULT (S_IRUSR | S_IWUSR | S_IRGRP)
> +
> +/*
> + * Default mode for directories.
> + */
> +#define PG_DIR_MODE_DEFAULT (S_IRWXU | S_IRGRP | S_IXGRP)
> For the first cut, this should not use S_IRGRP in the first declaration,
> and (S_IXGRP | S_IXGRP) in the second declaration.
Done.
>
> - if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
> + if (script == NULL && (script = fopen(output_path, "w")) == NULL)
> Why are those calls in pg_upgrade changed?
Done.
Those should have been removed already. Originally I had removed
fopen_priv() because it didn't serve a purpose anymore. In the
meantime, somebody else made it a macro to fopen(). I decided my patch
would be less noisy if I reverted to fopen_priv() but I missed a few places.
> TestLib.pm does not need the group-related extensions, right?
Done.
> check_pg_data_perm() looks useful even without $allow_group even if the
> grouping facility is reverted at the end. S_ISLNK should be considered
> as well for tablespaces or symlinks of pg_wal? We have such cases in
> the regression tests.
Hmmm, the link is just pointing to a directory whose permissions have
been changed. Why do we need to worry about the link?
> - 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();
> - }
> Hm. Why are those removed?
When I started working on this, pg_upgrade did not set umask and instead
did chmod for each dir created. umask() has since been added (even
before my patch) and so these are now noops. Seemed easier to remove
them than to change them all.
> setup_signals();
>
> - umask(S_IRWXG | S_IRWXO);
> -
> create_data_directory();
> This should not be moved around.
Hmmm - I moved it much earlier in the process which seems like a good
thing. Consider if there was a option to fixup permissions, like there
is to fsync. Isn't it best to set the mode as soon as possible to
prevent code from being inserted before it?
> What you need to do for frontend-only files in src/common/ is to define
> that at the top and not bother about FRONTEND cflags at all:
> #ifndef FRONTEND
> #error "This file is not expected to be compiled for backend code"
> #endif
Fixed.
> src/tools/msvc/Mkvcbuild.pm also needs to be updated with this new file,
> or you will break MSVC build.
Fixed.
New patches attached.
--
-David
da...@pgmasters.net
From 2caecfef0cb0b029c439786fe462d69b5caaf67b 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 9c1bdd25ea92fca5ded4a2889283cf8f47a28a0f Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Mon, 5 Mar 2018 14:36:08 -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 | 42 ++++------------
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 | 5 ++
src/bin/pg_rewind/file_ops.c | 7 +--
src/bin/pg_rewind/pg_rewind.c | 4 ++
src/bin/pg_rewind/t/001_basic.pl | 2 +-
src/bin/pg_rewind/t/002_databases.pl | 2 +-
src/bin/pg_rewind/t/003_extrafiles.pl | 4 +-
src/bin/pg_rewind/t/004_pg_xlog_symlink.pl | 2 +-
src/bin/pg_rewind/t/005_same_timeline.pl | 2 +-
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 | 80 ++++++++++++++++++++++++++++++
29 files changed, 234 insertions(+), 79 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 f3ddf828bb..e13a2ae8c4 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4495,7 +4495,7 @@ internal_forkexec(int argc, char *argv[], Port *port)
* As in OpenTemporaryFileInTablespace, try to make the
temp-file
* directory
*/
- mkdir(PG_TEMP_FILES_DIR, S_IRWXU);
+ MakeDirectoryDefaultPerm(PG_TEMP_FILES_DIR);
fp = AllocateFile(tmpfilename, PG_BINARY_W);
if (!fp)
diff --git a/src/backend/postmaster/syslogger.c
b/src/backend/postmaster/syslogger.c
index f70eea37df..ae23941f59 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -41,6 +41,7 @@
#include "postmaster/postmaster.h"
#include "postmaster/syslogger.h"
#include "storage/dsm.h"
+#include "storage/fd.h"
#include "storage/ipc.h"
#include "storage/latch.h"
#include "storage/pg_shmem.h"
@@ -322,7 +323,7 @@ SysLoggerMain(int argc, char *argv[])
/*
* Also, create new directory if not present;
ignore errors
*/
- mkdir(Log_directory, S_IRWXU);
+ MakeDirectoryDefaultPerm(Log_directory);
}
if (strcmp(Log_filename, currentLogFilename) != 0)
{
@@ -564,7 +565,7 @@ SysLogger_Start(void)
/*
* Create log directory if not present; ignore errors
*/
- mkdir(Log_directory, S_IRWXU);
+ MakeDirectoryDefaultPerm(Log_directory);
/*
* The initial logfile is created right in the postmaster, to verify
that
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index fc9ef22b0b..3a98360b50 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1166,13 +1166,14 @@ CreateSlotOnDisk(ReplicationSlot *slot)
* It's just barely possible that some previous effort to create or
drop a
* slot with this name left a temp directory lying around. If that seems
* to be the case, try to remove it. If the rmtree() fails, we'll error
- * out at the mkdir() below, so we don't bother checking success.
+ * out at the MakeDirectoryDefaultPerm() below, so we don't bother
checking
+ * success.
*/
if (stat(tmppath, &st) == 0 && S_ISDIR(st.st_mode))
rmtree(tmppath, true);
/* Create and fsync the temporary slot directory. */
- if (mkdir(tmppath, S_IRWXU) < 0)
+ if (MakeDirectoryDefaultPerm(tmppath) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\": %m",
diff --git a/src/backend/storage/file/copydir.c
b/src/backend/storage/file/copydir.c
index ca6342db0d..ff980dc1f5 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -41,7 +41,7 @@ copydir(char *fromdir, char *todir, bool recurse)
char fromfile[MAXPGPATH * 2];
char tofile[MAXPGPATH * 2];
- if (mkdir(todir, S_IRWXU) != 0)
+ if (MakeDirectoryDefaultPerm(todir) != 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create directory \"%s\":
%m", todir)));
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 2a18e94ff4..1491665375 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -84,6 +84,7 @@
#include "access/xlog.h"
#include "catalog/catalog.h"
#include "catalog/pg_tablespace.h"
+#include "common/file_perm.h"
#include "pgstat.h"
#include "portability/mem.h"
#include "storage/fd.h"
@@ -124,12 +125,6 @@
*/
#define FD_MINFREE 10
-/*
- * Default mode for created files, unless something else is specified using
- * the *Perm() function variants.
- */
-#define PG_FILE_MODE_DEFAULT (S_IRUSR | S_IWUSR)
-
/*
* A number of platforms allow individual processes to open many more files
* than they can really support when *many* processes do the same thing.
@@ -1435,7 +1430,7 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags,
mode_t fileMode)
void
PathNameCreateTemporaryDir(const char *basedir, const char *directory)
{
- if (mkdir(directory, S_IRWXU) < 0)
+ if (MakeDirectoryDefaultPerm(directory) < 0)
{
if (errno == EEXIST)
return;
@@ -1445,14 +1440,14 @@ PathNameCreateTemporaryDir(const char *basedir, const
char *directory)
* EEXIST to close a race against another process following the
same
* algorithm.
*/
- if (mkdir(basedir, S_IRWXU) < 0 && errno != EEXIST)
+ if (MakeDirectoryDefaultPerm(basedir) < 0 && errno != EEXIST)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("cannot create temporary
directory \"%s\": %m",
basedir)));
/* Try again. */
- if (mkdir(directory, S_IRWXU) < 0 && errno != EEXIST)
+ if (MakeDirectoryDefaultPerm(directory) < 0 && errno != EEXIST)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("cannot create temporary
subdirectory \"%s\": %m",
@@ -1602,11 +1597,11 @@ OpenTemporaryFileInTablespace(Oid tblspcOid, bool
rejectError)
* We might need to create the tablespace's tempfile directory,
if no
* one has yet done so.
*
- * Don't check for error from mkdir; it could fail if someone
else
- * just did the same thing. If it doesn't work then we'll bomb
out on
+ * Don't check error from MakeDirectoryDefaultPerm; it could
fail if someone
+ * else just did the same thing. If it doesn't work then we'll
bomb out on
* the second create attempt, instead.
*/
- mkdir(tempdirpath, S_IRWXU);
+ MakeDirectoryDefaultPerm(tempdirpath);
file = PathNameOpenFile(tempfilepath,
O_RDWR |
O_CREAT | O_TRUNC | PG_BINARY);
@@ -3555,3 +3550,16 @@ fsync_parent_path(const char *fname, int elevel)
return 0;
}
+
+/*
+ * Create a directory using PG_DIR_MODE_DEFAULT for permissions
+ *
+ * Directories in PGDATA should normally have specific permissions -- when
+ * using this function the caller does not need to know what they should be.
+ * For permissions other than the default use mkdir() directly.
+ */
+int
+MakeDirectoryDefaultPerm(const char *directoryName)
+{
+ return mkdir(directoryName, PG_DIR_MODE_DEFAULT);
+}
diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index 726db7b7f1..466613e4af 100644
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -137,6 +137,10 @@ proc_exit(int code)
else
snprintf(gprofDirName, 32, "gprof/%d", (int) getpid());
+ /*
+ * Use mkdir() instead of MakeDirectoryDefaultPerm() here
because non-default
+ * permissions are required.
+ */
mkdir("gprof", S_IRWXU | S_IRWXG | S_IRWXO);
mkdir(gprofDirName, S_IRWXU | S_IRWXG | S_IRWXO);
chdir(gprofDirName);
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 2efd3b75b1..4ef06c2349 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -64,6 +64,7 @@
#include "catalog/pg_authid.h"
#include "catalog/pg_class.h"
#include "catalog/pg_collation.h"
+#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/restricted_token.h"
#include "common/username.h"
@@ -144,6 +145,7 @@ static bool data_checksums = false;
static char *xlog_dir = NULL;
static char *str_wal_segment_size_mb = NULL;
static int wal_segment_size_mb;
+static mode_t file_mode_mask = PG_MODE_MASK_DEFAULT;
/* internal vars */
@@ -1170,12 +1172,6 @@ setup_config(void)
snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data);
writefile(path, conflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
/*
* create the automatic configuration file to store the configuration
@@ -1190,12 +1186,6 @@ setup_config(void)
sprintf(path, "%s/postgresql.auto.conf", pg_data);
writefile(path, autoconflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
free(conflines);
@@ -1277,12 +1267,6 @@ setup_config(void)
snprintf(path, sizeof(path), "%s/pg_hba.conf", pg_data);
writefile(path, conflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
free(conflines);
@@ -1293,12 +1277,6 @@ setup_config(void)
snprintf(path, sizeof(path), "%s/pg_ident.conf", pg_data);
writefile(path, conflines);
- if (chmod(path, S_IRUSR | S_IWUSR) != 0)
- {
- fprintf(stderr, _("%s: could not change permissions of \"%s\":
%s\n"),
- progname, path, strerror(errno));
- exit_nicely();
- }
free(conflines);
@@ -2692,7 +2670,7 @@ create_data_directory(void)
pg_data);
fflush(stdout);
- if (pg_mkdir_p(pg_data, S_IRWXU) != 0)
+ if (pg_mkdir_p(pg_data, PG_DIR_MODE_DEFAULT) != 0)
{
fprintf(stderr, _("%s: could not create
directory \"%s\": %s\n"),
progname, pg_data,
strerror(errno));
@@ -2710,7 +2688,7 @@ create_data_directory(void)
pg_data);
fflush(stdout);
- if (chmod(pg_data, S_IRWXU) != 0)
+ if (chmod(pg_data, PG_DIR_MODE_DEFAULT &
~file_mode_mask) != 0)
{
fprintf(stderr, _("%s: could not change
permissions of directory \"%s\": %s\n"),
progname, pg_data,
strerror(errno));
@@ -2778,7 +2756,7 @@ create_xlog_or_symlink(void)
xlog_dir);
fflush(stdout);
- if (pg_mkdir_p(xlog_dir, S_IRWXU) != 0)
+ if (pg_mkdir_p(xlog_dir, PG_DIR_MODE_DEFAULT)
!= 0)
{
fprintf(stderr, _("%s: could not create
directory \"%s\": %s\n"),
progname, xlog_dir,
strerror(errno));
@@ -2796,7 +2774,7 @@ create_xlog_or_symlink(void)
xlog_dir);
fflush(stdout);
- if (chmod(xlog_dir, S_IRWXU) != 0)
+ if (chmod(xlog_dir, PG_DIR_MODE_DEFAULT &
~file_mode_mask) != 0)
{
fprintf(stderr, _("%s: could not change
permissions of directory \"%s\": %s\n"),
progname, xlog_dir,
strerror(errno));
@@ -2846,7 +2824,7 @@ create_xlog_or_symlink(void)
else
{
/* Without -X option, just make the subdirectory normally */
- if (mkdir(subdirloc, S_IRWXU) < 0)
+ if (mkdir(subdirloc, PG_DIR_MODE_DEFAULT) < 0)
{
fprintf(stderr, _("%s: could not create directory
\"%s\": %s\n"),
progname, subdirloc, strerror(errno));
@@ -2882,8 +2860,6 @@ initialize_data_directory(void)
setup_signals();
- umask(S_IRWXG | S_IRWXO);
-
create_data_directory();
create_xlog_or_symlink();
@@ -2902,7 +2878,7 @@ initialize_data_directory(void)
* The parent directory already exists, so we only need mkdir()
not
* pg_mkdir_p() here, which avoids some failure modes; cf bug
#13853.
*/
- if (mkdir(path, S_IRWXU) < 0)
+ if (mkdir(path, PG_DIR_MODE_DEFAULT) < 0)
{
fprintf(stderr, _("%s: could not create directory
\"%s\": %s\n"),
progname, path, strerror(errno));
@@ -3159,6 +3135,8 @@ main(int argc, char *argv[])
}
}
+ /* Set the file mode creation mask */
+ umask(file_mode_mask);
/*
* Non-option argument specifies data directory as long as it wasn't
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index c0cfa6e92c..561608f1d8 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 15;
+use Test::More tests => 16;
my $tempdir = TestLib::tempdir;
my $xlogdir = "$tempdir/pgxlog";
@@ -45,6 +45,8 @@ mkdir $datadir;
command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ],
'successful creation');
+
+ ok(check_pg_data_perm($datadir, 0));
}
command_ok([ 'initdb', '-S', $datadir ], 'sync only');
command_fails([ 'initdb', $datadir ], 'existing data directory');
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 9bc830b085..1eef7179cc 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -25,6 +25,7 @@
#include "catalog/pg_control.h"
#include "common/controldata_utils.h"
+#include "common/file_perm.h"
#include "getopt_long.h"
#include "utils/pidfile.h"
@@ -2170,7 +2171,7 @@ main(int argc, char **argv)
*/
argv0 = argv[0];
- umask(S_IRWXG | S_IRWXO);
+ umask(PG_MODE_MASK_DEFAULT);
/* support --help and --version even if invoked as root */
if (argc > 1)
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl
b/src/bin/pg_ctl/t/001_start_stop.pl
index 5da4746cb4..909bf4d465 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 19;
+use Test::More tests => 20;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -57,10 +57,15 @@ command_ok([ 'pg_ctl', 'stop', '-D', "$tempdir/data" ],
'pg_ctl stop');
command_fails([ 'pg_ctl', 'stop', '-D', "$tempdir/data" ],
'second pg_ctl stop fails');
+# Log file for first perm test
+my $logFileName = "$tempdir/data/perm-test-600.log";
+
command_ok(
- [ 'pg_ctl', 'restart', '-D', "$tempdir/data" ],
+ [ 'pg_ctl', 'restart', '-D', "$tempdir/data", '-l', $logFileName ],
'pg_ctl restart with server not running');
-command_ok([ 'pg_ctl', 'restart', '-D', "$tempdir/data" ],
- 'pg_ctl restart with server running');
+
+# Log file should exist and have no group permissions
+ok(-f $logFileName);
+ok(check_pg_data_perm("$tempdir/data", 0));
system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data";
diff --git a/src/bin/pg_resetwal/pg_resetwal.c
b/src/bin/pg_resetwal/pg_resetwal.c
index a132cf2e9f..47abd41fc3 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -52,6 +52,7 @@
#include "catalog/catversion.h"
#include "catalog/pg_control.h"
#include "common/fe_memutils.h"
+#include "common/file_perm.h"
#include "common/restricted_token.h"
#include "storage/large_object.h"
#include "pg_getopt.h"
@@ -327,6 +328,9 @@ main(int argc, char *argv[])
exit(1);
}
+ /* Set dir/file mode mask */
+ umask(PG_MODE_MASK_DEFAULT);
+
/* Check that data directory matches our server version */
CheckDataVersion();
@@ -919,7 +923,7 @@ RewriteControlFile(void)
fd = open(XLOG_CONTROL_FILE,
O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
- S_IRUSR | S_IWUSR);
+ PG_FILE_MODE_DEFAULT);
if (fd < 0)
{
fprintf(stderr, _("%s: could not create pg_control file: %s\n"),
@@ -1201,7 +1205,7 @@ WriteEmptyXLOG(void)
unlink(path);
fd = open(path, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
- S_IRUSR | S_IWUSR);
+ PG_FILE_MODE_DEFAULT);
if (fd < 0)
{
fprintf(stderr, _("%s: could not open file \"%s\": %s\n"),
diff --git a/src/bin/pg_resetwal/t/001_basic.pl
b/src/bin/pg_resetwal/t/001_basic.pl
index 234bd70303..db0c2a630a 100644
--- a/src/bin/pg_resetwal/t/001_basic.pl
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 13;
+use Test::More tests => 14;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -36,6 +36,8 @@ is_deeply(
[sort(qw(. .. archive_status 000000010000000000000002))],
'WAL recreated');
+ok(check_pg_data_perm($pgdata, 0), 'check PGDATA permissions');
+
$node->start;
# Reset to specific WAL segment
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 00b5b42dd7..b18a7f954c 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -237,6 +237,9 @@ sub run_pg_rewind
"$tmp_folder/master-postgresql.conf.tmp",
"$master_pgdata/postgresql.conf");
+ chmod(0600, "$master_pgdata/postgresql.conf")
+ or die("unable to set permissions for $master_pgdata/postgresql.conf");
+
# Plug-in rewound node to the now-promoted standby node
my $port_standby = $node_standby->port;
$node_master->append_conf(
@@ -255,6 +258,8 @@ recovery_target_timeline='latest'
# Clean up after the test. Stop both servers, if they're still running.
sub clean_rewind_test
{
+ ok (check_pg_data_perm($node_master->data_dir(), 0));
+
$node_master->teardown_node if defined $node_master;
$node_standby->teardown_node if defined $node_standby;
}
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index 705383d184..d51914e901 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -18,6 +18,7 @@
#include <fcntl.h>
#include <unistd.h>
+#include "common/file_perm.h"
#include "file_ops.h"
#include "filemap.h"
#include "logging.h"
@@ -58,7 +59,7 @@ open_target_file(const char *path, bool trunc)
mode = O_WRONLY | O_CREAT | PG_BINARY;
if (trunc)
mode |= O_TRUNC;
- dstfd = open(dstpath, mode, 0600);
+ dstfd = open(dstpath, mode, PG_FILE_MODE_DEFAULT);
if (dstfd < 0)
pg_fatal("could not open target file \"%s\": %s\n",
dstpath, strerror(errno));
@@ -190,7 +191,7 @@ truncate_target_file(const char *path, off_t newsize)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
- fd = open(dstpath, O_WRONLY, 0);
+ fd = open(dstpath, O_WRONLY, PG_FILE_MODE_DEFAULT);
if (fd < 0)
pg_fatal("could not open file \"%s\" for truncation: %s\n",
dstpath, strerror(errno));
@@ -211,7 +212,7 @@ create_target_dir(const char *path)
return;
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
- if (mkdir(dstpath, S_IRWXU) != 0)
+ if (mkdir(dstpath, PG_DIR_MODE_DEFAULT) != 0)
pg_fatal("could not create directory \"%s\": %s\n",
dstpath, strerror(errno));
}
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 72ab2f8d5e..3d179e2c7d 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -24,6 +24,7 @@
#include "access/xlog_internal.h"
#include "catalog/catversion.h"
#include "catalog/pg_control.h"
+#include "common/file_perm.h"
#include "common/restricted_token.h"
#include "getopt_long.h"
#include "storage/bufpage.h"
@@ -185,6 +186,9 @@ main(int argc, char **argv)
exit(1);
}
+ /* Set dir/file mode mask */
+ umask(PG_MODE_MASK_DEFAULT);
+
/*
* Don't allow pg_rewind to be run as root, to avoid overwriting the
* ownership of files in the data directory. We need only check for root
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 736f34eae3..e7fc200fc0 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 8;
+use Test::More tests => 10;
use RewindTest;
diff --git a/src/bin/pg_rewind/t/002_databases.pl
b/src/bin/pg_rewind/t/002_databases.pl
index 37cdd712f3..0e0140b96f 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 6;
use RewindTest;
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl
b/src/bin/pg_rewind/t/003_extrafiles.pl
index 2433a4aab6..28933da343 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -3,7 +3,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 6;
use File::Find;
@@ -14,6 +14,8 @@ sub run_test
{
my $test_mode = shift;
+ umask(0077);
+
RewindTest::setup_cluster($test_mode);
RewindTest::start_master();
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index feadaa6a0f..70dd061915 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -14,7 +14,7 @@ if ($windows_os)
}
else
{
- plan tests => 4;
+ plan tests => 6;
}
use RewindTest;
diff --git a/src/bin/pg_rewind/t/005_same_timeline.pl
b/src/bin/pg_rewind/t/005_same_timeline.pl
index 0e334ee191..2fbca4ad7c 100644
--- a/src/bin/pg_rewind/t/005_same_timeline.pl
+++ b/src/bin/pg_rewind/t/005_same_timeline.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 1;
+use Test::More tests => 2;
use RewindTest;
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index f38bfacf02..595d43ee31 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -10,6 +10,7 @@
#include "postgres_fe.h"
#include "access/visibilitymap.h"
+#include "common/file_perm.h"
#include "pg_upgrade.h"
#include "storage/bufpage.h"
#include "storage/checksum.h"
@@ -44,7 +45,7 @@ copyFile(const char *src, const char *dst,
schemaName, relName, src, strerror(errno));
if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
- S_IRUSR | S_IWUSR)) < 0)
+ PG_FILE_MODE_DEFAULT)) < 0)
pg_fatal("error while copying relation \"%s.%s\": could not
create file \"%s\": %s\n",
schemaName, relName, dst, strerror(errno));
@@ -151,7 +152,7 @@ rewriteVisibilityMap(const char *fromfile, const char
*tofile,
schemaName, relName, fromfile,
strerror(errno));
if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
- S_IRUSR | S_IWUSR)) < 0)
+ PG_FILE_MODE_DEFAULT)) < 0)
pg_fatal("error while copying relation \"%s.%s\": could not
create file \"%s\": %s\n",
schemaName, relName, tofile, strerror(errno));
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index d12412799f..59df6fd88f 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -38,6 +38,7 @@
#include "pg_upgrade.h"
#include "catalog/pg_class.h"
+#include "common/file_perm.h"
#include "common/restricted_token.h"
#include "fe_utils/string_utils.h"
@@ -79,7 +80,7 @@ main(int argc, char **argv)
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_upgrade"));
/* Ensure that all files created by pg_upgrade are non-world-readable */
- umask(S_IRWXG | S_IRWXO);
+ umask(PG_MODE_MASK_DEFAULT);
parseCommandLine(argc, argv);
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 39983abea1..4e8db4aaf4 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -230,6 +230,17 @@ standard_initdb 'initdb'
pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir"
-B "$bindir" -p "$PGPORT" -P "$PGPORT"
+# make sure all directories and files have group permissions
+if [ $(find ${PGDATA} -type f ! -perm 600 | wc -l) -ne 0 ]; then
+ echo "files in PGDATA with permission != 600";
+ exit 1;
+fi
+
+if [ $(find ${PGDATA} -type d ! -perm 700 | wc -l) -ne 0 ]; then
+ echo "directories in PGDATA with permission != 700";
+ exit 1;
+fi
+
pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
case $testhost in
diff --git a/src/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 fdd427608b..21f4a9c55e 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -12,8 +12,11 @@ use warnings;
use Config;
use Exporter 'import';
+use Fcntl qw(:mode);
use File::Basename;
+use File::Find;
use File::Spec;
+use File::stat qw(stat);
use File::Temp ();
use IPC::Run;
use SimpleTee;
@@ -26,6 +29,7 @@ our @EXPORT = qw(
slurp_dir
slurp_file
append_to_file
+ check_pg_data_perm
check_pg_config
system_or_bail
system_log
@@ -222,6 +226,82 @@ sub append_to_file
close $fh;
}
+# Ensure all permissions in the pg_data directory are correct. Permissions
+# should be dir = 0700, file = 0600.
+sub check_pg_data_perm
+{
+ my ($pgdata) = @_;
+
+ # Result defaults to true
+ my $result = 1;
+
+ # Expected permission
+ my $expected_file_perm = 0600;
+ my $expected_dir_perm = 0700;
+
+ find
+ (
+ sub
+ {
+ my $file_stat = stat($File::Find::name);
+
+ defined($file_stat)
+ or die("unable to stat $File::Find::name");
+
+ my $file_mode = S_IMODE($file_stat->mode);
+
+ # Is this a file?
+ if (S_ISREG($file_stat->mode))
+ {
+ # postmaster.pid file must be 600
+ if ($File::Find::name =~ /\/postmaster\.pid$/)
+ {
+ if ($file_mode != 0600)
+ {
+ print(*STDERR,
"$File::Find::name mode must be 0600\n");
+
+ $result = 0;
+ return
+ }
+ }
+ else
+ {
+ if ($file_mode != $expected_file_perm)
+ {
+ print(*STDERR,
+
sprintf("$File::Find::name mode must be %04o\n",
+ $expected_file_perm));
+
+ $result = 0;
+ return
+ }
+ }
+ }
+ # Else a directory?
+ elsif (S_ISDIR($file_stat->mode))
+ {
+ if ($file_mode != $expected_dir_perm)
+ {
+ print(*STDERR,
+ sprintf("$File::Find::name mode
must be %04o\n",
+ $expected_dir_perm));
+
+ $result = 0;
+ return
+ }
+ }
+ # Else something we can't handle
+ else
+ {
+ die "unknown file type for $File::Find::name";
+ }
+ },
+ $pgdata
+ );
+
+ return $result;
+}
+
# Check presence of a given regexp within pg_config.h for the installation
# where tests are running, returning a match status result depending on
# that.
--
2.14.3 (Apple Git-98)
From d92d3e7d3778506b58b2bc99bdb81423439d651f Mon Sep 17 00:00:00 2001
From: David Steele <da...@pgmasters.net>
Date: Mon, 5 Mar 2018 14:38:18 -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 | 7 +++++-
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/001_basic.pl | 2 +-
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 | 26 +++++++++++++++++++-
src/test/perl/TestLib.pm | 35 +++++++++++++++++++++++----
src/tools/msvc/Mkvcbuild.pm | 2 +-
21 files changed, 260 insertions(+), 47 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 71f02300c2..8d8a48aad4 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>
@@ -2220,6 +2223,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 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/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4ef06c2349..56356d75c5 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2289,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"
@@ -2992,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}
};
@@ -3033,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)
{
@@ -3127,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"),
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 561608f1d8..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 => 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_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 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 909bf4d465..f8facc3f00 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_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";
+
+add_pg_data_group_perm("$tempdir/data");
+
+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 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 db0c2a630a..4a53358f58 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_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;
+add_pg_data_group_perm($pgdata);
+
$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_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 b18a7f954c..10d44fd259 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -116,10 +116,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(
@@ -237,7 +239,7 @@ 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
@@ -258,7 +260,8 @@ recovery_target_timeline='latest'
# Clean up after the test. Stop both servers, if they're still running.
sub clean_rewind_test
{
- ok (check_pg_data_perm($node_master->data_dir(), 0));
+ 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/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/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index e7fc200fc0..205bdd77ef 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -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_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..8e9c6c11f0 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -269,6 +269,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
@@ -406,10 +420,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);
@@ -460,8 +480,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
@@ -485,7 +509,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 21f4a9c55e..3d404eefac 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_pg_data_perm
+ add_pg_data_group_perm
check_pg_config
system_or_bail
system_log
@@ -226,18 +227,19 @@ sub append_to_file
close $fh;
}
-# Ensure all permissions in the pg_data directory are correct. Permissions
-# should be dir = 0700, file = 0600.
+# Ensure all permissions in the pg_data directory are correct. When
allow_group
+# is true then permissions should be dir = 0750, file = 0640. When allow_group
+# is false then permissions should be dir = 0700, file = 0600.
sub check_pg_data_perm
{
- my ($pgdata) = @_;
+ my ($pgdata, $allow_group) = @_;
# Result defaults to true
my $result = 1;
# Expected permission
- my $expected_file_perm = 0600;
- my $expected_dir_perm = 0700;
+ my $expected_file_perm = $allow_group ? 0640 : 0600;
+ my $expected_dir_perm = $allow_group ? 0750 : 0700;
find
(
@@ -302,6 +304,29 @@ sub check_pg_data_perm
return $result;
}
+# Add group permissions to a PGDATA directory
+sub add_pg_data_group_perm
+{
+ my ($pgdata) = @_;
+
+ find
+ (
+ sub
+ {
+ my $file_stat = stat($File::Find::name);
+
+ defined($file_stat)
+ or die("unable to stat $file_stat");
+
+ chmod(S_IMODE($file_stat->mode) |
+ (S_ISDIR($file_stat->mode) ? 0050 : 0040),
+ $File::Find::name)
+ or die "unable to chmod $File::Find::name";
+ },
+ $pgdata
+ );
+}
+
# 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)
signature.asc
Description: OpenPGP digital signature
