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

2012-03-21 Thread Kohei KaiGai
2012/3/20 Robert Haas robertmh...@gmail.com:
 On Fri, Mar 16, 2012 at 3:44 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 In the patch with copy-editing documentation following that commit, at in
 at their option, s/in// ?

 Oh, yeah.  Oops.  Thanks.

 Also 'rather than .. as mandated by the system':
 I'm having trouble parsing 'as'. It is also unclear to me what 'system'
 means: selinux or PostgreSQL, or both? I suspect it is PostgreSQL, since
 selinux is still enforcing / 'mandating' it's policy. What about rather
 than that the switch is controlled by the PostgreSQL server, as in the case
 of a trusted procedure.

 Well, I think it's both.  PostgreSQL is responsible for enforcing
 privileges on database objects, but it relies on SE-Linux to tell it
 whether a given access is allowable.  So, from PostgreSQL's point of
 view, it's delegating the decision to SE-Linux.  But SE-Linux views
 itself as a mechanism for enforcing a system-wide security policy, so
 views PostgreSQL as an instrument for carrying out its access control
 goals.  I don't know how to disentangle that.  I'm actually not
 entirely sure that I even believe the underlying sentiment that
 dynamic transitions are dangerous.  Maybe KaiGai could comment further
 on what we should be trying to convey here.

The reason why dynamic domain transition should be configured
carefully is that it partially allows users to switch their own privileges
in discretionary way, unlike trusted procedure.

The original model of selinux on operating system assumes all the
domain transition shall happen on execve(2) time, but it made clear
some sort of application is not happy with traditional fork - exec
lifecycle, such as web server, connection pooling software, or others.

Even as they perform according to the operations from users,
it does not fork - exec itself because of some reason, typically
performance. One point we should focus on is these applications
have relatively trustable portion and untrustable one.

The dynamic domain transition was designed to restrict privileges
more than the current one on the trustable portion, prior to launch
untrustable one. So, it never intend to switch client domain with
100% arbitrary. Its bottom line is restricted with the security policy;
that explicitly describes the range of domains being allowed to
translate.

So, we will be able to conclude dynamic domain transition is
harmless as long as it works to reduce privileges; that should
be guaranteed with the security policy.
It also means sepgsql_setcon() is harmless as long as it works
according to the decision of SELinux.

The connection pooling software scenario using trusted procedure
might be a bit confusing. In this case, the client domain is once
switched to the trusted one with mandatory way, then it switches
to more restricted domain in arbitrary way; thus, it is not allowed
to promote its privileges in arbitrary way.
We assume the trusted procedure is a enough small portion to
ensure bug or vulnerability free.

Joshua, please add some comments, if you have.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-21 Thread Robert Haas
On Wed, Mar 21, 2012 at 6:07 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 The reason why dynamic domain transition should be configured
 carefully is that it partially allows users to switch their own privileges
 in discretionary way, unlike trusted procedure.

 The original model of selinux on operating system assumes all the
 domain transition shall happen on execve(2) time, but it made clear
 some sort of application is not happy with traditional fork - exec
 lifecycle, such as web server, connection pooling software, or others.

 Even as they perform according to the operations from users,
 it does not fork - exec itself because of some reason, typically
 performance. One point we should focus on is these applications
 have relatively trustable portion and untrustable one.

 The dynamic domain transition was designed to restrict privileges
 more than the current one on the trustable portion, prior to launch
 untrustable one. So, it never intend to switch client domain with
 100% arbitrary. Its bottom line is restricted with the security policy;
 that explicitly describes the range of domains being allowed to
 translate.

 So, we will be able to conclude dynamic domain transition is
 harmless as long as it works to reduce privileges; that should
 be guaranteed with the security policy.
 It also means sepgsql_setcon() is harmless as long as it works
 according to the decision of SELinux.

 The connection pooling software scenario using trusted procedure
 might be a bit confusing. In this case, the client domain is once
 switched to the trusted one with mandatory way, then it switches
 to more restricted domain in arbitrary way; thus, it is not allowed
 to promote its privileges in arbitrary way.
 We assume the trusted procedure is a enough small portion to
 ensure bug or vulnerability free.

 Joshua, please add some comments, if you have.

I guess my feeling on this is that the warning in the documentation
isn't really helping anything here.  I mean, we don't need to document
that allowing people to give themselves more privileges is a security
hole; that much is obvious.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-20 Thread Robert Haas
On Fri, Mar 16, 2012 at 3:44 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 In the patch with copy-editing documentation following that commit, at in
 at their option, s/in// ?

Oh, yeah.  Oops.  Thanks.

 Also 'rather than .. as mandated by the system':
 I'm having trouble parsing 'as'. It is also unclear to me what 'system'
 means: selinux or PostgreSQL, or both? I suspect it is PostgreSQL, since
 selinux is still enforcing / 'mandating' it's policy. What about rather
 than that the switch is controlled by the PostgreSQL server, as in the case
 of a trusted procedure.

Well, I think it's both.  PostgreSQL is responsible for enforcing
privileges on database objects, but it relies on SE-Linux to tell it
whether a given access is allowable.  So, from PostgreSQL's point of
view, it's delegating the decision to SE-Linux.  But SE-Linux views
itself as a mechanism for enforcing a system-wide security policy, so
views PostgreSQL as an instrument for carrying out its access control
goals.  I don't know how to disentangle that.  I'm actually not
entirely sure that I even believe the underlying sentiment that
dynamic transitions are dangerous.  Maybe KaiGai could comment further
on what we should be trying to convey here.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-16 Thread Yeb Havinga

On 2012-03-15 21:45, Robert Haas wrote:

On Wed, Mar 14, 2012 at 11:10 AM, Kohei KaiGaikai...@kaigai.gr.jp  wrote:

If it is ready to commit, please remember the credit to Yeb's volunteer
on this patch.

Done.

In the patch with copy-editing documentation following that commit, at 
in at their option, s/in// ? Also 'rather than .. as mandated by the 
system': I'm having trouble parsing 'as'. It is also unclear to me what 
'system' means: selinux or PostgreSQL, or both? I suspect it is 
PostgreSQL, since selinux is still enforcing / 'mandating' it's policy. 
What about rather than that the switch is controlled by the PostgreSQL 
server, as in the case of a trusted procedure.


+Dynamic domain transitions should be considered carefully, because they
+allow users to switch their label, and therefore their privileges, in
+at their option, rather than (as in the case of a trusted procedure)
+as mandated by the system.

--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-15 Thread Robert Haas
On Wed, Mar 14, 2012 at 11:10 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 If it is ready to commit, please remember the credit to Yeb's volunteer
 on this patch.

Done.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-13 Thread Kohei KaiGai
2012/3/12 Robert Haas robertmh...@gmail.com:
 On Mon, Mar 12, 2012 at 12:30 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 Suppose that the connection starts out in context connection_pooler_t.
  Based on the identity of the user, we transition to foo_t, bar_t, or
 baz_t.  If it's possible, by any method, for one of those contexts to
 get back to connection_pooler_t, then we've got a problem.  We give a
 connection to user foo which is in foo_t; he transitions it back to
 connection_pooler_t, then to bar_t, and usurps user bar's privileges.
 Unless there's some way to prevent that, the only way to make this
 secure is to make the transition to foo_t irreversible.

 It is the reason why I advocate the idea to allow sepgsql_setcon()
 inside of trusted-procedures.

 The original use-case of Joshua does not allow connection_pooler_t
 to execute any SQL commands except for invocation of a particular
 trusted-procedures; that takes a secret credential as an argument,
 then it switches the client label to foo_t, bar_t or baz_t according to
 the supplied credential.
 These labels are allowed to switch back to the original
 connection_pooler_t, but it is unavailable to switch arbitrary label
 without suitable credential.

 Oh, I get it.

 Given that that's the intended use case, the current design does make
 sense, but it seems awfully special-purpose.  Not knowing that this is
 what you had in mind, I never would have guessed the reason for all
 this complexity.  I worry that this is too much of a purpose-built
 mechanism, and that nobody will ever be able to use it for much of
 anything beyond the extremely specific use case that you've laid out
 here.  I think that, at the very least, the comments and documentation
 need to make it clear that this is very deliberately intended to
 modify only the toplevel security context of the session, which may be
 different from the currently active context if a TP is in use; and
 also that the change will apply to future transactions only if the
 current transaction commits.

