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

[Rob Crittenden]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-927-1-selinux.patch
Type: text/x-diff
Size: 5765 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120124/9f4f5fc8/attachment.bin>