linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] [Patch 1/1] IBAC Patch
@ 2007-03-08 22:58 Mimi Zohar
  2007-03-08 23:08 ` Randy Dunlap
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Mimi Zohar @ 2007-03-08 22:58 UTC (permalink / raw)
  To: linux-security-module; +Cc: safford, serue, kjhall, zohar, linux-kernel

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.
+
+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.
+
+config SECURITY_IBAC_BOOTPARAM_VALUE
+	int "IBAC boot parameter default value"
+	depends on SECURITY_IBAC_BOOTPARAM
+	range 0 1
+	default 0
+	help
+	  This option sets the default value for the kernel parameter
+	  'ibac', which allows IBAC to be disabled at boot.  If this
+	  option is set to 0 (zero), the IBAC kernel parameter will
+	  default to 0, disabling IBAC at bootup.  If this option is
+	  set to 1 (one), the IBAC kernel parameter will default to 1,
+	  enabling IBAC at bootup.
+
+	  If you are unsure how to answer this question, answer 0.
+
Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
===================================================================
--- /dev/null
+++ linux-2.6.21-rc3-mm2/security/ibac/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for building IBAC
+#
+
+obj-$(CONFIG_SECURITY_IBAC) += ibac.o
+ibac-y 	:= ibac_main.o
Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
===================================================================
--- /dev/null
+++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
@@ -0,0 +1,126 @@
+/*
+ * Integrity Based Access Control (IBAC)
+ *
+ * Copyright (C) 2007 IBM Corporation
+ * Author: Mimi Zohar <zohar@us.ibm.com>
+ *
+ *      This program is free software; you can redistribute it and/or modify
+ *      it under the terms of the GNU General Public License as published by
+ *      the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/security.h>
+#include <linux/integrity.h>
+
+#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
+int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
+
+static int __init ibac_enabled_setup(char *str)
+{
+	ibac_enabled = simple_strtol(str, NULL, 0);
+	return 1;
+}
+
+__setup("ibac=", ibac_enabled_setup);
+#else
+int ibac_enabled = 0;
+#endif
+
+static unsigned int integrity_enforce = 0;
+static int __init integrity_enforce_setup(char *str)
+{
+	integrity_enforce = simple_strtol(str, NULL, 0);
+	return 1;
+}
+
+__setup("ibac_enforce=", integrity_enforce_setup);
+
+#define XATTR_NAME "security.evm.hash"
+
+static inline int is_kernel_thread(struct task_struct *tsk)
+{
+	return (!tsk->mm) ? 1 : 0;
+}
+
+static int ibac_bprm_check_security(struct linux_binprm *bprm)
+{
+	struct dentry *dentry = bprm->file->f_dentry;
+	int xattr_len;
+	char *xattr_value = NULL;
+	int rc, status;
+
+	rc = integrity_verify_metadata(dentry, XATTR_NAME,
+				       &xattr_value, &xattr_len, &status);
+	if (rc < 0 && rc == -EOPNOTSUPP) {
+		kfree(xattr_value);
+		return 0;
+	}
+
+	if (rc < 0) {
+		printk(KERN_INFO "verify_metadata %s failed "
+		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
+		if (!integrity_enforce)
+			rc = 0;
+		goto out;
+	}
+	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
+		if (!is_kernel_thread(current)) {
+			printk(KERN_INFO "verify_metadata %s "
+			       "(Integrity status: FAIL)\n", bprm->filename);
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+
+	rc = integrity_verify_data(dentry, &status);
+	if (rc < 0) {
+		printk(KERN_INFO "%s verify_data failed "
+		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
+		if (!integrity_enforce)
+			rc = 0;
+		goto out;
+	}
+	if (status != INTEGRITY_PASS) {
+		if (!is_kernel_thread(current)) {
+			printk(KERN_INFO "verify_data %s "
+			       "(Integrity status: FAIL)\n", bprm->filename);
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+
+	kfree(xattr_value);
+
+	/* measure all integrity level executables */
+	integrity_measure(dentry, bprm->filename, MAY_EXEC);
+	return 0;
+      out:
+	kfree(xattr_value);
+	return rc;
+}
+
+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");
+	return rc;
+}
+
+security_initcall(init_ibac);
Index: linux-2.6.21-rc3-mm2/security/Kconfig
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/Kconfig
+++ linux-2.6.21-rc3-mm2/security/Kconfig
@@ -125,5 +125,6 @@ config SECURITY_ROOTPLUG
 source security/selinux/Kconfig
 
 source security/slim/Kconfig
+source security/ibac/Kconfig
 endmenu
 
Index: linux-2.6.21-rc3-mm2/security/Makefile
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/Makefile
+++ linux-2.6.21-rc3-mm2/security/Makefile
@@ -14,6 +14,7 @@ endif
 obj-$(CONFIG_SECURITY)			+= security.o dummy.o inode.o
 obj-$(CONFIG_INTEGRITY)		+= integrity.o integrity_dummy.o
 obj-$(CONFIG_INTEGRITY_EVM)		+= evm/
+obj-$(CONFIG_SECURITY_IBAC)		+= ibac/
 # Must precede capability.o in order to stack properly.
 obj-$(CONFIG_SECURITY_SLIM)		+= slim/
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-08 22:58 [RFC] [Patch 1/1] IBAC Patch Mimi Zohar
@ 2007-03-08 23:08 ` Randy Dunlap
  2007-03-09 13:19   ` Mimi Zohar
  2007-03-09  3:19 ` Valdis.Kletnieks
  2007-03-14  2:27 ` Seth Arnold
  2 siblings, 1 reply; 16+ messages in thread
From: Randy Dunlap @ 2007-03-08 23:08 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, safford, serue, kjhall, zohar, linux-kernel

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)


> +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?

> +config SECURITY_IBAC_BOOTPARAM_VALUE
> +	int "IBAC boot parameter default value"
> +	depends on SECURITY_IBAC_BOOTPARAM
> +	range 0 1
> +	default 0
> +	help
> +	  This option sets the default value for the kernel parameter
> +	  'ibac', which allows IBAC to be disabled at boot.  If this
> +	  option is set to 0 (zero), the IBAC kernel parameter will
> +	  default to 0, disabling IBAC at bootup.  If this option is
> +	  set to 1 (one), the IBAC kernel parameter will default to 1,
> +	  enabling IBAC at bootup.
> +
> +	  If you are unsure how to answer this question, answer 0.
> +

> Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> @@ -0,0 +1,126 @@
> +/*
> + * Integrity Based Access Control (IBAC)
> + *
> + * Copyright (C) 2007 IBM Corporation
> + * Author: Mimi Zohar <zohar@us.ibm.com>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/kernel.h>
> +#include <linux/security.h>
> +#include <linux/integrity.h>
> +
> +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> +int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;

static int ?

> +static int __init ibac_enabled_setup(char *str)
> +{
> +	ibac_enabled = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +
> +__setup("ibac=", ibac_enabled_setup);
> +#else
> +int ibac_enabled = 0;

static int ?

> +#endif
> +
> +static unsigned int integrity_enforce = 0;

Don't init to 0 (not needed, consumes some binary file space).

> +static int __init integrity_enforce_setup(char *str)
> +{
> +	integrity_enforce = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +
> +__setup("ibac_enforce=", integrity_enforce_setup);
> +
> +#define XATTR_NAME "security.evm.hash"
> +
> +static inline int is_kernel_thread(struct task_struct *tsk)
> +{
> +	return (!tsk->mm) ? 1 : 0;
> +}
> +
> +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> +{
> +	struct dentry *dentry = bprm->file->f_dentry;
> +	int xattr_len;
> +	char *xattr_value = NULL;
> +	int rc, status;
> +
> +	rc = integrity_verify_metadata(dentry, XATTR_NAME,
> +				       &xattr_value, &xattr_len, &status);
> +	if (rc < 0 && rc == -EOPNOTSUPP) {

just	if (rc == -EOPNOTSUPP)
?

> +		kfree(xattr_value);
> +		return 0;
> +	}
> +
> +	if (rc < 0) {
> +		printk(KERN_INFO "verify_metadata %s failed "
> +		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);

How about adding "ibac: " to the beginning of each printk string,
so that someone will know the general source of these messages?


> +		if (!integrity_enforce)
> +			rc = 0;
> +		goto out;
> +	}
> +	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
> +		if (!is_kernel_thread(current)) {
> +			printk(KERN_INFO "verify_metadata %s "
> +			       "(Integrity status: FAIL)\n", bprm->filename);
> +			if (integrity_enforce) {
> +				rc = -EACCES;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	rc = integrity_verify_data(dentry, &status);
> +	if (rc < 0) {
> +		printk(KERN_INFO "%s verify_data failed "
> +		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
> +		if (!integrity_enforce)
> +			rc = 0;
> +		goto out;
> +	}
> +	if (status != INTEGRITY_PASS) {
> +		if (!is_kernel_thread(current)) {
> +			printk(KERN_INFO "verify_data %s "
> +			       "(Integrity status: FAIL)\n", bprm->filename);
> +			if (integrity_enforce) {
> +				rc = -EACCES;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	kfree(xattr_value);
> +
> +	/* measure all integrity level executables */
> +	integrity_measure(dentry, bprm->filename, MAY_EXEC);
> +	return 0;
> +      out:

Don't "hide" labels by indenting them so much.  You don't need to
indent them at all, or maybe 1 character/column.

> +	kfree(xattr_value);
> +	return rc;
> +}
> +
> +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??

> +	return rc;
> +}
> +
> +security_initcall(init_ibac);


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-08 22:58 [RFC] [Patch 1/1] IBAC Patch Mimi Zohar
  2007-03-08 23:08 ` Randy Dunlap
@ 2007-03-09  3:19 ` Valdis.Kletnieks
  2007-03-09 15:07   ` Serge E. Hallyn
  2007-03-12 21:47   ` Mimi Zohar
  2007-03-14  2:27 ` Seth Arnold
  2 siblings, 2 replies; 16+ messages in thread
From: Valdis.Kletnieks @ 2007-03-09  3:19 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, safford, serue, kjhall, zohar, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said:
> 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.)

OK, between this and the additional LIM hooks I didn't notice in an earlier
patch, we're starting to see the API.   The only problem is that although
it may be the right API for *your* code, I suspect it's a non-starter without
a discussion about whether it's the right *generic* API for an LIM (which will
require at least one dramatic bun fight about what "Integrity" means).

> Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
 
Minor congnitive-dissonance alert:

> +config SECURITY_IBAC_BOOTPARAM
> +	bool "IBAC boot parameter"
> +	depends on SECURITY_IBAC
> +	default y

> +	  If you are unsure how to answer this question, answer N.

