Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule

2012-09-12 Thread Tomas Babej 

On 09/11/2012 01:14 PM, Martin Kosek wrote:

On 09/06/2012 01:13 PM, Tomas Babej wrote:

On 09/05/2012 01:56 PM, Martin Kosek wrote:

On 09/03/2012 05:12 PM, Tomas Babej wrote:

Hi,

Both selinuxusermap-add and selinuxusermap-mod commands now behave
consistently in not allowing user/host category or user/host members
and HBAC rule being set at the same time. Also adds a bunch of unit
tests that check this behaviour.

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

Tomas


I found few issues with this patch:

1) Patch needs a rebase

2) Patch does not expect attributes to be set to None, i.e. to be left empty or
to be deleted, e.g.:

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all

Added SELinux User Map foo

Rule name: foo
SELinux User: guest_u:s0
User category: all
Enabled: TRUE

# ipa selinuxusermap-mod foo --usercat= --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-mod foo --usercat=
---
Modified SELinux User Map foo
---
Rule name: foo
SELinux User: guest_u:s0
Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule=foo
---
Modified SELinux User Map foo
---
Rule name: foo
SELinux User: guest_u:s0
HBAC Rule: foo
Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule= --usercat=all
ipa: ERROR: HBAC rule and local members cannot both be set

All these validation failures are not valid.

3) Additionally, I think it would be more readable and less error prone that if
instead of this blob:

+are_local_members_to_be_set  = 'usercategory' in _entry_attrs or \
+   'hostcategory' in _entry_attrs or \
+   'memberuser' in _entry_attrs or \
+   'memberhost' in _entry_attrs

You would use something like that:

are_local_members_to_be_set  = any(attr in _entry_attrs
 for attr in ('usercategory',
  'hostcategory',
  'memberuser',
  'memberhost'))

Martin

1.) Done.
2.) Corrected.
3.) Fixed.

Tomas

1) There are some (corner) cases where this approach still does not work:

# ipa selinuxusermap-show foo
   Rule name: foo
   SELinux User: guest_u:s0
   HBAC Rule: foo
   Enabled: TRUE
# ipa selinuxusermap-mod foo --usercat=all --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

HBAC rule attribute is being deleted and user category set, so this should not
be rejected.

