[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[Petr Vorel]

Hi Mimi, Lakshmi,

changes v2->v3:
fixed regression in my third commit.
(please verify it on installed LTP, or at least run make install in
testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

Kind regards,
Petr

Lachlan Sneff (1):
  IMA: Add a test to verify measurement of certificate imported into a
    keyring

Petr Vorel (3):
  IMA/ima_keys.sh: Fix policy content check usage
  IMA: Refactor datafiles directory
  IMA/ima_keys.sh: Enhance policy checks

 .../kernel/security/integrity/ima/README.md   |  12 +-
 .../security/integrity/ima/datafiles/Makefile |  10 +-
 .../ima/datafiles/ima_kexec/Makefile          |  11 ++
 .../datafiles/{ => ima_kexec}/kexec.policy    |   0
 .../integrity/ima/datafiles/ima_keys/Makefile |  11 ++
 .../datafiles/{ => ima_keys}/keycheck.policy  |   2 +-
 .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
 .../ima/datafiles/ima_policy/Makefile         |  11 ++
 .../datafiles/{ => ima_policy}/measure.policy |   0
 .../{ => ima_policy}/measure.policy-invalid   |   0
 .../security/integrity/ima/tests/ima_keys.sh  | 104 +++++++++++++++---
 11 files changed, 133 insertions(+), 28 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_kexec}/kexec.policy (100%)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_keys}/keycheck.policy (59%)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy (100%)
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy-invalid (100%)

-- 
2.28.0

[LTP] [PATCH v3 1/4] IMA/ima_keys.sh: Fix policy content check usage

[Petr Vorel]

require_ima_policy_content cannot be used in subshell $() evaluation,
because tst_brk does not quit the test. It calls cleanup for the
subshell process and main process then continue:

ima_keys 1 TCONF: IMA policy does not specify 'func=KEY_CHECK'
=> Here it's running first cleanup. umount errors are because parent
shell process still has $PWD in directory to be unmounted:
umount: /tmp/LTP_ima_keys.0dIVrwJKIG/mntpoint: target is busy.
ima_keys 1 TINFO: umount(/dev/loop0) failed, try 1 ...
ima_keys 1 TINFO: Likely gvfsd-trash is probing newly mounted  fs, kill it to speed up tests.
umount: /tmp/LTP_ima_keys.0dIVrwJKIG/mntpoint: target is busy.
...
ima_keys 1 TINFO: umount(/dev/loop0) failed, try 50 ...
ima_keys 1 TINFO: Likely gvfsd-trash is probing newly mounted  fs, kill it to speed up tests.
ima_keys 1 TWARN: Failed to umount(/dev/loop0) after 50 retries
tst_device.c:222: WARN: ioctl(/dev/loop0, LOOP_CLR_FD, 0) no ENXIO for too long

Usage: tst_device acquire [size [filename]]
   or: tst_device release /path/to/device

ima_keys 1 TWARN: Failed to release device '/dev/loop0'
rm: cannot remove '/tmp/LTP_ima_keys.0dIVrwJKIG/mntpoint': Device or resource busy
ima_keys 1 TINFO: AppArmor enabled, this may affect test results
ima_keys 1 TINFO: it can be disabled with TST_DISABLE_APPARMOR=1 (requires super/root)
ima_keys 1 TINFO: loaded AppArmor profiles: none
/opt/ltp/testcases/bin/ima_keys.sh: line 25:  6166 Terminated              sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9 -$pid  (wd: ~)

=> Here it should quit after running cleanup, but instead continue running:
ima_keys 1 TCONF: ima policy does not specify a keyrings to check