OK, I try to update the documentation and test cases with related
security policy, rather than the code base itself.

Please wait for a few days to update them.
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Kohei KaiGai
2012/3/9 Robert Haas robertmh...@gmail.com:
 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.

I hesitate to implement sepgsql_setcon() being not-transaction aware,
because any other functionality are all transaction aware, such as
SECURITY LABEL statement.
Even though the original use-case from Joshua does not require it
being transaction-aware, I'd like to keep consistency of behavior
with any other features. Is it really unacceptable complexity?

 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.  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 - 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.

I almost agree with your opinion. A semantics to stack security label
will be more straight-forward.

One reason of the redundant client_label_peer is to support reset
client label using NULL-input on sepgsql_setcon(). We need to save
the original value somewhere.

In case of sepgsql_setcon() being invoked inside of the trusted-
procedure, indeed, the existing behavior is confusing as you pointed
out. If we would changed the semantics using a stack structure,
an invocation of trusted procedure takes a label transition from A to B,
then it invokes sepgsql_setcon() to take a label transition from B to C,
and the label B shall be popped from the stack at end of the trusted
procedure. Eventually, the label C shall be applied on top of the stack.
Do we share same image to whole of the idea?

 The docs need updating, both to reflect the version bump in
 sepgsql-regtest.te and to describe the actual feature.

OK, please wait for a few days.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Kohei KaiGai
2012/3/11 Yeb Havinga yebhavi...@gmail.com:
 On 2012-03-10 10:39, I  wrote:


 I can probably write some docs tomorrow.


 Attached is v5 of the patch, with is exactly equal to v4 but with added
 documentation.

Thanks for your dedicated volunteer. I'm under checking of the updates
at documentation.

 Some other notes.

 - Robert asked why sepgsql_setcon with NULL to reset the value to the
 initial client label was supported. Maybe this could be a leftover from the
 initial implementation as GUC variable?

It is a practical reason. In case when httpd open the connection to PG and
set a suitable security label according to the given credential prior to launch
of user application, then keep this connection for upcoming request, it is
worthwhile to reset security label of the client.

 - earlier I suggested preventing setting a new client label from a trusted
 procedure, however I just read in the original post that this was how the
 current usecase of Joshua is set up. Suggestion withdrawn.

In the scenario of Joshua's security policy, it does not allow httpd to issue
SQL commands except for the trusted procedure that calls sepgsql_setcon()
according to the given credential. (Thus, we have no way to set an arbitrary
security label without credentials.) The security label being switched is
allowed to issue SQL commands with restricted privileges, and also allows
to translate into httpd's domain.
If we would not support sepgsql_setcon() in trusted procedure, we have to
allow httpd to translate an arbitrary label thus it also disallow to keep
connection because it should not revert the client label to httpd (it means
restricted users enable to switch someone arbitrary!)

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Robert Haas
On Sat, Mar 10, 2012 at 4:35 PM, Yeb Havinga yebhavi...@gmail.com wrote:
 The semantics are muddled because the client labels are mixed with labels
 from trusted procedures. The stack you described upthread may also exhibit
 surprising behaviour. If a trusted procedure is called, it's label is pushed
 onto the stack.

What I was proposing is that it would *replace* the label on top of the stack.

 Suppose it pushes another label by calling sepgsql_setcon().
 In the stack case, this is now the top of the stack and the result of
 sepgsql_get_client_label(). The procedure exists. What should now be the
 result of sepgsql_get_client_label()? Since labels are managed by a stack,
 we can only pop what's on top and need to pop twice,

The above avoids the need to pop twice.

 so we've lost the label
 pushed onto the stack by the trusted function, which means that trusted
 procedures cannot be used to change client labels beyond their own scope,
 but other functions would.

That's true, but I'm not convinced it's bad.  I mean, if you instruct
the system to change security labels for the duration of a trusted
procedure, then it shouldn't be surprising that you end up with the
same security label after it exits that you had before entering it.
At the very least, it has the virtue of being consistent with other
things, like how GUCs behave.  The current behavior is that you change
the context and you don't see the results of your own context change,
which seems far worse.

 Maybe this semantics muddling of trusted procedures and setting the client
 label can be avoided by denying changing the client label with
 sepgsql_setcon() from a trusted procedure, which would also make some sense:

 ERROR: cannot override the client label of a trusted procedure during
 execution.

That also seems possibly reasonable.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Robert Haas
On Mon, Mar 12, 2012 at 10:58 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 It is a practical reason. In case when httpd open the connection to PG and
 set a suitable security label according to the given credential prior to 
 launch
 of user application, then keep this connection for upcoming request, it is
 worthwhile to reset security label of the client.

But wait a minute - how is that any good?  That allows the client to
pretty trivially circumvent the security restriction that we were
trying to impose by doing sepgsql_setcon() in the first place.  It
doesn't matter how convenient it is if it's flagrantly insecure.

Am I missing something here?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Kohei KaiGai
2012/3/12 Robert Haas robertmh...@gmail.com:
 On Mon, Mar 12, 2012 at 10:58 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 It is a practical reason. In case when httpd open the connection to PG and
 set a suitable security label according to the given credential prior to 
 launch
 of user application, then keep this connection for upcoming request, it is
 worthwhile to reset security label of the client.

 But wait a minute - how is that any good?  That allows the client to
 pretty trivially circumvent the security restriction that we were
 trying to impose by doing sepgsql_setcon() in the first place.  It
 doesn't matter how convenient it is if it's flagrantly insecure.

 Am I missing something here?

It is a practical reason. If we would not support the reset feature,
the application has to know the security label of itself, as a target
label to be reverted. However, I'm not certain the status of script-
language binding of libselinux feature to obtain the self label,
although it is supported on Perl, Ruby and PHP (with extension
by myself) at least.

It seems to me a reasonable cost to track the original label to
eliminate a restriction of application side that tries to revert
the security label once switched.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Robert Haas
On Mon, Mar 12, 2012 at 11:13 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/3/12 Robert Haas robertmh...@gmail.com:
 On Mon, Mar 12, 2012 at 10:58 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 It is a practical reason. In case when httpd open the connection to PG and
 set a suitable security label according to the given credential prior to 
 launch
 of user application, then keep this connection for upcoming request, it is
 worthwhile to reset security label of the client.

 But wait a minute - how is that any good?  That allows the client to
 pretty trivially circumvent the security restriction that we were
 trying to impose by doing sepgsql_setcon() in the first place.  It
 doesn't matter how convenient it is if it's flagrantly insecure.

 Am I missing something here?

 It is a practical reason. If we would not support the reset feature,
 the application has to know the security label of itself, as a target
 label to be reverted. However, I'm not certain the status of script-
 language binding of libselinux feature to obtain the self label,
 although it is supported on Perl, Ruby and PHP (with extension
 by myself) at least.

You're still missing my point.  The issue isn't the particular choice
of mechanism for reverting to the original security label; it's the
fact that such a thing would be possible at all.

Suppose that the connection starts out in context connection_pooler_t.
 Based on the identity of the user, we transition to foo_t, bar_t, or
