From 8922c26e6a78168284f42489af1a28a01104b635 Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 4 Apr 2018 19:23:08 -0400 Subject: [PATCH 2/2] 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/ref/initdb.sgml | 19 ++++++++ doc/src/sgml/ref/pg_basebackup.sgml | 6 +++ doc/src/sgml/ref/pg_receivewal.sgml | 6 +++ doc/src/sgml/ref/pg_recvlogical.sgml | 11 +++++ doc/src/sgml/runtime.sgml | 14 +++++- src/backend/bootstrap/bootstrap.c | 7 ++- src/backend/main/main.c | 1 + src/backend/postmaster/postmaster.c | 44 ++++++++++++++---- src/backend/tcop/postgres.c | 8 +++- src/backend/utils/init/globals.c | 9 ++++ src/backend/utils/init/miscinit.c | 21 ++++----- src/backend/utils/misc/guc.c | 25 ++++++++++ src/bin/initdb/initdb.c | 32 ++++++++++--- src/bin/initdb/t/001_initdb.pl | 13 +++++- src/bin/pg_basebackup/pg_basebackup.c | 22 +++++---- src/bin/pg_basebackup/pg_receivewal.c | 7 +++ src/bin/pg_basebackup/pg_recvlogical.c | 7 +++ src/bin/pg_basebackup/streamutil.c | 69 ++++++++++++++++++++++++++++ src/bin/pg_basebackup/t/010_pg_basebackup.pl | 28 +++++++---- src/bin/pg_basebackup/t/020_pg_receivewal.pl | 2 +- src/bin/pg_ctl/pg_ctl.c | 12 ++++- src/bin/pg_ctl/t/001_start_stop.pl | 24 +++++++++- src/bin/pg_resetwal/pg_resetwal.c | 10 ++++ src/bin/pg_rewind/RewindTest.pm | 9 ++-- src/bin/pg_rewind/pg_rewind.c | 11 +++++ src/bin/pg_rewind/t/002_databases.pl | 6 ++- src/bin/pg_upgrade/pg_upgrade.c | 12 ++++- src/bin/pg_upgrade/test.sh | 16 +++---- src/common/Makefile | 2 +- src/common/file_perm.c | 68 +++++++++++++++++++++++++++ src/include/common/file_perm.h | 20 ++++++++ src/include/miscadmin.h | 1 + src/test/perl/PostgresNode.pm | 27 ++++++++++- src/test/perl/TestLib.pm | 25 ++++++++++ src/tools/msvc/Mkvcbuild.pm | 2 +- 35 files changed, 528 insertions(+), 68 deletions(-) diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml index 949b5a220f..1d41e91794 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/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 95045669c9..fc1edf4864 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -737,6 +737,12 @@ PostgreSQL documentation or later. + + pg_basebackup will preserve group permissions in + both the plain and tar formats if group + permissions are enabled on the source cluster. + + diff --git a/doc/src/sgml/ref/pg_receivewal.sgml b/doc/src/sgml/ref/pg_receivewal.sgml index e3f2ce1fcb..a18ddd4bff 100644 --- a/doc/src/sgml/ref/pg_receivewal.sgml +++ b/doc/src/sgml/ref/pg_receivewal.sgml @@ -425,6 +425,12 @@ PostgreSQL documentation not keep up with fetching the WAL data. + + pg_receivewal will preserve group permissions on + the received WAL files if group permissions are enabled on the source + cluster. + + diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml b/doc/src/sgml/ref/pg_recvlogical.sgml index a79ca20084..141c5cddce 100644 --- a/doc/src/sgml/ref/pg_recvlogical.sgml +++ b/doc/src/sgml/ref/pg_recvlogical.sgml @@ -399,6 +399,17 @@ PostgreSQL documentation + + Notes + + + pg_recvlogical will preserve group permissions on + the received WAL files if group permissions are enabled on the source + cluster. + + + + Examples diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 587b430527..40ce3d9813 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -137,7 +137,10 @@ postgres$ 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. @@ -2194,6 +2197,15 @@ pg_dumpall -p 5432 | psql -d postgres -p 5433 member of the group that has access to those certificate and key files. + + 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/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 28ff2f0979..9b74d1a158 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -24,6 +24,7 @@ #include "catalog/index.h" #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/file_perm.h" #include "libpq/pqsignal.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -223,7 +224,7 @@ AuxiliaryProcessMain(int argc, char *argv[]) /* If no -x argument, we are a CheckerProcess */ MyAuxProcType = CheckerProcess; - while ((flag = getopt(argc, argv, "B:c:d:D:Fkr:x:X:-:")) != -1) + while ((flag = getopt(argc, argv, "B:c:d:D:Fkr:x:X:-:g")) != -1) { switch (flag) { @@ -297,6 +298,10 @@ AuxiliaryProcessMain(int argc, char *argv[]) free(value); break; } + case 'g': + SetDataDirectoryCreatePerm(PG_DIR_MODE_GROUP); + umask(pg_mode_mask); + break; default: write_stderr("Try \"%s --help\" for more information.\n", progname); diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 38853e38eb..306cded3f7 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -334,6 +334,7 @@ help(const char *progname) printf(_(" -d 1-5 debugging level\n")); printf(_(" -D DATADIR database directory\n")); printf(_(" -e use European date input format (DMY)\n")); + printf(_(" -g allow group read/execute on data directory\n")); printf(_(" -F turn fsync off\n")); printf(_(" -h HOSTNAME host name or IP address to listen on\n")); printf(_(" -i enable TCP/IP connections\n")); diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 868fba8cac..fee602a056 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -588,7 +588,9 @@ PostmasterMain(int argc, char *argv[]) IsPostmasterEnvironment = true; /* - * for security, no dir or file created can be group or other accessible + * By default, no directory or file created can be group or other + * accessible. This may be modified later depending on the permissions of + * the data directory. */ umask(PG_MODE_MASK_DEFAULT); @@ -1522,25 +1524,47 @@ checkDataDir(void) #endif /* - * Check if the directory has group or world access. If so, reject. + * Check if the directory has correct permissions. If not, 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. + * Only two possible modes are allowed, 0700 and 0750. The latter mode + * indicates that group read/execute should be allowed on all newly created + * files and directories. * * 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_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 creation modes and mask based on the mode of the data directory. + * + * The mask was set earlier in startup to disallow group permissions on + * newly created files and directories. However, if group read/execute are + * present on the data directory then modify the create modes and mask to + * allow group read/execute on newly created files and directories and set + * the data_directory_mode GUC. + * + * Suppress when on Windows, because there may not be proper support + * for Unix-y file permissions. + */ +#if !defined(WIN32) && !defined(__CYGWIN__) + SetDataDirectoryCreatePerm(stat_buf.st_mode); + + if (pg_dir_create_mode == PG_DIR_MODE_GROUP) + { + umask(pg_mode_mask); + + SetConfigOption("data_directory_mode", "0750", PGC_INTERNAL, + PGC_S_OVERRIDE); + } #endif /* Look for PG_VERSION before looking for pg_control */ diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 6fc1cc272b..2cc27e31ea 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -42,6 +42,7 @@ #include "catalog/pg_type.h" #include "commands/async.h" #include "commands/prepare.h" +#include "common/file_perm.h" #include "jit/jit.h" #include "libpq/libpq.h" #include "libpq/pqformat.h" @@ -3397,7 +3398,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx, * postmaster/postmaster.c (the option sets should not conflict) and with * the common help() function in main/main.c. */ - while ((flag = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1) + while ((flag = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:g")) != -1) { switch (flag) { @@ -3560,6 +3561,11 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx, break; } + case 'g': + SetDataDirectoryCreatePerm(PG_DIR_MODE_GROUP); + umask(pg_mode_mask); + break; + default: errs++; break; diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index c1f0441b08..3de7add3f9 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -16,8 +16,11 @@ * *------------------------------------------------------------------------- */ +#include + #include "postgres.h" +#include "common/file_perm.h" #include "libpq/libpq-be.h" #include "libpq/pqcomm.h" #include "miscadmin.h" @@ -59,6 +62,12 @@ struct Latch *MyLatch; */ char *DataDir = NULL; +/* + * Mode of the data directory. The default is 0700 but may it be changed in + * checkDataDir() to 0750 if the data directory actually has that mode. + */ +int data_directory_mode = PG_DIR_MODE_GROUP; + char OutputFileName[MAXPGPATH]; /* debugging output file */ char my_exec_path[MAXPGPATH]; /* full path to my executable */ diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index f8f08f3f88..6235fab64e 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -829,7 +829,7 @@ CreateLockFile(const char *filename, bool amPostmaster, /* * Try to create the lock file --- O_EXCL makes this atomic. * - * Think not to make the file protection weaker than 0600. See + * Think not to make the file protection weaker than 0600/0640. See * comments below. */ fd = open(filename, O_RDWR | O_CREAT | O_EXCL, pg_file_create_mode); @@ -899,17 +899,14 @@ CreateLockFile(const char *filename, bool amPostmaster, * implies that the existing process has a different userid than we * do, which means it cannot be a competing postmaster. A postmaster * cannot successfully attach to a data directory owned by a userid - * other than its own. (This is now checked directly in - * checkDataDir(), but has been true for a long time because of the - * restriction that the data directory isn't group- or - * world-accessible.) Also, since we create the lockfiles mode 600, - * we'd have failed above if the lockfile belonged to another userid - * --- which means that whatever process kill() is reporting about - * isn't the one that made the lockfile. (NOTE: this last - * consideration is the only one that keeps us from blowing away a - * Unix socket file belonging to an instance of Postgres being run by - * someone else, at least on machines where /tmp hasn't got a - * stickybit.) + * other than its own, as enforced in checkDataDir(). Also, since we + * create the lockfiles mode 0600/0640, we'd have failed above if the + * lockfile belonged to another userid --- which means that whatever + * process kill() is reporting about isn't the one that made the + * lockfile. (NOTE: this last consideration is the only one that keeps + * us from blowing away a Unix socket file belonging to an instance of + * Postgres being run by someone else, at least on machines where /tmp + * hasn't got a stickybit.) */ if (other_pid != my_pid && other_pid != my_p_pid && other_pid != my_gp_pid) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 260ae264d8..fa92ce2e68 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -192,6 +192,7 @@ static void assign_application_name(const char *newval, void *extra); static bool check_cluster_name(char **newval, void **extra, GucSource source); static const char *show_unix_socket_permissions(void); static const char *show_log_file_mode(void); +static const char *show_data_directory_mode(void); /* Private functions in guc-file.l that need to be called from guc.c */ static ConfigVariable *ProcessConfigFileInternal(GucContext context, @@ -2042,6 +2043,21 @@ static struct config_int ConfigureNamesInt[] = NULL, NULL, show_log_file_mode }, + + { + {"data_directory_mode", PGC_INTERNAL, PRESET_OPTIONS, + gettext_noop("Mode of the data directory."), + gettext_noop("The parameter value is a numeric mode specification " + "in the form accepted by the chmod and umask system " + "calls. (To use the customary octal format the number " + "must start with a 0 (zero).)"), + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + }, + &data_directory_mode, + 0700, 0000, 0777, + NULL, NULL, show_data_directory_mode + }, + { {"work_mem", PGC_USERSET, RESOURCES_MEM, gettext_noop("Sets the maximum memory to be used for query workspaces."), @@ -10731,4 +10747,13 @@ show_log_file_mode(void) return buf; } +static const char * +show_data_directory_mode(void) +{ + static char buf[12]; + + snprintf(buf, sizeof(buf), "%04o", data_directory_mode); + return buf; +} + #include "guc-file.c" diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 83472d9ceb..c678e2ef0d 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -1168,6 +1168,19 @@ setup_config(void) "password_encryption = scram-sha-256"); } + /* + * If group access has been enabled for the cluster then it makes sense + * to ensure that the log files also allow group access. Otherwise a + * backup from a user in the group would fail if the log files were not + * relocated. + */ + if (pg_dir_create_mode == PG_DIR_MODE_GROUP) + { + conflines = replace_token(conflines, + "#log_file_mode = 0600", + "log_file_mode = 0640"); + } + snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data); writefile(path, conflines); @@ -1382,12 +1395,13 @@ bootstrap_template1(void) unsetenv("PGCLIENTENCODING"); snprintf(cmd, sizeof(cmd), - "\"%s\" --boot -x1 -X %u %s %s %s", + "\"%s\" --boot -x1 -X %u %s %s %s %s", backend_exec, wal_segment_size_mb * (1024 * 1024), data_checksums ? "-k" : "", boot_options, - debug ? "-d 5" : ""); + debug ? "-d 5" : "", + pg_dir_create_mode == PG_DIR_MODE_GROUP ? "-g" : ""); PG_CMD_OPEN; @@ -2312,6 +2326,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" @@ -2883,8 +2898,8 @@ initialize_data_directory(void) setup_signals(); - /* Set dir/file mode mask */ - umask(PG_MODE_MASK_DEFAULT); + /* Set mask based on requested PGDATA permissions */ + umask(pg_mode_mask); create_data_directory(); @@ -2942,8 +2957,9 @@ initialize_data_directory(void) fflush(stdout); snprintf(cmd, sizeof(cmd), - "\"%s\" %s template1 >%s", + "\"%s\" %s %s template1 >%s", backend_exec, backend_options, + pg_dir_create_mode == PG_DIR_MODE_GROUP ? "-g" : "", DEVNULL); PG_CMD_OPEN; @@ -3018,6 +3034,7 @@ main(int argc, char *argv[]) {"waldir", required_argument, NULL, 'X'}, {"wal-segsize", required_argument, NULL, 12}, {"data-checksums", no_argument, NULL, 'k'}, + {"allow-group-access", no_argument, NULL, 'g'}, {NULL, 0, NULL, 0} }; @@ -3059,7 +3076,7 @@ main(int argc, char *argv[]) /* process command-line options */ - while ((c = getopt_long(argc, argv, "dD:E:kL:nNU:WA:sST:X:", long_options, &option_index)) != -1) + while ((c = getopt_long(argc, argv, "dD:E:kL:nNU:WA:sST:X:g", long_options, &option_index)) != -1) { switch (c) { @@ -3153,6 +3170,9 @@ main(int argc, char *argv[]) case 12: str_wal_segment_size_mb = pg_strdup(optarg); break; + case 'g': + SetDataDirectoryCreatePerm(PG_DIR_MODE_GROUP); + break; default: /* getopt_long already emitted a complaint */ fprintf(stderr, _("Try \"%s --help\" for more information.\n"), diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl index 3331560445..f08fc6ea8f 100644 --- a/src/bin/initdb/t/001_initdb.pl +++ b/src/bin/initdb/t/001_initdb.pl @@ -4,9 +4,11 @@ use strict; use warnings; +use Fcntl ':mode'; +use File::stat qw{lstat}; use PostgresNode; use TestLib; -use Test::More tests => 16; +use Test::More tests => 18; my $tempdir = TestLib::tempdir; my $xlogdir = "$tempdir/pgxlog"; @@ -50,3 +52,12 @@ mkdir $datadir; } command_ok([ 'initdb', '-S', $datadir ], 'sync only'); command_fails([ 'initdb', $datadir ], 'existing data directory'); + +# Init a new db with group access +my $datadir_group = "$tempdir/data_group"; + +command_ok( + [ 'initdb', '-g', $datadir_group ], + 'successful creation with group access'); + +ok(check_mode_recursive($datadir_group, 0750, 0640)); diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 32b41e313c..5cd535184a 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -2470,14 +2470,6 @@ main(int argc, char **argv) } #endif - /* - * Verify that the target directory exists, or create it. For plaintext - * backups, always require the directory. For tar backups, require it - * unless we are writing to stdout. - */ - if (format == 'p' || strcmp(basedir, "-") != 0) - verify_dir_is_empty_or_create(basedir, &made_new_pgdata, &found_existing_pgdata); - /* connection in replication mode to server */ conn = GetConnection(); if (!conn) @@ -2486,6 +2478,20 @@ main(int argc, char **argv) exit(1); } + /* + * Set umask so that directories/files are created with the same permissions + * as directories/files in the source data directory. + */ + umask(pg_mode_mask); + + /* + * Verify that the target directory exists, or create it. For plaintext + * backups, always require the directory. For tar backups, require it + * unless we are writing to stdout. + */ + if (format == 'p' || strcmp(basedir, "-") != 0) + verify_dir_is_empty_or_create(basedir, &made_new_pgdata, &found_existing_pgdata); + /* determine remote server's xlog segment size */ if (!RetrieveWalSegSize(conn)) disconnect_and_exit(1); diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c index b82e073b86..f23ecf799f 100644 --- a/src/bin/pg_basebackup/pg_receivewal.c +++ b/src/bin/pg_basebackup/pg_receivewal.c @@ -19,6 +19,7 @@ #include #include +#include "common/file_perm.h" #include "libpq-fe.h" #include "access/xlog_internal.h" #include "getopt_long.h" @@ -703,6 +704,12 @@ main(int argc, char **argv) if (!RunIdentifySystem(conn, NULL, NULL, NULL, &db_name)) disconnect_and_exit(1); + /* + * Set umask so that directories/files are created with the same permissions + * as directories/files in the source data directory. + */ + umask(pg_mode_mask); + /* determine remote server's xlog segment size */ if (!RetrieveWalSegSize(conn)) disconnect_and_exit(1); diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c index 0866395944..e3b6404b1f 100644 --- a/src/bin/pg_basebackup/pg_recvlogical.c +++ b/src/bin/pg_basebackup/pg_recvlogical.c @@ -23,6 +23,7 @@ #include "streamutil.h" #include "access/xlog_internal.h" +#include "common/file_perm.h" #include "common/fe_memutils.h" #include "getopt_long.h" #include "libpq-fe.h" @@ -959,6 +960,12 @@ main(int argc, char **argv) disconnect_and_exit(1); } + /* + * Set umask so that directories/files are created with the same permissions + * as directories/files in the source data directory. + */ + umask(pg_mode_mask); + /* Drop a replication slot. */ if (do_drop_slot) { diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 1438f368ed..494acfec0e 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -23,6 +23,7 @@ #include "access/xlog_internal.h" #include "common/fe_memutils.h" +#include "common/file_perm.h" #include "datatype/timestamp.h" #include "fe_utils/connect.h" #include "port/pg_bswap.h" @@ -32,9 +33,16 @@ uint32 WalSegSz; +static bool RetrieveDataDirCreatePerm(PGconn *conn); + /* SHOW command for replication connection was introduced in version 10 */ #define MINIMUM_VERSION_FOR_SHOW_CMD 100000 +/* + * Group access is supported from version 11. + */ +#define MINIMUM_VERSION_FOR_GROUP_ACCESS 110000 + const char *progname; char *connection_string = NULL; char *dbhost = NULL; @@ -254,6 +262,16 @@ GetConnection(void) exit(1); } + /* + * Retrieve the source data directory mode and use to construct a umask + * for creating directories and files + */ + if (!RetrieveDataDirCreatePerm(tmpconn)) + { + PQfinish(tmpconn); + exit(1); + } + return tmpconn; } @@ -327,6 +345,57 @@ RetrieveWalSegSize(PGconn *conn) return true; } +/* + * From version 11, check the mode of the data directory to determine + * permissions to use for directories created by the utility. + */ +static bool +RetrieveDataDirCreatePerm(PGconn *conn) +{ + PGresult *res; + int data_directory_mode; + + /* check connection existence */ + Assert(conn != NULL); + + /* for previous versions leave the default group access */ + if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_GROUP_ACCESS) + return true; + + res = PQexec(conn, "SHOW data_directory_mode"); + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + fprintf(stderr, _("%s: could not send replication command \"%s\": %s\n"), + progname, "SHOW data_directory_mode", PQerrorMessage(conn)); + + PQclear(res); + return false; + } + if (PQntuples(res) != 1 || PQnfields(res) < 1) + { + fprintf(stderr, + _("%s: could not fetch group access flag: got %d rows and %d fields, expected %d rows and %d or more fields\n"), + progname, PQntuples(res), PQnfields(res), 1, 1); + + PQclear(res); + return false; + } + + if (sscanf(PQgetvalue(res, 0, 0), "%o", &data_directory_mode) != 1) + { + fprintf(stderr, _("%s: group access flag could not be parsed: %s\n"), + progname, PQgetvalue(res, 0, 0)); + + PQclear(res); + return false; + } + + SetDataDirectoryCreatePerm(data_directory_mode); + + PQclear(res); + return true; +} + /* * Run IDENTIFY_SYSTEM through a given connection and give back to caller * some result information if requested: diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 496c22da7d..9a29e6416a 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -5,7 +5,7 @@ use Config; use File::Basename qw(basename dirname); use PostgresNode; use TestLib; -use Test::More tests => 105; +use Test::More tests => 106; program_help_ok('pg_basebackup'); program_version_ok('pg_basebackup'); @@ -16,7 +16,7 @@ my $tempdir = TestLib::tempdir; my $node = get_new_node('main'); # Set umask so no test directories or files are created with group permissions -umask (0077); +umask(0077); # Initialize node without replication settings $node->init(extra => [ '--data-checksums' ]); @@ -43,11 +43,6 @@ $node->command_fails( ok(!-d "$tempdir/backup", 'backup directory was cleaned up'); -$node->command_fails([ 'pg_basebackup', '-D', "$tempdir/backup", '-n' ], - 'failing run with no-clean option'); - -ok(-d "$tempdir/backup", 'backup directory was created and left behind'); - open my $conf, '>>', "$pgdata/postgresql.conf"; print $conf "max_replication_slots = 10\n"; print $conf "max_wal_senders = 10\n"; @@ -157,6 +152,13 @@ ok(-f "$tempdir/tarbackup/base.tar", 'backup tar was created'); $node->command_fails( [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T=/foo" ], '-T with empty old directory fails'); + +$node->command_fails( + [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T=/foo", "-n" ], + 'failing run with no-clean option'); + +ok(-d "$tempdir/backup", 'backup directory was created and left behind'); + $node->command_fails( [ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-T/foo=" ], '-T with empty new directory fails'); @@ -190,11 +192,17 @@ unlink "$pgdata/$superlongname"; # skip on Windows. SKIP: { - skip "symlinks not supported on Windows", 17 if ($windows_os); + skip "symlinks not supported on Windows", 18 if ($windows_os); # Move pg_replslot out of $pgdata and create a symlink to it. $node->stop; + # Set umask so test directories and files are created with group permissions + umask (0027); + + # Enable group permissions on PGDATA + chmod_recursive("$pgdata", 0750, 0640); + rename("$pgdata/pg_replslot", "$tempdir/pg_replslot") or BAIL_OUT "could not move $pgdata/pg_replslot"; symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot") @@ -264,6 +272,10 @@ SKIP: "tablespace symlink was updated"); closedir $dh; + # Group access should be enabled on all backup files + ok(check_mode_recursive("$tempdir/backup1", 0750, 0640), + "check backup dir permissions"); + # Unlogged relation forks other than init should not be copied my ($tblspc1UnloggedBackupPath) = $tblspc1UnloggedPath =~ /[^\/]*\/[^\/]*\/[^\/]*$/g; diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl index d155d5b711..752ba5df93 100644 --- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl +++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl @@ -9,7 +9,7 @@ program_version_ok('pg_receivewal'); program_options_handling_ok('pg_receivewal'); # Set umask so no test directories or files are created with group permissions -umask (0077); +umask(0077); my $primary = get_new_node('primary'); $primary->init(allows_streaming => 1); diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index e83a7179ea..e0c9687a35 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -2171,7 +2171,7 @@ main(int argc, char **argv) */ argv0 = argv[0]; - /* Set dir/file mode mask */ + /* Set restrictive mode mask until PGDATA permissions are checked */ umask(PG_MODE_MASK_DEFAULT); /* support --help and --version even if invoked as root */ @@ -2407,6 +2407,16 @@ 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, + * + * Don't error here if the data directory cannot be stat'd. This is + * handled differently based on the command and we don't want to + * interfere with that logic. + */ + if (GetDataDirectoryCreatePerm(pg_data)) + umask(pg_mode_mask); } switch (ctl_command) diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl index 3d82abe696..91e7f00326 100644 --- a/src/bin/pg_ctl/t/001_start_stop.pl +++ b/src/bin/pg_ctl/t/001_start_stop.pl @@ -2,9 +2,11 @@ use strict; use warnings; use Config; +use Fcntl ':mode'; +use File::stat qw{lstat}; use PostgresNode; use TestLib; -use Test::More tests => 20; +use Test::More tests => 24; my $tempdir = TestLib::tempdir; my $tempdir_short = TestLib::tempdir_short; @@ -68,4 +70,24 @@ command_ok( ok(-f $logFileName); ok(check_mode_recursive("$tempdir/data", 0700, 0600)); +# Log file for second perm test +$logFileName = "$tempdir/data/perm-test-640.log"; + +# Change the data dir mode so log file will be created with group read +# privileges on the next start +system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data"; + +chmod_recursive("$tempdir/data", 0750, 0640); + +command_ok( + [ 'pg_ctl', 'start', '-D', "$tempdir/data", '-l', $logFileName ], + 'start server to check group permissions'); + +ok(-f $logFileName); +ok(check_mode_recursive("$tempdir/data", 0750, 0640)); + +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 bdf71886ee..8a0a805f1e 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -363,6 +363,16 @@ main(int argc, char *argv[]) exit(1); } + /* Set mask based on PGDATA permissions */ + if (!GetDataDirectoryCreatePerm(DataDir)) + { + fprintf(stderr, _("%s: unable to read permissions from \"%s\"\n"), + progname, DataDir); + exit(1); + } + + umask(pg_mode_mask); + /* Check that data directory matches our server version */ CheckDataVersion(); diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm index 7b632c7dcd..63d9bd517d 100644 --- a/src/bin/pg_rewind/RewindTest.pm +++ b/src/bin/pg_rewind/RewindTest.pm @@ -115,11 +115,13 @@ sub check_query sub setup_cluster { - my $extra_name = shift; + my $extra_name = shift; # Used to differentiate clusters + my $extra = shift; # Extra params for initdb # Initialize master, data checksums are mandatory $node_master = get_new_node('master' . ($extra_name ? "_${extra_name}" : '')); - $node_master->init(allows_streaming => 1); + $node_master->init( + allows_streaming => 1, extra => $extra); # Set wal_keep_segments to prevent WAL segment recycling after enforced # checkpoints in the tests. $node_master->append_conf('postgresql.conf', qq( @@ -237,7 +239,8 @@ sub run_pg_rewind "$tmp_folder/master-postgresql.conf.tmp", "$master_pgdata/postgresql.conf"); - chmod(0600, "$master_pgdata/postgresql.conf") + chmod($node_master->group_access() ? 0640 : 0600, + "$master_pgdata/postgresql.conf") or BAIL_OUT( "unable to set permissions for $master_pgdata/postgresql.conf"); diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index 72ab2f8d5e..b9ea6a4c21 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,16 @@ main(int argc, char **argv) exit(1); } + /* Set mask based on PGDATA permissions */ + if (!GetDataDirectoryCreatePerm(datadir_target)) + { + fprintf(stderr, _("%s: unable to read permissions from \"%s\"\n"), + progname, datadir_target); + exit(1); + } + + umask(pg_mode_mask); + /* * 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/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl index 37cdd712f3..570fa5cee0 100644 --- a/src/bin/pg_rewind/t/002_databases.pl +++ b/src/bin/pg_rewind/t/002_databases.pl @@ -1,7 +1,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 4; +use Test::More tests => 6; use RewindTest; @@ -9,7 +9,7 @@ sub run_test { my $test_mode = shift; - RewindTest::setup_cluster($test_mode); + RewindTest::setup_cluster($test_mode, ['-g']); RewindTest::start_master(); # Create a database in master. @@ -42,6 +42,8 @@ template1 ), 'database names'); + ok (check_mode_recursive( + $node_master->data_dir(), 0750, 0640, ['postmaster.pid'])); RewindTest::clean_rewind_test(); } diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 59df6fd88f..4abf4dc893 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,16 @@ main(int argc, char **argv) check_cluster_compatibility(live_check); + /* Set mask based on PGDATA permissions */ + if (!GetDataDirectoryCreatePerm(new_cluster.pgdata)) + { + pg_log(PG_FATAL, "unable to read permissions from \"%s\"\n", + new_cluster.pgdata); + exit(1); + } + + umask(pg_mode_mask); + check_and_dump_old_cluster(live_check); diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh index 574639d47e..24631a91c6 100644 --- a/src/bin/pg_upgrade/test.sh +++ b/src/bin/pg_upgrade/test.sh @@ -20,9 +20,9 @@ unset MAKELEVEL # Run a given "initdb" binary and overlay the regression testing # authentication configuration. standard_initdb() { - # To increase coverage of non-standard segment size without - # increase test runtime, run these tests with a lower setting. - "$1" -N --wal-segsize 1 + # To increase coverage of non-standard segment size and group access + # without increasing test runtime, run these tests with a custom setting. + "$1" -N --wal-segsize 1 -g if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ] then cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf" @@ -230,14 +230,14 @@ 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 correct permissions -if [ $(find ${PGDATA} -type f ! -perm 600 | wc -l) -ne 0 ]; then - echo "files in PGDATA with permission != 600"; +# make sure all directories and files have group permissions +if [ $(find ${PGDATA} -type f ! -perm 640 | wc -l) -ne 0 ]; then + echo "files in PGDATA with permission != 640"; exit 1; fi -if [ $(find ${PGDATA} -type d ! -perm 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 e9e75867f3..2bab992de0 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 index 5f0c02fb04..571d37116b 100644 --- a/src/common/file_perm.c +++ b/src/common/file_perm.c @@ -12,8 +12,76 @@ */ #include +#include "c.h" #include "common/file_perm.h" /* Modes for creating directories and files in the data directory */ int pg_dir_create_mode = PG_DIR_MODE_DEFAULT; int pg_file_create_mode = PG_FILE_MODE_DEFAULT; + +/* + * Mode mask to pass to umask(). This is more of a preventative measure since + * all file/directory creates should be performed using the create modes above. + */ +int pg_mode_mask = PG_MODE_MASK_DEFAULT; + +/* + * Set create modes and mask to use when writing to PGDATA based on the data + * directory mode passed. If group read/execute are present in the + * mode, then create modes and mask will be relaxed to allow group read/execute + * on all newly created files and directories. + */ +void +SetDataDirectoryCreatePerm(int dataDirMode) +{ + /* If the data directory mode has group access */ + if ((PG_DIR_MODE_GROUP & dataDirMode) == PG_DIR_MODE_GROUP) + { + pg_dir_create_mode = PG_DIR_MODE_GROUP; + pg_file_create_mode = PG_FILE_MODE_GROUP; + pg_mode_mask = PG_MODE_MASK_GROUP; + } + /* Else use default permissions */ + else + { + pg_dir_create_mode = PG_DIR_MODE_DEFAULT; + pg_file_create_mode = PG_FILE_MODE_DEFAULT; + pg_mode_mask = PG_MODE_MASK_DEFAULT; + } +} + +#ifdef FRONTEND + +/* + * Get the create modes and mask to use when writing to PGDATA by examining the + * mode of the PGDATA directory and calling SetDataDirectoryCreatePerm(). + * + * Errors are not handled here and should be reported by the frontend + * application when false is returned. + * + * Suppress when on Windows, because there may not be proper support for Unix-y + * file permissions. + */ +bool +GetDataDirectoryCreatePerm(const char *dataDir) +{ +#if !defined(WIN32) && !defined(__CYGWIN__) + struct stat statBuf; + + /* + * If an error occurs getting the mode then false It is the reponsibility of + * the frontend application to generate an error if the PGDATA directory + * cannot be accessed. + */ + if (stat(dataDir, &statBuf) == -1) + return false; + + /* Set permissions */ + SetDataDirectoryCreatePerm(statBuf.st_mode); + + return true; +#endif /* !defined(WIN32) && !defined(__CYGWIN__) */ +} + + +#endif /* FRONTEND */ diff --git a/src/include/common/file_perm.h b/src/include/common/file_perm.h index 46b91f0c7b..22afb14b55 100644 --- a/src/include/common/file_perm.h +++ b/src/include/common/file_perm.h @@ -19,14 +19,34 @@ */ #define PG_MODE_MASK_DEFAULT (S_IRWXG | S_IRWXO) +/* + * Mode mask for data directory permissions that also allows group read/execute. + */ +#define PG_MODE_MASK_GROUP (S_IWGRP | S_IRWXO) + /* Default mode for creating directories */ #define PG_DIR_MODE_DEFAULT S_IRWXU +/* Mode for creating directories that allows group read/execute */ +#define PG_DIR_MODE_GROUP (S_IRWXU | S_IRGRP | S_IXGRP) + /* Default mode for creating files */ #define PG_FILE_MODE_DEFAULT (S_IRUSR | S_IWUSR) +/* Mode for creating files that allows group read */ +#define PG_FILE_MODE_GROUP (S_IRUSR | S_IWUSR | S_IRGRP) + /* Modes for creating directories and files in the data directory */ extern int pg_dir_create_mode; extern int pg_file_create_mode; +/* Mode mask to pass to umask() */ +extern int pg_mode_mask; + +/* Set permissions and mask based on the provided mode */ +extern void SetDataDirectoryCreatePerm(int dataDirMode); + +/* Set permissions and mask based on the mode of the data directory */ +extern bool GetDataDirectoryCreatePerm(const char *dataDir); + #endif /* FILE_PERM_H */ diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index a429a19964..0438ea809c 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -153,6 +153,7 @@ extern PGDLLIMPORT bool IsBinaryUpgrade; extern PGDLLIMPORT bool ExitOnAnyError; extern PGDLLIMPORT char *DataDir; +extern PGDLLIMPORT int data_directory_mode; extern PGDLLIMPORT int NBuffers; extern PGDLLIMPORT int MaxBackends; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 76e571b98c..1bd91524d7 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -86,9 +86,11 @@ use Carp; use Config; use Cwd; use Exporter 'import'; +use Fcntl qw(:mode); use File::Basename; use File::Path qw(rmtree); use File::Spec; +use File::stat qw(stat); use File::Temp (); use IPC::Run; use RecursiveCopy; @@ -269,6 +271,26 @@ sub connstr =pod +=item $node->group_access() + +Does the data dir allow group access? + +=cut + +sub group_access +{ + my ($self) = @_; + + my $dir_stat = stat($self->data_dir); + + defined($dir_stat) + or die('unable to stat ' . $self->data_dir); + + return (S_IMODE($dir_stat->mode) == 0750); +} + +=pod + =item $node->data_dir() Returns the path to the data directory. postgresql.conf and pg_hba.conf are @@ -460,6 +482,9 @@ sub init } close $conf; + chmod($self->group_access ? 0640 : 0600, "$pgdata/postgresql.conf") + or die("unable to set permissions for $pgdata/postgresql.conf"); + $self->set_replication_conf if $params{allows_streaming}; $self->enable_archiving if $params{has_archiving}; } @@ -485,7 +510,7 @@ sub append_conf TestLib::append_to_file($conffile, $str . "\n"); - chmod(0600, $conffile) + chmod($self->group_access() ? 0640 : 0600, $conffile) or die("unable to set permissions for $conffile"); } diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 93610e4bc4..8047404efd 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -31,6 +31,7 @@ our @EXPORT = qw( slurp_file append_to_file check_mode_recursive + chmod_recursive check_pg_config system_or_bail system_log @@ -313,6 +314,30 @@ sub check_mode_recursive return $result; } +# Change mode recursively on a directory +sub chmod_recursive +{ + my ($dir, $dir_mode, $file_mode) = @_; + + find + ( + {follow_fast => 1, + wanted => + sub + { + my $file_stat = stat($File::Find::name); + + if (defined($file_stat)) + { + chmod(S_ISDIR($file_stat->mode) ? $dir_mode : $file_mode, + $File::Find::name) + or die "unable to chmod $File::Find::name"; + } + }}, + $dir + ); +} + # Check presence of a given regexp within pg_config.h for the installation # where tests are running, returning a match status result depending on # that. diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 1d3ed6b0b1..764c8b8cd2 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -126,7 +126,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)