Skip to contentSkip to conversation

proc: introduce pid_allowed mount option

RFC In review Vasiliy Kulikov 1 participant 2 messages Aug 10, 2011 – Aug 16, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

RFC 1/1 proc: introduce pid_allowed mount option

(This is just a PoC to show the concept of more flexible procfs permissions. Not a real patch.)

This patch adds support of pid_allowed=XX and attr_allowed=YY mount options for procfs. When set, all /proc/PID/ files are restricted to the owner, except filenames passed via pid_allowed= argument. E.g. with pid_allowed=sched sched file would be world readable and other files would be restricted to the task owner. The same for /proc/PID/attr/ files and attr_allowed=YY.

The new struct file_operations proc_pid_perms_fops was created to deal with permissions keeping in mind pid_allowed=. It is a wrapper for all /proc/PID/* and /proc/PID/task/TID/* file_operations. It checks current task permissions against the target task on each struct file access to avoid races against execve().

All pid_entry's gain the following permission checking algorithm by default:

1) if the filename is not passed via pid_allowed= then ptrace check is applied.

2) Otherwise, if file POSIX permissions deny access to the world, ptrace check is applied.

3) Otherwise, generic permission checking scheme is applied. For procfs files it additionally includes ptrace check if current euid and the procfs inode uid are equal.

For the simplicity meta values are allowed: "none" and "all". "all" means all files are a subject of (2) and (3) checks. "none" means all files are a subject of (1) check. "none" additionally restricts access to the /proc/PID/ directory itself.

Old ptrace checks are removed from the handlers as they are now obsoleted by proc_pid_perms_fops checks. Handlers may further restrict the permission model by introducing additional checks. For some files the ptrace check is too strong (environ, auxv, maps, numa_maps, and smaps), so no implicit permission checking is applied for them. These files should use pid_entry macros with "_PERMS" suffix.

Only /proc/PID/, /proc/PID/*, /proc/PID/task/TID/, /proc/PID/task/TID/*, /proc/PID/attr/* files' permission checking is changed. fd/ and fdinfo/ are outstanding files and check the permissions on their own.

A demonstration:

# mount -o remount,pid_allowed=none,attr_allowed=none /proc
# ls -ld /proc/1
dr-x------ 8 root root 0 2011-07-26 17:57 /proc/1
# mount -o remount,pid_allowed=stat /proc
# ls -ld /proc/1
dr-xr-xr-x 8 root root 0 2011-07-26 18:03 /proc/1
# ls -ld /proc/1/{stat,status,sched}
-rw------- 1 root root 0 2011-07-26 18:03 /proc/1/sched
-r--r--r-- 1 root root 0 2011-07-26 18:03 /proc/1/stat
-r-------- 1 root root 0 2011-07-26 18:03 /proc/1/status
# mount -o remount,pid_allowed=all /proc
# ls -ld /proc/1/{stat,status,sched}
-rw-r--r-- 1 root root 0 2011-07-26 18:03 /proc/1/sched
-r--r--r-- 1 root root 0 2011-07-26 18:03 /proc/1/stat
-r--r--r-- 1 root root 0 2011-07-26 18:03 /proc/1/status
# ls -ld /proc/1/task/1/ 
dr-xr-xr-x 6 root root 0 2011-07-26 18:03 /proc/1/task/1/
# mount -o remount,pid_allowed=sched /proc
# ls -ld /proc/1/task/1/{stat,status,sched}
-rw-r--r-- 1 root root 0 2011-07-26 18:04 /proc/1/task/1/sched
-r-------- 1 root root 0 2011-07-26 18:04 /proc/1/task/1/stat
-r-------- 1 root root 0 2011-07-26 18:04 /proc/1/task/1/status
# mount -o remount,pid_allowed=all /proc
# ls -ld /proc/1/task/1/{stat,status,sched}
-rw-r--r-- 1 root root 0 2011-07-26 18:04 /proc/1/task/1/sched
-r--r--r-- 1 root root 0 2011-07-26 18:04 /proc/1/task/1/stat
-r--r--r-- 1 root root 0 2011-07-26 18:04 /proc/1/task/1/status
# ls -l /proc/1/attr/
-rw------- 1 root root 0 2011-07-26 18:04 current
-rw------- 1 root root 0 2011-07-26 18:04 exec
-rw------- 1 root root 0 2011-07-26 18:04 fscreate
-rw------- 1 root root 0 2011-07-26 18:04 keycreate
-r-------- 1 root root 0 2011-07-26 18:04 prev
-rw------- 1 root root 0 2011-07-26 18:04 sockcreate
# mount -o remount,attr_allowed=exec:sockcreate /proc
# ls -l /proc/1/attr/
-rw------- 1 root root 0 2011-07-26 18:05 current
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 exec
-rw------- 1 root root 0 2011-07-26 18:05 fscreate
-rw------- 1 root root 0 2011-07-26 18:05 keycreate
-r-------- 1 root root 0 2011-07-26 18:05 prev
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 sockcreate
# mount -o remount,attr_allowed=all /proc
# ls -l /proc/1/attr/
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 current
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 exec
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 fscreate
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 keycreate
-r--r--r-- 1 root root 0 2011-07-26 18:05 prev
-rw-rw-rw- 1 root root 0 2011-07-26 18:05 sockcreate
# mount -o remount,attr_allowed=none /proc
# ls -l /proc/1/attr/
-rw------- 1 root root 0 2011-07-26 18:08 current
-rw------- 1 root root 0 2011-07-26 18:08 exec
-rw------- 1 root root 0 2011-07-26 18:08 fscreate
-rw------- 1 root root 0 2011-07-26 18:08 keycreate
-r-------- 1 root root 0 2011-07-26 18:08 prev
-rw------- 1 root root 0 2011-07-26 18:08 sockcreate

Questions/comments:

Is the whole idea of high granularity, but static permissions worth it? The previous patch with relaxed/fully-restricted modes was criticized for being highly specific to one particular set of requirements:

http://www.openwall.com/lists/kernel-hardening/2011/06/21/3

On/off mechanism was used in server environments in times of Linux 2.0-2.4 as part of -ow patch and currently as part of -grsecurity patch. There were no complains about too limited application. It serves a well defined needs (limiting processes' information to the owner, root, and a privileged "monitoring" group).

I'd be happy to hear any comments.

---
 fs/proc/Makefile              |    2 +-
 fs/proc/base.c                |  286 +++++++++++++++++++++------------------
 fs/proc/base_perms.c          |  305 +++++++++++++++++++++++++++++++++++++++++
 fs/proc/inode.c               |   19 +++
 fs/proc/internal.h            |   25 ++++
 fs/proc/root.c                |  124 ++++++++++++++++-
 fs/proc/task_nommu.c          |    2 +-
 include/linux/pid_namespace.h |   13 ++
 include/linux/proc_fs.h       |   12 ++
 kernel/pid.c                  |    2 +
 kernel/pid_namespace.c        |   10 ++-
 11 files changed, 662 insertions(+), 138 deletions(-)
fs/proc/Makefile +1 -1
index c1c7293..81020f9 100644
@@ -8,7 +8,7 @@ proc-y			:= nommu.o task_nommu.o
 proc-$(CONFIG_MMU)	:= mmu.o task_mmu.o
 
 proc-y       += inode.o root.o base.o generic.o array.o \
-		proc_tty.o
+		proc_tty.o base_perms.o
 proc-y	+= cmdline.o
 proc-y	+= consoles.o
 proc-y	+= cpuinfo.o
fs/proc/base.c +153 -133

520 lines not on this page · raw message

fs/proc/base_perms.c new file +305 -0

305 lines not on this page · raw message

fs/proc/inode.c +19 -0
index 74b48cf..022ff46 100644
@@ -18,6 +18,10 @@
 #include <linux/module.h>
 #include <linux/sysctl.h>
 #include <linux/slab.h>
+#include <linux/pid_namespace.h>
+#include <linux/seq_file.h>
+#include <linux/mount.h>
+
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -69,6 +73,8 @@ static struct inode *proc_alloc_inode(struct super_block *sb)
 	ei->sysctl_entry = NULL;
 	ei->ns = NULL;
 	ei->ns_ops = NULL;
+	ei->real_fops = NULL;
+	ei->dirent = NULL;
 	inode = &ei->vfs_inode;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 	return inode;
@@ -102,12 +108,25 @@ void __init proc_init_inodecache(void)
 					     init_once);
 }
 
+static int proc_show_options(struct seq_file *seq, struct vfsmount *vfs)
+{
+       struct super_block *sb = vfs->mnt_sb;
+       struct pid_namespace *pid = sb->s_fs_info;
+
+       if (pid->proc_perms.proc_pid_allowed)
+               seq_printf(seq, ",proc_pid_allowed=.");
+
+       return 0;
+}
+
 static const struct super_operations proc_sops = {
 	.alloc_inode	= proc_alloc_inode,
 	.destroy_inode	= proc_destroy_inode,
 	.drop_inode	= generic_delete_inode,
 	.evict_inode	= proc_evict_inode,
 	.statfs		= simple_statfs,
+	.remount_fs     = proc_remount,
+	.show_options   = proc_show_options,
 };
 
 static void __pde_users_dec(struct proc_dir_entry *pde)
fs/proc/internal.h +25 -0
index 7838e5c..fe9ed27 100644
@@ -137,11 +137,36 @@ int proc_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
 	const char *name, int len,
 	instantiate_t instantiate, struct task_struct *task, const void *ptr);
 int pid_revalidate(struct dentry *dentry, struct nameidata *nd);
+extern void __pid_revalidate(struct inode *inode, struct task_struct *task);
 struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *task);
 extern const struct dentry_operations pid_dentry_operations;
 int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat);
 int proc_setattr(struct dentry *dentry, struct iattr *attr);
+int proc_remount(struct super_block *sb, int *flags, char *data);
 
 extern const struct inode_operations proc_ns_dir_inode_operations;
 extern const struct file_operations proc_ns_dir_operations;
 
+extern const struct inode_operations proc_tgid_base_inode_operations;
+extern const struct inode_operations proc_tid_base_inode_operations;
+extern const struct inode_operations proc_attr_dir_inode_operations;
+
+extern const struct file_operations proc_pid_perms_fops;
+extern const int perms_size[];
+extern const struct pid_entry *dir_ents[];
+
+extern mode_t get_ent_perms(const struct pid_entry *p,
+		struct proc_perms *perms,
+		int ndir,
+		int nent);
+
+struct pid_entry {
+	char *name;
+	int len;
+	mode_t mode;
+	const struct inode_operations *iop;
+	const struct file_operations *fop;
+	union proc_op op;
+	bool need_perms_check;
+};
+
fs/proc/root.c +122 -2

157 lines not on this page · raw message

fs/proc/task_nommu.c +1 -1
index 980de54..da98ba2 100644
@@ -201,7 +201,7 @@ static void *m_start(struct seq_file *m, loff_t *pos)
 	if (!priv->task)
 		return ERR_PTR(-ESRCH);
 
-	mm = mm_for_maps(priv->task);
+	mm = get_task_mm(priv->task);
 	if (!mm || IS_ERR(mm)) {
 		put_task_struct(priv->task);
 		priv->task = NULL;
include/linux/pid_namespace.h +13 -0
index 38d1032..ed10329 100644
@@ -16,6 +16,18 @@ struct pidmap {
 
 struct bsd_acct_struct;
 
+enum {
+	PROC_PERMS_NTID,
+	PROC_PERMS_NTGID,
+	PROC_PERMS_NATTR,
+	PROC_PERMS_NMAX
+};
+
+struct proc_perms {
+	bool proc_pid_allowed;
+	bool *proc_ent_allowed[PROC_PERMS_NMAX];
+};
+
 struct pid_namespace {
 	struct kref kref;
 	struct pidmap pidmap[PIDMAP_ENTRIES];
@@ -30,6 +42,7 @@ struct pid_namespace {
 #ifdef CONFIG_BSD_PROCESS_ACCT
 	struct bsd_acct_struct *bacct;
 #endif
+	struct proc_perms proc_perms;
 };
 
 extern struct pid_namespace init_pid_ns;
include/linux/proc_fs.h +12 -0
index e7576cf..a1525d7 100644
@@ -115,10 +115,17 @@ struct proc_dir_entry *proc_create_data(const char *name, mode_t mode,
 extern void remove_proc_entry(const char *name, struct proc_dir_entry *parent);
 
 struct pid_namespace;
+struct proc_perms;
 
 extern int pid_ns_prepare_proc(struct pid_namespace *ns);
 extern void pid_ns_release_proc(struct pid_namespace *ns);
 
+extern int init_proc_perms(struct proc_perms *p);
+extern void free_proc_perms(struct proc_perms *p);
+
+extern void fill_proc_perms_ent(struct proc_perms *p, int ndir, bool val);
+extern bool set_proc_perms_ent(struct proc_perms *p, int ndir, const char *fname, bool val);
+
 /*
  * proc_tty.c
  */
