[PATCH] SELinux - canonicalize getxattr() (fwd)

[PATCH] SELinux - canonicalize getxattr() (fwd)

[James Morris]

This patch has been added to Andrew Morton's tree and is a candidate for 
the 2.6.15 kernel.

---------- Forwarded message ----------
Date: Tue, 13 Sep 2005 16:08:08 -0400 (EDT)
From: James Morris <jmorris@namei.org>
To: Andrew Morton <akpm@osdl.org>
Cc: linux-kernel@vger.kernel.org, Stephen Smalley <sds@epoch.ncsc.mil>,
    Chris Wright <chrisw@osdl.org>
Subject: [PATCH] SELinux - canonicalize getxattr()

This patch allows SELinux to canonicalize the value returned from 
getxattr() via  the security_inode_getsecurity() hook, which is called 
after the fs level getxattr() function.

The purpose of this is to allow the in-core security context for an inode 
to override the on-disk value.  This could happen in cases such as 
upgrading a system to a different labeling form (e.g. standard SELinux to 
MLS) without needing to do a full relabel of the filesystem.

In such cases, we want getxattr() to return the canonical security context 
that the kernel is using rather than what is stored on disk.

The implementation hooks into the inode_getsecurity(), adding another 
parameter to indicate the result of the preceding fs-level getxattr() 
call, so that SELinux knows whether to compare a value obtained from disk 
with the kernel value.

We also now allow getxattr() to work for mountpoint labeled filesystems 
(i.e. mount with option context=foo_t), as we are able to return the 
kernel value to the user.

Please apply.

Signed-off-by: James Morris <jmorris@namei.org>
Signed-off-by:  Stephen Smalley <sds@tycho.nsa.gov>

---

 fs/xattr.c               |   14 +++++++++-----
 include/linux/security.h |   11 +++++++----
 security/dummy.c         |    2 +-
 security/selinux/hooks.c |   46 ++++++++++++++++++++++++++++++++--------------
 4 files changed, 49 insertions(+), 24 deletions(-)


diff -purN -X dontdiff linux-2.6.13-mm2.o/fs/xattr.c linux-2.6.13-mm2.w/fs/xattr.c
--- linux-2.6.13-mm2.o/fs/xattr.c	2005-09-12 11:28:55.000000000 -0400
+++ linux-2.6.13-mm2.w/fs/xattr.c	2005-09-12 22:31:32.000000000 -0400
@@ -143,7 +143,7 @@ getxattr(struct dentry *d, char __user *
 	if (size) {
 		if (size > XATTR_SIZE_MAX)
 			size = XATTR_SIZE_MAX;
-		kvalue = kmalloc(size, GFP_KERNEL);
+		kvalue = kzalloc(size, GFP_KERNEL);
 		if (!kvalue)
 			return -ENOMEM;
 	}
@@ -154,11 +154,15 @@ getxattr(struct dentry *d, char __user *
 	error = -EOPNOTSUPP;
 	if (d->d_inode->i_op && d->d_inode->i_op->getxattr)
 		error = d->d_inode->i_op->getxattr(d, kname, kvalue, size);
-	else if (!strncmp(kname, XATTR_SECURITY_PREFIX,
-			  sizeof XATTR_SECURITY_PREFIX - 1)) {
+		
+	if (!strncmp(kname, XATTR_SECURITY_PREFIX,
+		     sizeof XATTR_SECURITY_PREFIX - 1)) {
 		const char *suffix = kname + sizeof XATTR_SECURITY_PREFIX - 1;
-		error = security_inode_getsecurity(d->d_inode, suffix, kvalue,
-						   size);
+		int rv = security_inode_getsecurity(d->d_inode, suffix, kvalue,
+						    size, error);
+		/* Security module active: overwrite error value */
+		if (rv != -EOPNOTSUPP)
+			error = rv;
 	}
 	if (error > 0) {
 		if (size && copy_to_user(value, kvalue, error))
diff -purN -X dontdiff linux-2.6.13-mm2.o/include/linux/security.h linux-2.6.13-mm2.w/include/linux/security.h
--- linux-2.6.13-mm2.o/include/linux/security.h	2005-09-12 11:28:56.000000000 -0400
+++ linux-2.6.13-mm2.w/include/linux/security.h	2005-09-12 21:00:25.000000000 -0400
@@ -385,6 +385,9 @@ struct swap_info_struct;
  *	NULL to request the size of the buffer required.  @size indicates
  *	the size of @buffer in bytes.  Note that @name is the remainder
  *	of the attribute name after the security. prefix has been removed.
+ *	@err is the return value from the preceding fs getxattr call,
+ *	and can be used by the security module to determine whether it
+ *	should try and canonicalize the attribute value.
  *	Return number of bytes used/required on success.
  * @inode_setsecurity:
  *	Set the security label associated with @name for @inode from the
@@ -1091,7 +1094,7 @@ struct security_operations {
 	int (*inode_getxattr) (struct dentry *dentry, char *name);
 	int (*inode_listxattr) (struct dentry *dentry);
 	int (*inode_removexattr) (struct dentry *dentry, char *name);
-  	int (*inode_getsecurity)(struct inode *inode, const char *name, void *buffer, size_t size);
+  	int (*inode_getsecurity)(struct inode *inode, const char *name, void *buffer, size_t size, int err);
   	int (*inode_setsecurity)(struct inode *inode, const char *name, const void *value, size_t size, int flags);
   	int (*inode_listsecurity)(struct inode *inode, char *buffer, size_t buffer_size);
 
@@ -1580,11 +1583,11 @@ static inline int security_inode_removex
 	return security_ops->inode_removexattr (dentry, name);
 }
 
-static inline int security_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size)
+static inline int security_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size, int err)
 {
 	if (unlikely (IS_PRIVATE (inode)))
 		return 0;
-	return security_ops->inode_getsecurity(inode, name, buffer, size);
+	return security_ops->inode_getsecurity(inode, name, buffer, size, err);
 }
 
 static inline int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags)
@@ -2222,7 +2225,7 @@ static inline int security_inode_removex
 	return cap_inode_removexattr(dentry, name);
 }
 