baz_t.  If it's possible, by any method, for one of those contexts to
get back to connection_pooler_t, then we've got a problem.  We give a
connection to user foo which is in foo_t; he transitions it back to
connection_pooler_t, then to bar_t, and usurps user bar's privileges.
Unless there's some way to prevent that, the only way to make this
secure is to make the transition to foo_t irreversible.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Kohei KaiGai
2012/3/12 Robert Haas robertmh...@gmail.com:
 On Mon, Mar 12, 2012 at 11:13 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/3/12 Robert Haas robertmh...@gmail.com:
 On Mon, Mar 12, 2012 at 10:58 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 It is a practical reason. In case when httpd open the connection to PG and
 set a suitable security label according to the given credential prior to 
 launch
 of user application, then keep this connection for upcoming request, it is
 worthwhile to reset security label of the client.

 But wait a minute - how is that any good?  That allows the client to
 pretty trivially circumvent the security restriction that we were
 trying to impose by doing sepgsql_setcon() in the first place.  It
 doesn't matter how convenient it is if it's flagrantly insecure.

 Am I missing something here?

 It is a practical reason. If we would not support the reset feature,
 the application has to know the security label of itself, as a target
 label to be reverted. However, I'm not certain the status of script-
 language binding of libselinux feature to obtain the self label,
 although it is supported on Perl, Ruby and PHP (with extension
 by myself) at least.

 You're still missing my point.  The issue isn't the particular choice
 of mechanism for reverting to the original security label; it's the
 fact that such a thing would be possible at all.

 Suppose that the connection starts out in context connection_pooler_t.
  Based on the identity of the user, we transition to foo_t, bar_t, or
 baz_t.  If it's possible, by any method, for one of those contexts to
 get back to connection_pooler_t, then we've got a problem.  We give a
 connection to user foo which is in foo_t; he transitions it back to
 connection_pooler_t, then to bar_t, and usurps user bar's privileges.
 Unless there's some way to prevent that, the only way to make this
 secure is to make the transition to foo_t irreversible.

It is the reason why I advocate the idea to allow sepgsql_setcon()
inside of trusted-procedures.

The original use-case of Joshua does not allow connection_pooler_t
to execute any SQL commands except for invocation of a particular
trusted-procedures; that takes a secret credential as an argument,
then it switches the client label to foo_t, bar_t or baz_t according to
the supplied credential.
These labels are allowed to switch back to the original
connection_pooler_t, but it is unavailable to switch arbitrary label
without suitable credential.

Please also note that the reset of security label is handled as
a switch from the current one to the original one; that takes
permission check as normal manner.
So, it is an option to prevent to reset the client label to the original
one; that is allowed to switch arbitrary label, in an environment
without connection pooling.

Do we still have problematic scenario here?

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-12 Thread Robert Haas
On Mon, Mar 12, 2012 at 12:30 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 Suppose that the connection starts out in context connection_pooler_t.
  Based on the identity of the user, we transition to foo_t, bar_t, or
 baz_t.  If it's possible, by any method, for one of those contexts to
 get back to connection_pooler_t, then we've got a problem.  We give a
 connection to user foo which is in foo_t; he transitions it back to
 connection_pooler_t, then to bar_t, and usurps user bar's privileges.
 Unless there's some way to prevent that, the only way to make this
 secure is to make the transition to foo_t irreversible.

 It is the reason why I advocate the idea to allow sepgsql_setcon()
 inside of trusted-procedures.

 The original use-case of Joshua does not allow connection_pooler_t
 to execute any SQL commands except for invocation of a particular
 trusted-procedures; that takes a secret credential as an argument,
 then it switches the client label to foo_t, bar_t or baz_t according to
 the supplied credential.
 These labels are allowed to switch back to the original
 connection_pooler_t, but it is unavailable to switch arbitrary label
 without suitable credential.

Oh, I get it.

Given that that's the intended use case, the current design does make
sense, but it seems awfully special-purpose.  Not knowing that this is
what you had in mind, I never would have guessed the reason for all
this complexity.  I worry that this is too much of a purpose-built
mechanism, and that nobody will ever be able to use it for much of
anything beyond the extremely specific use case that you've laid out
here.  I think that, at the very least, the comments and documentation
need to make it clear that this is very deliberately intended to
modify only the toplevel security context of the session, which may be
different from the currently active context if a TP is in use; and
also that the change will apply to future transactions only if the
current transaction commits.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-11 Thread Yeb Havinga

On 2012-03-10 10:39, I  wrote:


I can probably write some docs tomorrow.



Attached is v5 of the patch, with is exactly equal to v4 but with added 
documentation.


Some other notes.

- Robert asked why sepgsql_setcon with NULL to reset the value to the 
initial client label was supported. Maybe this could be a leftover from 
the initial implementation as GUC variable?
- earlier I suggested preventing setting a new client label from a 
trusted procedure, however I just read in the original post that this 
was how the current usecase of Joshua is set up. Suggestion withdrawn.


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data

diff --git a/contrib/sepgsql/expected/label.out b/contrib/sepgsql/expected/label.out
index bac169f..e967c7c 100644
--- a/contrib/sepgsql/expected/label.out
+++ b/contrib/sepgsql/expected/label.out
@@ -26,6 +26,11 @@ CREATE FUNCTION f4 () RETURNS text
 AS 'SELECT sepgsql_getcon()'
 LANGUAGE sql;
 SECURITY LABEL ON FUNCTION f4()
+IS 'system_u:object_r:sepgsql_nosuch_trusted_proc_exec_t:s0';
+CREATE FUNCTION f5 (text) RETURNS bool
+	AS 'SELECT sepgsql_setcon($1)'
+LANGUAGE sql;
+SECURITY LABEL ON FUNCTION f5(text)
 IS 'system_u:object_r:sepgsql_regtest_trusted_proc_exec_t:s0';
 --
 -- Tests for default labeling behavior
@@ -100,6 +105,223 @@ SELECT sepgsql_getcon();	-- client's label must be restored
 (1 row)
 
 --
+-- Test for Dynamic Domain Transition
+--
+-- validation of transaction aware dynamic-transition
+SELECT sepgsql_getcon();	-- confirm client privilege
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c25
+(1 row)
+
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c15');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c15
+(1 row)
+
+SELECT sepgsql_setcon(NULL);	-- failed to reset
+ERROR:  SELinux: security policy violation
+SELECT sepgsql_getcon();
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c15
+(1 row)
+
+BEGIN;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c12');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c12
+(1 row)
+
+SAVEPOINT svpt_1;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c9');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+ sepgsql_getcon  
+-
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c9
+(1 row)
+
+SAVEPOINT svpt_2;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c6');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+ sepgsql_getcon  
+-
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c6
+(1 row)
+
+SAVEPOINT svpt_3;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c3');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+ sepgsql_getcon  
+-
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c3
+(1 row)
+
+ROLLBACK TO SAVEPOINT svpt_2;
+SELECT sepgsql_getcon();		-- should be 's0:c0.c9'
+ sepgsql_getcon  
+-
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c9
+(1 row)
+
+ROLLBACK TO SAVEPOINT svpt_1;
+SELECT sepgsql_getcon();		-- should be 's0:c0.c12'
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c12
+(1 row)
+
+ABORT;
+SELECT sepgsql_getcon();		-- should be 's0:c0.c15'
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c15
+(1 row)
+
+BEGIN;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c8');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+ sepgsql_getcon  
+-
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c8
+(1 row)
+
+SAVEPOINT svpt_1;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c4');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+ sepgsql_getcon  
+-
+ 

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

2012-03-11 Thread Yeb Havinga

On 2012-03-11 11:33, Yeb Havinga wrote:

On 2012-03-10 10:39, I  wrote:


I can probably write some docs tomorrow.



Attached is v5 of the patch, with is exactly equal to v4 but with 
added documentation.


s/literalcube(1) == '(1)'/literal// in that patch please



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-10 Thread Yeb Havinga

On 2012-03-09 21:49, Robert Haas wrote:

On Tue, Mar 6, 2012 at 9:14 AM, Kohei KaiGaikai...@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


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

2012-03-10 Thread Robert Haas
On Sat, Mar 10, 2012 at 4:39 AM, Yeb Havinga yebhavi...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-10 Thread Yeb Havinga

