Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Aleksa Sarai <cyphar@cyphar.com>
To: Kees Cook <keescook@chromium.org>
Cc: "Florian Weimer" <fweimer@redhat.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Stephen Smalley" <stephen.smalley.work@gmail.com>,
	"Christian Heimes" <christian@python.org>,
	"Deven Bowers" <deven.desai@linux.microsoft.com>,
	"Tetsuo Handa" <penguin-kernel@I-love.SAKURA.ne.jp>,
	"John Johansen" <john.johansen@canonical.com>,
	"Kentaro Takeda" <takedakn@nttdata.co.jp>,
	"Lev R. Oshvang ." <levonshe@gmail.com>,
	"Alexei Starovoitov" <ast@kernel.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>,
	"Lakshmi Ramasubramanian" <nramas@linux.microsoft.com>,
	"Matthew Garrett" <mjg59@google.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Mickaël Salaün" <mickael.salaun@ssi.gouv.fr>,
	"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>,
	"Thibaut Sautereau" <thibaut.sautereau@ssi.gouv.fr>,
	"Vincent Strubel" <vincent.strubel@ssi.gouv.fr>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	"LSM List" <linux-security-module@vger.kernel.org>,
	"Linux FS Devel" <linux-fsdevel@vger.kernel.org>
Subject: Re: How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC)
Date: Tue, 19 May 2020 12:23:07 +1000
Message-ID: <20200519022307.oqpdb4vzghs3coyi@yavin.dot.cyphar.com> (raw)
In-Reply-To: <202005150847.2B1ED8F81@keescook>


[-- Attachment #1: Type: text/plain, Size: 3741 bytes --]

On 2020-05-15, Kees Cook <keescook@chromium.org> wrote:
> On Fri, May 15, 2020 at 04:43:37PM +0200, Florian Weimer wrote:
> > * Kees Cook:
> > 
> > > On Fri, May 15, 2020 at 10:43:34AM +0200, Florian Weimer wrote:
> > >> * Kees Cook:
> > >> 
> > >> > Maybe I've missed some earlier discussion that ruled this out, but I
> > >> > couldn't find it: let's just add O_EXEC and be done with it. It actually
> > >> > makes the execve() path more like openat2() and is much cleaner after
> > >> > a little refactoring. Here are the results, though I haven't emailed it
> > >> > yet since I still want to do some more testing:
> > >> > https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/o_exec/v1
> > >> 
> > >> I think POSIX specifies O_EXEC in such a way that it does not confer
> > >> read permissions.  This seems incompatible with what we are trying to
> > >> achieve here.
> > >
> > > I was trying to retain this behavior, since we already make this
> > > distinction between execve() and uselib() with the MAY_* flags:
> > >
> > > execve():
> > >         struct open_flags open_exec_flags = {
> > >                 .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> > >                 .acc_mode = MAY_EXEC,
> > >
> > > uselib():
> > >         static const struct open_flags uselib_flags = {
> > >                 .open_flag = O_LARGEFILE | O_RDONLY | __FMODE_EXEC,
> > >                 .acc_mode = MAY_READ | MAY_EXEC,
> > >
> > > I tried to retain this in my proposal, in the O_EXEC does not imply
> > > MAY_READ:
> > 
> > That doesn't quite parse for me, sorry.
> > 
> > The point is that the script interpreter actually needs to *read* those
> > files in order to execute them.
> 
> I think I misunderstood what you meant (Mickaël got me sorted out
> now). If O_EXEC is already meant to be "EXEC and _not_ READ nor WRITE",
> then yes, this new flag can't be O_EXEC. I was reading the glibc
> documentation (which treats it as a permission bit flag, not POSIX,
> which treats it as a complete mode description).

On the other hand, if we had O_EXEC (or O_EXONLY a-la O_RDONLY) then the
interpreter could re-open the file descriptor as O_RDONLY after O_EXEC
succeeds. Not ideal, but I don't think it's a deal-breaker.

Regarding O_MAYEXEC, I do feel a little conflicted.

I do understand that its goal is not to be what O_EXEC was supposed to
be (which is loosely what O_PATH has effectively become), so I think
that this is not really a huge problem -- especially since you could
just do O_MAYEXEC|O_PATH if you wanted to disallow reading explicitly.
It would be nice to have an O_EXONLY concept, but it's several decades
too late to make it mandatory (and making it optional has questionable
utility IMHO).

However, the thing I still feel mildly conflicted about is the sysctl. I
do understand the argument for it (ultimately, whether O_MAYEXEC is
usable on a system depends on the distribution) but it means that any
program which uses O_MAYEXEC cannot rely on it to provide the security
guarantees they expect. Even if the program goes and reads the sysctl
value, it could change underneath them. If this is just meant to be a
best-effort protection then this doesn't matter too much, but I just
feel uneasy about these kinds of best-effort protections.

I do wonder if we could require that fexecve(3) can only be done with
file descriptors that have been opened with O_MAYEXEC (obviously this
would also need to be a sysctl -- *sigh*). This would tie in to some of
the magic-link changes I wanted to push (namely, upgrade_mask).

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  parent reply index

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 15:31 [PATCH v5 0/6] Add support for O_MAYEXEC Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 1/6] fs: Add support for an O_MAYEXEC flag on openat2(2) Mickaël Salaün
2020-05-12 21:05   ` Kees Cook
2020-05-12 21:40     ` Christian Heimes
2020-05-12 22:56       ` Kees Cook
2020-05-13 10:13     ` Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 2/6] fs: Add a MAY_EXECMOUNT flag to infer the noexec mount property Mickaël Salaün
2020-05-12 21:09   ` Kees Cook
2020-05-14  8:14     ` Lev R. Oshvang .
2020-05-14 15:48       ` Kees Cook
2020-05-17 16:57         ` Lev R. Oshvang .
2020-05-05 15:31 ` [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2020-05-05 15:44   ` Randy Dunlap
2020-05-05 16:55     ` Mickaël Salaün
2020-05-05 17:40       ` Randy Dunlap
2020-05-12 21:48   ` Kees Cook
2020-05-13 11:09     ` Mickaël Salaün
2020-05-13 15:37   ` Stephen Smalley
2020-05-13 23:27     ` Kees Cook
2020-05-14  3:05       ` Kees Cook
2020-05-14 10:12         ` David Laight
2020-05-14 12:22         ` Stephen Smalley
2020-05-14 14:41           ` Kees Cook
2020-05-14 15:52             ` Stephen Smalley
2020-05-14 15:45           ` Kees Cook
2020-05-14 16:10             ` Stephen Smalley
2020-05-14 19:16               ` Mickaël Salaün
2020-05-15  0:58                 ` Tetsuo Handa
2020-05-15  8:01                 ` How about just O_EXEC? (was Re: [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC) Kees Cook
2020-05-15  8:43                   ` Florian Weimer
2020-05-15 14:37                     ` Kees Cook
2020-05-15 14:43                       ` Florian Weimer
2020-05-15 15:50                         ` Kees Cook
2020-05-18  7:26                           ` Florian Weimer
2020-05-19  2:23                           ` Aleksa Sarai [this message]
2020-05-19 10:13                             ` Mickaël Salaün
2020-05-15 11:04                   ` Mickaël Salaün
2020-05-15 15:46                     ` Kees Cook
2020-05-15 18:24                       ` Mickaël Salaün
2020-05-14 19:21       ` [PATCH v5 3/6] fs: Enable to enforce noexec mounts or file exec through O_MAYEXEC Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 4/6] selftest/openat2: Add tests for O_MAYEXEC enforcing Mickaël Salaün
2020-05-12 21:57   ` Kees Cook
2020-05-13 11:18     ` Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 5/6] doc: Add documentation for the fs.open_mayexec_enforce sysctl Mickaël Salaün
2020-05-12 22:00   ` Kees Cook
2020-05-13 11:20     ` Mickaël Salaün
2020-05-05 15:31 ` [PATCH v5 6/6] ima: add policy support for the new file open MAY_OPENEXEC flag Mickaël Salaün
2020-05-05 15:36 ` [PATCH v5 0/6] Add support for O_MAYEXEC Mickaël Salaün
2020-05-06 13:58   ` Lev R. Oshvang .
2020-05-06 15:41     ` Aleksa Sarai
2020-05-07  8:30     ` Mickaël Salaün
2020-05-07  8:05 ` David Laight
2020-05-07  8:36   ` Mickaël Salaün
2020-05-07  9:00     ` David Laight
2020-05-07  9:30       ` Mickaël Salaün
2020-05-07  9:44         ` David Laight
2020-05-07 13:38           ` Mickaël Salaün
2020-05-08  7:15             ` Lev R. Oshvang .
2020-05-08 14:01               ` Mimi Zohar

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=20200519022307.oqpdb4vzghs3coyi@yavin.dot.cyphar.com \
    --to=cyphar@cyphar.com \
    --cc=ast@kernel.org \
    --cc=christian@python.org \
    --cc=corbet@lwn.net \
    --cc=daniel@iogearbox.net \
    --cc=deven.desai@linux.microsoft.com \
    --cc=ericchiang@google.com \
    --cc=fweimer@redhat.com \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=levonshe@gmail.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=mickael.salaun@ssi.gouv.fr \
    --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=stephen.smalley.work@gmail.com \
    --cc=steve.dower@python.org \
    --cc=takedakn@nttdata.co.jp \
    --cc=thibaut.sautereau@ssi.gouv.fr \
    --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-Security-Module Archive on lore.kernel.org

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

Example config snippet for mirrors

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


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