-static inline int security_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size)
+static inline int security_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size, int err)
 {
 	return -EOPNOTSUPP;
 }
diff -purN -X dontdiff linux-2.6.13-mm2.o/security/dummy.c linux-2.6.13-mm2.w/security/dummy.c
--- linux-2.6.13-mm2.o/security/dummy.c	2005-09-12 11:28:57.000000000 -0400
+++ linux-2.6.13-mm2.w/security/dummy.c	2005-09-12 12:28:47.000000000 -0400
@@ -377,7 +377,7 @@ static int dummy_inode_removexattr (stru
 	return 0;
 }
 
-static int dummy_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size)
+static int dummy_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size, int err)
 {
 	return -EOPNOTSUPP;
 }
diff -purN -X dontdiff linux-2.6.13-mm2.o/security/selinux/hooks.c linux-2.6.13-mm2.w/security/selinux/hooks.c
--- linux-2.6.13-mm2.o/security/selinux/hooks.c	2005-09-12 11:28:57.000000000 -0400
+++ linux-2.6.13-mm2.w/security/selinux/hooks.c	2005-09-13 14:45:18.000000000 -0400
@@ -2198,9 +2198,6 @@ static int selinux_inode_getxattr (struc
 	struct inode *inode = dentry->d_inode;
 	struct superblock_security_struct *sbsec = inode->i_sb->s_security;
 
-	if (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)
-		return -EOPNOTSUPP;
-
 	return dentry_has_perm(current, NULL, dentry, FILE__GETATTR);
 }
 
@@ -2231,33 +2228,54 @@ static int selinux_inode_removexattr (st
 	return -EACCES;
 }
 