2) There are also some styling issues (you can use pep8 tool present in Fedora
to locate them on your own, e.g.:

ipalib/plugins/selinuxusermap.py:247:32: E203 whitespace before ':'
ipalib/plugins/selinuxusermap.py:247:70: E225 missing whitespace around operator
ipalib/plugins/selinuxusermap.py:249:36: E221 multiple spaces before operator
...

Martin

The corner case is fixed now and styling issues corrected as well.

Tomas
From 003e340bceb2bbae614f07edf1dd3d25d1f1ac23 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 6 Sep 2012 07:03:42 -0400
Subject: [PATCH] Make sure selinuxusemap behaves consistently to HBAC rule

Both selinuxusermap-add and selinuxusermap-mod commands now behave
consistently in not allowing user/host category or user/host members
and HBAC rule being set at the same time. Also adds a bunch of unit
tests that check this behaviour.

https://fedorahosted.org/freeipa/ticket/2983
---
 ipalib/plugins/selinuxusermap.py|  76 +++---
 tests/test_xmlrpc/test_selinuxusermap_plugin.py | 179 
 2 files changed, 237 insertions(+), 18 deletions(-)

diff --git a/ipalib/plugins/selinuxusermap.py b/ipalib/plugins/selinuxusermap.py
index 13bbb58ec0e6b7bd4275be17198c7452090a0781..32c55850b7d5b78f39cfae8960b8588a35b30251 100644
--- a/ipalib/plugins/selinuxusermap.py
+++ b/ipalib/plugins/selinuxusermap.py
@@ -70,6 +70,7 @@ SEEALSO:
 
 notboth_err = _('HBAC rule and local members cannot both be set')
 
+
 def validate_selinuxuser(ugettext, user):
 
 An SELinux user has 3 components: user:MLS:MCS. user and MLS are required.
@@ -91,7 +92,7 @@ def validate_selinuxuser(ugettext, user):
 
 # If we add in ::: we don't have to check to see if some values are
 # empty
-(name, mls, mcs, ignore) = (user + ':::').split(':',3)
+(name, mls, mcs, ignore) = (user + ':::').split(':', 3)
 
 if not regex_name.match(name):
 return _('Invalid SELinux user name, only a-Z and _ are allowed')
@@ -99,10 +100,12 @@ def validate_selinuxuser(ugettext, user):

Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule

2012-09-12 Thread Martin Kosek 
On 09/12/2012 10:24 AM, Tomas Babej wrote:
 On 09/11/2012 01:14 PM, Martin Kosek wrote:
 On 09/06/2012 01:13 PM, Tomas Babej wrote:
 On 09/05/2012 01:56 PM, Martin Kosek wrote:
 On 09/03/2012 05:12 PM, Tomas Babej wrote:
 Hi,

 Both selinuxusermap-add and selinuxusermap-mod commands now behave
 consistently in not allowing user/host category or user/host members
 and HBAC rule being set at the same time. Also adds a bunch of unit
 tests that check this behaviour.

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

 Tomas

 I found few issues with this patch:

 1) Patch needs a rebase

 2) Patch does not expect attributes to be set to None, i.e. to be left
 empty or
 to be deleted, e.g.:

 # ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all
 --hbacrule=
 ipa: ERROR: HBAC rule and local members cannot both be set

 # ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all
 
 Added SELinux User Map foo
 
 Rule name: foo
 SELinux User: guest_u:s0
 User category: all
 Enabled: TRUE

 # ipa selinuxusermap-mod foo --usercat= --hbacrule=
 ipa: ERROR: HBAC rule and local members cannot both be set

 # ipa selinuxusermap-mod foo --usercat=
 ---
 Modified SELinux User Map foo
 ---
 Rule name: foo
 SELinux User: guest_u:s0
 Enabled: TRUE

 # ipa selinuxusermap-mod foo --hbacrule=foo
 ---
 Modified SELinux User Map foo
 ---
 Rule name: foo
 SELinux User: guest_u:s0
 HBAC Rule: foo
 Enabled: TRUE

 # ipa selinuxusermap-mod foo --hbacrule= --usercat=all
 ipa: ERROR: HBAC rule and local members cannot both be set

 All these validation failures are not valid.

 3) Additionally, I think it would be more readable and less error prone
 that if
 instead of this blob:

 +are_local_members_to_be_set  = 'usercategory' in _entry_attrs or \
 +   'hostcategory' in _entry_attrs or \
 +   'memberuser' in _entry_attrs or \
 +   'memberhost' in _entry_attrs

 You would use something like that:

 are_local_members_to_be_set  = any(attr in _entry_attrs
  for attr in ('usercategory',
   'hostcategory',
   'memberuser',
   'memberhost'))

 Martin
 1.) Done.
 2.) Corrected.
 3.) Fixed.

 Tomas
 1) There are some (corner) cases where this approach still does not work:

 # ipa selinuxusermap-show foo
Rule name: foo
SELinux User: guest_u:s0
HBAC Rule: foo
Enabled: TRUE
 # ipa selinuxusermap-mod foo --usercat=all --hbacrule=
 ipa: ERROR: HBAC rule and local members cannot both be set

 HBAC rule attribute is being deleted and user category set, so this should 
 not
 be rejected.

 2) There are also some styling issues (you can use pep8 tool present in 
 Fedora
 to locate them on your own, e.g.:

 ipalib/plugins/selinuxusermap.py:247:32: E203 whitespace before ':'
 ipalib/plugins/selinuxusermap.py:247:70: E225 missing whitespace around 
 operator
 ipalib/plugins/selinuxusermap.py:249:36: E221 multiple spaces before operator
 ...

 Martin
 The corner case is fixed now and styling issues corrected as well.
 
 Tomas

Yup, works fine now.

ACK. 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 0011] Make sure selinuxusemap behaves consistently to HBAC rule

2012-09-11 Thread Martin Kosek 
On 09/06/2012 01:13 PM, Tomas Babej wrote:
 On 09/05/2012 01:56 PM, Martin Kosek wrote:
 On 09/03/2012 05:12 PM, Tomas Babej wrote:
 Hi,

 Both selinuxusermap-add and selinuxusermap-mod commands now behave
 consistently in not allowing user/host category or user/host members
 and HBAC rule being set at the same time. Also adds a bunch of unit
 tests that check this behaviour.

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

 Tomas

 I found few issues with this patch:

 1) Patch needs a rebase

 2) Patch does not expect attributes to be set to None, i.e. to be left empty 
 or
 to be deleted, e.g.:

 # ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all 
 --hbacrule=
 ipa: ERROR: HBAC rule and local members cannot both be set

 # ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all
 
 Added SELinux User Map foo
 
