linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: "Stéphane Graber" <stgraber@ubuntu.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"Aleksa Sarai" <cyphar@cyphar.com>,
	smbarber@chromium.org, "Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Alexey Dobriyan" <adobriyan@gmail.com>,
	"Serge Hallyn" <serge@hallyn.com>,
	"James Morris" <jmorris@namei.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"kernel list" <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	"Linux Containers" <containers@lists.linux-foundation.org>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	"Linux API" <linux-api@vger.kernel.org>
Subject: Re: [PATCH 00/24] user_namespace: introduce fsid mappings
Date: Tue, 11 Feb 2020 21:55:46 +0100	[thread overview]
Message-ID: <CAG48ez1GKOfXDZFD7-hGGjT8L9YEojn94DU5_=W8HL3pzdrCgg@mail.gmail.com> (raw)
In-Reply-To: <20200211165753.356508-1-christian.brauner@ubuntu.com>

On Tue, Feb 11, 2020 at 5:59 PM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
> This is the implementation of shiftfs which was cooked up during lunch at
> Linux Plumbers 2019 the day after the container's microconference. The
> idea is a design-stew from Stéphane, Aleksa, Eric, and myself. Back then
> we all were quite busy with other work and couldn't really sit down and
> implement it. But I took a few days last week to do this work, including
> demos and performance testing.
> This implementation does not require us to touch the vfs substantially
> at all. Instead, we implement shiftfs via fsid mappings.
> With this patch, it took me 20 mins to port both LXD and LXC to support
> shiftfs via fsid mappings.
>
> For anyone wanting to play with this the branch can be pulled from:
> https://github.com/brauner/linux/tree/fsid_mappings
> https://gitlab.com/brauner/linux/-/tree/fsid_mappings
> https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=fsid_mappings
>
> The main use case for shiftfs for us is in allowing shared writable
> storage to multiple containers using non-overlapping id mappings.
> In such a scenario you want the fsids to be valid and identical in both
> containers for the shared mount. A demo for this exists in [3].
> If you don't want to read on, go straight to the other demos below in
> [1] and [2].

I guess essentially this means that you want to have UID separation
between containers to prevent the containers - or their owners - from
interfering between each other, but for filesystem access, you don't
want to isolate them from each other using DAC controls on the files
and folders inside the containers' directory hierarchies, instead
relying on mode-0700 parent directories to restrict access to the
container owner? Or would you still have separate UIDs for e.g. the
container's UID range 0-65535, and then map the shared UID range at
100000, or something like that?

> People not as familiar with user namespaces might not be aware that fsid
> mappings already exist. Right now, fsid mappings are always identical to
> id mappings. Specifically, the kernel will lookup fsuids in the uid
> mappings and fsgids in the gid mappings of the relevant user namespace.

That's a bit like saying that a kernel without CONFIG_USER_NS still
has user ID mappings, they just happen to be identity mappings. :P

> With this patch series we simply introduce the ability to create fsid
> mappings that are different from the id mappings of a user namespace.
>
> In the usual case of running an unprivileged container we will have
> setup an id mapping, e.g. 0 100000 100000. The on-disk mapping will
> correspond to this id mapping, i.e. all files which we want to appear as
> 0:0 inside the user namespace will be chowned to 100000:100000 on the
> host. This works, because whenever the kernel needs to do a filesystem
> access it will lookup the corresponding uid and gid in the idmapping
> tables of the container.
> Now think about the case where we want to have an id mapping of 0 100000
> 100000 but an on-disk mapping of 0 300000 100000 which is needed to e.g.
> share a single on-disk mapping with multiple containers that all have
> different id mappings.
> This will be problematic. Whenever a filesystem access is requested, the
> kernel will now try to lookup a mapping for 300000 in the id mapping
> tables of the user namespace but since there is none the files will
> appear to be owned by the overflow id, i.e. usually 65534:65534 or
> nobody:nogroup.
>
> With fsid mappings we can solve this by writing an id mapping of 0
> 100000 100000 and an fsid mapping of 0 300000 100000. On filesystem
> access the kernel will now lookup the mapping for 300000 in the fsid
> mapping tables of the user namespace. And since such a mapping exists,
> the corresponding files will have correct ownership.

Sorry to bring up something as disgusting as setuid execution, but:
What happens when there's a setuid root file with ->i_uid==300000? I
guess the only way to make that work inside the containers would be
something like make_kuid(current_user_ns(),
from_kfsuid(current_user_ns(), inode->i_uid)) in the setuid execve
path?

> A note on proc (and sys), the proc filesystem is special in sofar as it
> only has a single superblock that is (currently but might be about to
> change) visible in all user namespaces (same goes for sys). This means
> it has special semantics in many ways, including how file ownership and
> access works. The fsid mapping implementation does not alter how proc
> (and sys) ownership works. proc and sys will both continue to lookup
> filesystem access in id mapping tables.

In your example, a process with namespaced UID set (0, 0, 0, 0) will
have kernel UIDs (100000, 100000, 100000, 300000), right? And then if
I want to open /proc/$pid/personality of another process with the same
UIDs, may_open() will call inode_permission() -> do_inode_permission()
-> generic_permission() -> acl_permission_check(), which will compare
current_fsuid() (which is 300000) against inode->i_uid. But
inode->i_uid was filled by proc_pid_make_inode()->task_dump_owner(),
which set inode->i_uid to 100000, right?

Also, e.g. __ptrace_may_access() uses cred->fsuid for a comparison
with another task's real/effective/saved UID.

[...]
> # Demos
> [1]: Create a container with different id and fsid mappings.
>      https://asciinema.org/a/300233
> [2]: Create a container with id mappings but without fsid mappings.
>      https://asciinema.org/a/300234
> [3]: Share storage between multiple containers with non-overlapping id
>      mappings.
>      https://asciinema.org/a/300235

(I really dislike this asciinema thing; if you want to quickly glance
through the output instead of reading at the same speed as it was
typed, a simple pastebin works much better unless you absolutely have
to show things that use stuff like ncurses UI.)

  parent reply	other threads:[~2020-02-11 20:56 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11 16:57 [PATCH 00/24] user_namespace: introduce fsid mappings Christian Brauner
2020-02-11 16:57 ` [PATCH 01/24] user_namespace: introduce fsid mappings infrastructure Christian Brauner
2020-02-11 17:26   ` Randy Dunlap
2020-02-11 16:57 ` [PATCH 02/24] proc: add /proc/<pid>/fsuid_map Christian Brauner
2020-02-11 16:57 ` [PATCH 03/24] proc: add /proc/<pid>/fsgid_map Christian Brauner
2020-02-11 16:57 ` [PATCH 04/24] fsuidgid: add fsid mapping helpers Christian Brauner
2020-02-11 16:57 ` [PATCH 05/24] proc: task_state(): use from_kfs{g,u}id_munged Christian Brauner
2020-02-11 16:57 ` [PATCH 06/24] fs: add is_userns_visible() helper Christian Brauner
2020-02-11 16:57 ` [PATCH 07/24] namei: may_{o_}create(): handle fsid mappings Christian Brauner
2020-02-11 16:57 ` [PATCH 08/24] inode: inode_owner_or_capable(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 09/24] capability: privileged_wrt_inode_uidgid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 10/24] stat: " Christian Brauner
2020-02-11 16:57 ` [PATCH 11/24] open: chown_common(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 12/24] posix_acl: " Christian Brauner
2020-02-11 16:57 ` [PATCH 13/24] attr: notify_change(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 14/24] commoncap: cap_task_fix_setuid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 15/24] commoncap:cap_bprm_set_creds(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 16/24] sys: __sys_setfsuid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 17/24] sys: __sys_setfsgid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 18/24] sys:__sys_setuid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 19/24] sys:__sys_setgid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 20/24] sys:__sys_setreuid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 21/24] sys:__sys_setregid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 22/24] sys:__sys_setresuid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 23/24] sys:__sys_setresgid(): " Christian Brauner
2020-02-11 16:57 ` [PATCH 24/24] devpts: " Christian Brauner
2020-02-11 20:55 ` Jann Horn [this message]
2020-02-12 14:51   ` [PATCH 00/24] user_namespace: introduce " Christian Brauner
2020-02-12 18:53     ` Jann Horn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAG48ez1GKOfXDZFD7-hGGjT8L9YEojn94DU5_=W8HL3pzdrCgg@mail.gmail.com' \
    --to=jannh@google.com \
    --cc=adobriyan@gmail.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=containers@lists.linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=ebiederm@xmission.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=smbarber@chromium.org \
    --cc=stgraber@ubuntu.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).