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

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_ls_dir & friends still have a hard-coded superuser check
Date: 2017-01-27 17:11:38
Message-ID: CA+TgmobY81nz5feg0tg9Quenad15zKT1z5JAUaLpsUnxqUNyNg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 27, 2017 at 11:34 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Robert Haas (robertmhaas(at)gmail(dot)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.

That's moving the goalposts a very large distance.

>> Actually, I think that's Stephen should have done something similar
>> for pgstattuple in commit fd321a1dfd64d30bf1652ea6b39b654304f68ae4, ...
>
> 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).

...whereas here you don't want to move the goalposts at all. I can't
understand this. When I want the superuser to have the option to hand
out pg_ls_dir() access for all directories pg_ls_dir() can access, you
complain that's too broad. Yet your own previous commit, which allows
pgstattuple() access to be granted, makes no provision for any
granularity of access control at all. And I think there is an
excellent argument - which I have made - that pgstattuple() is more
likely to expose sensitive information than pg_ls_dir().

Even if we someday have the capability for people to grant pg_ls_dir()
access on a directory-by-directory basis, I am sure there will still
be a way for them to grant access on all the directories to which
pg_ls_dir() can access today; after all, that's just two directories,
and their subdirectories. At most somebody would have to make two
GRANTs. Removing the hard-coded superuser() checks allows that use
case now, and doesn't prohibit you from implementing the other thing
later when and if and when we reach agreement on it.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-01-27 17:15:40 Re: Performance improvement for joins where outer side is unique
Previous Message Robert Haas 2017-01-27 17:00:49 Re: pageinspect: Hash index support