Re: has_table_privilege for a table in unprivileged schema causes an error

"David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
> On Sat, Aug 25, 2018 at 8:29 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> There's a backwards-compatibility argument for not changing this behavior,
>> sure, but I don't buy the other arguments you made here. And I don't
>> think there's all that much to the backwards-compatibility argument,
>> considering that the current behavior is to fail.

> +1; any code using these functions can reasonably be expected to handle
> true and false responses. Converting a present error into a false under
> that presumption results in similar, and arguably improved, semantics.

I took a closer look at what's going on here, and realized that the
existing code is a bit confused, or confusing, about whose privileges
it's testing. Consider this exchange (with HEAD, not the patch):

regression=# CREATE SCHEMA testns;
CREATE SCHEMA
regression=# CREATE TABLE testns.t1 (f1 int);
CREATE TABLE
regression=# CREATE USER regress_priv_user1;
CREATE ROLE
regression=# SELECT has_table_privilege('regress_priv_user1', 'testns.t1', 'SELECT');
has_table_privilege
---------------------
f
(1 row)

regression=# \c - regress_priv_user1
You are now connected to database "regression" as user "regress_priv_user1".
regression=> SELECT has_table_privilege('regress_priv_user1', 'testns.t1', 'SELECT');
ERROR: permission denied for schema testns

That is, whether you get an error or not depends on your *own*
privileges for the schema, not those of the user you're supposedly
testing. This seems rather inconsistent. If we apply the proposed
patch, then (I'd expect, but haven't tested) you should always get
the same results from has_table_privilege('u', 's.t', 'p') as from
doing has_table_privilege('s.t', 'p') as user u.

However ... if we do that, then Robert's previous concern about
information leakage comes to life, because an unprivileged user
could run has_table_privilege('postgres', 'testns.t1', 'SELECT')
and thereby find out whether t1 exists regardless of whether he
has any privilege for testns.

Mind you, I think that argument is mostly bunk, because that
same user can trivially find out whether t1 exists, and what
its privilege grants are, just by looking into pg_catalog.
So there's no real security improvement from disallowing this.

Anyway, it seems to me that the principle of least astonishment
suggests that the results of has_table_privilege should depend only
on the privileges of the user allegedly being tested, not those of
the calling user. Or if we decide differently, somebody has to write
some doc text explaining that it's not so.

Getting all the way to that might be a bit difficult though.
For example, in

SELECT has_function_privilege('someuser', 'func(schema.type)', 'usage');

the lookup and permission check for "schema" are buried a long
way down from the has_function_privilege code. It'd take a
lot of refactoring to keep it from throwing an error. I guess
maybe you could argue that privileges on the type are a different
question from privileges on the function, but it still seems ugly.

A related thought is that it's not clear whether there should be
any behavioral difference between

SELECT has_function_privilege('someuser', 'func(schema.type)'::text, 'usage');

SELECT has_function_privilege('someuser', 'func(schema.type)'::regprocedure, 'usage');

In the latter case, it's entirely unsurprising that the schema lookup
is done as the current user. However, if we define the first case
as being equivalent to the second, then the error that Yugo-san is
complaining of is something we can't/shouldn't fix, because certainly
"SELECT 'testns.t1'::regclass" is going to throw an error if you lack
usage privilege for testns.

So at this point I'm not sure what to think, other than that things
are inconsistent (and underdocumented).

regards, tom lane