procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Daniel Reichelt]

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

Hi all,

the attached patch enhances the procfs code to allow for modification of
/proc/<pid> dir access modes. It does so
- by hard-wiring (.config) a boot-time default mode (555, 550 or 500)
- which can be overriden by a boot parameter piddir_mode
- and also introduces an inode /proc/piddirs (644) which enables
runtime-configuration (already existing pid-dirs will be updated on next
access)


I started this quite a while back [1] and it worked fine ever since on
my servers. Now I finally enhanced that code by run-time configurability.


Let me know what you think and please CC me on replying.

Thanks,

Daniel


[1] http://lkml.org/lkml/2007/11/20/136

[-- Attachment #2: configurable-piddir-modes-2.6.38.patch --]
[-- Type: text/x-patch, Size: 9049 bytes --]

Index: linux-2.6.38/fs/proc/piddirs.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.38/fs/proc/piddirs.c	2011-03-24 04:53:33.596331008 +0100
@@ -0,0 +1,87 @@
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/pid_namespace.h>
+#include <linux/proc_fs.h>
+#include <linux/sched.h>
+#include <linux/seq_file.h>
+#include <linux/seqlock.h>
+#include <linux/time.h>
+#include "internal.h"
+
+/* prototypes */
+static int	__init param_piddir_mode(char *buf);
+static int	__init proc_piddirs_init(void);
+static int	piddirs_proc_open(struct inode *inode, struct file *file);
+static int	piddirs_proc_show(struct seq_file *m, void *v);
+static ssize_t	piddirs_proc_write(struct file *file, const char __user *buf,
+			size_t count, loff_t *ppos);
+
+/* variables */
+static const struct file_operations piddirs_proc_fops = {
+	.open		= piddirs_proc_open,
+	.read		= seq_read,
+	.write		= piddirs_proc_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+/* This char only stores the module parameter.
+   The actual piddir_creation_mode is stored in base.c */
+static char *piddir_mode;
+
+/* module config */
+module_param(piddir_mode, charp, 0);
+MODULE_PARM_DESC(piddir_mode, "Default creation mode");
+module_init(proc_piddirs_init);
+__setup("piddir_mode=", param_piddir_mode);
+
+
+
+
+static int __init param_piddir_mode(char *buf)
+{
+	if (strlen(buf))
+		set_piddir_creation_mode(buf[0]);
+	return 1;
+}
+
+static int __init proc_piddirs_init(void)
+{
+	proc_create(
+		"piddirs",
+		S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH,
+		NULL,
+		&piddirs_proc_fops
+	);
+	return 0;
+}
+
+static int piddirs_proc_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, piddirs_proc_show, NULL);
+}
+
+static int piddirs_proc_show(struct seq_file *m, void *v)
+{
+	seq_printf(m, "%c\n", get_piddir_creation_mode());
+	return 0;
+}
+
+static ssize_t piddirs_proc_write(struct file *file,
+		const char __user *buf,
+		size_t count, loff_t *ppos)
+{
+	char tmp;
+
+	if (
+		(count == 2) &&
+		!copy_from_user(&tmp, buf, 1) &&
+		(set_piddir_creation_mode(tmp) == 0)
+	) {
+		return count;
+	} else {
+		return -EFAULT;
+	}
+}
Index: linux-2.6.38/fs/proc/internal.h
===================================================================
--- linux-2.6.38.orig/fs/proc/internal.h	2011-03-22 10:00:09.717152492 +0100
+++ linux-2.6.38/fs/proc/internal.h	2011-03-22 11:28:42.205497133 +0100
@@ -94,6 +94,10 @@
 extern spinlock_t proc_subdir_lock;
 
 struct dentry *proc_pid_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *);
+#ifndef CONFIG_PROC_PIDDIRS_UNRESTRICTED
+char get_piddir_creation_mode(void);
+int set_piddir_creation_mode(char mode);
+#endif
 int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir);
 unsigned long task_vsize(struct mm_struct *);
 unsigned long task_statm(struct mm_struct *,
Index: linux-2.6.38/fs/proc/base.c
===================================================================
--- linux-2.6.38.orig/fs/proc/base.c	2011-03-22 10:00:09.797214588 +0100
+++ linux-2.6.38/fs/proc/base.c	2011-03-24 04:54:51.292565614 +0100
@@ -104,6 +104,14 @@
 	union proc_op op;
 };
 