-static int selinux_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size)
+/*
+ * Copy the in-core inode security context value to the user.  If the
+ * getxattr() prior to this succeeded, check to see if we need to
+ * canonicalize the value to be finally returned to the user.
+ *
+ * Permission check is handled by selinux_inode_getxattr hook.
+ */
+static int selinux_inode_getsecurity(struct inode *inode, const char *name, void *buffer, size_t size, int err)
 {
 	struct inode_security_struct *isec = inode->i_security;
 	char *context;
 	unsigned len;
 	int rc;
 
-	/* Permission check handled by selinux_inode_getxattr hook.*/
-
-	if (strcmp(name, XATTR_SELINUX_SUFFIX))
-		return -EOPNOTSUPP;
+	if (strcmp(name, XATTR_SELINUX_SUFFIX)) {
+		rc = -EOPNOTSUPP;
+		goto out;
+	}
 
 	rc = security_sid_to_context(isec->sid, &context, &len);
 	if (rc)
-		return rc;
+		goto out;
 
+	/* Probe for required buffer size */
 	if (!buffer || !size) {
-		kfree(context);
-		return len;
+		rc = len;
+		goto out_free;
 	}
+
 	if (size < len) {
-		kfree(context);
-		return -ERANGE;
+		rc = -ERANGE;
+		goto out_free;
+	}
+	
+	if (err > 0) {
+		if ((len == err) && !(memcmp(context, buffer, len))) {
+			/* Don't need to canonicalize value */
+			rc = err;
+			goto out_free;
+		}
+		memset(buffer, 0, size);
 	}
 	memcpy(buffer, context, len);
+	rc = len;
+out_free:	
 	kfree(context);
-	return len;
+out:
+	return rc;
 }
 
 static int selinux_inode_setsecurity(struct inode *inode, const char *name,

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

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

On Wed, 2005-09-14 at 10:15 -0400, James Morris wrote:
> This patch allows SELinux to canonicalize the value returned from 
> getxattr() via  the security_inode_getsecurity() hook, which is called 
> after the fs level getxattr() function.
> 
> The purpose of this is to allow the in-core security context for an inode 
> to override the on-disk value.  This could happen in cases such as 
> upgrading a system to a different labeling form (e.g. standard SELinux to 
> MLS) without needing to do a full relabel of the filesystem.
> 
> In such cases, we want getxattr() to return the canonical security context 
> that the kernel is using rather than what is stored on disk.

Hi,

An issue has arisen with regard to the canonicalization of getxattr
results and the use of type aliases in targeted policy; some private
discussion has occurred, but I'm taking discussion to the list as a
follow-up to the message where the canonicalization patch was posted to
the list.  

The bugzilla report is at
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=171846

Targeted policy presently defines a number of type aliases to provide
compatibility and allow sharing with strict policy, both in the policy
itself (macros, .te and .fc files) and in the on-disk xattrs.  Since
setxattr does not canonicalize the attributes, the alias is stored on
disk, so for example, .so files are labeled as shlib_t even though
targeted policy aliases this to lib_t.  This is advantageous for policy
sharing and for conversion between policies.  It also seems important
for allowing shared policy modules; by using the more specific names in
the module, the module should be able to be used on either targeted or
strict.

Prior to the patch to canonicalize getxattr results, getxattr would
return the on-disk xattr value, so it would also show the alias name.
With the canonicalization patch, it instead shows the primary name.
This creates confusion for programs like setfiles and restorecon that
compare the context obtained via matchpathcon with the context obtained
via getfilecon to determine whether the file needs to be relabeled,
yielding repeated messages about mislabeled files despite the files
actually being labeled correctly.  restorecon -Rv /lib on a rawhide
targeted system illustrates the point.

The canonicalization of getxattr results is desirable, but we do need to
address the problem above.  I've created experimental patches for
libsepol and libselinux to allow matchpathcon_init to canonicalize
contexts from file contexts against a policy (attached), although I
think it would be better to do this via a selinuxfs interface rather
than via sepol, as the latter requires reading of a binary policy.

However, regardless of how it is implemented, this change to
matchpathcon_init has another side effect:  rpm will then label files
with the primary names rather than the alias names, so subsequent
conversion of the system to a different policy where they were distinct
types will be more difficult (essentially requires permissive mode boot
and relabel, as .so files may not be executable under the new policy).
Not sure whether that is a real concern.

This change also doesn't address the issue in general, only for users of
matchpathcon.  If we implement it via selinuxfs, it would be feasible to
apply the canonicalization on all context translations, although even
that might be expensive.  With the current policy-based
canonicalization, it is definitely too expensive to apply on all context
translations unless we cache the policy in memory in some manner (e.g.
translation daemon).  But even that won't address the potential
confusion noted by Dan of a user performing a chcon -t shlib_t foo and
then seeing lib_t on the file upon a ls -Z.  I'm not sure whether that
is a major issue in practice.

Thoughts?

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: libsepol-canonicalize.patch --]
[-- Type: text/x-patch, Size: 2613 bytes --]

Index: libsepol/include/sepol/policydb.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/include/sepol/policydb.h,v
retrieving revision 1.20
diff -u -p -r1.20 policydb.h
--- libsepol/include/sepol/policydb.h	18 Oct 2005 13:28:24 -0000	1.20
+++ libsepol/include/sepol/policydb.h	27 Oct 2005 15:22:18 -0000
@@ -124,6 +124,16 @@ extern int sepol_policydb_to_image(sepol
 				   void **newdata, 
 				   size_t *newlen);
 
+/*
+ * Canonicalize a security context for a given policy.
+ * Sets '*newcon' to the canonicalized context.
+ * Caller must free *newcon via free().
+ */ 
+extern int sepol_policydb_canonicalize_context(sepol_handle_t *handle,
+					       sepol_policydb_t *p,
+					       const char *oldcon,
+					       char **newcon);
+
 #endif
 
 
Index: libsepol/src/libsepol.map
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/src/libsepol.map,v
retrieving revision 1.11
diff -u -p -r1.11 libsepol.map
--- libsepol/src/libsepol.map	21 Oct 2005 15:13:56 -0000	1.11
+++ libsepol/src/libsepol.map	27 Oct 2005 16:54:58 -0000
@@ -9,6 +9,7 @@
 	sepol_policydb_set_typesvers; sepol_policydb_set_vers;
 	sepol_policydb_read; sepol_policydb_write;
 	sepol_policydb_from_image; sepol_policydb_to_image;
