LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Christian Brauner <christian@brauner.io>
To: Jann Horn <jannh@google.com>
Cc: cyphar@cyphar.com, Al Viro <viro@zeniv.linux.org.uk>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Andy Lutomirski <luto@kernel.org>,
	jlayton@kernel.org, Bruce Fields <bfields@fieldses.org>,
	Arnd Bergmann <arnd@arndb.de>,
	shuah@kernel.org, David Howells <dhowells@redhat.com>,
	Tycho Andersen <tycho@tycho.ws>,
	kernel list <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org,
	linux-arch <linux-arch@vger.kernel.org>,
	linux-kselftest@vger.kernel.org, dev@opencontainers.org,
	containers@lists.linux-foundation.org
Subject: Re: [PATCH 1/3] namei: implement O_BENEATH-style AT_* flags
Date: Mon, 1 Oct 2018 15:00:39 +0200
Message-ID: <20181001130038.s5ztphs3pl2zt3ut@brauner.io> (raw)
In-Reply-To: <CAG48ez17EQuJQAZUg5hDFXhkjnnVFh39=aD+j0FBdsoTONSGEA@mail.gmail.com>

On Mon, Oct 01, 2018 at 02:28:03PM +0200, Jann Horn wrote:
> On Sat, Sep 29, 2018 at 4:28 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > Add the following flags for path resolution. The primary justification
> > for these flags is to allow for programs to be far more strict about how
> > they want path resolution to handle symlinks, mountpoint crossings, and
> > paths that escape the dirfd (through an absolute path or ".."
> > shenanigans).
> >
> > This is of particular concern to container runtimes that want to be very
> > careful about malicious root filesystems that a container's init might
> > have screwed around with (and there is no real way to protect against
> > this in userspace if you consider potential races against a malicious
> > container's init).
> >
> > * AT_BENEATH: Disallow ".." or absolute paths (either in the path or
> >   found during symlink resolution) to escape the starting point of name
> >   resolution, though ".." is permitted in cases like "foo/../bar".
> >   Relative symlinks are still allowed (as long as they don't escape the
> >   starting point).
> 
> As I said on the other thread, I would strongly prefer an API that
> behaves along the lines of David Drysdale's old patch
> https://lore.kernel.org/lkml/1439458366-8223-2-git-send-email-drysdale@google.com/
> : Forbid any use of "..". This would also be more straightforward to
> implement safely. If that doesn't work for you, I would like it if you
> could at least make that an option. I would like it if this API could
> mitigate straightforward directory traversal bugs such as
> https://bugs.chromium.org/p/project-zero/issues/detail?id=1583, where
> a confused deputy attempts to access a path like
> "/mnt/media_rw/../../data" while intending to access a directory under
> "/mnt/media_rw".

Oh, the semantics for this changed in this patchset, hah. I was still on
vacation so didn't get to look at it before it was sent out. From prior
discussion I remember that the original intention actual was what you
argue for. And the patchset should be as tight as possible. Having
special cases where ".." is allowed just sounds like an invitation for
userspace to get it wrong.
Aleksa, did you have a specific use-case in mind that made you change
this or was it already present in an earlier iteration of the patchset
by someone else?

> 
> > * AT_XDEV: Disallow mount-point crossing (both *down* into one, or *up*
> >   from one). The primary "scoping" use is to blocking resolution that
> >   crosses a bind-mount, which has a similar property to a symlink (in
> >   the way that it allows for escape from the starting-point). Since it
> >   is not possible to differentiate bind-mounts However since
> >   bind-mounting requires privileges (in ways symlinks don't) this has
> >   been split from LOOKUP_BENEATH. The naming is based on "find -xdev"
> >   (though find(1) doesn't walk upwards, the semantics seem obvious).
> >
> > * AT_NO_PROCLINK: Disallows ->get_link "symlink" jumping. This is a very
> >   specific restriction, and it exists because /proc/$pid/fd/...
> >   "symlinks" allow for access outside nd->root and pose risk to
> >   container runtimes that don't want to be tricked into accessing a host
> >   path (but do want to allow no-funny-business symlink resolution).
> 
> AT_BENEATH has to imply AT_NO_PROCLINK, right? Especially with the
> semantics you picked for AT_BENEATH. With the original O_BENEATH_ONLY
> semantics, it might be okay to not imply AT_NO_PROCLINK...
> 
> > * AT_NO_SYMLINK: Disallows symlink jumping *of any kind*. Implies
> >   AT_NO_PROCLINK (obviously).
> >
> > The AT_NO_*LINK flags return -ELOOP if path resolution would violates
> > their requirement, while the others all return -EXDEV. Currently these
> > are only enabled for the stat(2) family and the openat(2) family (the
> > latter has its own brand of O_* flags with the same semantics). Ideally
> > these flags would be supported by all *at(2) syscalls, but this will
> > require adding flags arguments to many of them (and will be done in a
> > separate patchset).

  reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-29 10:34 [PATCH 0/3] namei: implement various scoping " 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 [this message]
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
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=20181001130038.s5ztphs3pl2zt3ut@brauner.io \
    --to=christian@brauner.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=jannh@google.com \
    --cc=jlayton@kernel.org \
    --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=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