linux-kernel - [REGRESSION v4.16-rc6] [PATCH] mqueue: forbid unprivileged user access to internal mount

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180323060457.sxgsd3j2obi33fyw@gordon>
Date:   Fri, 23 Mar 2018 17:04:57 +1100
From:   Aleksa Sarai <asarai@...e.de>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        Al Viro <viro@...iv.linux.org.uk>
Cc:     linux-kernel@...r.kernel.org, containers@...ts.linux-foundation.org
Subject: [REGRESSION v4.16-rc6] [PATCH] mqueue: forbid unprivileged user
 access to internal mount

Hi all,

Felix reported weird behaviour on 4.16.0-rc6 with regards to mqueue[1],
which was introduced by 36735a6a2b5e ("mqueue: switch to on-demand
creation of internal mount").

Basically, the reproducer boils down to being able to mount mqueue if
you create a new user namespace, even if you don't unshare the IPC
namespace.

Previously this was not possible, and you would get an -EPERM. The mount
is the *host* mqueue mount, which is being cached and just returned from
mqueue_mount(). To be honest, I'm not sure if this is safe or not (or if
it was intentional -- since I'm not familiar with mqueue).

To me it looks like there is a missing permission check. I've included a
patch below that I've compile-tested, and should block the above case.
Can someone please tell me if I'm missing something? Is this actually
safe?

[1]: https://github.com/docker/docker/issues/36674

--8<--------------------------------------------------------------------

Fix a regression caused by 36735a6a2b5e ("mqueue: switch to on-demand
creation of internal mount"), where an unprivileged user is permitted to
mount mqueue even if they don't have CAP_SYS_ADMIN in the ipcns's
associated userns. This can be reproduced as in the following.

  % unshare -Urm                       # ipc isn't unshare'd
  # mount -t mqueue mqueue /dev/mqueue # should have failed
  # echo $?
  0

Previously the above would error out with an -EPERM, as the mount was
protected by mount_ns(), but the patch in question switched to
kern_mount_data() which doesn't do this necessary permission check. So
add it explicitly to mq_internal_mount().

Fixes: 36735a6a2b5e ("mqueue: switch to on-demand creation of internal mount")
Reported-by: Felix Abecassis <fabecassis@...dia.com>
Signed-off-by: Aleksa Sarai <asarai@...e.de>
---
 ipc/mqueue.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index d7f309f74dec..ddb85091398d 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -353,6 +353,12 @@ static struct vfsmount *mq_internal_mount(void)
 {
 	struct ipc_namespace *ns = current->nsproxy->ipc_ns;
 	struct vfsmount *m = ns->mq_mnt;
+	/*
+	 * Match the semantics of mount_ns, to avoid unprivileged users from being
+	 * able to mount mqueue from an IPC namespace they don't have ownership of.
+	 */
+	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN))
+		return ERR_PTR(-EPERM);
 	if (m)
 		return m;
 	m = kern_mount_data(&mqueue_fs_type, ns);
-- 
2.16.2

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)