On 2012-03-10 14:06, Robert Haas wrote:

On Sat, Mar 10, 2012 at 4:39 AM, Yeb Havingayebhavi...@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.


The semantics are muddled because the client labels are mixed with 
labels from trusted procedures. The stack you described upthread may 
also exhibit surprising behaviour. If a trusted procedure is called, 
it's label is pushed onto the stack. Suppose it pushes another label by 
calling sepgsql_setcon(). In the stack case, this is now the top of the 
stack and the result of sepgsql_get_client_label(). The procedure 
exists. What should now be the result of sepgsql_get_client_label()? 
Since labels are managed by a stack, we can only pop what's on top and 
need to pop twice, so we've lost the label pushed onto the stack by the 
trusted function, which means that trusted procedures cannot be used to 
change client labels beyond their own scope, but other functions would.


Maybe this semantics muddling of trusted procedures and setting the 
client label can be avoided by denying changing the client label with 
sepgsql_setcon() from a trusted procedure, which would also make some sense:


ERROR: cannot override the client label of a trusted procedure during 
execution.


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-09 Thread Yeb Havinga

On 2012-03-06 15:14, Kohei KaiGai wrote:

In case of sepgsql_setcon() being invoked with null argument
to reset security label of the client, but not committed yet,
the last item of the client_label_pending has null label.
(It performs as a mark of a security label being reset.)
Yes, I see that now. Another solution could be to append 
client_label_peer on the pending list instead of NULL, but maybe this is 
not important enough to discuss. I tried to do some testing by first 
transition into a smaller MLS context, then reset to the original again, 
but that is not allowed by the regtest policy. I searched the internet 
for 30 minutes about how to write a allow rule that would allow e.g. the 
transition from c1.c4 back to c1.c1023 but failed.


two other minor nitpicks

-- * contrib/sepgsql/label.c
-+ * contrib/sepgsqlet/label.c

typo here..

-+  /* Append the supplied new_label on the pending list. */
++  /*
++   * Append the supplied new_label on the pending list until
++   * the current transaction is committed.
++   */

the 'until the current transaction is committed' part is something going 
on outside of sepgsql_set_client_label() - this function just appends to 
the list, always. That the list is reset on transaction commit/abort 
time, is done and also already documented elsewhere. A new reader could 
be confused to not find transaction related things in 
sepgsql_set_client_label().


regards,
Yeb

--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-09 Thread Robert Haas
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.

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.

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.  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 - 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 docs need updating, both to reflect the version bump in
sepgsql-regtest.te and to describe the actual feature.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-06 Thread Kohei KaiGai
Hi, Yeb.

Thanks for your reviewing and patch updates.
(and sorry my delayed response...)

I'd like to point out a case when plabel-label is NULL.

In case of sepgsql_setcon() being invoked with null argument
to reset security label of the client, but not committed yet,
the last item of the client_label_pending has null label.
(It performs as a mark of a security label being reset.)

It is a case when sepgsql_get_client_label() should return
the client_label_peer, not plabel-label.
So, I reverted some of your replacement; that assumes the
pending label is valid with Assert() check to null value.

Most of comments update are quite helpful for me.
So, I merged your revised one in this patch.

Thanks so much!

2012/3/3 Yeb Havinga yebhavi...@gmail.com:
 On 2012-02-24 17:25, Yeb Havinga wrote:

 On 2012-02-23 12:17, Kohei KaiGai wrote:

 2012/2/20 Yeb Havingayebhavi...@gmail.com:

 On 2012-02-05 10:09, Kohei KaiGai wrote:

 The attached part-1 patch moves related routines from hooks.c to
 label.c
 because of references to static variables. The part-2 patch implements
 above
 mechanism.


 I took a short look at this patch but am stuck getting the regression
 test
 to run properly.

 First, patch 2 misses the file sepgsql.sql.in and therefore the creation
 function command for sepgsql_setcon is missing.

 Thanks for your comments.

 I added the definition of sepgsql_setcon function to sepgsql.sql.in file,
 in addition to patch rebasing.


 Very brief comments due to must leave keyboard soon:

 I read the source code and played a bit with setcon and the debugger, no
 strange things found.

 Code comments / questions:


 I took the liberty to change a few things, mostly comments, in the attached
 patch:


 maybe client_label_committed is a better name for client_label_setcon?


 this change was made.


 Is the double negation in the sentence below intended?


 several comments were changed / moved. There is now one place where te
 behaviour of the different client_label variables are explained.



 sepgsql_set_client_label(), maybe add a comment to !new_label that it is
 reset to the peer label.


 done.


 Is the assert client_label_peer != NULL in sepgsql_get_client_label
 necessary?
 new_label == NULL / pending_label-label == NULL means use the peer label.
 Why not use the peer label instead?


 It turned out that pending_label-label is invariantly non null. Changed
 code to assume that and added some Asserts.



 set_label: if new_label == current label according to getcon, is it
 necessary to add to the pending list?


 this question still remains. Maybe there would be reasons to hit selinux
 with the question: can I change from A-A.


 sepgsql_subxact_callback(), could this be made easier to read by just
 taking llast(client_label_pending), assert that plabel-subid == mySubId and
 then list_delete on pointer of that listcell?


 no this was a naieve suggestion, which fails in any case of a subtransaction
 with not exactly one call to sepgsql_setcon()


 Some comments contain typos, I can spend some time on this, though I'm not
 a native english speaker so it won't be perfect.


 sgml documentation must still be added. If time permits I can spend some
 time on that tomorrow.


 regards,
 Yeb Havinga


 --
 Yeb Havinga
 http://www.mgrid.net/
 Mastering Medical Data




-- 
KaiGai Kohei kai...@kaigai.gr.jp


pgsql-v9.2-sepgsql-setcon.part-2.v4.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-03-03 Thread Yeb Havinga

On 2012-02-24 17:25, Yeb Havinga wrote:

On 2012-02-23 12:17, Kohei KaiGai wrote:

2012/2/20 Yeb Havingayebhavi...@gmail.com:

On 2012-02-05 10:09, Kohei KaiGai wrote:
The attached part-1 patch moves related routines from hooks.c to 
label.c
because of references to static variables. The part-2 patch 
implements above

mechanism.


I took a short look at this patch but am stuck getting the 
regression test

to run properly.

First, patch 2 misses the file sepgsql.sql.in and therefore the 
creation

function command for sepgsql_setcon is missing.


Thanks for your comments.

I added the definition of sepgsql_setcon function to sepgsql.sql.in 
file,

in addition to patch rebasing.


Very brief comments due to must leave keyboard soon:

I read the source code and played a bit with setcon and the debugger, 
no strange things found.


Code comments / questions:


I took the liberty to change a few things, mostly comments, in the 
attached patch:


maybe client_label_committed is a better name for client_label_setcon?


this change was made.


Is the double negation in the sentence below intended?


several comments were changed / moved. There is now one place where te 
behaviour of the different client_label variables are explained.




sepgsql_set_client_label(), maybe add a comment to !new_label that it 
is reset to the peer label.


done.



Is the assert client_label_peer != NULL in sepgsql_get_client_label 
necessary?
new_label == NULL / pending_label-label == NULL means use the peer 
label. Why not use the peer label instead?


It turned out that pending_label-label is invariantly non null. Changed 
code to assume that and added some Asserts.




set_label: if new_label == current label according to getcon, is it 
necessary to add to the pending list?


this question still remains. Maybe there would be reasons to hit selinux 
with the question: can I change from A-A.


sepgsql_subxact_callback(), could this be made easier to read by just 
taking llast(client_label_pending), assert that plabel-subid == 
mySubId and then list_delete on pointer of that listcell?


no this was a naieve suggestion, which fails in any case of a 
subtransaction with not exactly one call to sepgsql_setcon()


Some comments contain typos, I can spend some time on this, though I'm 
not a native english speaker so it won't be perfect.


