Re: [RFC] [Patch 1/1] IBAC Patch

[Randy Dunlap <randy.dunlap@oracle.com>]

On Fri, 09 Mar 2007 08:19:36 -0500 Mimi Zohar wrote:

> On Thu, 2007-03-08 at 15:08 -0800, Randy Dunlap wrote:
> > On Thu, 08 Mar 2007 17:58:16 -0500 Mimi Zohar wrote:
> > 
> > > This is a request for comments for a new Integrity Based Access
> > > Control(IBAC) LSM module which bases access control decisions
> > > on the new integrity framework services. 
> > > 
> > > (Hopefully this will help clarify the interaction between an LSM 
> > > module and LIM module.)
> > > 
> > > Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> > > ===================================================================
> > > --- /dev/null
> > > +++ linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> > > @@ -0,0 +1,36 @@
> > > +config SECURITY_IBAC
> > > +	boolean "IBAC support"
> > > +	depends on SECURITY && SECURITY_NETWORK && INTEGRITY
> > > +	help
> > > +	  Integrity Based Access Control(IBAC) implements integrity
> > > +	  based access control.
> > 
> > Please make the help text do more than repeat the words I B A C...
> > Put a short explanation or say something like:
> > 	  See Documentation/security/foobar.txt for more information.
> > (and add that file)
> 
> Agreed.  Perhaps something like:
> 
> Integrity Based Access Control(IBAC) uses the Linux Integrity
> Module(LIM) API calls to verify an executable's metadata and 
> data's integrity.  Based on the results, execution permission 
> is permitted/denied.  Integrity providers may implement the 
> LIM hooks differently.  For more information on integrity
> verification refer to the specific integrity provider 
> documentation. 

Yes, thanks.

> > > +config SECURITY_IBAC_BOOTPARAM
> > > +	bool "IBAC boot parameter"
> > > +	depends on SECURITY_IBAC
> > > +	default y
> > > +	help
> > > +	  This option adds a kernel parameter 'ibac', which allows IBAC
> > > +	  to be disabled at boot.  If this option is selected, IBAC
> > > +	  functionality can be disabled with ibac=0 on the kernel
> > > +	  command line.  The purpose of this option is to allow a
> > > +	  single kernel image to be distributed with IBAC built in,
> > > +	  but not necessarily enabled.
> > > +
> > > +	  If you are unsure how to answer this question, answer N.
> > 
> > What's the downside to having this always builtin instead of
> > yet another config option?
> 
> The ability of changing LSM modules at runtime might be perceived
> as problematic.
> 
> > > +static struct security_operations ibac_security_ops = {
> > > +	.bprm_check_security = ibac_bprm_check_security
> > > +};
> > > +
> > > +static int __init init_ibac(void)
> > > +{
> > > +	int rc;
> > > +
> > > +	if (!ibac_enabled)
> > > +		return 0;
> > > +
> > > +	rc = register_security(&ibac_security_ops);
> > > +	if (rc != 0)
> > > +		panic("IBAC: Unable to register with kernel\n");
> > 
> > Normally we would not want to see a panic() from a register_xyz()
> > failure, but I guess you are arguing that an ibac register_security()
> > failure needs to halt everything??
> 
> Yes, as this implies that another LSM module registered the hooks first,
> preventing IBAC from registering itself. 
> 
> Thank you for your other comments.  They'll be addressed in the next
> ibac patch release.

Okie.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***