[SSSD] [PATCHES] Patch set fixing SELinux - HBAC linking
Jakub Hrozek
jhrozek at redhat.com
Fri Jul 20 13:40:26 UTC 2012
On Fri, Jul 20, 2012 at 03:06:38PM +0200, Jan Zelený wrote:
> Dne pátek 20 července 2012 14:32:10, Jakub Hrozek napsal(a):
> > On Fri, Jul 20, 2012 at 01:55:59PM +0200, Jan Zelený wrote:
> > > #156
> > > Added some debug messages
> >
> > This debug message is wrong:
> >
> > + DEBUG(SSSDBG_TRACE_FUNC, ("HBAC rule [%s] matched, moving "
> > + "SELinux user map [%s] to
> > confirmed\n", + hbac_dn,
> > seealso_dn));
> >
> > There is no "confirmed" list anymore. The rest of the patch looks good.
>
> You are right. Fixed
>
> > > #157
> > > The original priority patch had this condition in the wrong place,
> > > resulting in hostCategory == all not being taken into account
> >
> > This code assigns SELINUX_PRIORITY_HOST_CAT to any rule that contains
> > anything in the "hostcat" attribute. It needs to specifically check for
> > strcasecmp(hostcat, "all") and error out saying that the category is not
> > supported on any other input. The same applies to user categories.
> >
> > Similar to hbac_get_category() for how the input validity check is performed
> > for the HBAC rules.
>
> On the contrary, the check is there. At least I can see the strcasecmp in my
> vim ;-)
>
Both hostCategory and userCategory are multi-valued, but the code only
checks the first value unconditionally..
I know the UI only allows setting only "all" right now, but changing the
check the way hbac_get_category() does it would be more robust.
> > > #158
> > > The function ipa_selinux_map_merge() is no longer necessary since more
> > > generic function has been implemented and it is even used in the code
> >
> > Ack
> >
> > > #159
> > > This patch provides the fix for HBAC - SELinux linking itself. I'm not
> > > sure
> > > about defining those two constants on top. If anyone has better idea where
> > > to put them in order to consolidate them with the same constants private
> > > for HBAC code, I'm open to suggestions.
> >
> > They are already stored in ipa_selinux_user_map[], see ipa_opts.h
>
> Yes they are but I don't think it's wise to extract the name from selinux user
> map and apply it to HBAC rule. It will work, sure. But IMO it would be even
> more confusing and potentially dangerous than defining the constant this way.
>
> Sending corrected patch #156
Oh right, it's and HBAC attribute..
Can't you just include ipa_hbac_private.h, then?
More information about the sssd-devel
mailing list