sgml documentation must still be added. If time permits I can spend some 
time on that tomorrow.


regards,
Yeb Havinga


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data

diff --git a/contrib/sepgsql/expected/label.out b/contrib/sepgsql/expected/label.out
index bac169f..e967c7c 100644
--- a/contrib/sepgsql/expected/label.out
+++ b/contrib/sepgsql/expected/label.out
@@ -26,6 +26,11 @@ CREATE FUNCTION f4 () RETURNS text
 AS 'SELECT sepgsql_getcon()'
 LANGUAGE sql;
 SECURITY LABEL ON FUNCTION f4()
+IS 'system_u:object_r:sepgsql_nosuch_trusted_proc_exec_t:s0';
+CREATE FUNCTION f5 (text) RETURNS bool
+	AS 'SELECT sepgsql_setcon($1)'
+LANGUAGE sql;
+SECURITY LABEL ON FUNCTION f5(text)
 IS 'system_u:object_r:sepgsql_regtest_trusted_proc_exec_t:s0';
 --
 -- Tests for default labeling behavior
@@ -100,6 +105,223 @@ SELECT sepgsql_getcon();	-- client's label must be restored
 (1 row)
 
 --
+-- Test for Dynamic Domain Transition
+--
+-- validation of transaction aware dynamic-transition
+SELECT sepgsql_getcon();	-- confirm client privilege
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c25
+(1 row)
+
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c15');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c15
+(1 row)
+
+SELECT sepgsql_setcon(NULL);	-- failed to reset
+ERROR:  SELinux: security policy violation
+SELECT sepgsql_getcon();
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c15
+(1 row)
+
+BEGIN;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c12');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+  sepgsql_getcon  
+--
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c12
+(1 row)
+
+SAVEPOINT svpt_1;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c9');
+ sepgsql_setcon 
+
+ t
+(1 row)
+
+SELECT sepgsql_getcon();
+ sepgsql_getcon  
+-
+ unconfined_u:unconfined_r:unconfined_t:s0:c0.c9
+(1 row)
+
+SAVEPOINT svpt_2;
+SELECT sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c6');
+ sepgsql_setcon 
+
+ t
+(1 

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

2012-02-28 Thread Kohei KaiGai
2012/2/24 Yeb Havinga yebhavi...@gmail.com:
 On 2012-02-24 15:17, Yeb Havinga wrote:

 I don't know what's fishy about the mgrid user and root that causes
 c0.c1023 to be absent.


 more info:

 In shells started in a x environment under Xvnc, id -Z shows the system_u
 and c0.c1023 absent.

 In shells started from the ssh daemon, the id -Z matches what it should be
 according to the seusers file: unconfined_u and c0.c1023 present.

It seems to me the reason why your security label on bash is different from
the expected default one.
Unlike sshd daemon, vncserver does not assign security label on itself
according to the /etc/selinux/targeted/seusers, thus it inherits the label
of system startup script. It is also the reason why you saw system_u
at the head of security context.

I'll report this topic to selinux community to discuss the preferable solution.
Anyway, please use ssh connection for the testing purpose.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-24 Thread Yeb Havinga

On 2012-02-23 12:17, Kohei KaiGai wrote:

2012/2/20 Yeb Havingayebhavi...@gmail.com:

So maybe this is because my start domain is not s0-s0:c0.c1023

However, when trying to run bash or psql in domain
unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 I get permission
denied.

Distribution is FC15, sestatus
SELinux status: enabled
SELinuxfs mount:/selinux
Current mode:   enforcing
Mode from config file:  enforcing
Policy version: 24
Policy from config file:targeted


The default security policy does not permit dynamic domain transition
even if unconfined domain, in contradiction to its name.
(IMO, it is fair enough design to avoid single point of failure like root user.)

The security policy of regression test contains a set of rules to reduce
categories assigned to unconfined domain.
So, could you try the following steps.
1. Build the latest policy
 % make -f /usr/share/selinux/devel/Makefile -C contrib/sepgsql
2. Install the policy module
 % sudo semodule -i contrib/sepgsql/sepgsql-regtest.pp
3. Turn on the sepgsql_regression_test_mode
 % sudo setsebool -P sepgsql_regression_test_mode=1

I believe it allows to switch security label of the client, as long as we try to
reduce categories.


I remember these commands from the sepgsql contrib module documentation 
(though the semodule invocation in the documentation is with -u and the 
setsebool does not have the -P flag). semodule -l showed I had already 
installed version 1.04.


I just repeated all steps with the new patch, and get the same result:

LOG:  SELinux: denied { dyntransition } 
scontext=unconfined_u:unconfined_r:unconfined_t:s0 
tcontext=unconfined_u:unconfined_r:unconfined_t:s0:c0.c15 tclass=process
STATEMENT:  SELECT 
sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c15');


[mgrid@mgfedora sepgsql]$ getsebool sepgsql_regression_test_mode
sepgsql_regression_test_mode -- on
[root@mgfedora sepgsql]# semodule -l | egrep 'pgsql|postgres'
postgresql  1.12.1
sepgsql-regtest 1.04

Do I need Fedora 16 to run it?


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-24 Thread Kohei KaiGai
2012/2/24 Yeb Havinga yebhavi...@gmail.com:
 On 2012-02-23 12:17, Kohei KaiGai wrote:

 2012/2/20 Yeb Havingayebhavi...@gmail.com:

 So maybe this is because my start domain is not s0-s0:c0.c1023

 However, when trying to run bash or psql in domain
 unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 I get permission
 denied.

 Distribution is FC15, sestatus
 SELinux status:                 enabled
 SELinuxfs mount:                /selinux
 Current mode:                   enforcing
 Mode from config file:          enforcing
 Policy version:                 24
 Policy from config file:        targeted

 The default security policy does not permit dynamic domain transition
 even if unconfined domain, in contradiction to its name.
 (IMO, it is fair enough design to avoid single point of failure like root
 user.)

 The security policy of regression test contains a set of rules to reduce
 categories assigned to unconfined domain.
 So, could you try the following steps.
 1. Build the latest policy
     % make -f /usr/share/selinux/devel/Makefile -C contrib/sepgsql
 2. Install the policy module
     % sudo semodule -i contrib/sepgsql/sepgsql-regtest.pp
 3. Turn on the sepgsql_regression_test_mode
     % sudo setsebool -P sepgsql_regression_test_mode=1

 I believe it allows to switch security label of the client, as long as we
 try to
 reduce categories.


 I remember these commands from the sepgsql contrib module documentation
 (though the semodule invocation in the documentation is with -u and the
 setsebool does not have the -P flag). semodule -l showed I had already
 installed version 1.04.

 I just repeated all steps with the new patch, and get the same result:

 LOG:  SELinux: denied { dyntransition }
 scontext=unconfined_u:unconfined_r:unconfined_t:s0
 tcontext=unconfined_u:unconfined_r:unconfined_t:s0:c0.c15 tclass=process
 STATEMENT:  SELECT
 sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c15');

 [mgrid@mgfedora sepgsql]$ getsebool sepgsql_regression_test_mode
 sepgsql_regression_test_mode -- on
 [root@mgfedora sepgsql]# semodule -l | egrep 'pgsql|postgres'
 postgresql      1.12.1
 sepgsql-regtest 1.04

 Do I need Fedora 16 to run it?

Thanks for your continuous testing.

It seems to me you try to expand categories of the client.
The log saids sepgsql_setcon() tries to switch to ...:s0:c0.c15 from ...:s0.
It is not an admitted operations because of increasion of categories.

 LOG:  SELinux: denied { dyntransition }
 scontext=unconfined_u:unconfined_r:unconfined_t:s0
 tcontext=unconfined_u:unconfined_r:unconfined_t:s0:c0.c15 tclass=process

May I see your /etc/selinux/targeted/seusers ?

I think __default__ entry is configured to unconfined_u:s0, instead of
unconfined_u:s0:c0.c1023 as default.

In my environment, it is configured as follows:

  [root@iwashi ~]# cat /etc/selinux/targeted/seusers
  # This file is auto-generated by libsemanage
  # Do not edit directly.

  system_u:system_u:s0-s0:c0.c1023
  root:unconfined_u:s0-s0:c0.c1023
  __default__:unconfined_u:s0-s0:c0.c1023   === (*)

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-24 Thread Yeb Havinga

On 2012-02-24 14:20, Kohei KaiGai wrote:


It seems to me you try to expand categories of the client.
The log saids sepgsql_setcon() tries to switch to ...:s0:c0.c15 from ...:s0.
It is not an admitted operations because of increasion of categories.


Yes I had my eye on the missing c0.c1023 before but couldn't remember 
changing it, so wrongfully assumed that it would be semantically 
equivalent to c0.c1023.

LOG:  SELinux: denied { dyntransition }
scontext=unconfined_u:unconfined_r:unconfined_t:s0
tcontext=unconfined_u:unconfined_r:unconfined_t:s0:c0.c15 tclass=process

May I see your /etc/selinux/targeted/seusers ?

I think __default__ entry is configured to unconfined_u:s0, instead of
unconfined_u:s0:c0.c1023 as default.

In my environment, it is configured as follows:

   [root@iwashi ~]# cat /etc/selinux/targeted/seusers
   # This file is auto-generated by libsemanage
   # Do not edit directly.

   system_u:system_u:s0-s0:c0.c1023
   root:unconfined_u:s0-s0:c0.c1023
   __default__:unconfined_u:s0-s0:c0.c1023=== (*)



[mgrid@mgfedora ~]$ cat /etc/selinux/targeted/seusers
# This file is auto-generated by libsemanage
# Do not edit directly.

system_u:system_u:s0-s0:c0.c1023
root:unconfined_u:s0-s0:c0.c1023
__default__:unconfined_u:s0-s0:c0.c1023

but still

[mgrid@mgfedora ~]$ id -Z
system_u:unconfined_r:unconfined_t:s0
(I changed bash to run in the unconfined_u context before starting the 
regression test)


and

[root@mgfedora targeted]# id -Z
system_u:unconfined_r:unconfined_t:s0

When I created a new test user, it's selinux context showed the c0.c1023 
- I don't know what's fishy about the mgrid user and root that causes 
c0.c1023 to be absent. Maybe I should reinstall this virtual machine. 
After setting the user mgrid on s0-s0:c0.c1023 with


semanage login -a -S targeted -s unconfined_u -r s0-s0:c0.c1023 mgrid

the regression tests pass :-)

