On 2012-03-09 21:49, Robert Haas wrote:
On Tue, Mar 6, 2012 at 9:14 AM, Kohei KaiGai<kai...@kaigai.gr.jp> wrote:
[ new patch ]
Are we absolutely certain that we want the semantics of
sepgsql_setcon() to be transactional? Because if we made them
non-transactional, this would be a whole lot simpler, and it would
still meet the originally proposed use case, which is to allow the
security context of a connection to be changed on a one-time basis
before handing it off to a client application.
It would meet the original use case, but outside of that use case it
would be very easy to get POLA violations. Imagine a transaction like
1- do stuff under label A
2- setcon to B
3- do stuff under label B
When that transaction fails due to a serialization error, one would
expect that when the transaction is replayed, the initial actions are
executed under label A. If it was B, or any other further label in the
original transaction, it would be very hard to develop software in user
space that could cope with this behaviour.
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.
It seems to me that it would make more sense to view the set of
security labels in effect as a stack. When we enter a trusted
procedure, it pushes a new label on the stack; when we exit a trusted
procedure, it pops the top label off the stack. sepgsql_setcon()
changes the top label on the stack. If we want to retain
transactional semantics, then you can also have a toplevel label that
doesn't participate in the stack; a commit copies the sole item on the
stack into the toplevel label, and transaction start copies the
toplevel label into an empty stack.
Yes the additions be sepgsql_setcon look like a stack for pushing.
However, the current code that rollbacks the pending list to a certain
savepoint matches code from e.g. StandbyReleaseLocks(), so having a
concept like this as a list is not unknown to the current codebase.
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.
- and what I'm proposing is
that client_label_func shouldn't be separate, but rather should mutate
the stack of labels maintained by client_label_pending.
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.
The docs need updating, both to reflect the version bump in
sepgsql-regtest.te and to describe the actual feature.
I can probably write some docs tomorrow.
regards,
Yeb Havinga
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
