[RFC][PATCH] Enable atomic inode security labeling

From: Stephen Smalley (sds@private)
Date: Tue Jul 05 2005 - 08:58:17 PDT


This patch enables atomic security labeling of newly created inodes by
altering the fs code (presently only ext3, but the same approach
should generalize to other filesystems) to invoke a new LSM hook to
obtain the security attribute to apply to a newly created inode and to
set up the incore inode security state during the inode creation
transaction.  This parallels the existing processing for setting ACLs
on newly created inodes.  Otherwise, it is possible for new inodes to
be accessed by another thread via the dcache prior to complete
security setup (presently handled by the post_create/mkdir/... LSM
hooks in the VFS) and a newly created inode may be left unlabeled on
the disk in the event of a crash.  SELinux presently works around the
issue by ensuring that the incore inode security label is initialized to a
special SID that is inaccessible to unprivileged processes (in accordance 
with policy), thereby preventing inappropriate access but potentially
causing false denials on legitimate accesses.

A trivial test program that demonstrates such a false denial follows in
the next message.  The patch and test program were written by a
colleague here who prefers to remain unnamed.

Let me know if you see any reason to not send this along as an rfc to
linux-fsdevel.  For actual submission, I expect that we would also
include the necessary patch for ext2, but defer corresponding updates
to reiserfs, jfs and xfs to the maintainers of those filesystems, and
continue to fall back on the post_create/mkdir/... LSM hooks in the
VFS for those filesystems in the interim.

 fs/ext3/ialloc.c                  |    5 +++
 fs/ext3/xattr.h                   |    1 
 fs/ext3/xattr_security.c          |   22 +++++++++++++
 include/linux/security.h          |   41 +++++++++++++++++++++++++
 security/dummy.c                  |    7 ++++
 security/selinux/hooks.c          |   60 ++++++++++++++++++++++++++++++++++++++
 security/selinux/include/objsec.h |    1 
 7 files changed, 137 insertions(+)

diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext3/ialloc.c linux-2.6.13/fs/ext3/ialloc.c
--- linux-2.6.13.clean/fs/ext3/ialloc.c	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/fs/ext3/ialloc.c	2005-06-30 13:24:23.000000000 -0400
@@ -606,6 +606,11 @@ got:
 		DQUOT_FREE_INODE(inode);
 		goto fail2;
   	}
+	err = ext3_init_security(handle,inode, dir);
+	if (err) {
+		DQUOT_FREE_INODE(inode);
+		goto fail2;
+	}
 	err = ext3_mark_inode_dirty(handle, inode);
 	if (err) {
 		ext3_std_error(sb, err);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext3/xattr.h linux-2.6.13/fs/ext3/xattr.h
--- linux-2.6.13.clean/fs/ext3/xattr.h	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/fs/ext3/xattr.h	2005-06-30 13:24:23.000000000 -0400
@@ -67,6 +67,7 @@ extern struct xattr_handler ext3_xattr_s
 
 extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
 
+extern int ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir);
 extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
 extern int ext3_xattr_list(struct inode *, char *, size_t);
 extern int ext3_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/fs/ext3/xattr_security.c linux-2.6.13/fs/ext3/xattr_security.c
--- linux-2.6.13.clean/fs/ext3/xattr_security.c	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/fs/ext3/xattr_security.c	2005-06-30 13:24:23.000000000 -0400
@@ -9,6 +9,7 @@
 #include <linux/smp_lock.h>
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
+#include <linux/security.h>
 #include "xattr.h"
 
 static size_t
@@ -47,6 +48,27 @@ ext3_xattr_security_set(struct inode *in
 			      value, size, flags);
 }
 
