2011-06-15 18:51:56

by Vasily Kulikov

[permalink] [raw]
Subject: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

This patch adds support of mount options to restrict access to
/proc/PID/ directories. The default backward-compatible 'relaxed'
behaviour is left untouched.

The first mount option is called "hidepid" and its value defines how much
info about processes we want to be available for non-owners:

hidepid=0 (default) means the current behaviour - anybody may read all
world-readable /proc/PID/* files.

hidepid=1 means users may not access any /proc/<pid>/ directories, but their
own. Sensitive files like cmdline, io, sched*, status, wchan are now
protected against other users. As permission checking done in
proc_pid_permission() and files' permissions are left untouched,
programs expecting specific files' permissions are not confused.

hidepid=2 means hidepid=1 plus all /proc/PID/ will be invisible to
other users. It doesn't mean that it hides a fact whether a process
exists (it can be learned by other means, e.g. by sending signals), but
it hides process' euid and egid. It greatly compicates intruder's task of
gathering info about running processes, whether some daemon runs with
elevated privileges, whether other user runs some sensitive program,
whether other users run any program at all, etc.

gid=XXX defines a group that will be able to gather all processes' info.

v4 - call ptrace_may_access() as a last check to evade unnecessary
calls to security_ptrace_access_check() and capable() when in_group_p()
is sufficient.

Signed-off-by: Vasiliy Kulikov <[email protected]>
---
fs/proc/base.c | 62 ++++++++++++++++++++++++++++++++++++++++-
fs/proc/inode.c | 8 +++++
fs/proc/root.c | 18 +++++++++++-
include/linux/pid_namespace.h | 2 +
4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 14def99..b1562a3 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -627,8 +627,40 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr)
return 0;
}

+static int proc_pid_permission(struct inode *inode, int mask,
+ unsigned int flags)
+{
+ struct pid_namespace *pid = inode->i_sb->s_fs_info;
+ struct task_struct *task = get_proc_task(inode);
+
+ if (pid->hide_pid &&
+ !in_group_p(pid->pid_gid) &&
+ !ptrace_may_access(task, PTRACE_MODE_READ)) {
+ if (pid->hide_pid == 2)
+ return -ENOENT;
+ else
+ return -EPERM;
+ }
+ return generic_permission(inode, mask, flags, NULL);
+}
+
+/*
+ * May current process learn task's euid/egid?
+ */
+static bool proc_pid_may_getattr(struct pid_namespace *pid,
+ struct task_struct *task)
+{
+ if (pid->hide_pid < 2)
+ return true;
+ if (in_group_p(pid->pid_gid))
+ return true;
+ return ptrace_may_access(task, PTRACE_MODE_READ);
+}
+
+
static const struct inode_operations proc_def_inode_operations = {
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};

