[Freeipa-devel] [PATCH] 927 fix deleting hbac rules when selinux user maps are involved

[Rob Crittenden]

Martin Kosek wrote:
> On Tue, 2012-01-24 at 10:08 -0500, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Mon, 2012-01-23 at 12:20 -0500, Rob Crittenden wrote:
>>>> Martin Kosek wrote:
>>>>> On Tue, 2012-01-17 at 17:59 -0500, Rob Crittenden wrote:
>>>>>> When deleting an HBAC rule we need to ensure that an SELinux user map
>>>>>> isn't pointing at it. The search for this didn't work well at all.
>>>>>>
>>>>>> This patch corrects the search and makes it more specific.
>>>>>>
>>>>>> I also tested that it works with the --continue flag of hbacrule-del.
>>>>>>
>>>>>> The ticket has instructions on testing.
>>>>>>
>>>>>> rob
>>>>>
>>>>> Works fine. There is just one part that is IMO too complicated:
>>>>>
>>>>> +            hbacrule = options['seealso']
>>>>> +            kw = dict(cn=hbacrule, all=True)
>>>>>                 _entries = api.Command.hbacrule_find(None, **kw)['result']
>>>>>                 del options['seealso']
>>>>> -            if _entries:
>>>>> -                options['seealso'] = _entries[0]['dn']
>>>>> +            found = False
>>>>> +            # look for an exact match. The search may return partial
>>>>> +            # matches.
>>>>> +            for entry in _entries:
>>>>> +                if entry['cn'][0] == hbacrule:
>>>>> +                    found = True
>>>>> +                    options['seealso'] = entry['dn']
>>>>> +            if not found:
>>>>> +                return dict(count=0, result=[], truncated=False)
>>>>>
>>>>> I think hbacrule_find(None, cn=HBACRULE) should not return partial
>>>>> matches, but just the exact match (tried with hbacrule-find
>>>>> --name=HBACRULE). Then the loop over entries wouldn't be needed.
>>>>>
>>>>> Couldn't we simply call hbacrule_show since we want just one HBAC rule
>>>>> with a known primary key?
>>>>>
>>>>> Martin
>>>>>
>>>>
>>>> hbacrule_show would need to be modified to take a dn, that would be a
>>>> way to fix this.
>>>>
>>>> rob
>>>
>>> Not sure I see the problem with hbacrule_show. I tested this piece of
>>> code and it worked fine:
>>>
>>> selinuxusermap_find:
>>> ...
>>>           if 'seealso' in options:
>>>               hbacrule = options['seealso']
>>>
>>>               try:
>>>                   hbac = api.Command['hbacrule_show'](hbacrule,
>>> all=True)['result']
>>>                   dn = hbac['dn']
>>>               except errors.NotFound:
>>>                   return dict(count=0, result=[], truncated=False)
>>>               options['seealso'] = dn
>>> ...
>>>
>>> Martin
>>>
>>
>> Ok, I misunderstood your point. Yes, this is vastly better. Updated
>> patch attached.
>>
>> rob
>
> ACK if you change
>
> if 'seealso' in options:
>
> to:
>
> if options.get('seealso'):
>
> so that the following case is fixed:
>
> # ipa selinuxusermap-find --hbacrule=
> ipa: ERROR: 'cn' is required
>
> Martin
>

Fixed, pushed to master and ipa-2-2

rob