Rule name: foo
SELinux User: guest_u:s0
User category: all
Enabled: TRUE

 # ipa selinuxusermap-mod foo --usercat= --hbacrule=
 ipa: ERROR: HBAC rule and local members cannot both be set

 # ipa selinuxusermap-mod foo --usercat=
 ---
 Modified SELinux User Map foo
 ---
Rule name: foo
SELinux User: guest_u:s0
Enabled: TRUE

 # ipa selinuxusermap-mod foo --hbacrule=foo
 ---
 Modified SELinux User Map foo
 ---
Rule name: foo
SELinux User: guest_u:s0
HBAC Rule: foo
Enabled: TRUE

 # ipa selinuxusermap-mod foo --hbacrule= --usercat=all
 ipa: ERROR: HBAC rule and local members cannot both be set

 All these validation failures are not valid.

 3) Additionally, I think it would be more readable and less error prone that 
 if
 instead of this blob:

 +are_local_members_to_be_set  = 'usercategory' in _entry_attrs or \
 +   'hostcategory' in _entry_attrs or \
 +   'memberuser' in _entry_attrs or \
 +   'memberhost' in _entry_attrs

 You would use something like that:

 are_local_members_to_be_set  = any(attr in _entry_attrs
 for attr in ('usercategory',
  'hostcategory',
  'memberuser',
  'memberhost'))

 Martin
 1.) Done.
 2.) Corrected.
 3.) Fixed.
 
 Tomas

1) There are some (corner) cases where this approach still does not work:

# ipa selinuxusermap-show foo
  Rule name: foo
  SELinux User: guest_u:s0
  HBAC Rule: foo
  Enabled: TRUE
# ipa selinuxusermap-mod foo --usercat=all --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

HBAC rule attribute is being deleted and user category set, so this should not
be rejected.

2) There are also some styling issues (you can use pep8 tool present in Fedora
to locate them on your own, e.g.:

ipalib/plugins/selinuxusermap.py:247:32: E203 whitespace before ':'
ipalib/plugins/selinuxusermap.py:247:70: E225 missing whitespace around operator
ipalib/plugins/selinuxusermap.py:249:36: E221 multiple spaces before operator
...

Martin

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

Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule

2012-09-06 Thread Tomas Babej 

On 09/05/2012 01:56 PM, Martin Kosek wrote:

On 09/03/2012 05:12 PM, Tomas Babej wrote:

Hi,

Both selinuxusermap-add and selinuxusermap-mod commands now behave
consistently in not allowing user/host category or user/host members
and HBAC rule being set at the same time. Also adds a bunch of unit
tests that check this behaviour.

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

Tomas


I found few issues with this patch:

1) Patch needs a rebase

2) Patch does not expect attributes to be set to None, i.e. to be left empty or
to be deleted, e.g.:

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all

Added SELinux User Map foo

   Rule name: foo
   SELinux User: guest_u:s0
   User category: all
   Enabled: TRUE

# ipa selinuxusermap-mod foo --usercat= --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-mod foo --usercat=
---
Modified SELinux User Map foo
---
   Rule name: foo
   SELinux User: guest_u:s0
   Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule=foo
---
Modified SELinux User Map foo
---
   Rule name: foo
   SELinux User: guest_u:s0
   HBAC Rule: foo
   Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule= --usercat=all
ipa: ERROR: HBAC rule and local members cannot both be set

All these validation failures are not valid.

3) Additionally, I think it would be more readable and less error prone that if
instead of this blob:

+are_local_members_to_be_set  = 'usercategory' in _entry_attrs or \
+   'hostcategory' in _entry_attrs or \
+   'memberuser' in _entry_attrs or \
+   'memberhost' in _entry_attrs

You would use something like that:

are_local_members_to_be_set  = any(attr in _entry_attrs
for attr in ('usercategory',
 'hostcategory',
 'memberuser',
 'memberhost'))

Martin

1.) Done.
2.) Corrected.
3.) Fixed.

Tomas
From d77004bce8644b0f3a64860174539c9a2d640ef1 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 6 Sep 2012 07:03:42 -0400
Subject: [PATCH] Make sure selinuxusemap behaves consistently to HBAC rule