+#if defined CONFIG_PROC_PIDDIRS_MODE_U
+	char piddir_creation_mode = 'u';
+#elif defined CONFIG_PROC_PIDDIRS_MODE_UG
+	char piddir_creation_mode = 'g';
+#elif defined CONFIG_PROC_PIDDIRS_MODE_UGO
+	char piddir_creation_mode = 'o';
+#endif
+
 #define NOD(NAME, MODE, IOP, FOP, OP) {			\
 	.name = (NAME),					\
 	.len  = sizeof(NAME) - 1,			\
@@ -1779,8 +1787,15 @@
 	task = get_proc_task(inode);
 
 	if (task) {
-		if ((inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO)) ||
-		    task_dumpable(task)) {
+		if (
+			#ifndef CONFIG_PROC_PIDDIRS_UNRESTRICTED
+			(inode->i_mode == (S_IFDIR|S_IRUSR|S_IXUSR)) ||
+			(inode->i_mode ==
+				(S_IFDIR|S_IRUSR|S_IRGRP|S_IXUSR|S_IXGRP)) ||
+			#endif
+			(inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO)) ||
+			task_dumpable(task)
+		) {
 			rcu_read_lock();
 			cred = __task_cred(task);
 			inode->i_uid = cred->euid;
@@ -1791,6 +1806,21 @@
 			inode->i_gid = 0;
 		}
 		inode->i_mode &= ~(S_ISUID | S_ISGID);
+		#ifndef CONFIG_PROC_PIDDIRS_UNRESTRICTED
+		switch (piddir_creation_mode) {
+		case 'u':
+			inode->i_mode &= ~(S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
+			break;
+		case 'g':
+			inode->i_mode |= S_IRUSR|S_IXUSR|S_IRGRP|S_IXGRP;
+			inode->i_mode &= ~(S_IROTH|S_IXOTH);
+			break;
+		case 'o':
+			inode->i_mode |= S_IRUGO|S_IXUGO;
+			break;
+		}
+		#endif
+
 		security_task_to_inode(task, inode);
 		put_task_struct(task);
 		return 1;
@@ -2950,6 +2980,35 @@
 		pid_ns_release_proc(upid->ns);
 }
 
+#ifndef CONFIG_PROC_PIDDIRS_UNRESTRICTED
+char get_piddir_creation_mode(void)
+{
+	return piddir_creation_mode;
+}
+
+int set_piddir_creation_mode(char mode)
+{
+	switch (mode) {
+	case 'u':
+		printk(KERN_INFO "procfs: Setting /proc/<pid>-dir creation"
+			" mode to 500\n");
+		break;
+	case 'g':
+		printk(KERN_INFO "procfs: Setting /proc/<pid>-dir creation"
+			" mode to 550\n");
+		break;
+	case 'o':
+		printk(KERN_INFO "procfs: Setting /proc/<pid>-dir creation"
+			" mode to 555\n");
+		break;
+	default:
+		return -EINVAL;
+	}
+	piddir_creation_mode = mode;
+	return 0;
+}
+#endif
+
 static struct dentry *proc_pid_instantiate(struct inode *dir,
 					   struct dentry * dentry,
 					   struct task_struct *task, const void *ptr)
@@ -2961,7 +3020,22 @@
 	if (!inode)
 		goto out;
 
+	#ifndef CONFIG_PROC_PIDDIRS_UNRESTRICTED
+	switch (piddir_creation_mode) {
+	case 'u':
+		inode->i_mode = S_IFDIR|S_IRUSR|S_IXUSR;
+		break;
+	case 'g':
+		inode->i_mode = S_IFDIR|S_IRUSR|S_IRGRP|S_IXUSR|S_IXGRP;
+		break;
+	case 'o':
+		inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
+		break;
+	}
+	#else
 	inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
+	#endif
+
 	inode->i_op = &proc_tgid_base_inode_operations;
 	inode->i_fop = &proc_tgid_base_operations;
 	inode->i_flags|=S_IMMUTABLE;
