Skip to contentSkip to conversation

IBAC Patch

RFC PATCH In review Mimi Zohar 6 participants 12 messages Mar 8, 2007 – Mar 14, 2007 Security (LSM)

lore
Share & tools
lore

Mimi Zohar

author
#0 lore raw

RFC PATCH 1/1 IBAC Patch

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

security/ibac/Kconfig new file +36 -0
Index: 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.
+
security/ibac/Makefile new file +6 -0
Index: 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
security/ibac/ibac_main.c new file +126 -0

126 lines not on this page · raw message

security/Kconfig +1 -0
Index: 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
 
security/Makefile +1 -0
Index: 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
#1 lore raw
Quoted 12 lines · 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
===================================================================

security/ibac/Kconfig line 6

Quoted 6 lines
security/ibac/Kconfig new file +6 -0
@@ -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)

security/ibac/Kconfig line 20

Quoted 13 lines
security/ibac/Kconfig new file +13 -0
+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?

Quoted 30 lines
security/ibac/Kconfig new file +15 -0
+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
===================================================================

security/ibac/ibac_main.c new file +9 -0
@@ -0,0 +1,126 @@
+/*
+ * Integrity Based Access Control (IBAC)
+ *
+ * Copyright (C) 2007 IBM Corporation
+ * Author: Mimi Zohar <[email protected]>
+ *
+ *      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.

security/ibac/ibac_main.c line 19

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -0
+ */
+
+#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 ?

security/ibac/ibac_main.c line 29

Quoted 9 lines
security/ibac/ibac_main.c new file +9 -0
+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 ?

security/ibac/ibac_main.c line 32

Quoted 3 lines
security/ibac/ibac_main.c new file +3 -0
+#endif
+
+static unsigned int integrity_enforce = 0;

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

Quoted 15 lines
security/ibac/ibac_main.c new file +15 -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;
+}
+

security/ibac/ibac_main.c line 57

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -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)
?

security/ibac/ibac_main.c line 64

Quoted 7 lines
security/ibac/ibac_main.c new file +7 -0
+		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?

Quoted 30 lines
security/ibac/ibac_main.c new file +30 -0
+		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;

security/ibac/ibac_main.c line 104

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -0
+			}
+		}
+	}
+
+	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.

Quoted 8 lines
security/ibac/ibac_main.c new file +8 -0
+	kfree(xattr_value);
+	return rc;
+}
+
+static struct security_operations ibac_security_ops = {
+	.bprm_check_security = ibac_bprm_check_security
+};
+

security/ibac/ibac_main.c line 122

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -0
+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??

security/ibac/ibac_main.c line 126

Quoted 4 lines
security/ibac/ibac_main.c new file +4 -0
+	return rc;
+}
+
+security_initcall(init_ibac);

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

author
#2 lore raw
Quoted 9 lines · Randy Dunlap
Quoted 7 lines · 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.)

Quoted 16 lines

security/ibac/Kconfig line 6

Quoted 11 lines

Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
===================================================================

security/ibac/Kconfig new file +6 -0
@@ -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.

Quoted 8 lines
Quoted 8 lines
security/ibac/Kconfig new file +8 -0
+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
Quoted 8 lines

security/ibac/Kconfig line 20

Quoted 5 lines
security/ibac/Kconfig new file +5 -0
+	  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.

Quoted 10 lines
Quoted 10 lines
security/ibac/Kconfig new file +10 -0
+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;
Quoted 8 lines

security/ibac/Kconfig line 33

Quoted 4 lines
security/ibac/Kconfig new file +4 -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

#3 lore raw
Quoted 29 lines · On Fri, 09 Mar 2007 08:19:36 -0500 Mimi Zohar wrote:
Quoted 9 lines · Randy Dunlap
Quoted 7 lines · 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.)

Quoted 16 lines

security/ibac/Kconfig line 6

Quoted 11 lines

Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
===================================================================

security/ibac/Kconfig new file +6 -0
@@ -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:

Quoted 7 lines

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.

Quoted 39 lines
Quoted 8 lines
Quoted 8 lines
security/ibac/Kconfig new file +8 -0
+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
Quoted 8 lines

security/ibac/Kconfig line 20

Quoted 5 lines
security/ibac/Kconfig new file +5 -0
+	  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.

Quoted 10 lines
Quoted 10 lines
security/ibac/Kconfig new file +10 -0
+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;
Quoted 8 lines

security/ibac/Kconfig line 33

Quoted 4 lines
security/ibac/Kconfig new file +4 -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??

Quoted 5 lines

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

#4 lore raw
In reply to Mimi Zohar
Quoted 6 lines · Mimi Zohar

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

Quoted 1 line

Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig

Minor congnitive-dissonance alert:

security/ibac/Kconfig line 20

Quoted 6 lines
security/ibac/Kconfig new file +5 -0
+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.

#5 lore raw
Quoted 5 lines · Quoting [email protected] ([email protected]):
Quoted 4 lines · Mimi Zohar

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.

Quoted 8 lines
Quoted 2 lines

(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

author
#6 lore raw
In reply to [email protected]
Quoted 5 lines · On Thu, 2007-03-08 at 22:19 -0500, [email protected] wrote:
Quoted 4 lines · Mimi Zohar

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.

Quoted 8 lines
Quoted 2 lines

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

Quoted 4 lines
Quoted 1 line

Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig

Minor congnitive-dissonance alert:

Quoted 8 lines

security/ibac/Kconfig line 20

Quoted 6 lines
security/ibac/Kconfig new file +5 -0
+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

maintainer
#7 lore raw
Quoted 15 lines · Mimi Zohar
Quoted 5 lines · On Thu, 2007-03-08 at 22:19 -0500, [email protected] wrote:
Quoted 4 lines · Mimi Zohar

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.

Quoted 8 lines
Quoted 2 lines

(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).

Quoted 4 lines

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

author
#8 lore raw
Quoted 16 lines · Serge E. Hallyn
Quoted 15 lines · Mimi Zohar
Quoted 13 lines · On Thu, 2007-03-08 at 22:19 -0500, [email protected] wrote:
Quoted 6 lines · Mimi Zohar

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

Quoted 6 lines
Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 5 lines

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.

Quoted 3 lines

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

#9 lore raw
In reply to Mimi Zohar
Quoted 3 lines · Mimi Zohar

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.

Quoted 2 lines

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

Quoted 36 lines

Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
===================================================================

security/ibac/Makefile new file +6 -0
@@ -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
===================================================================

security/ibac/ibac_main.c new file +20 -0
@@ -0,0 +1,126 @@
+/*
+ * Integrity Based Access Control (IBAC)
+ *
+ * Copyright (C) 2007 IBM Corporation
+ * Author: Mimi Zohar <[email protected]>
+ *
+ *      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;
+

security/ibac/ibac_main.c line 30

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -0
+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?

security/ibac/ibac_main.c line 41

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -0
+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?

Quoted 18 lines
security/ibac/ibac_main.c new file +18 -0
+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;
+	}

security/ibac/ibac_main.c line 70

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -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?

security/ibac/ibac_main.c line 70

Quoted 2 lines
security/ibac/ibac_main.c new file +2 -0
+			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?

Quoted 7 lines
security/ibac/ibac_main.c new file +7 -0
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+

security/ibac/ibac_main.c line 89

Quoted 10 lines
security/ibac/ibac_main.c new file +10 -0
+	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?

security/ibac/ibac_main.c line 91

Quoted 2 lines
security/ibac/ibac_main.c new file +2 -0
+			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?)

security/ibac/ibac_main.c line 103

Quoted 12 lines
security/ibac/ibac_main.c new file +12 -0
+			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

author
#10 lore raw
Quoted 5 lines · Seth Arnold
Quoted 3 lines · Mimi Zohar

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.

Quoted 8 lines

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

Quoted 2 lines

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

Quoted 41 lines
Quoted 41 lines

Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
===================================================================

security/ibac/Makefile new file +6 -0
@@ -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
===================================================================

security/ibac/ibac_main.c new file +25 -0
@@ -0,0 +1,126 @@
+/*
+ * Integrity Based Access Control (IBAC)
+ *
+ * Copyright (C) 2007 IBM Corporation
+ * Author: Mimi Zohar <[email protected]>
+ *
+ *      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;
+}
Quoted 8 lines

security/ibac/ibac_main.c line 30

Quoted 5 lines
security/ibac/ibac_main.c new file +5 -0
+
+__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.

Quoted 5 lines
Quoted 5 lines
security/ibac/ibac_main.c new file +5 -0
+static unsigned int integrity_enforce = 0;
+static int __init integrity_enforce_setup(char *str)
+{
+	integrity_enforce = simple_strtol(str, NULL, 0);
+	return 1;
Quoted 8 lines

security/ibac/ibac_main.c line 41

Quoted 5 lines
security/ibac/ibac_main.c new file +5 -0
+}
+
+__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.

Quoted 22 lines
Quoted 22 lines
security/ibac/ibac_main.c new file +22 -0
+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);
Quoted 8 lines

security/ibac/ibac_main.c line 70

Quoted 6 lines
security/ibac/ibac_main.c new file +6 -0
+		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.

Quoted 5 lines

security/ibac/ibac_main.c line 70

Quoted 2 lines
security/ibac/ibac_main.c new file +2 -0
+			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.

Quoted 11 lines
Quoted 11 lines
security/ibac/ibac_main.c new file +11 -0
+			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);
Quoted 8 lines

security/ibac/ibac_main.c line 89

Quoted 6 lines
security/ibac/ibac_main.c new file +6 -0
+		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.

Quoted 5 lines

security/ibac/ibac_main.c line 91

Quoted 2 lines
security/ibac/ibac_main.c new file +2 -0
+			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.

Quoted 9 lines
Quoted 9 lines
security/ibac/ibac_main.c new file +9 -0
+			if (integrity_enforce) {
+				rc = -EACCES;
+				goto out;
+			}
+		}
+	}
+
+	kfree(xattr_value);
+
Quoted 8 lines

security/ibac/ibac_main.c line 103

Quoted 3 lines
security/ibac/ibac_main.c new file +3 -0
+	/* 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.

Quoted 1 line

Rest elided :) Thanks Mimi

Thank you for your comments.

Mimi Zohar

#11 lore raw
Quoted 1 line · Mimi Zohar

It's a little bit of both. :-) Initially it was written to help me with

:)

Quoted 4 lines

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.

Quoted 7 lines
Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+	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..)

Quoted 5 lines

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.

Quoted 1 line

Thank you for your comments.

My pleasure, thanks for the quick responses.