Andres, * Andres Freund (and...@anarazel.de) wrote: > On 2017-01-25 16:52:38 -0500, Stephen Frost wrote: > > * Robert Haas (robertmh...@gmail.com) wrote: > > > 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. > > > > The notion that security is 'worse' under (a) is flawed- it's no > > different. > > Huh? Obviously that's nonesense, given the pg_ls_dir example.
Robert's made it clear that he'd like to have a blanket rule that we don't have superuser checks in these code paths if they can be GRANT'd at the database level, which goes beyond pg_ls_dir. If the question was only about pg_ls_dir, then I still wouldn't be for it, because, as the bits you didn't quote discussed, it encourages users and 3rd party tool authors to base more things off of pg_ls_dir to look into the way PG stores data on disk, and affords more access than the monitoring user has any need for, none of which are good, imv. It also discourages people from implementing proper solutions which you can 'just use pg_ls_dir()', which I also don't agree with. If you really want to do an ls, go talk to the OS. ACLs are possible to provide that with more granularity than what would be available through pg_ls_dir(). We aren't in the "give a user the ability to do an ls" business, frankly. > > With regard to 'b', if their wrapper function is > > sufficiently careful to ensure that the caller isn't able to do anything > > which would increase the caller's level to that of superuser, then > > security is improved. > > Given how complex "sufficiently careful" is for security definer UDFs, > in comparison to estimating the security of granting to a function like > pg_ls_dir (or pretty much any other that doesn't call out to SQL level > stuff like operators, output functions, etc), I don't understand this. If you're implying that security definer UDFs are hard to write and get correct, then I agree with you there. I was affording the benefit of the doubt to that proposed approach. > > If the wrapper simply turns around can calls the underlying function, > > then it's no different from '(a)'. > > Except for stuff like search path. If you consider '(a)' to be the same as superuser, which I was postulating, then this doesn't strike me as making terribly much difference. > > I am suggesting that we shouldn't make it look like there are > > distinctions when there is actually no difference. That is a good thing > > for our users because it keeps them informed about what they're actually > > doing when they grant access. > > This position doesn't make a lick of sense to me. There's simply no > benefit at all in requiring to create wrapper functions, over allowing > to grant to non-superuser. Both is possible, secdef is a lot harder to > get right. And you already heavily started down the path of removing > superuser() type checks - you're just arguing to make it more or less > randomly less consistent. I find this bizarre considering I went through a detailed effort to go look at every superuser check in the system and discussed, on this list, the reasoning behind each and every one of them. I do generally consider arbitrary access to syscalls via the database to be a privilege which really only the superuser should have. > > I've commented on here and spoken numerous times about exactly that goal > > of reducing the checks in check_postgres.pl which require superuser. > > You're not actually doing that and nothing you've outlined in here so > > far makes me believe you see how having pg_write_file() access is > > equivilant to giving someone superuser, and that concerns me. > > That's the users responsibility, and Robert didnt' really suggest > granting pg_write_file() permissions, so this seems to be a straw man. He was not arguing for only pg_ls_dir(), but for checks in all "friends", which he later clarified to include pg_write_file(). Thanks! Stephen
Description: Digital signature