@@ -267,6 +274,11 @@ struct proc_inode {
 	struct pid *pid;
 	int fd;
 	union proc_op op;
+
+	/* Used by files inside /proc/PID/ only */
+	const struct file_operations *real_fops;
+	const void *dirent;
+
 	struct proc_dir_entry *pde;
 	struct ctl_table_header *sysctl;
 	struct ctl_table *sysctl_entry;
kernel/pid.c +2 -0
index 57a8346..ffa8b37 100644
@@ -565,6 +565,8 @@ void __init pidmap_init(void)
 	set_bit(0, init_pid_ns.pidmap[0].page);
 	atomic_dec(&init_pid_ns.pidmap[0].nr_free);
 
+	BUG_ON(init_proc_perms(&init_pid_ns.proc_perms));
+
 	init_pid_ns.pid_cachep = KMEM_CACHE(pid,
 			SLAB_HWCACHE_ALIGN | SLAB_PANIC);
 }
kernel/pid_namespace.c +9 -2
index e9c9adc..ab4261c 100644
@@ -97,12 +97,18 @@ static struct pid_namespace *create_pid_namespace(struct pid_namespace *parent_p
 	for (i = 1; i < PIDMAP_ENTRIES; i++)
 		atomic_set(&ns->pidmap[i].nr_free, BITS_PER_PAGE);
 
-	err = pid_ns_prepare_proc(ns);
+	err = init_proc_perms(&ns->proc_perms);
 	if (err)
 		goto out_put_parent_pid_ns;
 
+	err = pid_ns_prepare_proc(ns);
+	if (err)
+		goto out_free_proc_perms;
+
 	return ns;
 
+out_free_proc_perms:
+	free_proc_perms(&ns->proc_perms);
 out_put_parent_pid_ns:
 	put_pid_ns(parent_pid_ns);
 out_free_map:
@@ -119,6 +125,8 @@ static void destroy_pid_namespace(struct pid_namespace *ns)
 
 	for (i = 0; i < PIDMAP_ENTRIES; i++)
 		kfree(ns->pidmap[i].page);
+	
+	free_proc_perms(&ns->proc_perms);
 	kmem_cache_free(pid_ns_cachep, ns);
 }
