|
Message-ID: <20131003143653.GA32445@dztty> Date: Thu, 3 Oct 2013 15:36:54 +0100 From: Djalal Harouni <tixxdz@...ndz.org> To: Andy Lutomirski <luto@...capital.net> Cc: "Eric W. Biederman" <ebiederm@...ssion.com>, Kees Cook <keescook@...omium.org>, Al Viro <viro@...iv.linux.org.uk>, Andrew Morton <akpm@...ux-foundation.org>, Linus Torvalds <torvalds@...ux-foundation.org>, Ingo Molnar <mingo@...nel.org>, "Serge E. Hallyn" <serge.hallyn@...ntu.com>, Cyrill Gorcunov <gorcunov@...nvz.org>, David Rientjes <rientjes@...gle.com>, LKML <linux-kernel@...r.kernel.org>, Linux FS Devel <linux-fsdevel@...r.kernel.org>, kernel-hardening@...ts.openwall.com, tixxdz@...il.com Subject: Re: [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's opener may access task On Wed, Oct 02, 2013 at 05:44:17PM +0100, Andy Lutomirski wrote: > On Wed, Oct 2, 2013 at 3:55 PM, Djalal Harouni <tixxdz@...ndz.org> wrote: > > On Tue, Oct 01, 2013 at 06:36:34PM -0700, Andy Lutomirski wrote: > >> On 10/01/2013 01:26 PM, Djalal Harouni wrote: > >> > Since /proc entries varies at runtime, permission checks need to happen > >> > during each system call. > >> > > >> > However even with that /proc file descriptors can be passed to a more > >> > privileged process (e.g. a suid-exec) which will pass the classic > >> > ptrace_may_access() permission check. The open() call will be issued in > >> > general by an unprivileged process while the disclosure of sensitive > >> > /proc information will happen using a more privileged process at > >> > read(),write()... > >> > > >> > Therfore we need a more sophisticated check to detect if the cred of the > >> > process have changed, and if the cred of the original opener that are > >> > stored in the file->f_cred have enough permission to access the task's > >> > /proc entries during read(), write()... > >> > > >> > Add the proc_allow_access() function that will receive the file->f_cred > >> > as an argument, and tries to check if the opener had enough permission > >> > to access the task's /proc entries. > >> > > >> > This function should be used with the ptrace_may_access() check. > >> > > >> > Cc: Kees Cook <keescook@...omium.org> > >> > Suggested-by: Eric W. Biederman <ebiederm@...ssion.com> > >> > Signed-off-by: Djalal Harouni <tixxdz@...ndz.org> > >> > --- > >> > fs/proc/base.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> > fs/proc/internal.h | 2 ++ > >> > 2 files changed, 58 insertions(+) > >> > > >> > diff --git a/fs/proc/base.c b/fs/proc/base.c > >> > index e834946..c29eeae 100644 > >> > --- a/fs/proc/base.c > >> > +++ b/fs/proc/base.c > >> > @@ -168,6 +168,62 @@ int proc_same_open_cred(const struct cred *fcred) > >> > cap_issubset(cred->cap_permitted, fcred->cap_permitted)); > >> > } > >> > > >> > +/* Returns 0 on success, -errno on denial. */ > >> > +static int __proc_allow_access(const struct cred *cred, > >> > + struct task_struct *task, unsigned int mode) > >> > +{ > >> > + int ret = 0; > >> > + const struct cred *tcred; > >> > + const struct cred *fcred = cred; > >> > + > >> > + rcu_read_lock(); > >> > + tcred = __task_cred(task); > >> > + if (uid_eq(fcred->uid, tcred->euid) && > >> > + uid_eq(fcred->uid, tcred->suid) && > >> > + uid_eq(fcred->uid, tcred->uid) && > >> > + gid_eq(fcred->gid, tcred->egid) && > >> > + gid_eq(fcred->gid, tcred->sgid) && > >> > + gid_eq(fcred->gid, tcred->gid)) > >> > + goto out; > >> > + > >> > >> What's this for? Is it supposed to be an optimization? If so, it looks > >> potentially exploitable, although I don't really understand what you're > >> trying to do. > > This function should be used in addition to the ptrace_may_access() one. > > Sorry, I was unclear. I meant: what are the uid and gid checks for? The uid/gid are checks of the current (reader) on the target task, like the ptrace checks. fcred here is the cred of current at open time. -- Djalal Harouni http://opendz.org
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.