Re: sepgsql contrib module

(2011/01/20 12:10), Robert Haas wrote:
> 2011/1/5 KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> How about feasibility to merge this 4KL chunks during the rest of
>> 45 days? I think we should decide this general direction at first.
>
> I read through this code tonight and it looks pretty straightforward.
> I don't see much reason not to accept this more or less as-is. I'm a
> bit suspicious of this line:
>
> { "translation", SEPG_PROCESS__TRANSITION },
>
> I can't help wondering based on the rest of the table whether you
> intend to have the same word on that line twice, but you don't. On a
> related note, would it make sense to pare down this table to the
> entries that are actually used at the moment?

Sorry for giving you a confusion.
It was my typo, so should be fixed as:
{ "transition", SEPG_PROCESS_TRANSITION },

This permission shall be checked when a security label of client being
switched from X to Y, typically on execution of trusted-procedure.
Also, I missed to check on sepgsql_needs_fmgr_hook(). We don't want to
allow inlining the function on lack of permission.

I'll fix them soon.

> And how about adding a
> ProcessUtility_hook to trap evil non-DML statements that some
> nefarious user might issues?
>
It seems to me reasonable as long as the number of controlled command
are limited. For example, LOAD command may be a candidate being
controlled without exceptions.
However, it will be a tough work, if the plug-in tries to parse and
analyze supplied utility commands by itself.

> One other problem is that you need to work on your whitespace a bit.
> I believe in a few places you have a mixture of tabs and spaces. More
> seriously, pgindent is going to completely mangle things like this:
>
> /*
> * sepgsql_mode
> *
> * SEPGSQL_MODE_DISABLED : Disabled on runtime
> * SEPGSQL_MODE_DEFAULT : Same as system settings
> * SEPGSQL_MODE_PERMISSIVE : Always permissive mode
> * SEPGSQL_MODE_INTERNAL : Same as SEPGSQL_MODE_PERMISSIVE,
> * except for
> no audit prints
> */
>
> You have to write it with a line of dashes on the first and last
> lines, if you don't want it reformatted as a paragraph. It might be
> worth actually running pgindent over contrib/selinux and then check
> over the results.
>
OK, I'll try to run pgindent to confirm the effect of reformat.

> Finally, we need to work on the documentation.
>
I uploaded my draft here.
http://wiki.postgresql.org/wiki/SEPostgreSQL_Documentation

If reasonable, I'll move them into *.sgml style.

I may want to simplify the step to installation using an installer
script.

> But overall, it looks pretty good, IMHO.
>
Thanks for your reviewing in spite of a large patch.
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>