Fixes: f20f44d72 ("IMA/ima_keys.sh: Fix policy readability check")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as v2.

 testcases/kernel/security/integrity/ima/tests/ima_keys.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index 3aea26056..53c289054 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -16,11 +16,14 @@ TST_NEEDS_DEVICE=1
 # (450d0fd51564 - "IMA: Call workqueue functions to measure queued keys")
 test1()
 {
-	local keyrings keycheck_lines keycheck_line templates test_file="file.txt"
+	local keyrings keycheck_lines keycheck_line templates
+	local pattern="func=KEY_CHECK"
+	local test_file="file.txt"
 
 	tst_res TINFO "verifying key measurement for keyrings and templates specified in IMA policy file"
 
-	keycheck_lines=$(require_ima_policy_content "func=KEY_CHECK" "")
+	require_ima_policy_content "$pattern"
+	keycheck_lines=$(check_ima_policy_content "$pattern" "")
 	keycheck_line=$(echo "$keycheck_lines" | grep "keyrings" | head -n1)
 
 	if [ -z "$keycheck_line" ]; then
-- 
2.28.0

[LTP] [PATCH v3 2/4] IMA: Refactor datafiles directory

[Petr Vorel]

The IMA datafiles directory is structured so that it cannot be directly
expanded to include datafiles for tests other than `ima_policy.sh`
as it's installed into /opt/ltp/testcases/data/ima_policy.

Also not all policies are meant to be for ima_policy.sh, thus
move policies into their own directories based on the test which they
belong to. Rename policy directory to ima_policy to follow the
pattern that directory in sources match the installed directory.

Reported-by: Lachlan Sneff <t-josne@linux.microsoft.com>
Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com>
[ pvorel: based on Lachlan's patch, rewritten ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as v2.

 .../kernel/security/integrity/ima/datafiles/Makefile  | 10 +++++-----
 .../integrity/ima/datafiles/ima_kexec/Makefile        | 11 +++++++++++
 .../ima/datafiles/{ => ima_kexec}/kexec.policy        |  0
 .../integrity/ima/datafiles/ima_keys/Makefile         | 11 +++++++++++
 .../ima/datafiles/{ => ima_keys}/keycheck.policy      |  0
 .../integrity/ima/datafiles/ima_policy/Makefile       | 11 +++++++++++
 .../ima/datafiles/{ => ima_policy}/measure.policy     |  0
 .../datafiles/{ => ima_policy}/measure.policy-invalid |  0
 8 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_kexec}/kexec.policy (100%)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_keys}/keycheck.policy (100%)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy (100%)
 rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy-invalid (100%)

diff --git a/testcases/kernel/security/integrity/ima/datafiles/Makefile b/testcases/kernel/security/integrity/ima/datafiles/Makefile
index 369407112..6857ccfee 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/Makefile
+++ b/testcases/kernel/security/integrity/ima/datafiles/Makefile
@@ -1,6 +1,8 @@
 #
 #    testcases/kernel/security/integrity/ima/policy testcases Makefile.
 #
+#    Copyright (c) Linux Test Project, 2019-2020
+#    Copyright (c) 2020 Microsoft Corporation
 #    Copyright (C) 2009, Cisco Systems Inc.
 #
 #    This program is free software; you can redistribute it and/or modify
@@ -20,12 +22,10 @@
 # Ngie Cooper, July 2009
 #
 
-top_srcdir		?= ../../../../../..
+top_srcdir	?= ../../../../../..
 
 include	$(top_srcdir)/include/mk/env_pre.mk
 
-INSTALL_DIR		:= testcases/data/ima_policy
+SUBDIRS	:= ima_*
 
