Re: [HACKERS] pg_ls_dir & friends still have a hard-coded superusercheck - Mailing list pgsql-hackers

Robert,

* Robert Haas (robertmhaas@gmail.com) wrote:
> OK, fair enough.  get_raw_page() is clearly not something that we
> really want everybody to have access to by default, but if it were up
> to me, I'd change the permissions check inside the function to do a
> check for select privileges on the relation, and then I'd change the
> SQL script to revoke access from everybody but the superuser.

The way to do properly do this would not be to have some conflation of
the right to execute get_raw_page() combined with a typically granted
right like 'SELECT'.  Instead, it would be to have an explicit RAW_PAGE
or similar permission which could be GRANT'd to a user for a particular
relation, and then we could change the superuser() check into a check
against that right, and call it done.

The difficulty with the approach you're advocating is that a great many
users will likely have SELECT rights on a great many relations, but that
doesn't mean I really want them to have RAW_PAGE rights on all of them.

Of course, what this is really missing is an actual use-case where it
makes sense to grant out get_raw_page() to anyone.  Without that, I've
got a really hard time agreeing that it makes sense to remove the
existing superuser check or to spend any effort here.

> Actually, I think that's Stephen should have done something similar
> for pgstattuple in commit fd321a1dfd64d30bf1652ea6b39b654304f68ae4,
> and I think we should go back and fix it that way before that code
> lands in a released branch.  As I mentioned upthread, a user could run
> that even on sensitive tables like pg_authid.  That doesn't directly
> reveal any data, but it does let you take a lock on a table which you
> otherwise couldn't lock, and it reveals at least some information
> about the contents of the table.  If you ran it just before and after
> an insert or update, you might be able to infer the length of the new
> tuple.  That's probably not enough to give you a whole lotta help
> guessing what password that tuple contains ... but it's more than no
> information.  More fundamentally, I think it's entirely reasonable for
> the superuser to be able to decide who can and can't run the
> pgstattuple functions, but I strongly suspect that very few superusers
> would want to give users rights to run those functions on tables to
> which those users otherwise have no access.

Requiring pgstattuple() to check if a user has any rights on a table
means that an existing non-superuser monitoring tool that's calling
pgstattuple() for summary information would be required to have SELECT
rights across, most likely, all tables, even though the monitoring
user's got no need for that level of access.  Now, we could argue that
having access to just one column would be enough for that user, but
that's still more access than the monitoring user needs, and there's
still the issue of new tables (and default privileges don't currently
support column-level privileges anyway).

A new capability or way to grant "lock access" to all tables might be a
way to approach this, but it would have to be controllable on a
per-lock-mode basis, or a way to say "this user is allowed to lock and
extract high-level counts from every table in the system", though the
latter is essentially what GRANT'ing EXECUTE on the pgstattuple()
functions today is.  Perhaps the docs should be clearer as to what that
access means, but I don't see a need to include a warning with those
functions saying that they could be used to escalate privileges to a
superuser.

Perhaps a more generalized approach would be to structure a generic
function+table GRANT system, in addition to our existing
GRANT-RIGHT+table system, that allowed for per-function-per-table-level
grants.  My thinking is something along the lines of:

GRANT pgstattuple() ON TABLE q;
GRANT pgstattuple() ON ALL TABLES IN SCHEMA x;
GRANT pgstattuple() ON ALL TABLES IN DATABASE db;

Which would specifically tie together the two.  Of course, we'd need
similar support in ALTER DEFAULT PRIVILEGES to allow the capability to
be maintained.

What is unfortunate with any of this is that we don't, currently, have
any way to specify DEFAULT PRIVILEGES on pg_catalog, meaning that new
tables which are added to pg_catalog down the road won't get these
privileges.  For a monitoring tool that's trying to check $something
(bloat, for example) across all tables, that's a bit unfortunate.
Generally speaking, there's only a few catalog tables that tend to have
bloat issues and those are well-defined and don't change, so perhaps
this issue could be ignored, but a more complete solution would find a
way to address that case also.

Thanks!

Stephen