Re: [PATCH v23 7/8] samples/check-exec: Add an enlighten "inc" interpreter and 28 tests

[Nathan Chancellor <nathan@kernel.org>]

Hi Mickaël,

On Thu, Dec 12, 2024 at 06:42:22PM +0100, Mickaël Salaün wrote:
> Add a very simple script interpreter called "inc" that can evaluate two
> different commands (one per line):
> - "?" to initialize a counter from user's input;
> - "+" to increment the counter (which is set to 0 by default).
> 
> It is enlighten to only interpret executable files according to
> AT_EXECVE_CHECK and the related securebits:
> 
>   # Executing a script with RESTRICT_FILE is only allowed if the script
>   # is executable:
>   ./set-exec -f -- ./inc script-exec.inc # Allowed
>   ./set-exec -f -- ./inc script-noexec.inc # Denied
> 
>   # Executing stdin with DENY_INTERACTIVE is only allowed if stdin is an
>   # executable regular file:
>   ./set-exec -i -- ./inc -i < script-exec.inc # Allowed
>   ./set-exec -i -- ./inc -i < script-noexec.inc # Denied
> 
>   # However, a pipe is not executable and it is then denied:
>   cat script-noexec.inc | ./set-exec -i -- ./inc -i # Denied
> 
>   # Executing raw data (e.g. command argument) with DENY_INTERACTIVE is
>   # always denied.
>   ./set-exec -i -- ./inc -c "+" # Denied
>   ./inc -c "$(<script-ask.inc)" # Allowed
> 
>   # To directly execute a script, we can update $PATH (used by `env`):
>   PATH="${PATH}:." ./script-exec.inc
> 
>   # To execute several commands passed as argument:
> 
> Add a complete test suite to check the script interpreter against all
> possible execution cases:
> 
>   make TARGETS=exec kselftest-install
>   ./tools/testing/selftests/kselftest_install/run_kselftest.sh
> 
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Serge Hallyn <serge@hallyn.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Link: https://lore.kernel.org/r/20241212174223.389435-8-mic@digikod.net
...
> diff --git a/samples/check-exec/inc.c b/samples/check-exec/inc.c
> new file mode 100644
> index 000000000000..94b87569d2a2
> --- /dev/null
> +++ b/samples/check-exec/inc.c
...
> +/* 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);
> +	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);
> +}

The use of execveat() in this test case breaks the build when glibc is
less than 2.34, as that is the earliest version that has the execveat()
wrapper:

https://sourceware.org/git/?p=glibc.git;a=commit;h=19d83270fcd993cc349570164e21b06d57036704

  $ ldd --version | head -1
  ldd (Debian GLIBC 2.31-13+deb11u11) 2.31

  $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- mrproper allmodconfig samples/
  ...
  samples/check-exec/inc.c:81:8: error: call to undeclared function 'execveat'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     81 |         err = execveat(fileno(script), "", script_argv, envp,
        |               ^
  samples/check-exec/inc.c:81:8: note: did you mean 'execve'?
  /usr/include/unistd.h:551:12: note: 'execve' declared here
    551 | extern int execve (const char *__path, char *const __argv[],
        |            ^
  1 error generated.
  ...

Should this just use the syscall directly?

Cheers,
Nathan