The 'default' should in general match the hint we give the user.

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-08 23:08 ` Randy Dunlap
@ 2007-03-09 13:19   ` Mimi Zohar
  2007-03-09 18:26     ` Randy Dunlap
  0 siblings, 1 reply; 16+ messages in thread
From: Mimi Zohar @ 2007-03-09 13:19 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: linux-security-module, safford, serue, kjhall, zohar, linux-kernel

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. 

> > +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.

Mimi Zohar


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-09  3:19 ` Valdis.Kletnieks
@ 2007-03-09 15:07   ` Serge E. Hallyn
  2007-03-12 21:47   ` Mimi Zohar
  1 sibling, 0 replies; 16+ messages in thread
From: Serge E. Hallyn @ 2007-03-09 15:07 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Mimi Zohar, linux-security-module, safford, serue, kjhall, zohar,
	linux-kernel

Quoting Valdis.Kletnieks@vt.edu (Valdis.Kletnieks@vt.edu):
> On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said:
> > 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.)
> 
> OK, between this and the additional LIM hooks I didn't notice in an earlier
> patch, we're starting to see the API.   The only problem is that although
> it may be the right API for *your* code, I suspect it's a non-starter without
> a discussion about whether it's the right *generic* API for an LIM (which will
> require at least one dramatic bun fight about what "Integrity" means).

Casey's earlier message suggested this too.  'Integrity' here in
particular does not mean online integrity guarantees through, i.e.,
information flow control.  So perhaps instead of 'integrity' we should
make sure to always say 'integrity measurement'.  Of course then there
is already the 'integrity measurement architecture' which is only one
implementation of a LIM module, right?   So it would need to be renamed
to TIMA (TPM-enabled IMA) or something I guess.

-serge

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-09 13:19   ` Mimi Zohar
@ 2007-03-09 18:26     ` Randy Dunlap
  0 siblings, 0 replies; 16+ messages in thread
From: Randy Dunlap @ 2007-03-09 18:26 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, safford, serue, kjhall, zohar, linux-kernel

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 ***

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-09  3:19 ` Valdis.Kletnieks
  2007-03-09 15:07   ` Serge E. Hallyn
@ 2007-03-12 21:47   ` Mimi Zohar
  2007-03-13 15:31     ` Serge E. Hallyn
  1 sibling, 1 reply; 16+ messages in thread
From: Mimi Zohar @ 2007-03-12 21:47 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: linux-security-module, safford, serue, kjhall, zohar,
	linux-kernel, disec-devel

On Thu, 2007-03-08 at 22:19 -0500, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said:
> > 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.)
> 
> OK, between this and the additional LIM hooks I didn't notice in an earlier
> patch, we're starting to see the API.   The only problem is that although
> it may be the right API for *your* code, I suspect it's a non-starter without
> a discussion about whether it's the right *generic* API for an LIM (which will
> require at least one dramatic bun fight about what "Integrity" means).

Absolutely, we need to make sure that the set of LIM hooks is complete and that
nothing is missing in order to implement different types of LIM providers.  I'm 
copying the digsig mailing list for their input on requirements, which this API 
might not satisfy or perhaps address.

> > Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> 
> Minor congnitive-dissonance alert:
> 
> > +config SECURITY_IBAC_BOOTPARAM
> > +	bool "IBAC boot parameter"
> > +	depends on SECURITY_IBAC
> > +	default y
> 
> > +	  If you are unsure how to answer this question, answer N.
> 
> The 'default' should in general match the hint we give the user.

Oops, blush.  It will obviously be corrected in the next IBAC patch
release.

Mimi Zohar


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-12 21:47   ` Mimi Zohar
@ 2007-03-13 15:31     ` Serge E. Hallyn
  2007-03-14  9:46       ` Mimi Zohar
  0 siblings, 1 reply; 16+ messages in thread
From: Serge E. Hallyn @ 2007-03-13 15:31 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Valdis.Kletnieks, linux-security-module, safford, serue, kjhall,
	zohar, linux-kernel, disec-devel, selinux

Quoting Mimi Zohar (zohar@linux.vnet.ibm.com):
> On Thu, 2007-03-08 at 22:19 -0500, Valdis.Kletnieks@vt.edu wrote:
> > On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said:
> > > 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.)
> > 
> > OK, between this and the additional LIM hooks I didn't notice in an earlier
> > patch, we're starting to see the API.   The only problem is that although
> > it may be the right API for *your* code, I suspect it's a non-starter without
> > a discussion about whether it's the right *generic* API for an LIM (which will
> > require at least one dramatic bun fight about what "Integrity" means).
> 
> Absolutely, we need to make sure that the set of LIM hooks is complete and that
> nothing is missing in order to implement different types of LIM providers.  I'm 
> copying the digsig mailing list for their input on requirements, which this API 
> might not satisfy or perhaps address.

(Could you resend the IBAC patch to the disec list as well?)

Is IBAC basically a 'demo' lsm?  It only hooks bprm_security, so you
can't execute anything with a bad hash, right?  So really if you were to
also hook mmap, this would completely do away with the need for digsig?
IBAC is automatically able to use any integrity measurement modules you
write, whether they use gpg like digsig does right now, or use the tpm?

It also shows how simple it would be to hook selinux, though I guess
there are several ways we might want to hook selinux - 1. to check only
security.selinux xattrs, 2. to check integrity of entry point types, 3.
to check integrity of any files labeled by the integrity measurement
module as needing to be checked.

Is there any way for the LSM itself to direct which data and metadata
will be measured?  It looks like this is done by separately configuring
the integrity measurement module - which seems fine to me.

-serge

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-08 22:58 [RFC] [Patch 1/1] IBAC Patch Mimi Zohar
  2007-03-08 23:08 ` Randy Dunlap
  2007-03-09  3:19 ` Valdis.Kletnieks
@ 2007-03-14  2:27 ` Seth Arnold
  2007-03-14 11:25   ` Mimi Zohar
  2 siblings, 1 reply; 16+ messages in thread
From: Seth Arnold @ 2007-03-14  2:27 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, safford, serue, kjhall, zohar, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4878 bytes --]

On Thu, Mar 08, 2007 at 05:58:16PM -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. 

Thanks Mimi, nice to see an example of how the integrity framework ought
to be used.

> (Hopefully this will help clarify the interaction between an LSM 
> module and LIM module.)

Is this module intended to clarify an interface, or be useful in and of
itself?

> Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
> ===================================================================
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for building IBAC
> +#
> +
> +obj-$(CONFIG_SECURITY_IBAC) += ibac.o
> +ibac-y 	:= ibac_main.o
> Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> @@ -0,0 +1,126 @@
> +/*
> + * Integrity Based Access Control (IBAC)
> + *
> + * Copyright (C) 2007 IBM Corporation
> + * Author: Mimi Zohar <zohar@us.ibm.com>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/kernel.h>
> +#include <linux/security.h>
> +#include <linux/integrity.h>
> +
> +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> +int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
> +
> +static int __init ibac_enabled_setup(char *str)
> +{
> +	ibac_enabled = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +
> +__setup("ibac=", ibac_enabled_setup);
> +#else
> +int ibac_enabled = 0;
> +#endif

If the command line option isn't enabled, how will ibac_enabled ever be
set to '1'? Have I overlooked or forgotten some helper routine elsewhere?

> +static unsigned int integrity_enforce = 0;
> +static int __init integrity_enforce_setup(char *str)
> +{
> +	integrity_enforce = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +
> +__setup("ibac_enforce=", integrity_enforce_setup);
> +
> +#define XATTR_NAME "security.evm.hash"

Is this name unique to this IBAC module? Or should it be kept in sync
with the integrity framework?

> +static inline int is_kernel_thread(struct task_struct *tsk)
> +{
> +	return (!tsk->mm) ? 1 : 0;
> +}
> +
> +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> +{
> +	struct dentry *dentry = bprm->file->f_dentry;
> +	int xattr_len;
> +	char *xattr_value = NULL;
> +	int rc, status;
> +
> +	rc = integrity_verify_metadata(dentry, XATTR_NAME,
> +				       &xattr_value, &xattr_len, &status);
> +	if (rc < 0 && rc == -EOPNOTSUPP) {
> +		kfree(xattr_value);
> +		return 0;
> +	}
> +
> +	if (rc < 0) {
> +		printk(KERN_INFO "verify_metadata %s failed "
> +		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
> +		if (!integrity_enforce)
> +			rc = 0;
> +		goto out;
> +	}
> +	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
> +		if (!is_kernel_thread(current)) {

Please remind me why kernel threads are exempt?

> +			printk(KERN_INFO "verify_metadata %s "
> +			       "(Integrity status: FAIL)\n", bprm->filename);

Integrity status may be FAIL or NO_LABEL at this point -- would it be
more useful to report the whole truth?

> +			if (integrity_enforce) {
> +				rc = -EACCES;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	rc = integrity_verify_data(dentry, &status);
> +	if (rc < 0) {
> +		printk(KERN_INFO "%s verify_data failed "
> +		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
> +		if (!integrity_enforce)
> +			rc = 0;
> +		goto out;
> +	}
> +	if (status != INTEGRITY_PASS) {
> +		if (!is_kernel_thread(current)) {

Please remind me why kernel threads are exempt?

> +			printk(KERN_INFO "verify_data %s "
> +			       "(Integrity status: FAIL)\n", bprm->filename);

Same question about FAIL vs NO_LABEL.. (Would NO_LABEL be caught by a
failing verify_metadata above?)

> +			if (integrity_enforce) {
> +				rc = -EACCES;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	kfree(xattr_value);
> +
> +	/* measure all integrity level executables */
> +	integrity_measure(dentry, bprm->filename, MAY_EXEC);
> +	return 0;

If integrity_measure() fails (can it fail?) is allowing the exec still the
right approach? (I seem to recall that "measuring integrity" is actually
something more like "go off an compute the integrity, but don't compare
it against anything" -- but even if it fails, is continuing correct?)

Rest elided :) Thanks Mimi

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-13 15:31     ` Serge E. Hallyn
@ 2007-03-14  9:46       ` Mimi Zohar
  0 siblings, 0 replies; 16+ messages in thread
From: Mimi Zohar @ 2007-03-14  9:46 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Valdis.Kletnieks, linux-security-module, safford, serue, kjhall,
	zohar, linux-kernel, disec-devel, selinux

On Tue, 2007-03-13 at 10:31 -0500, Serge E. Hallyn wrote:
> Quoting Mimi Zohar (zohar@linux.vnet.ibm.com):
> > On Thu, 2007-03-08 at 22:19 -0500, Valdis.Kletnieks@vt.edu wrote:
> > > On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said:
> > > > 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.)
> > > 
> > > OK, between this and the additional LIM hooks I didn't notice in an earlier
> > > patch, we're starting to see the API.   The only problem is that although
> > > it may be the right API for *your* code, I suspect it's a non-starter without
> > > a discussion about whether it's the right *generic* API for an LIM (which will
> > > require at least one dramatic bun fight about what "Integrity" means).
> > 
> > Absolutely, we need to make sure that the set of LIM hooks is complete and that
> > nothing is missing in order to implement different types of LIM providers.  I'm 
> > copying the digsig mailing list for their input on requirements, which this API 
> > might not satisfy or perhaps address.
> 
> (Could you resend the IBAC patch to the disec list as well?)

Sure, I'm just about to post a new version of IBAC with the changes based on 
comments from the lsm and lkml lists.

> Is IBAC basically a 'demo' lsm?  It only hooks bprm_security, so you
> can't execute anything with a bad hash, right?  So really if you were to
> also hook mmap, this would completely do away with the need for digsig?
> IBAC is automatically able to use any integrity measurement modules you
> write, whether they use gpg like digsig does right now, or use the tpm?

You're absolutely correct on all accounts.

> It also shows how simple it would be to hook selinux, though I guess
> there are several ways we might want to hook selinux - 1. to check only
> security.selinux xattrs, 2. to check integrity of entry point types, 3.
> to check integrity of any files labeled by the integrity measurement
> module as needing to be checked.

Correct. Verifying the integrity of security.selinux xattrs would require 
adding security.selinux to /etc/evm.conf and calls within SELinux to 
integrity_verify_metadata() as needed. Verifying the integrity of files,
would require labeling the files with integrity measurements and
adding calls within SELinux to integrity_verify_data() as needed.

> Is there any way for the LSM itself to direct which data and metadata
> will be measured?  It looks like this is done by separately configuring
> the integrity measurement module - which seems fine to me.

Actually the current EVM implementation as an integrity provider relies on
an LSM module, or for that matter any other kernel module, to make calls 
to the integrity API: integrity_verify_metadata(), integrity_verify_data(), 
and integrity_measure().  The decision as to which files are to be measured
is left up to the LSM module.  For example, as you previously noted, IBAC
only verifies the metadata and data integrity of executables, whereas SLIM 
verifies the metadata and data integrity of SYSTEM level files and all
executables.

Mimi Zohar


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-14  2:27 ` Seth Arnold
@ 2007-03-14 11:25   ` Mimi Zohar
  2007-03-14 18:48     ` Seth Arnold
  0 siblings, 1 reply; 16+ messages in thread
