Re: ExecutorCheckPerms() hook

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

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