From: | Stephen Frost <sfrost(at)snowman(dot)net> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
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:32:01 |
Message-ID: | 20170127173201.GM9812@tamriel.snowman.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Robert,
* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> 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().
You're completely ignoring the use-cases for which these are being done.
I've outlined the precise use-case for pgstattuple()'s usage across the
entire database for which the admin has granted the EXECUTE access in.
I've not yet seen a use-case for access to pg_ls_dir() across all
directories pg_ls_dir() can access. My recollection is that you brought
up pg_wal, but that's hardly every directory, and some hypothetical tool
which could intelligently figure out what orphaned files exist. For the
former, I would recommend a function for exactly that (or perhaps
something better which provides more than just a count of files), for
the latter, that's something that I would be very worried that someone
trying to implement outside of core would get wrong or which could be
version-dependent. We've already got some code in core to find files
left over from a crash and deal with them and perhaps that could be
expanded to deal with other cases.
> 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.
With an appropriate use-case for it, I wouldn't be against it. I linked
to both use-cases and a provided patch for finer-grained access to
pg_ls_dir() and friends three years ago, which got shot down. I'm not
against it if the community wishes to reconsider that decision and
support having filesystem-like access where there's an appropriate
use-case for it, and with fine-grained access control provided to meet
that use-case.
Further, as it relates to goal-posts, if your goal is to let an admin
grant out the ability to see how many files are in pg_wal, you're
spending a great deal more effort than that would require. I wouldn't
object (and would actually appreciate) a function which is able to do
exactly that, and I'd go work with Greg S-M right away to make sure that
check_postgres.pl knows about that function and uses it in PG10 and
above.
I do object to someone coming along and saying "let's rip out all the
superuser() checks" and it would seem that I'm not alone.
Thanks!
Stephen
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2017-01-27 17:39:38 | Re: pg_ls_dir & friends still have a hard-coded superuser check |
Previous Message | Dmitry Dolgov | 2017-01-27 17:31:41 | Re: [PATCH] Generic type subscription |