linux-kernel - Re: [PATCH v21 6/6] samples/check-exec: Add an enlighten "inc" interpreter and 28 tests

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241127.Ob8DaeR9xaul@digikod.net>
Date: Wed, 27 Nov 2024 13:10:01 +0100
From: Mickaël Salaün <mic@...ikod.net>
To: Mimi Zohar <zohar@...ux.ibm.com>
Cc: Al Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Kees Cook <keescook@...omium.org>, 
	Paul Moore <paul@...l-moore.com>, 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>, 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
Subject: Re: [PATCH v21 6/6] samples/check-exec: Add an enlighten "inc"
 interpreter and 28 tests

On Tue, Nov 26, 2024 at 12:41:45PM -0500, Mimi Zohar wrote:
> On Fri, 2024-11-22 at 15:50 +0100, Mickaël Salaün wrote:
> > On Thu, Nov 21, 2024 at 03:34:47PM -0500, Mimi Zohar wrote:
> > > Hi Mickaël,
> > > 
> > > On Tue, 2024-11-12 at 20:18 +0100, Mickaël Salaün wrote:
> > > > 
> > > > +
> > > > +/* Returns 1 on error, 0 otherwise. */
> > > > +static int interpret_stream(FILE *script, char *const script_name,
> > > > +			    char *const *const envp, const bool restrict_stream)
> > > > +{
> > > > +	int err;
> > > > +	char *const script_argv[] = { script_name, NULL };
> > > > +	char buf[128] = {};
> > > > +	size_t buf_size = sizeof(buf);
> > > > +
> > > > +	/*
> > > > +	 * We pass a valid argv and envp to the kernel to emulate a native
> > > > +	 * script execution.  We must use the script file descriptor instead of
> > > > +	 * the script path name to avoid race conditions.
> > > > +	 */
> > > > +	err = execveat(fileno(script), "", script_argv, envp,
> > > > +		       AT_EMPTY_PATH | AT_EXECVE_CHECK);
> > > 
> > > At least with v20, the AT_CHECK always was being set, independent of whether
> > > set-exec.c set it.  I'll re-test with v21.
> > 
> > AT_EXECVE_CEHCK should always be set, only the interpretation of the
> > result should be relative to securebits.  This is highlighted in the
> > documentation.
> 
> Sure, that sounds correct.  With an IMA-appraisal policy, any unsigned script
> with the is_check flag set now emits an "cause=IMA-signature-required" audit
> message.  However since IMA-appraisal isn't enforcing file signatures, this
> sounds wrong.
> 
> New audit messages like "IMA-signature-required-by-interpreter" and "IMA-
> signature-not-required-by-interpreter" would need to be defined based on the
> SECBIT_EXEC_RESTRICT_FILE.

It makes sense.  Could you please send a patch for these
IMA-*-interpreter changes?  I'll include it in the next series.

> 
> 
> > > 
> > > > +	if (err && restrict_stream) {
> > > > +		perror("ERROR: Script execution check");
> > > > +		return 1;
> > > > +	}
> > > > +
> > > > +	/* Reads script. */
> > > > +	buf_size = fread(buf, 1, buf_size - 1, script);
> > > > +	return interpret_buffer(buf, buf_size);
> > > > +}
> > > > +
> > > 
> > > 
> > 
> 
>