[PATCH ltp v3 1/2] IMA: Move check_policy_writable to ima_setup.sh and rename it

[PATCH ltp v3 1/2] IMA: Move check_policy_writable to ima_setup.sh and rename it

[Alex Henrie]

Signed-off-by: Alex Henrie <alexh@vpitech.com>
---
 .../security/integrity/ima/tests/ima_policy.sh   | 16 +++-------------
 .../security/integrity/ima/tests/ima_setup.sh    | 10 ++++++++++
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 244cf081d..8924549df 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -11,19 +11,9 @@ TST_CNT=2
 
 . ima_setup.sh
 
-check_policy_writable()
-{
-	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
-
-	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
-	# CONFIG_IMA_READ_POLICY
-	echo "" 2> log > $IMA_POLICY
-	grep -q "Device or resource busy" log && tst_brk TCONF "$err"
-}
-
 setup()
 {
-	check_policy_writable
+	require_policy_writable
 
 	VALID_POLICY="$TST_DATAROOT/measure.policy"
 	[ -f $VALID_POLICY ] || tst_brk TCONF "missing $VALID_POLICY"
@@ -55,7 +45,7 @@ test1()
 
 	local p1
 
-	check_policy_writable
+	require_policy_writable
 	load_policy $INVALID_POLICY & p1=$!
 	wait "$p1"
 	if [ $? -ne 0 ]; then
@@ -71,7 +61,7 @@ test2()
 
 	local p1 p2 rc1 rc2
 
-	check_policy_writable
+	require_policy_writable
 	load_policy $VALID_POLICY & p1=$!
 	load_policy $VALID_POLICY & p2=$!
 	wait "$p1"; rc1=$?
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index 565f0bc3e..9c25d634d 100644
--- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -73,6 +73,16 @@ require_policy_readable()
 	fi
 }
 
+require_policy_writable()
+{
+	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
+
+	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
+	# CONFIG_IMA_READ_POLICY
+	echo "" 2> log > $IMA_POLICY
+	grep -q "Device or resource busy" log && tst_brk TCONF "$err"
+}
+
 check_ima_policy_content()
 {
 	local pattern="$1"
-- 
2.33.0

[PATCH ltp v3 2/2] IMA: Add tests for uid, gid, fowner, and fgroup options

[Alex Henrie]

Requires "ima: add gid support".

Signed-off-by: Alex Henrie <alexh@vpitech.com>
---
v3:
- Put new tests in their own function
- Don't require sudo or CONFIG_IMA_READ_POLICY=y for all tests
- Increase kernel version requirement for new tests to 5.16
- Delete test file and recreate it with correct ownership for each test
---
 .../integrity/ima/tests/ima_measurements.sh   | 49 ++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
index 1927e937c..5d22d12d3 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_measurements.sh
@@ -8,7 +8,7 @@
 
 TST_NEEDS_CMDS="awk cut sed"
 TST_SETUP="setup"
-TST_CNT=3
+TST_CNT=4
 TST_NEEDS_DEVICE=1
 
 . ima_setup.sh
@@ -103,7 +103,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
@@ -121,4 +121,49 @@ test3()
 	EXPECT_FAIL "grep $file $ASCII_MEASUREMENTS"
 }
 
+test4()
+{
+	local user="nobody"
+
+	tst_check_cmds chgrp chown sg sudo || return
+
+	# try to write to the policy, then check whether it can be written again
+	cat $IMA_POLICY > $IMA_POLICY 2> /dev/null
+	require_policy_writable
+
+	ROD rm -f $TEST_FILE
+	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
+
+	ROD rm -f $TEST_FILE
+	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
+
+	if tst_kvcmp -lt 5.16; then
+		tst_brk TCONF "gid and fgroup options require kernel 5.16 or newer"
+	fi
+
+	ROD rm -f $TEST_FILE
+	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 sg $user "sh -c 'cat $TEST_FILE > /dev/null'"
+	ima_check
+
+	ROD rm -f $TEST_FILE
+	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 $user $TEST_FILE
+	cat $TEST_FILE > /dev/null
+	ima_check
+}
+
 tst_run
