Re: ExecutorCheckPerms() hook

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: ExecutorCheckPerms() hook
Date: 2010-06-14 08:27:11
Message-ID: 4C15E7DF.2090308@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I attached three patches for the effort.
Each patch tries to tackle one theme, so it is not unreasonable.

But the ESP security hook patch (quite tiny) depends on the DML permission
refactoring patch (relatively larger). So, Robert suggested me to reconsider
the dependency of these patches.

The attached patch shall be applied on the head of the git repository.
It just adds a security hook on ExecCheckRTPerms() as Robert suggested
at first.
Of course, it does not allow to acquire the control on COPY TO/FROM and
RI_Initial_Check(). It will be refactored in the following patches.

Thanks,

(2010/05/27 12:00), KaiGai Kohei wrote:
> Stephen,
>
>>> The 'failure' may make an impression of generic errors not only permission denied.
>>> How about 'error_on_violation'?
>>
>> Maybe 'ereport_on_violation'? I dunno, guess one isn't really better
>> than the other. You need to go back and fix the comment though- you
>> still say 'abort' there.
>
> I have no preference between 'error_on_violation' and 'ereport_on_violation'.
> OK, I fixed it.
>
>>> BTW, I wonder whether acl.h is a correct place to explain about the hook,
>>> although I added comments for the hook.
>>
>> Guess I don't really see a problem putting the comments there. By the
>> way, have we got a place where we actually document the hooks we support
>> somewhere in the official documentation..? If so, that should certainly
>> be updated too..
>
> I could not find Executor hooks from doc/src/sgml using grep.
> If so, it might be worth to list them on the wikipage.
>
>>> I think we should add a series of explanation about ESP hooks in the internal
>>> section of the documentation, when the number of hooks reaches a dozen for
>>> example.
>>
>> I believe the goal will be to avoid reaching a dozen hooks for this.
>
> Maybe, all we need to hook on DML permissions is only this one.
>
>> All-in-all, I'm pretty happy with these. Couple minor places which
>> could use some copy editing, but that's about it.
>>
>> Next, we need to get the security label catalog and the grammar to
>> support it implemented and then from that an SELinux module should
>> be pretty easy to implement. Based on the discussions at PGCon, Robert
>> is working on the security label catalog and grammar. The current plan
>> is to have a catalog similar to pg_depend, to minimize impact to the
>> rest of the backend and to those who aren't interested in using security
>> labels.
>
> Pg_depend? not pg_description/pg_shdescription?
>
> I basically agree with the idea that minimizes damages to the existing schema
> of system catalogs, but I cannot imagine something like pg_depend well.
>
> I'd like to post a new thread to discuss the security label support. OK?
>
>> Of course, there will also need to be hooks there for an
>> external module to enforce restrictions associated with changing labels
>> on various objects in the system.
>
> Yes, the user given has to be validated by ESP.
>
> Thanks,

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

Attachment Content-Type Size
pgsql-v9.1-add-dml-hook.1.patch application/octect-stream 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message KaiGai Kohei 2010-06-14 08:34:21 [v9.1] add makeRangeTblEntry() into makefuncs.c
Previous Message Fujii Masao 2010-06-14 08:14:40 Re: Proposal for 9.1: WAL streaming from WAL buffers