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

[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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0011-3-Make-sure-selinuxusemap-behaves-consistently-to-HBAC.patch
Type: text/x-patch
Size: 15148 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120912/034bcd36/attachment.bin>