-- 
2.33.0

Re: [PATCH ltp v3 2/2] IMA: Add tests for uid, gid, fowner, and fgroup options

[Petr Vorel]

Hi Alex,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

Re: [PATCH ltp v3 1/2] IMA: Move check_policy_writable to ima_setup.sh and rename it

[Petr Vorel]

Hi Alex,

...
> --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh

As it's now a generally used function I'll add a comment:

# Because we don't grep kernel config for CONFIG_IMA_WRITE_POLICY, we just try
# to write empty string (invalid), thus policy must be repeatedly checked.
# Because after first write to policy policy will be removed on systems without
# CONFIG_IMA_WRITE_POLICY.
> +require_policy_writable()
> +{
> +	local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
> +
> +	[ -f $IMA_POLICY ] || tst_brk TCONF "$err"
> +	# CONFIG_IMA_READ_POLICY
> +	echo "" 2> log > $IMA_POLICY
> +	grep -q "Device or resource busy" log && tst_brk TCONF "$err"
> +}
> +

Kind regards,
Petr

Re: [PATCH ltp v3 2/2] IMA: Add tests for uid, gid, fowner, and fgroup options

[Petr Vorel]

Hi Alex,

> +test4()
> +{
> +	local user="nobody"
> +
> +	tst_check_cmds chgrp chown sg sudo || return
> +
> +	# try to write to the policy, then check whether it can be written again
> +	cat $IMA_POLICY > $IMA_POLICY 2> /dev/null
This is not needed because require_policy_writable call

> +	require_policy_writable
> +
> +	ROD rm -f $TEST_FILE
> +	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
> +
As I noted at first patch unfortunately we need another require_policy_writable
call here.  Because we don't grep kernel config for CONFIG_IMA_WRITE_POLICY,
we just try to write empty string (invalid), thus policy must be repeatedly
checked (see ima_policy.sh). Because after first write to policy (ROD echo
"measure uid=$(id -u $user)" \> $IMA_POLICY) policy will be removed on systems
without CONFIG_IMA_WRITE_POLICY.

> +	ROD rm -f $TEST_FILE
> +	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
> +
> +	if tst_kvcmp -lt 5.16; then
> +		tst_brk TCONF "gid and fgroup options require kernel 5.16 or newer"
> +	fi
> +
> +	ROD rm -f $TEST_FILE
> +	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 sg $user "sh -c 'cat $TEST_FILE > /dev/null'"
> +	ima_check
> +
> +	ROD rm -f $TEST_FILE
> +	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 $user $TEST_FILE
> +	cat $TEST_FILE > /dev/null
> +	ima_check
> +}
> +
>  tst_run

I still have 2 concerns about this patch (sorry that I didn't realise it before)

1) repeated running of ima_measurements.sh is broken:
ima_measurements 1 TINFO: verify adding record to the IMA measurement list
ima_measurements 1 TBROK: failed to get algorithm/digest for '/tmp/LTP_ima_measurements.6WSmgkHZ13/test.txt': measurement record not found

