Message-ID: <20240723.quohpho8Ie6i@digikod.net>
Date: Tue, 23 Jul 2024 15:15:49 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Jeff Xu <jeffxu@...gle.com>
Cc: Al Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Kees Cook <keescook@...omium.org>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, Paul Moore <paul@...l-moore.com>, Theodore Ts'o <tytso@....edu>, 
	Alejandro Colomar <alx@...nel.org>, Aleksa Sarai <cyphar@...har.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Andy Lutomirski <luto@...nel.org>, Arnd Bergmann <arnd@...db.de>, 
	Casey Schaufler <casey@...aufler-ca.com>, Christian Heimes <christian@...hon.org>, 
	Dmitry Vyukov <dvyukov@...gle.com>, Eric Biggers <ebiggers@...nel.org>, 
	Eric Chiang <ericchiang@...gle.com>, Fan Wu <wufan@...ux.microsoft.com>, 
	Florian Weimer <fweimer@...hat.com>, Geert Uytterhoeven <geert@...ux-m68k.org>, 
	James Morris <jamorris@...ux.microsoft.com>, Jan Kara <jack@...e.cz>, Jann Horn <jannh@...gle.com>, 
	Jonathan Corbet <corbet@....net>, Jordan R Abrahams <ajordanr@...gle.com>, 
	Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>, Luca Boccassi <bluca@...ian.org>, 
	Luis Chamberlain <mcgrof@...nel.org>, "Madhavan T . Venkataraman" <madvenka@...ux.microsoft.com>, 
	Matt Bobrowski <mattbobrowski@...gle.com>, Matthew Garrett <mjg59@...f.ucam.org>, 
	Matthew Wilcox <willy@...radead.org>, Miklos Szeredi <mszeredi@...hat.com>, 
	Mimi Zohar <zohar@...ux.ibm.com>, Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>, 
	Scott Shell <scottsh@...rosoft.com>, Shuah Khan <shuah@...nel.org>, 
	Stephen Rothwell <sfr@...b.auug.org.au>, Steve Dower <steve.dower@...hon.org>, 
	Steve Grubb <sgrubb@...hat.com>, Thibaut Sautereau <thibaut.sautereau@....gouv.fr>, 
	Vincent Strubel <vincent.strubel@....gouv.fr>, Xiaoming Ni <nixiaoming@...wei.com>, 
	Yin Fengwei <fengwei.yin@...el.com>, kernel-hardening@...ts.openwall.com, linux-api@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-security-module@...r.kernel.org, Elliott Hughes <enh@...gle.com>
Subject: Re: [RFC PATCH v19 1/5] exec: Add a new AT_CHECK flag to execveat(2)

