| Message ID | 20230131174245.2343342-3-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [ima-evm-utils,v2] Add tests for MMAP_CHECK and MMAP_CHECK_REQPROT hooks | expand |
On 1/31/23 12:42, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > +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... > + > + 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) > +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 > +++ 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? Stefan
On Tue, 2023-01-31 at 18:42 +0100, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > Add tests to ensure that, after applying the kernel patch 'ima: Align > ima_file_mmap() parameters with mmap_file LSM hook', the MMAP_CHECK hook > checks the protections applied by the kernel and not those requested by the > application. > > Also ensure that after applying 'ima: Introduce MMAP_CHECK_REQPROT hook', > the MMAP_CHECK_REQPROT hook checks the protections requested by the > application. > > Test both with the test_mmap application that by default requests the > PROT_READ protection flag. Its syntax is: > > test_mmap <file> <mode> > > where mode can be: > - exec: adds the PROT_EXEC protection flag to mmap() > - read_implies_exec: calls the personality() system call with > READ_IMPLIES_EXEC as the first argument before mmap() > - mprotect: adds the PROT_EXEC protection flag to a memory area in addition > to PROT_READ > - exec_on_writable: calls mmap() with PROT_EXEC on a file which has a > writable mapping > > Check the different combinations of hooks/modes and ensure that a > measurement entry is found in the IMA measurement list only when it is > expected. No measurement entry should be found when only the PROT_READ > protection flag is requested or the matching policy rule has the > MMAP_CHECK_REQPROT hook and the personality() system call was called with > READ_IMPLIES_EXEC. > > mprotect() with PROT_EXEC on an existing memory area protected with > PROT_READ should be denied (with an appraisal rule), regardless of the MMAP > hook specified in the policy. The same applies for mmap() with PROT_EXEC on > a file with a writable mapping. > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Nice! Including some comments, or at least the test assumption, would help simplify reviewing the code. Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Hi Roberto, > +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" > + > + 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 Using temporary files should prevent having multiple records. Having a verbose option to show the actual matching measurement list record would be nice. > + > + echo "found" > + return "$OK" > +} > + > + > +# Run in the new environment if TST_ENV is set. > +_run_env "$TST_KERNEL" "$PWD/$(basename "$0")" "TST_ENV=$TST_ENV TST_KERNEL=$TST_KERNEL PATH=$PATH LD_LIBRARY_PATH=$LD_LIBRARY_PATH VERBOSE=$VERBOSE" > + > +# Exit from the creator of the new environment. > +_exit_env "$TST_KERNEL" > + > +# Mount filesystems in the new environment. > +_init_env > + > +if [ "$(whoami)" != "root" ]; then > + echo "${CYAN}This script must be executed as root${NORM}" > + exit "$SKIP" > +fi > + > +if [ ! -f /sys/kernel/security/ima/policy ]; then > + echo "${CYAN}IMA policy file not found${NORM}" > + exit "$SKIP" > +fi > + > +if ! cat /sys/kernel/security/ima/policy &> /dev/null; then > + echo "${CYAN}IMA policy file is not readable${NORM}" > + exit "$SKIP" > +fi An existing policy with an mmap rule would affect this test. Check to see if one already exists. > + > +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 >
On Tue, 2023-01-31 at 19:00 -0500, Mimi Zohar wrote: > On Tue, 2023-01-31 at 18:42 +0100, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > Add tests to ensure that, after applying the kernel patch 'ima: Align > > ima_file_mmap() parameters with mmap_file LSM hook', the MMAP_CHECK hook > > checks the protections applied by the kernel and not those requested by the > > application. > > > > Also ensure that after applying 'ima: Introduce MMAP_CHECK_REQPROT hook', > > the MMAP_CHECK_REQPROT hook checks the protections requested by the > > application. > > > > Test both with the test_mmap application that by default requests the > > PROT_READ protection flag. Its syntax is: > > > > test_mmap <file> <mode> > > > > where mode can be: > > - exec: adds the PROT_EXEC protection flag to mmap() > > - read_implies_exec: calls the personality() system call with > > READ_IMPLIES_EXEC as the first argument before mmap() > > - mprotect: adds the PROT_EXEC protection flag to a memory area in addition > > to PROT_READ > > - exec_on_writable: calls mmap() with PROT_EXEC on a file which has a > > writable mapping > > > > Check the different combinations of hooks/modes and ensure that a > > measurement entry is found in the IMA measurement list only when it is > > expected. No measurement entry should be found when only the PROT_READ > > protection flag is requested or the matching policy rule has the > > MMAP_CHECK_REQPROT hook and the personality() system call was called with > > READ_IMPLIES_EXEC. > > > > mprotect() with PROT_EXEC on an existing memory area protected with > > PROT_READ should be denied (with an appraisal rule), regardless of the MMAP > > hook specified in the policy. The same applies for mmap() with PROT_EXEC on > > a file with a writable mapping. > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > Nice! Including some comments, or at least the test assumption, would > help simplify reviewing the code. An example of a test assumption, or background information, for the "mprotect" test is described in the ima_file_mprotect() function comment: * Files can be mmap'ed read/write and later changed to execute to circumvent * IMA's mmap appraisal policy rules. Due to locking issues (mmap semaphore * would be taken before i_mutex), files can not be measured or appraised at * this point. Eliminate this integrity gap by denying the mprotect * PROT_EXECUTE change, if an mmap appraise policy rule exists.
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@huawei.com> > > > > +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
On Wed, 2023-02-01 at 08:48 -0500, Mimi Zohar wrote: > On Tue, 2023-01-31 at 19:00 -0500, Mimi Zohar wrote: > > On Tue, 2023-01-31 at 18:42 +0100, Roberto Sassu wrote: > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > Add tests to ensure that, after applying the kernel patch 'ima: Align > > > ima_file_mmap() parameters with mmap_file LSM hook', the MMAP_CHECK hook > > > checks the protections applied by the kernel and not those requested by the > > > application. > > > > > > Also ensure that after applying 'ima: Introduce MMAP_CHECK_REQPROT hook', > > > the MMAP_CHECK_REQPROT hook checks the protections requested by the > > > application. > > > > > > Test both with the test_mmap application that by default requests the > > > PROT_READ protection flag. Its syntax is: > > > > > > test_mmap <file> <mode> > > > > > > where mode can be: > > > - exec: adds the PROT_EXEC protection flag to mmap() > > > - read_implies_exec: calls the personality() system call with > > > READ_IMPLIES_EXEC as the first argument before mmap() > > > - mprotect: adds the PROT_EXEC protection flag to a memory area in addition > > > to PROT_READ > > > - exec_on_writable: calls mmap() with PROT_EXEC on a file which has a > > > writable mapping > > > > > > Check the different combinations of hooks/modes and ensure that a > > > measurement entry is found in the IMA measurement list only when it is > > > expected. No measurement entry should be found when only the PROT_READ > > > protection flag is requested or the matching policy rule has the > > > MMAP_CHECK_REQPROT hook and the personality() system call was called with > > > READ_IMPLIES_EXEC. > > > > > > mprotect() with PROT_EXEC on an existing memory area protected with > > > PROT_READ should be denied (with an appraisal rule), regardless of the MMAP > > > hook specified in the policy. The same applies for mmap() with PROT_EXEC on > > > a file with a writable mapping. > > > > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > > > Nice! Including some comments, or at least the test assumption, would > > help simplify reviewing the code. > > An example of a test assumption, or background information, for the > "mprotect" test is described in the ima_file_mprotect() function > comment: > > * Files can be mmap'ed read/write and later changed to execute to circumvent > * IMA's mmap appraisal policy rules. Due to locking issues (mmap semaphore > * would be taken before i_mutex), files can not be measured or appraised at > * this point. Eliminate this integrity gap by denying the mprotect > * PROT_EXECUTE change, if an mmap appraise policy rule exists. Ok, I added the description to each test. Will send the new patch shortly. Roberto
On Tue, 2023-01-31 at 22:02 -0500, Mimi Zohar wrote: > Hi Roberto, > > > +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" > > + > > + 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 > > Using temporary files should prevent having multiple records. Having a > verbose option to show the actual matching measurement list record > would be nice. > > > + > > + echo "found" > > + return "$OK" > > +} > > + > > > > + > > +# Run in the new environment if TST_ENV is set. > > +_run_env "$TST_KERNEL" "$PWD/$(basename "$0")" "TST_ENV=$TST_ENV TST_KERNEL=$TST_KERNEL PATH=$PATH LD_LIBRARY_PATH=$LD_LIBRARY_PATH VERBOSE=$VERBOSE" > > + > > +# Exit from the creator of the new environment. > > +_exit_env "$TST_KERNEL" > > + > > +# Mount filesystems in the new environment. > > +_init_env > > + > > +if [ "$(whoami)" != "root" ]; then > > + echo "${CYAN}This script must be executed as root${NORM}" > > + exit "$SKIP" > > +fi > > + > > +if [ ! -f /sys/kernel/security/ima/policy ]; then > > + echo "${CYAN}IMA policy file not found${NORM}" > > + exit "$SKIP" > > +fi > > + > > +if ! cat /sys/kernel/security/ima/policy &> /dev/null; then > > + echo "${CYAN}IMA policy file is not readable${NORM}" > > + exit "$SKIP" > > +fi > > An existing policy with an mmap rule would affect this test. Check to > see if one already exists. Not sure about this. We are specifying the filesystem UUID and the file owner. Should be enough to avoid interferences. Also check_load_ima_rule() avoids duplicated rules. Roberto > > + > > +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 > >
On Wed, 2023-02-01 at 18:29 +0100, Roberto Sassu wrote: > On Tue, 2023-01-31 at 22:02 -0500, Mimi Zohar wrote: > > Hi Roberto, > > > > > +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" > > > + > > > + 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 > > > > Using temporary files should prevent having multiple records. Having a > > verbose option to show the actual matching measurement list record > > would be nice. > > > > > + > > > + echo "found" > > > + return "$OK" > > > +} > > > + > > > > > > > + > > > +# Run in the new environment if TST_ENV is set. > > > +_run_env "$TST_KERNEL" "$PWD/$(basename "$0")" "TST_ENV=$TST_ENV TST_KERNEL=$TST_KERNEL PATH=$PATH LD_LIBRARY_PATH=$LD_LIBRARY_PATH VERBOSE=$VERBOSE" > > > + > > > +# Exit from the creator of the new environment. > > > +_exit_env "$TST_KERNEL" > > > + > > > +# Mount filesystems in the new environment. > > > +_init_env > > > + > > > +if [ "$(whoami)" != "root" ]; then > > > + echo "${CYAN}This script must be executed as root${NORM}" > > > + exit "$SKIP" > > > +fi > > > + > > > +if [ ! -f /sys/kernel/security/ima/policy ]; then > > > + echo "${CYAN}IMA policy file not found${NORM}" > > > + exit "$SKIP" > > > +fi > > > + > > > +if ! cat /sys/kernel/security/ima/policy &> /dev/null; then > > > + echo "${CYAN}IMA policy file is not readable${NORM}" > > > + exit "$SKIP" > > > +fi > > > > An existing policy with an mmap rule would affect this test. Check to > > see if one already exists. > > Not sure about this. We are specifying the filesystem UUID and the file > owner. Should be enough to avoid interferences. Also > check_load_ima_rule() avoids duplicated rules. Since the policy rules are walked sequentially, existing mmap rules without the uuid or owner qualifiers will interfere with the test policy rules. To see the problem load a custom policy containing an mmap rule, without any qualifiers. Then run the mmap_check test. > > > > + > > > +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 > > > >
diff --git a/tests/Makefile.am b/tests/Makefile.am index a0463b7b5b5d..ca9c4ca18380 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -2,7 +2,9 @@ check_SCRIPTS = TESTS = $(check_SCRIPTS) check_SCRIPTS += ima_hash.test sign_verify.test boot_aggregate.test \ - fsverity.test portable_signatures.test + fsverity.test portable_signatures.test mmap_check.test + +check_PROGRAMS := test_mmap .PHONY: check_logs check_logs: diff --git a/tests/mmap_check.test b/tests/mmap_check.test new file mode 100755 index 000000000000..a045ee867437 --- /dev/null +++ b/tests/mmap_check.test @@ -0,0 +1,284 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (C) 2022-2023 Roberto Sassu <roberto.sassu@huawei.com> +# +# Check the behavior of MMAP_CHECK and MMAP_CHECK_REQPROT + +trap '_report_exit_and_cleanup _cleanup_env cleanup' SIGINT SIGTERM SIGSEGV EXIT + +PATCHES=( +'ima: Align ima_file_mmap() parameters with mmap_file LSM hook' +'ima: Introduce MMAP_CHECK_REQPROT hook' +) + +# Base VERBOSE on the environment variable, if set. +VERBOSE="${VERBOSE:-0}" + +cd "$(dirname "$0")" || exit "$FAIL" +export PATH=$PWD/../src:$PWD:$PATH +export LD_LIBRARY_PATH=$LD_LIBRARY_PATH +. ./functions.sh +_require evmctl + +cleanup() { + if [ "$g_loop_mounted" = "1" ]; then + popd > /dev/null || exit "$FAIL" + umount "$g_mountpoint" + fi + + if [ -n "$g_dev" ]; then + losetup -d "$g_dev" + fi + + if [ -n "$g_image" ]; then + rm -f "$g_image" + fi + + if [ -n "$g_mountpoint" ]; then + rm -Rf "$g_mountpoint" + fi + + if [ -n "$key_path_der" ]; then + rm -f "$key_path_der" + fi +} + +# Use the fsuuid= IMA policy keyword to select only files created/used by the +# tests below. Also use fowner= to differentiate between files created/used by +# individual tests. +IMA_UUID="28b23254-9467-44c0-b6ba-34b12e85a26e" +MMAP_CHECK_FOWNER=2000 +MMAP_CHECK_REQPROT_FOWNER=2001 +MEASURE_MMAP_CHECK_RULE="measure func=MMAP_CHECK fsuuid=$IMA_UUID fowner=$MMAP_CHECK_FOWNER" +MEASURE_MMAP_CHECK_REQPROT_RULE="measure func=MMAP_CHECK_REQPROT fsuuid=$IMA_UUID fowner=$MMAP_CHECK_REQPROT_FOWNER" +APPRAISE_MMAP_CHECK_RULE="appraise func=MMAP_CHECK fsuuid=$IMA_UUID fowner=$MMAP_CHECK_FOWNER" +APPRAISE_MMAP_CHECK_REQPROT_RULE="appraise func=MMAP_CHECK_REQPROT fsuuid=$IMA_UUID fowner=$MMAP_CHECK_REQPROT_FOWNER" + +check_load_ima_rule() { + local rule_loaded result new_policy + + rule_loaded=$(grep "$1" /sys/kernel/security/ima/policy) + if [ -z "$rule_loaded" ]; then + new_policy=$(mktemp -p "$g_mountpoint") + echo "$1" > "$new_policy" + echo "$new_policy" > /sys/kernel/security/ima/policy + result=$? + rm -f "$new_policy" + + if [ "$result" -ne 0 ]; then + echo "${RED}Failed to set IMA policy${NORM}" + return "$HARDFAIL" + fi + fi + + return "$OK" +} + +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" + + 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" +} + +check_deny() { + local hook="$1" + local arg="$2" + local test_file fowner rule result + + 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 + + echo "test" > "$test_file" + + if ! evmctl ima_sign -a sha256 --key "$key_path" "$test_file" &> /dev/null; then + echo "${RED}Cannot sign $test_file${NORM}" + return "$HARDFAIL" + fi + + fowner="$MMAP_CHECK_FOWNER" + rule="$APPRAISE_MMAP_CHECK_RULE" + + if [ "$hook" = "MMAP_CHECK_REQPROT" ]; then + fowner="$MMAP_CHECK_REQPROT_FOWNER" + rule="$APPRAISE_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 + + if ! test_mmap "$test_file" exec > /dev/null; then + echo "${RED}Cannot read $test_file${NORM}" + return "$HARDFAIL" + fi + + if [ "$TFAIL" != "yes" ]; then + echo -n "Result (expect denied): " + else + echo -n "Result (expect allowed): " + fi + + if test_mmap "$test_file" "$arg"; then + echo "allowed" + return "$FAIL" + fi + + echo "denied" + return "$OK" +} + +# Run in the new environment if TST_ENV is set. +_run_env "$TST_KERNEL" "$PWD/$(basename "$0")" "TST_ENV=$TST_ENV TST_KERNEL=$TST_KERNEL PATH=$PATH LD_LIBRARY_PATH=$LD_LIBRARY_PATH VERBOSE=$VERBOSE" + +# Exit from the creator of the new environment. +_exit_env "$TST_KERNEL" + +# Mount filesystems in the new environment. +_init_env + +if [ "$(whoami)" != "root" ]; then + echo "${CYAN}This script must be executed as root${NORM}" + exit "$SKIP" +fi + +if [ ! -f /sys/kernel/security/ima/policy ]; then + echo "${CYAN}IMA policy file not found${NORM}" + exit "$SKIP" +fi + +if ! cat /sys/kernel/security/ima/policy &> /dev/null; then + echo "${CYAN}IMA policy file is not readable${NORM}" + exit "$SKIP" +fi + +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" +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) + +openssl x509 -in "$key_path" -out "$key_path_der" -outform der +if ! keyctl padd asymmetric pubkey %keyring:.ima < "$key_path_der" &> /dev/null; then + echo "${RED}Public key cannot be added to the IMA keyring${NORM}" + exit "$FAIL" +fi + +g_mountpoint=$(mktemp -d) +g_image=$(mktemp) + +if [ -z "$g_mountpoint" ]; then + echo "${RED}Mountpoint directory not created${NORM}" + exit "$FAIL" +fi + +if ! dd if=/dev/zero of="$g_image" bs=1M count=20 &> /dev/null; then + echo "${RED}Cannot create test image${NORM}" + exit "$FAIL" +fi + +g_dev=$(losetup -f "$g_image" --show) +if [ -z "$g_dev" ]; then + echo "${RED}Cannot create loop device${NORM}" + exit "$FAIL" +fi + +if ! mkfs.ext4 -U "$IMA_UUID" -b 4096 "$g_dev" &> /dev/null; then + echo "${RED}Cannot format $g_dev${NORM}" + exit "$FAIL" +fi + +if ! mount -o i_version "$g_dev" "$g_mountpoint"; then + echo "${RED}Cannot mount loop device${NORM}" + exit "$FAIL" +fi + +g_loop_mounted=1 +pushd "$g_mountpoint" > /dev/null || exit "$FAIL" + +expect_fail check_mmap "MMAP_CHECK" "" +expect_pass check_mmap "MMAP_CHECK" "exec" +expect_pass_if '0' check_mmap "MMAP_CHECK" "read_implies_exec" + +expect_fail_if '1' check_mmap "MMAP_CHECK_REQPROT" "" +expect_pass_if '1' check_mmap "MMAP_CHECK_REQPROT" "exec" +expect_fail_if '1' check_mmap "MMAP_CHECK_REQPROT" "read_implies_exec" + +expect_pass check_deny "MMAP_CHECK" "mprotect" +expect_pass_if '1' check_deny "MMAP_CHECK_REQPROT" "mprotect" + +expect_pass check_deny "MMAP_CHECK" "exec_on_writable" +expect_pass_if '1' check_deny "MMAP_CHECK_REQPROT" "exec_on_writable" diff --git a/tests/test_mmap.c b/tests/test_mmap.c new file mode 100644 index 000000000000..6d1beb3bb6a2 --- /dev/null +++ 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; +}