Index: linux-2.6.38/fs/proc/Makefile
===================================================================
--- linux-2.6.38.orig/fs/proc/Makefile	2011-03-22 10:00:09.853177027 +0100
+++ linux-2.6.38/fs/proc/Makefile	2011-03-24 03:48:01.520964812 +0100
@@ -27,3 +27,6 @@
 proc-$(CONFIG_PROC_DEVICETREE)	+= proc_devtree.o
 proc-$(CONFIG_PRINTK)	+= kmsg.o
 proc-$(CONFIG_PROC_PAGE_MONITOR)	+= page.o
+proc-$(CONFIG_PROC_PIDDIRS_MODE_UGO)	+= piddirs.o
+proc-$(CONFIG_PROC_PIDDIRS_MODE_UG)	+= piddirs.o
+proc-$(CONFIG_PROC_PIDDIRS_MODE_U)	+= piddirs.o
Index: linux-2.6.38/fs/proc/Kconfig
===================================================================
--- linux-2.6.38.orig/fs/proc/Kconfig	2011-03-22 10:00:09.757199278 +0100
+++ linux-2.6.38/fs/proc/Kconfig	2011-03-24 03:46:28.488680884 +0100
@@ -67,3 +67,73 @@
 	  /proc/pid/smaps, /proc/pid/clear_refs, /proc/pid/pagemap,
 	  /proc/kpagecount, and /proc/kpageflags. Disabling these
           interfaces will reduce the size of the kernel by approximately 4kb.
+
+choice PROC_PIDDIRS
+	prompt "Restrict access to /proc/<pid>-dirs"
+	default PROC_PIDDIRS_MODE_UGO
+	help
+	  When set to "no restriction", the access mode for /proc/<pid>-dirs is
+	  555 (r-xr-xr-x) which is the traditional behaviour.
+
+
+	  Enabling one of the other options has three impacts:
+
+	  1) set the default access mode to the respective value
+
+	  2) enable a boot parameter called piddir_mode; possible values are
+	     - 'u': limit to user
+	     - 'g': limit to user and group
+	     - 'o': no limits (other may read)
+
+	     e.g. piddir_mode=u
+
+	  3) enable runtime-configuration of the access modes by issuing e.g.
+	  	echo g > /proc/piddirs
+	     Again, the possible values are 'u', 'g', 'o'.
+
+config PROC_PIDDIRS_UNRESTRICTED
+	bool "no restriction"
+	depends on PROC_FS
+	help
+	  Don't restrict access to /proc/<pid>-dirs, i.e. leave mode at 555
+	  respectively r-xr-xr-x . Runtime manipulation of access mode is
+	  disabled. This is the traditional mode of operation.
+
+	  If unsure, say Y.
+
+config PROC_PIDDIRS_MODE_UGO
+	bool "unrestricted by default"
+	depends on PROC_FS
+	help
+	  Don't restrict access to /proc/<pid>-dirs, i.e. leave mode at 555
+	  (r-xr-xr-x). The default access mode can be changed to 555 during
+	  runtime e.g. by issuing
+
+	  	echo o > /proc/piddirs
+
+	  If unsure, say N.
+
+config PROC_PIDDIRS_MODE_UG
+	bool "restrict to user and group by default"
+	depends on PROC_FS
+	help
+	  Restrict access to /proc/<pid>-dirs to user and group, i.e. set mode
+	  to 550 (r-xr-x---). The default access mode can be changed to 550
+	  during runtime e.g. by issuing
+
+	  	echo g > /proc/piddirs
+
+	  If unsure, say N.
+
+config PROC_PIDDIRS_MODE_U
+	bool "restrict to user by default"
+	depends on PROC_FS
+	help
+	  Restrict access to /proc/<pid>-dirs to user only, i.e. set mode to
+	  500 (r-x------). The default access mode can be changed to 500 during
+	  runtime e.g. by issuing
+
+	  	echo u > /proc/piddirs
+
+	  If unsure, say N.
+endchoice

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Alexey Dobriyan]