+int
+ext3_init_security(handle_t *handle, struct inode *inode, struct inode *dir)
+{
+	int err;
+	size_t len;
+	void *value;
+	char *name;
+
+	err = security_inode_init_security(inode, dir, &name, &value, &len);
+	if (err) {
+		if (err == -EOPNOTSUPP)
+			return 0;
+		return err;
+	}
+	err = ext3_xattr_set_handle(handle, inode, EXT3_XATTR_INDEX_SECURITY,
+				    name, value, len, 0);
+	kfree(name);
+	kfree(value);
+	return err;
+}
+
 struct xattr_handler ext3_xattr_security_handler = {
 	.prefix	= XATTR_SECURITY_PREFIX,
 	.list	= ext3_xattr_security_list,
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/include/linux/security.h linux-2.6.13/include/linux/security.h
--- linux-2.6.13.clean/include/linux/security.h	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/include/linux/security.h	2005-06-30 13:24:23.000000000 -0400
@@ -250,6 +250,25 @@ struct swap_info_struct;
  *	@inode contains the inode structure.
  *	Deallocate the inode security structure and set @inode->i_security to
  *	NULL. 
+ * @inode_init_security:
+ * 	Obtain the security attribute name suffix and value to set on a newly
+ *	created inode and set up the incore security field for the new inode.
+ *	This hook is called by the fs code as part of the inode creation 
+ *	transaction and provides for atomic labeling of the inode, unlike
+ *	the post_create/mkdir/... hooks called by the VFS.  The hook function
+ *	is expected to allocate the name and value via kmalloc, with the caller
+ *	being responsible for calling kfree after using them.  
+ *	If the security module does not use security attributes or does
+ *	not wish to put a security attribute on this particular inode, 
+ *	then it should return -EOPNOTSUPP to skip this processing.
+ *	@inode contains the inode structure of the newly created inode.
+ *	@dir contains the inode structure of the parent directory.
+ *	@name will be set to the allocated name suffix (e.g. selinux).
+ *	@value will be set to the allocated attribute value.
+ *	@len will be set to the length of the value.
+ *	Returns 0 if @name and @value have been successfully set,
+ *		-EOPNOTSUPP if no security attribute is needed, or
+ *		-ENOMEM on memory allocation failure.
  * @inode_create:
  *	Check permission to create a regular file.
  *	@dir contains inode structure of the parent of the new file.
@@ -1080,6 +1099,8 @@ struct security_operations {
 
 	int (*inode_alloc_security) (struct inode *inode);	
 	void (*inode_free_security) (struct inode *inode);
+	int (*inode_init_security) (struct inode *inode, struct inode *dir,
+				    char **name, void **value, size_t *len);
 	int (*inode_create) (struct inode *dir,
 	                     struct dentry *dentry, int mode);
 	void (*inode_post_create) (struct inode *dir,
@@ -1442,6 +1463,17 @@ static inline void security_inode_free (
 		return;
 	security_ops->inode_free_security (inode);
 }
+
+static inline int security_inode_init_security (struct inode *inode, 
+						struct inode *dir,
+						char **name,
+						void **value,
+						size_t *len)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return -EOPNOTSUPP;
+	return security_ops->inode_init_security (inode, dir, name, value, len);
+}
 	
 static inline int security_inode_create (struct inode *dir,
 					 struct dentry *dentry,
@@ -2171,6 +2203,15 @@ static inline int security_inode_alloc (
 
 static inline void security_inode_free (struct inode *inode)
 { }
+
+static inline int security_inode_init_security (struct inode *inode, 
+						struct inode *dir,
+						char **name,
+						void **value,
+						size_t *len)
+{
+	return -EOPNOTSUPP;
+}
 	
 static inline int security_inode_create (struct inode *dir,
 					 struct dentry *dentry,
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/security/dummy.c linux-2.6.13/security/dummy.c
--- linux-2.6.13.clean/security/dummy.c	2005-06-30 13:24:05.000000000 -0400
+++ linux-2.6.13/security/dummy.c	2005-06-30 13:24:23.000000000 -0400
@@ -258,6 +258,12 @@ static void dummy_inode_free_security (s
 	return;
 }
 
+static int dummy_inode_init_security (struct inode *inode, struct inode *dir,
+				      char **name, void **value, size_t *len)
+{
+	return -EOPNOTSUPP;
+}
+
 static int dummy_inode_create (struct inode *inode, struct dentry *dentry,
 			       int mask)
 {
@@ -886,6 +892,7 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, sb_post_pivotroot);
 	set_to_dummy_if_null(ops, inode_alloc_security);
 	set_to_dummy_if_null(ops, inode_free_security);
+	set_to_dummy_if_null(ops, inode_init_security);
 	set_to_dummy_if_null(ops, inode_create);
 	set_to_dummy_if_null(ops, inode_post_create);
 	set_to_dummy_if_null(ops, inode_link);
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/security/selinux/hooks.c linux-2.6.13/security/selinux/hooks.c
--- linux-2.6.13.clean/security/selinux/hooks.c	2005-06-30 13:24:05.000000000 -0400
+++ linux-2.6.13/security/selinux/hooks.c	2005-06-30 13:24:23.000000000 -0400
@@ -1272,6 +1272,7 @@ static int post_create(struct inode *dir
 	struct inode *inode;
 	struct inode_security_struct *dsec;
 	struct superblock_security_struct *sbsec;
+	struct inode_security_struct *isec;
 	u32 newsid;
 	char *context;
 	unsigned int len;
@@ -1291,6 +1292,11 @@ static int post_create(struct inode *dir
 		return 0;
 	}
 
+	isec = inode->i_security;
+
+	if (isec->security_attr_init)
+		return 0;	
+
 	if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
 		newsid = tsec->create_sid;
 	} else {
@@ -2016,6 +2022,59 @@ static void selinux_inode_free_security(
 	inode_free_security(inode);
 }
 
+static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
+				       char **name, void **value, 
+				       size_t *len)
+{
+	struct task_security_struct *tsec;
+	struct inode_security_struct *dsec;
+	struct superblock_security_struct *sbsec;
+	struct inode_security_struct *isec;
+	u32 newsid;
+	int rc;
+	char *namep, *context;
+ 
+	tsec = current->security;
+	dsec = dir->i_security;
+	sbsec = dir->i_sb->s_security;
+	isec = inode->i_security;
+	
+	if (tsec->create_sid && sbsec->behavior != SECURITY_FS_USE_MNTPOINT) {
+		newsid = tsec->create_sid;
+	} else {
+		rc = security_transition_sid(tsec->sid, dsec->sid,
+					     inode_mode_to_security_class(inode->i_mode),
+					     &newsid);
+		if (rc) {
+			printk(KERN_WARNING "%s:  "
+			       "security_transition_sid failed, rc=%d (dev=%s "
+			       "ino=%ld)\n",
+			       __FUNCTION__,
+			       -rc, inode->i_sb->s_id, inode->i_ino);
+			return rc;
+		}
+	}
+
+	inode_security_set_sid(inode, newsid);
+	
+	namep = kmalloc(sizeof(XATTR_SELINUX_SUFFIX), GFP_KERNEL);
+	if (!namep)
+		return -ENOMEM;
+	strcpy(namep, XATTR_SELINUX_SUFFIX);
+	*name = namep;
+	
+	rc = security_sid_to_context(newsid, &context, len);
+	if (rc) {
+		kfree(namep);
+		return rc;
+	}
+	*value = context;
+
+	isec->security_attr_init = 1;
+
+	return 0;
+}
+
 static int selinux_inode_create(struct inode *dir, struct dentry *dentry, int mask)
 {
 	return may_create(dir, dentry, SECCLASS_FILE);
@@ -4296,6 +4355,7 @@ static struct security_operations selinu
 
 	.inode_alloc_security =		selinux_inode_alloc_security,
 	.inode_free_security =		selinux_inode_free_security,
+	.inode_init_security =		selinux_inode_init_security,
 	.inode_create =			selinux_inode_create,
 	.inode_post_create =		selinux_inode_post_create,
 	.inode_link =			selinux_inode_link,
diff -X /home/sds/dontdiff -rup linux-2.6.13.clean/security/selinux/include/objsec.h linux-2.6.13/security/selinux/include/objsec.h
--- linux-2.6.13.clean/security/selinux/include/objsec.h	2005-06-30 13:01:17.000000000 -0400
+++ linux-2.6.13/security/selinux/include/objsec.h	2005-06-30 13:24:23.000000000 -0400
@@ -46,6 +46,7 @@ struct inode_security_struct {
 	unsigned char initialized;     /* initialization flag */
 	struct semaphore sem;
 	unsigned char inherit;         /* inherit SID from parent entry */
+	unsigned char security_attr_init; /* security attributes init flag */
 };
 
 struct file_security_struct {


-- 
Stephen Smalley
National Security Agency



This archive was generated by hypermail 2.1.3 : Tue Jul 05 2005 - 09:00:29 PDT