test label... ok
test dml  ... ok
test create   ... ok
test misc ... ok

I'll continue reviewing the patch.

--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-24 Thread Yeb Havinga

On 2012-02-24 15:17, Yeb Havinga wrote:
I don't know what's fishy about the mgrid user and root that causes 
c0.c1023 to be absent. 


more info:

In shells started in a x environment under Xvnc, id -Z shows the 
system_u and c0.c1023 absent.


In shells started from the ssh daemon, the id -Z matches what it should 
be according to the seusers file: unconfined_u and c0.c1023 present.


-- Yeb



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-24 Thread Yeb Havinga

On 2012-02-23 12:17, Kohei KaiGai wrote:

2012/2/20 Yeb Havingayebhavi...@gmail.com:

On 2012-02-05 10:09, Kohei KaiGai wrote:

The attached part-1 patch moves related routines from hooks.c to label.c
because of references to static variables. The part-2 patch implements above
mechanism.


I took a short look at this patch but am stuck getting the regression test
to run properly.

First, patch 2 misses the file sepgsql.sql.in and therefore the creation
function command for sepgsql_setcon is missing.


Thanks for your comments.

I added the definition of sepgsql_setcon function to sepgsql.sql.in file,
in addition to patch rebasing.


Very brief comments due to must leave keyboard soon:

I read the source code and played a bit with setcon and the debugger, no 
strange things found.


Code comments / questions:

this comment below is a lie, because setcon is set by 
sepgsql_xact_callback()

maybe client_label_committed is a better name for client_label_setcon?

static char *client_label_setcon= NULL;/* set by sepgsql_setcon() */

Is the double negation in the sentence below intended?

+ * Neither of them has no special state, the security label being 
initialized

+ * at database-logon time shall be returned.

Is the assert client_label_peer != NULL in sepgsql_get_client_label 
necessary?


sepgsql_set_client_label(), maybe add a comment to !new_label that it is 
reset to the peer label.


new_label == NULL / pending_label-label == NULL means use the peer 
label. Why not use the peer label instead?


set_label: if new_label == current label according to getcon, is it 
necessary to add to the pending list?


sepgsql_subxact_callback(), could this be made easier to read by just 
taking llast(client_label_pending), assert that plabel-subid == mySubId 
and then list_delete on pointer of that listcell?


Some comments contain typos, I can spend some time on this, though I'm 
not a native english speaker so it won't be perfect.


regards,
Yeb Havinga

--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-23 Thread Kohei KaiGai
2012/2/20 Yeb Havinga yebhavi...@gmail.com:
 On 2012-02-05 10:09, Kohei KaiGai wrote:

 The attached part-1 patch moves related routines from hooks.c to label.c
 because of references to static variables. The part-2 patch implements above
 mechanism.


 I took a short look at this patch but am stuck getting the regression test
 to run properly.

 First, patch 2 misses the file sepgsql.sql.in and therefore the creation
 function command for sepgsql_setcon is missing.

Thanks for your comments.

I added the definition of sepgsql_setcon function to sepgsql.sql.in file,
in addition to patch rebasing.

 So maybe this is because my start domain is not s0-s0:c0.c1023

 However, when trying to run bash or psql in domain
 unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 I get permission
 denied.

 Distribution is FC15, sestatus
 SELinux status:                 enabled
 SELinuxfs mount:                /selinux
 Current mode:                   enforcing
 Mode from config file:          enforcing
 Policy version:                 24
 Policy from config file:        targeted

The default security policy does not permit dynamic domain transition
even if unconfined domain, in contradiction to its name.
(IMO, it is fair enough design to avoid single point of failure like root user.)

The security policy of regression test contains a set of rules to reduce
categories assigned to unconfined domain.
So, could you try the following steps.
1. Build the latest policy
% make -f /usr/share/selinux/devel/Makefile -C contrib/sepgsql
2. Install the policy module
% sudo semodule -i contrib/sepgsql/sepgsql-regtest.pp
3. Turn on the sepgsql_regression_test_mode
% sudo setsebool -P sepgsql_regression_test_mode=1

I believe it allows to switch security label of the client, as long as we try to
reduce categories.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


pgsql-v9.2-sepgsql-setcon.part-2.v3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-20 Thread Yeb Havinga

On 2012-02-05 10:09, Kohei KaiGai wrote:
The attached part-1 patch moves related routines from hooks.c to 
label.c because of references to static variables. The part-2 patch 
implements above mechanism. 


I took a short look at this patch but am stuck getting the regression 
test to run properly.


First, patch 2 misses the file sepgsql.sql.in and therefore the creation 
function command for sepgsql_setcon is missing.


When that was solved, ./test_psql failed on the message that the psql 
file may not be of object type unconfined_t. Once it was changed to 
bin_t, the result output for the domain transition gives differences on 
this output (the other parts of label.sql were ok) :


--
-- Test for Dynamic Domain Transition
--
-- validation of transaction aware dynamic-transition
/usr/bin/runcon: /usr/local/pgsql/bin/psql: Permission denied
/usr/bin/runcon: /usr/local/pgsql/bin/psql: Permission denied
/usr/bin/runcon: /usr/local/pgsql/bin/psql: Permission denied

