linux-kernel - Re: [PATCH v21 1/6] exec: Add a new AT_EXECVE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHC9VhT9sRXauYX+=21MUdOmfTZL4=sdGQsXojJjjTsdui+F_g@mail.gmail.com>
Date: Wed, 4 Dec 2024 22:33:34 -0500
From: Paul Moore <paul@...l-moore.com>
To: Mickaël Salaün <mic@...ikod.net>
Cc: Jeff Xu <jeffxu@...omium.org>, Al Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Kees Cook <keescook@...omium.org>, 
	Serge Hallyn <serge@...lyn.com>, Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>, 
	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>, 
	Elliott Hughes <enh@...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>, Jeff Xu <jeffxu@...gle.com>, Jonathan Corbet <corbet@....net>, 
	Jordan R Abrahams <ajordanr@...gle.com>, Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, 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>, 
	"Theodore Ts'o" <tytso@....edu>, 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, Eric Paris <eparis@...hat.com>, 
	audit@...r.kernel.org
Subject: Re: [PATCH v21 1/6] exec: Add a new AT_EXECVE_CHECK flag to execveat(2)

On Thu, Nov 21, 2024 at 8:40 AM Mickaël Salaün <mic@...ikod.net> wrote:
> On Wed, Nov 20, 2024 at 08:06:07AM -0800, Jeff Xu wrote:
> > On Wed, Nov 20, 2024 at 1:42 AM Mickaël Salaün <mic@...ikod.net> wrote:
> > > On Tue, Nov 19, 2024 at 05:17:00PM -0800, Jeff Xu wrote:
> > > > On Tue, Nov 12, 2024 at 11:22 AM Mickaël Salaün <mic@...ikod.net> wrote:

...

> > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > > > index cd57053b4a69..8d9ba5600cf2 100644
> > > > > --- a/kernel/auditsc.c
> > > > > +++ b/kernel/auditsc.c
> > > > > @@ -2662,6 +2662,7 @@ void __audit_bprm(struct linux_binprm *bprm)
> > > > >
> > > > >         context->type = AUDIT_EXECVE;
> > > > >         context->execve.argc = bprm->argc;
> > > > > +       context->execve.is_check = bprm->is_check;
> > > >
> > > > Where is execve.is_check used ?
> > >
> > > It is used in bprm_execve(), exposed to the audit framework, and
> > > potentially used by LSMs.
> > >
> > bprm_execve() uses bprm->is_check, not  the context->execve.is_check.
>
> Correct, this is only for audit but not used yet.
>
> Paul, Eric, do you want me to remove this field, leave it, or extend
> this patch like this?
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 8d9ba5600cf2..12cf89fa224a 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1290,6 +1290,8 @@ static void audit_log_execve_info(struct audit_context *context,
>                 }
>         } while (arg < context->execve.argc);
>
> +       audit_log_format(*ab, " check=%d", context->execve.is_check);
> +
>         /* NOTE: the caller handles the final audit_log_end() call */
>
>  out:

I would prefer to drop the audit changes rather than add a new field
to the audit record at this point in time.  Once we have a better
understanding of how things are actually being deployed by distros,
providers, and admins we can make a more reasoned decision on what we
should audit with respect to AT_EXECVE_CHECK.

Beyond that it looks okay to me from a LSM and audit perspective, so
feel free to add my ACK once you've dropped the audit bits.

Acked-by: Paul Moore <paul@...l-moore.com>

-- 
paul-moore.com