From: Mimi Zohar @ 2007-03-14 11:25 UTC (permalink / raw)
  To: Seth Arnold
  Cc: linux-security-module, safford, serue, kjhall, zohar, linux-kernel

On Tue, 2007-03-13 at 19:27 -0700, Seth Arnold wrote:
> On Thu, Mar 08, 2007 at 05:58:16PM -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. 
> 
> Thanks Mimi, nice to see an example of how the integrity framework ought
> to be used.
> 
> > (Hopefully this will help clarify the interaction between an LSM 
> > module and LIM module.)
> 
> Is this module intended to clarify an interface, or be useful in and of
> itself?

It's a little bit of both. :-) Initially it was written to help me with 
implementing and testing the integrity provider.  But it could definitely stand
on it's own.  As Serge Hallyn commented http://lkml.org/lkml/2007/3/13/220, by 
adding the mmap hook, IBAC could replace the LSM aspect of digsig and a gpg 
based integrity provider, could be written, instead of EVM, which is TPM based.

> > Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6.21-rc3-mm2/security/ibac/Makefile
> > @@ -0,0 +1,6 @@
> > +#
> > +# Makefile for building IBAC
> > +#
> > +
> > +obj-$(CONFIG_SECURITY_IBAC) += ibac.o
> > +ibac-y 	:= ibac_main.o
> > Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> > @@ -0,0 +1,126 @@
> > +/*
> > + * Integrity Based Access Control (IBAC)
> > + *
> > + * Copyright (C) 2007 IBM Corporation
> > + * Author: Mimi Zohar <zohar@us.ibm.com>
> > + *
> > + *      This program is free software; you can redistribute it and/or modify
> > + *      it under the terms of the GNU General Public License as published by
> > + *      the Free Software Foundation, version 2 of the License.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/kernel.h>
> > +#include <linux/security.h>
> > +#include <linux/integrity.h>
> > +
> > +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> > +int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
> > +
> > +static int __init ibac_enabled_setup(char *str)
> > +{
> > +	ibac_enabled = simple_strtol(str, NULL, 0);
> > +	return 1;
> > +}
> > +
> > +__setup("ibac=", ibac_enabled_setup);
> > +#else
> > +int ibac_enabled = 0;
> > +#endif
> 
> If the command line option isn't enabled, how will ibac_enabled ever be
> set to '1'? Have I overlooked or forgotten some helper routine elsewhere?

I guess I was a bit over zealous in preventing IBAC from running 
unintentionally.  Will fix in the next IBAC patch release.

> > +static unsigned int integrity_enforce = 0;
> > +static int __init integrity_enforce_setup(char *str)
> > +{
> > +	integrity_enforce = simple_strtol(str, NULL, 0);
> > +	return 1;
> > +}
> > +
> > +__setup("ibac_enforce=", integrity_enforce_setup);
> > +
> > +#define XATTR_NAME "security.evm.hash"
> 
> Is this name unique to this IBAC module? Or should it be kept in sync
> with the integrity framework?

In order to verify the metadata integrity, an xattr needs to be
specified on the integrity_verify_metadata() call.  As IBAC does not
define an xattr of its own, I used this one, which at the time worked. 
But as EVM is only one integrity provider, this probably is not a good 
idea. To resolve this problem, I guess the integrity_verify_metadata()
API call should respond without requiring an actual xattr.

> > +static inline int is_kernel_thread(struct task_struct *tsk)
> > +{
> > +	return (!tsk->mm) ? 1 : 0;
> > +}
> > +
> > +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> > +{
> > +	struct dentry *dentry = bprm->file->f_dentry;
> > +	int xattr_len;
> > +	char *xattr_value = NULL;
> > +	int rc, status;
> > +
> > +	rc = integrity_verify_metadata(dentry, XATTR_NAME,
> > +				       &xattr_value, &xattr_len, &status);
> > +	if (rc < 0 && rc == -EOPNOTSUPP) {
> > +		kfree(xattr_value);
> > +		return 0;
> > +	}
> > +
> > +	if (rc < 0) {
> > +		printk(KERN_INFO "verify_metadata %s failed "
> > +		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
> > +		if (!integrity_enforce)
> > +			rc = 0;
> > +		goto out;
> > +	}
> > +	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
> > +		if (!is_kernel_thread(current)) {
> 
> Please remind me why kernel threads are exempt?

You really don't want to prevent kernel threads from working. Nasty things
happen.

> > +			printk(KERN_INFO "verify_metadata %s "
> > +			       "(Integrity status: FAIL)\n", bprm->filename);
> 
> Integrity status may be FAIL or NO_LABEL at this point -- would it be
> more useful to report the whole truth?

FAIL here indicates that the integrity of the metadata was bad, while
NOLABEL indicates, in the case of EVM, that there was no HMAC.  I'll
update the error message to differentiate between the two.

> > +			if (integrity_enforce) {
> > +				rc = -EACCES;
> > +				goto out;
> > +			}
> > +		}
> > +	}
> > +
> > +	rc = integrity_verify_data(dentry, &status);
> > +	if (rc < 0) {
> > +		printk(KERN_INFO "%s verify_data failed "
> > +		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
> > +		if (!integrity_enforce)
> > +			rc = 0;
> > +		goto out;
> > +	}
> > +	if (status != INTEGRITY_PASS) {
> > +		if (!is_kernel_thread(current)) {
> 
> Please remind me why kernel threads are exempt?

Same as above. 
> > +			printk(KERN_INFO "verify_data %s "
> > +			       "(Integrity status: FAIL)\n", bprm->filename);
> 
> Same question about FAIL vs NO_LABEL.. (Would NO_LABEL be caught by a
> failing verify_metadata above?)

For integrity_verify_data(), EVM verifies that the file hash matches the
one stored as an xattr.

> > +			if (integrity_enforce) {
> > +				rc = -EACCES;
> > +				goto out;
> > +			}
> > +		}
> > +	}
> > +
> > +	kfree(xattr_value);
> > +
> > +	/* measure all integrity level executables */
> > +	integrity_measure(dentry, bprm->filename, MAY_EXEC);
> > +	return 0;
> 
> If integrity_measure() fails (can it fail?) is allowing the exec still the
> right approach? (I seem to recall that "measuring integrity" is actually
> something more like "go off an compute the integrity, but don't compare
> it against anything" -- but even if it fails, is continuing correct?)

For integrity_measure(), EVM calls IMA, if enabled, to extend the
measurement list with the hash value it provides. In most cases, EVM
has already calculated the hash value, when it was called to verify the
data. integrity_measure() is not meant to be intrusive, so it is defined
as void.

> Rest elided :) Thanks Mimi

Thank you for your comments.

Mimi Zohar


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] [Patch 1/1] IBAC Patch
  2007-03-14 11:25   ` Mimi Zohar
@ 2007-03-14 18:48     ` Seth Arnold
  0 siblings, 0 replies; 16+ messages in thread
From: Seth Arnold @ 2007-03-14 18:48 UTC (permalink / raw)
  To: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]

On Wed, Mar 14, 2007 at 07:25:26AM -0400, Mimi Zohar wrote:
> It's a little bit of both. :-) Initially it was written to help me with 

:)

> implementing and testing the integrity provider.  But it could definitely stand
> on it's own.  As Serge Hallyn commented http://lkml.org/lkml/2007/3/13/220, by 
> adding the mmap hook, IBAC could replace the LSM aspect of digsig and a gpg 
> based integrity provider, could be written, instead of EVM, which is TPM based.

Thanks.

> > > +	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
> > > +		if (!is_kernel_thread(current)) {
> > 
> > Please remind me why kernel threads are exempt?
> 
> You really don't want to prevent kernel threads from working. Nasty things
> happen.

But under what conditions would a kernel thread not pass integrity? I
guess if it doesn't have an associated dentry... or the dentry refers
to something else? (What does knfsd do -- it is started by a userland
program which causes the kernel to start up some tasks for NFS..)

> For integrity_measure(), EVM calls IMA, if enabled, to extend the
> measurement list with the hash value it provides. In most cases, EVM
> has already calculated the hash value, when it was called to verify the
> data. integrity_measure() is not meant to be intrusive, so it is defined
> as void.

Oh, ok, thanks.

> Thank you for your comments.

My pleasure, thanks for the quick responses.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][Patch 1/1] IBAC Patch
  2007-06-19 22:23 ` Serge E. Hallyn
@ 2007-06-20 11:52   ` Mimi Zohar
  0 siblings, 0 replies; 16+ messages in thread
