Re: [v9.2] Add GUC sepgsql.client_label

2012/1/28 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
> 2012/1/26 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Thu, Jan 26, 2012 at 2:07 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> 2012/1/26 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>>> I'm wondering if a function would be a better fit than a GUC.  I don't
>>>> think you can really restrict the ability to revert a GUC change -
>>>> i.e. if someone does a SET and then a RESET, you pretty much have to
>>>> allow that.  I think.  But if you expose a function then it can work
>>>> however you like.
>>>>
>>> One benefit to use GUC is that we can utilize existing mechanism to
>>> revert a value set within a transaction block on error.
>>> If we implement same functionality with functions, XactCallback allows
>>> sepgsql to get control on appropriate timing?
>>
>> Not sure, but I thought the use case was to set this at connection
>> startup time and then hand the connection off to a client.  What keeps
>> the client from just issuing RESET?
>>
> In the use case of Joshua, the original security label does not privileges
> to reference any tables, and it cannot translate any domains without
> credential within IC-cards. Thus, RESET is harmless.
>
> However, I also assume one other possible use-case; the originator
> has privileges to translate 10 of different domains, but disallows to
> revert it. In this case, RESET without permission checks are harmful.
> (This scenario will be suitable with multi-category-model.)
>
> Apart from this issue, I found a problem on implementation using GUC
> options. It makes backend crash, if it tries to reset sepgsql.client_label
> without permissions within error handler because of double-errors.
>
I found an idea to avoid this scenario.

The problematic case is reset GUC variable because of transaction
rollback and permission violation, however, we don't intend to apply
permission checks, since it is not committed yet.
Thus, I considered to check state of the transaction using
IsTransactionState() on the assign_hook of GUC variable.

Unlike function based implementation, it is available to utilize existing
infrastructure to set the client_label to be transaction-aware.

It shall be implemented as:

void
sepgsql_assign_client_label(const char *newval, void *extra)
{
if (!IsTransactionState)
return;

/* check whether valid security context */

/* check permission check to switch current context */
}

How about such an implementation?

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>