+	sepol_policydb_canonicalize_context;
 	sepol_module_package_create; sepol_module_package_free; 
 	sepol_module_package_get_file_contexts;
 	sepol_module_package_get_file_contexts_len;
Index: libsepol/src/policydb_public.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libsepol/src/policydb_public.c,v
retrieving revision 1.3
diff -u -p -r1.3 policydb_public.c
--- libsepol/src/policydb_public.c	25 Oct 2005 12:09:09 -0000	1.3
+++ libsepol/src/policydb_public.c	27 Oct 2005 15:34:59 -0000
@@ -3,6 +3,7 @@
 #include "debug.h"
 
 #include <sepol/policydb/policydb.h>
+#include "context.h"
 
 /* Policy file interfaces. */
 
@@ -157,3 +158,23 @@ int sepol_policydb_to_image(sepol_handle
 	return policydb_to_image(handle, &p->p, newdata, newlen);
 }
 
+int sepol_policydb_canonicalize_context(sepol_handle_t *handle,
+					sepol_policydb_t *p,
+					const char *oldcon,
+					char **newcon)
+{
+	context_struct_t *con;
+	size_t len;
+	int rc = STATUS_ERR;
+
+	if (context_from_string(handle, &p->p, &con, 
+				oldcon, strlen(oldcon)+1) < 0)
+		return rc;
+	if (context_to_string(handle, &p->p, con, newcon, &len) < 0)
+		goto out;
+	rc = STATUS_SUCCESS;
+out:
+	context_destroy(con);
+	free(con);
+	return rc;
+}

[-- Attachment #3: libselinux-canonicalize.patch --]
[-- Type: text/x-patch, Size: 6513 bytes --]

Index: libselinux/src/load_policy.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/load_policy.c,v
retrieving revision 1.12
diff -u -p -r1.12 load_policy.c
--- libselinux/src/load_policy.c	18 Oct 2005 13:49:00 -0000	1.12
+++ libselinux/src/load_policy.c	27 Oct 2005 17:31:24 -0000
@@ -35,19 +35,12 @@ hidden_def(security_load_policy)
 
 int load_setlocaldefs hidden = 1;
 
-int selinux_mkload_policy(int preservebools)
+static int selinux_open_policy(int *version) 
 {
-	int vers = sepol_policy_kern_vers_max();
-	int kernvers = security_policyvers();
-	char path[PATH_MAX], **names;
-	struct stat sb;
-	size_t size;
-	void *map, *data;
-	int fd, rc = -1, *values, len, i, prot;
-	sepol_policydb_t *policydb;
-	sepol_policy_file_t *pf;
+	int vers = *version ?: sepol_policy_kern_vers_max();
+	char path[PATH_MAX];
+	int fd;
 
-search:
 	snprintf(path, sizeof(path), "%s.%d", 
 		 selinux_binary_policy_path(), vers);
 	fd = open(path, O_RDONLY);
@@ -60,6 +53,27 @@ search:
 	if (fd < 0)
 		return -1;
 
+	*version = vers;
+	return fd;
+}
+
+int selinux_mkload_policy(int preservebools)
+{
+	int vers = 0;
+	int kernvers = security_policyvers();
+	char **names;
+	struct stat sb;
+	size_t size;
+	void *map, *data;
+	int fd, rc = -1, *values, len, i, prot;
+	sepol_policydb_t *policydb;
+	sepol_policy_file_t *pf;
+
+search:
+	fd = selinux_open_policy(&vers);
+	if (fd < 0)
+		return -1;
+
 	if (fstat(fd, &sb) < 0)
 		goto close;
 
@@ -259,3 +273,40 @@ noload:
 	 */
 	return -1;
 }
+
+int hidden selinux_open_policydb(sepol_policydb_t **out_policydb)
+{
+	int vers = 0, fd, rc = -1;
+	FILE *fp;
+	sepol_policydb_t *policydb = NULL;
+	sepol_policy_file_t *pf = NULL;
+
+	fd = selinux_open_policy(&vers);
+	if (fd < 0)
+		return -1;
+
+	fp = fdopen(fd, "r");
+	if (!fp) {
+		close(fd);
+		return -1;
+	}
+
+	if (sepol_policy_file_create(&pf))
+		goto err;
+	if (sepol_policydb_create(&policydb))
+		goto err;
+	sepol_policy_file_set_fp(pf, fp);
+	if (sepol_policydb_read(policydb, pf))
+		goto err;
+	*out_policydb = policydb;
+	rc = 0;
+out:
+	sepol_policy_file_free(pf);
+	fclose(fp);
+	return rc;
+err:
+	rc = -1;
+	sepol_policydb_free(policydb);
+	goto out;
+}
+
Index: libselinux/src/matchpathcon.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/matchpathcon.c,v
retrieving revision 1.31
diff -u -p -r1.31 matchpathcon.c
--- libselinux/src/matchpathcon.c	5 Oct 2005 15:48:05 -0000	1.31
+++ libselinux/src/matchpathcon.c	27 Oct 2005 17:38:25 -0000
@@ -61,6 +61,32 @@ out:
 	return rc;
 }
 
