Re: [Freeipa-devel] [PATCH] 313 Validate SELinux users in config-mod

2012-09-27 Thread Martin Kosek 
On 09/26/2012 08:31 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 On 09/26/2012 12:32 PM, Petr Viktorin wrote:
 On 09/26/2012 12:25 PM, Petr Viktorin wrote:

 I found strange behavior in validate_selinuxuser. Perhaps it's material
 for another ticket. This command passes validation:

 $ ./ipa config_mod
 --ipaselinuxusermapdefault=unconfined_u:s0-s0:c0.c1023
 --ipaselinuxusermaporder='unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c4,c4:→Why


 is stuff allowed here?'
 [...]
 SELinux user map order:
 unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c3.c8,c4:→Why is stuff
 allowed here?
 Default SELinux user: unconfined_u:s0-s0:c0.c1023
 PAC type: MS-PAC


 Obviously extra info should not be allowed.
 Is s5-s1 or c4.c3 valid? Can the first value be higher than the second?
 AFAIK (I'm not an expert though), MCS doesn't allow dashes, so c0-c4
 should not be allowed. Chains like c1.c2.c3 also don't look right.


 ... Also, the MLS/MCS numeric limits are not enforced correctly:
 xguest_u:s92:c9,c0 passes.




 Right. We can create a ticket to harden the validation if we want. So far, 
 the
 purpose of this ticket/patch is to make validation of config values 
 consistent
 with selinuxusermap plugin. I will let Rob to chime in, but I would keep this
 patch as is.

 Martin

 
 Yes, please open another ticket for the validation issues.
 
 rob

https://fedorahosted.org/freeipa/ticket/3119

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 313 Validate SELinux users in config-mod

2012-09-27 Thread Petr Viktorin 

On 09/27/2012 09:59 AM, Martin Kosek wrote:

On 09/26/2012 08:31 PM, Rob Crittenden wrote:

Martin Kosek wrote:

On 09/26/2012 12:32 PM, Petr Viktorin wrote:

On 09/26/2012 12:25 PM, Petr Viktorin wrote:


I found strange behavior in validate_selinuxuser. Perhaps it's material
for another ticket. This command passes validation:

$ ./ipa config_mod
--ipaselinuxusermapdefault=unconfined_u:s0-s0:c0.c1023
--ipaselinuxusermaporder='unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c4,c4:→Why


is stuff allowed here?'
[...]
 SELinux user map order:
unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c3.c8,c4:→Why is stuff
allowed here?
 Default SELinux user: unconfined_u:s0-s0:c0.c1023
 PAC type: MS-PAC




Obviously extra info should not be allowed.
Is s5-s1 or c4.c3 valid? Can the first value be higher than the second?
AFAIK (I'm not an expert though), MCS doesn't allow dashes, so c0-c4
should not be allowed. Chains like c1.c2.c3 also don't look right.



... Also, the MLS/MCS numeric limits are not enforced correctly:
xguest_u:s92:c9,c0 passes.





Right. We can create a ticket to harden the validation if we want. So far, the
purpose of this ticket/patch is to make validation of config values consistent
with selinuxusermap plugin. I will let Rob to chime in, but I would keep this
patch as is.

Martin



Yes, please open another ticket for the validation issues.

rob


https://fedorahosted.org/freeipa/ticket/3119

Martin



ACK for the patch

--
Petr³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 313 Validate SELinux users in config-mod

2012-09-27 Thread Martin Kosek 
On 09/27/2012 10:42 AM, Petr Viktorin wrote:
 On 09/27/2012 09:59 AM, Martin Kosek wrote:
 On 09/26/2012 08:31 PM, Rob Crittenden wrote:
 Martin Kosek wrote:
 On 09/26/2012 12:32 PM, Petr Viktorin wrote:
 On 09/26/2012 12:25 PM, Petr Viktorin wrote:

 I found strange behavior in validate_selinuxuser. Perhaps it's material
 for another ticket. This command passes validation:

 $ ./ipa config_mod
 --ipaselinuxusermapdefault=unconfined_u:s0-s0:c0.c1023
 --ipaselinuxusermaporder='unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c4,c4:→Why



 is stuff allowed here?'
 [...]
  SELinux user map order:
 unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c3.c8,c4:→Why is stuff
 allowed here?
  Default SELinux user: unconfined_u:s0-s0:c0.c1023
  PAC type: MS-PAC


 Obviously extra info should not be allowed.
 Is s5-s1 or c4.c3 valid? Can the first value be higher than the 
 second?
 AFAIK (I'm not an expert though), MCS doesn't allow dashes, so c0-c4
 should not be allowed. Chains like c1.c2.c3 also don't look right.


 ... Also, the MLS/MCS numeric limits are not enforced correctly:
 xguest_u:s92:c9,c0 passes.




 Right. We can create a ticket to harden the validation if we want. So far, 
 the
 purpose of this ticket/patch is to make validation of config values 
 consistent
 with selinuxusermap plugin. I will let Rob to chime in, but I would keep 
 this
 patch as is.

 Martin


 Yes, please open another ticket for the validation issues.

 rob

 https://fedorahosted.org/freeipa/ticket/3119

 Martin

 
 ACK for the patch
 

Pushed to master, ipa-3-0.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 313 Validate SELinux users in config-mod

2012-09-26 Thread Rob Crittenden 

Martin Kosek wrote:

On 09/26/2012 12:32 PM, Petr Viktorin wrote:

On 09/26/2012 12:25 PM, Petr Viktorin wrote:


I found strange behavior in validate_selinuxuser. Perhaps it's material
for another ticket. This command passes validation:

$ ./ipa config_mod
--ipaselinuxusermapdefault=unconfined_u:s0-s0:c0.c1023
--ipaselinuxusermaporder='unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c4,c4:→Why

is stuff allowed here?'
[...]
SELinux user map order:
unconfined_u:s0-s0:c0.c1023$xguest_u:s5-s1:c0-c4.c3.c8,c4:→Why is stuff
allowed here?
Default SELinux user: unconfined_u:s0-s0:c0.c1023
PAC type: MS-PAC




Obviously extra info should not be allowed.
Is s5-s1 or c4.c3 valid? Can the first value be higher than the second?
AFAIK (I'm not an expert though), MCS doesn't allow dashes, so c0-c4
should not be allowed. Chains like c1.c2.c3 also don't look right.



... Also, the MLS/MCS numeric limits are not enforced correctly:
xguest_u:s92:c9,c0 passes.





Right. We can create a ticket to harden the validation if we want. So far, the
purpose of this ticket/patch is to make validation of config values consistent
with selinuxusermap plugin. I will let Rob to chime in, but I would keep this
patch as is.

Martin



Yes, please open another ticket for the validation issues.

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 313 Validate SELinux users in config-mod

2012-09-25 Thread Martin Kosek 
config-mod is capable of changing default SELinux user map order
and a default SELinux user. Validate the new config values to
prevent bogus default SELinux users to be assigned to IPA users.

https://fedorahosted.org/freeipa/ticket/2993

---
Note: I removed the previous validate construct:
-validate = dict(options)
-validate.update(entry_attrs)
... as entry_attrs contains both values set via standard options and *attr.

Martin
From 296eedc7cfd258b9e5eaf4f182b1a9625f5bf1a1 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 25 Sep 2012 13:46:56 +0200
Subject: [PATCH] Validate SELinux users in config-mod

config-mod is capable of changing default SELinux user map order
and a default SELinux user. Validate the new config values to
prevent bogus default SELinux users to be assigned to IPA users.

https://fedorahosted.org/freeipa/ticket/2993
---
 ipalib/plugins/config.py| 49 +
 tests/test_xmlrpc/test_config_plugin.py | 44 -
 2 files changed, 69 insertions(+), 24 deletions(-)

diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
index e02519d5759f4e4a6d6a7075fe896f8b2e69b451..1c62e0d942231fac442ee2c1f31431003c08e283 100644
--- a/ipalib/plugins/config.py
+++ b/ipalib/plugins/config.py
@@ -21,6 +21,7 @@
 from ipalib import api
 from ipalib import Bool, Int, Str, IA5Str, StrEnum, DNParam
 from ipalib.plugins.baseldap import *
+from ipalib.plugins.selinuxusermap import validate_selinuxuser
 from ipalib import _
 from ipalib.errors import ValidationError
 
@@ -258,30 +259,44 @@ class config_mod(LDAPUpdate):
 error=_('%(obj)s default attribute %(attr)s would not be allowed!') \
 % dict(obj=obj, attr=obj_attr))
 
-# Combine the current entry and options into a single object to
-# evaluate. This covers changes via setattr and options.
-# Note: this is not done in a validator because we may be changing
-#   the default user and map list at the same time and we don't
-#   have both values in a validator.
-validate = dict(options)
-validate.update(entry_attrs)
-if ('ipaselinuxusermapdefault' in validate or
-  'ipaselinuxusermaporder' in validate):
+if ('ipaselinuxusermapdefault' in entry_attrs or
+  'ipaselinuxusermaporder' in entry_attrs):
 config = None
 failedattr = 'ipaselinuxusermaporder'
-if 'ipaselinuxusermapdefault' in validate:
-defaultuser = validate['ipaselinuxusermapdefault']
+
+if 'ipaselinuxusermapdefault' in entry_attrs:
+defaultuser = entry_attrs['ipaselinuxusermapdefault']
 failedattr = 'ipaselinuxusermapdefault'
+
+# validate the new default user first
+if defaultuser is not None:
+error_message = validate_selinuxuser(_, defaultuser)
+
+if error_message:
+raise errors.ValidationError(name='ipaselinuxusermapdefault',
+error=error_message)
+
 else:
 config = ldap.get_ipa_config()[1]
-if 'ipaselinuxusermapdefault' in config:
-defaultuser = config['ipaselinuxusermapdefault'][0]
-else:
-defaultuser = None
+defaultuser = config.get('ipaselinuxusermapdefault', [None])[0]
 
-if 'ipaselinuxusermaporder' in validate:
-order = validate['ipaselinuxusermaporder']
+if 'ipaselinuxusermaporder' in entry_attrs:
+order = entry_attrs['ipaselinuxusermaporder']
 userlist = order.split('$')
+
+# validate the new user order first
+for user in userlist:
+if not user:
+raise errors.ValidationError(name='ipaselinuxusermaporder',
+error=_('A list of SELinux users delimited by $ expected'))
+
+error_message = validate_selinuxuser(_, user)
+if error_message:
+error_message = _(SELinux user '%(user)s' is not 
+valid: %(error)s) % dict(user=user,
+  error=error_message)
+raise errors.ValidationError(name='ipaselinuxusermaporder',
+error=error_message)
 else:
 if not config:
 config = ldap.get_ipa_config()[1]
diff --git a/tests/test_xmlrpc/test_config_plugin.py b/tests/test_xmlrpc/test_config_plugin.py
index 6d83f047e0e647270712003d77c40f3c1014f90f..3d9a31daf38b4b8a28febee90f5812cf78538ee7 100644
--- a/tests/test_xmlrpc/test_config_plugin.py
+++ b/tests/test_xmlrpc/test_config_plugin.py
@@