On Sat, Mar 10, 2012 at 4:39 AM, Yeb Havinga <yebhavinga@gmail.com> wrote:
>> As a separate but related note, the label management here seems to be
>> excessively complicated. In particular, it seems to me that the
>> semantics of sepgsql_get_client_label() become quite muddled under
>> this patch. An explicitly-set label overrides the default label, but
>> a trusted procedure's temporary label overrides that. So if you enter
>> a trusted procedure, and it calls sepgsql_setcon(), it'll change the
>> label, but no actual security transition will occur; then, when you
>> exit the trusted procedure, you'll get the new label in place of
>> whatever the caller had before. That seems like one heck of a
>> surprising set of semantics.
>
> I agree that sepgsql_get_*client*_label does not really match with a trusted
> procedure temporarily changing it.
I'm not complaining about the name of the function; I'm complaining
about the semantics.
>> In the current coding, I think
>> client_label_peer is redundant with client_label_committed - once the
>> latter is set, the former is unused, IIUC
>
> client_label_peer is used on reset of the client label, i.e. calling
> sepgsql_setcon with NULL.
Ugh. What's the point of supporting that?
> The drawback of having trusted procedures push things on this stack is that
> it would add a memory alloc and size overhead when calling trusted
> functions, especially for recursive functions.
Compared to all the other overhead of using sepgsql, that is miniscule
and not worth worrying about.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
