Message-ID: <CALmYWFsR+fWVuPLCoNhisTV5Lviv_dzb=5ajoKs0LaHsei9qTQ@mail.gmail.com>
Date: Fri, 19 Jul 2024 10:36:01 -0700
From: Jeff Xu <jeffxu@...gle.com>
To: Mickaël Salaün <mic@...ikod.net>
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 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.

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