However when I connect to the regression database directly, I can 
execute the first setcon command but get
regression=# SELECT 
sepgsql_setcon('unconfined_u:unconfined_r:unconfined_t:s0:c0.c12');

ERROR:  SELinux: security policy violation

logfile shows
LOG:  SELinux: denied { dyntransition } 
scontext=unconfined_u:unconfined_r:unconfined_t:s0 
tcontext=unconfined_u:unconfined_r:unconfined_t:s0:c0.c12 tclass=process


So maybe this is because my start domain is not s0-s0:c0.c1023

However, when trying to run bash or psql in domain 
unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 I get permission 
denied.


Distribution is FC15, sestatus
SELinux status: enabled
SELinuxfs mount:/selinux
Current mode:   enforcing
Mode from config file:  enforcing
Policy version: 24
Policy from config file:targeted

--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-15 Thread Robert Haas
On Sun, Feb 5, 2012 at 4:09 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 The attached part-1 patch moves related routines from hooks.c to
 label.c because of references to static variables.

I have committed this part.  Seems like a better location for that code, anyhow.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-11 Thread Kevin Grittner
Kevin Grittner  wrote:
Tom Lane wrote:
 
 I agree it's a bug that you can do what Kevin's example shows.
 
 I'll look at it and see if I can pull together a patch.
 
Attached.
 
Basically, if a GUC has a check function, this patch causes it to be
run on a RESET just like it is on a SET, to make sure that the
resulting value is valid to set within the context.  Some messages
needed adjustment.  While I was there, I made cod a little more
consistent among related GUCs.
 
I also added a little to the regression tests to cover this.
 
-Kevin




check-reset-v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-04 Thread Kevin Grittner
Tom Lane  wrote:
 
 More to the point, a GUC rollback transition *has to always
 succeed*.  Period.
 
I was about to point out the exception of the transaction_read_only
GUC, which according to the standard must not be changed except at
the beginning of a transaction or a subtransaction, and must not be
changed from on to off in a subtransaction.  Then I noticed that,
while we protect against an explicit change in a prohibited way, we
allow a RESET:
 
test=# begin transaction read only;
BEGIN
test=# select * from x;
 x 
---
 1
(1 row)

test=# set transaction_read_only = off;
ERROR:  transaction read-write mode must be set before any query
test=# rollback;
ROLLBACK
test=# begin transaction read only;
BEGIN
test=# select * from x;
 x 
---
 1
(1 row)

test=# reset transaction_read_only ;
RESET
test=# insert into x VALUES (2);
INSERT 0 1
test=# commit;
COMMIT
 
I think that's a problem.  It could allow back-door violations of
invariants enforced by triggers, and seems to violate the SQL
standard.  I think this should be considered a bug, although I'm not
sure whether it's safe to back-patch, given the change to existing
behavior.
 
Whether such a (required) exception to what you assert above should
open the door to any others is another question.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-04 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Tom Lane  wrote:
 More to the point, a GUC rollback transition *has to always
 succeed*.  Period.

 [ counterexample showing we should sometimes disallow RESET ]

This actually isn't what I was talking about: a RESET statement is a
commanded adoption of a new value that happens to be gotten off the
stack, and there's not a lot of logical difficulty in letting it fail.
What I was concerned about was the case where GUC is trying to
re-establish an old value during transaction rollback.  For example,
assume we are superuser to start with, and we do

begin;
set role unprivileged_user;
...
rollback;

The rollback needs to transition the role setting back to superuser.
A check based purely on allowed transitions would disallow that, since
it's not visibly different from the unprivileged_user trying to make
himself superuser.  You have to take the context of the state change
into account.

And yeah, I agree it's a bug that you can do what Kevin's example shows.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-02-04 Thread Kevin Grittner
Tom Lane  wrote:
 
 What I was concerned about was the case where GUC is trying to
 re-establish an old value during transaction rollback. For example,
 assume we are superuser to start with, and we do
 
 begin;
 set role unprivileged_user;
 ...
 rollback;
 
 The rollback needs to transition the role setting back to
 superuser.
 
Sorry for missing the point.  Yeah, I totally agree with that.
 
 And yeah, I agree it's a bug that you can do what Kevin's example
 shows.
 
I'll look at it and see if I can pull together a patch.
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-31 Thread Yeb Havinga

On 2012-01-30 19:19, Robert Haas wrote:

On Sun, Jan 29, 2012 at 7:27 AM, Kohei KaiGaikai...@kaigai.gr.jp  wrote:

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.)
Of course, we already have a trusted procedure mechanism which is 
intended to support temporary changes to the effective security label, 
so you might say, hey, people shouldn't do that. But they might. And I 
wouldn't like to bet that's the only case that could be problematic 
either. What about a setting in postgresql.conf? You could end up 
being asked to set the security label to some other value before 
you've initialized it. What about SET LOCAL? It's not OK to fail to 
revert that at transaction exit. Hence my suggestion of a function: if 
you use functions, you can implement whatever semantics you want. 


What about always allowing a transition to the default / postgresql.conf 
configured client label, so in the case of errors, or RESET, the 
transition to this domain is hardcoded to succeed. This would also be 
useful in conjunction with a connection pooler. This would still allow 
the option to prevent a back transition to non-default client labels.


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-31 Thread Robert Haas
On Tue, Jan 31, 2012 at 4:36 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 What about always allowing a transition to the default / postgresql.conf
 configured client label, so in the case of errors, or RESET, the transition
 to this domain is hardcoded to succeed. This would also be useful in
 conjunction with a connection pooler. This would still allow the option to
 prevent a back transition to non-default client labels.

I don't think you can make that work, because someone can still
attempt to RESET to a different value, and it's still not safe to make
that fail.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-31 Thread Yeb Havinga

On 2012-01-31 14:06, Robert Haas wrote:

On Tue, Jan 31, 2012 at 4:36 AM, Yeb Havingayebhavi...@gmail.com  wrote:

What about always allowing a transition to the default / postgresql.conf
configured client label, so in the case of errors, or RESET, the transition
to this domain is hardcoded to succeed. This would also be useful in
conjunction with a connection pooler. This would still allow the option to
prevent a back transition to non-default client labels.

I don't think you can make that work, because someone can still
attempt to RESET to a different value, and it's still not safe to make
that fail.


But the idea is that if that different value is a (possibly changed into 
a new) allowed default value, a RESET to that new default value will be 
allowed, by definition, because it is a default value.



--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-31 Thread Robert Haas
On Tue, Jan 31, 2012 at 9:10 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 On 2012-01-31 14:06, Robert Haas wrote:
 On Tue, Jan 31, 2012 at 4:36 AM, Yeb Havingayebhavi...@gmail.com  wrote:

 What about always allowing a transition to the default / postgresql.conf
 configured client label, so in the case of errors, or RESET, the
 transition
 to this domain is hardcoded to succeed. This would also be useful in
 conjunction with a connection pooler. This would still allow the option
 to
 prevent a back transition to non-default client labels.

 I don't think you can make that work, because someone can still
 attempt to RESET to a different value, and it's still not safe to make
 that fail.

 But the idea is that if that different value is a (possibly changed into a
 new) allowed default value, a RESET to that new default value will be
 allowed, by definition, because it is a default value.

*scratches head*

I'm not sure I follow you.  If you're saying that we can make this
work by always allowing the value to be reset, then I agree with you,
but I'm not sure those are the semantics KaiGai wants.  For instance,
if a connection pooler does:

SET sepgsql.client_label = 'bob_t';

...and then hands off to the client, the client can then do:

RESET sepgsql.client_label;
SET sepgsql.client_label = 'alice_t';

and that's bad.  More generally, the system security policy is
designed to answer questions about whether it's OK to transition from
A-B, and the fact that A-B is OK does not mean that B-A is OK, but
our GUC mechanism pretty much forces you to allow both of those
things, or neither.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-31 Thread Yeb Havinga

On 2012-01-31 15:28, Robert Haas wrote:


*scratches head*

