Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: "Mickaël Salaün" <mickael.salaun@ssi.gouv.fr>,
	"Florian Weimer" <fweimer@redhat.com>,
	"Mickaël Salaün" <mic@digikod.net>
Cc: linux-kernel@vger.kernel.org, "Aleksa Sarai" <cyphar@cyphar.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Christian Heimes" <christian@python.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Eric Chiang" <ericchiang@google.com>,
	"James Morris" <jmorris@namei.org>, "Jan Kara" <jack@suse.cz>,
	"Jann Horn" <jannh@google.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Kees Cook" <keescook@chromium.org>,
	"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>,
	"Song Liu" <songliubraving@fb.com>,
	"Steve Dower" <steve.dower@python.org>,
	"Steve Grubb" <sgrubb@redhat.com>,
	"Thibaut Sautereau" <thibaut.sautereau@ssi.gouv.fr>,
	"Vincent Strubel" <vincent.strubel@ssi.gouv.fr>,
	"Yves-Alexis Perez" <yves-alexis.perez@ssi.gouv.fr>,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 1/5] fs: Add support for an O_MAYEXEC flag on sys_open()
Date: Fri, 06 Sep 2019 14:38:11 -0400
Message-ID: <5a59b309f9d0603d8481a483e16b5d12ecb77540.camel@kernel.org> (raw)
In-Reply-To: <1fbf54f6-7597-3633-a76c-11c4b2481add@ssi.gouv.fr>

On Fri, 2019-09-06 at 19:14 +0200, Mickaël Salaün wrote:
> On 06/09/2019 18:48, Jeff Layton wrote:
> > On Fri, 2019-09-06 at 18:06 +0200, Mickaël Salaün wrote:
> > > On 06/09/2019 17:56, Florian Weimer wrote:
> > > > Let's assume I want to add support for this to the glibc dynamic loader,
> > > > while still being able to run on older kernels.
> > > > 
> > > > Is it safe to try the open call first, with O_MAYEXEC, and if that fails
> > > > with EINVAL, try again without O_MAYEXEC?
> > > 
> > > The kernel ignore unknown open(2) flags, so yes, it is safe even for
> > > older kernel to use O_MAYEXEC.
> > > 
> > 
> > Well...maybe. What about existing programs that are sending down bogus
> > open flags? Once you turn this on, they may break...or provide a way to
> > circumvent the protections this gives.
> 
> Well, I don't think we should nor could care about bogus programs that
> do not conform to the Linux ABI.
> 

But they do conform. The ABI is just undefined here. Unknown flags are
ignored so we never really know if $random_program may be setting them.

> > Maybe this should be a new flag that is only usable in the new openat2()
> > syscall that's still under discussion? That syscall will enforce that
> > all flags are recognized. You presumably wouldn't need the sysctl if you
> > went that route too.
> 
> Here is a thread about a new syscall:
> https://lore.kernel.org/lkml/1544699060.6703.11.camel@linux.ibm.com/
> 
> I don't think it fit well with auditing nor integrity. Moreover using
> the current open(2) behavior of ignoring unknown flags fit well with the
> usage of O_MAYEXEC (because it is only a hint to the kernel about the
> use of the *opened* file).
> 

The fact that open and openat didn't vet unknown flags is really a bug.

Too late to fix it now, of course, and as Aleksa points out, we've
worked around that in the past. Now though, we have a new openat2
syscall on the horizon. There's little need to continue these sorts of
hacks.

New open flags really have no place in the old syscalls, IMO.

> > Anyone that wants to use this will have to recompile anyway. If the
> > kernel doesn't support openat2 or if the flag is rejected then you know
> > that you have no O_MAYEXEC support and can decide what to do.
> 
> If we want to enforce a security policy, we need to either be the system
> administrator or the distro developer. If a distro ship interpreters
> using this flag, we don't need to recompile anything, but we need to be
> able to control the enforcement according to the mount point
> configuration (or an advanced MAC, or an IMA config). I don't see why an
> userspace process should check if this flag is supported or not, it
> should simply use it, and the sysadmin will enable an enforcement if it
> makes sense for the whole system.
> 

A userland program may need to do other risk mitigation if it sets
O_MAYEXEC and the kernel doesn't recognize it.

Personally, here's what I'd suggest:

- Base this on top of the openat2 set
- Change it that so that openat2() files are non-executable by default. Anyone wanting to do that needs to set O_MAYEXEC or upgrade the fd somehow.
- Only have the openat2 syscall pay attention to O_MAYEXEC. Let open and openat continue ignoring the new flag.

That works around a whole pile of potential ABI headaches. Note that
we'd need to make that decision before the openat2 patches are merged.

Even better would be to declare the new flag in some openat2-only flag
space, so there's no confusion about it being supported by legacy open
calls.

If glibc wants to implement an open -> openat2 wrapper in userland
later, it can set that flag in the wrapper implicitly to emulate the old
behavior.

Given that you're going to have to recompile software to take advantage
of this anyway, what's the benefit to changing legacy syscalls?

> > > > Or do I risk disabling this security feature if I do that?
> > > 
> > > It is only a security feature if the kernel support it, otherwise it is
> > > a no-op.
> > > 
> > 
> > With a security feature, I think we really want userland to aware of
> > whether it works.
> 
> If userland would like to enforce something, it can already do it
> without any kernel modification. The goal of the O_MAYEXEC flag is to
> enable the kernel, hence sysadmins or system designers, to enforce a
> global security policy that makes sense.
> 

I don't see how this helps anything if you can't tell whether the kernel
recognizes the damned thing. Also, our track record with global sysctl
switches like this is pretty poor. They're an administrative headache as
well as a potential attack vector.

I think your idea is a good one, but it could stand to have fewer moving
parts.
-- 
Jeff Layton <jlayton@kernel.org>


  reply index

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-06 15:24 [PATCH v2 0/5] Add support for O_MAYEXEC Mickaël Salaün
2019-09-06 15:24 ` [PATCH v2 1/5] fs: Add support for an O_MAYEXEC flag on sys_open() Mickaël Salaün
2019-09-06 15:56   ` Florian Weimer
2019-09-06 16:06     ` Mickaël Salaün
2019-09-06 16:48       ` Jeff Layton
2019-09-06 17:13         ` Aleksa Sarai
2019-09-06 19:43           ` Jeff Layton
2019-09-06 20:06             ` Andy Lutomirski
2019-09-06 20:51               ` Jeff Layton
2019-09-06 21:27                 ` Andy Lutomirski
2019-09-06 22:12                 ` Aleksa Sarai
2019-09-09  9:33               ` Mickaël Salaün
2019-09-06 22:05             ` Aleksa Sarai
2019-09-06 22:18               ` Aleksa Sarai
2019-09-06 17:14         ` Mickaël Salaün
2019-09-06 18:38           ` Jeff Layton [this message]
2019-09-06 18:41             ` Andy Lutomirski
2019-09-09  9:18               ` Mickaël Salaün
2019-09-09 15:49                 ` Andy Lutomirski
2019-09-06 18:44             ` Florian Weimer
2019-09-06 19:03             ` James Morris
2019-09-09  9:25               ` Mickaël Salaün
2019-09-09 10:12                 ` James Morris
2019-09-09 10:54                   ` Mickaël Salaün
2019-09-09 12:28                     ` Aleksa Sarai
2019-09-09 12:33                       ` Mickaël Salaün
2019-09-09 11:54                 ` Aleksa Sarai
2019-09-09 12:28                   ` Mickaël Salaün
2019-09-06 17:07       ` Aleksa Sarai
2019-09-06 17:20         ` Christian Brauner
2019-09-06 17:24           ` Mickaël Salaün
2019-09-06 17:40           ` Tycho Andersen
2019-09-06 18:27             ` Florian Weimer
2019-09-06 18:46               ` Tycho Andersen
2019-09-06 15:24 ` [PATCH v2 2/5] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount propertie Mickaël Salaün
2019-09-06 15:24 ` [PATCH v2 3/5] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2019-09-06 15:24 ` [PATCH v2 4/5] selftest/exec: Add tests for O_MAYEXEC enforcing Mickaël Salaün
2019-09-06 15:24 ` [PATCH v2 5/5] doc: Add documentation for the fs.open_mayexec_enforce sysctl Mickaël Salaün
2019-09-06 18:50 ` [PATCH v2 0/5] Add support for O_MAYEXEC Steve Grubb
2019-09-06 18:57   ` Florian Weimer
2019-09-06 19:07     ` Steve Grubb
2019-09-06 19:26       ` Andy Lutomirski
2019-09-06 22:44         ` Aleksa Sarai
2019-09-09  9:09           ` Mickaël Salaün
2019-09-09  0:16 ` James Morris

Reply instructions:

You may reply publically 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=5a59b309f9d0603d8481a483e16b5d12ecb77540.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=ast@kernel.org \
    --cc=christian@python.org \
    --cc=corbet@lwn.net \
    --cc=cyphar@cyphar.com \
    --cc=daniel@iogearbox.net \
    --cc=ericchiang@google.com \
    --cc=fweimer@redhat.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --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-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mic@digikod.net \
    --cc=mickael.salaun@ssi.gouv.fr \
    --cc=mjg59@google.com \
    --cc=mtk.manpages@gmail.com \
    --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=songliubraving@fb.com \
    --cc=steve.dower@python.org \
    --cc=thibaut.sautereau@ssi.gouv.fr \
    --cc=vincent.strubel@ssi.gouv.fr \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=yves-alexis.perez@ssi.gouv.fr \
    --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-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/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-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

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


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