On Wed, Jan 25, 2017 at 10:31 PM, Stephen Frost <sfr...@snowman.net> wrote: > Frankly, I get quite tired of the argument essentially being made here > that because pg_ls_dir() wouldn't grant someone superuser rights, that > we should remove superuser checks from everything. As long as you are > presenting it like that, I'm going to be quite dead-set against any of > it.
I'm not presenting it like that, and I think I've been quite clear about that. I'm making two separate arguments which you keep conflating. The first is that restricting the ability to GRANT access to a function, even a function that allows escalation to superuser privileges, doesn't improve security, because the superuser can still grant those privileges with more work. That argument favors removing all superuser checks as pointless. The second is that if we were to accept as official policy the idea that functions should have built-in superuser() checks when and only when they allow escalation to superuser, then those checks should be removed from those functions which do not allow such an escalation. That argument favors removing at least some of the superuser checks as inconsistent with policy. If there's a policy -- even one that I don't like -- it should be enforced consistently. I audited the core backend for hard-coded superuser() checks that made it categorically impossible to call a certain SQL function. I looked only for functions where the check was precisely if (!superuser()) ereport(ERROR, ...), so things that were contingent on rolreplication or anything other than superuser-ness are not included in this analysis. I found a total of five such functions, of which I can think of possible escalation-to-superuser risk for two. I did not examine contrib although that might be worth doing at some point. 1. pg_ls_dir. I cannot see how this can ever be used to get superuser privileges. 2. pg_stat_file. I cannot see how this can ever be used to get superuser privileges. 3. pg_read_binary_file. This could potentially let you get superuser or other privileges. Most obviously, if you read and parse the contents of pg_authid, you might be able to find passwords and then break into things. There might be other methods as well. 4. pg_read_file. Essentially the same risks, although reading data files will be a little harder with this function because any NUL byte is going to make the read error out. But given time and determination you can probably still ascertain the contents of every byte in the database, so the risks are about the same. 5. pg_import_system_collations. There doesn't seem to be any obvious way to use this to get superuser privileges, but there might be some subtle risk I'm missing. Attached is a patch that removes the hard-coded superuser checks from all five of these functions, based on the first argument above. Currently, I count three votes in favor of this approach and one opposed. If anyone else wants to weigh in, please do. It would be helpful if anyone weighing in can be clear about whether (a) they are in favor of the patch as proposed, or (b) they are not in favor of the patch as proposed but could support a narrower patch that removed the checks only from functions with no known escalate-to-superuser risks, or (c) they oppose all change. It would also be helpful if the reasons why each person takes the position that they do could be mentioned. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
remove-hardcoded-superuser-checks.patch
Description: invalid/octet-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
