All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
To: Stephen Smalley <sds@tycho.nsa.gov>,
	James Morris <jmorris@namei.org>,
	Eric Paris <eparis@parisplace.org>,
	Nick Piggin <npiggin@kernel.dk>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Cc: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
Subject: [PATCH] security/selinux: fix /proc/sys/ labeling
Date: Tue,  1 Feb 2011 02:17:54 +0200	[thread overview]
Message-ID: <1296519474-15714-1-git-send-email-lucian.grijincu@gmail.com> (raw)

From: Eric W. Biederman <ebiederm@xmission.com>

This fixes an old (2007) selinux regression: filesystem labeling for
/proc/sys returned
     -r--r--r-- unknown                          /proc/sys/fs/file-nr
instead of
     -r--r--r-- system_u:object_r:sysctl_fs_t:s0 /proc/sys/fs/file-nr

Events that lead to breaking of /proc/sys selinux labeling:

1) sysctl was reimplemented to route all calls through /proc/sysctl

    commit 77b14db502cb85a031fe8fde6c85d52f3e0acb63
    [PATCH] sysctl: reimplement the sysctl proc support

2) proc_dir_entry was removed from ctl_table:

    commit 3fbfa98112fc3962c416452a0baf2214381030e6
    [PATCH] sysctl: remove the proc_dir_entry member for the sysctl tables

3) selinux still walked the proc_dir_entry tree to apply
   labeling. Because ctl_tables don't have a proc_dir_entry, we did
   not label /proc/sys/ inodes any more. To achieve this the /proc/sys
   inodes were marked private and private inodes were ignored by
   selinux.

    commit bbaca6c2e7ef0f663bc31be4dad7cf530f6c4962
    [PATCH] selinux: enhance selinux to always ignore private inodes

    commit 86a71dbd3e81e8870d0f0e56b87875f57e58222b
    [PATCH] sysctl: hide the sysctl proc inodes from selinux

Access control checks have been done by means of a special sysctl hook
that was called for read/write accesses to any /proc/sys/ entry.

We don't have to do this because, instead of walking the
proc_dir_entry tree we can walk the dentry tree (as done in this
patch). With this patch:
* we don't mark /proc/sys inodes as private
* we don't need the sysclt security hook
* we walk the dentry tree to find the path to the inode.

We have to strip the PID in /proc/PID/ entries that have a
proc_dir_entry because selinux does not know how to label paths like
'/1/net/rpc/nfsd.fh' (and defaults to 'proc_t' labeling). Selinux does
know of '/net/rpc/nfsd.fh' (and applies the 'sysctl_rpc_t' label).

PID stripping from the path was done implicitly in the previous code
because the proc_dir_entry tree had the root in '/net' in the example
from above. The dentry tree has the root in '/1'.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Lucian Adrian Grijincu <lucian.grijincu@gmail.com>
---
 fs/proc/proc_sysctl.c    |    1 -
 security/selinux/hooks.c |  119 +++++++---------------------------------------
 2 files changed, 18 insertions(+), 102 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 09a1f92..fb707e0 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -32,7 +32,6 @@ static struct inode *proc_sys_make_inode(struct super_block *sb,
 	ei->sysctl_entry = table;
 
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_flags |= S_PRIVATE; /* tell selinux to ignore this inode */
 	inode->i_mode = table->mode;
 	if (!table->child) {
 		inode->i_mode |= S_IFREG;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e276eb4..7c5dfb1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -43,7 +43,6 @@
 #include <linux/fdtable.h>
 #include <linux/namei.h>
 #include <linux/mount.h>
-#include <linux/proc_fs.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv6.h>
 #include <linux/tty.h>
@@ -70,7 +69,6 @@
 #include <net/ipv6.h>
 #include <linux/hugetlb.h>
 #include <linux/personality.h>
-#include <linux/sysctl.h>
 #include <linux/audit.h>
 #include <linux/string.h>
 #include <linux/selinux.h>
@@ -1120,39 +1118,35 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
 }
 
 #ifdef CONFIG_PROC_FS
-static int selinux_proc_get_sid(struct proc_dir_entry *de,
+static int selinux_proc_get_sid(struct dentry *dentry,
 				u16 tclass,
 				u32 *sid)
 {
-	int buflen, rc;
-	char *buffer, *path, *end;
+	int rc;
+	char *buffer, *path;
 
 	buffer = (char *)__get_free_page(GFP_KERNEL);
 	if (!buffer)
 		return -ENOMEM;
 
-	buflen = PAGE_SIZE;
-	end = buffer+buflen;
-	*--end = '\0';
-	buflen--;
-	path = end-1;
-	*path = '/';
-	while (de && de != de->parent) {
-		buflen -= de->namelen + 1;
-		if (buflen < 0)
-			break;
-		end -= de->namelen;
-		memcpy(end, de->name, de->namelen);
-		*--end = '/';
-		path = end;
-		de = de->parent;
+	path = dentry_path_raw(dentry, buffer, PAGE_SIZE);
+	if (IS_ERR(path))
+		rc = PTR_ERR(path);
+	else {
+		/* each process gets a /proc/PID/ entry. Strip off the
+		 * PID part to get a valid selinux labeling.
+		 * e.g. /proc/1/net/rpc/nfs -> /net/rpc/nfs */
+		while (path[1] >= '0' && path[1] <= '9') {
+			path[1] = '/';
+			path++;
+		}
+		rc = security_genfs_sid("proc", path, tclass, sid);
 	}
-	rc = security_genfs_sid("proc", path, tclass, sid);
 	free_page((unsigned long)buffer);
 	return rc;
 }
 #else
-static int selinux_proc_get_sid(struct proc_dir_entry *de,
+static int selinux_proc_get_sid(struct dentry *dentry,
 				u16 tclass,
 				u32 *sid)
 {
@@ -1317,9 +1311,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 
 		if ((sbsec->flags & SE_SBPROC) && !S_ISLNK(inode->i_mode)) {
 			struct proc_inode *proci = PROC_I(inode);
-			if (proci->pde) {
+			if (opt_dentry && (proci->pde || proci->sysctl)) {
 				isec->sclass = inode_mode_to_security_class(inode->i_mode);
-				rc = selinux_proc_get_sid(proci->pde,
+				rc = selinux_proc_get_sid(opt_dentry,
 							  isec->sclass,
 							  &sid);
 				if (rc)
@@ -1862,82 +1856,6 @@ static int selinux_capable(struct task_struct *tsk, const struct cred *cred,
 	return task_has_capability(tsk, cred, cap, audit);
 }
 
-static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
-{
-	int buflen, rc;
-	char *buffer, *path, *end;
-
-	rc = -ENOMEM;
-	buffer = (char *)__get_free_page(GFP_KERNEL);
-	if (!buffer)
-		goto out;
-
-	buflen = PAGE_SIZE;
-	end = buffer+buflen;
-	*--end = '\0';
-	buflen--;
-	path = end-1;
-	*path = '/';
-	while (table) {
-		const char *name = table->procname;
-		size_t namelen = strlen(name);
-		buflen -= namelen + 1;
-		if (buflen < 0)
-			goto out_free;
-		end -= namelen;
-		memcpy(end, name, namelen);
-		*--end = '/';
-		path = end;
-		table = table->parent;
-	}
-	buflen -= 4;
-	if (buflen < 0)
-		goto out_free;
-	end -= 4;
-	memcpy(end, "/sys", 4);
-	path = end;
-	rc = security_genfs_sid("proc", path, tclass, sid);
-out_free:
-	free_page((unsigned long)buffer);
-out:
-	return rc;
-}
-
-static int selinux_sysctl(ctl_table *table, int op)
-{
-	int error = 0;
-	u32 av;
-	u32 tsid, sid;
-	int rc;
-
-	sid = current_sid();
-
-	rc = selinux_sysctl_get_sid(table, (op == 0001) ?
-				    SECCLASS_DIR : SECCLASS_FILE, &tsid);
-	if (rc) {
-		/* Default to the well-defined sysctl SID. */
-		tsid = SECINITSID_SYSCTL;
-	}
-
-	/* The op values are "defined" in sysctl.c, thereby creating
-	 * a bad coupling between this module and sysctl.c */
-	if (op == 001) {
-		error = avc_has_perm(sid, tsid,
-				     SECCLASS_DIR, DIR__SEARCH, NULL);
-	} else {
-		av = 0;
-		if (op & 004)
-			av |= FILE__READ;
-		if (op & 002)
-			av |= FILE__WRITE;
-		if (av)
-			error = avc_has_perm(sid, tsid,
-					     SECCLASS_FILE, av, NULL);
-	}
-
-	return error;
-}
-
 static int selinux_quotactl(int cmds, int type, int id, struct super_block *sb)
 {
 	const struct cred *cred = current_cred();
@@ -5398,7 +5316,6 @@ static struct security_operations selinux_ops = {
 	.ptrace_traceme =		selinux_ptrace_traceme,
 	.capget =			selinux_capget,
 	.capset =			selinux_capset,
-	.sysctl =			selinux_sysctl,
 	.capable =			selinux_capable,
 	.quotactl =			selinux_quotactl,
 	.quota_on =			selinux_quota_on,
-- 
1.7.4.rc1.7.g2cf08.dirty


             reply	other threads:[~2011-02-01  0:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-01  0:17 Lucian Adrian Grijincu [this message]
2011-02-01  1:32 ` [PATCH] security: remove unused security_sysctl hook Lucian Adrian Grijincu
2011-02-01 15:02 ` [PATCH] security/selinux: fix /proc/sys/ labeling Stephen Smalley
2011-02-01 15:53   ` Lucian Adrian Grijincu
2011-02-01 15:59     ` Stephen Smalley
2011-02-01 16:32       ` Lucian Adrian Grijincu
2011-02-01 16:37         ` Stephen Smalley
2011-02-01 16:42           ` [PATCH 1/2] " Lucian Adrian Grijincu
2011-02-01 16:44             ` [PATCH 2/2] security: remove unused security_sysctl hook Lucian Adrian Grijincu
2011-02-01 19:05               ` Stephen Smalley
2011-02-01 20:06                 ` Eric Paris
2011-02-14 19:33                   ` Lucian Adrian Grijincu
2011-02-14 19:53                     ` Eric Paris
2011-02-14 20:06                       ` Lucian Adrian Grijincu
2011-02-14 22:06                         ` James Morris
2011-02-01 19:04             ` [PATCH 1/2] security/selinux: fix /proc/sys/ labeling Stephen Smalley
2011-02-01 19:33             ` Eric W. Biederman
2011-02-01 19:33             ` Eric W. Biederman
2011-02-01 19:46               ` Lucian Adrian Grijincu
2011-02-01 20:14                 ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1296519474-15714-1-git-send-email-lucian.grijincu@gmail.com \
    --to=lucian.grijincu@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=eparis@parisplace.org \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=npiggin@kernel.dk \
    --cc=sds@tycho.nsa.gov \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.