Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
From: Jann Horn <jannh@google.com>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: "Al Viro" <viro@zeniv.linux.org.uk>,
	"Kees Cook" <keescook@chromium.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"kernel list" <linux-kernel@vger.kernel.org>,
	"Aleksa Sarai" <cyphar@cyphar.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	"Christian Heimes" <christian@python.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Deven Bowers" <deven.desai@linux.microsoft.com>,
	"Dmitry Vyukov" <dvyukov@google.com>,
	"Eric Biggers" <ebiggers@kernel.org>,
	"Eric Chiang" <ericchiang@google.com>,
	"Florian Weimer" <fweimer@redhat.com>,
	"James Morris" <jmorris@namei.org>, "Jan Kara" <jack@suse.cz>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Lakshmi Ramasubramanian" <nramas@linux.microsoft.com>,
	"Matthew Garrett" <mjg59@google.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Philippe Trébuchet" <philippe.trebuchet@ssi.gouv.fr>,
	"Scott Shell" <scottsh@microsoft.com>,
	"Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Steve Dower" <steve.dower@python.org>,
	"Steve Grubb" <sgrubb@redhat.com>,
	"Tetsuo Handa" <penguin-kernel@i-love.sakura.ne.jp>,
	"Thibaut Sautereau" <thibaut.sautereau@clip-os.org>,
	"Vincent Strubel" <vincent.strubel@ssi.gouv.fr>,
	"Kernel Hardening" <kernel-hardening@lists.openwall.com>,
	"Linux API" <linux-api@vger.kernel.org>,
	linux-integrity@vger.kernel.org,
	linux-security-module <linux-security-module@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v7 0/7] Add support for O_MAYEXEC
Date: Tue, 11 Aug 2020 01:03:03 +0200
Message-ID: <CAG48ez0NAV5gPgmbDaSjo=zzE=FgnYz=-OHuXwu0Vts=B5gesA@mail.gmail.com> (raw)
In-Reply-To: <917bb071-8b1a-3ba4-dc16-f8d7b4cc849f@digikod.net>

On Tue, Aug 11, 2020 at 12:43 AM Mickaël Salaün <mic@digikod.net> wrote:
> On 10/08/2020 22:21, Al Viro wrote:
> > On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
> >> It seems that there is no more complains nor questions. Do you want me
> >> to send another series to fix the order of the S-o-b in patch 7?
> >
> > There is a major question regarding the API design and the choice of
> > hooking that stuff on open().  And I have not heard anything resembling
> > a coherent answer.
>
> Hooking on open is a simple design that enables processes to check files
> they intend to open, before they open them. From an API point of view,
> this series extends openat2(2) with one simple flag: O_MAYEXEC. The
> enforcement is then subject to the system policy (e.g. mount points,
> file access rights, IMA, etc.).
>
> Checking on open enables to not open a file if it does not meet some
> requirements, the same way as if the path doesn't exist or (for whatever
> reasons, including execution permission) if access is denied.

You can do exactly the same thing if you do the check in a separate
syscall though.

And it provides a greater degree of flexibility; for example, you can
use it in combination with fopen() without having to modify the
internals of fopen() or having to use fdopen().

> It is a
> good practice to check as soon as possible such properties, and it may
> enables to avoid (user space) time-of-check to time-of-use (TOCTOU)
> attacks (i.e. misuse of already open resources).

The assumption that security checks should happen as early as possible
can actually cause security problems. For example, because seccomp was
designed to do its checks as early as possible, including before
ptrace, we had an issue for a long time where the ptrace API could be
abused to bypass seccomp filters.

Please don't decide that a check must be ordered first _just_ because
it is a security check. While that can be good for limiting attack
surface, it can also create issues when the idea is applied too
broadly.

I don't see how TOCTOU issues are relevant in any way here. If someone
can turn a script that is considered a trusted file into an untrusted
file and then maliciously change its contents, you're going to have
issues either way because the modifications could still happen after
openat(); if this was possible, the whole thing would kind of fall
apart. And if that isn't possible, I don't see any TOCTOU.

> It is important to keep
> in mind that the use cases we are addressing consider that the (user
> space) script interpreters (or linkers) are trusted and unaltered (i.e.
> integrity/authenticity checked). These are similar sought defensive
> properties as for SUID/SGID binaries: attackers can still launch them
> with malicious inputs (e.g. file paths, file descriptors, environment
> variables, etc.), but the binaries can then have a way to check if they
> can extend their trust to some file paths.
>
> Checking file descriptors may help in some use cases, but not the ones
> motivating this series.

It actually provides a superset of the functionality that your
existing patches provide.

> Checking (already) opened resources could be a
> *complementary* way to check execute permission, but it is not in the
> scope of this series.

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23 17:12 Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 1/7] exec: Change uselib(2) IS_SREG() failure to EACCES Mickaël Salaün
2020-08-11 18:59   ` Eric W. Biederman
2020-08-11 19:14     ` Eric W. Biederman
2020-07-23 17:12 ` [PATCH v7 2/7] exec: Move S_ISREG() check earlier Mickaël Salaün
2020-08-11 19:27   ` Eric W. Biederman
2020-07-23 17:12 ` [PATCH v7 3/7] exec: Move path_noexec() " Mickaël Salaün
2020-08-11 19:36   ` Eric W. Biederman
2020-08-13 15:31     ` Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 4/7] fs: Introduce O_MAYEXEC flag for openat2(2) Mickaël Salaün
2020-07-24 19:03   ` Kees Cook
2020-07-27  4:21   ` Al Viro
2020-07-27  5:27     ` Florian Weimer
2020-07-27 19:46       ` Mickaël Salaün
2020-08-11 19:51   ` Eric W. Biederman
2020-08-13 14:36     ` Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 5/7] fs,doc: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2020-07-24 19:03   ` Kees Cook
2020-08-11 19:58   ` Eric W. Biederman
2020-08-13 14:49     ` Mickaël Salaün
2020-07-23 17:12 ` [PATCH v7 6/7] selftest/openat2: Add tests for O_MAYEXEC enforcing Mickaël Salaün
2020-07-24 19:03   ` Kees Cook
2020-07-23 17:12 ` [PATCH v7 7/7] ima: add policy support for the new file open MAY_OPENEXEC flag Mickaël Salaün
2020-07-24 19:04   ` Kees Cook
2020-07-24 11:20 ` [PATCH v7 0/7] Add support for O_MAYEXEC Thibaut Sautereau
2020-07-24 19:06 ` Kees Cook
2020-07-25 11:15   ` Christian Brauner
2020-08-10 20:11   ` Mickaël Salaün
2020-08-10 20:21     ` Al Viro
2020-08-10 22:09       ` David Laight
2020-08-10 22:28         ` Al Viro
2020-08-10 22:47           ` Mickaël Salaün
2020-08-11  8:09             ` David Laight
2020-08-11  8:50               ` Mickaël Salaün
2020-08-10 22:43       ` Mickaël Salaün
2020-08-10 23:03         ` Jann Horn [this message]
2020-08-11  8:48           ` Mickaël Salaün
2020-08-11 13:56             ` Mimi Zohar
2020-08-11 14:02               ` Matthew Wilcox
2020-08-11 14:30                 ` Mimi Zohar
2020-08-11 17:18             ` Deven Bowers
2020-08-10 23:05         ` Al Viro
2020-08-11  8:49           ` Mickaël Salaün

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='CAG48ez0NAV5gPgmbDaSjo=zzE=FgnYz=-OHuXwu0Vts=B5gesA@mail.gmail.com' \
    --to=jannh@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=christian.brauner@ubuntu.com \
    --cc=christian@python.org \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=daniel@iogearbox.net \
    --cc=deven.desai@linux.microsoft.com \
    --cc=dvyukov@google.com \
    --cc=ebiggers@kernel.org \
    --cc=ericchiang@google.com \
    --cc=fweimer@redhat.com \
    --cc=jack@suse.cz \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mic@digikod.net \
    --cc=mjg59@google.com \
    --cc=mtk.manpages@gmail.com \
    --cc=nramas@linux.microsoft.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=philippe.trebuchet@ssi.gouv.fr \
    --cc=scottsh@microsoft.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=sgrubb@redhat.com \
    --cc=shuah@kernel.org \
    --cc=steve.dower@python.org \
    --cc=thibaut.sautereau@clip-os.org \
    --cc=vincent.strubel@ssi.gouv.fr \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=zohar@linux.ibm.com \
    /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

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

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

Example config snippet for mirrors

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


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