-INSTALL_TARGETS		:= measure.policy-invalid *.policy
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
+include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile b/testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
new file mode 100644
index 000000000..5e0d632a7
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) Linux Test Project, 2020
+
+top_srcdir	?= ../../../../../../..
+
+include	$(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR		:= testcases/data/ima_kexec
+INSTALL_TARGETS	:= *.policy
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/kexec.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_kexec/kexec.policy
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/kexec.policy
rename to testcases/kernel/security/integrity/ima/datafiles/ima_kexec/kexec.policy
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
new file mode 100644
index 000000000..452321843
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) Linux Test Project, 2020
+
+top_srcdir	?= ../../../../../../..
+
+include	$(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR		:= testcases/data/ima_keys
+INSTALL_TARGETS	:= *.policy
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/keycheck.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/keycheck.policy
rename to testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile b/testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
new file mode 100644
index 000000000..953e21556
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) Linux Test Project, 2020
+
+top_srcdir	?= ../../../../../../..
+
+include	$(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR		:= testcases/data/ima_policy
+INSTALL_TARGETS	:= *.policy-invalid *.policy
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_policy/measure.policy
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/measure.policy
rename to testcases/kernel/security/integrity/ima/datafiles/ima_policy/measure.policy
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/ima_policy/measure.policy-invalid
similarity index 100%
rename from testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
rename to testcases/kernel/security/integrity/ima/datafiles/ima_policy/measure.policy-invalid
-- 
2.28.0

[LTP] [PATCH v3 3/4] IMA: Add a test to verify measurement of certificate imported into a keyring

[Petr Vorel]

From: Lachlan Sneff <t-josne@linux.microsoft.com>

The IMA subsystem supports measuring certificates that have been
imported into either system built-in or user-defined keyrings.
A test to verify measurement of a certificate imported
into a keyring is required.

Add an IMA measurement test that verifies that an x509 certificate
can be imported into a newly-created, user-defined keyring and measured
correctly by the IMA subsystem.

A certificate used by the test is included in the `datafiles/keys`
directory.

There can be restrictions on importing a certificate into a builtin
trusted keyring. For example, the `.ima` keyring requires that
imported certs be signed by a kernel private key in certain
kernel configurations. For this reason, this test defines
a user-defined keyring and imports a certificate into that.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Lachlan Sneff <t-josne@linux.microsoft.com>
[ pvorel: Added key_import_test into keycheck.policy, reword
instructions in README.md, LTP API related fixes ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as v2.

 .../kernel/security/integrity/ima/README.md   |  12 ++--
 .../integrity/ima/datafiles/ima_keys/Makefile |   2 +-
 .../ima/datafiles/ima_keys/keycheck.policy    |   2 +-
 .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
 .../security/integrity/ima/tests/ima_keys.sh  |  68 +++++++++++++++---
 5 files changed, 70 insertions(+), 14 deletions(-)
 create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der

diff --git a/testcases/kernel/security/integrity/ima/README.md b/testcases/kernel/security/integrity/ima/README.md
index 392e1e868..68d046678 100644
--- a/testcases/kernel/security/integrity/ima/README.md
+++ b/testcases/kernel/security/integrity/ima/README.md
@@ -16,11 +16,15 @@ space, may contain equivalent measurement tcb rules, detecting them would
 require `IMA_READ_POLICY=y` therefore ignore this option.
 
 ### IMA key test
-`ima_keys.sh` requires a readable IMA policy, as well as a loaded policy
-with `func=KEY_CHECK keyrings=...`, see example in `keycheck.policy`.
+The measuring keys test (first test) in `ima_keys.sh` requires a readable IMA
+policy, as well as a loaded measure policy with `func=KEY_CHECK keyrings=...`.
 
-As well as what's required for the IMA tests, the following are also required
--in the kernel configuration:
+The certificate import test (second test) require measure policy with
+`func=KEY_CHECK keyrings=key_import_test`. Valid policy for both is in
+`keycheck.policy`.
+
+As well as what's required for the IMA tests, key tests require reading the IMA
+policy allowed in the kernel configuration:
 ```
 CONFIG_IMA_READ_POLICY=y
 ```
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
index 452321843..ac7ce33ab 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
@@ -6,6 +6,6 @@ top_srcdir	?= ../../../../../../..
 include	$(top_srcdir)/include/mk/env_pre.mk
 
 INSTALL_DIR		:= testcases/data/ima_keys
-INSTALL_TARGETS	:= *.policy
+INSTALL_TARGETS	:= *.policy x509_ima.der
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
index 3f1934a3d..623162002 100644
--- a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
+++ b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/keycheck.policy
@@ -1 +1 @@
-measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist template=ima-buf
+measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
diff --git a/testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der b/testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
new file mode 100644
index 0000000000000000000000000000000000000000..92be058da22adffa9d6b6e51efa0c737ebbbbdcd
GIT binary patch
literal 650
zcmXqLVrnyJVtl`VnTe5!NhJD#vj69`9|BBf8}FEsx@^_9$Clp>c-c6$+C196^D;7W
zvoaV27z!HjvoVLVaPe?t<QJFZCFZ6YN*hRmgqV4R$}{p4b2Al+Gt=`j^U@WvQ!5SS
z3}oO&a59SVLzFncG#ki?^BP(jSQr@@7#Ud_7)6Qm8W{k&hEOgIY;2s5>?=lA2Ij_I
z27|^<rp88wchfeduxmMW^j9qUxkIugeev4q7u7yrJR_rW$*!>VOo=tim8DK0r^FsU
zl)K`}`+CO4@4KBkoLmcj?fH`%wbDvU<d=4Z>6-SMf6Eh}{&)1rdsOoNQ-1fgBQ1t{
zVTqGwuK95LlFE)6i{@=vlP6!2`Y}x<BF&oXU_nlDxy@C+CNp&=W=00a#jys_20XwZ
zl@(@W{LjK<z+k`);_<VvFf*|?7|4P+d@N!tBCNWX-0#?!UAx9s`mgFmW+nI2#6kmk
zkhC(3gn?Lt$m4W@56wQ)?QVKW<nOtpT)HJrB?Q^`z&K?FdV8b(y8m)~mOOvswu^9m
z-o7mOwCAzaT*~`Y4wxFtmNA_89`W;j{r#iww*5OC5vgEziqD_%=ki$*`;s}`Pfwi{
zbotZ0X`ckHOmaVLm85n@E9hN_K;~PUB>DFAzh(;(UoMln9V>g;W#-LUGS7A`nYQKY
WBem{7L5JThS+;$vggi%(*E;~nlJ80Y

literal 0
HcmV?d00001

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index 53c289054..30950904e 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -6,8 +6,8 @@
 #
 # Verify that keys are measured correctly based on policy.
 
-TST_NEEDS_CMDS="cut grep sed tr xxd"
-TST_CNT=1
+TST_NEEDS_CMDS="cmp cut grep sed tr xxd"
+TST_CNT=2
 TST_NEEDS_DEVICE=1
 
 . ima_setup.sh
@@ -20,20 +20,22 @@ test1()
 	local pattern="func=KEY_CHECK"
 	local test_file="file.txt"
 
-	tst_res TINFO "verifying key measurement for keyrings and templates specified in IMA policy file"
+	tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy"
 
 	require_ima_policy_content "$pattern"
 	keycheck_lines=$(check_ima_policy_content "$pattern" "")
 	keycheck_line=$(echo "$keycheck_lines" | grep "keyrings" | head -n1)
 
 	if [ -z "$keycheck_line" ]; then
-		tst_brk TCONF "ima policy does not specify a keyrings to check"
+		tst_res TCONF "IMA policy does not specify a keyrings to check"
+		return
 	fi
 
 	keyrings=$(echo "$keycheck_line" | tr " " "\n" | grep "keyrings" | \
 		sed "s/\./\\\./g" | cut -d'=' -f2)
 	if [ -z "$keyrings" ]; then
-		tst_brk TCONF "ima policy has a keyring key-value specifier, but no specified keyrings"
+		tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings"
+		return
 	fi
 
 	templates=$(echo "$keycheck_line" | tr " " "\n" | grep "template" | \
@@ -49,11 +51,13 @@ test1()
 
 		echo "$line" | cut -d' ' -f6 | xxd -r -p > $test_file
 
-		expected_digest="$(compute_digest $algorithm $test_file)" || \
-			tst_brk TCONF "cannot compute digest for $algorithm"
+		if ! expected_digest="$(compute_digest $algorithm $test_file)"; then
+			tst_res TCONF "cannot compute digest for $algorithm"
+			return
+		fi
 
 		if [ "$digest" != "$expected_digest" ]; then
-			tst_res TFAIL "incorrect digest was found for the ($keyring) keyring"
+			tst_res TFAIL "incorrect digest was found for $keyring keyring"
 			return
 		fi
 	done
@@ -61,4 +65,52 @@ test1()
 	tst_res TPASS "specified keyrings were measured correctly"
 }
 
+# Create a new keyring, import a certificate into it, and verify
+# that the certificate is measured correctly by IMA.
+test2()
+{
+	tst_require_cmds evmctl keyctl openssl
+
+	local cert_file="$TST_DATAROOT/x509_ima.der"
+	local keyring_name="key_import_test"
+	local temp_file="file.txt"
+	local keyring_id
+
+	tst_res TINFO "verify measurement of certificate imported into a keyring"
+
+	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
+		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
+	fi
+
+	keyctl new_session > /dev/null
+
+	keyring_id=$(keyctl newring $keyring_name @s) || \
+		tst_brk TBROK "unable to create a new keyring"
+
+	tst_is_num $keyring_id || \
+		tst_brk TBROK "unable to parse the new keyring id"
+
+	evmctl import $cert_file $keyring_id > /dev/null || \
+		tst_brk TBROK "unable to import a certificate into $keyring_name keyring"
+
+	grep $keyring_name $ASCII_MEASUREMENTS | tail -n1 | cut -d' ' -f6 | \
+		xxd -r -p > $temp_file
+
+	if [ ! -s $temp_file ]; then
+		tst_res TFAIL "keyring $keyring_name not found in $ASCII_MEASUREMENTS"
+		return
+	fi
+
+	if ! openssl x509 -in $temp_file -inform der > /dev/null; then
+		tst_res TFAIL "logged certificate is not a valid x509 certificate"
+		return
+	fi
+
+	if cmp -s $temp_file $cert_file; then
+		tst_res TPASS "logged certificate matches the original"
+	else
+		tst_res TFAIL "logged certificate does not match original"
+	fi
+}
+
 tst_run
-- 
2.28.0

[LTP] [PATCH v3 4/4] IMA/ima_keys.sh: Enhance policy checks

[Petr Vorel]

Reuse policy check code.

Also check for all policy keyrings and templates.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v2->v3:
Fixed regression in my third commit.
Verified on:

cat /sys/kernel/security/ima/policy
measure func=KEY_CHECK keyrings=.ima|.builtin_trusted_keys template=ima-buf
measure func=KEY_CHECK keyrings=key_import_test template=ima-buf

 .../security/integrity/ima/tests/ima_keys.sh  | 49 ++++++++++++-------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
index 30950904e..ce15296fc 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_keys.sh
@@ -6,40 +6,54 @@
 #
 # Verify that keys are measured correctly based on policy.
 
-TST_NEEDS_CMDS="cmp cut grep sed tr xxd"
+TST_NEEDS_CMDS="cmp cut grep sed sort xxd"
 TST_CNT=2
 TST_NEEDS_DEVICE=1
+TST_SETUP="setup"
 
 . ima_setup.sh
 
+KEYCHECK_POLICY='func=KEY_CHECK'
+
+setup()
+{
+	require_ima_policy_content "^measure.*$KEYCHECK_POLICY"
+}
+
+check_keys_policy()
+{
+	local pattern="$1"
+
+	pattern="^measure.*($KEYCHECK_POLICY.*$pattern|$pattern.*$KEYCHECK_POLICY)"
+	if ! check_ima_policy_content "$pattern" '-E'; then
+		tst_res TCONF "IMA policy does not specify '$pattern'"
+		return 1
+	fi
+	return 0
+}
+
+
 # Based on https://lkml.org/lkml/2019/12/13/564.
 # (450d0fd51564 - "IMA: Call workqueue functions to measure queued keys")
 test1()
 {
-	local keyrings keycheck_lines keycheck_line templates
-	local pattern="func=KEY_CHECK"
+	local keycheck_lines i keyrings templates
+	local pattern='keyrings=[^[:space:]]+'
 	local test_file="file.txt"
 
 	tst_res TINFO "verify key measurement for keyrings and templates specified in IMA policy"
 
-	require_ima_policy_content "$pattern"
-	keycheck_lines=$(check_ima_policy_content "$pattern" "")
-	keycheck_line=$(echo "$keycheck_lines" | grep "keyrings" | head -n1)
+	keycheck_lines=$(check_keys_policy "$pattern") || return
 
-	if [ -z "$keycheck_line" ]; then
-		tst_res TCONF "IMA policy does not specify a keyrings to check"
-		return
-	fi
-
-	keyrings=$(echo "$keycheck_line" | tr " " "\n" | grep "keyrings" | \
-		sed "s/\./\\\./g" | cut -d'=' -f2)
+	keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \
+		sed "s/\./\\\./g" | cut -d'=' -f2; done | sort -u)
 	if [ -z "$keyrings" ]; then
 		tst_res TCONF "IMA policy has a keyring key-value specifier, but no specified keyrings"
 		return
 	fi
 
-	templates=$(echo "$keycheck_line" | tr " " "\n" | grep "template" | \
-		cut -d'=' -f2)
+	templates=$(for i in $keycheck_lines; do echo "$i" | grep "template" | \
+		cut -d'=' -f2; done | sort -u)
 
 	grep -E "($templates)*($keyrings)" $ASCII_MEASUREMENTS | while read line
 	do
@@ -73,14 +87,13 @@ test2()
 
 	local cert_file="$TST_DATAROOT/x509_ima.der"
 	local keyring_name="key_import_test"
+	local pattern="keyrings=[^[:space:]]*$keyring_name"
 	local temp_file="file.txt"
 	local keyring_id
 
 	tst_res TINFO "verify measurement of certificate imported into a keyring"
 
-	if ! check_ima_policy_content "^measure.*func=KEY_CHECK.*keyrings=.*$keyring_name"; then
-		tst_brk TCONF "IMA policy does not contain $keyring_name keyring"
-	fi
+	check_keys_policy "$pattern" >/dev/null || return
 
 	keyctl new_session > /dev/null
 
-- 
2.28.0

[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[Lakshmi Ramasubramanian]

On 8/17/20 6:09 AM, Petr Vorel wrote:

Hi Petr,

> Hi Mimi, Lakshmi,
> 
> changes v2->v3:
> fixed regression in my third commit.
> (please verify it on installed LTP, or at least run make install in
> testcases/kernel/security/integrity/ima/datafiles/ima_keys/)
> 

Verified keys tests and also kexec tests. Thanks.

Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

> 
> Lachlan Sneff (1):
>    IMA: Add a test to verify measurement of certificate imported into a
>      keyring
> 
> Petr Vorel (3):
>    IMA/ima_keys.sh: Fix policy content check usage
>    IMA: Refactor datafiles directory
>    IMA/ima_keys.sh: Enhance policy checks
> 
>   .../kernel/security/integrity/ima/README.md   |  12 +-
>   .../security/integrity/ima/datafiles/Makefile |  10 +-
>   .../ima/datafiles/ima_kexec/Makefile          |  11 ++
>   .../datafiles/{ => ima_kexec}/kexec.policy    |   0
>   .../integrity/ima/datafiles/ima_keys/Makefile |  11 ++
>   .../datafiles/{ => ima_keys}/keycheck.policy  |   2 +-
>   .../ima/datafiles/ima_keys/x509_ima.der       | Bin 0 -> 650 bytes
>   .../ima/datafiles/ima_policy/Makefile         |  11 ++
>   .../datafiles/{ => ima_policy}/measure.policy |   0
>   .../{ => ima_policy}/measure.policy-invalid   |   0
>   .../security/integrity/ima/tests/ima_keys.sh  | 104 +++++++++++++++---
>   11 files changed, 133 insertions(+), 28 deletions(-)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_kexec/Makefile
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_kexec}/kexec.policy (100%)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/Makefile
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_keys}/keycheck.policy (59%)
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_keys/x509_ima.der
>   create mode 100644 testcases/kernel/security/integrity/ima/datafiles/ima_policy/Makefile
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy (100%)
>   rename testcases/kernel/security/integrity/ima/datafiles/{ => ima_policy}/measure.policy-invalid (100%)
> 

[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[Mimi Zohar]

On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
> Hi Mimi, Lakshmi,
> 
> changes v2->v3:
> fixed regression in my third commit.
> (please verify it on installed LTP, or at least run make install in
> testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

I did, but nothing changed.  I probably need to set an environment
variable.

After building and installing LTP, it's finding the file, but some of
the issues still exist:

ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
grep: Unmatched ( or \(
ima_keys 1 TPASS: specified keyrings were measured correctly
ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
keyctl_session_to_parent: Operation not permitted
ima_keys 2 TPASS: logged certificate matches the original

IMA policy:
measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf 
measure func=KEY_CHECK keyrings=key_import_test template=ima-buf 

Mimi

[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[Petr Vorel]

> On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
> > Hi Mimi, Lakshmi,

> > changes v2->v3:
> > fixed regression in my third commit.
> > (please verify it on installed LTP, or at least run make install in
> > testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

> I did, but nothing changed.  I probably need to set an environment
> variable.
You also need to set LTPROOT (prefix, e.g. /opt/ltp).

> After building and installing LTP, it's finding the file, but some of
> the issues still exist:

> ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
> grep: Unmatched ( or \(
This should be fixed by v3 (fixed by for loop and sort -u)
https://patchwork.ozlabs.org/project/ltp/patch/20200817130916.27634-5-pvorel@suse.cz/
But I'll test it tomorrow with your IMA policy.

Thank you for testing!

Kind regards,
Petr

> ima_keys 1 TPASS: specified keyrings were measured correctly
> ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
> keyctl_session_to_parent: Operation not permitted
> ima_keys 2 TPASS: logged certificate matches the original

> IMA policy:
> measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf 
> measure func=KEY_CHECK keyrings=key_import_test template=ima-buf 

> Mimi

[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[Lakshmi Ramasubramanian]

On 8/17/20 12:18 PM, Mimi Zohar wrote:

> On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
>> Hi Mimi, Lakshmi,
>>
>> changes v2->v3:
>> fixed regression in my third commit.
>> (please verify it on installed LTP, or at least run make install in
>> testcases/kernel/security/integrity/ima/datafiles/ima_keys/)
> 
> I did, but nothing changed.  I probably need to set an environment
> variable.
> 
> After building and installing LTP, it's finding the file, but some of
> the issues still exist:
> 
> ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
> grep: Unmatched ( or \(
> ima_keys 1 TPASS: specified keyrings were measured correctly
> ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
> keyctl_session_to_parent: Operation not permitted
> ima_keys 2 TPASS: logged certificate matches the original
> 
> IMA policy:
> measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> measure func=KEY_CHECK keyrings=key_import_test template=ima-buf
> 

I think I see the problem

keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \
		sed "s/\./\\\./g" | cut -d'=' -f2; done | sort -u

The above line generates the list of keyrings (read from the IMA policy) 
with a newline after the first policy entry with "keyrings=". Please see 
below:

ima_keys 1 TINFO: \.ima|\.builtin_trusted_keys
key_import_test

When this is checked in the "do-done" loop grep returns "mismatched (" 
due to the newline.

I tried with "(" removed from the following line and that fixes the problem:

grep -E "($templates)*($keyrings)" $ASCII_MEASUREMENTS | while read line

But a better fix might be to remove the "newline" in $keyrings. I'll try 
that.

Regarding the following error:
keyctl_session_to_parent: Operation not permitted

The following line in test2() can be removed. Not sure if this is needed.
	keyctl new_session > /dev/null

thanks,
  -lakshmi

[LTP] [PATCH v3 0/4] IMA: verify measurement of certificate imported into a keyring

[Petr Vorel]

> On 8/17/20 12:18 PM, Mimi Zohar wrote:

> > On Mon, 2020-08-17 at 15:09 +0200, Petr Vorel wrote:
> > > Hi Mimi, Lakshmi,

> > > changes v2->v3:
> > > fixed regression in my third commit.
> > > (please verify it on installed LTP, or at least run make install in
> > > testcases/kernel/security/integrity/ima/datafiles/ima_keys/)

> > I did, but nothing changed.  I probably need to set an environment
> > variable.

> > After building and installing LTP, it's finding the file, but some of
> > the issues still exist:

> > ima_keys 1 TINFO: $TMPDIR is on tmpfs => run on loop device
> > ima_keys 1 TINFO: Formatting /dev/loop0 with ext3 extra opts=''
> > ima_keys 1 TINFO: verify key measurement for keyrings and templates specified in IMA policy
> > grep: Unmatched ( or \(
> > ima_keys 1 TPASS: specified keyrings were measured correctly
> > ima_keys 2 TINFO: verify measurement of certificate imported into a keyring
> > keyctl_session_to_parent: Operation not permitted
> > ima_keys 2 TPASS: logged certificate matches the original

> > IMA policy:
> > measure func=KEY_CHECK keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist|key_import_test template=ima-buf
> > measure func=KEY_CHECK keyrings=key_import_test template=ima-buf


> I think I see the problem

> keyrings=$(for i in $keycheck_lines; do echo "$i" | grep "keyrings" | \
> 		sed "s/\./\\\./g" | cut -d'=' -f2; done | sort -u

> The above line generates the list of keyrings (read from the IMA policy)
> with a newline after the first policy entry with "keyrings=". Please see
> below:

> ima_keys 1 TINFO: \.ima|\.builtin_trusted_keys
> key_import_test

> When this is checked in the "do-done" loop grep returns "mismatched (" due
> to the newline.

> I tried with "(" removed from the following line and that fixes the problem:

> grep -E "($templates)*($keyrings)" $ASCII_MEASUREMENTS | while read line

> But a better fix might be to remove the "newline" in $keyrings. I'll try
> that.
OK, I was too aggressive when removing tr dependency. I'll add another sed
command to remove new lines.

> Regarding the following error:
> keyctl_session_to_parent: Operation not permitted
I missed this problem.

> The following line in test2() can be removed. Not sure if this is needed.
> 	keyctl new_session > /dev/null
Not sure, I guess it'd work in existing session. But then it'd be good to
do cleanup.

> thanks,
>  -lakshmi

Kind regards,
Petr