Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: "Mickaël Salaün" <mickael.salaun@ssi.gouv.fr>
To: Aleksa Sarai <cyphar@cyphar.com>
Cc: "James Morris" <jmorris@namei.org>,
	"Jeff Layton" <jlayton@kernel.org>,
	"Florian Weimer" <fweimer@redhat.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	linux-kernel@vger.kernel.org,
	"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>, "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: Mon, 9 Sep 2019 14:28:57 +0200
Message-ID: <fa592f31-3fbf-d0da-d31a-d14a89488219@ssi.gouv.fr> (raw)
In-Reply-To: <20190909115437.jwpyslcdhhvzo7g5@yavin>


On 09/09/2019 13:54, Aleksa Sarai wrote:
> On 2019-09-09, Mickaël Salaün <mickael.salaun@ssi.gouv.fr> wrote:
>> On 06/09/2019 21:03, James Morris wrote:
>>> On Fri, 6 Sep 2019, Jeff Layton wrote:
>>>
>>>> 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.
>>>
>>> Agree here. It's unfortunate but a reality and Linus will reject any such
>>> changes which break existing userspace.
>>
>> Do you mean that adding new flags to open(2) is not possible?
>
> It is possible, as long as there is no case where a program that works
> today (and passes garbage to the unused bits in flags) works with the
> change.
>
> O_TMPFILE was okay because it's actually two flags (one is O_DIRECTORY)
> and no working program does file IO to a directory (there are also some
> other tricky things done there, I'll admit I don't fully understand it).
>
> O_EMPTYPATH works because it's a no-op with non-empty path strings, and
> empty path strings have always given an error (so no working program
> does it today).
>
> However, O_MAYEXEC will result in programs that pass garbage bits to
> potentially get -EACCES that worked previously.
>
>> As I said, O_MAYEXEC should be ignored if it is not supported by the
>> kernel, which perfectly fit with the current open(2) flags behavior, and
>> should also behave the same with openat2(2).
>
> NACK on having that behaviour with openat2(2). -EINVAL on unknown flags
> is how all other syscalls work (any new syscall proposed today that
> didn't do that would be rightly rejected), and is a quirk of open(2)
> which unfortunately cannot be fixed. The fact that *every new O_ flag
> needs to work around this problem* should be an indication that this
> interface mis-design should not be allowed to infect any more syscalls.

It's definitely OK (and a sane interface) to always return -EINVAL for
unknown flags with openat2(2) (and other new syscalls). With openat2(2),
userland need to handle the case where some flags may be unknown to the
kernel (and handling the fact that this syscall may be unknown too). So
there is not an issue with openat2(2).

However, *userland* should not try to infer possible security
restrictions from the O_MAYEXEC flag (then, my use of "ignore" above),
which may return -EACCES or not, according to the current running system
security policy.

Following this reasoning, the current behavior or open(2) is fine for
O_MAYEXEC. The openat2(2) strict flag handling (i.e. -EINVAL) is fine
too for O_MAYEXEC.

>
> Note that this point is regardless of the fact that O_MAYEXEC is a
> *security* flag -- if userspace wants to have a secure fallback on
> old kernels (which is "the right thing" to do) they would have to do
> more work than necessary. And programs that don't care don't have to do
> anything special.

Most of the time this reasoning is good for most security stuff.
However, the O_MAYEXEC flag is not a security feature on its own, it is
an indication to the kernel to how this file would be used by userland.
The *kernel* security policy may tell back to userland if the system
security policy allow it or not. Most of the time, Policy Decision
Points (PDP) and Policy Enforcement Points (PEP) are in the same
software component (e.g. the kernel). Here the kernel is the PDP and
userland interpreters are PDP. Obviously, it means that these
interpreters must be (sub)part of your TCB (thanks to other security
features).

>
> However with -EINVAL, the programs doing "the right thing" get an easy
> -EINVAL check. And programs that don't care can just un-set O_MAYEXEC
> and retry. You should be forced to deal with the case where a flag is
> not supported -- and this is doubly true of security flags!

I'm in favor of doing this for openat2(2) with O_MAYEXEC, but it is not
because of the "security purposes" of this flag, as I said above, it is
because it is a saner ABI that every syscall should follow. But again,
it doesn't change my point about open(2). :)


--
Mickaël Salaün

Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.

  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
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 [this message]
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=fa592f31-3fbf-d0da-d31a-d14a89488219@ssi.gouv.fr \
    --to=mickael.salaun@ssi.gouv.fr \
    --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=jlayton@kernel.org \
    --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=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