Re: sepgsql contrib module - Mailing list pgsql-hackers

(2011/01/20 12:10), Robert Haas wrote:
> 2011/1/5 KaiGai Kohei<kaigai@ak.jp.nec.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@ak.jp.nec.com>