On Wed, Jan 25, 2017 at 3:17 PM, Stephen Frost <sfr...@snowman.net> wrote: > Your email is 'pg_ls_dir & friends', which I took to imply at *least* > pg_read_file() and pg_read_binary_file(), and it's not unreasonable to > think you may have meant everything in adminpack, which also includes > pg_file_write(), pg_file_rename() and pg_file_unlink().
Well, I was talking about genfile.c, which doesn't contain those functions, but for the record I'm in favor of extirpating every single hard-coded superuser check from the backend without exception. Preventing people from calling functions by denying the ability to meaningfully GRANT EXECUTE on those functions doesn't actually stop them from delegating those functions to non-superusers. It either (a) causes them to give superuser privileges to accounts that otherwise would have had lesser privileges or (b) forces them to use wrapper functions to grant access rather than doing it directly or (c) some other dirty hack. If they pick (a), security is worse; if they pick (b) or (c), you haven't prevented them from doing what they wanted to do anyway. You've just made it annoying and inconvenient. Both EnterpriseDB's PEM and check_postgres.pl currently have a bunch of things that don't work unless you run as superuser. I think we should be trying to provide ways of reducing those lists. If I can't get agreement on method (a), I'm going to go look for ways of doing (b) or (c), which is more work and uglier but if I can't get consensus here then oh well. >> I don't really think it's necessary to outline the use case more than >> I have already. It's perfectly reasonable to want a monitoring tool >> to have access to pg_ls_dir() - for example, you could use that to >> monitor for relation files orphaned by a previous crash. > > Sure. What I believe would be better would be a function in PG that > allows you to know about all of the relation files which were orphaned > from a previous crash, and perhaps even one to go clean all of those up. > I certainly would have no issue with both of those being available to a > non-superuser. > > Sure, you could do the same with pg_ls_dir(), various complex bits of > SQL to figure out what's been orphaned, and pg_file_unlink() to handle > the nasty part of unlinking the files, but you could also use > pg_ls_dir() to look at the files in PG's home directory, or > pg_file_unlink() to remove the PG user's .bashrc. > > Does the monitoring tool need to be able to see the files in PG's root > directory, or to be able to unlink the PG user's .bashrc? No. I do not accept your authority to determine what monitoring tools need to or should do. Monitoring tools that use pg_ls_dir are facts on the ground, and your disapprobation doesn't change that at all. It just obstructs moving to a saner permissions framework. I agree that pg_file_unlink() is an unlikely need for a monitoring tool, but (1) conflating pg_ls_dir with pg_file_unlink is a bit unfair and (2) there's no actual point whatsoever in trying to restrict to whom the superuser can give permissions, because the superuser has numerous ways of working around any such restriction. > I don't think it's nannyism to keep things like pg_read_file and > pg_file_write as superuser-only. I entirely disagree. It is for the DBA to decide to whom and for what purpose he wishes to give permissions. And if somebody writes a tool -- monitoring or otherwise -- that needs those permissions, the DBA should be empowered to give those permissions and no more, and should be empowered to do that in a nice, clean way without a lot of hassle. The idea that you or I would try to decide which functions a user is allowed to want to grant access to and which ones they are not allowed to want to grant access to seems laughable to me. We're not smart enough to foresee every possible use case, and we shouldn't try. Even for a function that theoretically allows a privilege escalation to superuser (like pg_write_file), it's got to be better to give a user account permission to use that one function than to just give up and give that account superuser privileges, because escalating privileges with that blunt instrument would take more work than a casual hacker would be willing to put in. For a function like the one that this thread actually started out being about, like pg_ls_dir, it's vastly better. You mention that you're not sure that pg_ls_dir() would allow a privilege escalation to superuser, but I think we both know that there is probably no such escalation. Why you'd rather have people running check_postgres.pl's wal_files check under an actual superuser account rather than an account that only has rights to do pg_ls_dir is beyond me. Your response will no doubt be that there should otherwise be some other way to write that check that doesn't use pg_ls_dir. Maybe. But the thing is that pg_ls_dir is a general tool. You can use it to do a lot of things, and you can write a monitoring probe that does those things without having to wait for a new release of PostgreSQL. So your argument boils down to saying that when somebody wants to monitor something that could be checked using pg_ls_dir(), they shouldn't do write a query using pg_ls_dir(). Instead, they should submit a new patch to core that adds a special-purpose function implementing the exact functionality they need, get consensus on it, get somebody to commit it, wait for a new PostgreSQL release to come out, wait for the users who might want the probe to upgrade to that new release, and THEN they can do the monitoring they want to do. Now, I say that's ridiculous. What's actually going to happen is that the tool author is going to say "this probe requires superuser" and move on. The fact that check_postgres.pl has done exactly that for about 5 different probes -- not all because of pg_ls_dir, but one of them is for that reason -- suggests that this will in fact be a typical reaction whenever a fine-grained privilege is not available to be granted. We can decide to alleviate that pain or we can decide against it, but telling people "don't do that" is not going to work. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers