Dne pátek 20 července 2012 15:40:26, Jakub Hrozek napsal(a):
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.
You are right. It is a bit different (although related) issue but I extended
the patch to include the fix as well.
> > > #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?
I didn't exactly like that solution either so I moved those two constants to
ipa_hbac.h which is supposed to be a public HBAC interface. The "right
solution" would be to construct a map for HBAC rules, I know we discussed this
with Stephen several months back but we never really got to do that.
Jan