| Message ID | 20210909165111.51038-2-alexh@vpitech.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IMA: Add tests for uid, gid, fowner, and fgroup options | expand |
Hi Alex, > Requires "ima: add gid support". I haven't test the patch yet, but LTP supports (unlike kselftest) various kernel versions. Thus there should be some check to prevent old kernels failing. You could certainly wrap new things with if tst_kvcmp. If there is a chance new functionality can be detected, we prefer it because various features are sometimes backported to enterprise distros' kernels. Also, adding new test ima_measurements02.sh with TST_MIN_KVER would also work, although for IMA tests I usually kept everything in a single file. ... > +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh > @@ -8,6 +8,7 @@ > TST_NEEDS_CMDS="awk cut sed" You should add sudo: TST_NEEDS_CMDS="awk cut sed sudo" > TST_SETUP="setup" > +TST_CLEANUP="cleanup" > TST_CNT=3 > TST_NEEDS_DEVICE=1 > @@ -20,6 +21,13 @@ setup() > TEST_FILE="$PWD/test.txt" > POLICY="$IMA_DIR/policy" > [ -f "$POLICY" ] || tst_res TINFO "not using default policy" > + > + cat $IMA_POLICY > policy-original This might not work if CONFIG_IMA_READ_POLICY is not set. There is check_policy_readable() helper in ima_setup.sh. Is it really needed anyway? > +} > + > +cleanup() > +{ > + cat policy-original > $IMA_POLICY Again, this will not work if CONFIG_IMA_WRITE_POLICY not set. And this is very likely not to be set. ... Kind regards, Petr
On Thu, 9 Sep 2021 22:21:22 +0200 Petr Vorel <pvorel@suse.cz> wrote: > > Requires "ima: add gid support". > I haven't test the patch yet, but LTP supports (unlike kselftest) various kernel > versions. Thus there should be some check to prevent old kernels failing. > You could certainly wrap new things with if tst_kvcmp. If there is a chance new > functionality can be detected, we prefer it because various features are > sometimes backported to enterprise distros' kernels. > > Also, adding new test ima_measurements02.sh with TST_MIN_KVER would also work, > although for IMA tests I usually kept everything in a single file. I'll add a tst_kvcmp check under the assumption that this feature will be added before Linux 5.15. > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh > > @@ -8,6 +8,7 @@ > > > TST_NEEDS_CMDS="awk cut sed" > You should add sudo: > > TST_NEEDS_CMDS="awk cut sed sudo" Will do. > > TST_SETUP="setup" > > +TST_CLEANUP="cleanup" > > TST_CNT=3 > > TST_NEEDS_DEVICE=1 > > > @@ -20,6 +21,13 @@ setup() > > TEST_FILE="$PWD/test.txt" > > POLICY="$IMA_DIR/policy" > > [ -f "$POLICY" ] || tst_res TINFO "not using default policy" > > + > > + cat $IMA_POLICY > policy-original > This might not work if CONFIG_IMA_READ_POLICY is not set. There is > check_policy_readable() helper in ima_setup.sh. Is it really needed anyway? It looks like CONFIG_IMA_WRITE_POLICY only makes it possible to add new rules at runtime, not remove them, so the cleanup code didn't actually work. I'll remove it. > > +} > > + > > +cleanup() > > +{ > > + cat policy-original > $IMA_POLICY > Again, this will not work if CONFIG_IMA_WRITE_POLICY not set. > And this is very likely not to be set. The new tests require the policy to be writable. I'll move the check_policy_writable function from ima_policy.sh to ima_setup.sh and use it in ima_measurements.sh as well. Thanks for the feedback, -Alex
Hi Alex, > On Thu, 9 Sep 2021 22:21:22 +0200 > Petr Vorel <pvorel@suse.cz> wrote: > > > Requires "ima: add gid support". > > I haven't test the patch yet, but LTP supports (unlike kselftest) various kernel > > versions. Thus there should be some check to prevent old kernels failing. > > You could certainly wrap new things with if tst_kvcmp. If there is a chance new > > functionality can be detected, we prefer it because various features are > > sometimes backported to enterprise distros' kernels. > > Also, adding new test ima_measurements02.sh with TST_MIN_KVER would also work, > > although for IMA tests I usually kept everything in a single file. > I'll add a tst_kvcmp check under the assumption that this feature will > be added before Linux 5.15. +1. Please let me know when you manage to get this mainlined (merged into Mimi's tree is enough), we should also add the commit hash of this feature. > > > +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh > > > @@ -8,6 +8,7 @@ > > > TST_NEEDS_CMDS="awk cut sed" > > You should add sudo: > > TST_NEEDS_CMDS="awk cut sed sudo" > Will do. +1 > > > TST_SETUP="setup" > > > +TST_CLEANUP="cleanup" > > > TST_CNT=3 > > > TST_NEEDS_DEVICE=1 > > > @@ -20,6 +21,13 @@ setup() > > > TEST_FILE="$PWD/test.txt" > > > POLICY="$IMA_DIR/policy" > > > [ -f "$POLICY" ] || tst_res TINFO "not using default policy" > > > + > > > + cat $IMA_POLICY > policy-original > > This might not work if CONFIG_IMA_READ_POLICY is not set. There is > > check_policy_readable() helper in ima_setup.sh. Is it really needed anyway? > It looks like CONFIG_IMA_WRITE_POLICY only makes it possible to add new > rules at runtime, not remove them, so the cleanup code didn't actually > work. I'll remove it. FYI I have on my TODO list loading policy before testing [1]. > > > +} > > > + > > > +cleanup() > > > +{ > > > + cat policy-original > $IMA_POLICY > > Again, this will not work if CONFIG_IMA_WRITE_POLICY not set. > > And this is very likely not to be set. > The new tests require the policy to be writable. I'll move the > check_policy_writable function from ima_policy.sh to ima_setup.sh and > use it in ima_measurements.sh as well. +1. FYI there is IMA specific README.md [2], in case anything needs to be updated. > Thanks for the feedback, yw. Thanks for taking care about testing! Kind regards, Petr > -Alex [1] https://github.com/linux-test-project/ltp/issues/720 [2] https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/security/integrity/ima/README.md
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh index 1927e937c..3c1bcbf88 100755 --- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh +++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh @@ -8,6 +8,7 @@ TST_NEEDS_CMDS="awk cut sed" TST_SETUP="setup" +TST_CLEANUP="cleanup" TST_CNT=3 TST_NEEDS_DEVICE=1 @@ -20,6 +21,13 @@ setup() TEST_FILE="$PWD/test.txt" POLICY="$IMA_DIR/policy" [ -f "$POLICY" ] || tst_res TINFO "not using default policy" + + cat $IMA_POLICY > policy-original +} + +cleanup() +{ + cat policy-original > $IMA_POLICY } ima_check() @@ -103,7 +111,7 @@ test3() local file="$dir/test.txt" # Default policy does not measure user files - tst_res TINFO "verify not measuring user files" + tst_res TINFO "verify not measuring user files by default" tst_check_cmds sudo || return if ! id $user >/dev/null 2>/dev/null; then @@ -116,9 +124,34 @@ test3() cd $dir # need to read file to get updated $ASCII_MEASUREMENTS sudo -n -u $user sh -c "echo $(date) user file > $file; cat $file > /dev/null" + EXPECT_FAIL "grep $file $ASCII_MEASUREMENTS" cd .. - EXPECT_FAIL "grep $file $ASCII_MEASUREMENTS" + tst_res TINFO "verify measuring user files when requested via uid" + ROD echo "measure uid=$(id -u $user)" \> $IMA_POLICY + ROD echo "$(date) uid test" \> $TEST_FILE + sudo -n -u $user sh -c "cat $TEST_FILE > /dev/null" + ima_check + + tst_res TINFO "verify measuring user files when requested via gid" + ROD echo "measure gid=$(id -g $user)" \> $IMA_POLICY + ROD echo "$(date) gid test" \> $TEST_FILE + sudo -n -u $user sh -c "cat $TEST_FILE > /dev/null" + ima_check + + tst_res TINFO "verify measuring user files when requested via fowner" + ROD echo "measure fowner=$(id -u $user)" \> $IMA_POLICY + ROD echo "$(date) fowner test" \> $TEST_FILE + chown $user $TEST_FILE + cat $TEST_FILE > /dev/null + ima_check + + tst_res TINFO "verify measuring user files when requested via fgroup" + ROD echo "measure fgroup=$(id -g $user)" \> $IMA_POLICY + ROD echo "$(date) fgroup test" \> $TEST_FILE + chgrp $(id -g $user) $TEST_FILE + cat $TEST_FILE > /dev/null + ima_check } tst_run
Requires "ima: add gid support". Signed-off-by: Alex Henrie <alexh@vpitech.com> --- .../integrity/ima/tests/ima_measurements.sh | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-)