From: Mimi Zohar @ 2007-06-20 11:52 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: linux-security-module, safford, serue, sailer, zohar, linux-kernel

On Tue, 2007-06-19 at 17:23 -0500, Serge E. Hallyn wrote:

> > +#define get_file_security(file) ((unsigned long)(file->f_security))
> > +#define set_file_security(file, val) (file->f_security = (void *)val)
> > +
> > +#define get_task_security(task) ((unsigned long)(task->security))
> > +#define set_task_security(task, val) (task->security = (void *)val)
> 
> I understand the above are likely remnants of stacker lsm support, and I
> hate to say this, but not only is having those in there going to be
> frowned upon, it then leads you later on to do things like
> 
> 	if (get_file_security(file)==0)
> 
> when using 0 for null upsets people in itself.

Instead of allocating memory for file->f_security, it uses 
file->f_security itself, for storing 0 or 1.  So it isn't
checking to see if it is NULL per se.

> > +
> > +#define XATTR_MEASURE_SUFFIX "measure"
> > +#define XATTR_MEASURE_SUFFIX_LEN (sizeof (XATTR_MEASURE_SUFFIX) -1)
> > +
> > +struct ibac_isec_data {
> > +	int mmapped;		/* no. of times inode mmapped */
> > +	int measure;		/* inode tagged to be measured */
> > +	spinlock_t lock;	/* protect inode state */
> > +};
> > +
> > +static int ibac_inode_alloc_security(struct inode *inode)
> > +{
> > +	struct ibac_isec_data *isec;
> > +
> > +	isec = kzalloc(sizeof(struct ibac_isec_data), GFP_KERNEL);
> > +	if (!isec)
> > +		return -ENOMEM;
> > +
> > +	spin_lock_init(&isec->lock);
> > +	inode->i_security = isec;
> 
> Heh, for file and task security you use the above helpers, but for inode
> you do it like this?  :)  Please replace all x_security assignments and
> checks with this format.

For file and task, the code uses the security field itself to store
information as opposed to allocating storage for it.   In the case of 
inode, it is using it for both the original digsig mmap tracking and 
now to tag the inode that it needs to be measured.  Tagging the inode
to be measured is based on the existence of the security.measure xattr,
which is controlled by a userspace application.  The difference is
that storage is allocated for inode->i_security.

> > +/*
> > + * For all inodes allocate inode->i_security(isec), before the security
> > + * subsystem is enabled.
> > + */
> > +static void ibac_fixup_inodes(void)
> > +{
> > +	struct super_block *sb;
> > +
> > +	spin_lock(&sb_lock);
> > +	list_for_each_entry(sb, &super_blocks, s_list) {
> > +		struct inode *inode;
> > +
> > +		spin_unlock(&sb_lock);
> > +
> > +		spin_lock(&inode_lock);
> > +		list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> > +			spin_unlock(&inode_lock);
> > +
> > +			spin_lock(&inode->i_lock);
> > +			if (!inode->i_security)
> > +				ibac_inode_alloc_security(inode);
> 
> since ibac_inode_alloc_security can return -ENOMEM, maybe you should at
> least check for that condition and warn the user?

Yes, that definitely is a good idea.  Will do.

> > +			spin_unlock(&inode->i_lock);
> > +
> > +			spin_lock(&inode_lock);
> > +		}
> > +		spin_unlock(&inode_lock);
> > +
> > +		spin_lock(&sb_lock);
> > +	}
> > +	spin_unlock(&sb_lock);
> > +}
> > +

> > +static int ibac_inode_permission(struct inode *inode, int mask,
> > +				 struct nameidata *nd)
> > +{
> > +	struct ibac_isec_data *isec = inode->i_security;
> > +	struct dentry *dentry;
> > +	char *path = NULL;
> > +	char *fname = NULL;
> > +	int rc = 0;
> > +	int measure;
> > +
> > +	dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
> > +	if (!dentry)
> > +		return 0;
> > +	if (nd) {		/* preferably use fullname */
> > +		path = (char *)__get_free_page(GFP_KERNEL);
> > +		if (path)
> > +			fname = d_path(nd->dentry, nd->mnt, path, PAGE_SIZE);
> > +	}
> > +
> > +	if (!fname)		/* no choice, use short name */
> > +		fname = (!dentry->d_name.name) ? (char *)dentry->d_iname :
> > +		    (char *)dentry->d_name.name;
> 
> Please separate the above out into a helper function.

Ok.

> This name is only ever used for debugging, right?  I didn't miss any
> place where the name is used for some security decision?

Correct, the filename is not used for any security decision, but it 
is passed to integrity_measure(), which the IMA integrity provider
associates with the given hash value.  To see the filename hints
'cat /sys/kernel/security/ima/ascii_runtime_measurements'.

> > +
> > +	/* Measure labeled files */
> > +	spin_lock(&isec->lock);
> > +	measure = isec->measure;
> > +	spin_unlock(&isec->lock);
> > +
> > +	if (S_ISREG(inode->i_mode) && (measure == 1)
> > +	    && (mask & MAY_READ)) {
> > +		rc = verify_metadata_integrity(dentry);
> > +		if (rc == 0)
> > +			rc = verify_and_measure_integrity(dentry, NULL,
> > +							  fname, mask);
> > +	}
> > +
> > +	/* Deny permission to write, if currently mmapped. */
> > +	if (inode && mask & MAY_WRITE) {
> > +		spin_lock(&isec->lock);
> > +		if (isec->mmapped > 0) {
> > +			printk(KERN_INFO "%s: %s - denied write access"
> > +			       " (isec=%d)\n",
> > +			       __FUNCTION__, fname, isec->mmapped);
> > +			rc = -EPERM;
> > +		}
> > +		spin_unlock(&isec->lock);
> > +	}
> > +
> > +	if (!nd || !nd->dentry)
> > +		dput(dentry);
> > +	if (path)
> > +		free_page((unsigned long)path);
> > +	return rc;
> > +}


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC][Patch 1/1] IBAC Patch
  2007-06-18 20:48 [RFC][Patch " Mimi Zohar
@ 2007-06-19 22:23 ` Serge E. Hallyn
  2007-06-20 11:52   ` Mimi Zohar
  0 siblings, 1 reply; 16+ messages in thread
From: Serge E. Hallyn @ 2007-06-19 22:23 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-security-module, safford, serue, zohar, linux-kernel

Quoting Mimi Zohar (zohar@linux.vnet.ibm.com):
> This is a re-release of Integrity Based Access Control(IBAC) LSM module
> which bases access control decisions on the new integrity framework
> services.  IBAC is a sample LSM module to help clarify the interaction
> between LSM and Linux Integrity Modules(LIM).
> 
> New to this release of IBAC is digsig's functionality of preventing
> files open for write to be mmapped, and files that are mmapped from being
> opened for write.
> 
> IBAC originally verified/measured executables only in the
> bprm_check_security() hook.  By only doing the verification/measurement
> in bprm_check_security(), libraries could be loaded without first being
> verified/measured.  This release of IBAC, files are also verified/measured 
> in the file_mmap() hook, which catches the libraries, and inode_permission() 
> hook, which verifies/measures files tagged, by a userspace application,
> with the extended attribute 'security.measure'.
> 
> IBAC can be included or excluded in the kernel configuration.  If
> included in the kernel and IBAC_BOOTPARAM is enabled, IBAC can also be
> enabled/disabled on the kernel command line with 'ibac='.
> 
> IBAC can be configured to either verify and enforce integrity or to just log
> integrity failures on the kernel command line with 'ibac_enforce='.  When
> IBAC_BOOTPARAM is enabled, the default is only to log integrity failures.
> 
> Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
> ---
> 
> Index: linux-2.6.22-rc4-mm2/security/ibac/Kconfig
> ===================================================================
> --- /dev/null
> +++ linux-2.6.22-rc4-mm2/security/ibac/Kconfig
> @@ -0,0 +1,54 @@
> +config SECURITY_IBAC
> +	boolean "IBAC support"
> +	depends on SECURITY && SECURITY_NETWORK && INTEGRITY
> +	help
> +	  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.
> +
> +config SECURITY_IBAC_BOOTPARAM
> +	bool "IBAC boot parameter"
> +	depends on SECURITY_IBAC
> +	default n
> +	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.
> +
> +config SECURITY_IBAC_BOOTPARAM_VALUE
> +	int "IBAC boot parameter default value"
> +	depends on SECURITY_IBAC_BOOTPARAM
> +	range 0 1
> +	default 0
> +	help
> +	  This option sets the default value for the kernel parameter
> +	  'ibac', which allows IBAC to be enabled at boot.  If this
> +	  option is set to 1 (one), the IBAC kernel parameter will
> +	  default to 1, enabling IBAC at bootup.  If this option is
> +	  set to 0 (zero), the IBAC kernel parameter will default to 0,
> +	  disabling IBAC at bootup.
> +
> +	  If you are unsure how to answer this question, answer 0.
> +
> +config SECURITY_IBAC_ENFORCE
> +	bool "IBAC integrity enforce boot parameter"
> +	depends on SECURITY_IBAC_BOOTPARAM
> +	default y
> +	help
> +	  This option adds a kernel parameter 'ibac_enforce', which
> +	  allows integrity enforcement to be enabled/disabled at boot.
> +	  If this option is selected, integrity enforcement can be
> +	  enabled with ibac_enforce=1 on the kernel command line.
> +	  The default is not to enforce integrity, but simply log
> +	  integrity verification errors.
> +
> +	  If you are unsure how to answer this question, answer Y.
> Index: linux-2.6.22-rc4-mm2/security/ibac/Makefile
> ===================================================================
> --- /dev/null
> +++ linux-2.6.22-rc4-mm2/security/ibac/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for building IBAC
> +#
> +
> +obj-$(CONFIG_SECURITY_IBAC) += ibac.o
> +ibac-y 	:= ibac_main.o
> Index: linux-2.6.22-rc4-mm2/security/ibac/ibac_main.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.22-rc4-mm2/security/ibac/ibac_main.c
> @@ -0,0 +1,434 @@
> +/*
> + * Integrity Based Access Control(IBAC) sample LSM module calling LIM hooks.
> + *
> + * Copyright (C) 2007 IBM Corporation
> + * Author: Mimi Zohar <zohar@us.ibm.com>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/security.h>
> +#include <linux/xattr.h>
> +#include <linux/integrity.h>
> +#include <linux/writeback.h>
> +
> +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> +static int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
> +
> +static int __init ibac_enabled_setup(char *str)
> +{
> +	ibac_enabled = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +
> +__setup("ibac=", ibac_enabled_setup);
> +
> +static int integrity_enforce;
> +static int __init integrity_enforce_setup(char *str)
> +{
> +	integrity_enforce = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +
> +__setup("ibac_enforce=", integrity_enforce_setup);
> +
> +#else
> +static int ibac_enabled = 1;
> +static int integrity_enforce = 1;
> +#endif
> +
> +#define get_file_security(file) ((unsigned long)(file->f_security))
> +#define set_file_security(file, val) (file->f_security = (void *)val)
> +
> +#define get_task_security(task) ((unsigned long)(task->security))
> +#define set_task_security(task, val) (task->security = (void *)val)

I understand the above are likely remnants of stacker lsm support, and I
hate to say this, but not only is having those in there going to be
frowned upon, it then leads you later on to do things like

	if (get_file_security(file)==0)

when using 0 for null upsets people in itself.

> +
> +#define XATTR_MEASURE_SUFFIX "measure"
> +#define XATTR_MEASURE_SUFFIX_LEN (sizeof (XATTR_MEASURE_SUFFIX) -1)
> +
> +struct ibac_isec_data {
> +	int mmapped;		/* no. of times inode mmapped */
> +	int measure;		/* inode tagged to be measured */
> +	spinlock_t lock;	/* protect inode state */
> +};
> +
> +static int ibac_inode_alloc_security(struct inode *inode)
> +{
> +	struct ibac_isec_data *isec;
> +
> +	isec = kzalloc(sizeof(struct ibac_isec_data), GFP_KERNEL);
> +	if (!isec)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&isec->lock);
> +	inode->i_security = isec;