On Thu, Mar 24, 2011 at 05:09:44AM +0100, Daniel Reichelt wrote:
> the attached patch enhances the procfs code to allow for modification of
> /proc/<pid> dir access modes. It does so
> - by hard-wiring (.config) a boot-time default mode (555, 550 or 500)
> - which can be overriden by a boot parameter piddir_mode
> - and also introduces an inode /proc/piddirs (644) which enables
> runtime-configuration (already existing pid-dirs will be updated on next
> access)

> Let me know what you think and please CC me on replying.

Keeping u/g/o inside kernel is horrible.

What is the usecase? Content of /proc/* is identical.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Daniel Reichelt]

> Keeping u/g/o inside kernel is horrible.

Why exactly? Since it's only a char and not char[] I don't see the
disadvantage over int or a define or whatever. Of course I could always
change that if that's a de-facto standard I just didn't know about.


> What is the usecase? Content of /proc/* is identical.

Use-case is to isolate process information from other users' or groups'
eyes, e.g. with 550 the output of ps aux only lists processes of the
groups your user is a member of.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Alexey Dobriyan]

On Thu, Mar 24, 2011 at 09:41:58AM +0100, Daniel Reichelt wrote:
> > Keeping u/g/o inside kernel is horrible.
> 
> Why exactly? Since it's only a char and not char[] I don't see the
> disadvantage over int or a define or whatever. Of course I could always
> change that if that's a de-facto standard I just didn't know about.

Keep mode_t inside kernel, this will get rid of many ifdefs.
 
> > What is the usecase? Content of /proc/* is identical.
> 
> Use-case is to isolate process information from other users' or groups'
> eyes, e.g. with 550 the output of ps aux only lists processes of the
> groups your user is a member of.

This is doable with some ps(1) switch, I'm sure.

The content of /proc/$PID directory is not a secret.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Al Viro]

On Thu, Mar 24, 2011 at 08:22:30PM +0200, Alexey Dobriyan wrote:
> On Thu, Mar 24, 2011 at 09:41:58AM +0100, Daniel Reichelt wrote:
> > > Keeping u/g/o inside kernel is horrible.
> > 
> > Why exactly? Since it's only a char and not char[] I don't see the
> > disadvantage over int or a define or whatever. Of course I could always
> > change that if that's a de-facto standard I just didn't know about.
> 
> Keep mode_t inside kernel, this will get rid of many ifdefs.
>  
> > > What is the usecase? Content of /proc/* is identical.
> > 
> > Use-case is to isolate process information from other users' or groups'
> > eyes, e.g. with 550 the output of ps aux only lists processes of the
> > groups your user is a member of.
> 
> This is doable with some ps(1) switch, I'm sure.
> 
> The content of /proc/$PID directory is not a secret.

More to the point, permissions in /proc/<pid>/* don't do us much good.
As the matter of fact, we ought to make them all flat - i.e. same for
user/group/other, since we have to recheck access rights on every damn
IO operations.  Checks done at open() are useless here - have the
task exec suid-root binary and they are obsolete.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Daniel Reichelt]

>> Why exactly? Since it's only a char and not char[] I don't see the
>> disadvantage over int or a define or whatever. Of course I could always
>> change that if that's a de-facto standard I just didn't know about.
> 
> Keep mode_t inside kernel, this will get rid of many ifdefs.

Too obvious, point made.


>> Use-case is to isolate process information from other users' or groups'
>> eyes, e.g. with 550 the output of ps aux only lists processes of the
>> groups your user is a member of.
> 
> This is doable with some ps(1) switch, I'm sure.
> 
> The content of /proc/$PID directory is not a secret.

Sure, I could just run ps ux instead of ps aux and I'm done - in case I
wanna see only MY procs. That's my very point: sometimes it needs to be
a secret and not by ps-invoking-user's choice at that but by an admin's
enforcement. There are cases where I wouldn't want anybody ELSE to know
ANYTHING about my procs, not even their existence. So even when I'm root
on a box and I could restrict user-space tools...there's always another
unrestricted one. A curious user just compiles his own toy and goes fishing.

Real-world example: amongst many other (administrative) isolation
mechanisms to keep users apart, I've been using this approach for years
to enforce privacy in several hosting environments. Just think of poorly
implemented software which doesn't mask cmdline parameters like
--password. Of course one could argue "Just switch to another software."
Needless to say, that's often not option.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Daniel Reichelt]

> More to the point, permissions in /proc/<pid>/* don't do us much good.
> As the matter of fact, we ought to make them all flat - i.e. same for
> user/group/other, since we have to recheck access rights on every damn
> IO operations.  Checks done at open() are useless here - have the
> task exec suid-root binary and they are obsolete.

Well, my patch is about modes of the pid-dirs themselves, not their
contents. And it changes procfs' behaviour about modes both on initial
creation and during revalidation on access. However flattening all the
piddir's entries DOES pose a security risk. Have a look at the
"traditional behaviour": piddir world-readable, however e.g.
<pid>/environ isn't. Often it's a workaround for broken software to
specify a password within an environment variable instead of by cmdline.
Since up until now all processes including their full cmdlines are
visible to everyone, environ must be considered more sensitive than a
cmdline.

The security implications are taken care of by my patch. The more
restrictive the modes for the piddir, the less access to its contents.
So from a security point of view, my patch takes care of that and IMHO
there's no need to put hands on the procfs' management of modes for
piddir-contents.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Al Viro]

On Thu, Mar 24, 2011 at 08:18:54PM +0100, Daniel Reichelt wrote:
> Well, my patch is about modes of the pid-dirs themselves, not their
> contents. And it changes procfs' behaviour about modes both on initial
> creation and during revalidation on access. However flattening all the
> piddir's entries DOES pose a security risk. Have a look at the
> "traditional behaviour": piddir world-readable, however e.g.
> <pid>/environ isn't. Often it's a workaround for broken software to
> specify a password within an environment variable instead of by cmdline.
> Since up until now all processes including their full cmdlines are
> visible to everyone, environ must be considered more sensitive than a
> cmdline.

Bull.  /proc/<pid>/foo contents is sensitive, your patch doesn't do
you any good.  fork(), open /proc/<child's PID>/foo in parent, then
exec suid-root binary in child.  At that point mode_t of any files
or directories does not matter anymore.

You *must* do whatever access checks at read(2) time for these files.
And if you do that, the checks at open(2) time do not matter.

In particular, /proc/*/environ does ptrace_may_access() on read(2) (with
a race fixed by today's merge, BTW).  It could very well be r--r--r--;
having it r-------- does not increase security at all.

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Christian Kujau]

