Re: PATCH: Configurable file mode mask

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>
Cc: 'David Steele' <david(at)pgmasters(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>
Subject: Re: PATCH: Configurable file mode mask
Date: 2017-03-10 13:34:01
Message-ID: 20170310133400.GC9812@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Tsunakawa, Takayuki (tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com) wrote:
> From: pgsql-hackers-owner(at)postgresql(dot)org
> > [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of David Steele
> > PostgreSQL currently requires the file mode mask (umask) to be 0077.
> > However, this precludes the possibility of a user in the postgres group
> > performing a backup (or whatever). Now that
> > pg_start_backup()/pg_stop_backup() privileges can be delegated to an
> > unprivileged user, it makes sense to also allow a (relatively) unprivileged
> > user to perform the backup at the file system level as well.
>
> I'd like to help review this. First, let me give some questions and comments.

Great!

> 1.What's the concrete use case of this feature? Do you intend to extend the concept of multiple DBAs to the full range of administration of a single database instance, or just multiple OS users for database backup?

This is to allow a non-postgres user to perform a backup of the
database. Perhaps this could be leveraged for other administration
functions, but it's not clear how off-hand to me and the backup use-case
is the reason for adding this.

> If you think that multiple OS user support is desirable to reduce the administration burdon on a single person, then isn't the automated backup sufficient (such as with cron)?

I'm not quite sure what the question here is, but it is desirable to
minimize the amount of access any process requires to only that which is
required to perform its duties. In the case of backup, only read access
to the data directory and access to connect to PG and run certain
functions is required. The ability to run those functions as a
non-superuser was added in 9.6, this continues the work to minimize what
a backup user needs to perform a backup of the system by allowing a user
to have only read-only access to the data directory.

There are multiple reasons why matching the privileges a process has to
only that which is required is good practice. Minimizing impact to
ongoing operations from a compromise of the backup user and reducing the
risk that bugs in backup software could disrupt operations are two of
those.

> 2.Backup should always be considered with recovery. If you allow another OS user to back up the database, can you allow him to recover the database as well?

That would not be our recommended approach, but it would be possible to
do. Our recommended solution is for the backup user to only be able to
perform the backup. In this use-case, the restore would be run by the
OS user who owns the database (eg: postgres), who would have read-only
access to the backup repository.

To be clear, this is not hypothetical, pgBackrest supports these
configurations and has been tested with this approach. As noted, there
are a few additional items that need to be addressed which weren't
covered in the initial testing (on Debian-based systems, the pid file
and SSL key aren't in the data directory, so they didn't pose a problem,
but they should be addressed so that this can work on other
distributions).

> For example, assume the PostgreSQL user account (the OS user who does initdb and pg_ctl start/stop) is dba1, and dba2 backs up the database using tar or cpio.
> When dba2 restores the backup, the owner of the database cluster becomes dba2. If the file permission only allows one user to write the file, then dba1 can't start the instance.

If the uesr chose to configure the system with both dba1 and dba2 having
write access to the data directory, performing a restore as dba2 would
be possible and the backup utility could be sure to remove all existing
files and restore them from the backup, ensuring that all of the files
would be owned by a single user (dba2 in this case). Of course, one
would then need to adjust the startup process to run as dba2 instead.

With group write access to all of the files and directories, it may be
possible to actually run with the dba1 user even though the files are
owned by dba2, but we would not recommend it as the result would be a
mix of files owned by one dba or the other. This is not the use-case
this is being developed for.

> 3.The default location of the SSL key file is $PGDATA, so the permission of the key file is likely to become 0640. But the current postgres requires it to be 0600. See src/backend/libpq/be-secure-openssl.c.

Yes, that needs to be addressed. There was discussion on another thread
that it would be useful to support the SSL key file having group read
access, but since this patch is handling the other files it seems like
it would make sense to do that change here also.

> 4.I've seen a few users to place .pgpass file in $PGDATA and set the environment variable PGPASSFILE to point to it. They expect it to be back up with other database files. So I'm afraid the permission of .pgpass file also becomes 0640 some time. However, the current code requires it to be 0600. See src/interface/libpq/fe-connect.c.

This is not a configuration which we would recommend (generally
speaking, it's not really a good idea to drop random files into
$PGDATA). That said, there was discussion on another thread about
supporting this with an explicit client-side option to allow it. That
would be independent from this patch, I believe.

> 5.I think some explanation about the concept of multiple OS users is necessary, such as here:
>
> 16.1. Short Version
> https://www.postgresql.org/docs/devel/static/install-short.html
>
> 18.2. Creating a Database Cluster
> https://www.postgresql.org/docs/devel/static/creating-cluster.html

I agree that we should update the documention for this, including those.

> [FYI]
> Oracle instructs the user, during the software installation, to put "umask 022" in ~/.bashrc or so.
> MySQL's files in the data directory appears to be 0640.

When it comes to MySQL, at least, this may be distribution dependent.
In any case, it's good to see that we are not the only ones doing this.

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2017-03-10 13:51:25 Re: WIP: Faster Expression Processing v4
Previous Message Jim Nasby 2017-03-10 13:14:58 Re: Report the number of skipped frozen pages by manual VACUUM