Re: Monitoring roles patch

> On Mar 28, 2017, at 11:06 AM, Mark Dilger <hornschnorter(at)gmail(dot)com> wrote:
>
>
>> On Mar 28, 2017, at 9:47 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Does
>>> pg_read_all_stats still have access to stats for mysecuretable?
>>
>> Yes, because the ACL on the table controls reading/writing the data in
>> the table. It doesn't have any bearing on any kind of table metadata.
>> A user who has no privileges on a table can already look at (for
>> example) pg_stat_all_tables and see the sort of info you're talking
>> about. This patch would just allow members of a specific role get the
>> on-disk size as well, *if* you choose to use it.
>
> I don't entirely agree here. Security conscious database users may well
> restrict access to that view. I added a check to the regression tests to
> verify that it works:
>
> + SET ROLE regress_locktable_user;
> + SELECT COUNT(*) FROM pg_stat_all_tables;
> + count
> + -------
> + 290
> + (1 row)
> +
> + RESET ROLE;
> + REVOKE ALL PRIVILEGES ON pg_stat_all_tables FROM PUBLIC;
> + SET ROLE regress_locktable_user;
> + SELECT COUNT(*) FROM pg_stat_all_tables;
> + ERROR: permission denied for relation pg_stat_all_tables
> + RESET ROLE;
> + SELECT COUNT(*) FROM pg_stat_all_tables;
> + count
> + -------
> + 292
> + (1 row)
> +
>
> The inability to revoke access to this sort of information being proposed
> makes me a bit uneasy. Mostly, I think, I'm bothered because there may
> be people who have revoked privileges on a lot of things, thereby restricting
> access to superuser, who won't necessarily notice this new feature in
> postgres 10. That could be a source of security holes that we get blamed
> for.

After a bit of introspection, I think what is really bothering me is not the
inability to revoke permissions, since as you say I can choose to not assign
the role to anybody. What bothers me is that this feature implicitly redefines
what is meant by the keyword PUBLIC. If I go around the database
revoking privileges on everything from PUBLIC, I should end up with
a database that is inaccessible to anyone but superuser, right? All views,
functions, tables, etc., would all be locked down. But after this proposed
change, IIUC, there would still be a bit of access available to this/these
proposed non-superuser role(s) which have hardcoded permissions.

That's quite a significant change to the security model of the database,
and I don't think it is something users would expect from the release notes
if the release notes for this feature say something like:

"added database monitoring functionality"

To be fair, I have not tried to revoke everything from everybody on a
database to verify that their aren't already problems of this sort. Perhaps
the cows have already left the barn.

mark