| From 491fcbda4829119c6860d6f2536f92cb763713ba Mon Sep 17 00:00:00 2001 |
| From: Christian Brauner <christian.brauner@ubuntu.com> |
| Date: Wed, 15 Jan 2020 14:42:34 +0100 |
| Subject: [PATCH] ptrace: reintroduce usage of subjective credentials in |
| ptrace_has_cap() |
| |
| commit 6b3ad6649a4c75504edeba242d3fd36b3096a57f upstream. |
| |
| Commit 69f594a38967 ("ptrace: do not audit capability check when outputing /proc/pid/stat") |
| introduced the ability to opt out of audit messages for accesses to various |
| proc files since they are not violations of policy. While doing so it |
| somehow switched the check from ns_capable() to |
| has_ns_capability{_noaudit}(). That means it switched from checking the |
| subjective credentials of the task to using the objective credentials. This |
| is wrong since. ptrace_has_cap() is currently only used in |
| ptrace_may_access() And is used to check whether the calling task (subject) |
| has the CAP_SYS_PTRACE capability in the provided user namespace to operate |
| on the target task (object). According to the cred.h comments this would |
| mean the subjective credentials of the calling task need to be used. |
| This switches ptrace_has_cap() to use security_capable(). Because we only |
| call ptrace_has_cap() in ptrace_may_access() and in there we already have a |
| stable reference to the calling task's creds under rcu_read_lock() there's |
| no need to go through another series of dereferences and rcu locking done |
| in ns_capable{_noaudit}(). |
| |
| As one example where this might be particularly problematic, Jann pointed |
| out that in combination with the upcoming IORING_OP_OPENAT feature, this |
| bug might allow unprivileged users to bypass the capability checks while |
| asynchronously opening files like /proc/*/mem, because the capability |
| checks for this would be performed against kernel credentials. |
| |
| To illustrate on the former point about this being exploitable: When |
| io_uring creates a new context it records the subjective credentials of the |
| caller. Later on, when it starts to do work it creates a kernel thread and |
| registers a callback. The callback runs with kernel creds for |
| ktask->real_cred and ktask->cred. To prevent this from becoming a |
| full-blown 0-day io_uring will call override_cred() and override |
| ktask->cred with the subjective credentials of the creator of the io_uring |
| instance. With ptrace_has_cap() currently looking at ktask->real_cred this |
| override will be ineffective and the caller will be able to open arbitray |
| proc files as mentioned above. |
| Luckily, this is currently not exploitable but will turn into a 0-day once |
| IORING_OP_OPENAT{2} land in v5.6. Fix it now! |
| |
| Cc: Oleg Nesterov <oleg@redhat.com> |
| Cc: Eric Paris <eparis@redhat.com> |
| Cc: stable@vger.kernel.org |
| Reviewed-by: Kees Cook <keescook@chromium.org> |
| Reviewed-by: Serge Hallyn <serge@hallyn.com> |
| Reviewed-by: Jann Horn <jannh@google.com> |
| Fixes: 69f594a38967 ("ptrace: do not audit capability check when outputing /proc/pid/stat") |
| Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/kernel/ptrace.c b/kernel/ptrace.c |
| index 705887f63288..f7db0123a06b 100644 |
| --- a/kernel/ptrace.c |
| +++ b/kernel/ptrace.c |
| @@ -259,12 +259,17 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state) |
| return ret; |
| } |
| |
| -static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode) |
| +static bool ptrace_has_cap(const struct cred *cred, struct user_namespace *ns, |
| + unsigned int mode) |
| { |
| + int ret; |
| + |
| if (mode & PTRACE_MODE_NOAUDIT) |
| - return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE); |
| + ret = security_capable(cred, ns, CAP_SYS_PTRACE, CAP_OPT_NOAUDIT); |
| else |
| - return has_ns_capability(current, ns, CAP_SYS_PTRACE); |
| + ret = security_capable(cred, ns, CAP_SYS_PTRACE, CAP_OPT_NONE); |
| + |
| + return ret == 0; |
| } |
| |
| /* Returns 0 on success, -errno on denial. */ |
| @@ -316,7 +321,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) |
| gid_eq(caller_gid, tcred->sgid) && |
| gid_eq(caller_gid, tcred->gid)) |
| goto ok; |
| - if (ptrace_has_cap(tcred->user_ns, mode)) |
| + if (ptrace_has_cap(cred, tcred->user_ns, mode)) |
| goto ok; |
| rcu_read_unlock(); |
| return -EPERM; |
| @@ -335,7 +340,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) |
| mm = task->mm; |
| if (mm && |
| ((get_dumpable(mm) != SUID_DUMP_USER) && |
| - !ptrace_has_cap(mm->user_ns, mode))) |
| + !ptrace_has_cap(cred, mm->user_ns, mode))) |
| return -EPERM; |
| |
| return security_ptrace_access_check(task, mode); |
| -- |
| 2.7.4 |
| |