+static int selinux_canonicalize_context(sepol_policydb_t *policydb,
+					security_context_t oldcon,
+					security_context_t *newcon)
+{
+	int ret;
+	security_context_t roldcon = oldcon, rnewcon = NULL;
+
+	if (context_translations && trans_to_raw_context(oldcon, &roldcon))
+		return -1;
+
+ 	ret = sepol_policydb_canonicalize_context(NULL, 
+						  policydb, roldcon, &rnewcon);
+
+	if (context_translations) {
+		freecon(roldcon);
+		if (ret >= 0 && raw_to_trans_context(rnewcon, newcon)) {
+			*newcon = NULL;
+			ret = -1;
+		}
+		freecon(rnewcon);
+	} else if (ret >= 0)
+		*newcon = rnewcon;
+
+	return ret;
+}
+
 static void (*myprintf)(const char *fmt, ...) = &default_printf;
 
 void set_matchpathcon_printf(void (*f)(const char *fmt, ...))
@@ -214,6 +240,8 @@ static int find_stem_from_file(const cha
 	return -1;
 }
 
+static sepol_policydb_t *policydb;
+
 /*
  * The array of specifications, initially in the
  * same order as in the specification file.
@@ -451,6 +479,7 @@ static void spec_hasMetaChars(struct spe
 	}
 	return;
 }
+
 static int process_line( const char *path, char *line_buf, int pass, unsigned lineno, int mls_enabled) {
 	int items, len, regerr;
 	char *buf_p;
@@ -557,21 +586,34 @@ static int process_line( const char *pat
 		
 	skip_type:
 		if (strcmp(context, "<<none>>")) {
+			/* Translate the context for MLS or strip the level. */
 			if (context_translations) {
-				if (raw_to_trans_context(context,
-				                    &spec_arr[nspec].context)) {
+				char *trans = NULL;
+				if (raw_to_trans_context(context, &trans)) {
 					myprintf("%s: line %u has invalid "
 					         "context %s\n",
 					         path, lineno, context);
 					return 0;
 				}
 				free(context);
-				context = spec_arr[nspec].context;
+				context = trans;
 			} else {
 				if (STRIP_LEVEL(&context, mls_enabled))
 					return -1;
 			}
 
+			/* Canonicalize the context. */
+			if (policydb) {
+				char *canon = NULL;
+				if (!selinux_canonicalize_context(policydb,
+								  context, 
+								  &canon)) {
+					free(context);
+					context = canon;
+				}
+			}
+
+			/* Validate the context. */
 			if (myinvalidcon(path, lineno, context))
 				return 0;
 		}
@@ -626,6 +668,12 @@ int matchpathcon_init(const char *path)
 			__fsetlocking(localfp, FSETLOCKING_BYCALLER);
 	}
 
+	/* Canonicalize contexts against policy when acting on the
+	   active file contexts configuration. */
+	if (!strcmp(path, selinux_file_context_path())) {
+		(void) selinux_open_policydb(&policydb);
+	}
+
 	/* 
 	 * Perform two passes over the specification file.
 	 * The first pass counts the number of specifications and
Index: libselinux/src/selinux_internal.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/selinux_internal.h,v
retrieving revision 1.15
diff -u -p -r1.15 selinux_internal.h
--- libselinux/src/selinux_internal.h	27 Oct 2005 12:16:37 -0000	1.15
+++ libselinux/src/selinux_internal.h	27 Oct 2005 17:32:02 -0000
@@ -1,4 +1,5 @@
 #include <selinux/selinux.h>
+#include <sepol/policydb.h>
 #include "dso.h"
 
 hidden_proto(selinux_mkload_policy)
@@ -59,9 +60,9 @@ hidden_proto(selinux_path)
 hidden_proto(selinux_check_passwd_access)
 hidden_proto(matchpathcon_init)
 hidden_proto(selinux_users_path)
-hidden_proto(selinux_usersconf_path);
-hidden_proto(selinux_translations_path);
-hidden_proto(selinux_getenforcemode);
+hidden_proto(selinux_usersconf_path)
+hidden_proto(selinux_translations_path)
+hidden_proto(selinux_getenforcemode)
 
 extern int context_translations hidden;
 extern int hidden trans_to_raw_context(char *trans, char **rawp);
@@ -69,3 +70,5 @@ extern int hidden raw_to_trans_context(c
 
 extern int load_setlocaldefs hidden;
 extern int require_seusers hidden;
+
+extern int hidden selinux_open_policydb(sepol_policydb_t **policydb);

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[James Morris]

On Thu, 27 Oct 2005, Stephen Smalley wrote:

> Thoughts?

It may be helpful to have type aliases, but are they truly necessary?  

Seems like a lot of potential problems arise from them, including 
confusing SELinux developers.

Perhaps type aliasing would be better implemented as a higher level policy 
construct?


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

On Thu, 2005-10-27 at 15:23 -0400, James Morris wrote:
> On Thu, 27 Oct 2005, Stephen Smalley wrote:
> 
> > Thoughts?
> 
> It may be helpful to have type aliases, but are they truly necessary?  
> 
> Seems like a lot of potential problems arise from them, including 
> confusing SELinux developers.
> 
> Perhaps type aliasing would be better implemented as a higher level policy 
> construct?

There seem to be three uses of the aliases:
1) compatibility across policy changes, e.g. when a type is removed or
renamed, an alias can be defined so that any existing processes or
objects with the type aren't rendered unlabeled upon the policy reload,
2) on-disk compatibility between policies, so that a filesystem
initially labeled for targeted policy is still fairly useable for
bootstrapping a strict policy system even though the latter has
finer-grained types,
3) sharing among policies, both at a source level (macros, .te
files, .fc files) and for binary policy modules, so that policy modules
(source or binary) can refer to types that may then be coalesced or kept
separate as appropriate for a given base policy.

#3 could be done entirely via a higher level construct, but we don't
have such a higher level construct yet and are relying on the ability to
share among policies in this manner.  #2 may not be critical.  #1 seems
to require that the kernel retain a notion of the aliases.

If #2 is not critical, then changing matchpathcon_init in the proposed
manner may be sufficient (whether using sepol or selinuxfs as the
backend).  With that change, upgraded systems that have existing uses of
alias names in on-disk xattrs won't cause spurious complaints by
setfiles/restorecon, and a clean install should yield a system with no
alias names stored in the on-disk xattrs since rpm will get canonical
names.  We could instrument the setfilecon functions in libselinux to
also canonicalize the context prior to calling setxattr.  On second
thought, it seems overkill to do it on context translations, as this is
only an issue for file contexts and is already covered for getxattr, so
we are primarily concerned with matchpathcon and setfilecon.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Chad Sellers]

On Thursday 27 October 2005 16:02, Stephen Smalley wrote:
> On Thu, 2005-10-27 at 15:23 -0400, James Morris wrote:
> > On Thu, 27 Oct 2005, Stephen Smalley wrote:
> > > Thoughts?
> >
> > It may be helpful to have type aliases, but are they truly necessary?
> >
> > Seems like a lot of potential problems arise from them, including
> > confusing SELinux developers.
> >
> > Perhaps type aliasing would be better implemented as a higher level
> > policy construct?
>
> There seem to be three uses of the aliases:
> 1) compatibility across policy changes, e.g. when a type is removed or
> renamed, an alias can be defined so that any existing processes or
> objects with the type aren't rendered unlabeled upon the policy reload,
> 2) on-disk compatibility between policies, so that a filesystem
> initially labeled for targeted policy is still fairly useable for
> bootstrapping a strict policy system even though the latter has
> finer-grained types,
> 3) sharing among policies, both at a source level (macros, .te
> files, .fc files) and for binary policy modules, so that policy modules
> (source or binary) can refer to types that may then be coalesced or kept
> separate as appropriate for a given base policy.
>
> #3 could be done entirely via a higher level construct, but we don't
> have such a higher level construct yet and are relying on the ability to
> share among policies in this manner.  #2 may not be critical.  #1 seems
> to require that the kernel retain a notion of the aliases.
>
> If #2 is not critical, then changing matchpathcon_init in the proposed
> manner may be sufficient (whether using sepol or selinuxfs as the
> backend).  With that change, upgraded systems that have existing uses of
> alias names in on-disk xattrs won't cause spurious complaints by
> setfiles/restorecon, and a clean install should yield a system with no
> alias names stored in the on-disk xattrs since rpm will get canonical
> names.  We could instrument the setfilecon functions in libselinux to
> also canonicalize the context prior to calling setxattr.  On second
> thought, it seems overkill to do it on context translations, as this is
> only an issue for file contexts and is already covered for getxattr, so
> we are primarily concerned with matchpathcon and setfilecon.

Did this ever get resolved?  I see that this patch has made it into the 
rawhide kernel.  This means that when installing a new policy rpm, my screen 
is flooded with fixfiles relabeling lib_t to shlib_t, due to shlib_t being a 
typealias of lib_t in targeted.  Are we worried about this right now, or are 
these messages ok?

-- 
----------------------
Chad Sellers
Tresys Technology, LLC
csellers@tresys.com
http://www.tresys.com

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Daniel J Walsh]

Chad Sellers wrote:
> On Thursday 27 October 2005 16:02, Stephen Smalley wrote:
>   
>> On Thu, 2005-10-27 at 15:23 -0400, James Morris wrote:
>>     
>>> On Thu, 27 Oct 2005, Stephen Smalley wrote:
>>>       
>>>> Thoughts?
>>>>         
>>> It may be helpful to have type aliases, but are they truly necessary?
>>>
>>> Seems like a lot of potential problems arise from them, including
>>> confusing SELinux developers.
>>>
>>> Perhaps type aliasing would be better implemented as a higher level
>>> policy construct?
>>>       
>> There seem to be three uses of the aliases:
>> 1) compatibility across policy changes, e.g. when a type is removed or
>> renamed, an alias can be defined so that any existing processes or
>> objects with the type aren't rendered unlabeled upon the policy reload,
>> 2) on-disk compatibility between policies, so that a filesystem
>> initially labeled for targeted policy is still fairly useable for
>> bootstrapping a strict policy system even though the latter has
>> finer-grained types,
>> 3) sharing among policies, both at a source level (macros, .te
>> files, .fc files) and for binary policy modules, so that policy modules
>> (source or binary) can refer to types that may then be coalesced or kept
>> separate as appropriate for a given base policy.
>>
>> #3 could be done entirely via a higher level construct, but we don't
>> have such a higher level construct yet and are relying on the ability to
>> share among policies in this manner.  #2 may not be critical.  #1 seems
>> to require that the kernel retain a notion of the aliases.
>>
>> If #2 is not critical, then changing matchpathcon_init in the proposed
>> manner may be sufficient (whether using sepol or selinuxfs as the
>> backend).  With that change, upgraded systems that have existing uses of
>> alias names in on-disk xattrs won't cause spurious complaints by
>> setfiles/restorecon, and a clean install should yield a system with no
>> alias names stored in the on-disk xattrs since rpm will get canonical
>> names.  We could instrument the setfilecon functions in libselinux to
>> also canonicalize the context prior to calling setxattr.  On second
>> thought, it seems overkill to do it on context translations, as this is
>> only an issue for file contexts and is already covered for getxattr, so
>> we are primarily concerned with matchpathcon and setfilecon.
>>     
>
> Did this ever get resolved?  I see that this patch has made it into the 
> rawhide kernel.  This means that when installing a new policy rpm, my screen 
> is flooded with fixfiles relabeling lib_t to shlib_t, due to shlib_t being a 
> typealias of lib_t in targeted.  Are we worried about this right now, or are 
> these messages ok?
>
>   
We are supposed to be patching restorecon/chcon/setfiles to make these 
go away or at most warn.  These tools should be asking the kernel for 
the correct context for a file  IE shlib_t -> lib_t in targeted policy 
and then says it is ok.


-- 



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

On Wed, 2005-11-09 at 16:30 -0500, Chad Sellers wrote:
> Did this ever get resolved?  I see that this patch has made it into the 
> rawhide kernel.  This means that when installing a new policy rpm, my screen 
> is flooded with fixfiles relabeling lib_t to shlib_t, due to shlib_t being a 
> typealias of lib_t in targeted.  Are we worried about this right now, or are 
> these messages ok?

A kernel patch was upstreamed to allow userspace to get the canonical
context, and matchpathcon_init and setfiles were changed to use that
support when present in the kernel in order to canonicalize the context,
so that in the targeted policy case, both contexts will resolve to lib_t
and they will compare as equal.  The messages from fixfiles are what you
would see _before_ the kernel patch is included.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

On Wed, 2005-11-09 at 17:46 -0500, Daniel J Walsh wrote:
> We are supposed to be patching restorecon/chcon/setfiles to make these 
> go away or at most warn.  These tools should be asking the kernel for 
> the correct context for a file  IE shlib_t -> lib_t in targeted policy 
> and then says it is ok.

I already took care of restorecon (via the modified matchpathcon logic)
and setfiles (which has its own callback).  But they need a kernel that
has the new support for canonicalizing the contexts, and that only just
showed up in 2.6.14-git13.  Not in rawhide yet AFAIK.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

On Thu, 2005-11-10 at 07:31 -0500, Stephen Smalley wrote:
> On Wed, 2005-11-09 at 17:46 -0500, Daniel J Walsh wrote:
> > We are supposed to be patching restorecon/chcon/setfiles to make these 
> > go away or at most warn.  These tools should be asking the kernel for 
> > the correct context for a file  IE shlib_t -> lib_t in targeted policy 
> > and then says it is ok.
> 
> I already took care of restorecon (via the modified matchpathcon logic)
> and setfiles (which has its own callback).  But they need a kernel that
> has the new support for canonicalizing the contexts, and that only just
> showed up in 2.6.14-git13.  Not in rawhide yet AFAIK.

BTW, in addition to altering matchpatchcon for this purpose, I added
security_canonicalize_context interfaces to libselinux to allow other
applications to also obtain canonical contexts.  So chcon could call
that interface to get the canonical context and compare the result with
the user-supplied context.  The implementation provides a fallback for
kernels that do not yet support the new functionality, simply returning
the provided context in that case, so that nothing breaks (you just
don't get the benefit of the canonicalization until the kernel includes
the patch).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

On Wed, 2005-11-09 at 16:30 -0500, Chad Sellers wrote:
> Did this ever get resolved?  I see that this patch has made it into the 
> rawhide kernel.  This means that when installing a new policy rpm, my screen 
> is flooded with fixfiles relabeling lib_t to shlib_t, due to shlib_t being a 
> typealias of lib_t in targeted.  Are we worried about this right now, or are 
> these messages ok?

So the short answer is to not worry about these messages, as they do not
reflect actual changes in labels (and are an issue already with existing
targeted policy updates, not just refpolicy), and they will cease when
the rawhide kernel picks up the latest -git snapshot.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Chad Sellers]

On Thursday 10 November 2005 10:13, Stephen Smalley wrote:
> On Wed, 2005-11-09 at 16:30 -0500, Chad Sellers wrote:
> > Did this ever get resolved?  I see that this patch has made it into the
> > rawhide kernel.  This means that when installing a new policy rpm, my
> > screen is flooded with fixfiles relabeling lib_t to shlib_t, due to
> > shlib_t being a typealias of lib_t in targeted.  Are we worried about
> > this right now, or are these messages ok?
>
> So the short answer is to not worry about these messages, as they do not
> reflect actual changes in labels (and are an issue already with existing
> targeted policy updates, not just refpolicy), and they will cease when
> the rawhide kernel picks up the latest -git snapshot.

OK.  Anyone have an idea when this will hit rawhide?  Before test1?

-- 
----------------------
Chad Sellers
Tresys Technology, LLC
csellers@tresys.com
(410)290-1411 x117
http://www.tresys.com

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

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

On Thu, 2005-11-10 at 10:22 -0500, Chad Sellers wrote:
> On Thursday 10 November 2005 10:13, Stephen Smalley wrote:
> > On Wed, 2005-11-09 at 16:30 -0500, Chad Sellers wrote:
> > > Did this ever get resolved?  I see that this patch has made it into the
> > > rawhide kernel.  This means that when installing a new policy rpm, my
> > > screen is flooded with fixfiles relabeling lib_t to shlib_t, due to
> > > shlib_t being a typealias of lib_t in targeted.  Are we worried about
> > > this right now, or are these messages ok?
> >
> > So the short answer is to not worry about these messages, as they do not
> > reflect actual changes in labels (and are an issue already with existing
> > targeted policy updates, not just refpolicy), and they will cease when
> > the rawhide kernel picks up the latest -git snapshot.
> 
> OK.  Anyone have an idea when this will hit rawhide?  Before test1?

I don't know; the kernel patch just showed up in -git13, so it should be
included in the next rawhide kernel update, but I don't know when that
will happen.

In any event, you should able to redirect output from the rpm update to
a log file and just filter out lib_t/shlib_t noise to see any other
changes of interest.

I'm doing the following:
- updating to the latest rawhide targeted policy first (or rolling back
to it from prior attempted install of the refpolicy),
- running fixfiles restorecon to ensure that files are labeled
consistently with that policy (should already be handled upon the
update, but just wanted to be sure),
- running rpm -Uvh on the new policy rpm, redirecting stdout and stderr
to a log file.

The restorecon is still running, but so far I have the following output
excluding the lib_t->shlib_t noise:
$ grep -v 'lib_t.*shlib_t' log


-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: log2.bz2 --]
[-- Type: application/x-bzip, Size: 1698 bytes --]

Re: [PATCH] SELinux - canonicalize getxattr() (fwd)

[Stephen Smalley]

On Thu, 2005-11-10 at 10:33 -0500, Stephen Smalley wrote:
> I'm doing the following:
> - updating to the latest rawhide targeted policy first (or rolling back
> to it from prior attempted install of the refpolicy),

And also updating to latest rawhide libse* and policycoreutils packages.

> - running fixfiles restorecon to ensure that files are labeled
> consistently with that policy (should already be handled upon the
> update, but just wanted to be sure),
> - running rpm -Uvh on the new policy rpm, redirecting stdout and stderr
> to a log file.
> 
> The restorecon is still running, but so far I have the following output
> excluding the lib_t->shlib_t noise:
> $ grep -v 'lib_t.*shlib_t' log

Ok, rpm finished running, and there was no further output from
restorecon.  But there is also no installed policy on the system
(outside of the sandbox).  Hmmm...did rpm remove the installed files
because they are no longer owned by the policy package itself, whereas
they used to be?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.