First run:
ima_measurements 1 TINFO: timeout per run is 0h 5m 0s
ima_measurements 1 TINFO: IMA kernel config:
ima_measurements 1 TINFO: CONFIG_IMA=y
ima_measurements 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
ima_measurements 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_measurements 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
ima_measurements 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
ima_measurements 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
ima_measurements 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
ima_measurements 1 TINFO: CONFIG_IMA_READ_POLICY=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
ima_measurements 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_measurements 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
ima_measurements 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
ima_measurements 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-5.3.18-54-default root=UUID=478230c4-ef04-4f7e-ad47-733fd1f28a76 ima_policy=tcb splash=silent video=1024x768 plymouth.ignore-serial-consoles console=ttyS0 console=tty softlockup_panic=1 kernel.softlockup_panic=1 mitigations=auto ignore_loglevel
ima_measurements 1 TINFO: verify adding record to the IMA measurement list
ima_measurements 1 TINFO: computing digest for sha256 algorithm
ima_measurements 1 TPASS: correct digest found
ima_measurements 2 TINFO: verify updating record in the IMA measurement list
ima_measurements 2 TINFO: computing digest for sha256 algorithm
ima_measurements 2 TPASS: correct digest found
ima_measurements 3 TINFO: verify not measuring user files by default
ima_measurements 3 TPASS: grep /tmp/LTP_ima_measurements.ma8YpOrvRS/user/test.txt /sys/kernel/security/ima/ascii_runtime_measurements failed as expected
ima_measurements 4 TINFO: verify measuring user files when requested via uid
ima_measurements 4 TINFO: computing digest for sha256 algorithm
ima_measurements 4 TPASS: correct digest found
ima_measurements 4 TCONF: IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)

Summary:
passed   4
failed   0
broken   0
skipped  1
warnings 0

Repeated run:
ima_measurements 1 TINFO: timeout per run is 0h 5m 0s
ima_measurements 1 TINFO: IMA kernel config:
ima_measurements 1 TINFO: CONFIG_IMA=y
ima_measurements 1 TINFO: CONFIG_IMA_MEASURE_PCR_IDX=10
ima_measurements 1 TINFO: CONFIG_IMA_LSM_RULES=y
ima_measurements 1 TINFO: CONFIG_IMA_NG_TEMPLATE=y
ima_measurements 1 TINFO: CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
ima_measurements 1 TINFO: CONFIG_IMA_DEFAULT_HASH_SHA256=y
ima_measurements 1 TINFO: CONFIG_IMA_DEFAULT_HASH="sha256"
ima_measurements 1 TINFO: CONFIG_IMA_READ_POLICY=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE_BOOTPARAM=y
ima_measurements 1 TINFO: CONFIG_IMA_APPRAISE_MODSIG=y
ima_measurements 1 TINFO: CONFIG_IMA_TRUSTED_KEYRING=y
ima_measurements 1 TINFO: CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY=y
ima_measurements 1 TINFO: CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS=y
ima_measurements 1 TINFO: /proc/cmdline: BOOT_IMAGE=/boot/vmlinuz-5.3.18-54-default root=UUID=478230c4-ef04-4f7e-ad47-733fd1f28a76 ima_policy=tcb splash=silent video=1024x768 plymouth.ignore-serial-consoles console=ttyS0 console=tty softlockup_panic=1 kernel.softlockup_panic=1 mitigations=auto ignore_loglevel
ima_measurements 1 TINFO: verify adding record to the IMA measurement list
ima_measurements 1 TBROK: failed to get algorithm/digest for '/tmp/LTP_ima_measurements.6WSmgkHZ13/test.txt': measurement record not found
ima_measurements 1 TINFO: computing digest for  algorithm
ima_measurements 1 TCONF: cannot compute digest for  algorithm
ima_measurements 1 TINFO: AppArmor enabled, this may affect test results
ima_measurements 1 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ima_measurements 1 TINFO: loaded AppArmor profiles: none

I need to have closer look what the problem is. It can be fixed by checking
policy with require_policy_writable in the setup. But that's problem for missing
coverage (TCONF for first 3 tests).

2) writing policy in ima_measurement.sh leads to TCONF ima_policy.sh on systems
without CONFIG_IMA_WRITE_POLICY. This has has no easy solution. Either we manage
to load each policy for each overlay [1] or add support for reboot to LTP [2] and
restart after each test or just some functionality will be skipped due policy
disappear after writting.

But still IMHO it'd be better to separate test4() into it's own file to not
affect the original 3 tests in ima_measurement.sh (unless we fix the problem
with repeated running of ima_measurement.sh).

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/issues/720
[2] https://github.com/linux-test-project/ltp/issues/868