Re: PATCH: Configurable file mode mask

Chapman,

* Chapman Flack (chap(at)anastigmatix(dot)net) wrote:
> I'm not advocating the Sisyphean task of having PG incorporate
> knowledge of all those possibilities. I'm advocating the conservative
> approach: have PG be that well-behaved application that those extended
> semantics are generally all designed to play well with, and just not
> do stuff that obstructs or tramples over what the admin takes care
> to set up.

I think we get that you're advocating removing the checks and
permissions-setting that the PG tools do, however...

> I wonder how complicated it would have to be really. On any system
> with a POSIX base, I guess it's possible for PGDATA to have an S_ISVTX
> "sticky" bit in the mode, which does have an official significance (but
> one that only affects whether non-postgres can rename or unlink things
> in the directory, which might be of little practical significance).
> Perhaps its meaning could be overloaded with "the admin is handling
> the permissions, thank you", and postmaster and various command-line
> utilities could see it, and refrain from any gratuitous chmods or
> refusals to function.
>
> Or, if overloading S_ISVTX seems in poor taste, what would be wrong
> with simply checking for an empty file PERMISSIONS-ARE-MANAGED in
> PGDATA and responding the same way?
>
> Or, assuming some form of ACL is available, just let the admin
> change the owner and group of PGDATA to other than postgres,
> grant no access to other, and give rwx to postgres in the ACL?

None of these suggestions really sound workable to me. I certainly
don't think we should be overloading the meaning of a specific and
entirely independent filesystem permission (and really don't even
want to imagine what it would require to do something like that on
Windows...), dropping an empty file in the directory strikes me as a
ripe target for confusion, and we have specific checks to try and
make sure we are running as the owner that owns the directory with
some pretty good reasons around that (avoiding multiple postmasters
running in the same PGDATA directory, specifically).

> PG could then reason as follows: * I do not own this directory.
> * I am not the group of this directory. * It grants no access to other.
> * Yet, I find myself listing and accessing files in it without
> difficulty. * The admin has set this up for me in a way I do not
> understand. * I will refrain from messing with it.

Removing the check that says "we aren't going to try to run PG in this
directory if we aren't the owner of it" also doesn't seem like it's
necessairly a great plan.

> Three ideas off the top of my head. Probably more where they came from.

None of them really seem workable though. On the other hand, let's
consider what this patch actually ends up doing when POSIX ACLs are
involved. This allows ACLs to be used where they weren't before and
with appropriate defaults set even to work for new files being created,
which wouldn't work before. Yes, if you create a default ACL which
says "grant this other user write access to files in the PG data
directory" then that won't actually be honored because we will
chmod(640) the file and the POSIX ACL system actually works with the
user/group privilege system, like so:

Create the "data" dir, as initdb would:
➜ ~ mkdir xyz
➜ ~ chmod 750 xyz
➜ ~ ls -ld xyz
drwxr-x--- 2 sfrost sfrost 4096 Mar 13 19:07 xyz

Set a default ACL to allow the "daemon" user read/write access to files
created:

➜ ~ setfacl -dm u:daemon:rw xyz
➜ ~ getfacl xyz
# file: xyz
# owner: sfrost
# group: sfrost
user::rwx
group::r-x
other::---
default:user::rwx
default:user:daemon:rw-
default:group::r-x
default:mask::rwx
default:other::---

Create a file the way PG would:
➜ ~ touch xyz/a
➜ ~ chmod 640 xyz/a
➜ ~ getfacl xyz/a
# file: xyz/a
# owner: sfrost
# group: sfrost
user::rw-
user:daemon:rw- #effective:r--
group::r-x #effective:r--
mask::r--
other::---

The daemon user ends up with read-only access (note the '#effective',
which shows that the POSIX ACL system isn't overriding the "regular"
ACLs).

... but that's basically what we want.

Multiple users having write access to the data directory could be quite
bad as you might possibly get two postmasters running against the same
data directory at the same time and there's basically no case where
that's a good thing to have happen. This change does let users grant
out read access to other users/groups, even beyond what's possible using
the traditional user/group system, so this opens up a lot more possible
options for advanced users, provided they set the defaults
appropriately at the directory level (which, presumably, an
administrator versed in POSIX ACLs and wishing to use them would know,
or would figure out quickly).

Yes, perhaps there's some argument to be made that we should have an
option where we don't force any privileges, but that can certainly be
considered a future capability and what's being implemented here doesn't
actually break use-cases which worked before and allows users more
freedom than they had before to use POSIX ACLs if they want to use them,
and without having to modify PG to understand POSIX ACLs explicitly.

Also, to the point you raise up-thread, where you remove access to the
data directory from the group that owns the directory, but grant it to
some other user, yes, files inside the data directory would end up with
group access for that other group. That won't matter as long as the
group doesn't have rights on the directory, though it would certainly be
cleaner to just have a dedicated group where it isn't an issue for that
group to have read access to the files. I would imagine anyone using
POSIX ACLs would understand this without too much difficulty. Of
course, POSIX default ACLs would also continue to work for files created
under the data directory, as illustrated above.

In all, this is looking like a pretty good improvement while also having
some caution about what privileges are effectively allowed.

Thanks!

Stephen