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

[Martin Kosek]

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