I'm not sure I follow you.  If you're saying that we can make this
work by always allowing the value to be reset, then I agree with you,
but I'm not sure those are the semantics KaiGai wants.  For instance,
if a connection pooler does:

SET sepgsql.client_label = 'bob_t';

...and then hands off to the client, the client can then do:

RESET sepgsql.client_label;
SET sepgsql.client_label = 'alice_t';

and that's bad.


Hmm yes this is a problem. Reading the original post better, it is also 
not the intended behaviour to support repeatable client_label switches.


However, single-directed domain transition from bigger-privileges to 
smaller-privileged domain by users' operation is also supported on 
operating system, and useful feature to restrict applications capability 
at beginning of the session.


--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-31 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 and that's bad.  More generally, the system security policy is
 designed to answer questions about whether it's OK to transition from
 A-B, and the fact that A-B is OK does not mean that B-A is OK, but
 our GUC mechanism pretty much forces you to allow both of those
 things, or neither.

More to the point, a GUC rollback transition *has to always succeed*.
Period.  Now, the value that it's trying to roll back to was presumably
considered legitimate at some previous time, but if you're designing a
system that is based purely on state transitions it could very well see
the rollback transition as invalid.  That is just going to be too
fragile to be acceptable.

I think that this will have to be set up so that it understands the
difference between a forward transition and a rollback and only checks
the former.  If that's not possible, this is not going to get in.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-30 Thread Robert Haas
On Sun, Jan 29, 2012 at 7:27 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/1/28 Kohei KaiGai kai...@kaigai.gr.jp:
 2012/1/26 Robert Haas robertmh...@gmail.com:
 On Thu, Jan 26, 2012 at 2:07 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/1/26 Robert Haas robertmh...@gmail.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?

Well, I think the idea that GUC changes can be reverted at any time is
fairly deeply ingrained in the GUC machinery.  So I think that's a
hack: it might happen to work today (or at least on the cases we can
think of to test), but I wouldn't be a bit surprised if there are
other cases where it fails, either now or in the future.  I don't see
any good reason to assume that unrevertable changes are OK even inside
a transaction context.  For example, if someone applies a context to a
function with ALTER FUNCTION, they're going to expect that the altered
context remains in effect while the function is running and gets
reverted on exit.  If you throw an error when the system tries to
revert the change at function-exit time, it might not crash the
server, but it's certainly going to violate the principle of least
astonishment.

Of course, we already have a trusted procedure mechanism which is
intended to support temporary changes to the effective security label,
so you might say, hey, people shouldn't do that.  But they might.  And
I wouldn't like to bet that's the only case that could be problematic
either.  What about a setting in postgresql.conf?  You could end up
being asked to set the security label to some other value before
you've initialized it.  What about SET LOCAL?  It's not OK to fail to
revert that at transaction exit.

Hence my suggestion of a function: if you use functions, you can
implement whatever semantics you want.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-29 Thread Kohei KaiGai
2012/1/28 Kohei KaiGai kai...@kaigai.gr.jp:
 2012/1/26 Robert Haas robertmh...@gmail.com:
 On Thu, Jan 26, 2012 at 2:07 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/1/26 Robert Haas robertmh...@gmail.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 kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-28 Thread Kohei KaiGai
2012/1/26 Robert Haas robertmh...@gmail.com:
 On Thu, Jan 26, 2012 at 2:07 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/1/26 Robert Haas robertmh...@gmail.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.

So, I'll try to modify the patch with an idea to use a function.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-26 Thread Robert Haas
On Tue, Jan 10, 2012 at 6:28 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 This patch adds a new GUC sepgsql.client_label that allows client
 process to switch its privileges into another one, as long as the
 system security policy admits this transition.
 Because of this feature, I ported two permissions from process class
 of SELinux; setcurrent and dyntransition. The first one checks
 whether the client has a right to switch its privilege. And the other
 one checks a particular transition path from X to Y.

 This feature might seem to break assumption of the sepgsql's security
 model. However, single-directed domain transition from
 bigger-privileges to smaller-privileged domain by users' operation is
 also supported on operating system, and useful feature to restrict
 applications capability at beginning of the session.

 A few weeks ago, I got a requirement from Joshua Brindle. He is
 working for Web-application that uses CAC (Common Access Card) for its
 authentication, and wanted to integrate its security credential and
 security label of selinux/sepgsql.
 One problem was the system environment unavailable to use
 labeled-networking (IPsec), thus, it was not an option to switch the
 security label of processes on web-server side. An other solution is
 to port dynamic-transition feature into sepgsql, as an analogy of
 operating system.

 An expected scenario is below:
 The web-server is running with WEBSERV domain. It is allowed to
 connect to PostgreSQL, and also allowed to invoke an trusted-procedure
 that takes an argument of security-credential within CAC, but, nothing
 else are allowed.
 The trusted-procedure is allowed to reference a table between
 security-credential and security-label to be assigned on, then it
 switches the security label of client into CLIENT_n.
 The CLIENT_n shall be allowed to access tables, functions and others
 according to the security policy, and also allowed to reset
 sepgsql.security_label to revert WEBSERV. However, he is not
 available to switch other domain without security-credential stored
 within CAC card.

 I and Joshua agreed this scenario is reasonable and secure.
 So, we'd like to suggest this new feature towards v9.2 timeline.

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.

On another note, this is an awfully large patch.  Is there a separate
patch here that just does code rearrangement that should be separated
out?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-26 Thread Kohei KaiGai
2012/1/26 Robert Haas robertmh...@gmail.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?

 On another note, this is an awfully large patch.  Is there a separate
 patch here that just does code rearrangement that should be separated
 out?

OK. I moved some routines related to client_label into label.c.
I'll separate this part from the new functionality part.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


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

2012-01-26 Thread Robert Haas
On Thu, Jan 26, 2012 at 2:07 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2012/1/26 Robert Haas robertmh...@gmail.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?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] [v9.2] Add GUC sepgsql.client_label

2012-01-10 Thread Kohei KaiGai
This patch adds a new GUC sepgsql.client_label that allows client
process to switch its privileges into another one, as long as the
system security policy admits this transition.
Because of this feature, I ported two permissions from process class
of SELinux; setcurrent and dyntransition. The first one checks
whether the client has a right to switch its privilege. And the other
one checks a particular transition path from X to Y.

This feature might seem to break assumption of the sepgsql's security
model. However, single-directed domain transition from
bigger-privileges to smaller-privileged domain by users' operation is
also supported on operating system, and useful feature to restrict
applications capability at beginning of the session.

A few weeks ago, I got a requirement from Joshua Brindle. He is
working for Web-application that uses CAC (Common Access Card) for its
authentication, and wanted to integrate its security credential and
security label of selinux/sepgsql.
One problem was the system environment unavailable to use
labeled-networking (IPsec), thus, it was not an option to switch the
security label of processes on web-server side. An other solution is
to port dynamic-transition feature into sepgsql, as an analogy of
operating system.

An expected scenario is below:
The web-server is running with WEBSERV domain. It is allowed to
connect to PostgreSQL, and also allowed to invoke an trusted-procedure
that takes an argument of security-credential within CAC, but, nothing
else are allowed.
The trusted-procedure is allowed to reference a table between
security-credential and security-label to be assigned on, then it
switches the security label of client into CLIENT_n.
The CLIENT_n shall be allowed to access tables, functions and others
according to the security policy, and also allowed to reset
sepgsql.security_label to revert WEBSERV. However, he is not
available to switch other domain without security-credential stored
within CAC card.

I and Joshua agreed this scenario is reasonable and secure.
So, we'd like to suggest this new feature towards v9.2 timeline.

Thanks,

[*1] CAC - Common Access Card
http://en.wikipedia.org/wiki/Common_Access_Card
-- 
KaiGai Kohei kai...@kaigai.gr.jp


pgsql-v9.2-guc-sepgsql.client_label.v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers