[RFC][Patch 1/6] integrity: new hooks

[RFC][Patch 1/6] integrity: new hooks

[Mimi Zohar]

This patch adds integrity hooks used to implement an integrity service
provider and updates the previously submitted dummy provider to
support these new hooks.

Index: linux-2.6.21-rc3-mm2/security/integrity_dummy.c
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/integrity_dummy.c
+++ linux-2.6.21-rc3-mm2/security/integrity_dummy.c
@@ -3,7 +3,7 @@
  *
  * Instantiate integrity subsystem
  *
- * Copyright (C) 2005,2006 IBM Corporation
+ * Copyright (C) 2005,2006,2007 IBM Corporation
  * Author: Mimi Zohar <zohar@us.ibm.com>
  *
  *      This program is free software; you can redistribute it and/or modify
@@ -62,9 +62,78 @@ static void dummy_measure(struct dentry 
 	return;
 }
 
+static int dummy_inode_alloc_integrity(struct inode *inode)
+{
+	return 0;
+}
+
+static void dummy_inode_free_integrity(struct inode *inode)
+{
+	return;
+}
+
+static void dummy_inode_init_integrity(struct inode *inode, struct inode *dir,
+				      char **name, void **value, size_t * len)
+{
+	return;
+}
+
+static void dummy_file_free_integrity(struct file *file)
+{
+	return;
+}
+
+static int dummy_inode_setxattr(struct dentry *dentry, char *name, void *value,
+				size_t size, int flags)
+{
+	if (!strncmp(name, XATTR_SECURITY_PREFIX,
+		     sizeof(XATTR_SECURITY_PREFIX) - 1) &&
+	    !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+	return 0;
+}
+
+static void dummy_inode_post_setxattr(struct dentry *dentry, char *name)
+{
+}
+
+static void dummy_d_instantiate(struct dentry *dentry, struct inode *inode)
+{
+	return;
+}
+
 struct integrity_operations dummy_integrity_ops = {
 	.verify_metadata = dummy_verify_metadata,
 	.verify_data = dummy_verify_data,
-	.measure = dummy_measure
+	.measure = dummy_measure,
+	.inode_setxattr = dummy_inode_setxattr,
+	.inode_post_setxattr = dummy_inode_post_setxattr,
+	.inode_alloc_integrity = dummy_inode_alloc_integrity,
+	.inode_init_integrity = dummy_inode_init_integrity,
+	.inode_free_integrity = dummy_inode_free_integrity,
+	.file_free_integrity = dummy_file_free_integrity,
+	.d_instantiate = dummy_d_instantiate
 };
 
+#define set_to_dummy_if_null(ops, function)				\
+	do {								\
+		if (!ops->function) {					\
+			ops->function = dummy_##function;		\
+			printk(KERN_INFO "Had to override the " #function \
+			" security operation with the dummy one.\n");\
+			}						\
+	} while (0)
+
+void integrity_fixup_ops(struct integrity_operations *ops)
+{
+	set_to_dummy_if_null(ops, verify_metadata);
+	set_to_dummy_if_null(ops, verify_data);
+	set_to_dummy_if_null(ops, measure);
+	set_to_dummy_if_null(ops, inode_setxattr);
+	set_to_dummy_if_null(ops, inode_post_setxattr);
+	set_to_dummy_if_null(ops, inode_alloc_integrity);
+	set_to_dummy_if_null(ops, inode_init_integrity);
+	set_to_dummy_if_null(ops, inode_free_integrity);
+	set_to_dummy_if_null(ops, file_free_integrity);
+	set_to_dummy_if_null(ops, d_instantiate);
+}
Index: linux-2.6.21-rc3-mm2/security/integrity_dummy.h
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/integrity_dummy.h
+++ linux-2.6.21-rc3-mm2/security/integrity_dummy.h
@@ -1,7 +1,7 @@
 /*
  * integrity_dummy.h
  *
- * Copyright (C) 2005,2006 IBM Corporation
+ * Copyright (C) 2005,2006,2007 IBM Corporation
  * Author: Mimi Zohar <zohar@us.ibm.com>
  *
  *      This program is free software; you can redistribute it and/or modify
@@ -10,3 +10,4 @@
  */
 
 extern struct integrity_operations dummy_integrity_ops;
+extern void integrity_fixup_ops(struct integrity_operations *ops);
Index: linux-2.6.21-rc3-mm2/include/linux/integrity.h
===================================================================
--- linux-2.6.21-rc3-mm2.orig/include/linux/integrity.h
+++ linux-2.6.21-rc3-mm2/include/linux/integrity.h
@@ -1,7 +1,7 @@
 /*
  * integrity.h
  *
- * Copyright (C) 2005,2006 IBM Corporation
+ * Copyright (C) 2005,2006,2007 IBM Corporation
  * Author: Mimi Zohar <zohar@us.ibm.com>
  *
  *      This program is free software; you can redistribute it and/or modify
@@ -45,6 +45,45 @@
  *	@filename either contains the full pathname/short file name.
  *	@mask contains the filename permission status(i.e. read, write, append).
  *
+ * @inode_setxattr:
+ *	Check permission before permitting an integrity extended attribute
+ * 	to be set.
+ *	@value identified by @name for @dentry.
+ *	Return 0 if permission is granted.
+ *
+ * @inode_post_setxattr:
+ *	Update inode integrity xattr after successful setxattr operation.
+ * 	identified by @name for @dentry.
+ *
+ * @inode_alloc_integrity:
+ *	Allocate and attach an integrity structure to @inode->i_integrity.  The
+ * 	i_integrity field is initialized to NULL when the inode structure is
+ * 	allocated.
+ * 	@inode contains the inode structure.
+ * 	Return 0 if operation was successful.
+ *
+ * @inode_free_integrity:
+ *	@inode contains the inode structure.
+ * 	Deallocate the inode integrity structure and set @inode->i_integrity to
+ * 	NULL.
+ *
+ * @inode_init_integrity:
+ *	Create inode integrity xattr for a new inode based on the new security
+ * 	xattr information.
+ *	@inode contains the inode structure of the newly created inode.
+ *	@dir contains the inode structure of the parent directory.
+ * 	@name contains the security xattr name suffix.
+ *	@value contains the security attribute value.
+ *	@len contains the length of the security attribute value.
+ *
+ * @file_free_integrity:
+ *	Update the integrity xattr value as necessary.
+ * 	*file contains the file structure being closed.
+ *
+ * @d_instantiate:
+ *	Initialize the integrity structure of an inode for a dentry.
+ *	@dentry to complete.
+ *	@inode to attach to this dentry.
  */
 
 #define PASS_STR "INTEGRITY_PASS"
@@ -57,13 +96,22 @@ struct integrity_operations {
 	int (*verify_data) (struct dentry *dentry, int *status);
 	void (*measure) (struct dentry *dentry,
 			const unsigned char *filename, int mask);
+	int (*inode_setxattr) (struct dentry *dentry, char *name, void *value,
+			       size_t size, int flags);
+	void (*inode_post_setxattr) (struct dentry *dentry, char *name);
+	int (*inode_alloc_integrity) (struct inode *inode);
+	void (*inode_free_integrity) (struct inode *inode);
+	void (*inode_init_integrity) (struct inode *inode, struct inode *dir,
+				    char **name, void **value, size_t *len);
+	void (*file_free_integrity) (struct file * file);
+	void (*d_instantiate) (struct dentry *dentry, struct inode *inode);
 };
 extern int register_integrity(struct integrity_operations *ops);
 extern int unregister_integrity(struct integrity_operations *ops);
 
 /* global variables */
 extern struct integrity_operations *integrity_ops;
-enum integrity_verify_status {
+enum integrity_status {
 	INTEGRITY_PASS = 0, INTEGRITY_FAIL = -1, INTEGRITY_NOLABEL = -2
 };
 
@@ -77,8 +125,7 @@ static inline int integrity_verify_metad
 			xattr_value, xattr_val_len, status);
 }
 
-static inline int integrity_verify_data(struct dentry *dentry,
-					int *status)
+static inline int integrity_verify_data(struct dentry *dentry, int *status)
 {
 	return integrity_ops->verify_data(dentry, status);
 }
@@ -88,6 +135,55 @@ static inline void integrity_measure(str
 {
 	integrity_ops->measure(dentry, filename, mask);
 }
+
+static inline int integrity_inode_setxattr(struct dentry *dentry, char *name,
+					   void *value, size_t size, int flags)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return integrity_ops->inode_setxattr(dentry, name, value, size, flags);
+}
+
+static inline void integrity_inode_post_setxattr(struct dentry *dentry, char *name)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return;
+ 	integrity_ops->inode_post_setxattr(dentry, name);
+}
+
+static inline int integrity_inode_alloc(struct inode *inode)
+{
+	return integrity_ops->inode_alloc_integrity(inode);
+}
+
+static inline void integrity_inode_free(struct inode *inode)
+{
+	integrity_ops->inode_free_integrity(inode);
+}
+
+static inline void integrity_inode_init_integrity(struct inode *inode,
+						struct inode *dir,
+						char **name,
+						void **value,
+						size_t *len)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return;
+	integrity_ops->inode_init_integrity(inode, dir, name, value, len);
+	return;
+}
+
+static inline void integrity_file_free(struct file *file)
+{
+	integrity_ops->file_free_integrity(file);
+}
+
+static inline void integrity_d_instantiate(struct dentry *dentry, struct inode *inode)
+{
+	if (unlikely (inode && IS_PRIVATE(inode)))
+		return;
+	integrity_ops->d_instantiate(dentry, inode);
+}
 #else
 static inline int integrity_verify_metadata(struct dentry *dentry,
 			char *xattr_name, char **xattr_value,
@@ -97,8 +193,7 @@ static inline int integrity_verify_metad
 	return 0;
 }
 
-static inline int integrity_verify_data(struct dentry *dentry,
-					int *status)
+static inline int integrity_verify_data(struct dentry *dentry, int *status)
 {
 	status = INTEGRITY_PASS;
 	return 0;
@@ -108,5 +203,47 @@ static inline void integrity_measure(str
 			const unsigned char *filename, int mask)
 {
 }
+
+static inline int integrity_inode_setxattr(struct dentry *dentry, char *name,
+					   void *value, size_t size, int flags)
+{
+	return 0;
+}
+
+static inline void integrity_inode_post_setxattr(struct dentry *dentry, char *name)
+{ }
+
+static inline int integrity_inode_alloc(struct inode *inode)
+{
+	return 0;
+}
+
+static inline void integrity_inode_free(struct inode *inode)
+{ }
+
+static inline int integrity_inode_init_integrity(struct inode *inode,
+						struct inode *dir,
+						char **name,
+						void **value,
+						size_t *len)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int integrity_file_permission(struct file *file, int mask)
+{
+	return 0;
+}
+
+static inline void integrity_file_free(struct file *file)
+{
+	return;
+}
+
+static inline void integrity_d_instantiate(struct dentry *dentry, struct inode *inode)
+{
+	return;
+}
+
 #endif
 #endif
Index: linux-2.6.21-rc3-mm2/security/integrity.c
===================================================================
--- linux-2.6.21-rc3-mm2.orig/security/integrity.c
+++ linux-2.6.21-rc3-mm2/security/integrity.c
@@ -3,7 +3,7 @@
  *
  * register integrity subsystem
  *
- * Copyright (C) 2005,2006 IBM Corporation
+ * Copyright (C) 2005,2006,2007 IBM Corporation
  * Author: Mimi Zohar <zohar@us.ibm.com>
  *
  *      This program is free software; you can redistribute it and/or modify
@@ -27,6 +27,7 @@ int register_integrity(struct integrity_
 		return -EAGAIN;
 
 	integrity_ops = ops;
+	integrity_fixup_ops(integrity_ops);
 	return 0;
 }
 

Re: [RFC][Patch 1/6] integrity: new hooks

[Chris Wright]

* Mimi Zohar (zohar@linux.vnet.ibm.com) wrote:
> +static int dummy_inode_setxattr(struct dentry *dentry, char *name, void *value,
> +				size_t size, int flags)
> +{
> +	if (!strncmp(name, XATTR_SECURITY_PREFIX,
> +		     sizeof(XATTR_SECURITY_PREFIX) - 1) &&
> +	    !capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +	return 0;
> +}

Hold on, what is all this?  Duplication of all of this code is a no go.

thanks,
-chris

Re: [RFC][Patch 1/6] integrity: new hooks

[Serge E. Hallyn]

Quoting Chris Wright (chrisw@sous-sol.org):
> * Mimi Zohar (zohar@linux.vnet.ibm.com) wrote:
> > +static int dummy_inode_setxattr(struct dentry *dentry, char *name, void *value,
> > +				size_t size, int flags)
> > +{
> > +	if (!strncmp(name, XATTR_SECURITY_PREFIX,
> > +		     sizeof(XATTR_SECURITY_PREFIX) - 1) &&
> > +	    !capable(CAP_SYS_ADMIN))
> > +		return -EPERM;
> > +	return 0;
> > +}
> 
> Hold on, what is all this?  Duplication of all of this code is a no go.

It's unfortunate, agreed, but

use of LSM as an integrity framework was also a no-go.

Options?

thanks,
-serge

Re: [RFC][Patch 1/6] integrity: new hooks

[Chris Wright]

* Serge E. Hallyn (serue@us.ibm.com) wrote:
> It's unfortunate, agreed, but
> use of LSM as an integrity framework was also a no-go.
> 
> Options?

There's too much dup because stuff like above is just access control
not integrity measurement.  Need to break off the parts that really
are different.  But I don't have a great option offhand, I recognize
the complication.

thanks,
-chris

Re: [RFC][Patch 1/6] integrity: new hooks

[Casey Schaufler]

--- "Serge E. Hallyn" <serue@us.ibm.com> wrote:

 
> It's unfortunate, agreed, but
> 
> use of LSM as an integrity framework was also a
> no-go.

You're going to have to justify this assertion.
I know of at least one work-in-progress for which
LSM works just fine. Not to mention the Integrity
claims of SELinux.


Casey Schaufler
casey@schaufler-ca.com

Re: [RFC][Patch 1/6] integrity: new hooks

[Chris Wright]

* Casey Schaufler (casey@schaufler-ca.com) wrote:
> 
> --- "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> 
>  
> > It's unfortunate, agreed, but
> > 
> > use of LSM as an integrity framework was also a
> > no-go.
> 
> You're going to have to justify this assertion.
> I know of at least one work-in-progress for which
> LSM works just fine. Not to mention the Integrity
> claims of SELinux.

Serge is referring to the objections to EVM/IMA
as LSM because it was not doing access control,
rather integrity measurements.  It's all in
somewhere the archives.

thanks,
-chris

Re: [RFC][Patch 1/6] integrity: new hooks

[Serge E. Hallyn]

Quoting Casey Schaufler (casey@schaufler-ca.com):
> 
> --- "Serge E. Hallyn" <serue@us.ibm.com> wrote:
> 
> 
> > It's unfortunate, agreed, but
> > 
> > use of LSM as an integrity framework was also a
> > no-go.
> 
> You're going to have to justify this assertion.

You misunderstand.  I wasn't saying it wouldn't work :)  I was saying
that it's been said repeatedly that evm should be implemented as an
integrity, not security, module.

I think it should be done as both.  The part which measures the
integrity of files should be an integrity subsystem.  The part which
uses those results to either allow/refuse actions or take some other
action (i.e. shut down the system) should be an lsm.

> I know of at least one work-in-progress for which
> LSM works just fine. Not to mention the Integrity
> claims of SELinux.

-serge

Re: [RFC][Patch 1/6] integrity: new hooks

[Valdis.Kletnieks]

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

On Thu, 08 Mar 2007 12:46:47 CST, "Serge E. Hallyn" said:
> I think it should be done as both.  The part which measures the
> integrity of files should be an integrity subsystem.  The part which
> uses those results to either allow/refuse actions or take some other
> action (i.e. shut down the system) should be an lsm.

That would be good - the allow/deny parts, being security, can use the
existing LSM hooks, and the integrity part can use the LIM hooks.

Umm... wait a minute - *what* Linux Integrity Module hooks? :)

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

Re: [RFC][Patch 1/6] integrity: new hooks

[Serge E. Hallyn]

Quoting Valdis.Kletnieks@vt.edu (Valdis.Kletnieks@vt.edu):
> On Thu, 08 Mar 2007 12:46:47 CST, "Serge E. Hallyn" said:
> > I think it should be done as both.  The part which measures the
> > integrity of files should be an integrity subsystem.  The part which
> > uses those results to either allow/refuse actions or take some other
> > action (i.e. shut down the system) should be an lsm.
> 
> That would be good - the allow/deny parts, being security, can use the
> existing LSM hooks, and the integrity part can use the LIM hooks.

Yes, the only problem is that there will still be 'duplication' at some
call sites.  For instance even if something is only doing measurements, at
setxattr, for instance, it still needs to be able to refuse a change if
it can't allocate memory to store measurement data.  So we still might
need an fsnotify-like approach to consolidate security and integrity
hooks.  (As Mimi pointed out, I guess it'll have to make sure to call
the security ones first)

I suppose something more clever might be doable - for instance since we
are pairing the measurement module with an enforcement module, the
enforcement module could kmalloc() what the measurement module will need
for data and return -ENOMEM if need be, then the measurement module just
fills the data in.  If there's no place for it to store the data, then
it knows that the enforcement module must be ok not having that data.

> Umm... wait a minute - *what* Linux Integrity Module hooks? :)

-serge

Re: [RFC][Patch 1/6] integrity: new hooks

[Mimi Zohar]

On Thu, 2007-03-08 at 15:38 -0500, Valdis.Kletnieks@vt.edu wrote:
> On Thu, 08 Mar 2007 12:46:47 CST, "Serge E. Hallyn" said:
> > I think it should be done as both.  The part which measures the
> > integrity of files should be an integrity subsystem.  The part which
> > uses those results to either allow/refuse actions or take some other
> > action (i.e. shut down the system) should be an lsm.
> 
> That would be good - the allow/deny parts, being security, can use the
> existing LSM hooks, and the integrity part can use the LIM hooks.
> 
> Umm... wait a minute - *what* Linux Integrity Module hooks? :)

Hm, integrity-service-api-and-dummy-provider.patch contains:
integrity_verify_metadata, integrity_verify_data, and integrity_measure,
which could be referred to as either LIM hooks or as the API. This patch 
set adds 8 new LIM hooks.

Mimi Zohar