Re: [v9.2] Object access hooks with arguments support (v1)

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.2] Object access hooks with arguments support (v1)
Date: 2011-10-18 17:23:11
Message-ID: CADyhKSVo_cn+4iv16O=1WUi-PW=66U8qshv0SFwoQOERJ-ELSA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2011/10/18 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Oct 18, 2011 at 11:25 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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 privileges
Owner | 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(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2011-10-18 17:32:37 Re: synchronized snapshots
Previous Message Tom Lane 2011-10-18 17:22:51 Re: synchronized snapshots