Heh, for file and task security you use the above helpers, but for inode
you do it like this?  :)  Please replace all x_security assignments and
checks with this format.

> +	return 0;
> +}
> +
> +static void ibac_inode_free_security(struct inode *inode)
> +{
> +	struct ibac_isec_data *isec = inode->i_security;
> +
> +	if (isec) {
> +		inode->i_security = NULL;
> +		kfree(isec);
> +	}
> +}
> +
> +/*
> + * For all inodes allocate inode->i_security(isec), before the security
> + * subsystem is enabled.
> + */
> +static void ibac_fixup_inodes(void)
> +{
> +	struct super_block *sb;
> +
> +	spin_lock(&sb_lock);
> +	list_for_each_entry(sb, &super_blocks, s_list) {
> +		struct inode *inode;
> +
> +		spin_unlock(&sb_lock);
> +
> +		spin_lock(&inode_lock);
> +		list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> +			spin_unlock(&inode_lock);
> +
> +			spin_lock(&inode->i_lock);
> +			if (!inode->i_security)
> +				ibac_inode_alloc_security(inode);

since ibac_inode_alloc_security can return -ENOMEM, maybe you should at
least check for that condition and warn the user?

> +			spin_unlock(&inode->i_lock);
> +
> +			spin_lock(&inode_lock);
> +		}
> +		spin_unlock(&inode_lock);
> +
> +		spin_lock(&sb_lock);
> +	}
> +	spin_unlock(&sb_lock);
> +}
> +
> +static void ibac_d_instantiate(struct dentry *dentry, struct inode *inode)
> +{
> +	struct ibac_isec_data *isec;
> +	char *xattr_flags = XATTR_SECURITY_PREFIX XATTR_MEASURE_SUFFIX;
> +
> +	if (!inode)
> +		return;
> +
> +	if (!inode->i_op || !inode->i_op->getxattr)
> +		return;
> +
> +	if (vfs_getxattr(dentry, xattr_flags, NULL, 0) > 0) {
> +		isec = inode->i_security;
> +		spin_lock(&isec->lock);
> +		isec->measure = 1;
> +		spin_unlock(&isec->lock);
> +	}
> +}
> +
> +static inline int is_kernel_thread(struct task_struct *tsk)
> +{
> +	return (!tsk->mm) ? 1 : 0;
> +}
> +
> +static int verify_metadata_integrity(struct dentry *dentry)
> +{
> +	int rc, status;
> +
> +	if (!dentry)
> +		return 0;
> +
> +	rc = integrity_verify_metadata(dentry, NULL, NULL, NULL, &status);
> +	if (rc == -EOPNOTSUPP)
> +		return 0;
> +
> +	if (rc < 0) {
> +		printk(KERN_INFO "ibac: verify_metadata %s failed"
> +		       "(rc: %d - status: %d)\n",
> +		       dentry->d_name.name, rc, status);
> +		if (!integrity_enforce)
> +			rc = 0;
> +		goto out;
> +	}
> +	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
> +		if (!is_kernel_thread(current)) {
> +			printk(KERN_INFO "ibac: verify_metadata %s "
> +			       "(Integrity status: %s)\n",
> +			       dentry->d_name.name,
> +			       status == INTEGRITY_FAIL ? "FAIL" : "NOLABEL");
> +			if (integrity_enforce) {
> +				rc = -EACCES;
> +				goto out;
> +			}
> +		}
> +	}
> +	return 0;
> +out:
> +	return rc;
> +}
> +
> +static int verify_and_measure_integrity(struct dentry *dentry,
> +					struct file *file,
> +					char *filename, int mask)
> +{
> +	int rc, status;
> +
> +	if (!dentry && !file)
> +		return 0;
> +
> +	rc = integrity_verify_data(dentry, file, &status);
> +	if (rc < 0) {
> +		printk(KERN_INFO "ibac: %s verify_data failed "
> +		       "(rc: %d - status: %d)\n", filename, rc, status);
> +		if (!integrity_enforce)
> +			rc = 0;
> +		goto out;
> +	}
> +
> +	if (status != INTEGRITY_PASS) {
> +		if (!is_kernel_thread(current)) {
> +			printk(KERN_INFO "ibac: verify_data %s "
> +			       "(Integrity status: FAIL)\n", filename);
> +			if (integrity_enforce) {
> +				rc = -EACCES;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	/* Only measure files that passed integrity verification. */
> +	integrity_measure(dentry, file, filename, mask);
> +	return 0;
> +out:
> +	/*
> +	 * Verification failed, but as integrity is not being enforced,
> +	 * we still need to measure.
> +	 */
> +	if (rc == 0)
> +		integrity_measure(dentry, file, filename, mask);
> +	return rc;
> +}
> +
> +static int ibac_inode_permission(struct inode *inode, int mask,
> +				 struct nameidata *nd)
> +{
> +	struct ibac_isec_data *isec = inode->i_security;
> +	struct dentry *dentry;
> +	char *path = NULL;
> +	char *fname = NULL;
> +	int rc = 0;
> +	int measure;
> +
> +	dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
> +	if (!dentry)
> +		return 0;
> +	if (nd) {		/* preferably use fullname */
> +		path = (char *)__get_free_page(GFP_KERNEL);
> +		if (path)
> +			fname = d_path(nd->dentry, nd->mnt, path, PAGE_SIZE);
> +	}
> +
> +	if (!fname)		/* no choice, use short name */
> +		fname = (!dentry->d_name.name) ? (char *)dentry->d_iname :
> +		    (char *)dentry->d_name.name;

Please separate the above out into a helper function.

This name is only ever used for debugging, right?  I didn't miss any
place where the name is used for some security decision?

> +
> +	/* Measure labeled files */
> +	spin_lock(&isec->lock);
> +	measure = isec->measure;
> +	spin_unlock(&isec->lock);
> +
> +	if (S_ISREG(inode->i_mode) && (measure == 1)
> +	    && (mask & MAY_READ)) {
> +		rc = verify_metadata_integrity(dentry);
> +		if (rc == 0)
> +			rc = verify_and_measure_integrity(dentry, NULL,
> +							  fname, mask);
> +	}
> +
> +	/* Deny permission to write, if currently mmapped. */
> +	if (inode && mask & MAY_WRITE) {
> +		spin_lock(&isec->lock);
> +		if (isec->mmapped > 0) {
> +			printk(KERN_INFO "%s: %s - denied write access"
> +			       " (isec=%d)\n",
> +			       __FUNCTION__, fname, isec->mmapped);
> +			rc = -EPERM;
> +		}
> +		spin_unlock(&isec->lock);
> +	}
> +
> +	if (!nd || !nd->dentry)
> +		dput(dentry);
> +	if (path)
> +		free_page((unsigned long)path);
> +	return rc;
> +}
> +
> +/*
> + * If the file is opened for writing, deny mmap(PROT_EXEC) access.
> + * Otherwise, increment the inode->i_security, which is our own
> + * writecount.  When the file is closed, f->f_security will be 1,
> + * and so we will decrement the inode->i_security.
> + * Just to be clear:
> + * 	file->f_security is 1 or 0.
> + * 	inode->i_security->mmapped is the *number* of processes which
> + * 		have this file mmapped(PROT_EXEC), so it can be >1.
> + */
> +static int ibac_deny_write_access(struct file *file)
> +{
> +	struct inode *inode = file->f_dentry->d_inode;
> +	struct ibac_isec_data *isec = inode->i_security;
> +
> +	spin_lock(&inode->i_lock);
> +	if (atomic_read(&inode->i_writecount) > 0) {
> +		spin_unlock(&inode->i_lock);
> +		return -ETXTBSY;
> +	}
> +
> +	spin_lock(&isec->lock);
> +	isec->mmapped += 1;
> +	spin_unlock(&isec->lock);
> +	set_file_security(file, 1);
> +	spin_unlock(&inode->i_lock);
> +
> +	return 0;
> +}
> +
> +/*
> + * decrement our writer count on the inode.  When it hits 0, we will
> + * again allow opening the inode for writing.
> + */
> +static void ibac_allow_write_access(struct file *file)
> +{
> +	struct inode *inode = file->f_dentry->d_inode;
> +	struct ibac_isec_data *isec = inode->i_security;
> +
> +	spin_lock(&inode->i_lock);
> +	spin_lock(&isec->lock);
> +	isec->mmapped -= 1;
> +	spin_unlock(&isec->lock);
> +	set_file_security(file, 0);
> +	spin_unlock(&inode->i_lock);
> +}
> +
> +/*
> + * the file is being closed.  If we ever mmaped it for exec, then
> + * file->f_security>0, and we decrement the inode usage count to
> + * show that we are done with it.
> + */
> +static void ibac_file_free_security(struct file *file)
> +{
> +	if (file->f_security)
> +		ibac_allow_write_access(file);
> +}
> +
> +/*
> + * We don't want to validate files which can be written while they are
> + * being executed.
> + * This means NFS.
> + */
> +static inline int is_unprotected_file(struct file *file)
> +{
> +	if (strcmp(file->f_dentry->d_inode->i_sb->s_type->name, "nfs") == 0)
> +		return 1;
> +	return 0;
> +}
> +
> +/*
> + * Verify data integrity, metadata integrity and measure it.
> + */
> +static int ibac_file_mmap(struct file *file,
> +			  unsigned long reqprot,
> +			  unsigned long calcprot, unsigned long flags)
> +{
> +	unsigned long prot = reqprot;
> +	int rc;
> +
> +	if (!(prot & VM_EXEC))
> +		return 0;
> +	if (!file || !file->f_dentry)
> +		return 0;
> +
> +	if (is_unprotected_file(file))
> +		return (-EPERM);
> +	/*
> +	 * if file_security is set, then this process has already
> +	 * incremented the writer count on this inode, don't do
> +	 * it again.
> +	 */
> +	if (get_file_security(file) == 0) {
> +		rc = ibac_deny_write_access(file);
> +		if (rc) {
> +			if (S_ISCHR(file->f_dentry->d_inode->i_mode))
> +				rc = 0;
> +			goto out;
> +		}
> +	}
> +
> +	rc = verify_metadata_integrity(file->f_dentry);
> +	if (rc == 0) {
> +		char *fname = NULL;
> +		char *path = NULL;
> +
> +		path = (char *)__get_free_page(GFP_KERNEL);
> +		if (path)
> +			fname = d_path(file->f_dentry, file->f_path.mnt,
> +				       path, PAGE_SIZE);
> +
> +		if (!fname)	/* no choice, use short name */
> +			fname = (!file->f_dentry->d_name.name) ?
> +			    (char *)file->f_dentry->d_iname :
> +			    (char *)file->f_dentry->d_name.name;
> +
> +		rc = verify_and_measure_integrity(NULL, file, fname, MAY_EXEC);
> +		if (path)
> +			free_page((unsigned long)path);
> +	}
> +out:
> +	return rc;
> +}
> +
> +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> +{
> +	struct dentry *dentry = bprm->file->f_dentry;
> +	int rc;
> +
> +	rc = verify_metadata_integrity(dentry);
> +	if (rc == 0)
> +		rc = verify_and_measure_integrity(dentry,
> +						  bprm->file, bprm->filename,
> +						  MAY_EXEC);
> +	return rc;
> +}
> +
> +static struct security_operations ibac_security_ops = {
> +	.bprm_check_security = ibac_bprm_check_security,
> +	.file_mmap = ibac_file_mmap,
> +	.file_free_security = ibac_file_free_security,
> +	.inode_alloc_security = ibac_inode_alloc_security,
> +	.inode_free_security = ibac_inode_free_security,
> +	.inode_permission = ibac_inode_permission,
> +	.d_instantiate = ibac_d_instantiate
> +};
> +
> +static int __init init_ibac(void)
> +{
> +	int rc;
> +
> +	if (!ibac_enabled)
> +		return 0;
> +
> +	ibac_fixup_inodes();
> +	rc = register_security(&ibac_security_ops);
> +	if (rc != 0)
> +		panic("ibac: unable to register with kernel\n");
> +	return rc;
> +}
> +
> +security_initcall(init_ibac);
> Index: linux-2.6.22-rc4-mm2/security/Kconfig
> ===================================================================
> --- linux-2.6.22-rc4-mm2.orig/security/Kconfig
> +++ linux-2.6.22-rc4-mm2/security/Kconfig
> @@ -114,5 +114,6 @@ config SECURITY_ROOTPLUG
>  
>  source security/selinux/Kconfig
>  
> +source security/ibac/Kconfig
>  endmenu
>  
> Index: linux-2.6.22-rc4-mm2/security/Makefile
> ===================================================================
> --- linux-2.6.22-rc4-mm2.orig/security/Makefile
> +++ linux-2.6.22-rc4-mm2/security/Makefile
> @@ -14,6 +14,7 @@ endif
>  obj-$(CONFIG_SECURITY)			+= security.o dummy.o inode.o
>  obj-$(CONFIG_INTEGRITY)		+= integrity.o integrity_dummy.o
>  obj-$(CONFIG_IMA_MEASURE)		+= ima/
> +obj-$(CONFIG_SECURITY_IBAC)		+= ibac/
>  # Must precede capability.o in order to stack properly.
>  obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
>  obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
> Index: linux-2.6.22-rc4-mm2/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-2.6.22-rc4-mm2.orig/Documentation/kernel-parameters.txt
> +++ linux-2.6.22-rc4-mm2/Documentation/kernel-parameters.txt
> @@ -42,6 +42,7 @@ parameter is applicable:
>  	FB	The frame buffer device is enabled.
>  	HW	Appropriate hardware is enabled.
>  	IA-64	IA-64 architecture is enabled.
> +	IBAC	Integrity Based Access Control support is enabled.
>  	IMA	Integrity measurement architecture is enabled.
>  	IOSCHED	More than one I/O scheduler is enabled.
>  	IP_PNP	IP DHCP, BOOTP, or RARP is enabled.
> @@ -761,6 +762,17 @@ and is between 256 and 4096 characters. 
>  	ihash_entries=	[KNL]
>  			Set number of hash buckets for inode cache.
>  
> +	ibac=		[IBAC] Disable or enable IBAC at boot time.
> +                        Format: { "0" | "1" }
> +                        See security/ibac/Kconfig help text.
> +                        0 -- disable.
> +                        1 -- enable.
> +                        Default value is set via kernel config option.
> +
> +	ibac_enforce=	[IBAC] Enable integrity enforcing at boot time.
> +                        Format: { "0" | "1" }
> +                        Default is 0 to disable integrity enforcing.
> +
>  	ima=		[IMA] Disable or enable IMA at boot time.
>  			Format: { "0" | "1" }
>  			See security/ima/Kconfig help text.
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC][Patch 1/1] IBAC Patch
@ 2007-06-18 20:48 Mimi Zohar
  2007-06-19 22:23 ` Serge E. Hallyn
  0 siblings, 1 reply; 16+ messages in thread
From: Mimi Zohar @ 2007-06-18 20:48 UTC (permalink / raw)
  To: linux-security-module; +Cc: safford, serue, zohar, linux-kernel

This is a re-release of Integrity Based Access Control(IBAC) LSM module
which bases access control decisions on the new integrity framework
services.  IBAC is a sample LSM module to help clarify the interaction
between LSM and Linux Integrity Modules(LIM).

New to this release of IBAC is digsig's functionality of preventing
files open for write to be mmapped, and files that are mmapped from being
opened for write.

IBAC originally verified/measured executables only in the
bprm_check_security() hook.  By only doing the verification/measurement
in bprm_check_security(), libraries could be loaded without first being
verified/measured.  This release of IBAC, files are also verified/measured 
in the file_mmap() hook, which catches the libraries, and inode_permission() 
hook, which verifies/measures files tagged, by a userspace application,
with the extended attribute 'security.measure'.

IBAC can be included or excluded in the kernel configuration.  If
included in the kernel and IBAC_BOOTPARAM is enabled, IBAC can also be
enabled/disabled on the kernel command line with 'ibac='.

IBAC can be configured to either verify and enforce integrity or to just log
integrity failures on the kernel command line with 'ibac_enforce='.  When
IBAC_BOOTPARAM is enabled, the default is only to log integrity failures.

Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---

Index: linux-2.6.22-rc4-mm2/security/ibac/Kconfig
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4-mm2/security/ibac/Kconfig
@@ -0,0 +1,54 @@
+config SECURITY_IBAC
+	boolean "IBAC support"
+	depends on SECURITY && SECURITY_NETWORK && INTEGRITY
+	help
+	  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.
+
+config SECURITY_IBAC_BOOTPARAM
+	bool "IBAC boot parameter"
+	depends on SECURITY_IBAC
+	default n
+	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.
+
+config SECURITY_IBAC_BOOTPARAM_VALUE
+	int "IBAC boot parameter default value"
+	depends on SECURITY_IBAC_BOOTPARAM
+	range 0 1
+	default 0
+	help
+	  This option sets the default value for the kernel parameter
+	  'ibac', which allows IBAC to be enabled at boot.  If this
+	  option is set to 1 (one), the IBAC kernel parameter will
+	  default to 1, enabling IBAC at bootup.  If this option is
+	  set to 0 (zero), the IBAC kernel parameter will default to 0,
+	  disabling IBAC at bootup.
+
+	  If you are unsure how to answer this question, answer 0.
+
+config SECURITY_IBAC_ENFORCE
+	bool "IBAC integrity enforce boot parameter"
+	depends on SECURITY_IBAC_BOOTPARAM
+	default y
+	help
+	  This option adds a kernel parameter 'ibac_enforce', which
+	  allows integrity enforcement to be enabled/disabled at boot.
+	  If this option is selected, integrity enforcement can be
+	  enabled with ibac_enforce=1 on the kernel command line.
+	  The default is not to enforce integrity, but simply log
+	  integrity verification errors.
+
+	  If you are unsure how to answer this question, answer Y.
Index: linux-2.6.22-rc4-mm2/security/ibac/Makefile
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4-mm2/security/ibac/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for building IBAC
+#
+
+obj-$(CONFIG_SECURITY_IBAC) += ibac.o
+ibac-y 	:= ibac_main.o
Index: linux-2.6.22-rc4-mm2/security/ibac/ibac_main.c
===================================================================
--- /dev/null
+++ linux-2.6.22-rc4-mm2/security/ibac/ibac_main.c
@@ -0,0 +1,434 @@
+/*
+ * Integrity Based Access Control(IBAC) sample LSM module calling LIM hooks.
+ *
+ * Copyright (C) 2007 IBM Corporation
+ * Author: Mimi Zohar <zohar@us.ibm.com>
+ *
+ *      This program is free software; you can redistribute it and/or modify
+ *      it under the terms of the GNU General Public License as published by
+ *      the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/security.h>
+#include <linux/xattr.h>
+#include <linux/integrity.h>
+#include <linux/writeback.h>
+
+#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
+static int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
+
+static int __init ibac_enabled_setup(char *str)
+{
+	ibac_enabled = simple_strtol(str, NULL, 0);
+	return 1;
+}
+
+__setup("ibac=", ibac_enabled_setup);
+
+static int integrity_enforce;
+static int __init integrity_enforce_setup(char *str)
+{
+	integrity_enforce = simple_strtol(str, NULL, 0);
+	return 1;
+}
+
+__setup("ibac_enforce=", integrity_enforce_setup);
+
+#else
+static int ibac_enabled = 1;
+static int integrity_enforce = 1;
+#endif
+
+#define get_file_security(file) ((unsigned long)(file->f_security))
+#define set_file_security(file, val) (file->f_security = (void *)val)
+
+#define get_task_security(task) ((unsigned long)(task->security))
+#define set_task_security(task, val) (task->security = (void *)val)
+
+#define XATTR_MEASURE_SUFFIX "measure"
+#define XATTR_MEASURE_SUFFIX_LEN (sizeof (XATTR_MEASURE_SUFFIX) -1)
+
+struct ibac_isec_data {
+	int mmapped;		/* no. of times inode mmapped */
+	int measure;		/* inode tagged to be measured */
+	spinlock_t lock;	/* protect inode state */
+};
+
+static int ibac_inode_alloc_security(struct inode *inode)
+{
+	struct ibac_isec_data *isec;
+
+	isec = kzalloc(sizeof(struct ibac_isec_data), GFP_KERNEL);
+	if (!isec)
+		return -ENOMEM;
+
+	spin_lock_init(&isec->lock);
+	inode->i_security = isec;
+	return 0;
+}
+
+static void ibac_inode_free_security(struct inode *inode)
+{
+	struct ibac_isec_data *isec = inode->i_security;
+
+	if (isec) {
+		inode->i_security = NULL;
+		kfree(isec);
+	}
+}
+
+/*
+ * For all inodes allocate inode->i_security(isec), before the security
+ * subsystem is enabled.
+ */
+static void ibac_fixup_inodes(void)
+{
+	struct super_block *sb;
+
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &super_blocks, s_list) {
+		struct inode *inode;
+
+		spin_unlock(&sb_lock);
+
+		spin_lock(&inode_lock);
+		list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+			spin_unlock(&inode_lock);
+
+			spin_lock(&inode->i_lock);
+			if (!inode->i_security)
+				ibac_inode_alloc_security(inode);
+			spin_unlock(&inode->i_lock);
+
+			spin_lock(&inode_lock);
+		}
+		spin_unlock(&inode_lock);
+
+		spin_lock(&sb_lock);
+	}
+	spin_unlock(&sb_lock);
+}
+
+static void ibac_d_instantiate(struct dentry *dentry, struct inode *inode)
+{
+	struct ibac_isec_data *isec;
+	char *xattr_flags = XATTR_SECURITY_PREFIX XATTR_MEASURE_SUFFIX;
+
+	if (!inode)
+		return;
+
+	if (!inode->i_op || !inode->i_op->getxattr)
+		return;
+
+	if (vfs_getxattr(dentry, xattr_flags, NULL, 0) > 0) {
+		isec = inode->i_security;
+		spin_lock(&isec->lock);
+		isec->measure = 1;
+		spin_unlock(&isec->lock);
+	}
+}
+
+static inline int is_kernel_thread(struct task_struct *tsk)
+{
+	return (!tsk->mm) ? 1 : 0;
+}
+
+static int verify_metadata_integrity(struct dentry *dentry)
+{
+	int rc, status;
+
+	if (!dentry)
+		return 0;
+
+	rc = integrity_verify_metadata(dentry, NULL, NULL, NULL, &status);
+	if (rc == -EOPNOTSUPP)
+		return 0;
+
+	if (rc < 0) {
+		printk(KERN_INFO "ibac: verify_metadata %s failed"
+		       "(rc: %d - status: %d)\n",
+		       dentry->d_name.name, rc, status);
+		if (!integrity_enforce)
+			rc = 0;
+		goto out;
+	}
+	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
+		if (!is_kernel_thread(current)) {
+			printk(KERN_INFO "ibac: verify_metadata %s "
+			       "(Integrity status: %s)\n",
+			       dentry->d_name.name,
+			       status == INTEGRITY_FAIL ? "FAIL" : "NOLABEL");
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+	return 0;
+out:
+	return rc;
+}
+
+static int verify_and_measure_integrity(struct dentry *dentry,
+					struct file *file,
+					char *filename, int mask)
+{
+	int rc, status;
+
+	if (!dentry && !file)
+		return 0;
+
+	rc = integrity_verify_data(dentry, file, &status);
+	if (rc < 0) {
+		printk(KERN_INFO "ibac: %s verify_data failed "
+		       "(rc: %d - status: %d)\n", filename, rc, status);
+		if (!integrity_enforce)
+			rc = 0;
+		goto out;
+	}
+
+	if (status != INTEGRITY_PASS) {
+		if (!is_kernel_thread(current)) {
+			printk(KERN_INFO "ibac: verify_data %s "
+			       "(Integrity status: FAIL)\n", filename);
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+
+	/* Only measure files that passed integrity verification. */
+	integrity_measure(dentry, file, filename, mask);
+	return 0;
+out:
+	/*
+	 * Verification failed, but as integrity is not being enforced,
+	 * we still need to measure.
+	 */
+	if (rc == 0)
+		integrity_measure(dentry, file, filename, mask);
+	return rc;
+}
+
+static int ibac_inode_permission(struct inode *inode, int mask,
+				 struct nameidata *nd)
+{
+	struct ibac_isec_data *isec = inode->i_security;
+	struct dentry *dentry;
+	char *path = NULL;
+	char *fname = NULL;
+	int rc = 0;
+	int measure;
+
+	dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
+	if (!dentry)
+		return 0;
+	if (nd) {		/* preferably use fullname */
+		path = (char *)__get_free_page(GFP_KERNEL);
+		if (path)
+			fname = d_path(nd->dentry, nd->mnt, path, PAGE_SIZE);
+	}
+
+	if (!fname)		/* no choice, use short name */
+		fname = (!dentry->d_name.name) ? (char *)dentry->d_iname :
+		    (char *)dentry->d_name.name;
+
+	/* Measure labeled files */
+	spin_lock(&isec->lock);
+	measure = isec->measure;
+	spin_unlock(&isec->lock);
+
+	if (S_ISREG(inode->i_mode) && (measure == 1)
+	    && (mask & MAY_READ)) {
+		rc = verify_metadata_integrity(dentry);
+		if (rc == 0)
+			rc = verify_and_measure_integrity(dentry, NULL,
+							  fname, mask);
+	}
+
+	/* Deny permission to write, if currently mmapped. */
+	if (inode && mask & MAY_WRITE) {
+		spin_lock(&isec->lock);
+		if (isec->mmapped > 0) {
+			printk(KERN_INFO "%s: %s - denied write access"
+			       " (isec=%d)\n",
+			       __FUNCTION__, fname, isec->mmapped);
+			rc = -EPERM;
+		}
+		spin_unlock(&isec->lock);
+	}
+
+	if (!nd || !nd->dentry)
+		dput(dentry);
+	if (path)
+		free_page((unsigned long)path);
+	return rc;
+}
+
+/*
+ * If the file is opened for writing, deny mmap(PROT_EXEC) access.
+ * Otherwise, increment the inode->i_security, which is our own
+ * writecount.  When the file is closed, f->f_security will be 1,
+ * and so we will decrement the inode->i_security.
+ * Just to be clear:
+ * 	file->f_security is 1 or 0.
+ * 	inode->i_security->mmapped is the *number* of processes which
+ * 		have this file mmapped(PROT_EXEC), so it can be >1.
+ */
+static int ibac_deny_write_access(struct file *file)
+{
+	struct inode *inode = file->f_dentry->d_inode;
+	struct ibac_isec_data *isec = inode->i_security;
+
+	spin_lock(&inode->i_lock);
+	if (atomic_read(&inode->i_writecount) > 0) {
+		spin_unlock(&inode->i_lock);
+		return -ETXTBSY;
+	}
+
+	spin_lock(&isec->lock);
+	isec->mmapped += 1;
+	spin_unlock(&isec->lock);
+	set_file_security(file, 1);
+	spin_unlock(&inode->i_lock);
+
+	return 0;
+}
+
+/*
+ * decrement our writer count on the inode.  When it hits 0, we will
+ * again allow opening the inode for writing.
+ */
+static void ibac_allow_write_access(struct file *file)
+{
+	struct inode *inode = file->f_dentry->d_inode;
+	struct ibac_isec_data *isec = inode->i_security;
+
+	spin_lock(&inode->i_lock);
+	spin_lock(&isec->lock);
+	isec->mmapped -= 1;
+	spin_unlock(&isec->lock);
+	set_file_security(file, 0);
+	spin_unlock(&inode->i_lock);
+}
+
+/*
+ * the file is being closed.  If we ever mmaped it for exec, then
+ * file->f_security>0, and we decrement the inode usage count to
+ * show that we are done with it.
+ */
+static void ibac_file_free_security(struct file *file)
+{
+	if (file->f_security)
+		ibac_allow_write_access(file);
+}
+
+/*
+ * We don't want to validate files which can be written while they are
+ * being executed.
+ * This means NFS.
+ */
+static inline int is_unprotected_file(struct file *file)
+{
+	if (strcmp(file->f_dentry->d_inode->i_sb->s_type->name, "nfs") == 0)
+		return 1;
+	return 0;
+}
+
+/*
+ * Verify data integrity, metadata integrity and measure it.
+ */
+static int ibac_file_mmap(struct file *file,
+			  unsigned long reqprot,
+			  unsigned long calcprot, unsigned long flags)
+{
+	unsigned long prot = reqprot;
+	int rc;
+
+	if (!(prot & VM_EXEC))
+		return 0;
+	if (!file || !file->f_dentry)
+		return 0;
+
+	if (is_unprotected_file(file))
+		return (-EPERM);
+	/*
+	 * if file_security is set, then this process has already
+	 * incremented the writer count on this inode, don't do
+	 * it again.
+	 */
+	if (get_file_security(file) == 0) {
+		rc = ibac_deny_write_access(file);
+		if (rc) {
+			if (S_ISCHR(file->f_dentry->d_inode->i_mode))
+				rc = 0;
+			goto out;
+		}
+	}
+
+	rc = verify_metadata_integrity(file->f_dentry);
+	if (rc == 0) {
+		char *fname = NULL;
+		char *path = NULL;
+
+		path = (char *)__get_free_page(GFP_KERNEL);
+		if (path)
+			fname = d_path(file->f_dentry, file->f_path.mnt,
+				       path, PAGE_SIZE);
+
+		if (!fname)	/* no choice, use short name */
+			fname = (!file->f_dentry->d_name.name) ?
+			    (char *)file->f_dentry->d_iname :
+			    (char *)file->f_dentry->d_name.name;
+
+		rc = verify_and_measure_integrity(NULL, file, fname, MAY_EXEC);
+		if (path)
+			free_page((unsigned long)path);
+	}
+out:
+	return rc;
+}
+
+static int ibac_bprm_check_security(struct linux_binprm *bprm)
+{
+	struct dentry *dentry = bprm->file->f_dentry;
+	int rc;
+
+	rc = verify_metadata_integrity(dentry);
+	if (rc == 0)
+		rc = verify_and_measure_integrity(dentry,
+						  bprm->file, bprm->filename,
+						  MAY_EXEC);
+	return rc;
+}
+
+static struct security_operations ibac_security_ops = {
+	.bprm_check_security = ibac_bprm_check_security,
+	.file_mmap = ibac_file_mmap,
+	.file_free_security = ibac_file_free_security,
+	.inode_alloc_security = ibac_inode_alloc_security,
+	.inode_free_security = ibac_inode_free_security,
+	.inode_permission = ibac_inode_permission,
+	.d_instantiate = ibac_d_instantiate
+};
+
+static int __init init_ibac(void)
+{
+	int rc;
+
+	if (!ibac_enabled)
+		return 0;
+
+	ibac_fixup_inodes();
+	rc = register_security(&ibac_security_ops);
+	if (rc != 0)
+		panic("ibac: unable to register with kernel\n");
+	return rc;
+}
+
+security_initcall(init_ibac);
Index: linux-2.6.22-rc4-mm2/security/Kconfig
===================================================================
--- linux-2.6.22-rc4-mm2.orig/security/Kconfig
+++ linux-2.6.22-rc4-mm2/security/Kconfig
@@ -114,5 +114,6 @@ config SECURITY_ROOTPLUG
 
 source security/selinux/Kconfig
 
+source security/ibac/Kconfig
 endmenu
 
Index: linux-2.6.22-rc4-mm2/security/Makefile
===================================================================
--- linux-2.6.22-rc4-mm2.orig/security/Makefile
+++ linux-2.6.22-rc4-mm2/security/Makefile
@@ -14,6 +14,7 @@ endif
 obj-$(CONFIG_SECURITY)			+= security.o dummy.o inode.o
 obj-$(CONFIG_INTEGRITY)		+= integrity.o integrity_dummy.o
 obj-$(CONFIG_IMA_MEASURE)		+= ima/
+obj-$(CONFIG_SECURITY_IBAC)		+= ibac/
 # Must precede capability.o in order to stack properly.
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
 obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
Index: linux-2.6.22-rc4-mm2/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.22-rc4-mm2.orig/Documentation/kernel-parameters.txt
+++ linux-2.6.22-rc4-mm2/Documentation/kernel-parameters.txt
@@ -42,6 +42,7 @@ parameter is applicable:
 	FB	The frame buffer device is enabled.
 	HW	Appropriate hardware is enabled.
 	IA-64	IA-64 architecture is enabled.
+	IBAC	Integrity Based Access Control support is enabled.
 	IMA	Integrity measurement architecture is enabled.
 	IOSCHED	More than one I/O scheduler is enabled.
 	IP_PNP	IP DHCP, BOOTP, or RARP is enabled.
@@ -761,6 +762,17 @@ and is between 256 and 4096 characters. 
 	ihash_entries=	[KNL]
 			Set number of hash buckets for inode cache.
 
+	ibac=		[IBAC] Disable or enable IBAC at boot time.
+                        Format: { "0" | "1" }
+                        See security/ibac/Kconfig help text.
+                        0 -- disable.
+                        1 -- enable.
+                        Default value is set via kernel config option.
+
+	ibac_enforce=	[IBAC] Enable integrity enforcing at boot time.
+                        Format: { "0" | "1" }
+                        Default is 0 to disable integrity enforcing.
+
 	ima=		[IMA] Disable or enable IMA at boot time.
 			Format: { "0" | "1" }
 			See security/ima/Kconfig help text.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC] [Patch 1/1] IBAC Patch
@ 2007-03-14  9:49 Mimi Zohar
  0 siblings, 0 replies; 16+ messages in thread
From: Mimi Zohar @ 2007-03-14  9:49 UTC (permalink / raw)
  To: linux-security-module
  Cc: safford, serue, kjhall, zohar, linux-kernel, disec-devel

This is a posting of an updated IBAC patch, based on comments from the
LSM and LKML mailing lists, which include the following fixes:
   - Updated Kconfig SECURITY_IBAC description
     and SECURITY_IBAC_BOOTPARAM default value
   - Prefixed all log messages with "ibac:"
   - Redefined a couple of 'int' variables as 'static int'

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,41 @@
+config SECURITY_IBAC
+	boolean "IBAC support"
+	depends on SECURITY && SECURITY_NETWORK && INTEGRITY
+	help
+	  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.
+
+config SECURITY_IBAC_BOOTPARAM
+	bool "IBAC boot parameter"
+	depends on SECURITY_IBAC
+	default n
+	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.
+
+config SECURITY_IBAC_BOOTPARAM_VALUE
+	int "IBAC boot parameter default value"
+	depends on SECURITY_IBAC_BOOTPARAM
+	range 0 1
+	default 0
+	help
+	  This option sets the default value for the kernel parameter
+	  'ibac', which allows IBAC to be disabled at boot.  If this
+	  option is set to 0 (zero), the IBAC kernel parameter will
+	  default to 0, disabling IBAC at bootup.  If this option is
+	  set to 1 (one), the IBAC kernel parameter will default to 1,
+	  enabling IBAC at bootup.
+
+	  If you are unsure how to answer this question, answer 0.
+
Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
===================================================================
--- /dev/null
+++ linux-2.6.21-rc3-mm2/security/ibac/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for building IBAC
+#
+
+obj-$(CONFIG_SECURITY_IBAC) += ibac.o
+ibac-y 	:= ibac_main.o
Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
===================================================================
--- /dev/null
+++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
@@ -0,0 +1,126 @@
+/*
+ * Integrity Based Access Control (IBAC)
+ *
+ * Copyright (C) 2007 IBM Corporation
+ * Author: Mimi Zohar <zohar@us.ibm.com>
+ *
+ *      This program is free software; you can redistribute it and/or modify
+ *      it under the terms of the GNU General Public License as published by
+ *      the Free Software Foundation, version 2 of the License.
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/security.h>
+#include <linux/integrity.h>
+
+#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
+static int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
+
+static int __init ibac_enabled_setup(char *str)
+{
+	ibac_enabled = simple_strtol(str, NULL, 0);
+	return 1;
+}
+
+__setup("ibac=", ibac_enabled_setup);
+#else
+static int ibac_enabled = 0;
+#endif
+
+static unsigned int integrity_enforce;
+static int __init integrity_enforce_setup(char *str)
+{
+	integrity_enforce = simple_strtol(str, NULL, 0);
+	return 1;
+}
+
+__setup("ibac_enforce=", integrity_enforce_setup);
+
+#define XATTR_NAME "security.evm.hash"
+
+static inline int is_kernel_thread(struct task_struct *tsk)
+{
+	return (!tsk->mm) ? 1 : 0;
+}
+
+static int ibac_bprm_check_security(struct linux_binprm *bprm)
+{
+	struct dentry *dentry = bprm->file->f_dentry;
+	int xattr_len;
+	char *xattr_value = NULL;
+	int rc, status;
+
+	rc = integrity_verify_metadata(dentry, XATTR_NAME,
+				       &xattr_value, &xattr_len, &status);
+	if (rc == -EOPNOTSUPP) {
+		kfree(xattr_value);
+		return 0;
+	}
+
+	if (rc < 0) {
+		printk(KERN_INFO "ibac: verify_metadata %s failed "
+		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
+		if (!integrity_enforce)
+			rc = 0;
+		goto out;
+	}
+	if (status != INTEGRITY_PASS) {	/* FAIL | NO_LABEL */
+		if (!is_kernel_thread(current)) {
+			printk(KERN_INFO "ibac: verify_metadata %s "
+			       "(Integrity status: FAIL)\n", bprm->filename);
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+
+	rc = integrity_verify_data(dentry, &status);
+	if (rc < 0) {
+		printk(KERN_INFO "ibac: %s verify_data failed "
+		       "(rc: %d - status: %d)\n", bprm->filename, rc, status);
+		if (!integrity_enforce)
+			rc = 0;
+		goto out;
+	}
+	if (status != INTEGRITY_PASS) {
+		if (!is_kernel_thread(current)) {
+			printk(KERN_INFO "ibac: verify_data %s "
+			       "(Integrity status: FAIL)\n", bprm->filename);
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+
+	kfree(xattr_value);
+
+	/* measure all integrity level executables */
+	integrity_measure(dentry, bprm->filename, MAY_EXEC);
+	return 0;
+out:
+	kfree(xattr_value);
+	return rc;
+}
+
+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");
+	return rc;
+}
+
+security_initcall(init_ibac);
Index: linux-2.6.21-rc3-mm2/security/Kconfig
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/Kconfig
+++ linux-2.6.21-rc3-mm2/security/Kconfig
@@ -125,5 +125,6 @@ config SECURITY_ROOTPLUG
 source security/selinux/Kconfig
 
 source security/slim/Kconfig
+source security/ibac/Kconfig
 endmenu
 
Index: linux-2.6.21-rc3-mm2/security/Makefile
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/Makefile
+++ linux-2.6.21-rc3-mm2/security/Makefile
@@ -14,6 +14,7 @@ endif
 obj-$(CONFIG_SECURITY)			+= security.o dummy.o inode.o
 obj-$(CONFIG_INTEGRITY)		+= integrity.o integrity_dummy.o
 obj-$(CONFIG_INTEGRITY_EVM)		+= evm/
+obj-$(CONFIG_SECURITY_IBAC)		+= ibac/
 # Must precede capability.o in order to stack properly.
 obj-$(CONFIG_SECURITY_SLIM)		+= slim/
 obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2007-06-20 11:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-08 22:58 [RFC] [Patch 1/1] IBAC Patch Mimi Zohar
2007-03-08 23:08 ` Randy Dunlap
2007-03-09 13:19   ` Mimi Zohar
2007-03-09 18:26     ` Randy Dunlap
2007-03-09  3:19 ` Valdis.Kletnieks
2007-03-09 15:07   ` Serge E. Hallyn
2007-03-12 21:47   ` Mimi Zohar
2007-03-13 15:31     ` Serge E. Hallyn
2007-03-14  9:46       ` Mimi Zohar
2007-03-14  2:27 ` Seth Arnold
2007-03-14 11:25   ` Mimi Zohar
2007-03-14 18:48     ` Seth Arnold
2007-03-14  9:49 Mimi Zohar
2007-06-18 20:48 [RFC][Patch " Mimi Zohar
2007-06-19 22:23 ` Serge E. Hallyn
2007-06-20 11:52   ` Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).