diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index a189a8efc3f..5d5f2d23c4f 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8144,6 +8144,23 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' + + data_directory_mode (integer) + + data_directory_mode configuration parameter + + + + + On Unix systems this parameter reports the permissions of the data + directory defined by () at startup. + (On Microsoft Windows this parameter will always display + 0700). See + for more information. + + + + debug_assertions (boolean) diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml index 826dd91f729..10a8a86a030 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, @@ -188,6 +196,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 95045669c93..fc1edf48645 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 e3f2ce1fcb7..a18ddd4bff1 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 a79ca200849..141c5cddce1 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 587b4305274..330e38a29e9 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -137,7 +137,22 @@ 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 same group as the cluster owner to take a backup of the + cluster data or perform other operations that only require read access. + + + + Note that enabling or disabling group access on an existing cluster requires + the cluster to be shut down and the appropriate mode to be set on all + directories and files before restarting + PostgreSQL. Otherwise, a mix of modes might + exist in the data directory. For clusters that allow access only by the + owner, the appropriate modes are 0700 for directories + and 0600 for files. For clusters that also allow + reads by the group, the appropriate modes are 0750 + for directories and 0640 for files. @@ -2194,6 +2209,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 644084d1c3b..59cd4b17f32 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -349,13 +349,15 @@ AuxiliaryProcessMain(int argc, char *argv[]) proc_exit(1); } - /* Validate we have been given a reasonable-looking DataDir */ - Assert(DataDir); - ValidatePgVersion(DataDir); - - /* Change into DataDir (if under postmaster, should be done already) */ + /* + * Validate we have been given a reasonable-looking DataDir and change + * into it (if under postmaster, should be done already). + */ if (!IsUnderPostmaster) + { + checkDataDir(); ChangeToDataDir(); + } /* If standalone, create lockfile for data directory */ if (!IsUnderPostmaster) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 10afecffb37..ccf63e93097 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -391,7 +391,7 @@ static DNSServiceRef bonjour_sdref = NULL; static void CloseServerPorts(int status, Datum arg); static void unlink_external_pid_file(int status, Datum arg); static void getInstallationPaths(const char *argv0); -static void checkDataDir(void); +static void checkControlFile(void); static Port *ConnCreate(int serverFd); static void ConnFree(Port *port); static void reset_shared(int port); @@ -588,7 +588,12 @@ PostmasterMain(int argc, char *argv[]) IsPostmasterEnvironment = true; /* - * for security, no dir or file created can be group or other accessible + * We should not be creating any files or directories before we check the + * data directory (see checkDataDir()), but just in case set the umask to + * the most restrictive (owner-only) permissions. + * + * checkDataDir() will reset the umask based on the data directory + * permissions. */ umask(PG_MODE_MASK_OWNER); @@ -877,6 +882,9 @@ PostmasterMain(int argc, char *argv[]) /* Verify that DataDir looks reasonable */ checkDataDir(); + /* Check that pg_control exists */ + checkControlFile(); + /* And switch working directory into it */ ChangeToDataDir(); @@ -1469,82 +1477,17 @@ getInstallationPaths(const char *argv0) */ } - /* - * Validate the proposed data directory + * Check that pg_control exists in the correct location in the data directory. + * + * No attempt is made to validate the contents of pg_control here. This is + * just a sanity check to see if we are looking at a real data directory. */ static void -checkDataDir(void) +checkControlFile(void) { char path[MAXPGPATH]; FILE *fp; - struct stat stat_buf; - - Assert(DataDir); - - if (stat(DataDir, &stat_buf) != 0) - { - if (errno == ENOENT) - ereport(FATAL, - (errcode_for_file_access(), - errmsg("data directory \"%s\" does not exist", - DataDir))); - else - ereport(FATAL, - (errcode_for_file_access(), - errmsg("could not read permissions of directory \"%s\": %m", - DataDir))); - } - - /* eventual chdir would fail anyway, but let's test ... */ - if (!S_ISDIR(stat_buf.st_mode)) - ereport(FATAL, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("specified data directory \"%s\" is not a directory", - DataDir))); - - /* - * Check that the directory belongs to my userid; if not, reject. - * - * This check is an essential part of the interlock that prevents two - * postmasters from starting in the same directory (see CreateLockFile()). - * Do not remove or weaken it. - * - * XXX can we safely enable this check on Windows? - */ -#if !defined(WIN32) && !defined(__CYGWIN__) - if (stat_buf.st_uid != geteuid()) - ereport(FATAL, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("data directory \"%s\" has wrong ownership", - DataDir), - errhint("The server must be started by the user that owns the data directory."))); -#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. - * - * 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)) - ereport(FATAL, - (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), - errmsg("data directory \"%s\" has group or world access", - DataDir), - errdetail("Permissions should be u=rwx (0700)."))); -#endif - - /* Look for PG_VERSION before looking for pg_control */ - ValidatePgVersion(DataDir); snprintf(path, sizeof(path), "%s/global/pg_control", DataDir); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 7bdecc32ec0..5095a4f6867 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3731,8 +3731,7 @@ PostgresMain(int argc, char *argv[], * Validate we have been given a reasonable-looking DataDir (if under * postmaster, assume postmaster did this already). */ - Assert(DataDir); - ValidatePgVersion(DataDir); + checkDataDir(); /* Change into DataDir (if under postmaster, was done already) */ ChangeToDataDir(); diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index c1f0441b081..0a3163398f3 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_OWNER; + 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 f8f08f3f88b..03b28c3604a 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -88,6 +88,100 @@ SetDatabasePath(const char *path) DatabasePath = MemoryContextStrdup(TopMemoryContext, path); } +/* + * Validate the proposed data directory. + * + * Also initialize file and directory create modes and mode mask. + */ +void +checkDataDir(void) +{ + struct stat stat_buf; + + Assert(DataDir); + + if (stat(DataDir, &stat_buf) != 0) + { + if (errno == ENOENT) + ereport(FATAL, + (errcode_for_file_access(), + errmsg("data directory \"%s\" does not exist", + DataDir))); + else + ereport(FATAL, + (errcode_for_file_access(), + errmsg("could not read permissions of directory \"%s\": %m", + DataDir))); + } + + /* eventual chdir would fail anyway, but let's test ... */ + if (!S_ISDIR(stat_buf.st_mode)) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("specified data directory \"%s\" is not a directory", + DataDir))); + + /* + * Check that the directory belongs to my userid; if not, reject. + * + * This check is an essential part of the interlock that prevents two + * postmasters from starting in the same directory (see CreateLockFile()). + * Do not remove or weaken it. + * + * XXX can we safely enable this check on Windows? + */ +#if !defined(WIN32) && !defined(__CYGWIN__) + if (stat_buf.st_uid != geteuid()) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("data directory \"%s\" has wrong ownership", + DataDir), + errhint("The server must be started by the user that owns the data directory."))); +#endif + + /* + * Check if the directory has correct permissions. If not, reject. + * + * 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 & PG_MODE_MASK_GROUP) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("data directory \"%s\" has invalid permissions", + DataDir), + 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); + + umask(pg_mode_mask); + data_directory_mode = pg_dir_create_mode; +#endif + + /* Check for PG_VERSION */ + ValidatePgVersion(DataDir); +} + /* * Set data directory, but make sure it's an absolute path. Use this, * never set DataDir directly. @@ -829,7 +923,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 +993,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 71c2b4eff16..8441837e0ff 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -194,6 +194,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, @@ -2044,6 +2045,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."), @@ -10744,4 +10760,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 3765548a249..ec1f0c4bff1 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); @@ -2312,6 +2325,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 +2897,13 @@ initialize_data_directory(void) setup_signals(); - /* Set dir/file mode mask */ - umask(PG_MODE_MASK_OWNER); + /* + * Set mask based on requested PGDATA permissions. pg_mode_mask, and + * friends like pg_dir_create_mode, are set to owner-only by default and + * then updated if -g is passed in by calling SetDataDirectoryCreatePerm() + * when parsing our options (see above). + */ + umask(pg_mode_mask); create_data_directory(); @@ -3018,6 +3037,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 +3079,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 +3173,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 9dfb2ed96ff..8609d2ecbcc 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"; @@ -57,3 +59,19 @@ mkdir $datadir; } command_ok([ 'initdb', '-S', $datadir ], 'sync only'); command_fails([ 'initdb', $datadir ], 'existing data directory'); + +# Check group access on PGDATA +SKIP: +{ + skip "unix-style permissions not supported on Windows", 2 if ($windows_os); + + # 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), + 'check PGDATA permissions'); +} diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 32b41e313cc..58f780c0691 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,24 @@ 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. + * + * pg_mode_mask is set to owner-only by default and then updated in + * GetConnection() where we get the mode from the server-side with + * RetrieveDataDirCreatePerm() and then call SetDataDirectoryCreatePerm(). + */ + 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 b82e073b862..535355ebdad 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,16 @@ 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. + * + * pg_mode_mask is set to owner-only by default and then updated in + * GetConnection() where we get the mode from the server-side with + * RetrieveDataDirCreatePerm() and then call SetDataDirectoryCreatePerm(). + */ + 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 0866395944c..ef85c9af4c7 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,16 @@ 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. + * + * pg_mode_mask is set to owner-only by default and then updated in + * GetConnection() where we get the mode from the server-side with + * RetrieveDataDirCreatePerm() and then call SetDataDirectoryCreatePerm(). + */ + 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 1438f368edc..4fd536931b1 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 it to construct a umask + * for creating directories and files. + */ + if (!RetrieveDataDirCreatePerm(tmpconn)) + { + PQfinish(tmpconn); + exit(1); + } + return tmpconn; } @@ -327,6 +345,64 @@ RetrieveWalSegSize(PGconn *conn) return true; } +/* + * RetrieveDataDirCreatePerm + * + * This function is used to determine the privileges on the server's PG data + * directory and, based on that, set what the permissions will be for + * directories and files we create. + * + * PG11 added support for (optionally) group read/execute rights to be set on + * the data directory. Prior to PG11, only the owner was allowed to have rights + * on the data directory. + */ +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 ac5bb99f1b8..f502a2e3c71 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -6,7 +6,7 @@ use File::Basename qw(basename dirname); use File::Path qw(rmtree); use PostgresNode; use TestLib; -use Test::More tests => 105; +use Test::More tests => 106; program_help_ok('pg_basebackup'); program_version_ok('pg_basebackup'); @@ -44,10 +44,17 @@ $node->command_fails( ok(!-d "$tempdir/backup", 'backup directory was cleaned up'); +# Create a backup directory that is not empty so the next commnd will fail +# but leave the data directory behind +mkdir("$tempdir/backup") + or BAIL_OUT("unable to create $tempdir/backup"); +append_to_file("$tempdir/backup/dir-not-empty.txt"); + $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'); +rmtree("$tempdir/backup"); open my $conf, '>>', "$pgdata/postgresql.conf"; print $conf "max_replication_slots = 10\n"; @@ -200,11 +207,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") @@ -275,6 +288,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_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 5ede385e6ab..143021de05f 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_OWNER); /* 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 067a084c87c..2f9dfa7b81b 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 => 21; +use Test::More tests => 24; my $tempdir = TestLib::tempdir; my $tempdir_short = TestLib::tempdir_short; @@ -74,6 +76,27 @@ SKIP: ok(check_mode_recursive("$tempdir/data", 0700, 0600)); } +# Log file for group access test +$logFileName = "$tempdir/data/perm-test-640.log"; + +SKIP: +{ + skip "group access not supported on Windows", 3 if ($windows_os); + + system_or_bail 'pg_ctl', 'stop', '-D', "$tempdir/data"; + + # Change the data dir mode so log file will be created with group read + # privileges on the next start + 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" ], 'pg_ctl restart with server running'); diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index bdf71886ee2..8a0a805f1ed 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 7b632c7dcdb..63d9bd517df 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 72ab2f8d5eb..b9ea6a4c213 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 37cdd712f31..c364965d3ae 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,15 @@ template1 ), 'database names'); + # Permissions on PGDATA should have group permissions + SKIP: + { + skip "unix-style permissions not supported on Windows", 1 if ($windows_os); + + ok(check_mode_recursive($node_master->data_dir(), 0750, 0640), + 'check PGDATA permissions'); + } + RewindTest::clean_rewind_test(); } diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c index 1d351881435..cc8e8c94c5e 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_OWNER); 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 574639d47e5..24631a91c61 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/file_perm.c b/src/common/file_perm.c index fdfbb9a44c4..d640d6a1fdd 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_OWNER; -int pg_file_create_mode = PG_FILE_MODE_OWNER; +int pg_dir_create_mode = PG_DIR_MODE_OWNER; +int pg_file_create_mode = PG_FILE_MODE_OWNER; + +/* + * 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_OWNER; + +/* + * 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_OWNER; + pg_file_create_mode = PG_FILE_MODE_OWNER; + pg_mode_mask = PG_MODE_MASK_OWNER; + } +} + +#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 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 return false. The caller is + * responsible for generating an error, if appropriate, indicating that we + * were unable to access the data directory. + */ + 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 37631a7191d..3090f789317 100644 --- a/src/include/common/file_perm.h +++ b/src/include/common/file_perm.h @@ -21,14 +21,34 @@ */ #define PG_MODE_MASK_OWNER (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_OWNER 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_OWNER (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; +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 b5ad8419684..e167ee8fcbe 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; @@ -323,6 +324,7 @@ extern void SetSessionAuthorization(Oid userid, bool is_superuser); extern Oid GetCurrentRoleId(void); extern void SetCurrentRoleId(Oid roleid, bool is_superuser); +extern void checkDataDir(void); extern void SetDataDir(const char *dir); extern void ChangeToDataDir(void); diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 76e571b98c8..1bd91524d7e 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 93610e4bc40..8047404efd4 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.