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

[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