Re: [v9.2] Object access hooks with arguments support (v1) - Mailing list pgsql-hackers

From Kohei KaiGai
Subject Re: [v9.2] Object access hooks with arguments support (v1)
Date
Msg-id CADyhKSVo_cn+4iv16O=1WUi-PW=66U8qshv0SFwoQOERJ-ELSA@mail.gmail.com
Whole thread Raw
In response to Re: [v9.2] Object access hooks with arguments support (v1)  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: [v9.2] Object access hooks with arguments support (v1)  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
2011/10/18 Robert Haas <robertmhaas@gmail.com>:
> On Tue, Oct 18, 2011 at 11:25 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
>> For example, I hope sepgsql to perform as follows when user create a new table.
>> - It computes a default security label that needs Oid of the namespace.
>> - It checks db_table:{create} permission on the security label being computed.
>> - In addition, it checks db_table:{insert} permission when SELECT INTO
>> - Also, it checks these permissions on columns being newly created.
>> - However, It does not check permissions when the tables are internally created,
>>  such as toast tables or temporary relations created by make_new_heap().
>
> That's superficially reasonable, but I don't feel good about passing
> is_select_info to the object access hook here.  If insert permission
> is required there, then it ought to be getting checked by selinux at
> the same place that it's getting checked for at the DAC level.  If we
> get into a situation where sepgsql is checking permissions using some
> system that's totally different from what we do for DAC, I think
> that's going to produce confusing and illogical results.  This is
> ground we've been over before.  Similarly with fdw_srvname.  The only
> excuse for passing that is to handle a permissions check for foreign
> data wrappers that isn't being done for DAC: if there is a DAC
> permissions check, then the MAC one should be done there also, not
> someplace totally different.
>
If you are suggesting DAC and MAC permissions should be checked
on the same place like as we already doing at ExecCheckRTPerms(),
I'd like to agree with the suggestion, rather than all the checks within
object_access_hook, although it will take code reworking around
access controls.

In the example table creation, heap_create_with_catalog() is invoked
by 5 routines, however, 3 of them are just internal usages, so it is not
preferable to apply permission checks on table creation....

If we may have a hook on table creation next to the place currently
we checks permission on the namespace being created, it is quite
helpful to implement sepgsql.


BTW, it seems to me SELECT INTO should also check insert permission
on DAC level, because it become an incorrect assumption that owner of
table has insert permission on its creation time.

postgres=# ALTER DEFAULT PRIVILEGES FOR USER tak GRANT select ON TABLES TO tak;
ALTER DEFAULT PRIVILEGES
postgres=# \ddp        Default access privilegesOwner | Schema | Type  | Access privileges
-------+--------+-------+-------------------tak   |        | table | tak=r/tak
(1 row)

postgres=# SET SESSION AUTHORIZATION tak;
SET
postgres=> SELECT * INTO t1 FROM (VALUES(1,'aaa'), (2,'bbb'), (3,'ccc')) AS v;
SELECT 3
postgres=> SELECT * FROM t1;column1 | column2
---------+---------      1 | aaa      2 | bbb      3 | ccc
(3 rows)

postgres=> INSERT INTO t1 VALUES (4,'ddd');
ERROR:  permission denied for relation t1

Oops!
--
KaiGai Kohei <kaigai@kaigai.gr.jp>


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: synchronized snapshots
Next
From: Simon Riggs
Date:
Subject: Re: synchronized snapshots