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 postmaster.opts, which record information about the running postmaster, not about the postmaster which will eventually use this backup. - (These files can confuse pg_ctl.) + (These files can confuse pg_ctl.) If group read + access is enabled on the data directory and an unprivileged user in the + PostgreSQL group is performing the backup, then + postmaster.pid will not be readable and must be + excluded. 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.) + + For security reasons the new cluster created by initdb + will only be accessible by the cluster owner by default. The + 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. + + initdb initializes the database cluster's default locale and character set encoding. The character set encoding, @@ -301,6 +309,17 @@ PostgreSQL documentation + + + + + + Allows users in the same group as the cluster owner to read all cluster + files created by initdb. + + + + diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index a2ebd3e21c..8f602dc705 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -137,7 +137,10 @@ postgres$ initdb -D /usr/local/pgsql/data database, it is essential that it be secured from unauthorized access. initdb therefore revokes access permissions from everyone but the - PostgreSQL user. + PostgreSQL user, and optionally, group. + Group access, when enabled, is read-only. This allows an unprivileged + user in the PostgreSQL group to take a backup of + the cluster data or perform other operations that only require read access. @@ -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. + + 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. + + 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 4d836aaeaf..e0b67d7689 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..cdbf4d52d8 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) { @@ -3150,6 +3128,8 @@ main(int argc, char *argv[]) break; case 12: str_wal_segment_size_mb = pg_strdup(optarg); + case 'g': + file_mode_mask = PG_MODE_MASK_ALLOW_GROUP; break; default: /* getopt_long already emitted a complaint */ @@ -3159,6 +3139,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 62c72c3fcf..56cf0b1d9b 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" @@ -2107,7 +2108,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) @@ -2342,6 +2344,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_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 #include +#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 872621489f..a1088259af 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/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 + +#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.