Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule
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
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 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): return _('Inv
Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule
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
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 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 _entry_attrs[x]!=None) or \ +
Re: [Freeipa-devel] [PATCH 0011] Make sure selinuxusemap behaves consistently to HBAC rule
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