| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 09 Sep 2017 22:47:40 +0100
From: Ben Hutchings <ben@...adent.org.uk>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC: akpm@...ux-foundation.org,
"Eric W. Biederman" <ebiederm@...ssion.com>,
"Jann Horn" <jann@...jh.net>,
"Serge E. Hallyn" <serge.hallyn@...ntu.com>,
"Al Viro" <viro@...iv.linux.org.uk>, "Willy Tarreau" <w@....eu>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
"Linus Torvalds" <torvalds@...ux-foundation.org>,
"Casey Schaufler" <casey@...aufler-ca.com>,
"Kees Cook" <keescook@...omium.org>,
"Andy Lutomirski" <luto@...nel.org>,
"James Morris" <james.l.morris@...cle.com>,
"Ingo Molnar" <mingo@...hat.com>,
"Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
"Oleg Nesterov" <oleg@...hat.com>
Subject: [PATCH 3.2 101/106] ptrace: use fsuid, fsgid, effective creds for
fs access checks
3.2.93-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Jann Horn <jann@...jh.net>
commit caaee6234d05a58c5b4d05e7bf766131b810a657 upstream.
By checking the effective credentials instead of the real UID / permitted
capabilities, ensure that the calling process actually intended to use its
credentials.
To ensure that all ptrace checks use the correct caller credentials (e.g.
in case out-of-tree code or newly added code omits the PTRACE_MODE_*CREDS
flag), use two new flags and require one of them to be set.
The problem was that when a privileged task had temporarily dropped its
privileges, e.g. by calling setreuid(0, user_uid), with the intent to
perform following syscalls with the credentials of a user, it still passed
ptrace access checks that the user would not be able to pass.
While an attacker should not be able to convince the privileged task to
perform a ptrace() syscall, this is a problem because the ptrace access
check is reused for things in procfs.
In particular, the following somewhat interesting procfs entries only rely
on ptrace access checks:
/proc/$pid/stat - uses the check for determining whether pointers
should be visible, useful for bypassing ASLR
/proc/$pid/maps - also useful for bypassing ASLR
/proc/$pid/cwd - useful for gaining access to restricted
directories that contain files with lax permissions, e.g. in
this scenario:
lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
drwx------ root root /root
drwxr-xr-x root root /root/foobar
-rw-r--r-- root root /root/foobar/secret
Therefore, on a system where a root-owned mode 6755 binary changes its
effective credentials as described and then dumps a user-specified file,
this could be used by an attacker to reveal the memory layout of root's
processes or reveal the contents of files he is not allowed to access
(through /proc/$pid/cwd).
[akpm@...ux-foundation.org: fix warning]
Signed-off-by: Jann Horn <jann@...jh.net>
Acked-by: Kees Cook <keescook@...omium.org>
Cc: Casey Schaufler <casey@...aufler-ca.com>
Cc: Oleg Nesterov <oleg@...hat.com>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: James Morris <james.l.morris@...cle.com>
Cc: "Serge E. Hallyn" <serge.hallyn@...ntu.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: Al Viro <viro@...iv.linux.org.uk>
Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Willy Tarreau <w@....eu>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
[bwh: Backported to 3.2:
- Drop changes to kcmp, procfs map_files, procfs has_pid_permissions()
- Keep using uid_t, gid_t and == operator for IDs
- Adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -380,7 +380,7 @@ static int do_task_stat(struct seq_file
state = *get_task_state(task);
vsize = eip = esp = 0;
- permitted = ptrace_may_access(task, PTRACE_MODE_READ);
+ permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
mm = get_task_mm(task);
if (mm) {
vsize = task_vsize(mm);
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -216,7 +216,7 @@ static struct mm_struct *mm_access(struc
struct mm_struct *mm_for_maps(struct task_struct *task)
{
- return mm_access(task, PTRACE_MODE_READ);
+ return mm_access(task, PTRACE_MODE_READ_FSCREDS);
}
static int proc_pid_cmdline(struct task_struct *task, char * buffer)
@@ -288,7 +288,7 @@ static int proc_pid_wchan(struct task_st
wchan = get_wchan(task);
if (lookup_symbol_name(wchan, symname) < 0)
- if (!ptrace_may_access(task, PTRACE_MODE_READ))
+ if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
return 0;
else
return sprintf(buffer, "%lu", wchan);
@@ -302,7 +302,7 @@ static int lock_trace(struct task_struct
int err = mutex_lock_killable(&task->signal->cred_guard_mutex);
if (err)
return err;
- if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
+ if (!ptrace_may_access(task, PTRACE_MODE_ATTACH_FSCREDS)) {
mutex_unlock(&task->signal->cred_guard_mutex);
return -EPERM;
}
@@ -544,7 +544,7 @@ static int proc_fd_access_allowed(struct
*/
task = get_proc_task(inode);
if (task) {
- allowed = ptrace_may_access(task, PTRACE_MODE_READ);
+ allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);
put_task_struct(task);
}
return allowed;
@@ -769,7 +769,7 @@ static int mem_open(struct inode* inode,
if (!task)
return -ESRCH;
- mm = mm_access(task, PTRACE_MODE_ATTACH);
+ mm = mm_access(task, PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS);
put_task_struct(task);
if (IS_ERR(mm))
@@ -2627,7 +2627,7 @@ static int do_io_accounting(struct task_
if (result)
return result;
- if (!ptrace_may_access(task, PTRACE_MODE_READ)) {
+ if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) {
result = -EACCES;
goto out_unlock;
}
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -91,7 +91,7 @@ static int proc_ns_dir_readdir(struct fi
goto out_no_task;
ret = -EPERM;
- if (!ptrace_may_access(task, PTRACE_MODE_READ))
+ if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
goto out;
ret = 0;
@@ -154,7 +154,7 @@ static struct dentry *proc_ns_dir_lookup
goto out_no_task;
error = ERR_PTR(-EPERM);
- if (!ptrace_may_access(task, PTRACE_MODE_READ))
+ if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
goto out;
last = &ns_entries[ARRAY_SIZE(ns_entries) - 1];
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -130,9 +130,31 @@ extern void __ptrace_unlink(struct task_
extern void exit_ptrace(struct task_struct *tracer);
#define PTRACE_MODE_READ 1
#define PTRACE_MODE_ATTACH 2
+#define PTRACE_MODE_FSCREDS 0x08
+#define PTRACE_MODE_REALCREDS 0x10
+
+/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
+#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
+#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
+#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
+#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS)
+
/* Returns 0 on success, -errno on denial. */
extern int __ptrace_may_access(struct task_struct *task, unsigned int mode);
-/* Returns true on success, false on denial. */
+/**
+ * ptrace_may_access - check whether the caller is permitted to access
+ * a target task.
+ * @task: target task
+ * @mode: selects type of access and caller credentials
+ *
+ * Returns true on success, false on denial.
+ *
+ * One of the flags PTRACE_MODE_FSCREDS and PTRACE_MODE_REALCREDS must
+ * be set in @mode to specify whether the access was requested through
+ * a filesystem syscall (should use effective capabilities and fsuid
+ * of the caller) or through an explicit syscall such as
+ * process_vm_writev or ptrace (and should use the real credentials).
+ */
extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
static inline int ptrace_reparented(struct task_struct *child)
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3004,7 +3004,7 @@ find_lively_task_by_vpid(pid_t vpid)
/* Reuse ptrace permission checks for now. */
err = -EACCES;
- if (!ptrace_may_access(task, PTRACE_MODE_READ))
+ if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS))
goto errout;
return task;
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -2627,7 +2627,7 @@ SYSCALL_DEFINE3(get_robust_list, int, pi
}
ret = -EPERM;
- if (!ptrace_may_access(p, PTRACE_MODE_READ))
+ if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
goto err_unlock;
head = p->robust_list;
--- a/kernel/futex_compat.c
+++ b/kernel/futex_compat.c
@@ -154,7 +154,7 @@ compat_sys_get_robust_list(int pid, comp
}
ret = -EPERM;
- if (!ptrace_may_access(p, PTRACE_MODE_READ))
+ if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
goto err_unlock;
head = p->compat_robust_list;
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -220,6 +220,14 @@ int ptrace_check_attach(struct task_stru
int __ptrace_may_access(struct task_struct *task, unsigned int mode)
{
const struct cred *cred = current_cred(), *tcred;
+ int dumpable = 0;
+ uid_t caller_uid;
+ gid_t caller_gid;
+
+ if (!(mode & PTRACE_MODE_FSCREDS) == !(mode & PTRACE_MODE_REALCREDS)) {
+ WARN(1, "denying ptrace access check without PTRACE_MODE_*CREDS\n");
+ return -EPERM;
+ }
/* May we inspect the given task?
* This check is used both for attaching with ptrace
@@ -229,19 +237,34 @@ int __ptrace_may_access(struct task_stru
* because setting up the necessary parent/child relationship
* or halting the specified task is impossible.
*/
- int dumpable = 0;
+
/* Don't let security modules deny introspection */
if (same_thread_group(task, current))
return 0;
rcu_read_lock();
+ if (mode & PTRACE_MODE_FSCREDS) {
+ caller_uid = cred->fsuid;
+ caller_gid = cred->fsgid;
+ } else {
+ /*
+ * Using the euid would make more sense here, but something
+ * in userland might rely on the old behavior, and this
+ * shouldn't be a security problem since
+ * PTRACE_MODE_REALCREDS implies that the caller explicitly
+ * used a syscall that requests access to another process
+ * (and not a filesystem syscall to procfs).
+ */
+ caller_uid = cred->uid;
+ caller_gid = cred->gid;
+ }
tcred = __task_cred(task);
if (cred->user->user_ns == tcred->user->user_ns &&
- (cred->uid == tcred->euid &&
- cred->uid == tcred->suid &&
- cred->uid == tcred->uid &&
- cred->gid == tcred->egid &&
- cred->gid == tcred->sgid &&
- cred->gid == tcred->gid))
+ (caller_uid == tcred->euid &&
+ caller_uid == tcred->suid &&
+ caller_uid == tcred->uid &&
+ caller_gid == tcred->egid &&
+ caller_gid == tcred->sgid &&
+ caller_gid == tcred->gid))
goto ok;
if (ns_capable(tcred->user->user_ns, CAP_SYS_PTRACE))
goto ok;
@@ -308,7 +331,7 @@ static int ptrace_attach(struct task_str
goto out;
task_lock(task);
- retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH);
+ retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
task_unlock(task);
if (retval)
goto unlock_creds;
--- a/mm/process_vm_access.c
+++ b/mm/process_vm_access.c
@@ -299,7 +299,7 @@ static ssize_t process_vm_rw_core(pid_t
}
task_lock(task);
- if (__ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
+ if (__ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS)) {
task_unlock(task);
rc = -EPERM;
goto put_task_struct;
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -141,12 +141,17 @@ int cap_ptrace_access_check(struct task_
{
int ret = 0;
const struct cred *cred, *child_cred;
+ const kernel_cap_t *caller_caps;
rcu_read_lock();
cred = current_cred();
child_cred = __task_cred(child);
+ if (mode & PTRACE_MODE_FSCREDS)
+ caller_caps = &cred->cap_effective;
+ else
+ caller_caps = &cred->cap_permitted;
if (cred->user->user_ns == child_cred->user->user_ns &&
- cap_issubset(child_cred->cap_permitted, cred->cap_permitted))
+ cap_issubset(child_cred->cap_permitted, *caller_caps))
goto out;
if (ns_capable(child_cred->user->user_ns, CAP_SYS_PTRACE))
goto out;
Powered by blists - more mailing lists