On Tue, 2023-01-31 at 17:39 -0500, Stefan Berger wrote:
>
> On 1/31/23 12:42, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> >
> > +check_mmap() {
> > + local hook="$1"
> > + local arg="$2"
> > + local test_file fowner rule result test_file_entry
> > +
> > + echo -e "\nTest: ${FUNCNAME[0]} (hook=\"$hook\", test_mmap arg: \"$arg\")"
> > +
> > + if ! test_file=$(mktemp -p "$PWD"); then
> > + echo "${RED}Cannot write $test_file${NORM}"
> > + return "$HARDFAIL"
> > + fi
> > +
> > + fowner="$MMAP_CHECK_FOWNER"
> > + rule="$MEASURE_MMAP_CHECK_RULE"
> > +
> > + if [ "$hook" = "MMAP_CHECK_REQPROT" ]; then
> > + fowner="$MMAP_CHECK_REQPROT_FOWNER"
> > + rule="$MEASURE_MMAP_CHECK_REQPROT_RULE"
> > + fi
> > +
> > + if ! chown "$fowner" "$test_file"; then
> > + echo "${RED}Cannot change owner of $test_file${NORM}"
> > + return "$HARDFAIL"
> > + fi
> > +
> > + check_load_ima_rule "$rule"
> > + result=$?
> > + if [ $result -ne "$OK" ]; then
> > + return $result
> > + fi
> > +
> > + test_mmap "$test_file" "$arg"
>
> In this case it should succeed or fail depending on the $rule? I am just wondering whether to check $? here as well for expected outcome...
I agree. For the check_mmap() test, test_mmap is always expected to
succeed.
> > +
> > + if [ "$TFAIL" != "yes" ]; then
> > + echo -n "Result (expect found): "
> > + else
> > + echo -n "Result (expect not found): "
> > + fi
> > +
> > + test_file_entry=$(awk '$5 == "'"$test_file"'"' < /sys/kernel/security/ima/ascii_runtime_measurements)
> > + if [ -z "$test_file_entry" ]; then
> > + echo "not found"
> > + return "$FAIL"
> > + fi
> > +
> > + echo "found"
> > + return "$OK"
> > +}
> > +if [ -n "$TST_KEY_PATH" ]; then
> > + if [ "${TST_KEY_PATH:0:1}" != "/" ]; then
> > + echo "${RED}Absolute path required for the signing key${NORM}"
> > + exit "$FAIL"
> > + fi
> > +
> > + if [ ! -f "$TST_KEY_PATH" ]; then
> > + echo "${RED}Kernel signing key not found in $TST_KEY_PATH${NORM}"
> > + exit "$FAIL"
> > + fi
> > +
> > + key_path="$TST_KEY_PATH"
>
> g_key_path ? or pass as parameter to check_deny (better IMO)
There are other global variables. Also the expect_ lines now are clean
and say more or less what the test is about. Maybe better g_key_path,
will think about for new tests.
> > +elif [ -f "$PWD/../signing_key.pem" ]; then
> > + key_path="$PWD/../signing_key.pem"
> > +elif [ -f "/lib/modules/$(uname -r)/source/certs/signing_key.pem" ]; then
> > + key_path="/lib/modules/$(uname -r)/source/certs/signing_key.pem"
> > +elif [ -f "/lib/modules/$(uname -r)/build/certs/signing_key.pem" ]; then
> > + key_path="/lib/modules/$(uname -r)/build/certs/signing_key.pem"
> > +else
> > + echo "${CYAN}Kernel signing key not found${NORM}"
> > + exit "$SKIP"
> > +fi
> > +
> > +key_path_der=$(mktemp)
>
> g_key_path_der for consistency
Ok.
> > +++ b/tests/test_mmap.c
> > @@ -0,0 +1,75 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
> > + *
> > + * Tool to test IMA MMAP_CHECK and MMAP_CHECK_REQPROT hooks.
> > + */
> > +#include <stdio.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <sys/stat.h>
> > +#include <sys/mman.h>
> > +#include <sys/personality.h>
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + struct stat st;
> > + void *ptr, *ptr_write = NULL;
> > + int ret, fd, fd_write, prot = PROT_READ;
> > +
> > + if (!argv[1])
> > + return -ENOENT;
> > +
> > + if (argv[2] && !strcmp(argv[2], "read_implies_exec")) {
> > + ret = personality(READ_IMPLIES_EXEC);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + if (stat(argv[1], &st) == -1)
> > + return -errno;
> > +
> > + if (argv[2] && !strcmp(argv[2], "exec_on_writable")) {
> > + fd_write = open(argv[1], O_RDWR);
> > + if (fd_write == -1)
> > + return -errno;
> > +
> > + ptr_write = mmap(0, st.st_size, PROT_WRITE, MAP_SHARED,
> > + fd_write, 0);
> > + close(fd_write);
> > +
> > + if (ptr_write == (void *)-1)
> > + return -errno;
> > + }
> > +
> > + fd = open(argv[1], O_RDONLY);
> > + if (fd == -1) {
> > + if (ptr_write)
> > + munmap(ptr_write, st.st_size);
> > +
> > + return -errno;
> > + }
> > +
> > + if (argv[2] && !strncmp(argv[2], "exec", 4))
> > + prot |= PROT_EXEC;
> > +
> > + ptr = mmap(0, st.st_size, prot, MAP_PRIVATE, fd, 0);
> > +
> > + close(fd);
> > +
> > + if (ptr_write)
> > + munmap(ptr_write, st.st_size);
> > +
> > + if (ptr == (void *)-1)
> > + return -errno;
> > +
> > + ret = 0;
> > +
> > + if (argv[2] && !strcmp(argv[2], "mprotect"))
> > + ret = mprotect(ptr, st.st_size, PROT_EXEC);
> > +
> > + munmap(ptr, st.st_size);
> > + return ret;
> > +}
>
> Are there any unexpected failure cases here where it should report an error to the user?
Uhm, ok. I differentiated when an error could occur from when it should
not.
Roberto
