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

[Mimi Zohar <zohar@linux.ibm.com>]

On Wed, 2024-11-27 at 13:10 +0100, Mickaël Salaün wrote:
> 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.

Sent as an RFC.  The audit message is only updated for the missing signature
case.  However, all of the audit messages in ima_appraise_measurement() should
be updated.  The current method doesn't scale.

Mimi

> > 
> > 
> > > > 
> > > > > +	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);
> > > > > +}
> > > > > +
> > > > 
> > > > 
> > > 
> > 
> > 
>