On Fri, Jul 19, 2024 at 10:36:01AM -0700, Jeff Xu wrote:
> On Fri, Jul 19, 2024 at 8:31 AM Mickaël Salaün <mic@...ikod.net> wrote:
> >
> > On Fri, Jul 19, 2024 at 08:12:37AM -0700, Jeff Xu wrote:
> > > On Thu, Jul 18, 2024 at 5:24 AM Mickaël Salaün <mic@...ikod.net> wrote:
> > > >
> > > > On Wed, Jul 17, 2024 at 07:08:17PM -0700, Jeff Xu wrote:
> > > > > On Wed, Jul 17, 2024 at 3:01 AM Mickaël Salaün <mic@...ikod.net> wrote:
> > > > > >
> > > > > > On Tue, Jul 16, 2024 at 11:33:55PM -0700, Jeff Xu wrote:
> > > > > > > On Thu, Jul 4, 2024 at 12:02 PM Mickaël Salaün <mic@...ikod.net> wrote:
> > > > > > > >
> > > > > > > > Add a new AT_CHECK flag to execveat(2) to check if a file would be
> > > > > > > > allowed for execution.  The main use case is for script interpreters and
> > > > > > > > dynamic linkers to check execution permission according to the kernel's
> > > > > > > > security policy. Another use case is to add context to access logs e.g.,
> > > > > > > > which script (instead of interpreter) accessed a file.  As any
> > > > > > > > executable code, scripts could also use this check [1].
> > > > > > > >
> > > > > > > > This is different than faccessat(2) which only checks file access
> > > > > > > > rights, but not the full context e.g. mount point's noexec, stack limit,
> > > > > > > > and all potential LSM extra checks (e.g. argv, envp, credentials).
> > > > > > > > Since the use of AT_CHECK follows the exact kernel semantic as for a
> > > > > > > > real execution, user space gets the same error codes.
> > > > > > > >
> > > > > > > So we concluded that execveat(AT_CHECK) will be used to check the
> > > > > > > exec, shared object, script and config file (such as seccomp config),
> > > > > >
> > > > > > "config file" that contains executable code.
> > > > > >
> > > > > Is seccomp config  considered as "contains executable code", seccomp
> > > > > config is translated into bpf, so maybe yes ? but bpf is running in
> > > > > the kernel.
> > > >
> > > > Because seccomp filters alter syscalls, they are similar to code
> > > > injection.
> > > >
> > > > >
> > > > > > > I'm still thinking  execveat(AT_CHECK) vs faccessat(AT_CHECK) in
> > > > > > > different use cases:
> > > > > > >
> > > > > > > execveat clearly has less code change, but that also means: we can't
> > > > > > > add logic specific to exec (i.e. logic that can't be applied to
> > > > > > > config) for this part (from do_execveat_common to
> > > > > > > security_bprm_creds_for_exec) in future.  This would require some
> > > > > > > agreement/sign-off, I'm not sure from whom.
> > > > > >
> > > > > > I'm not sure to follow. We could still add new flags, but for now I
> > > > > > don't see use cases.  This patch series is not meant to handle all
> > > > > > possible "trust checks", only executable code, which makes sense for the
> > > > > > kernel.
> > > > > >
> > > > > I guess the "configfile" discussion is where I get confused, at one
> > > > > point, I think this would become a generic "trust checks" api for
> > > > > everything related to "generating executable code", e.g. javascript,
> > > > > java code, and more.
> > > > > We will want to clearly define the scope of execveat(AT_CHECK)
> > > >
> > > > The line between data and code is blurry.  For instance, a configuration
> > > > file can impact the execution flow of a program.  So, where to draw the
> > > > line?
> > > >
> > > > It might makes sense to follow the kernel and interpreter semantic: if a
> > > > file can be executed by the kernel (e.g. ELF binary, file containing a
> > > > shebang, or just configured with binfmt_misc), then this should be
> > > > considered as executable code.  This applies to Bash, Python,
> > > > Javascript, NodeJS, PE, PHP...  However, we can also make a picture
> > > > executable with binfmt_misc.  So, again, where to draw the line?
> > > >
> > > > I'd recommend to think about interaction with the outside, through
> > > > function calls, IPCs, syscalls...  For instance, "running" an image
> > > > should not lead to reading or writing to arbitrary files, or accessing
> > > > the network, but in practice it is legitimate for some file formats...
> > > > PostScript is a programming language, but mostly used to draw pictures.
> > > > So, again, where to draw the line?
> > > >
> > > The javascript is run by browser and java code by java runtime, do
> > > they meet the criteria? they do not interact with the kernel directly,
> > > however they might have the same "executable" characteristics and the
> > > app might not want them to be put into non-exec mount.
> > >
> > > If the answer is yes, they can also use execveat(AT_CHECK),  the next
> > > question is: does it make sense for javacript/java code to go through
> > > execveat() code path, allocate bprm, etc ? (I don't have answer, maybe
> > > it is)
> >
> > Java and NodeJS can do arbitrary syscalls (through their runtime) and
> > they can access arbitrary files, so according to my below comment, yes
> > they should be managed as potentially dangerous executable code.
> >
> > The question should be: is this code trusted? Most of the time it is
> > not, hence the security model of web browser and their heavy use of
> > sandboxing.  So no, I don't think it would make sense to check this kind
> > of code more than what the browser already do.
> >
> 
> If I understand you correctly, Java/NodeJS won't use
> execveat(AT_CHECK), we will leave that work to the web browser/java
> runtime's sandboxer.
> This is good because the scope is more narrow/clear.

Yes for browser's sandboxes because the code comes from the network
(i.e. not authenticated at the kernel level, and mostly untrusted).

For standalone Java applications (stored in the filesystem), the Java
runtime(s) should be patched as other script interpreters.

> 
> Thanks
> -Jeff
> 
> > I'll talk about this use case in the next patch series.
> >
> > >
> > > > We should follow the principle of least astonishment.  What most users
> > > > would expect?  This should follow the *common usage* of executable
> > > > files.  At the end, the script interpreters will be patched by security
> > > > folks for security reasons.  I think the right question to ask should
> > > > be: could this file format be (ab)used to leak or modify arbitrary
> > > > files, or to perform arbitrary syscalls?  If the answer is yes, then it
> > > > should be checked for executability.  Of course, this excludes bugs
> > > > exploited in the file format parser.
> > > >
> > > > I'll extend the next patch series with this rationale.
> > > >
> > >

Powered by blists - more mailing lists