---
Signature · 1 line
Vasiliy
#1 lore raw

Hi,

Quoted 6 lines · Vasiliy Kulikov

This patch adds support of pid_allowed=XX and attr_allowed=YY mount options for procfs. When set, all /proc/PID/ files are restricted to the owner, except filenames passed via pid_allowed= argument. E.g. with pid_allowed=sched sched file would be world readable and other files would be restricted to the task owner. The same for /proc/PID/attr/ files and attr_allowed=YY.

[...]

Quoted 11 lines

Is the whole idea of high granularity, but static permissions worth it? The previous patch with relaxed/fully-restricted modes was criticized for being highly specific to one particular set of requirements:

http://www.openwall.com/lists/kernel-hardening/2011/06/21/3

On/off mechanism was used in server environments in times of Linux 2.0-2.4 as part of -ow patch and currently as part of -grsecurity patch. There were no complains about too limited application. It serves a well defined needs (limiting processes' information to the owner, root, and a privileged "monitoring" group).

Any comments about the approach in general?

Different procfs patches were maintained out of tree for ~10 years, all of them had/has the same goal - restrict processes/networking information from unprivileged users. All of them were not high granularity, just on/off.

As to chmod/chown'able approach with inheritable permission across fork() - I cannot imagine any case where it has any advantages over static approach (which is implemented in this patch), but I see disadvantages:

* What to do on exec'ing setuid binary? We have to re'chmod files to some sane defaults - first umask set.
* For usermode helpers? - second umask set.
* On privilege dropping (setuid(2))? We can have umask
set for all users, but it might not be convenient. We can have umask set for each user. Or we can use userspace for re-chmod'ing files like PAM, but what to do for apps that don't use PAM?..
* Privilege restore? - a umask set again.

What is the goal of procfs high granularity permission model? What needs does it serve? Every high granularity security model quickly becomes hard to maintain/configure properly. Given there were no out of tree attempts to extend procfs in this direction (well, at least no widespread attempts), I'm afraid such complication worth it.

I think on/off approach addresses the issue, which I've tried to explain earlier, and it is "generic enough" for the existing needs.

Thanks,

Signature · 2 lines
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments