From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: [RFC PATCH] fs: call_usermodehelper_root helper introduced Date: Thu, 23 May 2013 03:00:33 -0700 Message-ID: <87k3mq9fsu.fsf@xmission.com> References: <20130522072840.27720.85023.stgit@localhost.localdomain> <878v36ex6n.fsf@xmission.com> <519DCE5D.6070204@parallels.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: , , , , , , , , , , , To: Stanislav Kinsbursky Return-path: In-Reply-To: <519DCE5D.6070204@parallels.com> (Stanislav Kinsbursky's message of "Thu, 23 May 2013 12:07:57 +0400") Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Stanislav Kinsbursky writes: > 22.05.2013 21:33, Eric W. Biederman =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> Stanislav Kinsbursky writes: >> >>> Usermode helper executes all binaries in global "init" root context= =2E This >>> doesn't allow to call a binary from other root context (for example= in a >>> container). >>> Currently, both containerized NFS client and NFS server requires an= ability to >>> execute a binary in a container's root context. Root swap can be do= ne in >>> "init" callback, passed by UMH caller. >>> But since we have 2 callers already (and more of them are expected = to appear >>> in future) and because set_fs_root() in not exported, it looks reas= onable to >>> add one more generic UMH helper to generic fs code. >>> Root path reference must be hold by the caller, since it will be pu= t on UMH >>> thread exit. >> >> Awesome. With this patch as an uprivilieged user I get to pick whic= h >> binary the kernel will execute. At least if nfs and nfsd ever runs = in a >> user namespace (something that looks like only matter of time). >> > > Not really. Only by using a kernel module to call the UMH. > And an unprivileged can't load a module as far a I know. > I.e. NFSd, for example, will use unprivileged user's root to perform = this call. To help me understand the context which instances of call user mode helper are you expecting to use this facility? >> I think this is a seriously bad idea. >> >> Why can't we do this in userspace with setns as we do with the core = dump >> helper? >> > > Could you, please, clarify, how setns can help here? setns can change the mount namespace, and chroot can change to root directory in the specified mount namespace. Essentially you can enter into a containers complete context (pid, mnt, root, etc) comming from the outside. =46or nfs and nfsd I believe everything is controlled by mounts. So th= e alternate solution is to have a kernel thread for calling your user mod= e helper processes that is created with plain old kernel_thread aka fork when we perform the controlling nfs mount, and have that thread be the parent process for any containerized user mode helper calls. >> I am missing a lot of context here and capturing the context of a >> process at time time we mount the filesystem and reconstituing it in >> call user mode helper seems like something we could do. >> >> This patch as it stands looks like it would compete for the honor of= the >> easiest kernel feature to exploit. >> > > Hmmm... > As far as I can see (maybe I'm missing something), there main securit= y issue that could be here > is allowing of using any passed root to swap to. Assuming that it is mounts that control this, and root in a user namespace will be able to mount nfs. Then root in a user namespace wil= l be able to cause any program to run as the global root by simply performing the appropriate set of mounts, and triggering the user mode helper. Instant gaining of all capabilities and privilieges on the system given to a binary of your choice. > What about using the current root instead of passed one? I.e. taking = the root to swap to inside the UMH. > Does this keeps the isolation on the same level? Passing the necessary information to the user mode helper so the user mode helper can perform the swaps should be fine. That essentially is what I was suggesting when I mentioned setns above. Eric