Re: ExecutorCheckPerms() hook

(2010/07/22 8:45), Robert Haas wrote:
> 2010/5/24 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> (2010/05/24 22:18), Robert Haas wrote:
>>> 2010/5/24 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>>> BTW, I guess the reason why permissions on attributes are not checked here is
>>>> that we missed it at v8.4 development.
>>>
>>> That's a little worrying. Can you construct and post a test case
>>> where this results in a user-visible failure in CVS HEAD?
>>
>> Sorry, after more detailed consideration, it seems to me the permission
>> checks in RI_Initial_Check() and its fallback mechanism are nonsense.
>>
>> See the following commands.
>>
>> postgres=# CREATE USER ymj;
>> CREATE ROLE
>> postgres=# CREATE TABLE pk_tbl (a int primary key, b text);
>> NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "pk_tbl_pkey" for table "pk_tbl"
>> CREATE TABLE
>> postgres=# CREATE TABLE fk_tbl (x int, y text);
>> CREATE TABLE
>> postgres=# ALTER TABLE pk_tbl OWNER TO ymj;
>> ALTER TABLE
>> postgres=# ALTER TABLE fk_tbl OWNER TO ymj;
>> ALTER TABLE
>> postgres=# REVOKE ALL ON pk_tbl, fk_tbl FROM ymj;
>> REVOKE
>> postgres=# GRANT REFERENCES ON pk_tbl, fk_tbl TO ymj;
>> GRANT
>>
>> At that time, the 'ymj' has ownership and REFERENCES permissions on
>> both of pk_tbl and fk_tbl. In this case, RI_Initial_Check() shall return
>> and the fallback-seqscan will run. But,
>>
>> postgres=> ALTER TABLE fk_tbl ADD FOREIGN KEY (x) REFERENCES pk_tbl (a);
>> ERROR: permission denied for relation pk_tbl
>> CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."pk_tbl" x WHERE "a" OPERATOR(pg_catalog.=) $1 FOR SHARE OF x"
>>
>> From more careful observation of the code, the validateForeignKeyConstraint()
>> also calls RI_FKey_check_ins() for each scanned tuples, but it also execute
>> SELECT statement using SPI_*() interface internally.
>>
>> In other words, both of execution paths entirely require SELECT permission
>> to validate new FK constraint.
>
> I think the relevant case might be where ymj owns fk_tbl but not
> pk_tbl, and has REFERENCES but not SELECT on pk_tbl.
>
> Come to think of it, I wonder if REFERENCES on fk_tbl ought to be
> sufficient to create a foreign key. Currently, it requires ownership:
>
> rhaas=> ALTER TABLE fk_tbl ADD FOREIGN KEY (x) REFERENCES pk_tbl (a);
> ERROR: must be owner of relation fk_tbl
>
+1.

We can find out a similar case in CreateTrigger().
If I was granted TRIGGER privilege on a certain table, I can create a new
trigger on the table without its ownership. More commonly, it allows us
to modify a certain property of the table without its ownership.

Perhaps, if SQL permission would be more fine-grained, for example,
"RENAME" permission might control RENAME TO statement, rather than
its ownership.

What is the reason why we check its ownership here, although we already
have REFERENCES permission to control ADD FOREIGN KEY?

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>