Both selinuxusermap-add and selinuxusermap-mod commands now behave
consistently in not allowing user/host category or user/host members
and HBAC rule being set at the same time. Also adds a bunch of unit
tests that check this behaviour.

https://fedorahosted.org/freeipa/ticket/2983
---
 ipalib/plugins/selinuxusermap.py|  49 +--
 tests/test_xmlrpc/test_selinuxusermap_plugin.py | 179 
 2 files changed, 216 insertions(+), 12 deletions(-)

diff --git a/ipalib/plugins/selinuxusermap.py b/ipalib/plugins/selinuxusermap.py
index 13bbb58ec0e6b7bd4275be17198c7452090a0781..131a83ce5674628041601e98028a013d47b40a4a 100644
--- a/ipalib/plugins/selinuxusermap.py
+++ b/ipalib/plugins/selinuxusermap.py
@@ -242,8 +242,21 @@ class selinuxusermap_add(LDAPCreate):
 # rules are enabled by default
 entry_attrs['ipaenabledflag'] = 'TRUE'
 validate_selinuxuser_inlist(ldap, entry_attrs['ipaselinuxuser'])
-if 'seealso' in options:
-entry_attrs['seealso'] = self.obj._normalize_seealso(options['seealso'])
+
+# hbacrule is not allowed when usercat or hostcat is set
+is_to_be_set = lambda x : x in entry_attrs and entry_attrs[x]!=None
+
+are_local_members_to_be_set  = any(is_to_be_set(attr)
+   for attr in ('usercategory',
+'hostcategory'))
+
+is_hbacrule_to_be_set = is_to_be_set('seealso')
+
+if is_hbacrule_to_be_set and are_local_members_to_be_set:
+raise errors.MutuallyExclusiveError(reason=notboth_err)
+
+if is_hbacrule_to_be_set:
+entry_attrs['seealso'] = self.obj._normalize_seealso(entry_attrs['seealso'])
 
 return dn
 
@@ -276,18 +289,30 @@ class selinuxusermap_mod(LDAPUpdate):
 except errors.NotFound:
 self.obj.handle_not_found(*keys)
 
-if 'seealso' in options and ('usercategory' in _entry_attrs or
-  'hostcategory' in _entry_attrs or
-  'memberuser' in _entry_attrs or
-  'memberhost' in _entry_attrs):
+# makes sure the local members and hbacrule is not set at the same time
+# memberuser or memberhost could have been set using --setattr
+is_to_be_set = lambda x: (x in _entry_attrs and

Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule

2012-09-05 Thread Martin Kosek 
On 09/03/2012 05:12 PM, Tomas Babej wrote:
 Hi,
 
 Both selinuxusermap-add and selinuxusermap-mod commands now behave
 consistently in not allowing user/host category or user/host members
 and HBAC rule being set at the same time. Also adds a bunch of unit
 tests that check this behaviour.
 
 https://fedorahosted.org/freeipa/ticket/2983
 
 Tomas
 

I found few issues with this patch:

1) Patch needs a rebase

2) Patch does not expect attributes to be set to None, i.e. to be left empty or
to be deleted, e.g.:

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-add foo --selinuxuser=guest_u:s0 --usercat=all

Added SELinux User Map foo

  Rule name: foo
  SELinux User: guest_u:s0
  User category: all
  Enabled: TRUE

# ipa selinuxusermap-mod foo --usercat= --hbacrule=
ipa: ERROR: HBAC rule and local members cannot both be set

# ipa selinuxusermap-mod foo --usercat=
---
Modified SELinux User Map foo
---
  Rule name: foo
  SELinux User: guest_u:s0
  Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule=foo
---
Modified SELinux User Map foo
---
  Rule name: foo
  SELinux User: guest_u:s0
  HBAC Rule: foo
  Enabled: TRUE

# ipa selinuxusermap-mod foo --hbacrule= --usercat=all
ipa: ERROR: HBAC rule and local members cannot both be set

All these validation failures are not valid.

3) Additionally, I think it would be more readable and less error prone that if
instead of this blob:

+are_local_members_to_be_set  = 'usercategory' in _entry_attrs or \
+   'hostcategory' in _entry_attrs or \
+   'memberuser' in _entry_attrs or \
+   'memberhost' in _entry_attrs

You would use something like that:

are_local_members_to_be_set  = any(attr in _entry_attrs
   for attr in ('usercategory',
'hostcategory',
'memberuser',
'memberhost'))

Martin

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