LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Christian Brauner <christian@brauner.io>
To: Alban Crequy <alban@kinvolk.io>, cyphar@cyphar.com
Cc: jlayton@kernel.org, bfields@fieldses.org,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	arnd@arndb.de, shuah@kernel.org, dhowells@redhat.com,
	luto@kernel.org, "Eric W . Biederman" <ebiederm@xmission.com>,
	tycho@tycho.ws, LKML <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-arch@vger.kernel.org, linux-kselftest@vger.kernel.org,
	dev <dev@opencontainers.org>,
	Linux Containers <containers@lists.linux-foundation.org>,
	jsafrane@redhat.com, msau@google.com
Subject: Re: [PATCH 0/3] namei: implement various scoping AT_* flags
Date: Sun, 30 Sep 2018 16:02:10 +0200
Message-ID: <58BB23FF-E652-4C58-AEE4-4B5376D03BF0@brauner.io> (raw)
In-Reply-To: <CADZs7q5mFGMvtbAHzjKdC8b7byz8pvBOaLBOTxnXRwTZ1LfCAw@mail.gmail.com>

On September 30, 2018 3:54:31 PM GMT+02:00, Alban Crequy <alban@kinvolk.io> wrote:
>On Sat, Sep 29, 2018 at 12:35 PM Aleksa Sarai <cyphar@cyphar.com>
>wrote:
>>
>> The need for some sort of control over VFS's path resolution (to
>avoid
>> malicious paths resulting in inadvertent breakouts) has been a very
>> long-standing desire of many userspace applications. This patchset is
>a
>> revival of Al Viro's old AT_NO_JUMPS[1] patchset with a few
>additions.
>>
>> The most obvious change is that AT_NO_JUMPS has been split as
>dicussed
>> in the original thread, along with a further split of AT_NO_PROCLINKS
>> which means that each individual property of AT_NO_JUMPS is now a
>> separate flag:
>>
>>   * Path-based escapes from the starting-point using "/" or ".." are
>>     blocked by AT_BENEATH.
>>   * Mountpoint crossings are blocked by AT_XDEV.
>>   * /proc/$pid/fd/$fd resolution is blocked by AT_NO_PROCLINKS (more
>>         correctly it actually blocks any user of nd_jump_link()
>because it
>>         allows out-of-VFS path resolution manipulation).
>>
>> AT_NO_JUMPS is now effectively (AT_BENEATH|AT_XDEV|AT_NO_PROCLINKS).
>At
>> Linus' suggestion in the original thread, I've also implemented
>> AT_NO_SYMLINKS which just denies _all_ symlink resolution (including
>> "proclink" resolution).
>
>It seems quite useful to me.
>
>> An additional improvement was made to AT_XDEV. The original
>AT_NO_JUMPS
>> path didn't consider "/tmp/.." as a mountpoint crossing -- this patch
>> blocks this as well (feel free to ask me to remove it if you feel
>this
>> is not sane).
>>
>> Currently I've only enabled these for openat(2) and the stat(2)
>family.
>> I would hope we could enable it for basically every *at(2) syscall --
>> but many of them appear to not have a @flags argument and thus we'll
>> need to add several new syscalls to do this. I'm more than happy to
>send
>> those patches, but I'd prefer to know that this preliminary work is
>> acceptable before doing a bunch of copy-paste to add new sets of
>*at(2)
>> syscalls.
>
>What do you think of an equivalent feature AT_NO_SYMLINKS flag for
>mount()?

That's something we discussed  but that would need to be part of the new mount API work by David. The current mount API doesn't take AT_* flags since it doesn't operate on fds and we're (sort of) out of mount flags.

>
>I guess that would have made the fix for CVE-2017-1002101 in
>Kubernetes easier to write:
>https://kubernetes.io/blog/2018/04/04/fixing-subpath-volume-vulnerability/
>
>> One additional feature I've implemented is AT_THIS_ROOT (I imagine
>this
>> is probably going to be more contentious than the refresh of
>> AT_NO_JUMPS, so I've included it in a separate patch). The patch
>itself
>> describes my reasoning, but the shortened version of the premise is
>that
>> continer runtimes need to have a way to resolve paths within a
>> potentially malicious rootfs. Container runtimes currently do this in
>> userspace[2] which has implicit race conditions that are not
>resolvable
>> in userspace (or use fork+exec+chroot and SCM_RIGHTS passing which is
>> inefficient). AT_THIS_ROOT allows for per-call chroot-like semantics
>for
>> path resolution, which would be invaluable for us -- and the
>> implementation is basically identical to AT_BENEATH (except that we
>> don't return errors when someone actually hits the root).
>>
>> I've added some selftests for this, but it's not clear to me whether
>> they should live here or in xfstests (as far as I can tell there are
>no
>> other VFS tests in selftests, while there are some tests that look
>like
>> generic VFS tests in xfstests). If you'd prefer them to be included
>in
>> xfstests, let me know.
>>
>> [1]: https://lore.kernel.org/patchwork/patch/784221/
>> [2]: https://github.com/cyphar/filepath-securejoin
>>
>> Aleksa Sarai (3):
>>   namei: implement O_BENEATH-style AT_* flags
>>   namei: implement AT_THIS_ROOT chroot-like path resolution
>>   selftests: vfs: add AT_* path resolution tests
>>
>>  fs/fcntl.c                                    |   2 +-
>>  fs/namei.c                                    | 158
>++++++++++++------
>>  fs/open.c                                     |  10 ++
>>  fs/stat.c                                     |  15 +-
>>  include/linux/fcntl.h                         |   3 +-
>>  include/linux/namei.h                         |   8 +
>>  include/uapi/asm-generic/fcntl.h              |  20 +++
>>  include/uapi/linux/fcntl.h                    |  10 ++
>>  tools/testing/selftests/Makefile              |   1 +
>>  tools/testing/selftests/vfs/.gitignore        |   1 +
>>  tools/testing/selftests/vfs/Makefile          |  13 ++
>>  tools/testing/selftests/vfs/at_flags.h        |  40 +++++
>>  tools/testing/selftests/vfs/common.sh         |  37 ++++
>>  .../selftests/vfs/tests/0001_at_beneath.sh    |  72 ++++++++
>>  .../selftests/vfs/tests/0002_at_xdev.sh       |  54 ++++++
>>  .../vfs/tests/0003_at_no_proclinks.sh         |  50 ++++++
>>  .../vfs/tests/0004_at_no_symlinks.sh          |  49 ++++++
>>  .../selftests/vfs/tests/0005_at_this_root.sh  |  66 ++++++++
>>  tools/testing/selftests/vfs/vfs_helper.c      | 154
>+++++++++++++++++
>>  19 files changed, 707 insertions(+), 56 deletions(-)
>>  create mode 100644 tools/testing/selftests/vfs/.gitignore
>>  create mode 100644 tools/testing/selftests/vfs/Makefile
>>  create mode 100644 tools/testing/selftests/vfs/at_flags.h
>>  create mode 100644 tools/testing/selftests/vfs/common.sh
>>  create mode 100755
>tools/testing/selftests/vfs/tests/0001_at_beneath.sh
>>  create mode 100755 tools/testing/selftests/vfs/tests/0002_at_xdev.sh
>>  create mode 100755
>tools/testing/selftests/vfs/tests/0003_at_no_proclinks.sh
>>  create mode 100755
>tools/testing/selftests/vfs/tests/0004_at_no_symlinks.sh
>>  create mode 100755
>tools/testing/selftests/vfs/tests/0005_at_this_root.sh
>>  create mode 100644 tools/testing/selftests/vfs/vfs_helper.c
>>
>> --
>> 2.19.0


  reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-29 10:34 Aleksa Sarai
2018-09-29 10:34 ` [PATCH 1/3] namei: implement O_BENEATH-style " Aleksa Sarai
2018-09-29 14:48   ` Christian Brauner
2018-09-29 15:34     ` Aleksa Sarai
2018-09-30  4:38   ` Aleksa Sarai
2018-10-01 12:28   ` Jann Horn
2018-10-01 13:00     ` Christian Brauner
2018-10-01 16:04       ` Aleksa Sarai
2018-10-04 17:20         ` Christian Brauner
2018-09-29 13:15 ` [PATCH 2/3] namei: implement AT_THIS_ROOT chroot-like path resolution Aleksa Sarai
2018-09-29 13:15   ` [PATCH 3/3] selftests: vfs: add AT_* path resolution tests Aleksa Sarai
2018-09-29 16:35   ` [PATCH 2/3] namei: implement AT_THIS_ROOT chroot-like path resolution Jann Horn
2018-09-29 17:25     ` Andy Lutomirski
2018-10-01  9:46       ` Aleksa Sarai
2018-10-01  5:44     ` Aleksa Sarai
2018-10-01 10:13       ` Jann Horn
2018-10-01 16:18         ` Aleksa Sarai
2018-10-04 17:27           ` Christian Brauner
2018-10-01 10:42       ` Christian Brauner
2018-10-01 11:29         ` Jann Horn
2018-10-01 12:35           ` Christian Brauner
2018-10-01 13:55       ` Bruce Fields
2018-10-01 14:28       ` Andy Lutomirski
2018-10-02  7:32         ` Aleksa Sarai
2018-10-03 22:09           ` Andy Lutomirski
2018-10-06 20:56           ` Florian Weimer
2018-10-06 21:49             ` Christian Brauner
2018-10-01 14:00     ` Christian Brauner
2018-10-04 16:26     ` Aleksa Sarai
2018-10-04 17:31       ` Christian Brauner
2018-10-04 18:26       ` Jann Horn
2018-10-05 15:07         ` Aleksa Sarai
2018-10-05 15:55           ` Jann Horn
2018-10-06  2:10             ` Aleksa Sarai
2018-10-08 10:50               ` Jann Horn
2018-09-29 14:25 ` [PATCH 0/3] namei: implement various scoping AT_* flags Andy Lutomirski
2018-09-29 15:45   ` Aleksa Sarai
2018-09-29 16:34     ` Andy Lutomirski
2018-09-29 19:44       ` Matthew Wilcox
2018-09-29 14:38 ` Christian Brauner
2018-09-30  4:44   ` Aleksa Sarai
2018-09-30 13:54 ` Alban Crequy
2018-09-30 14:02   ` Christian Brauner [this message]
2018-09-30 19:45 ` Mickaël Salaün
2018-09-30 21:46   ` Jann Horn
2018-09-30 22:37     ` Mickaël Salaün
2018-10-01 20:14       ` James Morris
2018-10-01  4:08 ` Dave Chinner
2018-10-01  5:47   ` Aleksa Sarai
2018-10-01  6:14     ` Dave Chinner
2018-10-01 13:28 ` David Laight
2018-10-01 16:15   ` Aleksa Sarai
2018-10-03 13:21     ` David Laight

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=58BB23FF-E652-4C58-AEE4-4B5376D03BF0@brauner.io \
    --to=christian@brauner.io \
    --cc=alban@kinvolk.io \
    --cc=arnd@arndb.de \
    --cc=bfields@fieldses.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=cyphar@cyphar.com \
    --cc=dev@opencontainers.org \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=jlayton@kernel.org \
    --cc=jsafrane@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=msau@google.com \
    --cc=shuah@kernel.org \
    --cc=tycho@tycho.ws \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git