Re: PATCH: Configurable file mode mask - Mailing list pgsql-hackers
From | Stephen Frost |
---|---|
Subject | Re: PATCH: Configurable file mode mask |
Date | |
Msg-id | 20180313234911.GF2416@tamriel.snowman.net Whole thread Raw |
In response to | Re: PATCH: Configurable file mode mask (Chapman Flack <chap@anastigmatix.net>) |
List | pgsql-hackers |
Chapman, * Chapman Flack (chap@anastigmatix.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
Attachment
pgsql-hackers by date: