Date: Tue, 11 Aug 2020 10:48:37 +0200 From: Mickaël Salaün <mic@...ikod.net> To: Jann Horn <jannh@...gle.com>, Kees Cook <keescook@...omium.org>, Deven Bowers <deven.desai@...ux.microsoft.com>, Mimi Zohar <zohar@...ux.ibm.com> Cc: Al Viro <viro@...iv.linux.org.uk>, Andrew Morton <akpm@...ux-foundation.org>, kernel list <linux-kernel@...r.kernel.org>, Aleksa Sarai <cyphar@...har.com>, Alexei Starovoitov <ast@...nel.org>, Andy Lutomirski <luto@...nel.org>, Christian Brauner <christian.brauner@...ntu.com>, Christian Heimes <christian@...hon.org>, Daniel Borkmann <daniel@...earbox.net>, Dmitry Vyukov <dvyukov@...gle.com>, Eric Biggers <ebiggers@...nel.org>, Eric Chiang <ericchiang@...gle.com>, Florian Weimer <fweimer@...hat.com>, James Morris <jmorris@...ei.org>, Jan Kara <jack@...e.cz>, Jonathan Corbet <corbet@....net>, Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>, Matthew Garrett <mjg59@...gle.com>, Matthew Wilcox <willy@...radead.org>, Michael Kerrisk <mtk.manpages@...il.com>, Philippe Trébuchet <philippe.trebuchet@....gouv.fr>, Scott Shell <scottsh@...rosoft.com>, Sean Christopherson <sean.j.christopherson@...el.com>, Shuah Khan <shuah@...nel.org>, Steve Dower <steve.dower@...hon.org>, Steve Grubb <sgrubb@...hat.com>, Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>, Thibaut Sautereau <thibaut.sautereau@...p-os.org>, Vincent Strubel <vincent.strubel@....gouv.fr>, Kernel Hardening <kernel-hardening@...ts.openwall.com>, Linux API <linux-api@...r.kernel.org>, linux-integrity@...r.kernel.org, linux-security-module <linux-security-module@...r.kernel.org>, linux-fsdevel <linux-fsdevel@...r.kernel.org> Subject: Re: [PATCH v7 0/7] Add support for O_MAYEXEC On 11/08/2020 01:03, Jann Horn wrote: > On Tue, Aug 11, 2020 at 12:43 AM Mickaël Salaün <mic@...ikod.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'd be interested with such security issue examples. I hope that delaying checks will not be an issue for mechanisms such as IMA or IPE: https://firstname.lastname@example.org/ Any though Mimi, Deven, Chrome OS folks? > > 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. Sure, and if the scripts are not protected in some way there is no point to check anything. > >> 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. It also brings new issues with multiple file descriptor origins (e.g. memfd_create). > >> Checking (already) opened resources could be a >> *complementary* way to check execute permission, but it is not in the >> scope of this series.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.