Re: pg_ls_dir & friends still have a hard-coded superuser check

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> > [ good general plan ]
>
> > 3. Make a list of all functions that would cause security problems.
> > One by one, precisely. If we did remove all superuser checks we would
> > need this list documented to advise people of the risks, so it must
> > exist before any commit can be made, assuming we believe in
> > documentation. Notice that I am after risk documentation,
>
> Yeah, I think documentation is the crux of the issue. If there is some
> non-obvious reason why letting somebody use pg_ls_dir() is more of a
> security hazard than it appears on the surface, the answer is to document
> that so DBAs can decide for themselves whether to take the risk.
>
> Count me +1 for removing hard-wired superuser checks, but carefully
> and with an overall plan.

To be clear, I'm also in agreement with (and have been making efforts
to, for quite a few years, with some progress) removing the hard-wired
superuser checks. I've tried to do so carefully and with a guiding
principle that the existing superuser-only capabilities should be split
up in a way which makes sense for particular use-cases and allows the
administrator to grant only precisely what they wish to for the use-case
in question. The individual-level grants for things like
pg_start/stop_backup(), along with some others, were really already at
the "right" level for the use-case in question, which meant that what
was needed was a way to revoke EXECUTE from public initially and to
maintain what the administrator wished the rights to be, which is what
was done.

I do like the idea which Dave mentioned up-thread of having default
roles which start out with the necessary privileges, so that the user
doesn't have to grant EXECUTE on pg_start_backup(), and then also grant
EXECUTE on pg_stop_backup(). We've gotten to the point where that might
actually be possible, now that we have a couple of existing default
roles, and a namespace in which to create more, if we choose to.

That a monitoring tool needs to be able to list the contents of the
pg_wal PG directory is great, but if that's what it needs, then that's
what the admin should be able to provide it with, and not have to
provide it with the ability to list every directory on the system, or
even every other directory in PG's data directory. Privileges should be
granted out, as much as possible, to match what the requirement is, and
tossing everything out the window because we happen to have an existing
function like pg_ls_dir() that just needs that pesky superuser() check
removed isn't a good answer to the use-case in question. I'm all for
generalizing things and providing blanket access when that's what the
use-case calls for, to be clear, but that's not where this discussion
has (had?) been going.

Thanks!

Stephen