static int mounts_open_common(struct inode *inode, struct file *file,
@@ -1670,6 +1702,7 @@ static const struct inode_operations proc_pid_link_inode_operations = {
.readlink = proc_pid_readlink,
.follow_link = proc_pid_follow_link,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};


@@ -1737,6 +1770,7 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
struct inode *inode = dentry->d_inode;
struct task_struct *task;
const struct cred *cred;
+ struct pid_namespace *pid = dentry->d_sb->s_fs_info;

generic_fillattr(inode, stat);

@@ -1745,6 +1779,14 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
stat->gid = 0;
task = pid_task(proc_pid(inode), PIDTYPE_PID);
if (task) {
+ if (!proc_pid_may_getattr(pid, task)) {
+ rcu_read_unlock();
+ /*
+ * This doesn't prevent learning whether PID exists,
+ * it only makes getattr() consistent with readdir().
+ */
+ return -ENOENT;
+ }
if ((inode->i_mode == (S_IFDIR|S_IRUGO|S_IXUGO)) ||
task_dumpable(task)) {
cred = __task_cred(task);
@@ -2188,6 +2230,7 @@ static const struct inode_operations proc_fd_inode_operations = {
.lookup = proc_lookupfd,
.permission = proc_fd_permission,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};

static struct dentry *proc_fdinfo_instantiate(struct inode *dir,
@@ -2240,6 +2283,7 @@ static const struct file_operations proc_fdinfo_operations = {
static const struct inode_operations proc_fdinfo_inode_operations = {
.lookup = proc_lookupfdinfo,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};


@@ -2477,6 +2521,7 @@ static const struct inode_operations proc_attr_dir_inode_operations = {
.lookup = proc_attr_dir_lookup,
.getattr = pid_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};

#endif
@@ -2872,6 +2917,7 @@ static const struct inode_operations proc_tgid_base_inode_operations = {
.lookup = proc_tgid_base_lookup,
.getattr = pid_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};

static void proc_flush_task_mnt(struct vfsmount *mnt, pid_t pid, pid_t tgid)
@@ -3075,6 +3121,12 @@ static int proc_pid_fill_cache(struct file *filp, void *dirent, filldir_t filldi
proc_pid_instantiate, iter.task, NULL);
}

+static int fake_filldir(void *buf, const char *name, int namelen,
+ loff_t offset, u64 ino, unsigned d_type)
+{
+ return 0;
+}
+
/* for the /proc/ directory itself, after non-process stuff has been done */
int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
{
@@ -3082,6 +3134,7 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
struct task_struct *reaper;
struct tgid_iter iter;
struct pid_namespace *ns;
+ filldir_t __filldir;

if (filp->f_pos >= PID_MAX_LIMIT + TGID_OFFSET)
goto out_no_task;
@@ -3103,8 +3156,13 @@ int proc_pid_readdir(struct file * filp, void * dirent, filldir_t filldir)
for (iter = next_tgid(ns, iter);
iter.task;
iter.tgid += 1, iter = next_tgid(ns, iter)) {
+ if (proc_pid_may_getattr(ns, iter.task))
+ __filldir = filldir;
+ else
+ __filldir = fake_filldir;
+
filp->f_pos = iter.tgid + TGID_OFFSET;
- if (proc_pid_fill_cache(filp, dirent, filldir, iter) < 0) {
+ if (proc_pid_fill_cache(filp, dirent, __filldir, iter) < 0) {
put_task_struct(iter.task);
goto out;
}
@@ -3214,6 +3272,7 @@ static const struct inode_operations proc_tid_base_inode_operations = {
.lookup = proc_tid_base_lookup,
.getattr = pid_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};

static struct dentry *proc_task_instantiate(struct inode *dir,
@@ -3439,6 +3498,7 @@ static const struct inode_operations proc_task_inode_operations = {
.lookup = proc_task_lookup,
.getattr = proc_task_getattr,
.setattr = proc_setattr,
+ .permission = proc_pid_permission,
};

static const struct file_operations proc_task_operations = {
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index b5f49eb..34ab842 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -107,6 +107,14 @@ void __init proc_init_inodecache(void)

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->pid_gid)
+ seq_printf(seq, ",gid=%lu", (unsigned long)pid->pid_gid);
+ if (pid->hide_pid != 0)
+ seq_printf(seq, ",hidepid=%u", pid->hide_pid);
+
return 0;
}

diff --git a/fs/proc/root.c b/fs/proc/root.c
index b2571fe..fe6f2c1 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -37,10 +37,12 @@ static int proc_set_super(struct super_block *sb, void *data)
}

enum {
- Opt_err,
+ Opt_gid, Opt_hidepid, Opt_err,
};

static const match_table_t tokens = {
+ {Opt_hidepid, "hidepid=%u"},
+ {Opt_gid, "gid=%u"},
{Opt_err, NULL},
};

@@ -63,6 +65,20 @@ static int proc_parse_options(char *options, struct pid_namespace *pid)
args[0].to = args[0].from = 0;
token = match_token(p, tokens, args);
switch (token) {
+ case Opt_gid:
+ if (match_int(&args[0], &option))
+ return 0;
+ pid->pid_gid = option;
+ break;
+ case Opt_hidepid:
+ if (match_int(&args[0], &option))
+ return 0;
+ if (option < 0 || option > 2) {
+ pr_err("proc: hidepid value must be between 0 and 2.\n");
+ return 0;
+ }
+ pid->hide_pid = option;
+ break;
default:
pr_err("proc: unrecognized mount option \"%s\" "
"or missing value", p);
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 38d1032..e7cf666 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -30,6 +30,8 @@ struct pid_namespace {
#ifdef CONFIG_BSD_PROCESS_ACCT
struct bsd_acct_struct *bacct;
#endif
+ gid_t pid_gid;
+ int hide_pid;
};

extern struct pid_namespace init_pid_ns;
--
1.7.0.4


2011-06-16 02:24:50

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

(2011/06/16 3:51), Vasiliy Kulikov wrote:
> This patch adds support of mount options to restrict access to
> /proc/PID/ directories. The default backward-compatible 'relaxed'
> behaviour is left untouched.
>
> The first mount option is called "hidepid" and its value defines how much
> info about processes we want to be available for non-owners:
>
> hidepid=0 (default) means the current behaviour - anybody may read all
> world-readable /proc/PID/* files.
>
> hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> own. Sensitive files like cmdline, io, sched*, status, wchan are now
> protected against other users. As permission checking done in
> proc_pid_permission() and files' permissions are left untouched,
> programs expecting specific files' permissions are not confused.
>
> hidepid=2 means hidepid=1 plus all /proc/PID/ will be invisible to
> other users. It doesn't mean that it hides a fact whether a process
> exists (it can be learned by other means, e.g. by sending signals), but
> it hides process' euid and egid. It greatly compicates intruder's task of
> gathering info about running processes, whether some daemon runs with
> elevated privileges, whether other user runs some sensitive program,
> whether other users run any program at all, etc.
>
> gid=XXX defines a group that will be able to gather all processes' info.

Hmm...

Maybe I missed patch [0/5] or I haven't got it. Anyway I haven't see it.
Can you please describe your use case? Why do we need two new hidepid mode?
Moreover, if we use hidepid=[12], it may break some procps tools. What do
you think about compatibility issue? And, why don't you use just pid namespace?

I'm sorry if you already answered.

2011-06-16 08:47:29

by Vasily Kulikov

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

Hi KOSAKI,

On Thu, Jun 16, 2011 at 11:24 +0900, KOSAKI Motohiro wrote:
> Maybe I missed patch [0/5] or I haven't got it. Anyway I haven't see it.

0/5: https://lkml.org/lkml/2011/6/15/172 As 0's description doesn't go
into "git log", I didn't put exhaustive explaination there.

Documentation: https://lkml.org/lkml/2011/6/15/176


> Can you please describe your use case? Why do we need two new hidepid mode?

It can be used everywhere where a confidence is an issue. The most
visible part is cmdline and comm hiding, which forbids learning what
binaries other users run. status, perhaps io, stat and sched may be
indirectly used for the same purpose. These and other files may be used
for gaining indirect infomation about what kinds (and/or what amount) of
computations were performed.

Changing all these files' permissions is arguable (it's not obvious what
files are sources of potential infoleaks and in what cases), it might
lead to compatibility issues.


> Moreover, if we use hidepid=[12], it may break some procps tools. What do
> you think about compatibility issue?

Good question :) In times of -ow patch there was a pstree patch to show
independent process trees:

http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/psmisc/psmisc-21.5-owl-restricted-proc.diff?rev=1.2;content-type=text%2Fplain

It is simple to port it to the latest version of pstree. Current
version of pstree prints only one tree:

?─┬─bonobo-activati───{bonobo-activat}
├─clock-applet
├─cpufreq-applet
├─dbus-daemon
├─dbus-launch
[...]


I didn't spot any visibility issues with other process related tools -
ps and similar just print accessible processes' information skipping
nonaccessible ones.


> And, why don't you use just pid namespace?

Pid namespaces are somewhat good, but it is already possible to use
different security domains (uid and gid) inside of pid_namespace without
any trust between them (OK, root is an exception :)). Hidepid enhances
already implemented separation. Sometimes pid namespace's separation
may be too strong (no process-related communications between processes
of different namespaces unless they are parent and child namespaces).
Besides, namespaces are very expensive resource in sense of memory.


Thanks,

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

2011-06-20 05:08:16

by James Morris

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

[please cc: the lsm list with this kind of thing]

> This patch adds support of mount options to restrict access to
> /proc/PID/ directories. The default backward-compatible 'relaxed'
> behaviour is left untouched.

Can you provide evidence that this is a useful feature? e.g. examples of
exploits / techniques which would be _usefully_ hampered or blocked.

> The first mount option is called "hidepid" and its value defines how much
> info about processes we want to be available for non-owners:
>
> hidepid=0 (default) means the current behaviour - anybody may read all
> world-readable /proc/PID/* files.

Why not utilize unix perms on the proc files? Perhaps via stricter
overall defaults which are selected at kernel build or runtime.

> hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> own. Sensitive files like cmdline, io, sched*, status, wchan are now
> protected against other users. As permission checking done in
> proc_pid_permission() and files' permissions are left untouched,
> programs expecting specific files' permissions are not confused.

IMHO such programs are beyond broken and have voided their kernel
warranty.


--
James Morris
<[email protected]>

2011-06-20 10:39:27

by Vasily Kulikov

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

On Mon, Jun 20, 2011 at 15:07 +1000, James Morris wrote:
> > This patch adds support of mount options to restrict access to
> > /proc/PID/ directories. The default backward-compatible 'relaxed'
> > behaviour is left untouched.
>
> Can you provide evidence that this is a useful feature? e.g. examples of
> exploits / techniques which would be _usefully_ hampered or blocked.

First, most of these files are usefull in sense of statistics gathering
and debugging. There is no reason to provide this information to the
world.

Second, yes, it blocks one source of information used in timing attacks,
just use reading the counters as more or less precise time measurement
when actual timing measurements are not precise enough.

Third, such level of privacy (especially cmdline and comm) may be the
goal itself, where users may be anxious whether anybody knows what they
do.


> > The first mount option is called "hidepid" and its value defines how much
> > info about processes we want to be available for non-owners:
> >
> > hidepid=0 (default) means the current behaviour - anybody may read all
> > world-readable /proc/PID/* files.
>
> Why not utilize unix perms on the proc files? Perhaps via stricter
> overall defaults which are selected at kernel build or runtime.
>
> > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > protected against other users. As permission checking done in
> > proc_pid_permission() and files' permissions are left untouched,
> > programs expecting specific files' permissions are not confused.
>
> IMHO such programs are beyond broken and have voided their kernel
> warranty.

Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
procps use both /proc/PID's uid and gid. Are all of them totally broken?

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

2011-06-20 10:44:22

by James Morris

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:

> > Can you provide evidence that this is a useful feature? e.g. examples of
> > exploits / techniques which would be _usefully_ hampered or blocked.
>
> First, most of these files are usefull in sense of statistics gathering
> and debugging. There is no reason to provide this information to the
> world.
>
> Second, yes, it blocks one source of information used in timing attacks,
> just use reading the counters as more or less precise time measurement
> when actual timing measurements are not precise enough.

Can you provide concrete examples?

> > > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > > protected against other users. As permission checking done in
> > > proc_pid_permission() and files' permissions are left untouched,
> > > programs expecting specific files' permissions are not confused.
> >
> > IMHO such programs are beyond broken and have voided their kernel
> > warranty.
>
> Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
> procps use both /proc/PID's uid and gid. Are all of them totally broken?

If they depend on specific permissions, yes.

To access the information, why not just create a group with Unix read
access to these files?



- James
--
James Morris
<[email protected]>

2011-06-20 11:31:08

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

2011/6/20 James Morris <[email protected]>:
> On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:
>
>> > Can you provide evidence that this is a useful feature? ?e.g. examples of
>> > exploits / techniques which would be _usefully_ hampered or blocked.
>>
>> First, most of these files are usefull in sense of statistics gathering
>> and debugging. ?There is no reason to provide this information to the
>> world.
>>
>> Second, yes, it blocks one source of information used in timing attacks,
>> just use reading the counters as more or less precise time measurement
>> when actual timing measurements are not precise enough.
>
> Can you provide concrete examples?

Vasiliy,

I'm now stand aside James. I mean, if we don't understand your usecase
clearly. we can't gurantee to don't break the feature in the future. So,
we strongly hope to understand it.

Moreover, _now_ I haven't understand your concrete usecase, and then
_I_ can't review and be convinced your code. Please please avoid one line
answer as far as possible, please provide us more information.

2011-06-20 13:38:36

by Solar Designer

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

Hi,

I am reading this discussion via the kernel-hardening list.

On Mon, Jun 20, 2011 at 03:07:48PM +1000, James Morris wrote:
> [please cc: the lsm list with this kind of thing]
>
> > This patch adds support of mount options to restrict access to
> > /proc/PID/ directories. The default backward-compatible 'relaxed'
> > behaviour is left untouched.
>
> Can you provide evidence that this is a useful feature? e.g. examples of
> exploits / techniques which would be _usefully_ hampered or blocked.

To me, this is primarily a privacy feature on multi-user servers. This
was the primary intent behind my original implementation for Linux 2.0,
for which there was demand. Then I forward-ported this (still as an
unofficial patch) to 2.2 and later to 2.4, and Brad Spengler
forward-ported it to 2.4 and 2.6 in grsecurity. There was always
demand, primarily for privacy.

On shared web hosts that run users' scripts as the users (for greater
security and resource limits separation), process lists reveal website
access activity of each hosting user to others.

Network connections reveal where other users are connecting from and to,
which may be a privacy leak and it may allow for more focused attacks.

As to attacks not limited to reduced privacy for the users, but allowing
for unauthorized access as a next step, real-world'ish examples include
capturing passwords and filenames that another user (or their script)
passes via a command line. Yes, it is wrong to pass sensitive info like
that, yet it is sometimes done. Partial countermeasures such as mysql
overwriting -pXXX in argv (which it does) leave race conditions.
Restricting access to other users' process info is a more complete
countermeasure. Even if a bypass is ever found, it may be fixed as a
bug. Thus, the countermeasure is not as fundamentally flawed as
zeroizing of argv (yet I have nothing against that being done as well, I
find it just fine as long as its limitations are understood).
Then there are things such as usernames to other resources (databases,
etc.), which may allow for more focused attacks. As to filenames, it is
quite common for users to share semi-private files via a mode 711
directory, so the filenames become somewhat sensitive. (Of course, I am
aware of ways for users to avoid that, but security hardening, which is
what we're doing here, is about reducing risks assuming that user
behavior stays the same. This does not imply that it should stay the
same. We may be advocating for changes in behavior towards best
practices. This is just multi-layered security, where one layer does
not depend on another.)

> > The first mount option is called "hidepid" and its value defines how much
> > info about processes we want to be available for non-owners:
> >
> > hidepid=0 (default) means the current behaviour - anybody may read all
> > world-readable /proc/PID/* files.
>
> Why not utilize unix perms on the proc files? Perhaps via stricter
> overall defaults which are selected at kernel build or runtime.

My original patches used stricter permissions only. However, in this
discussion it was pointed out that this could be bypassed via netlink.
[ I am curious to know when this became possible - that is, whether some
of my old patches were similarly insufficient in that respect or not. ]
I did not follow the discussion closely, but my understanding is that
Vasiliy started changing things in response to that finding.

> > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > protected against other users. As permission checking done in
> > proc_pid_permission() and files' permissions are left untouched,
> > programs expecting specific files' permissions are not confused.
>
> IMHO such programs are beyond broken and have voided their kernel
> warranty.

My understanding is that they (at least start-stop-daemon) check owner
info (for good reasons), but not permissions. So we're free to change
permissions.

Thanks,

Alexander

2011-06-20 13:58:13

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

gid= is bad choice because
a) e. g. VFAT uses uid=/gid= mount options to make all inodes to have
certain uid/gid
b) uid=/gid=, IIRC, will be added as generic VFS mount options (like ro)
with semantics described in a)

so having different semantics for /proc won't be good.

2011-06-20 17:06:12

by Vasily Kulikov

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

(cc'ed Eric)

On Mon, Jun 20, 2011 at 20:43 +1000, James Morris wrote:
> > > > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
> > > > own. Sensitive files like cmdline, io, sched*, status, wchan are now
> > > > protected against other users. As permission checking done in
> > > > proc_pid_permission() and files' permissions are left untouched,
> > > > programs expecting specific files' permissions are not confused.
> > >
> > > IMHO such programs are beyond broken and have voided their kernel
> > > warranty.
> >
> > Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
> > procps use both /proc/PID's uid and gid. Are all of them totally broken?
>
> If they depend on specific permissions, yes.

Could you please then clarify why does this patch changes
pid_revalidate() behaviour:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=99f895518368252ba862cc15ce4eb98ebbe1bec6

It changes files permissions to allow userspace apps to quickly stat
files, not looking into /proc/PID/status. So, uid and gid are explicit
ABI. Breaking procfs uid/gid attributes would break these apps.

Or am I missing something?


Thanks,

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

2011-06-20 19:41:45

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

Vasiliy Kulikov <[email protected]> writes:

> (cc'ed Eric)
>
> On Mon, Jun 20, 2011 at 20:43 +1000, James Morris wrote:
>> > > > hidepid=1 means users may not access any /proc/<pid>/ directories, but their
>> > > > own. Sensitive files like cmdline, io, sched*, status, wchan are now
>> > > > protected against other users. As permission checking done in
>> > > > proc_pid_permission() and files' permissions are left untouched,
>> > > > programs expecting specific files' permissions are not confused.
>> > >
>> > > IMHO such programs are beyond broken and have voided their kernel
>> > > warranty.
>> >
>> > Policykit, Debian's start-stop-daemon, util-linux use /proc/PID's uid.
>> > procps use both /proc/PID's uid and gid. Are all of them totally broken?
>>
>> If they depend on specific permissions, yes.
>
> Could you please then clarify why does this patch changes
> pid_revalidate() behaviour:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=99f895518368252ba862cc15ce4eb98ebbe1bec6
>
> It changes files permissions to allow userspace apps to quickly stat
> files, not looking into /proc/PID/status. So, uid and gid are explicit
> ABI. Breaking procfs uid/gid attributes would break these apps.
>
> Or am I missing something?

No, you are not missing something. The uid and gid of proc files are
part of the userspace ABI. The files are owned by the uid and gid
of the respective process. The commit in question did not change
the ABI it documented it because I found out the hardware way that
a small change there breaks userspace.

Eric

2011-06-20 23:20:01

by James Morris

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:

> > If they depend on specific permissions, yes.
>
> Could you please then clarify why does this patch changes
> pid_revalidate() behaviour:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=99f895518368252ba862cc15ce4eb98ebbe1bec6
>
> It changes files permissions to allow userspace apps to quickly stat
> files, not looking into /proc/PID/status. So, uid and gid are explicit
> ABI. Breaking procfs uid/gid attributes would break these apps.
>

Right, but I'm saying that apps which depend on specific permissions are
broken, which is not the uid / gid attributes.


--
James Morris
<[email protected]>

2011-06-21 18:28:38

by Vasily Kulikov

[permalink] [raw]
Subject: Re: [RFC 2/5 v4] procfs: add hidepid= and gid= mount options

On Mon, Jun 20, 2011 at 20:43 +1000, James Morris wrote:
> On Mon, 20 Jun 2011, Vasiliy Kulikov wrote:
>
> > > Can you provide evidence that this is a useful feature? e.g. examples of
> > > exploits / techniques which would be _usefully_ hampered or blocked.
> >
> > First, most of these files are usefull in sense of statistics gathering
> > and debugging. There is no reason to provide this information to the
> > world.
> >
> > Second, yes, it blocks one source of information used in timing attacks,
> > just use reading the counters as more or less precise time measurement
> > when actual timing measurements are not precise enough.
>
> Can you provide concrete examples?

This is a PoC of ~user/.ssh/authorized_keys presence infoleak (and
whether it is empty) using taskstats interface:

http://www.openwall.com/lists/oss-security/2011/06/21/12

/proc/PID/io can be used too.

More close interaction with ssh client would gain authorized_keys' size or,
probably, what pam module denied the access.


Thanks,

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