Not sure If I understand correctly, but:

On Thu, 24 Mar 2011 at 20:37, Al Viro wrote:
> Bull.  /proc/<pid>/foo contents is sensitive, your patch doesn't do
> you any good.  fork(), open /proc/<child's PID>/foo in parent, then
> exec suid-root binary in child.

...you would have to roll your own suid-root binary to be able to look 
into other /proc/PID directories, no? But making a binary suid-root 
requires root to begin with.

I'd love to finally have a more restrictive /proc directory. Even if it 
only makes things "harder" (not necessarily "impossible") to get 
information from user users on the same box.

Christian.
-- 
BOFH excuse #216:

What office are you in? Oh, that one.  Did you know that your building was built over the universities first nuclear research site? And wow, aren't you the lucky one, your office is right over where the core is buried!

Re: procfs: boot- and runtime configurable access mode for /proc/<pid> dirs

[Al Viro]

On Fri, Mar 25, 2011 at 02:24:48PM -0700, Christian Kujau wrote:
> 
> Not sure If I understand correctly, but:
> 
> On Thu, 24 Mar 2011 at 20:37, Al Viro wrote:
> > Bull.  /proc/<pid>/foo contents is sensitive, your patch doesn't do
> > you any good.  fork(), open /proc/<child's PID>/foo in parent, then
> > exec suid-root binary in child.
> 
> ...you would have to roll your own suid-root binary to be able to look 
> into other /proc/PID directories, no? But making a binary suid-root 
> requires root to begin with.

No.  You could look into /proc/PID/foo of process running a suid-root binary
that you have execve()'ed.  A binary that had been there legitimately.  If
these files have sensitive contents, you have a bad problem on hands (and we
had quite a few of those); IOW, you need to do checks at read(2) time, since
the identity of owner can change between open() and read().  And if you do
those checks there, you don't need to care about open-time checks at all,
since any attempt to do IO will be rejected anyway.