Re: [HACKERS] pg_ls_dir & friends still have a hard-coded superusercheck

Andres,

* Andres Freund (andres@anarazel.de) wrote:
> On 2017-01-25 16:52:38 -0500, Stephen Frost wrote:
> > * Robert Haas (robertmhaas@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